[cfe-dev] Inlining temporary destructors in the analyzer

[Richard Smith]

On Thu, Jul 31, 2014 at 12:36 PM, Jordan Rose <jordan_rose at apple.com> wrote:

>
> On Jul 31, 2014, at 1:20 , Manuel Klimek <klimek at google.com> wrote:
>
> 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.
>
>
> Okay, analyzer design time. :-)
>
> C++ has a distinction between lvalues and rvalues (actually glvalues and
> prvalues). There are a number of things that go into this distinction, but
> the part the analyzer cares about is that glvalues refer to true objects
> and prvalues do not. (In C++ parlance, "object" refers to anything with
> storage, a type, and lifetime, not just class instances. In the analyzer,
> we call such things "regions".)
>
> If you were to build a very literal C++ compiler, every glvalue would be
> represented by an address. PRValues*, on the other hand, don't really need
> addresses; they only exist to be used transiently.
>
> ** How do you capitalize "prvalue"?*
>
> The analyzer pretty much always believes the compiler when it comes to
> glvalues vs. prvalues. That means that the value of a glvalue expression is
> *always* a Loc. A prvalue expression, on the other hand, will only be a
> Loc if it's a pointer type*. If you're talking about a struct prvalue, what
> you're really talking about is the bindings in its memory region—it has no
> identity. This is represented by a CompoundVal or LazyCompoundVal. This
> model worked great for C, but it's starting to show strain in C++.
>
> ** or one of the other types mentioned in Loc::isLocType. But not all of
> those can be expression values.*
>
> Besides "regular" temporaries, this is also a problem for prvalue function
> arguments. When the analyzer inlines a function, it just blithely binds the
> arguments to the proper VarRegions created for each ParmVarDecl in the
> current stack frame. That doesn't really fly if we're supposed to be
> calling the copy constructor. Worse, the copy constructor call happens
> *outside* of the function call, which means it happens before we've even
> decided whether or not to inline, which means there are no VarRegions yet
> to construct into. We've/I've been putting off dealing with this one for a
> while; we get around it mostly because we *don't inline constructors for
> temporaries.*
>
> The general temporary problem goes to what Richard was saying: a temporary
> expression is an rvalue, but if we have to run a constructor then clearly
> there *was* a region at some point.
>

Here's how I think of this:

A glvalue expression is a function from the environment to a region.
A prvalue expression is a function of (environment, region) that produces
no output.

That is, glvalue expressions tell you how to find a region, and prvalue
expressions tell you how to initialize a region.

As a special case, you probably shouldn't bother to create a region when
evaluating prvalues of non-class, non-array types (neither IRGen nor the
constant evaluator do so). The standard blesses this with its "value that
is not associated with an object" wording, but you can't actually observe
the difference.

The C++11 standard (at least the version I have) is not wonderful about
> classifying temporaries ([basic.lval]p1):
>
> - An *lvalue*...designates a function or an object.
>
> - An *xvalue* (an “eXpiring” value) also refers to an object, usually
> near the end of its lifetime.
>
> - A *glvalue* (“generalized” lvalue) is an lvalue or an xvalue.
>
> - An *rvalue...*is an xvalue, a temporary object (12.2) or subobject
> thereof, or a value that is not associated with an object.
>
> - A *prvalue* (“pure” rvalue) is an rvalue that is not an xvalue.
>
>
> So temporary objects are prvalues. Even though they are modifiable
> (defined later in the same section). Even though with construction and
> destruction they end up needing a fixed region, or at least consistency in
> region contents.
>
> I don't have a great answer here. My guess is we'll probably end up
> turning on *some* inlining for temporaries to start, but not all.
>
> Oh, right, so what's that 'isTemporaryPRValue' doing? That's taking the
> few cases where the AST wants a prvalue but the analyzer really needs a
> valid Loc, and conjuring a region on the fly. That's important for things
> that need to have referential identity, like the 'this' pointer in a method
> called on a temporary, or (say) a temporary that's been lifetime-extended.
>
> Yuck, right? Just making all temporaries into xvalues would most likely
> make 60% of these problems go away.
> 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/ea2cd915/attachment.html>