aaronpuchert added a comment. Could you explain why we are doing this? Clang has its own static analyzer <https://clang-analyzer.llvm.org/>, which can find null dereferences <https://clang.llvm.org/docs/analyzer/checkers.html#core-nulldereference-c-c-objc> as well but also tracks constraints to prevent false positives like this.
There is a page somewhere with current analysis runs on LLVM, although I can't find it right now. ================ Comment at: clang/lib/Analysis/ThreadSafety.cpp:1938-1940 + // The default value for the argument VD to the current function is + // nullptr. So we assert that VD is non null because we deref VD here. + assert(VD && "!VD"); ---------------- dblaikie wrote: > 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. > > 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) 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. 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. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78853/new/ https://reviews.llvm.org/D78853 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
