rnkovacs added inline comments.
================
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:32-40
+namespace clang {
+namespace ento {
+template<> struct ProgramStateTrait<PtrSet>
+ : public ProgramStatePartialTrait<PtrSet> {
+ static void *GDMIndex() {
+ static int Index = 0;
+ return &Index;
----------------
NoQ wrote:
> Please add a comment on how this template is useful. This trick is used by
> some checkers, but it's a veeeery unobvious trick. We should probably add a
> macro for that, i.e. `REGISTER_FACTORY_WITH_PROGRAMSTATE` or something like
> that.
Should I do that then? Maybe in `CheckerContext.h`?
================
Comment at: lib/StaticAnalyzer/Checkers/DanglingInternalBufferChecker.cpp:115
SVal RawPtr = Call.getReturnValue();
if (!RawPtr.isUnknown()) {
+ // Start tracking this raw pointer by adding it to the set of symbols
----------------
NoQ wrote:
> I'd much rather unwrap the symbol here, i.e. `if (SymbolRef Sym =
> RawPtr.getAsSymbol())`. A lot of other cornercases may occur if the
> implementation is accidentally inlined (`Undefined`, concrete regions). Also,
> speculatively, `.getAsSymbol(/* search for symbolic base = */ true)` for the
> same reason//*//.
>
> If we didn't care about inlined implementations, the `Unknown` check would
> have been redundant. So it should also be safe to straightforwardly ignore
> inlined implementations by consulting `C.wasInlined`, then the presence of
> the symbol can be asserted. But i'd rather speculatively care about inlined
> implementations as long as it seems easy.
>
> __
> //*// In fact your code relies on a very wonky implementation detail of our
> `SVal` hierarchy: namely, pointer-type return values of conservatively
> evaluated functions are always expressed as `&SymRegion{conj_$N<pointer
> type>}` and never as `&element{SymRegion{conj_$N<pointer type>}, 0 S32b,
> pointee type}`. Currently nobody knows the rules under which zero element
> regions are added in different parts of the analyzer, i.e. what is the
> "canonical" representation of the symbolic pointer, though i made a few
> attempts to speculate.
I wasn't aware of much of this. Thanks for the detailed explanation :)
https://reviews.llvm.org/D49057
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
