<div dir="ltr"><div class="gmail_quote"><div>+jordan who has more context, too (Bhargava has expressed interest in helping to fix the remaining problems)<br></div><div>(cc' delesley mainly for FYI, as he was curious about some of the limitations of the C++ cfg)</div><div><br></div><div>The state of the temporary destructor problem is the following (Jordan will correct me if I'm wrong):</div><div>Currently, the SA doesn't cannot inline (which in SA terms means "symbolically execute") constructors and destructors of temporary values of class type due to a couple of problems.</div><div><br></div><div>1. The CFG doesn't handle lifetime-extended temporaries correctly.</div><div>I had a patch out that addressed this (<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D4706&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=CnzuN65ENJ1H9py9XLiRvC_UQz6u3oG6GUNn7_wosSM&m=Ll-2IiBhR1agWyZoAnEdy01U_mRxyCbZdZmlYhJwGBo&s=MxabHAdNnXQb_saKYUbFdevAGUCgZuj2qzl99Ak8b8w&e=">http://reviews.llvm.org/D4706</a>); look at the changes to CFG.cpp for what the CFG changes necessary here. Basically, the core of the problem is that lifetime extended temporaries have a rather complex encoding in clang's AST, and one has to actually run over the full statement to see what comes out as liftime extended; to construct the CFG, the static analyzer runs over the AST multiple times (iirc):</div><div>a) it figures out the scopes needed (for this it needs to figure out which variables are lifetime extended)</div><div>b) it runs again to fill in the scopes</div><div>c) during b) it visits parts again to figure out the temporary destructors that need to run</div><div>The fact that it visits multiple times makes the code contain a bit of duplication (see the patch) that I wanted to remove, but never got around to. I think the patch was pretty close to being able to be submitted, and I don't fully remember what was missing (iirc some tests were breaking and I didn't know how to deal with it).</div><div>I think getting that patch submitted would be a great way to get started on this :)</div><div><br></div><div>2. The static analyzer has an incorrect model for the representation of temporaries; it encodes them as immutable symbolic values, which they are not (they have memory regions that the constructor / destructor or other methods on the temporary need to work on). This is a rather intricate change. During the last LLVM dev meeting, Jordan had hacked together a proof of concept for some of this, but so far I haven't gotten back around to pull out all those patches, understand them, and clean them up (patch from Jordan from back then attached).</div><div><br></div><div>3. An idea for an intermediate step towards (1) was to only inline temporary constructors/destructors if we know they are not bound to parameters (which is where the problem with the memory region hits). Patch <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D4740&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=CnzuN65ENJ1H9py9XLiRvC_UQz6u3oG6GUNn7_wosSM&m=Ll-2IiBhR1agWyZoAnEdy01U_mRxyCbZdZmlYhJwGBo&s=F8N75m6dXTGySGuMiUAJ1JyRQtm1C6FbwfaCxyYRnhE&e=">http://reviews.llvm.org/D4740</a> was an attempt to go that way (this is similar as the patch in (1)).</div><div><br></div><div>Let me know if you have more questions :)</div><div><br></div><div>Cheers,</div><div>/Manuel</div><div dir="ltr"><br></div><div dir="ltr">On Tue, Mar 17, 2015 at 4:02 PM Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote">On Tue, Mar 17, 2015 at 4:00 PM Bhargava Shastry <<a href="mailto:bshastry@sec.t-labs.tu-berlin.de" target="_blank">bshastry@sec.t-labs.tu-berlin.de</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On a tangent, does Google use Clang SA on large codebases esp. Chromium<br>
that has a massive C++ LoC count? If not, what are the top reasons for<br>
not doing so? Lack of C++ support seems to be the Google position on<br>
this [1] but am wondering if that is the only reason.<br></blockquote><div><br></div></div></div><div dir="ltr"><div class="gmail_quote"><div>Yes, this is the only reason; we'd *love* to be able to use it. I already shot considerable time and effort into trying to get this good enough, but I think I'd need to spend another couple of weeks, which I currently just don't have.</div></div></div><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
[1]: <a href="https://urldefense.proofpoint.com/v2/url?u=https-3A__code.google.com_p_chromium_wiki_ClangStaticAnalyzer&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=CnzuN65ENJ1H9py9XLiRvC_UQz6u3oG6GUNn7_wosSM&m=Ll-2IiBhR1agWyZoAnEdy01U_mRxyCbZdZmlYhJwGBo&s=zakn8LsXOIQ954bgTtrdq7xYAe2MlxSSISt77b3zn3I&e=" target="_blank">https://code.google.com/p/<u></u>chromium/wiki/<u></u>ClangStaticAnalyzer</a><br>
<br>
Regards,<br>
Bhargava<br>
<br>
On 03/17/2015 03:00 PM, Manuel Klimek wrote:<br>
> On Tue, Mar 17, 2015 at 2:50 PM Bhargava Shastry<br>
> <<a href="mailto:bshastry@sec.t-labs.tu-berlin.de" target="_blank">bshastry@sec.t-labs.tu-<u></u>berlin.de</a><br>
> <mailto:<a href="mailto:bshastry@sec.t-labs.tu-berlin.de" target="_blank">bshastry@sec.t-labs.<u></u>tu-berlin.de</a>>> wrote:<br>
><br>
> Hi,<br>
><br>
> On 03/17/2015 01:09 PM, Manuel Klimek wrote:<br>
> > How can you prove a comparison against garbage value from that code?<br>
> > Seems like somebody can set m_x to anything between the<br>
> constructor and<br>
> > the call to method.<br>
> > If you want to catch this, you'll at least need:<br>
> > void f() {<br>
> > foo f;<br>
> > f.method();<br>
> > }<br>
><br>
> Apologies for having left out the crucial function that instantiates a<br>
> foo object. Agree that this is the missing piece.<br>
><br>
> > ... and then the SA needs to "inline" both the call to the constructor<br>
> > and the method call to see the problem.<br>
><br>
> My understanding is that, during symbolic execution, Clang SA ``visits"<br>
> function calls in the procedure under analysis. So, in the function void<br>
> f() above, Clang SA would metaphorically step into foo's constructor and<br>
> subsequently method() and prove garbage value in two steps i.e.,<br>
><br>
><br>
> Yes, that's what the SA calls "inlining". I agree that it's confusing :)<br>
><br>
><br>
><br>
> Step 1. Call to f.method() from void f()<br>
> Step 2. Garbage value comparison in method()<br>
><br>
> Is inlining how Clang SA really does this? Afaik, Clang SA visits the<br>
> call graph for a translation unit in topological order. In the example,<br>
> this means, when void f() is being analyzed, both ctor declaration and<br>
> method declarations would be visited, no?<br>
><br>
><br>
> Well, it depends. Whether the SA drills into a function depends on many<br>
> things.<br>
><br>
><br>
><br>
><br>
> Regards,<br>
> Bhargava<br>
><br>
> --<br>
> Bhargava Shastry <<a href="mailto:bshastry@sec.t-labs.tu-__berlin.de" target="_blank">bshastry@sec.t-labs.tu-__<u></u>berlin.de</a><br>
> <mailto:<a href="mailto:bshastry@sec.t-labs.tu-berlin.de" target="_blank">bshastry@sec.t-labs.<u></u>tu-berlin.de</a>>><br>
> Security in Telecommunications<br>
> TU Berlin / Telekom Innovation Laboratories<br>
> Ernst-Reuter-Platz 7, Sekr TEL 17 / D - 10587 Berlin, Germany<br>
> phone: +49 30 8353 58235<br>
><br>
<br>
--<br>
Bhargava Shastry <<a href="mailto:bshastry@sec.t-labs.tu-berlin.de" target="_blank">bshastry@sec.t-labs.tu-<u></u>berlin.de</a>><br>
Security in Telecommunications<br>
TU Berlin / Telekom Innovation Laboratories<br>
Ernst-Reuter-Platz 7, Sekr TEL 17 / D - 10587 Berlin, Germany<br>
phone: +49 30 8353 58235<br>
</blockquote></div></div></blockquote></div></div>