NoQ added a comment.
Whoops forgot to submit inline comments.
================
Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1404
+ // When trying to dereference a void pointer, read the first byte.
+ T = Ctx.CharTy;
+ }
----------------
dcoughlin wrote:
> Nit: It seems a bit odd to read the first byte here since (unless I'm
> misunderstanding) this would never be triggered by actual C semantics, only
> by a checker. Did you consider just returning UnknownVal() in this case?
`UnknownVal` seems to be quite similar to assertion failure: in both cases we'd
have to force the checker to specify `CharTy` explicitly. But assertions are
better because the author of the checker would instantly know that he needs to
specify the type, instead of never noticing the problem, or spending a lot of
time in the debugger trying to understand why he gets an `UnknownVal`.
================
Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1408
}
+ assert(!T.isNull() && "Unable to auto-detect binding type!");
+ assert(!T->isVoidType() && "Attempted to retrieve a void value!");
----------------
dcoughlin wrote:
> I think you missed handling the AllocaRegion case from the old version in
> your new version. This means the assert will fire on the following when
> core.alpha is enabled:
> ```
> void foo(void *dest) {
> void *src = __builtin_alloca(5);
> memcpy(dest, src, 1);
> }
> ```
Nice one!
Always been that way though. In the old code, if `AllocaRegion` is supplied and
no type is explicitly specified, `cast<SymbolicRegion>(MR)` would fail. Also
`ElementRegion` is created in both old and new code if the type was specified.
https://reviews.llvm.org/D38358
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
