
<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Aug 18, 2016, at 1:17 AM, Gábor Horváth <<a href="mailto:xazax.hun@gmail.com" class="">xazax.hun@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><div class=""><div class=""><div class=""><div class=""><div class="">Hi!<br class=""><br class=""></div>In the CodeChecker tool (<a href="https://github.com/Ericsson/codechecker" class="">https://github.com/Ericsson/codechecker</a>) we support suppressing false positive bugs as a post processing step. After all the issues were reported, CodeChecker filters out the suppressed ones, and those will not be displayed to the user. <br class=""><br class=""></div>In our experience, however, and external tool only suppression suppport is not sufficient for certian reasons:<br class=""></div>* When a checker generates a sink node during analysis, the rest of the path will not be covered by the Static Analyzer. Unfortunately, if the user suppress the bug in an external tool, the coverage will not get better. This way false positive results can hide true positive results regardless of suppression.<br class=""></div></div></div></div></blockquote><div><br class=""></div><div>I’m a bit worried that in many cases suppressing one issue along a path and removing it a sink would simply cause another false positive along that path. I’m not convinced that the increase in coverage — along a path that we already know has a false positive — is worth the risk of requiring multiple suppressions.</div><div><br class=""></div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="">* The compiler could do better job diagnosing ill formed suppressions (invalid source range, typo in checker name etc).<br class=""></div><div class="">* Tools that are developed on top of the compiler need not to introduce their own customized solution for suppression. <br class=""><br class=""></div><div class="">It is beneficial to have a suppression format that is standard across all clang tools. So the same format could be used to:<br class=""></div><div class="">* Suppress clang warnings<br class=""></div><div class="">* Suppress clang static analyzer warnings<br class=""></div><div class="">* Suppress clang tidy warnings<br class=""><br class=""></div><div class="">There are two main approaches to suppress warnings that I can think of right now:<br class=""><br class=""></div><div class="">Suppress using comments (or pragma):<br class=""></div></div></div></blockquote><div><br class=""></div><div>I think pragmas are definitely the way to go for in-source suppression (as compared to comments). They offer a nice way to specify a code *region* in which issues can be suppressed and are in a format that the compiler can easily understand. They could be wrapped in macros to provide a nicer interface if needed. Unlike tools that live outside of clang, if we stick the suppression in clang we don’t need to resort to comments for compatibility.</div><div><br class=""></div><div>While in general I think it is better to add assertions to teach the analyzer about invariants rather than explicitly suppressing diagnostics, there are some cases where assertions don’t help:</div><div><br class=""></div><div>- Warnings about class/struct declarations (for example, Ben Craig’s PaddingChecker or the ObjC -dealloc checker). These declarations even be #ifdef’ed out when running under the analyzer because other code relies on them! Pragmas seem like the right choice here.</div><div><br class=""></div><div>- Leaks and other typestate-like issues. There is usually no way to assert “I already freed this thing”. Pragmas could help here. Another possibility is to add some kind of analyzer-specific function that when called means “Suppress any diagnostic involving the value passed to this call”. </div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="">The syntax could be something like:<br class=""></div><div class="">// clang suppress warning-or-checker-name [optional line offset][optional column range] [comment]<br class=""></div></div></div></blockquote><div><br class=""></div><div>I think it is probably better to do this on a per-diagnostic basis rather than a per-checker basis. This would let the user suppress the kind of bug and not be concerned with which particular checker happens to emit the diagnostic. While the checker names are already in some sense API (they are included in build scripts), exposing them in this was does increase the API surface.</div><div><br class=""></div><div>Devin</div><div><br class=""></div><div><br class=""></div><div><br class=""></div></div></body></html>