Index: lib/StaticAnalyzer/Core/BugReporter.cpp =================================================================== --- lib/StaticAnalyzer/Core/BugReporter.cpp +++ lib/StaticAnalyzer/Core/BugReporter.cpp @@ -3304,6 +3304,48 @@ return nullptr; } +static bool isDominatedByNoReturnBlocks(const ExplodedNode *N) { + const CFG &Cfg = N->getCFG(); + + const CFGBlock *StartBlk = findBlockForNode(N); + if (!StartBlk) + return false; + + llvm::SmallVector DFSWorkList; + llvm::SmallPtrSet Visited; + + DFSWorkList.push_back(StartBlk); + while (!DFSWorkList.empty()) { + const CFGBlock *Blk = DFSWorkList.back(); + DFSWorkList.pop_back(); + + if (Blk->hasNoReturnElement()) { + // Nice, this block is noreturn. What about other blocks? + continue; + } + + if (Blk == &Cfg.getExit()) { + // We're leaving the current CFG. We're no longer sure what happens next. + return false; + } + + if (Visited.count(Blk)) { + // We've encountered a loop. We won't see anything new here anymore. + continue; + } + Visited.insert(Blk); + + for (const auto &Succ : Blk->succs()) { + // See what's going on in reachable child blocks. + if (const CFGBlock *SuccBlk = Succ.getReachableBlock()) + DFSWorkList.push_back(SuccBlk); + } + } + + // Nothing reached the exit. It can only mean one thing: there's no return. + return true; +} + static BugReport * FindReportInEquivalenceClass(BugReportEquivClass& EQ, SmallVectorImpl &bugReports) { @@ -3360,9 +3402,8 @@ // 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; + if (isDominatedByNoReturnBlocks(errorNode)) + 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. Index: test/Analysis/max-nodes-suppress-on-sink.c =================================================================== --- test/Analysis/max-nodes-suppress-on-sink.c +++ test/Analysis/max-nodes-suppress-on-sink.c @@ -15,6 +15,8 @@ void clang_analyzer_warnIfReached(void); +int coin(); + void test_single_cfg_block_sink() { void *p = malloc(1); // no-warning (wherever the leak warning may occur here) @@ -29,3 +31,53 @@ // the leak report. exit(0); } + +// A similar test with more complicated control flow before the no-return thing, +// so that the no-return thing wasn't in the same CFG block. +void test_more_complex_control_flow_before_sink() { + void *p = malloc(1); // no-warning + + clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} + clang_analyzer_warnIfReached(); // no-warning + + if (coin()) + exit(0); + else + exit(1); +} + +// A loop before the no-return function, to make sure that +// the dominated-by-sink analysis doesn't hang. +void test_loop_before_sink(int n) { + void *p = malloc(1); // no-warning + + clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} + clang_analyzer_warnIfReached(); // no-warning + + for (int i = 0; i < n; ++i) { + } + exit(1); +} + +// We're not sure if this is no-return. +void test_loop_with_sink(int n) { + void *p = malloc(1); // expected-warning@+2{{Potential leak of memory}} + + clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} + clang_analyzer_warnIfReached(); // no-warning + + for (int i = 0; i < n; ++i) + if (i == 0) + exit(1); +} + +// Handle unreachable blocks correctly. +void test_unreachable_successor_blocks() { + void *p = malloc(1); // no-warning + + clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} + clang_analyzer_warnIfReached(); // no-warning + + if (1) // no-crash + exit(1); +}