llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-analysis Author: Akira Hatanaka (ahatanak) <details> <summary>Changes</summary> The CFG treats calls to functions attributed 'analyzer_noreturn' like calls to 'noreturn' functions, so -Wunreachable-code reported the code following such a call as never executed even though the call can return. Record in CFGBlock whether a noreturn block ends in a real 'noreturn' call or an 'analyzer_noreturn' call. The CFG keeps the code after an 'analyzer_noreturn' call as the alternate successor of the exit edge and the reachability scan used by -Wunreachable-code follows it. All other clients still treat the two kinds identically. rdar://188741139 --- Full diff: https://github.com/llvm/llvm-project/pull/229577.diff 5 Files Affected: - (modified) clang/include/clang/Analysis/CFG.h (+34-11) - (modified) clang/lib/Analysis/CFG.cpp (+20-11) - (modified) clang/lib/Analysis/ReachableCode.cpp (+28-12) - (added) clang/test/Analysis/analyzer-noreturn-cfg-output.c (+18) - (added) clang/test/Sema/warn-unreachable-analyzer-noreturn.c (+38) ``````````diff diff --git a/clang/include/clang/Analysis/CFG.h b/clang/include/clang/Analysis/CFG.h index 749ed3fc764525..8ac6614d45dc53 100644 --- a/clang/include/clang/Analysis/CFG.h +++ b/clang/include/clang/Analysis/CFG.h @@ -923,17 +923,26 @@ class CFGBlock { AdjacentBlocks Preds; AdjacentBlocks Succs; + /// The kind of noreturn element a block contains. + enum NoReturnKind : unsigned { + NR_None, + /// A function call attributed as 'analyzer_noreturn'. + NR_AnalyzerOnly, + /// A function call or implicit destructor attributed as 'noreturn'. + NR_Real + }; + /// This bit is set when the basic block contains a function call - /// or implicit destructor that is attributed as 'noreturn'. In that case, - /// control cannot technically ever proceed past this block. All such blocks - /// will have a single immediate successor: the exit block. This allows them - /// to be easily reached from the exit block and using this bit quickly - /// recognized without scanning the contents of the block. + /// or implicit destructor that is attributed as 'noreturn' or + /// 'analyzer_noreturn'. In that case, control cannot technically ever + /// proceed past this block. All such blocks will have a single immediate + /// successor: the exit block. This allows them to be easily reached from the + /// exit block and using this bit quickly recognized without scanning the + /// contents of the block. /// - /// Optimization Note: This bit could be profitably folded with Terminator's + /// Optimization Note: These bits could be profitably folded with Terminator's /// storage if the memory usage of CFGBlock becomes an issue. - LLVM_PREFERRED_TYPE(bool) - unsigned HasNoReturnElement : 1; + unsigned NoReturn : 2; /// The parent CFG that owns this CFGBlock. CFG *Parent; @@ -941,7 +950,7 @@ class CFGBlock { public: explicit CFGBlock(unsigned blockid, BumpVectorContext &C, CFG *parent) : Elements(C), Terminator(nullptr), BlockID(blockid), Preds(C, 1), - Succs(C, 1), HasNoReturnElement(false), Parent(parent) {} + Succs(C, 1), NoReturn(NR_None), Parent(parent) {} // Statement iterators using iterator = ElementList::iterator; @@ -1123,7 +1132,11 @@ class CFGBlock { void setTerminator(CFGTerminator Term) { Terminator = Term; } void setLabel(Stmt *Statement) { Label = Statement; } void setLoopTarget(const Stmt *loopTarget) { LoopTarget = loopTarget; } - void setHasNoReturnElement() { HasNoReturnElement = true; } + /// Mark the block as ending in a noreturn element. + void setHasNoReturnElement(bool AnalyzerOnly = false) { + assert(NoReturn == NR_None && "block already has a noreturn element"); + NoReturn = AnalyzerOnly ? NR_AnalyzerOnly : NR_Real; + } /// Returns true if the block would eventually end with a sink (a noreturn /// node). @@ -1149,7 +1162,17 @@ class CFGBlock { Stmt *getLabel() { return Label; } const Stmt *getLabel() const { return Label; } - bool hasNoReturnElement() const { return HasNoReturnElement; } + /// Returns true if the block has a noreturn element, whether it is a real + /// 'noreturn' or an 'analyzer_noreturn'. + bool hasNoReturnElement() const { return NoReturn != NR_None; } + + /// Returns true if the block ends in an 'analyzer_noreturn' call (as opposed + /// to a real 'noreturn' one). Most analyses treat the two indistinguishably + /// (see hasNoReturnElement). This is for clients that need to tell them + /// apart (e.g., -Wunreachable-code). + bool hasOnlyAnalyzerNoReturnElement() const { + return NoReturn == NR_AnalyzerOnly; + } unsigned getBlockID() const { return BlockID; } diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index 5aaaf5a8c33a85..556e08f5c1df80 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -795,7 +795,7 @@ class CFGBuilder { void autoCreateBlock() { if (!Block) Block = createBlock(); } CFGBlock *createBlock(bool add_successor = true); - CFGBlock *createNoReturnBlock(); + CFGBlock *createNoReturnBlock(bool AnalyzerOnly = false); CFGBlock *addStmt(Stmt *S) { return Visit(S, AddStmtChoice::AlwaysAdd); @@ -1810,12 +1810,14 @@ CFGBlock *CFGBuilder::createBlock(bool add_successor) { return B; } -/// createNoReturnBlock - Used to create a block is a 'noreturn' point in the -/// CFG. It is *not* connected to the current (global) successor, and instead -/// directly tied to the exit block in order to be reachable. -CFGBlock *CFGBuilder::createNoReturnBlock() { +/// createNoReturnBlock - Used to create a block that is a 'noreturn' or +/// 'analyzer_noreturn' point in the CFG. It is *not* connected to the current +/// (global) successor, and instead directly tied to the exit block in order to +/// be reachable. If \p AnalyzerOnly is true, the block is recorded as ending +/// in an 'analyzer_noreturn' call rather than a real 'noreturn' one. +CFGBlock *CFGBuilder::createNoReturnBlock(bool AnalyzerOnly) { CFGBlock *B = createBlock(false); - B->setHasNoReturnElement(); + B->setHasNoReturnElement(AnalyzerOnly); addSuccessor(B, &cfg->getExit(), Succ); return B; } @@ -2887,6 +2889,7 @@ CFGBlock *CFGBuilder::VisitCallExpr(CallExpr *C, AddStmtChoice asc) { // If this is a call to a no-return function, this stops the block here. bool NoReturn = getFunctionExtInfo(*calleeType).getNoReturn(); + bool AnalyzerNoReturn = false; bool AddEHEdge = false; @@ -2908,9 +2911,10 @@ CFGBlock *CFGBuilder::VisitCallExpr(CallExpr *C, AddStmtChoice asc) { if (!FD->isVariadic()) findConstructionContextsForArguments(C); - if (FD->isNoReturn() || FD->isAnalyzerNoReturn() || - C->isBuiltinAssumeFalse(*Context)) + if (FD->isNoReturn() || C->isBuiltinAssumeFalse(*Context)) NoReturn = true; + else if (FD->isAnalyzerNoReturn()) + AnalyzerNoReturn = true; if (FD->hasAttr<NoThrowAttr>()) AddEHEdge = false; if (isBuiltinAssumeWithSideEffects(FD->getASTContext(), C) || @@ -2923,14 +2927,15 @@ CFGBlock *CFGBuilder::VisitCallExpr(CallExpr *C, AddStmtChoice asc) { AddEHEdge = false; if (OmitArguments) { - assert(!NoReturn && "noreturn calls with unevaluated args not implemented"); + assert(!NoReturn && !AnalyzerNoReturn && + "noreturn calls with unevaluated args not implemented"); assert(!AddEHEdge && "EH calls with unevaluated args not implemented"); autoCreateBlock(); appendStmt(Block, C); return Visit(C->getCallee()); } - if (!NoReturn && !AddEHEdge) { + if (!NoReturn && !AnalyzerNoReturn && !AddEHEdge) { autoCreateBlock(); appendCall(Block, C); @@ -2944,7 +2949,9 @@ CFGBlock *CFGBuilder::VisitCallExpr(CallExpr *C, AddStmtChoice asc) { } if (NoReturn) - Block = createNoReturnBlock(); + Block = createNoReturnBlock(/*AnalyzerOnly=*/false); + else if (AnalyzerNoReturn) + Block = createNoReturnBlock(/*AnalyzerOnly=*/true); else Block = createBlock(); @@ -6195,6 +6202,8 @@ static void print_block(raw_ostream &OS, const CFG* cfg, OS << " (EXIT)]\n"; else if (&B == cfg->getIndirectGotoBlock()) OS << " (INDIRECT GOTO DISPATCH)]\n"; + else if (B.hasOnlyAnalyzerNoReturnElement()) + OS << " (ANALYZER NORETURN)]\n"; else if (B.hasNoReturnElement()) OS << " (NORETURN)]\n"; else diff --git a/clang/lib/Analysis/ReachableCode.cpp b/clang/lib/Analysis/ReachableCode.cpp index 4a9ab5d9f0f73e..20cf9f39bdde10 100644 --- a/clang/lib/Analysis/ReachableCode.cpp +++ b/clang/lib/Analysis/ReachableCode.cpp @@ -313,10 +313,10 @@ static bool shouldTreatSuccessorsAsReachable(const CFGBlock *B, return isConfigurationValue(Cond, PP); } -static unsigned scanFromBlock(const CFGBlock *Start, - llvm::BitVector &Reachable, +static unsigned scanFromBlock(const CFGBlock *Start, llvm::BitVector &Reachable, Preprocessor *PP, - bool IncludeSometimesUnreachableEdges) { + bool IncludeSometimesUnreachableEdges, + bool TreatAnalyzerNoReturnAsReturning = false) { unsigned count = 0; // Prep work queue @@ -331,6 +331,15 @@ static unsigned scanFromBlock(const CFGBlock *Start, WL.push_back(Start); + auto MarkReachable = [&](const CFGBlock *B) { + unsigned blockID = B->getBlockID(); + if (!Reachable[blockID]) { + Reachable.set(blockID); + WL.push_back(B); + ++count; + } + }; + // Find the reachable blocks from 'Start'. while (!WL.empty()) { const CFGBlock *item = WL.pop_back_val(); @@ -345,9 +354,21 @@ static unsigned scanFromBlock(const CFGBlock *Start, if (!IncludeSometimesUnreachableEdges) TreatAllSuccessorsAsReachable = false; + // Some callers need to distinguish 'analyzer_noreturn' calls from + // 'noreturn' calls, since the code after an 'analyzer_noreturn' call isn't + // dead. For those callers (TreatAnalyzerNoReturnAsReturning is true), + // continue with the code that follows the call, which the CFG keeps as the + // alternate successor of the exit edge. + bool FollowAlternate = TreatAnalyzerNoReturnAsReturning && + item->hasOnlyAnalyzerNoReturnElement(); + for (CFGBlock::const_succ_iterator I = item->succ_begin(), E = item->succ_end(); I != E; ++I) { const CFGBlock *B = *I; + if (FollowAlternate) { + if (const CFGBlock *Alt = I->getPossiblyUnreachableBlock()) + MarkReachable(Alt); + } if (!B) do { const CFGBlock *UB = I->getPossiblyUnreachableBlock(); if (!UB) @@ -366,14 +387,8 @@ static unsigned scanFromBlock(const CFGBlock *Start, } while (false); - if (B) { - unsigned blockID = B->getBlockID(); - if (!Reachable[blockID]) { - Reachable.set(blockID); - WL.push_back(B); - ++count; - } - } + if (B) + MarkReachable(B); } } return count; @@ -382,7 +397,8 @@ static unsigned scanFromBlock(const CFGBlock *Start, static unsigned scanMaybeReachableFromBlock(const CFGBlock *Start, Preprocessor &PP, llvm::BitVector &Reachable) { - return scanFromBlock(Start, Reachable, &PP, true); + return scanFromBlock(Start, Reachable, &PP, true, + /*TreatAnalyzerNoReturnAsReturning=*/true); } //===----------------------------------------------------------------------===// diff --git a/clang/test/Analysis/analyzer-noreturn-cfg-output.c b/clang/test/Analysis/analyzer-noreturn-cfg-output.c new file mode 100644 index 00000000000000..15e1763c75b1fb --- /dev/null +++ b/clang/test/Analysis/analyzer-noreturn-cfg-output.c @@ -0,0 +1,18 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG %s 2>&1 | FileCheck %s + +void analyzer_stop(void) __attribute__((analyzer_noreturn)); +void real_stop(void) __attribute__((noreturn)); + +// CHECK-LABEL: void analyzer( +// CHECK: (ANALYZER NORETURN)] +// CHECK-NOT: (NORETURN)] +void analyzer(void) { + analyzer_stop(); +} + +// CHECK-LABEL: void real( +// CHECK: (NORETURN)] +// CHECK-NOT: (ANALYZER NORETURN)] +void real(void) { + real_stop(); +} diff --git a/clang/test/Sema/warn-unreachable-analyzer-noreturn.c b/clang/test/Sema/warn-unreachable-analyzer-noreturn.c new file mode 100644 index 00000000000000..128d9440c216c3 --- /dev/null +++ b/clang/test/Sema/warn-unreachable-analyzer-noreturn.c @@ -0,0 +1,38 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wunreachable-code -Wreturn-type -Wimplicit-fallthrough %s + +// 'analyzer_noreturn' calls can return, so the code after them is not +// unreachable. -Wreturn-type and -Wimplicit-fallthrough do treat them as +// noreturn. + +void analyzer_stop(const char *) __attribute__((analyzer_noreturn)); +void real_stop(void) __attribute__((noreturn)); +void use(int); + +void after_analyzer_noreturn(int x) { + if (x) + return; + analyzer_stop("bad"); + use(1); // no warning +} + +void analyzer_then_real(void) { + analyzer_stop("bad"); + real_stop(); + use(1); // expected-warning {{code will never be executed}} +} + +int return_type(int x) { + if (x) + return 1; + analyzer_stop("bad"); +} // no warning + +void fallthrough(int x) { + switch (x) { + case 1: + analyzer_stop("bad"); + case 2: // no warning + use(2); + break; + } +} `````````` </details> https://github.com/llvm/llvm-project/pull/229577 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
