Szelethus added inline comments.
================
Comment at: lib/StaticAnalyzer/Checkers/UninitializedObjectChecker.cpp:488-491
// TODO: Dereferencing should be done according to the dynamic type.
while (Optional<Loc> L = DerefdV.getAs<Loc>()) {
DerefdV = State->getSVal(*L);
}
----------------
Szelethus wrote:
> NoQ wrote:
> > Ok, this isn't related to that review, but i still don't understand why
> > this loop terminates. We might skip `void *` above, but we don't skip `void
> > *****`, right? And if we go on dereferencing that, we'll eventually get to
> > a `void *` that can point to anything.
> >
> > A bit more of an intuition dump: I strongly believe that in `getSVal(Loc,
> > T)` the optional argument `T` should be mandatory. Omitting it almost
> > always leads to errors, because there are no types in machine memory, and
> > our `RegionStore` tries to work exactly like machine memory, and
> > `getSVal(Loc)` makes a best-effort guess on what the type of the region is,
> > which in some cases may work reliably (these are exactly the cases where
> > you don't mind passing the type directly), but is usually error-prone in
> > other cases. So whenever i see a `getSVal(Loc)` without a type, i imagine
> > that the code essentially reads raw bytes from the symbolic memory and
> > makes random guesses on what they mean.
> >
> > So the point is "oh we can do better with dynamic type info", it's "we're
> > doing something completely unreliable here". It's not just about void
> > pointers, it's about the fact that the underlying storage simply has no
> > types at all.
> From the code a couple lines back:
>
> if (isVoidPointer(FD)) {
> IsAnyFieldInitialized = true;
> return false;
> }
>
> Note that isn't `Type::isVoidPointer`, I'm calling a statically defined
> function to filter out all void pointer cases.
> ```
> /// Returns whether FD can be (transitively) dereferenced to a void pointer
> type
> /// (void*, void**, ...). The type of the region behind a void pointer isn't
> /// known, and thus FD can not be analyzed.
> bool isVoidPointer(const FieldDecl *FD);
> ```
>
> I see your point, this is a bit dodgy. I'm already working on a fix, and hope
> to upload it along with other important fixes during the next week. It's high
> on my priority list :)
Also note that there are tests in `cxx-uninitialized-object-ptr-ref.cpp` for
`void*`, `void*&`, `void**`, and `void**&&`.
https://reviews.llvm.org/D48325
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
