[PATCH] D116597: [analyzer] Don't track function calls as control dependencies
Balázs Benics via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 30 08:12:31 PDT 2022
steakhal accepted this revision.
steakhal added a comment.
This revision is now accepted and ready to land.
I'm convinced!
================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1968
+static const Expr *peelOffOuterExpr(const Expr *Ex, const ExplodedNode *N) {
+
Ex = Ex->IgnoreParenCasts();
----------------
Szelethus wrote:
> steakhal wrote:
> > extra blank line
> That is intentional -- I think it makes the code more readable. Separates the function signature from the implementation.
Okay.
================
Comment at: clang/test/Analysis/track-control-dependency-conditions.cpp:41
- foo(); // TODO: Add nodes here about flag's value being invalidated.
+ foo(); // TODO: Add nodes here about flag's value being invalidated.
if (flag) // expected-note-re {{{{^}}Assuming 'flag' is 0{{$}}}}
----------------
Some of these hunks are unrelated to the parts you actually changed.
Strictly peaking I don't mind them, just pointing out.
`clang-format-diff.py` would format only the changed lines.
================
Comment at: clang/test/Analysis/track-control-dependency-conditions.cpp:202-203
if (!conjurePointer())
- // tracking-note-re at -1{{{{^}}Calling 'conjurePointer'{{$}}}}
- // tracking-note-re at -2{{{{^}}Returning from 'conjurePointer'{{$}}}}
- // debug-note-re at -3{{{{^}}Tracking condition '!conjurePointer()'{{$}}}}
- // expected-note-re at -4{{{{^}}Assuming the condition is true{{$}}}}
- // expected-note-re at -5{{{{^}}Taking true branch{{$}}}}
+ // expected-note-re at -1{{{{^}}Assuming the condition is true{{$}}}}
+ // expected-note-re at -2{{{{^}}Taking true branch{{$}}}}
*ptr = 5; // expected-warning{{Dereference of null pointer}}
----------------
For the record, I never understood why we have two notes about the same thing: //we are taking that branch//.
Maybe we should keep simplifying these notes in the future as well.
================
Comment at: clang/test/Analysis/track-control-dependency-conditions.cpp:1036
+ 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]}}
----------------
Szelethus wrote:
> steakhal wrote:
> > What if this expression is enclosed by a logical operator such as `&&`?
> For each of those operators, a different CFGBlock would be created:
>
> ```
> if (A && B)
> C;
> D;
>
> C
> / \
> B------->
> / \
> A---------> D
> ```
>
> This means that operands of || and && is retrievable through `CFGBlock::getLastCondition()`, so I shouldn't need to tear the AST apart to that extent. Though, I admit, you likely don't need to go very far to fool my implementation for realizing whether the condition boils down to a function call.
I see, thanks.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D116597/new/
https://reviews.llvm.org/D116597
More information about the cfe-commits
mailing list