Index: clang/include/clang/Analysis/CFG.h =================================================================== --- clang/include/clang/Analysis/CFG.h +++ clang/include/clang/Analysis/CFG.h @@ -1428,6 +1428,7 @@ void viewCFG(const LangOptions &LO) const; void print(raw_ostream &OS, const LangOptions &LO, bool ShowColors) const; void dump(const LangOptions &LO, bool ShowColors) const; + void dump(bool ShowColors=true) const; //===--------------------------------------------------------------------===// // Internal: constructors and data. Index: clang/lib/Analysis/CFG.cpp =================================================================== --- clang/lib/Analysis/CFG.cpp +++ clang/lib/Analysis/CFG.cpp @@ -5909,6 +5909,10 @@ print(llvm::errs(), LO, ShowColors); } +void CFG::dump(bool ShowColors) const { + dump(LangOptions{}, ShowColors); +} + /// print - A simple pretty printer of a CFG that outputs to an ostream. void CFG::print(raw_ostream &OS, const LangOptions &LO, bool ShowColors) const { StmtPrinterHelper Helper(this, LO); Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -82,6 +82,10 @@ return nullptr; } +/// \return A subexpression of @c Ex which represents the +/// expression-of-interest. +static const Expr *peelOffOuterExpr(const Expr *Ex, const ExplodedNode *N); + /// Given that expression S represents a pointer that would be dereferenced, /// try to find a sub-expression from which the pointer came from. /// This is used for tracking down origins of a null or undefined value: @@ -1918,11 +1922,30 @@ return nullptr; if (const Expr *Condition = NB->getLastCondition()) { + + // If we can't retrieve a sensible condition, just bail out. + const Expr *InnerExpr = peelOffOuterExpr(Condition, N); + if (!InnerExpr) + return nullptr; + + // If the condition was a function call, we likely won't gain much from + // tracking it either. Evidence suggests that it will mostly trigger in + // scenarios like this: + // + // void f(int *x) { + // x = nullptr; + // if (alwaysTrue()) // We don't need a whole lot of explanation + // // here, the function name is good enough. + // *x = 5; + // } + if (isa(InnerExpr)) + return nullptr; + // Keeping track of the already tracked conditions on a visitor level // isn't sufficient, because a new visitor is created for each tracked // expression, hence the BugReport level set. if (BR.addTrackedCondition(N)) { - getParentTracker().track(Condition, N, + getParentTracker().track(InnerExpr, N, {bugreporter::TrackingKind::Condition, /*EnableNullFPSuppression=*/false}); return constructDebugPieceForTrackedCondition(Condition, N, BRC); @@ -1937,10 +1960,8 @@ // Implementation of trackExpressionValue. //===----------------------------------------------------------------------===// -/// \return A subexpression of @c Ex which represents the -/// expression-of-interest. -static const Expr *peelOffOuterExpr(const Expr *Ex, - const ExplodedNode *N) { +static const Expr *peelOffOuterExpr(const Expr *Ex, const ExplodedNode *N) { + Ex = Ex->IgnoreParenCasts(); if (const auto *FE = dyn_cast(Ex)) return peelOffOuterExpr(FE->getSubExpr(), N); Index: clang/test/Analysis/return-value-guaranteed.cpp =================================================================== --- clang/test/Analysis/return-value-guaranteed.cpp +++ clang/test/Analysis/return-value-guaranteed.cpp @@ -24,7 +24,6 @@ // class-note@-1 {{The value 0 is assigned to 'F.Field'}} return !MCAsmParser::Error(); // class-note@-1 {{'MCAsmParser::Error' returns true}} - // class-note@-2 {{Returning zero, which participates in a condition later}} } bool parseFile() { @@ -58,7 +57,6 @@ struct MCAsmParser { static bool Error() { return false; // class-note {{'MCAsmParser::Error' returns false}} - // class-note@-1 {{Returning zero, which participates in a condition later}} } }; @@ -74,7 +72,6 @@ return MCAsmParser::Error(); // class-note@-1 {{Calling 'MCAsmParser::Error'}} // class-note@-2 {{Returning from 'MCAsmParser::Error'}} - // class-note@-3 {{Returning zero, which participates in a condition later}} } bool parseFile() { Index: clang/test/Analysis/track-control-dependency-conditions.cpp =================================================================== --- clang/test/Analysis/track-control-dependency-conditions.cpp +++ clang/test/Analysis/track-control-dependency-conditions.cpp @@ -149,14 +149,13 @@ int *x = 0; // expected-note-re{{{{^}}'x' initialized to a null pointer value{{$}}}} if (ConvertsToBool()) - // debug-note-re@-1{{{{^}}Tracking condition 'ConvertsToBool()'{{$}}}} - // expected-note-re@-2{{{{^}}Assuming the condition is true{{$}}}} - // expected-note-re@-3{{{{^}}Taking true branch{{$}}}} + // expected-note-re@-1{{{{^}}Assuming the condition is true{{$}}}} + // expected-note-re@-2{{{{^}}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 conversion_to_bool namespace note_from_different_but_not_nested_stackframe { @@ -186,14 +185,13 @@ int *getIntPtr(); void storeValue(int **i) { - *i = getIntPtr(); // tracking-note-re{{{{^}}Value assigned to 'i', which participates in a condition later{{$}}}} + *i = getIntPtr(); } int *conjurePointer() { int *i; - storeValue(&i); // tracking-note-re{{{{^}}Calling 'storeValue'{{$}}}} - // tracking-note-re@-1{{{{^}}Returning from 'storeValue'{{$}}}} - return i; // tracking-note-re{{{{^}}Returning pointer (loaded from 'i'), which participates in a condition later{{$}}}} + storeValue(&i); + return i; } void f(int *ptr) { @@ -201,11 +199,9 @@ // expected-note-re@-1{{{{^}}Taking false branch{{$}}}} ; if (!conjurePointer()) - // tracking-note-re@-1{{{{^}}Calling 'conjurePointer'{{$}}}} - // tracking-note-re@-2{{{{^}}Returning from 'conjurePointer'{{$}}}} - // debug-note-re@-3{{{{^}}Tracking condition '!conjurePointer()'{{$}}}} - // expected-note-re@-4{{{{^}}Assuming the condition is true{{$}}}} - // expected-note-re@-5{{{{^}}Taking true branch{{$}}}} + // debug-note-re@-1{{{{^}}Tracking condition '!conjurePointer()'{{$}}}} + // expected-note-re@-2{{{{^}}Assuming the condition is true{{$}}}} + // expected-note-re@-3{{{{^}}Taking true branch{{$}}}} *ptr = 5; // expected-warning{{Dereference of null pointer}} // expected-note@-1{{Dereference of null pointer}} } @@ -226,9 +222,8 @@ // expected-note-re@-1{{{{^}}Taking false branch{{$}}}} ; if (!conjurePointer()) - // debug-note-re@-1{{{{^}}Tracking condition '!conjurePointer()'{{$}}}} - // expected-note-re@-2{{{{^}}Assuming the condition is true{{$}}}} - // expected-note-re@-3{{{{^}}Taking true branch{{$}}}} + // expected-note-re@-1{{{{^}}Assuming the condition is true{{$}}}} + // expected-note-re@-2{{{{^}}Taking true branch{{$}}}} *ptr = 5; // expected-warning{{Dereference of null pointer}} // expected-note@-1{{Dereference of null pointer}} } @@ -246,9 +241,8 @@ int *x = 0; // expected-note-re{{{{^}}'x' initialized to a null pointer value{{$}}}} if (cast(conjure())) - // debug-note-re@-1{{{{^}}Tracking condition 'cast(conjure())'{{$}}}} - // expected-note-re@-2{{{{^}}Assuming the condition is false{{$}}}} - // expected-note-re@-3{{{{^}}Taking false branch{{$}}}} + // expected-note-re@-1{{{{^}}Assuming the condition is false{{$}}}} + // expected-note-re@-2{{{{^}}Taking false branch{{$}}}} return; *x = 5; // expected-warning{{Dereference of null pointer}} // expected-note@-1{{Dereference of null pointer}} @@ -278,11 +272,9 @@ bool coin(); bool flipCoin() { - if (coin()) // tracking-note-re{{{{^}}Assuming the condition is false{{$}}}} - // tracking-note-re@-1{{{{^}}Taking false branch{{$}}}} - // debug-note-re@-2{{{{^}}Tracking condition 'coin()'{{$}}}} + if (coin()) return true; - return coin(); // tracking-note-re{{{{^}}Returning value, which participates in a condition later{{$}}}} + return coin(); } void i(int *ptr) { @@ -290,11 +282,8 @@ // expected-note-re@-1{{{{^}}Taking false branch{{$}}}} ; if (!flipCoin()) - // tracking-note-re@-1{{{{^}}Calling 'flipCoin'{{$}}}} - // tracking-note-re@-2{{{{^}}Returning from 'flipCoin'{{$}}}} - // debug-note-re@-3{{{{^}}Tracking condition '!flipCoin()'{{$}}}} - // expected-note-re@-4{{{{^}}Assuming the condition is true{{$}}}} - // expected-note-re@-5{{{{^}}Taking true branch{{$}}}} + // expected-note-re@-1{{{{^}}Assuming the condition is true{{$}}}} + // expected-note-re@-2{{{{^}}Taking true branch{{$}}}} *ptr = 5; // expected-warning{{Dereference of null pointer}} // expected-note@-1{{Dereference of null pointer}} } @@ -302,29 +291,32 @@ namespace important_returning_value_note_in_linear_function { bool coin(); +int flag; struct super_complicated_template_hackery { static constexpr bool value = false; }; -bool flipCoin() { +void flipCoin() { if (super_complicated_template_hackery::value) - // tracking-note-re@-1{{{{^}}'value' is false{{$}}}} - // tracking-note-re@-2{{{{^}}Taking false branch{{$}}}} - return true; - return coin(); // tracking-note-re{{{{^}}Returning value, which participates in a condition later{{$}}}} + // expected-note-re@-1{{{{^}}'value' is 0{{$}}}} + // expected-note-re@-2{{{{^}}Taking false branch{{$}}}} + return; + flag = false; // tracking-note-re{{{{^}}The value 0 is assigned to 'flag', which participates in a condition later{{$}}}} } void i(int *ptr) { + flag = true; if (ptr) // expected-note-re{{{{^}}Assuming 'ptr' is null{{$}}}} // expected-note-re@-1{{{{^}}Taking false branch{{$}}}} ; - if (!flipCoin()) - // tracking-note-re@-1{{{{^}}Calling 'flipCoin'{{$}}}} - // tracking-note-re@-2{{{{^}}Returning from 'flipCoin'{{$}}}} - // debug-note-re@-3{{{{^}}Tracking condition '!flipCoin()'{{$}}}} - // expected-note-re@-4{{{{^}}Assuming the condition is true{{$}}}} - // expected-note-re@-5{{{{^}}Taking true branch{{$}}}} + flipCoin(); + // tracking-note-re@-1{{{{^}}Calling 'flipCoin'{{$}}}} + // tracking-note-re@-2{{{{^}}Returning from 'flipCoin'{{$}}}} + if (!flag) + // debug-note-re@-1{{{{^}}Tracking condition '!flag'{{$}}}} + // expected-note-re@-2{{{{^}}'flag' is 0{{$}}}} + // expected-note-re@-3{{{{^}}Taking true branch{{$}}}} *ptr = 5; // expected-warning{{Dereference of null pointer}} // expected-note@-1{{Dereference of null pointer}} } @@ -1000,3 +992,52 @@ } } // end of namespace only_track_the_evaluated_condition + +namespace operator_call_in_condition_point { + +struct Error { + explicit operator bool() { + return true; + } +}; + +Error couldFail(); + +void f(int *x) { + x = nullptr; // expected-note {{Null pointer value stored to 'x'}} + if (auto e = couldFail()) // expected-note {{Taking true branch}} + *x = 5; // expected-warning {{Dereference of null pointer (loaded from variable 'x') [core.NullDereference]}} + // expected-note@-1 {{Dereference}} +} + +} // namespace operator_call_in_condition_point + +namespace funcion_call_in_condition_point { + +int alwaysTrue() { + return true; +} + +void f(int *x) { + x = nullptr; // expected-note {{Null pointer value stored to 'x'}} + if (alwaysTrue()) // expected-note {{Taking true branch}} + *x = 5; // expected-warning {{Dereference of null pointer (loaded from variable 'x') [core.NullDereference]}} + // expected-note@-1 {{Dereference}} +} + +} // namespace funcion_call_in_condition_point + +namespace funcion_call_negated_in_condition_point { + +int alwaysFalse() { + return false; +} + +void f(int *x) { + x = nullptr; // expected-note {{Null pointer value stored to 'x'}} + if (!alwaysFalse()) // expected-note {{Taking true branch}} + *x = 5; // expected-warning {{Dereference of null pointer (loaded from variable 'x') [core.NullDereference]}} + // expected-note@-1 {{Dereference}} +} + +} // namespace funcion_call_negated_in_condition_point