<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"><br></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"><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">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"><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"><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">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">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">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">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">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">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 class="gmail_attr"><br></div><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">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 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">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">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 class="gmail_attr"><br></div><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">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 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">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">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 class="gmail_attr"><br></div><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">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">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"><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">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"><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"><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">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">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">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><ul></ul><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">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>