<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<br>
<br>
<div class="moz-cite-prefix">On 8/8/19 2:05 PM, Kristóf Umann wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAGcXOD6c=-AOdHfFMpfpscg6e43Q=-jkKaQk7A2VmTvuYiD7Qg@mail.gmail.com">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<div dir="ltr">
<div dir="ltr">
<div class="gmail_attr">I got another round! The following new
run names were added:</div>
<div class="gmail_attr"><br
class="gmail-Apple-interchange-newline">
.* + all non-control dependency changes: [1, 2] applied<br
class="gmail-Apple-interchange-newline">
.*AFTER tracking conditions + all non-control dependency
changes: [1, 2] applied, "track-conditions" enabled</div>
.*AFTER tracking conditions FINAL: "track-conditions" enabled,
[1-8] applied (also with ~10 or so NFC patches)<br
class="gmail-Apple-interchange-newline">
.*AFTER tracking conditions FINAL + DEBUG notes:
"track-conditions" and "track-conditions-debug" enabled, [1-8]
applied (also with ~10 or so NFC patches)</div>
<div dir="ltr"><font face="Arial"><span style="white-space:pre-wrap">
</span></font><span style="font-variant-numeric:normal;font-variant-east-asian:normal;font-family:Arial;vertical-align:baseline;white-space:pre-wrap">[1] </span><a
href="https://reviews.llvm.org/D64232" target="_blank"
style="text-decoration-line:none" moz-do-not-send="true"><span style="font-family:Arial;font-variant-numeric:normal;font-variant-east-asian:normal;text-decoration-line:underline;vertical-align:baseline;white-space:pre-wrap">https://reviews.llvm.org/D64232</span></a><span style="font-variant-numeric:normal;font-variant-east-asian:normal;font-family:Arial;vertical-align:baseline;white-space:pre-wrap"> [analyzer] Prune calls to functions with linear CFGs that return a non-zero constrained value</span> <br>
[2] <a href="https://reviews.llvm.org/D64287" target="_blank"
moz-do-not-send="true">https://reviews.llvm.org/D64287</a> [analyzer]
Track the right hand side of the last store regardless of its
value<br>
<span style="font-variant-numeric:normal;font-variant-east-asian:normal;font-family:Arial;vertical-align:baseline;white-space:pre-wrap">[3] </span><a
href="https://reviews.llvm.org/D64270" target="_blank"
style="text-decoration-line:none" moz-do-not-send="true"><span style="font-family:Arial;font-variant-numeric:normal;font-variant-east-asian:normal;text-decoration-line:underline;vertical-align:baseline;white-space:pre-wrap">https://reviews.llvm.org/D64270</span></a><span style="font-variant-numeric:normal;font-variant-east-asian:normal;font-family:Arial;vertical-align:baseline;white-space:pre-wrap"> [analyzer][NFC] Prepare visitors for different tracking kinds</span><br>
<div>
<div><span
id="gmail-m_-2694677468341043285gmail-m_2838973005318108645gmail-m_-1755177567490753650gmail-docs-internal-guid-6633fc6c-7fff-4cb0-ba03-da8b5433cf09">
<p dir="ltr"
style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-variant-numeric:normal;font-variant-east-asian:normal;font-family:Arial;vertical-align:baseline;white-space:pre-wrap">[4] </span><a
href="https://reviews.llvm.org/D64272"
target="_blank" style="text-decoration-line:none"
moz-do-not-send="true"><span style="font-family:Arial;font-variant-numeric:normal;font-variant-east-asian:normal;text-decoration-line:underline;vertical-align:baseline;white-space:pre-wrap">https://reviews.llvm.org/D64272</span></a><span style="font-variant-numeric:normal;font-variant-east-asian:normal;font-family:Arial;vertical-align:baseline;white-space:pre-wrap"> [analyzer] Note last writes to a condition only in a nested stackframe</span></p>
</span></div>
</div>
<div class="gmail_attr">[5] <a
href="https://reviews.llvm.org/D65287"
moz-do-not-send="true">https://reviews.llvm.org/D65287</a> [analyzer][CFG]
Don't track the condition of asserts<br>
</div>
<div class="gmail_attr">[6] <a
href="https://reviews.llvm.org/D65575"
moz-do-not-send="true">https://reviews.llvm.org/D65575</a> [analyzer]
Mention whether an event is about a condition in a bug
report part 1</div>
<div class="gmail_attr">[7] <a
href="https://reviews.llvm.org/D65724"
moz-do-not-send="true">https://reviews.llvm.org/D65724</a> [analyzer]
Don't make ConditionBRVisitor events prunable when the
condition is an interesting field</div>
<div class="gmail_attr">[8] <a
href="https://reviews.llvm.org/D65725"
moz-do-not-send="true">https://reviews.llvm.org/D65725</a> [analyzer]
Mention whether an event is about a condition in a bug
report part 2</div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr">Conclusions:<br>
</div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr">Shockingly, no condition tracking
related changes were made in the FINAL run for either
Bitcoin or Xerces, so we only have LLVM/Clang to talk about.
This time, I added comments, which, after clicking on a
report, can be retrieved by clicking on 'Comments' in the
upper right corner. These comments are far more precise than
the notes I left previously.</div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr"><a
href="http://cc.elte.hu:15001/GSoC2019-July-5/#is-unique=on&run=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL"
moz-do-not-send="true">http://cc.elte.hu:15001/GSoC2019-July-5/#is-unique=on&run=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL</a><br>
</div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr">The negatives:</div>
<div class="gmail_attr">1. Extra notes to functions following
this pattern were very-very common:</div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr"><font face="monospace">bool a(int b) {<br>
return b == 0;</font>
<div class="gmail_attr"><font face="monospace"> // Assuming
'b' is equal to 0</font></div>
<div class="gmail_attr"><font face="monospace">
// Returning the value 1, which will be (a part of a)
condition</font></div>
<font face="monospace">}<br>
<br>
void f(int b) {<br>
int *x = 0;<br>
if (a(b)) // call to 'a', returning from 'a'<br>
*x = 5; // nullptr dereference<br>
}</font></div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr">These notes are totally meaningless,
and they aren't pruned out by [1] because the assumption
notes are non-prunable (they explain a variable, b in this
case, that is marked as interesting, as the return value is
tracked by ReturnVisitor). A possible solution would be to
prune linear functions in ConditionBRVisitor as well
similarly to [1], but then we'd lose the assumption note.
Instead, moving the non-prunable assumption note to the
actual collapse point would be better, but a quick look at
the generated ExplodedGraph reveals that this no longer an
bug report construction issue: the collapse point is in fact
at the return statement, rather then the condition.</div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr">So, I think the bug report should
always explain how the analyzer thinks. Ideally, this should
be identical to how the bug could be reproduced, but if it
is not (in this case the assumption should be made when a(b)
is the condition), maybe the logic of the analysis should be
changed instead.</div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr">Example: Notes 19-23, <a
href="http://cc.elte.hu:15001/GSoC2019-July-5/#is-unique=on&run=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&reportHash=8c536e75492d2588898e03178a704efb&report=12672&subtab=8c536e75492d2588898e03178a704efb"
moz-do-not-send="true">http://cc.elte.hu:15001/GSoC2019-July-5/#is-unique=on&run=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&reportHash=8c536e75492d2588898e03178a704efb&report=12672&subtab=8c536e75492d2588898e03178a704efb</a></div>
</div>
</div>
</blockquote>
<br>
This is due to the eagerly-assume behavior. The state split does
actually happen at == or ! rather than at the if-statement.<br>
<br>
I guess you could check if the node is wearing an "ExprEngine:
Eagerly Assume" tag. <br>
<br>
<br>
<blockquote type="cite"
cite="mid:CAGcXOD6c=-AOdHfFMpfpscg6e43Q=-jkKaQk7A2VmTvuYiD7Qg@mail.gmail.com">
<div dir="ltr">
<div dir="ltr">
<div class="gmail_attr">2. I didn't come across this too
often, but it happened:</div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr"><font face="monospace">int getInt();
int getInt2();<br>
<br>
bool a() {<br>
return getInt2() && getInt();</font></div>
<div class="gmail_attr"><font face="monospace"> // Returning
value, which will be (a part of a) condition<br>
}<br>
<br>
void f() {<br>
int *x = 0;</font><br style="font-family:monospace">
<span style="font-family:monospace"> if (a()) // call to
'a', returning from 'a'</span><br
style="font-family:monospace">
<span style="font-family:monospace"> *x = 5; // nullptr
dereference</span><font face="monospace"><br>
}</font><br>
</div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr"><font face="arial, sans-serif">This
note wasn't pruned because a() has in fact more than 3
basic blocks, but it really doesn't change anything. I
guess we *could* prune it, but I'm leaning on accepting it
as a sacrifice.</font></div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr">Example: Notes 65-68, <a
href="http://cc.elte.hu:15001/GSoC2019-July-5/#is-unique=on&run=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&reportHash=8cc2c55ab79efc36fcd2b1725648c2c3&report=12678&subtab=8cc2c55ab79efc36fcd2b1725648c2c3"
moz-do-not-send="true">http://cc.elte.hu:15001/GSoC2019-July-5/#is-unique=on&run=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&reportHash=8cc2c55ab79efc36fcd2b1725648c2c3&report=12678&subtab=8cc2c55ab79efc36fcd2b1725648c2c3</a></div>
</div>
</div>
</blockquote>
<br>
I guess it depends:<br>
<br>
- If we're assuming that the condition is true, and the function
returns A && B, the note is indeed redundant.<br>
- If we're assuming that the condition is true, and the function
returns A || B, it's worth mentioning whether we think that A is
true or we think that B is true.<br>
- If we're assuming that the condition is false, and the function
returns A && B, it's worth mentioning whether we think that
A is false or we think that B is false.<br>
- If we're assuming that the condition is false, and the function
returns A || B, the note is indeed redundant.<br>
<br>
<blockquote type="cite"
cite="mid:CAGcXOD6c=-AOdHfFMpfpscg6e43Q=-jkKaQk7A2VmTvuYiD7Qg@mail.gmail.com">
<div dir="ltr">
<div dir="ltr">
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr">3. Some function calls are lost due to
retrieving information from nested stack frames only, rather
than different stack frames. I only encountered this when
the condition variable was invalidated: The "AFTER tracking
conditions WITH improved moderation" runs display the path
to the invalidation point, yet the FINAL runs do not.</div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr">Example: Displaying the invalidation
point on note 9: <a
href="http://cc.elte.hu:15001/GSoC2019-July-5/#is-unique=on&run=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&reportHash=1d6a5c6fcca79b12b760b41924c9c2c3&report=7774&subtab=1d6a5c6fcca79b12b760b41924c9c2c3"
moz-do-not-send="true">http://cc.elte.hu:15001/GSoC2019-July-5/#is-unique=on&run=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&reportHash=1d6a5c6fcca79b12b760b41924c9c2c3&report=7774&subtab=1d6a5c6fcca79b12b760b41924c9c2c3</a></div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr">4. Calls to operator!= in foreach
loops -- Are these necessary? Is there any, even articial
cases where this is actually meaningful? This is pretty much
the only non-constructive addition of notes that are
introduced only because of condition tracking.</div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr">Example: Notes 13-17, 22-26, 37-41: <a
href="http://cc.elte.hu:15001/GSoC2019-July-5/#is-unique=on&run=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&reportHash=50cc2bed223dc4ba62f46d0665346a5e&report=12743&subtab=50cc2bed223dc4ba62f46d0665346a5e"
moz-do-not-send="true">http://cc.elte.hu:15001/GSoC2019-July-5/#is-unique=on&run=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&reportHash=50cc2bed223dc4ba62f46d0665346a5e&report=12743&subtab=50cc2bed223dc4ba62f46d0665346a5e</a></div>
</div>
</div>
</blockquote>
<br>
Mmm, yeah, i'd rather skip them. We don't make much sense out of
iterators anyway. I wish IteratorChecker could provide us with
better notes.<br>
<br>
<blockquote type="cite"
cite="mid:CAGcXOD6c=-AOdHfFMpfpscg6e43Q=-jkKaQk7A2VmTvuYiD7Qg@mail.gmail.com">
<div dir="ltr">
<div dir="ltr">
<div class="gmail_attr">Positives:</div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr">In general, the wording as the
analyses were run may be imperfect, but they indicate what
the message is about very precisely. The path of execution
is much better explained, and even if the extra function
call isn't meaningful, the note messages state clearly that
we're only talking about a condition, I can skip them
without being bothered by it too much. I am given extra
information without the cost of polluting the rest of the
report.</div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr">Here is a collection of my favorites,
though I found most of the extra notes nice:</div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr">1. Here, we encounter a nullpointer
dereference issue. Op0 is assumed to be equal to Op1, but
only due to condition tracking do we explain that Op1 is
believed to be null, and THAT is why this assumption (Op1 ==
Op0) is so important!</div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr">Notes 23-31: <a
href="http://cc.elte.hu:15001/GSoC2019-July-5/#is-unique=on&run=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&reportHash=6b5f3e43213b64eaef4a25a249c59baf&report=12955&subtab=6b5f3e43213b64eaef4a25a249c59baf"
moz-do-not-send="true">http://cc.elte.hu:15001/GSoC2019-July-5/#is-unique=on&run=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&reportHash=6b5f3e43213b64eaef4a25a249c59baf&report=12955&subtab=6b5f3e43213b64eaef4a25a249c59baf</a><br>
</div>
<div class="gmail_attr"><br>
</div>
</div>
</div>
</blockquote>
<br>
The note is indeed extremely helpful, but the warning itself should
have been suppressed by the "inlined defensive check" heuristic. I
guess the heuristic doesn't know how to carry over to the other side
of the `==`.<br>
<br>
<br>
<blockquote type="cite"
cite="mid:CAGcXOD6c=-AOdHfFMpfpscg6e43Q=-jkKaQk7A2VmTvuYiD7Qg@mail.gmail.com">
<div dir="ltr">
<div dir="ltr">
<div class="gmail_attr">2. The seldom addition of extra
"Returning value, which will be (a part of a) condition"
notes, love them!</div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr">Example: Note 13 of <a
href="http://cc.elte.hu:15001/GSoC2019-July-5/#is-unique=on&run=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&reportHash=5086ed6620709edc2d9a5ed4609e186d&report=12888&subtab=5086ed6620709edc2d9a5ed4609e186d"
moz-do-not-send="true">http://cc.elte.hu:15001/GSoC2019-July-5/#is-unique=on&run=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&reportHash=5086ed6620709edc2d9a5ed4609e186d&report=12888&subtab=5086ed6620709edc2d9a5ed4609e186d</a><br>
</div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr">3. This one is weird, I'm not yet sure
how this note came about, but yet again, it isn't clear AT
ALL in the original report why CountS will be a source of a
null dereference error. The extra note tells us that CountS
is non-null, but its pointee is.</div>
<div class="gmail_attr"><br>
</div>
<div class="gmail_attr">Note 19: <a
href="http://cc.elte.hu:15001/GSoC2019-July-5/#is-unique=on&run=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&reportHash=356d88cb062474fdf17584bf1451bffc&report=13412&subtab=356d88cb062474fdf17584bf1451bffc"
moz-do-not-send="true">http://cc.elte.hu:15001/GSoC2019-July-5/#is-unique=on&run=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%20WITH%20condition%20tracking%20FINAL&reportHash=356d88cb062474fdf17584bf1451bffc&report=13412&subtab=356d88cb062474fdf17584bf1451bffc</a></div>
</div>
</div>
</blockquote>
<br>
You're talking about the "Assuming pointer value is null" thingy? I
suspect that it's a bug and the note is in fact meaningless. See
also <a class="moz-txt-link-freetext" href="https://reviews.llvm.org/D65889?id=214186#inline-590619">https://reviews.llvm.org/D65889?id=214186#inline-590619</a> - i've
seen them before but i suspect that i suddenly see more of them
these days. Generally, i still don't understand the report after
seeing these notes. Even if the notes tell anything about the
pointee, the code they're attached to doesn't.<br>
<br>
I also don't understand why did we bring in the copy-constructor.
Was there an invalidation of all heap going on that also touched
*CountS?<br>
<br>
<blockquote type="cite"
cite="mid:CAGcXOD6c=-AOdHfFMpfpscg6e43Q=-jkKaQk7A2VmTvuYiD7Qg@mail.gmail.com">
<div dir="ltr">
<div dir="ltr">
<div class="gmail_attr">Also, adding the actual mail that was
lost ;)</div>
<div class="gmail_attr"><br>
</div>
<div dir="ltr" class="gmail_attr">On Wed, 24 Jul 2019 at
13:57, Kristóf Umann <<a
href="mailto:dkszelethus@gmail.com" target="_blank"
moz-do-not-send="true">dkszelethus@gmail.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div dir="ltr">
<div dir="ltr"><br>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Wed, 24 Jul 2019 at
04:29, Artem Dergachev <<a
href="mailto:noqnoqneo@gmail.com" target="_blank"
moz-do-not-send="true">noqnoqneo@gmail.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">Xerces - DOMRangeImpl.cpp: Ugh,
are those "Value assigned to..." notes triggered by
invalidation? I have a plan of my own to wipe such
notes out (or at least formulate them more
carefully), so i guess it's not an action item for
you (nor much of a regression), but still ugh.
Another thing to think about for those is to
suppress the warning entirely when a region that
stores is important condition is first assumed to
contain false, then invalidated, then assumed to
contain true.<br>
</div>
</blockquote>
<div><br>
</div>
<div>Totally agreed.</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">Xerces - XMLString.hpp: Mmm, i
guess passing unconstrained values as parameters is
also not very meaningful. It doesn't look like it's
a *big* problem, but i guess it's worth considering
fixing eventually.<br>
</div>
</blockquote>
<div><br>
</div>
<div>In this particular case, partly due to me not being
familiar with the code, I found the note helpful, but
in LLVM there were plenty of reports where I didn't
feel this way, so I'll get that fixed.</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">> Assert conditions are not
yet ignored, I have a neat solution locally<br>
<br>
Just to make sure - do you have a test case with
control flow within the condition, eg. `assert((a ||
b) && "the universe is collapsing")`?<br>
</div>
</blockquote>
<div><br>
</div>
<div>I don't :) Thanks!</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">
<div
class="m_-2694677468341043285gmail-m_2838973005318108645gmail-m_-1755177567490753650moz-cite-prefix">On
7/23/19 12:49 PM, Kristóf Umann wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">I'm happy to delight you all with
some more results! I was far more strict on my
judgement this time around, marking a report
good when I genuinely believed that the note
made the report better.
<div><br>
</div>
<div>All the runs were made on monorepo
commit <span
style="font-family:Geneva,Arial,Helvetica,sans-serif;font-size:13.3333px;background-color:rgb(247,252,255)">db4e335b8643056f5afa2a0d8d175d95b1fb7e28</span>
with the following patches applied:<br>
<br>
BEFORE tracking condition: Without any
changes being made, default CodeChecker
settings.</div>
<div>AFTER tracking conditions: Without any
changes being made, track-conditions set to
true.</div>
<div>AFTER tracking conditions + DEBUG notes:
Without any changes being made,
track-conditions and track-conditions-debug
set to true.</div>
<div>AFTER linear pruning (or similar run
name): [1] applied, track-conditions set to
true</div>
AFTER tracking conditions WITH improved
moderation: [1, 2, 3, 4] applied,
track-conditions set to true (stupid name but
it's gonna stay like this for now :) )<br
class="m_-2694677468341043285gmail-m_2838973005318108645gmail-m_-1755177567490753650gmail-Apple-interchange-newline">
AFTER tracking conditions WITH improved
moderation: [1, 2, 3, 4] applied,
track-conditions and
track-conditions-debug set to true
<div><span style="font-variant-numeric:normal;font-variant-east-asian:normal;font-family:Arial;vertical-align:baseline;white-space:pre-wrap">
[1] </span><a href="https://reviews.llvm.org/D64232"
style="text-decoration-line:none"
target="_blank" moz-do-not-send="true"><span style="font-family:Arial;font-variant-numeric:normal;font-variant-east-asian:normal;text-decoration-line:underline;vertical-align:baseline;white-space:pre-wrap">https://reviews.llvm.org/D64232</span></a><span style="font-variant-numeric:normal;font-variant-east-asian:normal;font-family:Arial;vertical-align:baseline;white-space:pre-wrap"> [analyzer] Prune calls to functions with linear CFGs that return a non-zero constrained value</span> <br>
</div>
<div>[2] <a
href="https://reviews.llvm.org/D64287"
target="_blank" moz-do-not-send="true">https://reviews.llvm.org/D64287</a> [analyzer]
Track the right hand side of the last store
regardless of its value</div>
<div><span
id="m_-2694677468341043285gmail-m_2838973005318108645gmail-m_-1755177567490753650gmail-docs-internal-guid-6633fc6c-7fff-4cb0-ba03-da8b5433cf09">
<p dir="ltr"
style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-family:Arial;font-variant-numeric:normal;font-variant-east-asian:normal;vertical-align:baseline;white-space:pre-wrap">[3] </span><a
href="https://reviews.llvm.org/D64270"
style="text-decoration-line:none"
target="_blank" moz-do-not-send="true"><span style="font-family:Arial;font-variant-numeric:normal;font-variant-east-asian:normal;text-decoration-line:underline;vertical-align:baseline;white-space:pre-wrap">https://reviews.llvm.org/D64270</span></a><span style="font-family:Arial;font-variant-numeric:normal;font-variant-east-asian:normal;vertical-align:baseline;white-space:pre-wrap"> [analyzer][NFC] Prepare visitors for different tracking kinds</span></p>
<p dir="ltr"
style="line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span style="font-family:Arial;font-variant-numeric:normal;font-variant-east-asian:normal;vertical-align:baseline;white-space:pre-wrap">[4] </span><a
href="https://reviews.llvm.org/D64272"
style="text-decoration-line:none"
target="_blank" moz-do-not-send="true"><span style="font-family:Arial;font-variant-numeric:normal;font-variant-east-asian:normal;text-decoration-line:underline;vertical-align:baseline;white-space:pre-wrap">https://reviews.llvm.org/D64272</span></a><span style="font-family:Arial;font-variant-numeric:normal;font-variant-east-asian:normal;vertical-align:baseline;white-space:pre-wrap"> [analyzer] Note last writes to a condition only in a nested stackframe</span></p>
</span>
<div>
<div>
<div><br>
</div>
<div>LLVM/Clang (no longer analyzing
clang-tools-extra): <a
href="http://cc.elte.hu:15001/GSoC2019-July-5/#run=LLVM%2FClang%20AFTER%20condition%20tracking%20WITH%20improved%20moderation&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%20AFTER%20condition%20tracking%20WITH%20improved%20moderation&is-unique=on"
target="_blank"
moz-do-not-send="true">http://cc.elte.hu:15001/GSoC2019-July-5/#run=LLVM%2FClang%20AFTER%20condition%20tracking%20WITH%20improved%20moderation&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=LLVM%2FClang%20AFTER%20condition%20tracking%20WITH%20improved%20moderation&is-unique=on</a></div>
<div><br>
</div>
<div>Bitcoin: <a
href="http://cc.elte.hu:15001/GSoC2019-July-5/#run=Bitcoint%20AFTER%20condition%20tracking%20WITH%20improved%20moderation&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=Bitcoint%20AFTER%20condition%20tracking%20WITH%20improved%20moderation&is-unique=on"
target="_blank"
moz-do-not-send="true">http://cc.elte.hu:15001/GSoC2019-July-5/#run=Bitcoint%20AFTER%20condition%20tracking%20WITH%20improved%20moderation&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=Bitcoint%20AFTER%20condition%20tracking%20WITH%20improved%20moderation&is-unique=on</a></div>
</div>
<div><br>
</div>
<div>Xerces: <a
href="http://cc.elte.hu:15001/GSoC2019-July-5/#run=Xerces%20AFTER%20condition%20tracking%20WITH%20improved%20moderation&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=Xerces%20AFTER%20condition%20tracking%20WITH%20improved%20moderation&is-unique=on"
target="_blank" moz-do-not-send="true">http://cc.elte.hu:15001/GSoC2019-July-5/#run=Xerces%20AFTER%20condition%20tracking%20WITH%20improved%20moderation&review-status=Confirmed&review-status=False%20positive&review-status=Intentional&detection-status=New&detection-status=Reopened&detection-status=Unresolved&tab=Xerces%20AFTER%20condition%20tracking%20WITH%20improved%20moderation&is-unique=on</a></div>
</div>
</div>
<div><br>
</div>
<div>Some conclusions:</div>
<div>
<ul>
<li>[4] Actually notes last writes in <i>different</i>,
not <i>nested</i> stack frames. Gotta
fix that.</li>
<li>[1] Returning an unconstrained value
when the function is linear is <b>never</b> meaningful.
Not for conditions, not for anything
else. Gotta fix that too. It's worth
talking about whether we should prune
these for conditions even if the
function isn't linear.</li>
<li>[1] This one gets rid of almost all
the operator bool calls. Maybe in the
rare case it doesn't, we should preserve
them? I always planned to get rid of
calls to operator bool, but now I'm
hesitant.</li>
<li>Assert conditions are not yet ignored,
I have a neat solution locally, but I
haven't published or tested it yet.</li>
</ul>
</div>
</div>
</div>
</blockquote>
</div>
</blockquote>
<div><br>
</div>
<div>One more thing: If the condition is a binary
operator, like (B || A) is assumed true because A is
true, or (A && B) is assumed false because A
is false, we should only track A.</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px
0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">
<blockquote type="cite">
<div dir="ltr">
<div dir="ltr">
<div>
<div>There honestly isn't that much I have
on my mind, I think after the next round
of analyses we should be good! I really
should've made an analysis with all the
non-condition tracking changes to get an
even better perspective, so I'll do that
next time.</div>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Tue, 25
Jun 2019 at 04:00, Artem Dergachev <<a
href="mailto:noqnoqneo@gmail.com"
target="_blank" moz-do-not-send="true">noqnoqneo@gmail.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote"
style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">Yup, looks much
better! Let's actually dig through other
projects a bit, but results on LLVM look
fairly perfect to me.<br>
<br>
<br>
> ASTDiagnostic.cpp<br>
<br>
Why wasn't the extra tracking removed in
the moderate mode? There doesn't seem to
be an obvious overwrite for `kind`. Is it
reacting to an invalidation?<br>
</div>
</blockquote>
<div> </div>
<div>The report is now untouched! </div>
<div><br>
</div>
<blockquote class="gmail_quote"
style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">>
DominanceFrontierImpl.h<br>
<br>
Why does it track so much more expressions
in the moderate mode (even if not
producing any actual nodes for them)? Is
it because you changed the de-duplication
method from by-expr to by-node in between?<br>
</div>
</blockquote>
<div> </div>
<div>I'll make a testcase for that. Honestly,
I'm thinking of reimplementing the visitor
as well, so that I'll collect the control
dependencies in a set when the visitor
object is created, and track them at
BlockExit locations, that should limit it,
because yeah, I believe they originate from
the de-duplication.</div>
<div><br>
</div>
<blockquote class="gmail_quote"
style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div bgcolor="#FFFFFF">> CGObjCGNU.cpp<br>
<br>
I'd like to think more about this one. It
doesn't look to me that the new notes are
helpful here, for the reason that the
problem is pretty much entirely between
the definition of OID and the use, while
all of the notes are outside that range.
The report wouldn't become less valid (for
a certain definition of valid) if we
simply omit everything except the last
three notes:<br>
- 'OID' is initialized to a null pointer
value<br>
- Loop body skipped when range is empty<br>
- Called ++ object pointer is null<br>
I don't see any immediate solutions here;
i guess the right thing to do would be to
simply learn how to chop off the
irrelevant beginning of the path.</div>
</blockquote>
<div><br>
</div>
<div>This time around, BodyFarm.cpp is in the
same boat. I really just made a
fundamentally bad (like, really bad) report
even worse.</div>
</div>
</div>
</blockquote>
</div>
</blockquote>
</div>
</div>
</blockquote>
</div>
</div>
</blockquote>
<br>
</body>
</html>