xazax.hun marked an inline comment as done.
xazax.hun added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:603
+ // point.
+ class CollectReachableSymbolsCallback final : public SymbolVisitor {
+ ProgramStateRef State;
----------------
NoQ wrote:
> xazax.hun wrote:
> > NoQ wrote:
> > > WDYT about re-using `ExprEngine::escapeValue()` by changing it to accept
> > > `ArrayRef<SVal>` instead of a single `SVal`?
> > It is not entirely the same. Here we only collect symbols from non-stack
> > regions. There (as far as I understand) we collect all symbols. I could add
> > a flag but at that point I wonder if it is worth the change.
> Umm, wait, right, so what are you doing in this callback to begin with? The
> code says "gather all symbols that are encountered as immediate values stored
> in traversed regions". Why not simply "gather all symbols that are traversed
> from parameter regions"?
My understanding is the following but correct me if I am wrong:
```
int *getConjuredSymbol();
call(getConjuredSymbol());
```
So we have can talk about two symbols here. One symbol is what was returned by
`getConjuredSymbol` and the other is the pointee, the symbol that we get when
we dereference the result of `getConjuredSymbol`. And my understanding is that
we want to invoke escape for the latter and not the former.
`ExprEngine::escapeValue` invalidates the former but not the latter. The
visitor here invalidates the latter but not the former. This behavior solves
most of my test cases in FuchsiaHandleChecker.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71224/new/
https://reviews.llvm.org/D71224
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
