https://github.com/spavloff updated https://github.com/llvm/llvm-project/pull/144408
>From baedd4867ba8f62b54ea9942d84fadc9a716dc02 Mon Sep 17 00:00:00 2001 From: Serge Pavlov <sepavl...@gmail.com> Date: Mon, 16 Jun 2025 18:19:44 +0700 Subject: [PATCH 1/3] [Analysis] Avoid some warnings about exit from noreturn function Compiler sometimes issues warnings on exit from 'noreturn' functions, in the code like: [[noreturn]] extern void nonreturnable(); void (*func_ptr)(); [[noreturn]] void foo() { func_ptr = nonreturnable; (*func_ptr)(); } where exit cannot take place because the function pointer is actually a pointer to noreturn function. This change introduces small data analysis that can remove some of the warnings in the cases when compiler can prove that the set of reaching definitions consists of noreturn functions only. --- clang/lib/Sema/AnalysisBasedWarnings.cpp | 114 +++++++++++++++++++++++ clang/test/SemaCXX/noreturn-vars.cpp | 107 +++++++++++++++++++++ 2 files changed, 221 insertions(+) create mode 100644 clang/test/SemaCXX/noreturn-vars.cpp diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 2a107a36e24b4..2853e1e7ebf3e 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -36,6 +36,7 @@ #include "clang/Analysis/AnalysisDeclContext.h" #include "clang/Analysis/CFG.h" #include "clang/Analysis/CFGStmtMap.h" +#include "clang/Analysis/FlowSensitive/DataflowWorklist.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/DiagnosticSema.h" #include "clang/Basic/SourceLocation.h" @@ -399,6 +400,108 @@ static bool isNoexcept(const FunctionDecl *FD) { return false; } +/// Checks if the given variable, which is assumed to be a function pointer, is +/// initialized with a function having 'noreturn' attribute. +static bool isInitializedWithNoReturn(const VarDecl *VD) { + if (const Expr *Init = VD->getInit()) { + if (auto *ListInit = dyn_cast<InitListExpr>(Init); + ListInit && ListInit->getNumInits() > 0) + Init = ListInit->getInit(0); + if (auto *DeclRef = dyn_cast<DeclRefExpr>(Init->IgnoreParenCasts())) + if (auto *FD = dyn_cast<FunctionDecl>(DeclRef->getDecl())) + return FD->isNoReturn(); + } + return false; +} + +/// Checks if the given expression is a reference to a function with +/// 'noreturn' attribute. +static bool isReferenceToNoReturn(const Expr *E) { + if (auto *DRef = dyn_cast<DeclRefExpr>(E->IgnoreParenCasts())) + if (auto *FD = dyn_cast<FunctionDecl>(DRef->getDecl())) + return FD->isNoReturn(); + return false; +} + +namespace { + +/// Looks for statements, that can define value of the given variable. +struct TransferFunctions : public StmtVisitor<TransferFunctions> { + const VarDecl *Var; + std::optional<bool> AllValuesAreNoReturn; + + TransferFunctions(const VarDecl *VD) : Var(VD) {} + + void VisitDeclStmt(DeclStmt *DS) { + for (auto *DI : DS->decls()) + if (auto *VD = dyn_cast<VarDecl>(DI)) + if (VarDecl *Def = VD->getDefinition()) + if (Def == Var) + AllValuesAreNoReturn = isInitializedWithNoReturn(Def); + } + + void VisitUnaryOperator(UnaryOperator *UO) { + if (UO->getOpcode() == UO_AddrOf) { + if (auto *DRef = + dyn_cast<DeclRefExpr>(UO->getSubExpr()->IgnoreParenCasts())) + if (DRef->getDecl() == Var) + AllValuesAreNoReturn = false; + } + } + + void VisitBinaryOperator(BinaryOperator *BO) { + if (BO->getOpcode() == BO_Assign) + if (auto *DRef = dyn_cast<DeclRefExpr>(BO->getLHS()->IgnoreParenCasts())) + if (DRef->getDecl() == Var) + AllValuesAreNoReturn = isReferenceToNoReturn(BO->getRHS()); + } + + void VisitCallExpr(CallExpr *CE) { + for (CallExpr::arg_iterator I = CE->arg_begin(), E = CE->arg_end(); I != E; + ++I) { + const Expr *Arg = *I; + if (Arg->isGLValue() && !Arg->getType().isConstQualified()) + if (auto *DRef = dyn_cast<DeclRefExpr>(Arg->IgnoreParenCasts())) + if (auto VD = dyn_cast<VarDecl>(DRef->getDecl())) + if (VD->getDefinition() == Var) + AllValuesAreNoReturn = false; + } + } +}; +} // namespace + +// Checks if all possible values of the given variable are functions with +// 'noreturn' attribute. +static bool areAllValuesNoReturn(const VarDecl *VD, const CFGBlock &VarBlk, + AnalysisDeclContext &AC) { + // The set of possible values of a constant variable is determined by + // its initializer. + if (VD->getType().isConstant(AC.getASTContext())) { + if (const VarDecl *Def = VD->getDefinition()) + return isInitializedWithNoReturn(Def); + return false; + } + + // Scan function statements for definitions of the given variable. + TransferFunctions TF(VD); + BackwardDataflowWorklist Worklist(*AC.getCFG(), AC); + Worklist.enqueueBlock(&VarBlk); + while (const CFGBlock *B = Worklist.dequeue()) { + for (CFGBlock::const_reverse_iterator ri = B->rbegin(), re = B->rend(); + ri != re; ++ri) { + if (std::optional<CFGStmt> cs = ri->getAs<CFGStmt>()) { + const Stmt *S = cs->getStmt(); + TF.Visit(const_cast<Stmt *>(S)); + if (TF.AllValuesAreNoReturn) + return *TF.AllValuesAreNoReturn; + } + } + Worklist.enqueuePredecessors(B); + } + + return false; +} + //===----------------------------------------------------------------------===// // Check for missing return value. //===----------------------------------------------------------------------===// @@ -525,6 +628,17 @@ static ControlFlowKind CheckFallThrough(AnalysisDeclContext &AC) { HasAbnormalEdge = true; continue; } + if (auto *Call = dyn_cast<CallExpr>(S)) { + const Expr *Callee = Call->getCallee(); + if (Callee->getType()->isPointerType()) + if (auto *DeclRef = + dyn_cast<DeclRefExpr>(Callee->IgnoreParenImpCasts())) + if (auto *VD = dyn_cast<VarDecl>(DeclRef->getDecl())) + if (areAllValuesNoReturn(VD, B, AC)) { + HasAbnormalEdge = true; + continue; + } + } HasPlainEdge = true; } diff --git a/clang/test/SemaCXX/noreturn-vars.cpp b/clang/test/SemaCXX/noreturn-vars.cpp new file mode 100644 index 0000000000000..31d16db46e4a2 --- /dev/null +++ b/clang/test/SemaCXX/noreturn-vars.cpp @@ -0,0 +1,107 @@ +// RUN: %clang_cc1 -fsyntax-only %s -verify + +[[noreturn]] extern void noret(); +[[noreturn]] extern void noret2(); +extern void ordinary(); + +typedef void (*func_type)(void); + +// Constant initialization. + +void (* const const_fptr)() = noret; +[[noreturn]] void test_global_const() { + const_fptr(); +} + +const func_type const_fptr_cast = (func_type)noret2; +[[noreturn]] void test_global_cast() { + const_fptr_cast(); +} + +void (* const const_fptr_list)() = {noret}; +[[noreturn]] void test_global_list() { + const_fptr_list(); +} + +const func_type const_fptr_fcast = func_type(noret2); +[[noreturn]] void test_global_fcast() { + const_fptr_fcast(); +} + +[[noreturn]] void test_local_const() { + void (* const fptr)() = noret; + fptr(); +} + +// Global variable assignment. +void (*global_fptr)() = noret; + +[[noreturn]] void test_global_noassign() { + global_fptr(); +} // expected-warning {{function declared 'noreturn' should not return}} + +[[noreturn]] void test_global_assign() { + global_fptr = noret; + global_fptr(); +} + +[[noreturn]] void test_global_override() { + global_fptr = ordinary; + global_fptr = noret; + global_fptr(); +} + +[[noreturn]] void test_global_switch_01(int x) { + switch(x) { + case 1: + global_fptr = noret; + break; + default: + global_fptr = noret2; + break; + } + global_fptr(); +} + +[[noreturn]] void test_global_switch_02(int x) { + switch(x) { + case 1: + global_fptr = ordinary; + break; + default: + global_fptr = noret; + break; + } + global_fptr(); +} + +// Local variable assignment. + +[[noreturn]] void test_local_init() { + func_type func_ptr = noret; + func_ptr(); +} + +[[noreturn]] void test_local_assign() { + void (*func_ptr)(void); + func_ptr = noret; + func_ptr(); +} + +// Escaped value. + +extern void abc_01(func_type &); +extern void abc_02(func_type *); + +[[noreturn]] void test_escape_ref() { + func_type func_ptr = noret; + abc_01(func_ptr); + func_ptr(); +} // expected-warning {{function declared 'noreturn' should not return}} + +[[noreturn]] void test_escape_addr() { + func_type func_ptr = noret; + abc_02(&func_ptr); + func_ptr(); +} // expected-warning {{function declared 'noreturn' should not return}} + >From 9b42598f54016142af41fff196713fb98b15897d Mon Sep 17 00:00:00 2001 From: Serge Pavlov <sepavl...@gmail.com> Date: Thu, 19 Jun 2025 00:19:46 +0700 Subject: [PATCH 2/3] Fixed implementation --- clang/lib/Sema/AnalysisBasedWarnings.cpp | 50 ++++++- clang/test/SemaCXX/noreturn-vars.cpp | 170 +++++++++++++++++++---- 2 files changed, 189 insertions(+), 31 deletions(-) diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 2853e1e7ebf3e..332aa8da934f4 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -46,6 +46,7 @@ #include "clang/Sema/SemaInternal.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/BitVector.h" +#include "llvm/ADT/DenseMap.h" #include "llvm/ADT/MapVector.h" #include "llvm/ADT/STLFunctionalExtras.h" #include "llvm/ADT/SmallVector.h" @@ -432,6 +433,8 @@ struct TransferFunctions : public StmtVisitor<TransferFunctions> { TransferFunctions(const VarDecl *VD) : Var(VD) {} + void reset() { AllValuesAreNoReturn = std::nullopt; } + void VisitDeclStmt(DeclStmt *DS) { for (auto *DI : DS->decls()) if (auto *VD = dyn_cast<VarDecl>(DI)) @@ -475,28 +478,63 @@ struct TransferFunctions : public StmtVisitor<TransferFunctions> { static bool areAllValuesNoReturn(const VarDecl *VD, const CFGBlock &VarBlk, AnalysisDeclContext &AC) { // The set of possible values of a constant variable is determined by - // its initializer. - if (VD->getType().isConstant(AC.getASTContext())) { + // its initializer, unless it is a function parameter. + if (!isa<ParmVarDecl>(VD) && VD->getType().isConstant(AC.getASTContext())) { if (const VarDecl *Def = VD->getDefinition()) return isInitializedWithNoReturn(Def); return false; } - // Scan function statements for definitions of the given variable. + // In multithreaded environment the value of a global variable may be changed + // asynchronously. + if (!VD->getDeclContext()->isFunctionOrMethod()) + return false; + + // Check the condition "all values are noreturn". It is satisfied if the + // variable is set to "noreturn" value in the current block or all its + // predecessors satisfies the condition. + using MapTy = llvm::DenseMap<const CFGBlock *, std::optional<bool>>; + using ValueTy = MapTy::value_type; + MapTy BlocksToCheck; + BlocksToCheck[&VarBlk] = std::nullopt; + const auto BlockSatisfiesCondition = [](ValueTy Item) { + return Item.getSecond().value_or(false); + }; + TransferFunctions TF(VD); BackwardDataflowWorklist Worklist(*AC.getCFG(), AC); Worklist.enqueueBlock(&VarBlk); while (const CFGBlock *B = Worklist.dequeue()) { + // First check the current block. for (CFGBlock::const_reverse_iterator ri = B->rbegin(), re = B->rend(); ri != re; ++ri) { if (std::optional<CFGStmt> cs = ri->getAs<CFGStmt>()) { const Stmt *S = cs->getStmt(); + TF.reset(); TF.Visit(const_cast<Stmt *>(S)); - if (TF.AllValuesAreNoReturn) - return *TF.AllValuesAreNoReturn; + if (TF.AllValuesAreNoReturn) { + if (!TF.AllValuesAreNoReturn.value()) + return false; + BlocksToCheck[B] = true; + break; + } } } - Worklist.enqueuePredecessors(B); + + // If all checked blocks satisfy the condition, the check is finished. + if (std::all_of(BlocksToCheck.begin(), BlocksToCheck.end(), + BlockSatisfiesCondition)) + return true; + + // If this block does not contain the variable definition, check + // its predecessors. + if (!BlocksToCheck[B]) { + Worklist.enqueuePredecessors(B); + BlocksToCheck.erase(B); + for (const auto &PredBlk : B->preds()) + if (!BlocksToCheck.contains(PredBlk)) + BlocksToCheck[PredBlk] = std::nullopt; + } } return false; diff --git a/clang/test/SemaCXX/noreturn-vars.cpp b/clang/test/SemaCXX/noreturn-vars.cpp index 31d16db46e4a2..ca65fcf5ca31d 100644 --- a/clang/test/SemaCXX/noreturn-vars.cpp +++ b/clang/test/SemaCXX/noreturn-vars.cpp @@ -43,50 +43,171 @@ void (*global_fptr)() = noret; [[noreturn]] void test_global_assign() { global_fptr = noret; global_fptr(); +} // expected-warning {{function declared 'noreturn' should not return}} + +// Local variable assignment. + +[[noreturn]] void test_init() { + func_type func_ptr = noret; + func_ptr(); } -[[noreturn]] void test_global_override() { - global_fptr = ordinary; - global_fptr = noret; - global_fptr(); +[[noreturn]] void test_assign() { + void (*func_ptr)(void); + func_ptr = noret; + func_ptr(); +} + +[[noreturn]] void test_override() { + func_type func_ptr; + func_ptr = ordinary; + func_ptr = noret; + func_ptr(); +} + +[[noreturn]] void test_if_all(int x) { + func_type func_ptr; + if (x > 0) + func_ptr = noret; + else + func_ptr = noret2; + func_ptr(); +} + +[[noreturn]] void test_if_mix(int x) { + func_type func_ptr; + if (x > 0) + func_ptr = noret; + else + func_ptr = ordinary; + func_ptr(); +} // expected-warning {{function declared 'noreturn' should not return}} + +[[noreturn]] void test_if_opt(int x) { + func_type func_ptr = noret; + if (x > 0) + func_ptr = ordinary; + func_ptr(); +} // expected-warning {{function declared 'noreturn' should not return}} + +[[noreturn]] void test_if_opt2(int x) { + func_type func_ptr = ordinary; + if (x > 0) + func_ptr = noret; + func_ptr(); +} // expected-warning {{function declared 'noreturn' should not return}} + +[[noreturn]] void test_if_nest_all(int x, int y) { + func_type func_ptr; + if (x > 0) { + if (y > 0) + func_ptr = noret; + else + func_ptr = noret2; + } else { + if (y < 0) + func_ptr = noret2; + else + func_ptr = noret; + } + func_ptr(); +} + +[[noreturn]] void test_if_nest_mix(int x, int y) { + func_type func_ptr; + if (x > 0) { + if (y > 0) + func_ptr = noret; + else + func_ptr = noret2; + } else { + if (y < 0) + func_ptr = ordinary; + else + func_ptr = noret; + } + func_ptr(); +} // expected-warning {{function declared 'noreturn' should not return}} + +[[noreturn]] void test_switch_all(int x) { + func_type func_ptr; + switch(x) { + case 1: + func_ptr = noret; + break; + default: + func_ptr = noret2; + break; + } + func_ptr(); } -[[noreturn]] void test_global_switch_01(int x) { +[[noreturn]] void test_switch_mix(int x) { + func_type func_ptr; switch(x) { case 1: - global_fptr = noret; - break; + func_ptr = ordinary; + break; default: - global_fptr = noret2; - break; + func_ptr = noret; + break; } - global_fptr(); + func_ptr(); +} // expected-warning {{function declared 'noreturn' should not return}} + +[[noreturn]] void test_switch_fall(int x) { + func_type func_ptr; + switch(x) { + case 1: + func_ptr = ordinary; + default: + func_ptr = noret; + break; + } + func_ptr(); } -[[noreturn]] void test_global_switch_02(int x) { +[[noreturn]] void test_switch_all_nest(int x, int y) { + func_type func_ptr; switch(x) { case 1: - global_fptr = ordinary; - break; + func_ptr = noret; + break; default: - global_fptr = noret; - break; + if (y > 0) + func_ptr = noret2; + else + func_ptr = noret; + break; } - global_fptr(); + func_ptr(); } -// Local variable assignment. +[[noreturn]] void test_switch_mix_nest(int x, int y) { + func_type func_ptr; + switch(x) { + case 1: + func_ptr = noret; + break; + default: + if (y > 0) + func_ptr = noret2; + else + func_ptr = ordinary; + break; + } + func_ptr(); +} // expected-warning {{function declared 'noreturn' should not return}} -[[noreturn]] void test_local_init() { - func_type func_ptr = noret; +// Function parameters. + +[[noreturn]] void test_param(void (*func_ptr)() = noret) { func_ptr(); -} +} // expected-warning {{function declared 'noreturn' should not return}} -[[noreturn]] void test_local_assign() { - void (*func_ptr)(void); - func_ptr = noret; +[[noreturn]] void test_const_param(void (* const func_ptr)() = noret) { func_ptr(); -} +} // expected-warning {{function declared 'noreturn' should not return}} // Escaped value. @@ -104,4 +225,3 @@ extern void abc_02(func_type *); abc_02(&func_ptr); func_ptr(); } // expected-warning {{function declared 'noreturn' should not return}} - >From 5336817e5b27dc6ff85d43117d22c6cdf448c87f Mon Sep 17 00:00:00 2001 From: Serge Pavlov <sepavl...@gmail.com> Date: Thu, 19 Jun 2025 01:31:04 +0700 Subject: [PATCH 3/3] Update Release Notes --- clang/docs/ReleaseNotes.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 33ee8a53b5f37..65717bf35a574 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -629,6 +629,10 @@ Improvements to Clang's diagnostics #GH69470, #GH59391, #GH58172, #GH46215, #GH45915, #GH45891, #GH44490, #GH36703, #GH32903, #GH23312, #GH69874. +- Clang now does not issue a warning about returning from a function declared with + the ``[[noreturn]]`` attribute when the function body is ended with a call via + pointer, provided it can be proven that the pointer only points to + ``[[noreturn]]`` functions.. Improvements to Clang's time-trace ---------------------------------- _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits