This is an archive of the discontinued LLVM Phabricator instance.

[Analyzer] Suppress false positives on the clang static analyzer
Needs ReviewPublic

Authored by nikhgupt on Sep 9 2016, 10:45 AM.

Details

Summary

There is a need for suppressing static analyzer warnings in specific parts of a source file (usually for silencing known false positives).

This change allows a user to suppress the static analyzer on a line-by-line basis using the comment: //clang_sa_ignore[<specific checker here>]
eg:

int b = 5;
int c = 0;
int tx = b/c;  // clang_sa_ignore [core.DivideZero]

If there is more than one warning associated with the line that you would like to suppress, then provide a comma separated list of ids:

// clang_sa_ignore [<id1>,<id2>,<id3>]

The user must annotate the specified checker(s). In order to do this, the analyzer warnings contain the identity of the checker being invoked. The warning message will be of the form:
<file name>:<line number>:<column number>:clang_sa_warning: <warning text> [<id>]

eg: /local/mnt/workspace/analyzer-ignore.cpp:65:13: clang_sa_warning: Division by zero [core.DivideZero]

Contains contributions from hiraditya (Aditya Kumar)

Diff Detail

Event Timeline

nikhgupt updated this revision to Diff 70861.Sep 9 2016, 10:45 AM
nikhgupt retitled this revision from to [Analyzer] Suppress false positives on the clang static analyzer.
nikhgupt updated this object.
nikhgupt added a subscriber: cfe-commits.
hiraditya edited edge metadata.Sep 9 2016, 1:54 PM

IIRC, I was the primary author of this code, I'd really appreciate if you could attribute the code to the authors. Thanks

nikhgupt updated this object.Sep 12 2016, 8:23 AM
nikhgupt edited edge metadata.
NoQ added a subscriber: NoQ.Sep 12 2016, 10:20 AM
zaks.anna edited edge metadata.Sep 16 2016, 3:33 PM

It is not clear to me that we've reached a consensus on cfe-dev list that suppressing with comments and printing the checker name is the way to go.

nikhgupt added a comment.EditedSep 19 2016, 1:35 PM

It is not clear to me that we've reached a consensus on cfe-dev list that suppressing with comments and printing the checker name is the way to go.

I'm new to the LLVM upstreaming process and have not been a part of the previous threads discussing this. It is my understanding that false positive suppression is of importance to the community. What is the common consensus on implementing Analyzer suppressions?

While suppressing with the use of comments is debatable, my findings indicate that a blind suppression statement for a line of code (ie: without the use of a checker name) can lead to some confusion with developers. For instance, the (simplified) code example below emits two analyzer warnings on the last line: A dead-code warning for 'c' as well as a division-by-zero warning for the arithmetic operation. A blind suppression by a developer who assumes that this would only emit a false positive deadcode warning, will unintentionally suppress the crucial division by zero warning.

void dummyFunc(){
  int a=5;
  int b=0;
  volatile int c = a/b;
}

By annotating the warnings they intend on suppressing we can ensure that developers are aware of any other bugs that can emerge from that line.

ie:

void dummyFunc(){
  int a=5;
  int b=0;
  volatile int c = a/b; //clang_sa_ignore[deadcode,core.DivideZero]
}

In order to do so, we will have to make the specific warning checker name visible to the user.

The thread from cfe-dev is called "Clang Static Analyzer: False Positive Suppression Support":
http://clang-developers.42468.n3.nabble.com/Clang-Static-Analyzer-False-Positive-Suppression-Support-tt4053071.html

dkrupp added a subscriber: dkrupp.Sep 26 2017, 12:15 AM