Inlining temporary destructor calls...

Hi Jordan,

I’ve started to look into inlining temporary destructors, but I’m hitting a wall.
Specifically, I think I fail at finding the right MemRegion in ProcessTemporaryDtor (there is a comment in there, that says we need the region for the VisitCXXDestructor call).

Any help / pointers / brain-dumps on how to do it would be highly appreciated.

Thanks!
/Manuel

Hm. I haven't thought about it enough to give you a fool-proof plan, but one possibility could be to store the MemRegion in the state at the point of the CXXBindTemporaryExpr, rather than just a boolean flag. I don't quite know what the ramifications of that would be, though.

(By the time we get to the destructor, the value of the CXXBindTemporaryExpr is almost certainly no longer in the Environment.)

Jordan

> Hi Jordan,
>
> I've started to look into inlining temporary destructors, but I'm
hitting a wall.
> Specifically, I think I fail at finding the right MemRegion in
ProcessTemporaryDtor (there is a comment in there, that says we need the
region for the VisitCXXDestructor call).
>
> Any help / pointers / brain-dumps on how to do it would be highly
appreciated.

Hm. I haven't thought about it enough to give you a fool-proof plan, but
one possibility could be to store the MemRegion in the state at the point
of the CXXBindTemporaryExpr, rather than just a boolean flag. I don't quite
know what the ramifications of that would be, though.

(By the time we get to the destructor, the value of the
CXXBindTemporaryExpr is almost certainly no longer in the Environment.)

Are there other cases in which we store the MemRegion so we make sure it
doesn't get expunged from the Environment, so I can take a look at similar
code?

Thanks,
/Manuel

It’s not a “so it doesn’t get purged from the Environment”—it’s most likely leaving the Environment no matter what. Instead, you just keep a reference to the MemRegion. CStringChecker does something a bit like this.

…but wait, we need the contents of the region to not be cleaned out of the state either. So maybe there’s a better way to go: change the LiveVariables analysis to not consider the CXXBindTemporaryExpr dead until the end of the whole full-expression. (And then do, uh, something about lifetime extension, which isn’t quite handled correctly right now anyway.)

Jordan

How does this currently work for normal variables?
I expect in:
{
  A a;
  ...
}
the MemRegion for 'a' is still available at the end of the block? At least,
if I look into the handing of all other types of destructors in
ExprEngine.cpp, it looks like they just unconditionally conjure up the
MemRegion for the destructed object.

Also, do you have any tips on how to effectively debug those issues? I'm
basically trying to dump the state etc, but for temporaries it's quite hard
to see which part of the state belongs to them (I'll probably need to write
a patch to make that better? :slight_smile:

That’s also handled by LiveVariables. That analysis tracks which expressions and which variables are live at a given statement, and then those can be used as roots when deciding what regions and symbols are still reachable. Checkers can also mark symbols and regions reachable explicitly (see the SymbolReaper class). We somehow need to keep the object region alive until we can run the destructor.

Since CFGTemporaryDtor knows which CXXBindTemporaryExpr it uses, it might be possible to just mark that expression live at that point. It looks like we’re already doing basically the same thing for local variable destructors; see LiveVariablesImpl::runOnBlock.

Checkers can augment how the state is dumped to include information from the GDM (or any information, really). Maybe a new debug checker?

Jordan

> Hi Jordan,
>
> I've started to look into inlining temporary destructors, but I'm
hitting a wall.
> Specifically, I think I fail at finding the right MemRegion in
ProcessTemporaryDtor (there is a comment in there, that says we need the
region for the VisitCXXDestructor call).
>
> Any help / pointers / brain-dumps on how to do it would be highly
appreciated.

Hm. I haven't thought about it enough to give you a fool-proof plan, but
one possibility could be to store the MemRegion in the state at the point
of the CXXBindTemporaryExpr, rather than just a boolean flag. I don't quite
know what the ramifications of that would be, though.

(By the time we get to the destructor, the value of the
CXXBindTemporaryExpr is almost certainly no longer in the Environment.)

Are there other cases in which we store the MemRegion so we make sure it
doesn't get expunged from the Environment, so I can take a look at similar
code?

It's not a "so it doesn't get purged from the Environment"—it's most
likely leaving the Environment no matter what. Instead, you just keep a
reference to the MemRegion. CStringChecker does something a bit like this.

...but wait, we need the contents of the region to not be cleaned out of
the state either. So maybe there's a better way to go: change the
LiveVariables analysis to not consider the CXXBindTemporaryExpr dead until
the end of the whole full-expression. (And then do, uh, something about
lifetime extension, which isn't quite handled correctly right now anyway.)

How does this currently work for normal variables?
I expect in:
{
  A a;
  ...
}
the MemRegion for 'a' is still available at the end of the block? At
least, if I look into the handing of all other types of destructors in
ExprEngine.cpp, it looks like they just unconditionally conjure up the
MemRegion for the destructed object.

That's also handled by LiveVariables. That analysis tracks which
expressions and which variables are live at a given statement, and then
those can be used as roots when deciding what regions and symbols are still
reachable. Checkers can also mark symbols and regions reachable explicitly
(see the SymbolReaper class). We somehow need to keep the object region
alive until we can run the destructor.

Since CFGTemporaryDtor knows which CXXBindTemporaryExpr it uses, it might
be possible to just mark that expression live at that point. It looks like
we're already doing basically the same thing for local variable
destructors; see LiveVariablesImpl::runOnBlock.

Thanks, that helps a lot.