Author: george.karpenkov Date: Wed Aug 22 16:17:25 2018 New Revision: 340475
URL: http://llvm.org/viewvc/llvm-project?rev=340475&view=rev Log: [analyzer] Track non-zero values in ReturnVisitor Tracking those can help to provide much better diagnostics in many cases. In general, most of the visitor machinery should be refactored to allow tracking the origin of arbitrary values. rdar://36039765 Differential Revision: https://reviews.llvm.org/D51131 Added: cfe/trunk/test/Analysis/diagnostics/track_subexpressions.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp cfe/trunk/test/Analysis/uninit-const.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=340475&r1=340474&r2=340475&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Wed Aug 22 16:17:25 2018 @@ -887,15 +887,6 @@ public: RetE = RetE->IgnoreParenCasts(); - // If we can't prove the return value is 0, just mark it interesting, and - // make sure to track it into any further inner functions. - if (!State->isNull(V).isConstrainedTrue()) { - BR.markInteresting(V); - ReturnVisitor::addVisitorIfNecessary(N, RetE, BR, - EnableNullFPSuppression); - return nullptr; - } - // If we're returning 0, we should track where that 0 came from. bugreporter::trackNullOrUndefValue(N, RetE, BR, /*IsArg*/ false, EnableNullFPSuppression); @@ -904,20 +895,33 @@ public: SmallString<64> Msg; llvm::raw_svector_ostream Out(Msg); - if (V.getAs<Loc>()) { - // If we have counter-suppression enabled, make sure we keep visiting - // future nodes. We want to emit a path note as well, in case - // the report is resurrected as valid later on. - if (EnableNullFPSuppression && - Options.shouldAvoidSuppressingNullArgumentPaths()) - Mode = MaybeUnsuppress; + if (State->isNull(V).isConstrainedTrue()) { + if (V.getAs<Loc>()) { + + // If we have counter-suppression enabled, make sure we keep visiting + // future nodes. We want to emit a path note as well, in case + // the report is resurrected as valid later on. + if (EnableNullFPSuppression && + Options.shouldAvoidSuppressingNullArgumentPaths()) + Mode = MaybeUnsuppress; + + if (RetE->getType()->isObjCObjectPointerType()) { + Out << "Returning nil"; + } else { + Out << "Returning null pointer"; + } + } else { + Out << "Returning zero"; + } - if (RetE->getType()->isObjCObjectPointerType()) - Out << "Returning nil"; - else - Out << "Returning null pointer"; } else { - Out << "Returning zero"; + if (auto CI = V.getAs<nonloc::ConcreteInt>()) { + Out << "Returning the value " << CI->getValue(); + } else if (V.getAs<Loc>()) { + Out << "Returning pointer"; + } else { + Out << "Returning value"; + } } if (LValue) { @@ -1262,10 +1266,9 @@ FindLastStoreBRVisitor::VisitNode(const InitE = InitE->IgnoreParenCasts(); bugreporter::trackNullOrUndefValue(StoreSite, InitE, BR, IsParam, EnableNullFPSuppression); - } else { - ReturnVisitor::addVisitorIfNecessary(StoreSite, InitE->IgnoreParenCasts(), - BR, EnableNullFPSuppression); } + ReturnVisitor::addVisitorIfNecessary(StoreSite, InitE->IgnoreParenCasts(), + BR, EnableNullFPSuppression); } // Okay, we've found the binding. Emit an appropriate message. Added: cfe/trunk/test/Analysis/diagnostics/track_subexpressions.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/diagnostics/track_subexpressions.cpp?rev=340475&view=auto ============================================================================== --- cfe/trunk/test/Analysis/diagnostics/track_subexpressions.cpp (added) +++ cfe/trunk/test/Analysis/diagnostics/track_subexpressions.cpp Wed Aug 22 16:17:25 2018 @@ -0,0 +1,19 @@ +// RUN: %clang_analyze_cc1 -x c++ -analyzer-checker=core -analyzer-output=text -verify %s + +typedef unsigned char uint8_t; + +#define UINT8_MAX 255 +#define TCP_MAXWIN 65535 + +uint8_t get_uint8_max() { + uint8_t rcvscale = UINT8_MAX; // expected-note{{'rcvscale' initialized to 255}} + return rcvscale; // expected-note{{Returning the value 255 (loaded from 'rcvscale')}} +} + +void shift_by_undefined_value() { + uint8_t shift_amount = get_uint8_max(); // expected-note{{'shift_amount' initialized to 255}} + // expected-note@-1{{Calling 'get_uint8_max'}} + // expected-note@-2{{Returning from 'get_uint8_max'}} + (void)(TCP_MAXWIN << shift_amount); // expected-warning{{The result of the left shift is undefined due to shifting by '255', which is greater or equal to the width of type 'int'}} + // expected-note@-1{{The result of the left shift is undefined due to shifting by '255', which is greater or equal to the width of type 'int'}} +} Modified: cfe/trunk/test/Analysis/uninit-const.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/uninit-const.cpp?rev=340475&r1=340474&r2=340475&view=diff ============================================================================== --- cfe/trunk/test/Analysis/uninit-const.cpp (original) +++ cfe/trunk/test/Analysis/uninit-const.cpp Wed Aug 22 16:17:25 2018 @@ -49,15 +49,17 @@ void f7(void) { int& f6_1_sub(int &p) { - return p; + return p; // expected-note{{Returning without writing to 'p'}} + // expected-note@-1{{Returning pointer (reference to 't')}} } void f6_1(void) { int t; // expected-note{{'t' declared without an initial value}} int p = f6_1_sub(t); //expected-warning {{Assigned value is garbage or undefined}} - //expected-note@-1 {{Calling 'f6_1_sub'}} - //expected-note@-2 {{Returning from 'f6_1_sub'}} - //expected-note@-3 {{Assigned value is garbage or undefined}} + //expected-note@-1 {{Passing value via 1st parameter 'p'}} + //expected-note@-2 {{Calling 'f6_1_sub'}} + //expected-note@-3 {{Returning from 'f6_1_sub'}} + //expected-note@-4 {{Assigned value is garbage or undefined}} int q = p; doStuff6(q); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
