From d9a81ccf05925e25df17cf64f7636ae78cd99d3d Mon Sep 17 00:00:00 2001 From: Kristof Umann Date: Wed, 21 Aug 2019 22:38:00 +0000 Subject: [PATCH] [analyzer] Mention whether an event is about a condition in a bug report part 2 In D65724, I do a pretty thorough explanation about how I'm solving this problem, I think that summary nails whats happening here ;) Differential Revision: https://reviews.llvm.org/D65725 llvm-svn: 369596 --- .../Core/BugReporterVisitors.cpp | 30 +++++ .../track-control-dependency-conditions.cpp | 120 +++++++++++++++++- 2 files changed, 144 insertions(+), 6 deletions(-) diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index ccd147081d96..973580ed00ae 100644 --- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -213,6 +213,22 @@ getConcreteIntegerValue(const Expr *CondVarExpr, const ExplodedNode *N) { return None; } +static bool isVarAnInterestingCondition(const Expr *CondVarExpr, + const ExplodedNode *N, + const BugReport *B) { + // Even if this condition is marked as interesting, it isn't *that* + // interesting if it didn't happen in a nested stackframe, the user could just + // follow the arrows. + if (!B->getErrorNode()->getStackFrame()->isParentOf(N->getStackFrame())) + return false; + + if (Optional V = getSValForVar(CondVarExpr, N)) + if (Optional K = B->getInterestingnessKind(*V)) + return *K == bugreporter::TrackingKind::Condition; + + return false; +} + static bool isInterestingExpr(const Expr *E, const ExplodedNode *N, const BugReport *B) { if (Optional V = getSValForVar(E, N)) @@ -2454,6 +2470,10 @@ PathDiagnosticPieceRef ConditionBRVisitor::VisitTrueTest( const LocationContext *LCtx = N->getLocationContext(); const SourceManager &SM = BRC.getSourceManager(); + if (isVarAnInterestingCondition(BExpr->getLHS(), N, &R) || + isVarAnInterestingCondition(BExpr->getRHS(), N, &R)) + Out << WillBeUsedForACondition; + // Convert 'field ...' to 'Field ...' if it is a MemberExpr. std::string Message = Out.str(); Message[0] = toupper(Message[0]); @@ -2499,6 +2519,9 @@ PathDiagnosticPieceRef ConditionBRVisitor::VisitConditionVariable( const LocationContext *LCtx = N->getLocationContext(); PathDiagnosticLocation Loc(CondVarExpr, BRC.getSourceManager(), LCtx); + if (isVarAnInterestingCondition(CondVarExpr, N, &report)) + Out << WillBeUsedForACondition; + auto event = std::make_shared(Loc, Out.str()); if (isInterestingExpr(CondVarExpr, N, &report)) @@ -2524,6 +2547,9 @@ PathDiagnosticPieceRef ConditionBRVisitor::VisitTrueTest( const LocationContext *LCtx = N->getLocationContext(); + if (isVarAnInterestingCondition(DRE, N, &report)) + Out << WillBeUsedForACondition; + // If we know the value create a pop-up note to the 'DRE'. if (!IsAssuming) { PathDiagnosticLocation Loc(DRE, BRC.getSourceManager(), LCtx); @@ -2563,6 +2589,10 @@ PathDiagnosticPieceRef ConditionBRVisitor::VisitTrueTest( if (!Loc.isValid() || !Loc.asLocation().isValid()) return nullptr; + if (isVarAnInterestingCondition(ME, N, &report)) + Out << WillBeUsedForACondition; + + // If we know the value create a pop-up note. if (!IsAssuming) return std::make_shared(Loc, Out.str()); diff --git a/clang/test/Analysis/track-control-dependency-conditions.cpp b/clang/test/Analysis/track-control-dependency-conditions.cpp index 02c53418a209..e95cc7cbb1ec 100644 --- a/clang/test/Analysis/track-control-dependency-conditions.cpp +++ b/clang/test/Analysis/track-control-dependency-conditions.cpp @@ -443,7 +443,7 @@ void f(int flag) { int *x = 0; // expected-note-re{{{{^}}'x' initialized to a null pointer value{{$}}}} auto lambda = [flag]() { - if (!flag) // tracking-note-re{{{{^}}Assuming 'flag' is not equal to 0{{$}}}} + if (!flag) // tracking-note-re{{{{^}}Assuming 'flag' is not equal to 0, which participates in a condition later{{$}}}} // tracking-note-re@-1{{{{^}}Taking false branch{{$}}}} halt(); }; @@ -474,7 +474,7 @@ void f(int flag) { int *x = 0; // expected-note-re{{{{^}}'x' initialized to a null pointer value{{$}}}} auto lambda = [&flag]() { - if (!flag) // tracking-note-re{{{{^}}Assuming 'flag' is not equal to 0{{$}}}} + if (!flag) // tracking-note-re{{{{^}}Assuming 'flag' is not equal to 0, which participates in a condition later{{$}}}} // tracking-note-re@-1{{{{^}}Taking false branch{{$}}}} halt(); }; @@ -486,18 +486,42 @@ void f(int flag) { if (flag) // expected-note-re{{{{^}}'flag' is not equal to 0{{$}}}} // expected-note-re@-1{{{{^}}Taking true branch{{$}}}} - // debug-note-re@-2{{{{^}}Tracking condition 'flag'}} + // debug-note-re@-2{{{{^}}Tracking condition 'flag'{{$}}}} *x = 5; // expected-warning{{Dereference of null pointer}} // expected-note@-1{{Dereference of null pointer}} } } // end of namespace condition_lambda_capture_by_reference_assumption +namespace collapse_point_not_in_condition_bool { + +[[noreturn]] void halt(); + +void check(bool b) { + if (!b) // tracking-note-re{{{{^}}Assuming 'b' is true, which participates in a condition later{{$}}}} + // tracking-note-re@-1{{{{^}}Taking false branch{{$}}}} + halt(); +} + +void f(bool flag) { + int *x = 0; // expected-note-re{{{{^}}'x' initialized to a null pointer value{{$}}}} + + check(flag); // tracking-note-re{{{{^}}Calling 'check'{{$}}}} + // tracking-note-re@-1{{{{^}}Returning from 'check'{{$}}}} + + if (flag) // expected-note-re{{{{^}}'flag' is true{{$}}}} + // expected-note-re@-1{{{{^}}Taking true branch{{$}}}} + // debug-note-re@-2{{{{^}}Tracking condition 'flag'{{$}}}} + *x = 5; // expected-warning{{Dereference of null pointer}} + // expected-note@-1{{Dereference of null pointer}} +} +} // end of namespace collapse_point_not_in_condition_bool + namespace collapse_point_not_in_condition { [[noreturn]] void halt(); void assert(int b) { - if (!b) // tracking-note-re{{{{^}}Assuming 'b' is not equal to 0{{$}}}} + if (!b) // tracking-note-re{{{{^}}Assuming 'b' is not equal to 0, which participates in a condition later{{$}}}} // tracking-note-re@-1{{{{^}}Taking false branch{{$}}}} halt(); } @@ -522,7 +546,7 @@ namespace unimportant_write_before_collapse_point { [[noreturn]] void halt(); void assert(int b) { - if (!b) // tracking-note-re{{{{^}}Assuming 'b' is not equal to 0{{$}}}} + if (!b) // tracking-note-re{{{{^}}Assuming 'b' is not equal to 0, which participates in a condition later{{$}}}} // tracking-note-re@-1{{{{^}}Taking false branch{{$}}}} halt(); } @@ -556,6 +580,31 @@ void f6(int x) { } // end of namespace dont_crash_on_nonlogical_binary_operator +namespace collapse_point_not_in_condition_binary_op { + +[[noreturn]] void halt(); + +void check(int b) { + if (b == 1) // tracking-note-re{{{{^}}Assuming 'b' is not equal to 1, which participates in a condition later{{$}}}} + // tracking-note-re@-1{{{{^}}Taking false branch{{$}}}} + halt(); +} + +void f(int flag) { + int *x = 0; // expected-note-re{{{{^}}'x' initialized to a null pointer value{{$}}}} + + check(flag); // tracking-note-re{{{{^}}Calling 'check'{{$}}}} + // tracking-note-re@-1{{{{^}}Returning from 'check'{{$}}}} + + if (flag) // expected-note-re{{{{^}}Assuming 'flag' is not equal to 0{{$}}}} + // expected-note-re@-1{{{{^}}Taking true branch{{$}}}} + // debug-note-re@-2{{{{^}}Tracking condition 'flag'{{$}}}} + *x = 5; // expected-warning{{Dereference of null pointer}} + // expected-note@-1{{Dereference of null pointer}} +} + +} // end of namespace collapse_point_not_in_condition_binary_op + namespace collapse_point_not_in_condition_as_field { [[noreturn]] void halt(); @@ -564,7 +613,7 @@ struct IntWrapper { IntWrapper(); void check() { - if (!b) // tracking-note-re{{{{^}}Assuming field 'b' is not equal to 0{{$}}}} + if (!b) // tracking-note-re{{{{^}}Assuming field 'b' is not equal to 0, which participates in a condition later{{$}}}} // tracking-note-re@-1{{{{^}}Taking false branch{{$}}}} halt(); return; @@ -585,6 +634,65 @@ void f(IntWrapper i) { } // end of namespace collapse_point_not_in_condition_as_field +namespace assignemnt_in_condition_in_nested_stackframe { +int flag; + +bool coin(); + +[[noreturn]] void halt(); + +void foo() { + if ((flag = coin())) + // tracking-note-re@-1{{{{^}}Value assigned to 'flag', which participates in a condition later{{$}}}} + // tracking-note-re@-2{{{{^}}Assuming 'flag' is not equal to 0, which participates in a condition later{{$}}}} + // tracking-note-re@-3{{{{^}}Taking true branch{{$}}}} + return; + halt(); + return; +} + +void f() { + int *x = 0; // expected-note-re{{{{^}}'x' initialized to a null pointer value{{$}}}} + + foo(); // tracking-note-re{{{{^}}Calling 'foo'{{$}}}} + // tracking-note-re@-1{{{{^}}Returning from 'foo'{{$}}}} + if (flag) // expected-note-re{{{{^}}'flag' is not equal to 0{{$}}}} + // expected-note-re@-1{{{{^}}Taking true branch{{$}}}} + // debug-note-re@-2{{{{^}}Tracking condition 'flag'{{$}}}} + *x = 5; // expected-warning{{Dereference of null pointer}} + // expected-note@-1{{Dereference of null pointer}} +} +} // end of namespace assignemnt_in_condition_in_nested_stackframe + +namespace condition_variable_less { +int flag; + +bool coin(); + +[[noreturn]] void halt(); + +void foo() { + if (flag > 0) + // tracking-note-re@-1{{{{^}}Assuming 'flag' is > 0, which participates in a condition later{{$}}}} + // tracking-note-re@-2{{{{^}}Taking true branch{{$}}}} + return; + halt(); + return; +} + +void f() { + int *x = 0; // expected-note-re{{{{^}}'x' initialized to a null pointer value{{$}}}} + + foo(); // tracking-note-re{{{{^}}Calling 'foo'{{$}}}} + // tracking-note-re@-1{{{{^}}Returning from 'foo'{{$}}}} + if (flag) // expected-note-re{{{{^}}'flag' is not equal to 0{{$}}}} + // expected-note-re@-1{{{{^}}Taking true branch{{$}}}} + // debug-note-re@-2{{{{^}}Tracking condition 'flag'{{$}}}} + *x = 5; // expected-warning{{Dereference of null pointer}} + // expected-note@-1{{Dereference of null pointer}} +} +} // end of namespace condition_variable_less + namespace dont_track_assertlike_conditions { extern void __assert_fail(__const char *__assertion, __const char *__file,