Skip to content

Commit

Permalink
[analyzer] Improve suppress-on-sink behavior in incomplete analyses.
Browse files Browse the repository at this point in the history
Warnings with suppress-on-sink are discarded during FlushReports when
BugReporter notices that all paths in ExplodedGraph that pass through the
warning eventually run into a sink node.

However, suppress-on-sink fails to filter out false positives when the analysis
terminates too early - by running into analyzer limits, such as block count
limits or graph size limits - and the interruption hits the narrow window
between throwing the leak report and reaching the no-return function call. In
such case the report is there, however suppression-on-sink doesn't work, because
the sink node was never constructed in the incomplete ExplodedGraph.

This patch implements a very partial solution: also suppress reports thrown
against a statement-node that corresponds to a statement that belongs to a
no-return block of the CFG.

rdar://problem/28832541

Differential Revision: https://reviews.llvm.org/D28023

llvm-svn: 290341
  • Loading branch information
haoNoQ committed Dec 22, 2016
1 parent 4d260bf commit 0e0a8b4
Showing 2 changed files with 57 additions and 0 deletions.
26 changes: 26 additions & 0 deletions clang/lib/StaticAnalyzer/Core/BugReporter.cpp
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@
#include "clang/AST/StmtCXX.h"
#include "clang/AST/StmtObjC.h"
#include "clang/Analysis/CFG.h"
#include "clang/Analysis/CFGStmtMap.h"
#include "clang/Analysis/ProgramPoint.h"
#include "clang/Basic/SourceManager.h"
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
@@ -3285,6 +3286,19 @@ struct FRIEC_WLItem {
};
}

static const CFGBlock *findBlockForNode(const ExplodedNode *N) {
ProgramPoint P = N->getLocation();
if (auto BEP = P.getAs<BlockEntrance>())
return BEP->getBlock();

// Find the node's current statement in the CFG.
if (const Stmt *S = PathDiagnosticLocation::getStmt(N))
return N->getLocationContext()->getAnalysisDeclContext()
->getCFGStmtMap()->getBlock(S);

return nullptr;
}

static BugReport *
FindReportInEquivalenceClass(BugReportEquivClass& EQ,
SmallVectorImpl<BugReport*> &bugReports) {
@@ -3333,6 +3347,18 @@ FindReportInEquivalenceClass(BugReportEquivClass& EQ,
continue;
}

// See if we are in a no-return CFG block. If so, treat this similarly
// to being post-dominated by a sink. This works better when the analysis
// is incomplete and we have never reached a no-return function
// we're post-dominated by.
// This is not quite enough to handle the incomplete analysis case.
// We may be post-dominated in subsequent blocks, or even
// inter-procedurally. However, it is not clear if more complicated
// cases are generally worth suppressing.
if (const CFGBlock *B = findBlockForNode(errorNode))
if (B->hasNoReturnElement())
continue;

// At this point we know that 'N' is not a sink and it has at least one
// successor. Use a DFS worklist to find a non-sink end-of-path node.
typedef FRIEC_WLItem WLItem;
31 changes: 31 additions & 0 deletions clang/test/Analysis/max-nodes-suppress-on-sink.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config max-nodes=12 -verify %s

// Here we test how "suppress on sink" feature of certain bugtypes interacts
// with reaching analysis limits.

// If we report a warning of a bug-type with "suppress on sink" attribute set
// (such as MallocChecker's memory leak warning), then failing to reach the
// reason for the sink (eg. no-return function such as "exit()") due to analysis
// limits (eg. max-nodes option), we may produce a false positive.

typedef __typeof(sizeof(int)) size_t;
void *malloc(size_t);

extern void exit(int) __attribute__ ((__noreturn__));

void clang_analyzer_warnIfReached(void);

void test_single_cfg_block_sink() {
void *p = malloc(1); // no-warning (wherever the leak warning may occur here)

// Due to max-nodes option in the run line, we should reach the first call
// but bail out before the second call.
// If the test on these two lines starts failing, see if modifying
// the max-nodes run-line helps.
clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
clang_analyzer_warnIfReached(); // no-warning

// Even though we do not reach this line, we should still suppress
// the leak report.
exit(0);
}

0 comments on commit 0e0a8b4

Please sign in to comment.