NoQ added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:603
+ // point.
+ class CollectReachableSymbolsCallback final : public SymbolVisitor {
+ ProgramStateRef State;
----------------
xazax.hun wrote:
> 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.
How about `escapeValue(getSVal(getArgSVal()))`? (the type for `getSVal()` could
be obtained from the AST).
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
