[cfe-dev] Inlining temporary destructors in the analyzer

[Manuel Klimek]

On Thu, Jul 31, 2014 at 9:12 AM, Richard Smith <richard at metafoo.co.uk>
wrote:

> 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.
>

Not all temporaries have MaterializeTemporary though (some just have a
CXXBindTemporaryExpr if they die after the full expression). How should we
handle that case then?
Also, while we're at it - would it make sense to have the
MaterializeTemporaryExpr somehow reference the CXXBindTemoprary it handles
(iff Materialize actually materializes a C++ object). I imagine we'd run
into problems with conditional operators, though, where the Materialize can
reference one of multiple possible constructed objects (the
CXXBindTemporary is inside the conditional operator branches).

Cheers,
/Manuel


>
> 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/405eac2a/attachment.html>