rdevshp wrote:
> This is slightly inaccurate: you are changing the removal of _symbols_, and
> not just the removal of constraints. (Of course, when a symbol is removed
> this way, constraints about it are also removed.) The leak warning checkers
> register a `check::DeadSymbols` callback to detect the point when a symbol is
> removed ("marked as dead") and use this opportunity to produce a bug report
> if the symbol was in an "active/open" state. To preserve the constraints
> about more symbols, you need to keep those symbols alive, and this means that
> they will also be alive for the POV of the leak warning checkers.
I checked for the string ConstraintSMT inside the llvm repo and the string
matches are (for commit 5cdb776bcee4b274d67c19f966725e6a580347bc):
```
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
29: ConstraintSMTType;
30:REGISTER_TRAIT_WITH_PROGRAMSTATE(ConstraintSMT, ConstraintSMTType)
229: ConstraintSMTType CZ = State->get<ConstraintSMT>();
230: ConstraintSMTType::Factory &CZFactory =
State->get_context<ConstraintSMT>();
265: return State->set<ConstraintSMT>(CZ);
270: ConstraintSMTType Constraints = State->get<ConstraintSMT>();
280: for (ConstraintSMTType::iterator I = Constraints.begin();
298: return S1->get<ConstraintSMT>() == S2->get<ConstraintSMT>();
361: return State->add<ConstraintSMT>(std::make_pair(Sym, Exp));
370: auto CZ = State->get<ConstraintSMT>();
388: State->add<ConstraintSMT>(std::make_pair(Sym, Exp));
391: NewState->get<ConstraintSMT>().Profile(ID);
```
All these references come from SMTConstraintManager.h, and except from the
printJson method, it appears that there are no other potential accesses to the
internal ConstraintSMT program state (which is accessed from
`State->get<ConstraintSMT>()`) for files other than SMTConstraintManager.cpp.
So I believe other files essentially only rely on the Z3 solver result instead
of the internal ConstraintSMT program state, which is what is modified by
removeDeadBindings.
I checked and indeed this PR can affect the result of the leak checker, but for
the example that was found where it affected the leaker checking results, it is
only because it properly trimmed the infeasible paths and removed the false
positives.
e.g.
malloc_path_constraint_bridge.c:
```c
typedef __SIZE_TYPE__ size_t;
void *malloc(size_t);
void free(void *);
void malloc_path_constraint_bridge(int a, int b, int c) {
void *p = malloc(1);
if (a != b) {
free(p);
return;
}
if (b != c) {
free(p);
return;
}
if (a != c)
return;
free(p);
}
```
checking this file with `-cc1 -analyze
-analyze-function=malloc_path_constraint_bridge
-analyzer-checker=core,unix.Malloc,debug.ExprInspection
-analyzer-constraints=unsupported-z3 -analyzer-output=text
malloc_path_constraint_bridge.c` gives me a false positive potential memory
leak diagnostics with the unpatched clang, and produces no memory leak
diagnostics with the patched clang.
As `removeDeadBindings` essentially only touches the ConstraintSMT program
state (through `return State->set<ConstraintSMT>(CZ)`) (where each entry in the
ImmutableSet is an std::pair of plain pointers (const SymExpr * and const
SMTExpr *), it does not look like this PR causes false negatives for the leak
warning checkers (please let me know if I missed anything).
https://github.com/llvm/llvm-project/pull/215240
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
