Szelethus added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp:309-311
+void ErrorReturnChecker::checkAccess(CheckerContext &C, ProgramStateRef State,
+ const Stmt *LoadS, SymbolRef CallSym,
+ const CalledFunctionData *CFD) const {
----------------
NoQ wrote:
> Why scan parent statements proactively given that we will reach them later
> anyway? Looks like you're using a wrong checker callback.
Thats one of the things I'm not super familiar with just yet. Are you
referencing how a CFGBlock is built up? I remember trying to solve a similar
problem (dig writes/reads out of `Stmt *`s) when working on the reaching
definitions calculator, and I question my methodology as well.
test.cpp:
```lang=c++
int getInt();
void different_assignments() {
int i = getInt(); // Return value to check.
if (static_cast<bool>(i < 7))
;
}
```
AST:
```
`-FunctionDecl 0xaf6ff0 <line:3:1, line:8:1> line:3:6 different_assignments
'void ()'
`-CompoundStmt 0xaf72d8 <col:30, line:8:1>
|-DeclStmt 0xaf71e8 <line:4:3, col:19>
| `-VarDecl 0xaf70a8 <col:3, col:18> col:7 used i 'int' cinit
| `-CallExpr 0xaf71c8 <col:11, col:18> 'int'
| `-ImplicitCastExpr 0xaf71b0 <col:11> 'int (*)()'
<FunctionToPointerDecay>
| `-DeclRefExpr 0xaf7158 <col:11> 'int ()' lvalue Function 0xaf6eb8
'getInt' 'int ()'
`-IfStmt 0xaf72c0 <line:6:3, line:7:5>
|-CXXStaticCastExpr 0xaf7288 <line:6:7, col:30> 'bool' static_cast<_Bool>
<NoOp>
| `-BinaryOperator 0xaf7258 <col:25, col:29> 'bool' '<'
| |-ImplicitCastExpr 0xaf7240 <col:25> 'int' <LValueToRValue>
| | `-DeclRefExpr 0xaf7200 <col:25> 'int' lvalue Var 0xaf70a8 'i' 'int'
| `-IntegerLiteral 0xaf7220 <col:29> 'int' 7
`-NullStmt 0xaf72b8 <line:7:5>
```
CFG:
```
void different_assignments()
[B3 (ENTRY)]
Succs (1): B2
[B1]
Preds (1): B2
Succs (1): B0
[B2]
1: getInt
2: [B2.1] (ImplicitCastExpr, FunctionToPointerDecay, int (*)(void))
3: [B2.2]()
4: int i = getInt();
5: i
6: [B2.5] (ImplicitCastExpr, LValueToRValue, int)
7: 7
8: [B2.6] < [B2.7]
9: static_cast<bool>([B2.8]) (CXXStaticCastExpr, NoOp, _Bool)
T: if [B2.9]
Preds (1): B3
Succs (2): B1 B0
[B0 (EXIT)]
Preds (2): B1 B2
```
I can see that a single statement has multiple CFGElements associated with it
(implying that multiple checker callbacks might be invoked). Is your point tat
we shouldn't do all the analysis at Block 2 CFGElement 6, but rather at Block 2
Element 9?
As I see this checker, what we want at the end of the day is to see a critical
return value (say `ptr`) or **a derived value of it** (say `(ptr != nullptr &&
someOtherPrecondition()`) in a branch condition. But I think working from
`checkBranchCondition` and working down the AST (does `ptr` or some other
return value participate in this condition?) , as opposed to using
`checkLoation` and working working up (is `ptr`'s read here a part of a
condition?) doesn't immediately feel like an easier, shorter or less
error-prone solution.
A random idea, what we could try out is to store accesses to return values in
`checkLocation` in the GDM, and check in `checkBranchCondition` whether its a
substatement.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72705/new/
https://reviews.llvm.org/D72705
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
