[cfe-dev] Inlining temporary destructors in the analyzer

[Richard Smith]

On Wed, Jul 30, 2014 at 8:19 PM, 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.)
>
> 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.
>

C++ itself is very definitely moving in this direction. When you have:

  struct S { S(); int n; };

the expression S() is a prvalue but S().n is now an xvalue. Clang doesn't
implement that yet, but the obvious way to do it is to create

  MemberExpr xvalue .n
  `- MaterializeTemporaryExpr xvalue
    `- CXXTemporaryObjectExpr prvalue S()

... that is, materialize a temporary any time member access is performed.
(The same thing happens when array indexing is performed on an array
temporary.) This naturally extends to also materializing a temporary when a
member function is called on it.

FWIW, this representation would also fix the wrong lifetime extension in a
case like:

  int &&r = S().n;

... where we currently fail to lifetime-extend the S object because we
never materialized it.

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
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140731/48e54133/attachment.html>