[cfe-dev] CFG support for temporary object desctructors

[Jordan Rose]

Hi, Pavel. It's great to hear that you're interested in this. 

As I remember from last time I looked at this, there are three things holding us back from turning on support for temporary destructors in the analyzer. The first is that the analyzer is not wired up to handle destructor CFG nodes for arbitrary regions. I don't think this is a major blocking issue, but it might take a bit of threading through all the code.

The second is a lot more difficult. When you pass temporaries to a function, what happens when they are destroyed?

void byRef(const Object &ref);
void byVal(Object val);

byRef(Object())
byVal(Object())

The destructor here has to run at the end of the full-expression containing the call (roughly, at the outermost expression). However, the function call may have changed some of the object's contents (even 'byRef', if the object has fields marked 'mutable'). We need to make sure that's reflected when the chosen destructor is inlined.

The inlining problem is compounded by the fact that we decay structs to a collection of values (nonloc::LazyCompoundVal) whenever we need to treat them as rvalues. This is usually the right thing to do, but has very confusing results for temporaries being copied in and out of functions. According to the standard, the copy constructor happens in the caller (and that's how it appears in the AST), but the region it's being copied into is based on a ParmVarDecl that's part of a StackFrameContext for inlining the function...which we may not decide to inline after all. Ignoring temporary destructors our current behavior is indistinguishable from the standard, but as soon as we start claiming to support temporary destructors we're going to hit this problem.

The third problem is that we simply haven't put time into qualifying and validating the temporary destructor logic. Marcin implemented it a long time ago in the CFG, then Chandler and Doug made sure it was in good enough condition to use for the analysis-based warnings, but we haven't actually tested it in the analyzer.

Now, all of that said, you're only interested in making 'noreturn' work right now, so for that it seems reasonable to treat the destructors for temporary objects as opaque in the analyzer. It may turn out that we were properly conservative in everything we've done so far that turning it on will just work, but I'd like to see a fair number of test cases before we start shipping that.

'noreturn' is actually the simplest thing here: that's just a change in the shape of the CFG, not necessarily in its use. It's all the other destructors that come along for the ride that worry me. (And the setup of the call to the noreturn destructor as well.)

What do you think?
Jordan

P.S. I asked "what do you think?", but I'm going to be on vacation for the next week, so please don't expect an immediate response. Sorry about the time for this response.


On Jun 20, 2013, at 2:00 , Pavel Labath <labath at google.com> wrote:

> Greetings,
> 
> I would be interested in working on bug <http://llvm.org/bugs/show_bug.cgi?id=15599>, dealing with noreturn destructors. From the bug I've learned that this happens because the CFG does not have proper support for temporary destructors and that adding this is a "hard problem".
> 
> I've done some experiments with enabling cfg-temporary-dtors analyzer option and to me the resulting CFG looks exactly like I would expect it to be. I also tried some examples with lifetime extension of temporaries and those looked ok too.
> 
> So, my question is: What am I missing? Is there some known problem with the CFG when this option is enabled? If not, why is this option not enabled by default? When turning this option on I get a test failure in two other tests, but if this is not a result of some CFG bug, I can take a look and try to fix these issues instead.
> 
> I would appreciate any thoughts or directions where to look.
> regards,
> pl

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20130624/98fa68a6/attachment.html>