<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On 6 August 2013 02:31, Jordan Rose <span dir="ltr"><<a href="mailto:jordan_rose@apple.com" target="_blank">jordan_rose@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>
  I downloaded this patch and rebuilt, and this already doesn't pass the destructors::testConsistency test in temporaries.cpp. That might match your "I don't think the patch is complete yet".<br>
<br>
  More importantly, however, your test case doesn't actually trigger the environment cleanup. Try adding a trivial inlined function ("return true") and I think you'll see there's a problem. Liveness isn't cumulative as you go backwards; it's more like a flashlight, where a small set of live things are true. We're not currently taking so much advantage of this, but it's convenient to know the difference between an expression that //happens// not to have a value and one that //should// not have a value.<br>

<br>
  ```if (isa<Expr>(S)) {<br>
    val.liveStmts = LV.SSetFact.remove(val.liveStmts, S);<br>
  }```<br>
<br>
  I'd be happy if your conjecture does turn out to be true (and admit that LiveVariables was written a long time ago and so my mental model is rusty), but I think it's going to turn out to be more work than that.<br>

<br>
<a href="http://llvm-reviews.chandlerc.com/D1259" target="_blank">http://llvm-reviews.chandlerc.com/D1259</a><br>
</blockquote></div><br></div><div class="gmail_extra">Ok, the destructor problem solved. It wasn't that hard, I just panicked. :) (But the question still remains whether to run the array destructor before, or after the elements'. Or it does not matter.)</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">Now, back to the liveness issue. I re-ran the analyzer with debug output on the following program:</div><div class="gmail_extra"><div class="gmail_extra">struct A {</div>
<div class="gmail_extra">  A();</div><div class="gmail_extra">  ~A();</div><div class="gmail_extra">  operator bool() const;</div><div class="gmail_extra">};</div><div class="gmail_extra"><br></div><div class="gmail_extra">
bool u();</div><div class="gmail_extra"><br></div><div class="gmail_extra">bool f() { return false; }</div><div class="gmail_extra"><br></div><div class="gmail_extra">void ff() {</div><div class="gmail_extra">  if (((f(), u()) || (f(), u())) && (f(), A())) {</div>
<div class="gmail_extra">  } else {</div><div class="gmail_extra">  }</div><div class="gmail_extra">}</div><div><br></div><div>And I still get the same thing:</div><div><div>----- Position: 4.0</div><div>Store (direct and default bindings), 0x43c5508 :</div>
<div> (GlobalInternalSpaceRegion,0,default) : conj_$3{int}</div><div><br></div><div> (GlobalSystemSpaceRegion,0,default) : conj_$4{int}</div><div><br></div><div><br></div><div><br></div><div>Expressions:</div><div> (0x43b7b10,0x43ac188) f() : 0 S8b</div>
<div> (0x43b7b10,0x43ac230) u : &code{u}</div><div> (0x43b7b10,0x43ac248) u() : conj_$5{_Bool}</div><div> (0x43b7b10,0x43ac270) f() , u() : conj_$5{_Bool}</div><div> (0x43b7b10,0x43ac2b8) (f() , u()) || (f() , u()) : 0 S8b</div>
<div>Ranges of symbol values:</div><div> conj_$5{_Bool} : { [0, 0] }</div><div>Expression 0x43ac2b8 is live.</div><div>Attempting to use saved value of 0x43ac2b8...</div><div>Saving condition value of 0x43ac738 (0)</div><div>
Attempting to use saved value of 0x43ac760...</div></div><div><br></div><div><br></div><div>And I am pretty sure I am triggering the environment cleanup, because the "Expression ... is live." line is actually output from within the removeDeadBindings() function:</div>
<div><div>--- a/lib/StaticAnalyzer/Core/Environment.cpp</div><div>+++ b/lib/StaticAnalyzer/Core/Environment.cpp</div><div>@@ -166,6 +166,7 @@ EnvironmentManager::removeDeadBindings(Environment Env,</div><div>     const SVal &X = I.getData();</div>
<div> </div><div>     if (SymReaper.isLive(BlkExpr.getStmt(), BlkExpr.getLocationContext())) {</div><div>+      llvm::errs() << "Expression " << BlkExpr.getStmt() << " is live.\n";</div>
<div>       // Copy the binding to the new map.</div><div>       EBMapRef = EBMapRef.add(BlkExpr, X);</div></div><div><br></div><div><br></div><div>The code you quoted above does not contradict any of this. It is responsible for removing the expression from the live set once we encounter that expression in the cfg. Because the liveness analysis runs backwards, the expression, once marked as live will stay live until encountering the said expression in the cfg. Putting that in the normal direction, it will be live from the moment it is produced, until we reach the last consumer of the expression.</div>
<div><br></div><div>cheers,</div><div>pl</div></div></div>