[cfe-dev] Inlining temporary destructors in the analyzer

[Manuel Klimek]

On Thu, Jul 31, 2014 at 5:19 AM, Jordan Rose <jordan_rose at apple.com> wrote:

>
> On Jul 30, 2014, at 8:14 , Manuel Klimek <klimek at google.com> wrote:
>
> Hi Jordan,
>
> I made some progress on inlining temporary destructors. My main problem
> currently is that somehow the initializer values in my constructor get lost
> if I don't also reference 'this' via some other way.
>
> An examples shows that best. Consider:
> struct A {
>   A(int* y) : z(y) {}
>   ~A() { *z = 23; }
>   int *z;
> };
> void f() {
>   int x = 0;
>   {(A(&x));}
> }
>
> This leads to a "Dereference of undefined pointer value" warning on *z =
> 23.
> Funnily enough this warning goes away (and all values get correctly
> propagated) if change the constructor to be:
> A(int* y) : z(y) { (void)this; }
>
> Obviously this also works fine if I use a local variable instead of a
> temporary.
>
> Any ideas what might be going wrong here would be greatly appreciated.
>
>
> My guess is that the temporary is an rvalue, and in the analyzer a NonLoc,
> and that the only reason this even appears to work is because we needed to
> be able to call *methods* on temporaries and so will create a temporary
> region for them on demand to serve as the 'this' pointer. However, I don't
> think we expect that to happen for *constructors,* and so we don't keep
> the region around after the constructor has finished executing. (That is,
> when it's just a method being called—and neither the constructor nor
> destructor are inlined—it doesn't actually matter what you do to the
> contents of the region, because nothing else can access it.)
>
>
So, funnily enough if I remove this code:
  if (isTemporaryPRValue(CCE, ThisV))
  ThisV = state->getSVal(ThisV.castAs<Loc>());
which puts the temp region into a lazycompoundregion, the memory seems to
be correctly handled. I have not yet looked what else this breaks though.


> I am a little confused as to how your workaround actually changes
> anything, but regardless there is an issue here.
>
> To fix this, we may need to treat temporaries as Loc values always, and
> figure out how to teach the rest of the analyzer to ignore the fact that
> the Expr says it's an rvalue. Alternately, we could update the compiler to
> actually treat these as xvalues (which are glvalues), and therefore we'd
> have a location we could deal with. I vaguely remember talking to Richard
> about this at one point and him thinking it to be a reasonable idea.
>
> Or I could be totally off-base on this, because I'm coming up with this
> from memory rather than actually digging into the program trace.
>
> Jordan
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140731/62e76e65/attachment.html>