Index: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -841,7 +841,7 @@ /// This visitor is intended to be used when another visitor discovers that an /// interesting value comes from an inlined function call. class ReturnVisitor : public BugReporterVisitor { - const StackFrameContext *StackFrame; + const StackFrameContext *CalleeSFC; enum { Initial, MaybeUnsuppress, @@ -853,10 +853,9 @@ AnalyzerOptions& Options; public: - ReturnVisitor(const StackFrameContext *Frame, - bool Suppressed, + ReturnVisitor(const StackFrameContext *Frame, bool Suppressed, AnalyzerOptions &Options) - : StackFrame(Frame), EnableNullFPSuppression(Suppressed), + : CalleeSFC(Frame), EnableNullFPSuppression(Suppressed), Options(Options) {} static void *getTag() { @@ -866,7 +865,7 @@ void Profile(llvm::FoldingSetNodeID &ID) const override { ID.AddPointer(ReturnVisitor::getTag()); - ID.AddPointer(StackFrame); + ID.AddPointer(CalleeSFC); ID.AddBoolean(EnableNullFPSuppression); } @@ -950,7 +949,6 @@ if (Optional RetLoc = RetVal.getAs()) EnableNullFPSuppression = State->isNull(*RetLoc).isConstrainedTrue(); - BR.markInteresting(CalleeContext); BR.addVisitor(llvm::make_unique(CalleeContext, EnableNullFPSuppression, Options)); @@ -960,7 +958,7 @@ BugReporterContext &BRC, BugReport &BR) { // Only print a message at the interesting return statement. - if (N->getLocationContext() != StackFrame) + if (N->getLocationContext() != CalleeSFC) return nullptr; Optional SP = N->getLocationAs(); @@ -974,7 +972,7 @@ // Okay, we're at the right return statement, but do we have the return // value available? ProgramStateRef State = N->getState(); - SVal V = State->getSVal(Ret, StackFrame); + SVal V = State->getSVal(Ret, CalleeSFC); if (V.isUnknownOrUndef()) return nullptr; @@ -1008,6 +1006,8 @@ SmallString<64> Msg; llvm::raw_svector_ostream Out(Msg); + bool WouldEventBeMeaningless = false; + if (State->isNull(V).isConstrainedTrue()) { if (V.getAs()) { @@ -1030,10 +1030,19 @@ } else { if (auto CI = V.getAs()) { Out << "Returning the value " << CI->getValue(); - } else if (V.getAs()) { - Out << "Returning pointer"; } else { - Out << "Returning value"; + // There is nothing interesting about returning a value, when it is + // plain value without any constraints, and the function is guaranteed + // to return that every time. We could use CFG::isLinear() here, but + // constexpr branches are obvious to the compiler, not necesserily to + // the programmer. + if (N->getCFG().size() == 3) + WouldEventBeMeaningless = true; + + if (V.getAs()) + Out << "Returning pointer"; + else + Out << "Returning value"; } } @@ -1052,11 +1061,20 @@ Out << " (loaded from '" << *DD << "')"; } - PathDiagnosticLocation L(Ret, BRC.getSourceManager(), StackFrame); + PathDiagnosticLocation L(Ret, BRC.getSourceManager(), CalleeSFC); if (!L.isValid() || !L.asLocation().isValid()) return nullptr; - return std::make_shared(L, Out.str()); + auto EventPiece = std::make_shared(L, Out.str()); + + // If we determined that the note is meaningless, make it prunable, and + // don't mark the stackframe interesting. + if (WouldEventBeMeaningless) + EventPiece->setPrunable(true); + else + BR.markInteresting(CalleeSFC); + + return EventPiece; } PathDiagnosticPieceRef visitNodeMaybeUnsuppress(const ExplodedNode *N, @@ -1071,7 +1089,7 @@ if (!CE) return nullptr; - if (CE->getCalleeContext() != StackFrame) + if (CE->getCalleeContext() != CalleeSFC) return nullptr; Mode = Satisfied; @@ -1083,7 +1101,7 @@ CallEventManager &CallMgr = StateMgr.getCallEventManager(); ProgramStateRef State = N->getState(); - CallEventRef<> Call = CallMgr.getCaller(StackFrame, State); + CallEventRef<> Call = CallMgr.getCaller(CalleeSFC, State); for (unsigned I = 0, E = Call->getNumArgs(); I != E; ++I) { Optional ArgV = Call->getArgSVal(I).getAs(); if (!ArgV) @@ -1126,7 +1144,7 @@ void finalizeVisitor(BugReporterContext &, const ExplodedNode *, BugReport &BR) override { if (EnableNullFPSuppression && ShouldInvalidate) - BR.markInvalid(ReturnVisitor::getTag(), StackFrame); + BR.markInvalid(ReturnVisitor::getTag(), CalleeSFC); } }; Index: cfe/trunk/test/Analysis/diagnostics/find_last_store.c =================================================================== --- cfe/trunk/test/Analysis/diagnostics/find_last_store.c +++ cfe/trunk/test/Analysis/diagnostics/find_last_store.c @@ -2,13 +2,11 @@ typedef struct { float b; } c; void *a(); void *d() { - return a(); // expected-note{{Returning pointer}} + return a(); } void no_find_last_store() { - c *e = d(); // expected-note{{Calling 'd'}} - // expected-note@-1{{Returning from 'd'}} - // expected-note@-2{{'e' initialized here}} + c *e = d(); // expected-note{{'e' initialized here}} (void)(e || e->b); // expected-note{{Assuming 'e' is null}} // expected-note@-1{{Left side of '||' is false}} Index: cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp =================================================================== --- cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp +++ cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp @@ -119,7 +119,7 @@ bool coin(); bool foo() { - return coin(); // tracking-note{{Returning value}} + return coin(); } int bar; @@ -127,12 +127,10 @@ void test() { int *x = 0; // expected-note{{'x' initialized to a null pointer value}} - if (int flag = foo()) // tracking-note{{Calling 'foo'}} - // tracking-note@-1{{Returning from 'foo'}} - // tracking-note@-2{{'flag' initialized here}} - // debug-note@-3{{Tracking condition 'flag'}} - // expected-note@-4{{Assuming 'flag' is not equal to 0}} - // expected-note@-5{{Taking true branch}} + if (int flag = foo()) // tracking-note{{'flag' initialized here}} + // debug-note@-1{{Tracking condition 'flag'}} + // expected-note@-2{{Assuming 'flag' is not equal to 0}} + // expected-note@-3{{Taking true branch}} *x = 5; // expected-warning{{Dereference of null pointer}} // expected-note@-1{{Dereference of null pointer}} @@ -143,39 +141,114 @@ bool coin(); struct ConvertsToBool { - operator bool() const { return coin(); } // tracking-note{{Returning value}} + operator bool() const { return coin(); } }; void test() { int *x = 0; // expected-note{{'x' initialized to a null pointer value}} if (ConvertsToBool()) - // tracking-note@-1 {{Calling 'ConvertsToBool::operator bool'}} - // tracking-note@-2{{Returning from 'ConvertsToBool::operator bool'}} - // debug-note@-3{{Tracking condition 'ConvertsToBool()'}} - // expected-note@-4{{Assuming the condition is true}} - // expected-note@-5{{Taking true branch}} + // debug-note@-1{{Tracking condition 'ConvertsToBool()'}} + // expected-note@-2{{Assuming the condition is true}} + // expected-note@-3{{Taking true branch}} *x = 5; // expected-warning{{Dereference of null pointer}} // expected-note@-1{{Dereference of null pointer}} } } // end of namespace variable_declaration_in_condition +namespace important_returning_pointer_loaded_from { +bool coin(); + +int *getIntPtr(); + +void storeValue(int **i) { + *i = getIntPtr(); // tracking-note{{Value assigned to 'i'}} +} + +int *conjurePointer() { + int *i; + storeValue(&i); // tracking-note{{Calling 'storeValue'}} + // tracking-note@-1{{Returning from 'storeValue'}} + return i; // tracking-note{{Returning pointer (loaded from 'i')}} +} + +void f(int *ptr) { + if (ptr) // expected-note{{Assuming 'ptr' is null}} + // expected-note@-1{{Taking false branch}} + ; + if (!conjurePointer()) + // tracking-note@-1{{Calling 'conjurePointer'}} + // tracking-note@-2{{Returning from 'conjurePointer'}} + // debug-note@-3{{Tracking condition '!conjurePointer()'}} + // expected-note@-4{{Assuming the condition is true}} + // expected-note@-5{{Taking true branch}} + *ptr = 5; // expected-warning{{Dereference of null pointer}} + // expected-note@-1{{Dereference of null pointer}} +} +} // end of namespace important_returning_pointer_loaded_from + +namespace unimportant_returning_pointer_loaded_from { +bool coin(); + +int *getIntPtr(); + +int *conjurePointer() { + int *i = getIntPtr(); // tracking-note{{'i' initialized here}} + return i; // tracking-note{{Returning pointer (loaded from 'i')}} +} + +void f(int *ptr) { + if (ptr) // expected-note{{Assuming 'ptr' is null}} + // expected-note@-1{{Taking false branch}} + ; + if (!conjurePointer()) + // tracking-note@-1{{Calling 'conjurePointer'}} + // tracking-note@-2{{Returning from 'conjurePointer'}} + // debug-note@-3{{Tracking condition '!conjurePointer()'}} + // expected-note@-4{{Assuming the condition is true}} + // expected-note@-5{{Taking true branch}} + *ptr = 5; // expected-warning{{Dereference of null pointer}} + // expected-note@-1{{Dereference of null pointer}} +} +} // end of namespace unimportant_returning_pointer_loaded_from + +namespace unimportant_returning_pointer_loaded_from_through_cast { + +void *conjure(); + +int *cast(void *P) { + return static_cast(P); +} + +void f() { + int *x = 0; // expected-note{{'x' initialized to a null pointer value}} + + if (cast(conjure())) + // tracking-note@-1{{Passing value via 1st parameter 'P'}} + // debug-note@-2{{Tracking condition 'cast(conjure())'}} + // expected-note@-3{{Assuming the condition is false}} + // expected-note@-4{{Taking false branch}} + return; + *x = 5; // expected-warning{{Dereference of null pointer}} + // expected-note@-1{{Dereference of null pointer}} +} + +} // end of namespace unimportant_returning_pointer_loaded_from_through_cast + namespace unimportant_returning_value_note { bool coin(); -bool flipCoin() { return coin(); } // tracking-note{{Returning value}} +bool flipCoin() { return coin(); } void i(int *ptr) { if (ptr) // expected-note{{Assuming 'ptr' is null}} // expected-note@-1{{Taking false branch}} ; if (!flipCoin()) - // tracking-note@-1{{Calling 'flipCoin'}} - // tracking-note@-2{{Returning from 'flipCoin'}} - // debug-note@-3{{Tracking condition '!flipCoin()'}} - // expected-note@-4{{Assuming the condition is true}} - // expected-note@-5{{Taking true branch}} + // debug-note@-1{{Tracking condition '!flipCoin()'}} + // expected-note@-2{{Assuming the condition is true}} + // expected-note@-3{{Taking true branch}} *ptr = 5; // expected-warning{{Dereference of null pointer}} // expected-note@-1{{Dereference of null pointer}} } @@ -207,6 +280,36 @@ } } // end of namespace important_returning_value_note +namespace important_returning_value_note_in_linear_function { +bool coin(); + +struct super_complicated_template_hackery { + static constexpr bool value = false; +}; + +bool flipCoin() { + if (super_complicated_template_hackery::value) + // tracking-note@-1{{'value' is false}} + // tracking-note@-2{{Taking false branch}} + return true; + return coin(); // tracking-note{{Returning value}} +} + +void i(int *ptr) { + if (ptr) // expected-note{{Assuming 'ptr' is null}} + // expected-note@-1{{Taking false branch}} + ; + if (!flipCoin()) + // tracking-note@-1{{Calling 'flipCoin'}} + // tracking-note@-2{{Returning from 'flipCoin'}} + // debug-note@-3{{Tracking condition '!flipCoin()'}} + // expected-note@-4{{Assuming the condition is true}} + // expected-note@-5{{Taking true branch}} + *ptr = 5; // expected-warning{{Dereference of null pointer}} + // expected-note@-1{{Dereference of null pointer}} +} +} // end of namespace important_returning_value_note_in_linear_function + namespace tracked_condition_is_only_initialized { int getInt(); Index: cfe/trunk/test/Analysis/uninit-vals.c =================================================================== --- cfe/trunk/test/Analysis/uninit-vals.c +++ cfe/trunk/test/Analysis/uninit-vals.c @@ -149,8 +149,6 @@ RetVoidFuncType f = foo_radar12278788_fp; return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}} //expected-note@-1 {{Undefined or garbage value returned to caller}} - //expected-note@-2 {{Calling 'foo_radar12278788_fp'}} - //expected-note@-3 {{Returning from 'foo_radar12278788_fp'}} } void rdar13665798() { @@ -164,8 +162,6 @@ RetVoidFuncType f = foo_radar12278788_fp; return ((RetIntFuncType)f)(); //expected-warning {{Undefined or garbage value returned to caller}} //expected-note@-1 {{Undefined or garbage value returned to caller}} - //expected-note@-2 {{Calling 'foo_radar12278788_fp'}} - //expected-note@-3 {{Returning from 'foo_radar12278788_fp'}} }(); } @@ -182,18 +178,14 @@ void use(struct Point p); void testUseHalfPoint() { - struct Point p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}} - // expected-note@-1{{Returning from 'getHalfPoint'}} - // expected-note@-2{{'p' initialized here}} + struct Point p = getHalfPoint(); // expected-note{{'p' initialized here}} use(p); // expected-warning{{uninitialized}} // expected-note@-1{{uninitialized}} } void testUseHalfPoint2() { struct Point p; - p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}} - // expected-note@-1{{Returning from 'getHalfPoint'}} - // expected-note@-2{{Value assigned to 'p'}} + p = getHalfPoint(); // expected-note{{Value assigned to 'p'}} use(p); // expected-warning{{uninitialized}} // expected-note@-1{{uninitialized}} }