[cfe-dev] [PATCH] Clang Static Analyzer support for temporary destructors

[Manuel Klimek]

On Tue, Apr 29, 2014 at 3:27 AM, Jordan Rose <jordan_rose at apple.com> wrote:

>
> On Apr 28, 2014, at 9:35 , Manuel Klimek <klimek at google.com> wrote:
>
> Hi Jordan,
>
> I have looked into the remaining problems today; let me quickly summarize
> so we make sure we're on the same page.
> There are basically 2 problems left:
> 1. The invariant
> As you mentioned on the rollback CL, the current layout of the CFG
> regarding temporary destructors violates the invariants assumed by other
> parts of the analyzer: namely, a CFG block with a terminator must not be
> empty.
> After looking into it, I fully agree that we need a principled solution
> here; the question is: do we want to keep the invariant, and fix the CFG
> with temporary destructors to match the invariant, or do we want to say the
> invariant doesn't hold, and we should fix the rest of the code to not
> assume that invariant (and work with the current layout)? (I don't even
> know whether the former is possible theoretically).
> Any hints on what your gut tells you the right solutions is would be
> helpful, and minimize dead-ends ;)
> How would you want the CFG to look here?
>
>
> I'm not actually sure why we care that a CFG block with a terminator is
> not empty. It might be because we expect the terminator condition to have
> been computed within the block, and that it's trying to catch mistakes in
> constructing the CFG. The possible things that could be relying on this:
>
> - maybe we ask for the first CFGElement unilaterally if the block isn't
> the entrance or exit block
> - worse, maybe we assume the block *is* the entrance or exit block if
> it's "empty" (since the terminator isn't included in the element count)
> - other things I haven't thought of
>
> But really I don't think this is an important invariant, and if the right
> solution to (2) involves removing it, then so be it.
>

Ok, I've tried to look into what exactly is happening in
ExprEngine.cpp:1355, ResolveCondition.
I have not yet found out what ResolveCondition should actually do :)
It seems like if it's called with a logical binary operator, it's always
called with the TerminatorCondition of the given CFGBlock.
Most of the time, it will just return that condition (namely if the
condition is the last statement in the block).
If the condition is not the last statement in the block, it will drill down
through the condition into the innermost RHS of logical binary operators,
and then return that.

Can you explain what's going on here? (That's the point where the analyzer
currently assumes the block is not empty, assert(I != E) triggers).

Thanks!
/Manuel


>
>
>
> 2. Taking all the correct branches
> So far I'm not sure I have seen a case where we're not running into (1),
> but I'm not sure (the case where the temporary with the no-return dtor is
> the last in a sequence of binary logical ops seems to be (1) at least).
> The correct solution seems to be to add tracking lifetime of temporaries
> in conditionals correctly. alexmc is working on a patch here.
>
>
> I agree; it's the particular value of "correctly" that makes the problem
> difficult. ;-) Very glad Alex has been able to spend time on this.
>
> Jordan
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20140429/1e77a967/attachment.html>