<div dir="ltr"><div class="gmail_default" style="font-family:tahoma,sans-serif;font-size:small">My previous email details why we are doing this: <a href="http://lists.llvm.org/pipermail/llvm-dev/2020-April/141167.html" style="font-family:Arial,Helvetica,sans-serif">http://lists.llvm.org/pipermail/llvm-dev/2020-April/141167.html</a></div><div class="gmail_default" style="font-family:tahoma,sans-serif;font-size:small">Basically, we ran the PREfast static analysis tool on LLVM/Clang and it reported a lot of warnings. I guess some of them are false positives after all.</div><div class="gmail_default" style="font-family:tahoma,sans-serif;font-size:small"><br></div><div class="gmail_default" style="font-family:tahoma,sans-serif;font-size:small">As David suggests that maybe I should validate these warnings by also running the clang static analyzer. There are a lot of other warnings reported by the tool. Here is the full list: <a href="https://docs.google.com/spreadsheets/d/1h_3tHxsgBampxb7PXoB5lgwiBSpTty9RLe5maIQxnTk/edit?usp=sharing">https://docs.google.com/spreadsheets/d/1h_3tHxsgBampxb7PXoB5lgwiBSpTty9RLe5maIQxnTk/edit?usp=sharing</a>. If the community is interested in getting those fixed I can upstream patches.</div><div class="gmail_default" style="font-family:tahoma,sans-serif;font-size:small"><br></div><div class="gmail_default" style="font-family:tahoma,sans-serif;font-size:small">--Mandeep</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Apr 28, 2020 at 10:44 AM Aaron Puchert via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</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">aaronpuchert added a comment.<br>
<br>
Could you explain why we are doing this? Clang has its own static analyzer <<a href="https://clang-analyzer.llvm.org/" rel="noreferrer" target="_blank">https://clang-analyzer.llvm.org/</a>>, which can find null dereferences <<a href="https://clang.llvm.org/docs/analyzer/checkers.html#core-nulldereference-c-c-objc" rel="noreferrer" target="_blank">https://clang.llvm.org/docs/analyzer/checkers.html#core-nulldereference-c-c-objc</a>> as well but also tracks constraints to prevent false positives like this.<br>
<br>
There is a page somewhere with current analysis runs on LLVM, although I can't find it right now.<br>
<br>
<br>
<br>
================<br>
Comment at: clang/lib/Analysis/ThreadSafety.cpp:1938-1940<br>
+    // The default value for the argument VD to the current function is<br>
+    // nullptr. So we assert that VD is non null because we deref VD here.<br>
+    assert(VD && "!VD");<br>
----------------<br>
dblaikie wrote:<br>
> Doesn't seem like the most informative comment or assertion string - the invariant  "isScopedVar implies VD is non-null" is established earlier in the function, where isScopedVar only becomes true under the "VD is non-null" condition at 1809.<br>
> <br>
> Would it be better to improve whatever static analysis you're using to be able to track that correlation, rather than adding lots of extra assertions to LLVM? (can the Clang Static Analyzer understand this code and avoid warning on it, for instance - that'd be a good existence proof for such "smarts" being reasonably possible for static analysis)<br>
I haven't tested it, but the Clang static analyzer shouldn't complain here. It explores different code paths and collects constraints along them. All code paths including the assignment `isScopedVar = true` should have `VD` being non-null as constraint, because that's the condition to get there.<br>
<br>
I know that solving constraints can be hard in general, but here the constraint is exactly what we need to disprove null references from happening.<br>
<br>
<br>
CHANGES SINCE LAST ACTION<br>
  <a href="https://reviews.llvm.org/D78853/new/" rel="noreferrer" target="_blank">https://reviews.llvm.org/D78853/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D78853" rel="noreferrer" target="_blank">https://reviews.llvm.org/D78853</a><br>
<br>
<br>
<br>
</blockquote></div>