Szelethus added inline comments.

================
Comment at: 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp:187-191
   // If FR is a pointer pointing to a non-primitive type.
   if (Optional<nonloc::LazyCompoundVal> RecordV =
           DerefdV.getAs<nonloc::LazyCompoundVal>()) {
 
     const TypedValueRegion *R = RecordV->getRegion();
----------------
NoQ wrote:
> Szelethus wrote:
> > NoQ wrote:
> > > This looks like one more situation where we dereference a location to get 
> > > a value and then struggle to get back to the location that we've 
> > > dereferenced by looking at the value. Can we just use `V`?
> > I've struggled with derefencing for months now -- I'm afraid I just don't 
> > really get what you'd like to see here.
> > 
> > Here's what I attempted to implement:
> > I'd like to obtain the pointee's region of a `Loc` region, even if it has 
> > to be casted to another type, like through void pointers and 
> > `nonloc::LocAsInteger`, and continue analysis on said region as usual.
> > 
> > The trickiest part I can't seem to get right is the acquisition of the 
> > pointee region. For the problem this patch attempts to solve, even though 
> > `DynT` correctly says that the dynamic type is `DynTDerived2 *`, `DerefdV` 
> > contains a region for `DynTBase`.
> > 
> > I uploaded a new patch, D51057, which hopefully settles derefence related 
> > issues. Please note that it **does not **replace this diff, as the acquired 
> > region is still of type `DynTBase`.
> > 
> > I find understanding these intricate details of the analyzer very 
> > difficult, as I found very little documentation about how this works, which 
> > often left me guessing what the proper way to do this is. Can you recommend 
> > some literature for me on this field?
> > Can you recommend some literature for me on this field?
> 
> This is pretty specific to our analyzer. `SVal`/`SymExpr`/`MemRegion` 
> hierarchy is tightly coupled to implementation details of the `RegionStore`, 
> which is our memory model. There's a paper on it [1]. We have some in-tree 
> documentation of the `RegionStore` [2] (other docs there are also interesting 
> to read). And there's my old workbook [3]. And i guess that's it.
> 
> [1] Xu, Zhongxing & Kremenek, Ted & Zhang, Jian. (2010). A Memory Model for 
> Static Analysis of C Programs. 535-548. 10.1007/978-3-642-16558-0_44.
> [2] 
> https://github.com/llvm-mirror/clang/blob/master/docs/analyzer/RegionStore.txt
> [3] 
> https://github.com/haoNoQ/clang-analyzer-guide/releases/download/v0.1/clang-analyzer-guide-v0.1.pdf
Thank you! I'm aware of these works, and have read them already.

The actual implementation of the analyzer is most described in your book, and 
I've often got the best ideas from that. However, some very well described 
things in there have absolutely no documentation in the actual code -- for 
example, it isn't obvious at all to me what `CompundValue` is, and I was 
surprised to learn what it does from your guide. Other examples are the 
difference between `TypedValueRegion`s `getLocationType` and `getValueType`, 
`SymExpr` in general, and so on.

I think it would also be very valuable to have a link in `docs/analyzer` to 
your book. On another note, would you mind if I were to put some of the 
information from there into the actual code?


================
Comment at: test/Analysis/cxx-uninitialized-object-inheritance.cpp:787
   // TODO: we'd expect the note: {{uninitialized field 'this->x'}}
   int x; // no-note
 };
----------------
NoQ wrote:
> Szelethus wrote:
> > The checker should be able to catch this one -- for some reason it is 
> > regarded as an unknown region. Odd, as the test case right after this one 
> > works perfectly.
> There's a variety of problems we have with empty base classes, might be one 
> of those, and they are usually easy to fix because, well, yes it's a special 
> case, but it's also an extremely simple case.
> 
> I encourage you to open up the Exploded Graph and study it carefully to see 
> what and where goes wrong (not for this revision).
I'd love to learn about other parts of the analyzer (besides chasing pointers), 
but for now, this seems to have fixed itself ;)


https://reviews.llvm.org/D50892



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to