diff --git a/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp index 0b51d5677929c..7b48d44bce062 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp @@ -49,6 +49,8 @@ void UncheckedOptionalAccessCheck::check( const auto *FuncDecl = Result.Nodes.getNodeAs(FuncID); if (FuncDecl->isTemplated()) return; + CFG::BuildOptions Opts; + Opts.ExtendedNoReturnAnalysis = true; UncheckedOptionalAccessDiagnoser Diagnoser(ModelOptions); // FIXME: Allow user to set the (defaulted) SAT iterations max for @@ -56,7 +58,9 @@ void UncheckedOptionalAccessCheck::check( if (llvm::Expected> Diags = dataflow::diagnoseFunction( - *FuncDecl, *Result.Context, Diagnoser)) + *FuncDecl, *Result.Context, Diagnoser, + dataflow::kDefaultMaxSATIterations, + dataflow::kDefaultMaxBlockVisits, Opts)) for (const UncheckedOptionalAccessDiagnostic &Diag : *Diags) { diag(Diag.Range.getBegin(), "unchecked access to optional value") << Diag.Range; diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp index 3167b85f0e024..084ec84d02937 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp @@ -141,6 +141,39 @@ void nullable_value_after_swap(BloombergLP::bdlb::NullableValue &opt1, Bloo } } +void assertion_handler_imp() __attribute__((analyzer_noreturn)); + +void assertion_handler(); + +void assertion_handler() { + do { + assertion_handler_imp(); + } while(0); +} + +void function_calling_analyzer_noreturn(const bsl::optional& opt) +{ + if (!opt) { + assertion_handler(); // This will be deduced to have an implicit `analyzer_noreturn` attribute. + } + + *opt; // no-warning: The previous condition guards this dereference. +} + +// Should be considered as 'noreturn' by CFG +void halt() { + for(;;) {} +} + +void function_calling_no_return_from_cfg(const bsl::optional& opt) +{ + if (!opt) { + halt(); + } + + *opt; // no-warning: The previous condition guards this dereference. +} + template void function_template_without_user(const absl::optional &opt) { opt.value(); // no-warning diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h index 08fe1f881503b..ad7a0b58510f8 100644 --- a/clang/include/clang/AST/Decl.h +++ b/clang/include/clang/AST/Decl.h @@ -2021,6 +2021,18 @@ class FunctionDecl : public DeclaratorDecl, }; + /// Three-state classification for function return behavior in CFG analysis: + /// undefined (not analyzed), no-return (never returns), or normal return. + enum class AnalyzerSinkKind { + // Function has no explicit `analyzer_noreturn` or `noreturn` attribute + Undefined, + // Function never returns control to the caller + NoReturn, + // Function returns control to the caller (marked with + // `analyzer_noreturn(false)`) + NoSink + }; + /// Stashed information about a defaulted/deleted function body. class DefaultedOrDeletedFunctionInfo final : llvm::TrailingObjects build(const FunctionDecl &Func); + static llvm::Expected + build(const FunctionDecl &Func, const CFG::BuildOptions &CfgInitOpts = {}); /// Builds an `AdornedCFG` from an AST node. `D` is the function in which /// `S` resides. `D.isTemplated()` must be false. - static llvm::Expected build(const Decl &D, Stmt &S, - ASTContext &C); + static llvm::Expected + build(const Decl &D, Stmt &S, ASTContext &C, + const CFG::BuildOptions &CfgInitOpts = {}); /// Returns the `Decl` containing the statement used to construct the CFG, if /// available. diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h index e6efde091871f..acd784c70f90b 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h @@ -322,8 +322,9 @@ llvm::Expected> diagnoseFunction(const FunctionDecl &FuncDecl, ASTContext &ASTCtx, DiagnosisCallbacks Diagnoser, std::int64_t MaxSATIterations = kDefaultMaxSATIterations, - std::int32_t MaxBlockVisits = kDefaultMaxBlockVisits) { - llvm::Expected Context = AdornedCFG::build(FuncDecl); + std::int32_t MaxBlockVisits = kDefaultMaxBlockVisits, + const CFG::BuildOptions &CfgOptions = {}) { + llvm::Expected Context = AdornedCFG::build(FuncDecl, CfgOptions); if (!Context) return Context.takeError(); @@ -383,10 +384,11 @@ llvm::Expected> diagnoseFunction(const FunctionDecl &FuncDecl, ASTContext &ASTCtx, DiagnosisCallback Diagnoser, std::int64_t MaxSATIterations = kDefaultMaxSATIterations, - std::int32_t MaxBlockVisits = kDefaultMaxBlockVisits) { + std::int32_t MaxBlockVisits = kDefaultMaxBlockVisits, + const CFG::BuildOptions &Opts = {}) { DiagnosisCallbacks Callbacks = {nullptr, Diagnoser}; return diagnoseFunction(FuncDecl, ASTCtx, Callbacks, MaxSATIterations, - MaxBlockVisits); + MaxBlockVisits, Opts); } /// Abstract base class for dataflow "models": reusable analysis components that diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 224cb6a32af28..83dca6a25c223 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -974,7 +974,8 @@ def AnalyzerNoReturn : InheritableAttr { // vendor namespace, or should it use a vendor namespace specific to the // analyzer? let Spellings = [GNU<"analyzer_noreturn">]; - // TODO: Add subject list. + let Args = [DefaultBoolArgument<"Value", /*default=*/1, /*fake=*/0>]; + // let Subjects = SubjectList<[Function, ObjCMethod]>; let Documentation = [Undocumented]; } diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 83fcd87aec2f8..cc8ad83332998 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -3596,6 +3596,25 @@ bool FunctionDecl::isNoReturn() const { return false; } +FunctionDecl::AnalyzerSinkKind FunctionDecl::getAnalyzerSinkKind() const { + if (isNoReturn()) + return AnalyzerSinkKind::NoReturn; + + if (auto *Attr = getAttr()) + return Attr->getValue() ? AnalyzerSinkKind::NoReturn + : AnalyzerSinkKind::NoSink; + + return AnalyzerSinkKind::Undefined; +} + +void FunctionDecl::setAnalyzerSinkKind(FunctionDecl::AnalyzerSinkKind kind) { + dropAttr(); + + if (kind != AnalyzerSinkKind::Undefined) + addAttr(AnalyzerNoReturnAttr::CreateImplicit( + getASTContext(), kind == AnalyzerSinkKind::NoReturn, getLocation())); +} + bool FunctionDecl::isMemberLikeConstrainedFriend() const { // C++20 [temp.friend]p9: // A non-template friend declaration with a requires-clause [or] diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index d960d5130332b..838f27d9ba632 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -2833,8 +2833,29 @@ CFGBlock *CFGBuilder::VisitCallExpr(CallExpr *C, AddStmtChoice asc) { if (!FD->isVariadic()) findConstructionContextsForArguments(C); - if (FD->isNoReturn() || C->isBuiltinAssumeFalse(*Context)) - NoReturn = true; + auto SinkKind = FD->getAnalyzerSinkKind(); + NoReturn |= (SinkKind == FunctionDecl::AnalyzerSinkKind::NoReturn) || + C->isBuiltinAssumeFalse(*Context); + + if (BuildOpts.ExtendedNoReturnAnalysis && !NoReturn && + SinkKind == FunctionDecl::AnalyzerSinkKind::Undefined) { + auto *CanCD = FD->getCanonicalDecl(); + auto *DefFD = CanCD->getDefinition(); + + CanCD->setAnalyzerSinkKind(FunctionDecl::AnalyzerSinkKind::NoSink); + + if (DefFD && DefFD->getBody()) { + auto CalleeCFG = CFG::buildCFG(DefFD, DefFD->getBody(), + &DefFD->getASTContext(), BuildOpts); + + if (CalleeCFG && CalleeCFG->getEntry().isInevitablySinking( + CFGBlock::InterProcAnalysis::On)) { + CanCD->setAnalyzerSinkKind(FunctionDecl::AnalyzerSinkKind::NoReturn); + NoReturn = true; + } + } + } + if (FD->hasAttr()) AddEHEdge = false; if (isBuiltinAssumeWithSideEffects(FD->getASTContext(), C) || @@ -6288,7 +6309,14 @@ void CFGBlock::printTerminatorJson(raw_ostream &Out, const LangOptions &LO, // There may be many more reasons why a sink would appear during analysis // (eg. checkers may generate sinks arbitrarily), but here we only consider // sinks that would be obvious by looking at the CFG. -static bool isImmediateSinkBlock(const CFGBlock *Blk) { +// +// This function also performs inter-procedural analysis by recursively +// examining called functions to detect forwarding chains to noreturn +// functions. When a function is determined to never return through this +// analysis, it's automatically marked with analyzer_noreturn attribute +// for caching and future reference. +static bool isImmediateSinkBlock(const CFGBlock *Blk, + CFGBlock::InterProcAnalysis flag) { if (Blk->hasNoReturnElement()) return true; @@ -6298,10 +6326,60 @@ static bool isImmediateSinkBlock(const CFGBlock *Blk) { // at least for now, but once we have better support for exceptions, // we'd need to carefully handle the case when the throw is being // immediately caught. - if (llvm::any_of(*Blk, [](const CFGElement &Elm) { + if (llvm::any_of(*Blk, [](const CFGElement &Elm) -> bool { + if (std::optional StmtElm = Elm.getAs()) + return isa(StmtElm->getStmt()); + return false; + })) + return true; + + if (flag != CFGBlock::InterProcAnalysis::On) + return false; + + auto HasNoReturnCall = [&](const CallExpr *CE) { + if (!CE) + return false; + + auto *FD = CE->getDirectCallee(); + + if (!FD) + return false; + + auto *CanCD = FD->getCanonicalDecl(); + auto *DefFD = CanCD->getDefinition(); + auto NoRetKind = CanCD->getAnalyzerSinkKind(); + auto NoReturn = false; + + if ((NoRetKind == FunctionDecl::AnalyzerSinkKind::Undefined) && DefFD && + DefFD->getBody()) { + // HACK: we are gonna cache analysis result as implicit + // `analyzer_noreturn` attribute + auto *MutCD = const_cast(CanCD); + + // Mark function as `analyzer_noreturn(false)` to: + // * prevent infinite recursion in noreturn analysis + // * indicate that we've already analyzed(-ing) this function + // * serve as a safe default assumption (function may return) + MutCD->setAnalyzerSinkKind(FunctionDecl::AnalyzerSinkKind::NoSink); + + auto CalleeCFG = + CFG::buildCFG(DefFD, DefFD->getBody(), &DefFD->getASTContext(), {}); + + NoReturn = CalleeCFG && CalleeCFG->getEntry().isInevitablySinking(flag); + + // Override to `analyzer_noreturn(true)` + if (NoReturn) + MutCD->setAnalyzerSinkKind(FunctionDecl::AnalyzerSinkKind::NoReturn); + + } else + NoReturn = (NoRetKind == FunctionDecl::AnalyzerSinkKind::NoReturn); + + return NoReturn; + }; + + if (llvm::any_of(*Blk, [&](const CFGElement &Elm) { if (std::optional StmtElm = Elm.getAs()) - if (isa(StmtElm->getStmt())) - return true; + return HasNoReturnCall(dyn_cast(StmtElm->getStmt())); return false; })) return true; @@ -6309,11 +6387,11 @@ static bool isImmediateSinkBlock(const CFGBlock *Blk) { return false; } -bool CFGBlock::isInevitablySinking() const { +bool CFGBlock::isInevitablySinking(InterProcAnalysis flag) const { const CFG &Cfg = *getParent(); const CFGBlock *StartBlk = this; - if (isImmediateSinkBlock(StartBlk)) + if (isImmediateSinkBlock(StartBlk, flag)) return true; llvm::SmallVector DFSWorkList; @@ -6333,7 +6411,7 @@ bool CFGBlock::isInevitablySinking() const { for (const auto &Succ : Blk->succs()) { if (const CFGBlock *SuccBlk = Succ.getReachableBlock()) { - if (!isImmediateSinkBlock(SuccBlk) && !Visited.count(SuccBlk)) { + if (!isImmediateSinkBlock(SuccBlk, flag) && !Visited.count(SuccBlk)) { // If the block has reachable child blocks that aren't no-return, // add them to the worklist. DFSWorkList.push_back(SuccBlk); diff --git a/clang/lib/Analysis/FlowSensitive/AdornedCFG.cpp b/clang/lib/Analysis/FlowSensitive/AdornedCFG.cpp index 6c4847c7c23fb..f98004bf35e77 100644 --- a/clang/lib/Analysis/FlowSensitive/AdornedCFG.cpp +++ b/clang/lib/Analysis/FlowSensitive/AdornedCFG.cpp @@ -132,17 +132,20 @@ StmtToBlockMap::StmtToBlockMap(const CFG &Cfg) } // namespace internal -llvm::Expected AdornedCFG::build(const FunctionDecl &Func) { +llvm::Expected +AdornedCFG::build(const FunctionDecl &Func, + const CFG::BuildOptions &CfgInitOpts) { if (!Func.doesThisDeclarationHaveABody()) return llvm::createStringError( std::make_error_code(std::errc::invalid_argument), "Cannot analyze function without a body"); - return build(Func, *Func.getBody(), Func.getASTContext()); + return build(Func, *Func.getBody(), Func.getASTContext(), CfgInitOpts); } -llvm::Expected AdornedCFG::build(const Decl &D, Stmt &S, - ASTContext &C) { +llvm::Expected +AdornedCFG::build(const Decl &D, Stmt &S, ASTContext &C, + const CFG::BuildOptions &CfgInitOpts) { if (D.isTemplated()) return llvm::createStringError( std::make_error_code(std::errc::invalid_argument), @@ -155,7 +158,9 @@ llvm::Expected AdornedCFG::build(const Decl &D, Stmt &S, std::make_error_code(std::errc::invalid_argument), "Can only analyze C++"); - CFG::BuildOptions Options; + CFG::BuildOptions Options = CfgInitOpts; + + // Override some options Options.PruneTriviallyFalseEdges = true; Options.AddImplicitDtors = true; Options.AddTemporaryDtors = true; diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index a4e8de49a4229..6b5d0742792f2 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -2068,7 +2068,23 @@ static void handleAnalyzerNoReturnAttr(Sema &S, Decl *D, const ParsedAttr &AL) { } } - D->addAttr(::new (S.Context) AnalyzerNoReturnAttr(S.Context, AL)); + bool Value = true; + + if (AL.getNumArgs() > 0) { + auto *E = AL.getArgAsExpr(0); + + if (S.CheckBooleanCondition(AL.getLoc(), E, true).isInvalid()) + return; + + if (!E->EvaluateAsBooleanCondition(Value, S.Context, true)) { + S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type) + << AL << 1 << AANT_ArgumentIntOrBool << E->getSourceRange(); + + return; + } + } + + D->addAttr(::new (S.Context) AnalyzerNoReturnAttr(S.Context, AL, Value)); } // PS3 PPU-specific. diff --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h index 2ba84373531c6..0032cc6079f93 100644 --- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h +++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h @@ -175,6 +175,11 @@ template struct AnalysisInputs { return std::move(*this); } + AnalysisInputs &&withCfgBuildOptions(CFG::BuildOptions opts) && { + CfgBuildOptions = std::move(opts); + return std::move(*this); + } + /// Required. Input code that is analyzed. llvm::StringRef Code; /// Required. All functions that match this matcher are analyzed. @@ -201,6 +206,8 @@ template struct AnalysisInputs { std::function()> SolverFactory = [] { return std::make_unique(); }; + /// CFG build options + CFG::BuildOptions CfgBuildOptions = {}; }; /// Returns assertions based on annotations that are present after statements in @@ -288,7 +295,7 @@ checkDataflow(AnalysisInputs AI, llvm::errc::invalid_argument, "Could not find the target function."); // Build the control flow graph for the target function. - auto MaybeACFG = AdornedCFG::build(*Target); + auto MaybeACFG = AdornedCFG::build(*Target, AI.CfgBuildOptions); if (!MaybeACFG) return MaybeACFG.takeError(); auto &ACFG = *MaybeACFG; diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index 214aaee9f97f6..2a428890bd346 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -5950,14 +5950,13 @@ TEST(TransferTest, ForStmtBranchWithoutConditionDoesNotExtendFlowCondition) { Code, [](const llvm::StringMap> &Results, ASTContext &ASTCtx) { - ASSERT_THAT(Results.keys(), UnorderedElementsAre("loop_body")); const Environment &LoopBodyEnv = getEnvironmentAtAnnotation(Results, "loop_body"); const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); ASSERT_THAT(FooDecl, NotNull()); - auto &LoopBodyFooVal= getFormula(*FooDecl, LoopBodyEnv); + auto &LoopBodyFooVal = getFormula(*FooDecl, LoopBodyEnv); EXPECT_FALSE(LoopBodyEnv.proves(LoopBodyFooVal)); }); } diff --git a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp index 9fb7bebdbe41e..7048ac3da9202 100644 --- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp @@ -49,6 +49,7 @@ using namespace ast_matchers; using llvm::IsStringMapEntry; using ::testing::DescribeMatcher; using ::testing::IsEmpty; +using ::testing::Not; using ::testing::NotNull; using ::testing::Test; using ::testing::UnorderedElementsAre; @@ -693,6 +694,147 @@ TEST_F(NoreturnDestructorTest, ConditionalOperatorNestedBranchReturns) { // FIXME: Called functions at point `p` should contain only "foo". } +class AnalyzerNoreturnTest : public Test { +protected: + template + void runDataflow(llvm::StringRef Code, Matcher Expectations) { + tooling::FileContentMappings FilesContents; + FilesContents.push_back( + std::make_pair("noreturn_test_defs.h", R"( + void assertionHandler() __attribute__((analyzer_noreturn)); + + void assertionTrampoline() { + assertionHandler(); + } + + void trap() {} + )")); + FilesContents.push_back(std::make_pair( + "noreturn_test_defs_canonical.h", R"( + extern void assertionHandler(); + + void assertionSecondTrampoline() { + assertionHandler(); + } + )")); + FilesContents.push_back(std::make_pair( + "noreturn_test_defs_noretcfg.h", R"( + // will be marged as noreturn by CFG + void assertionHandler() { + for (;;) {} + } + + void assertionTrampoline() { + assertionHandler(); + } + + void trap() {} + )")); + + CFG::BuildOptions Opts; + Opts.ExtendedNoReturnAnalysis = true; + + ASSERT_THAT_ERROR( + test::checkDataflow( + AnalysisInputs( + Code, ast_matchers::hasName("target"), + [](ASTContext &C, Environment &) { + return FunctionCallAnalysis(C); + }) + .withASTBuildArgs({"-fsyntax-only", "-std=c++17"}) + .withASTBuildVirtualMappedFiles(std::move(FilesContents)) + .withCfgBuildOptions(std::move(Opts)), + /*VerifyResults=*/ + [&Expectations]( + const llvm::StringMap< + DataflowAnalysisState> &Results, + const AnalysisOutputs &) { + EXPECT_THAT(Results, Expectations); + }), + llvm::Succeeded()); + } +}; + +TEST_F(AnalyzerNoreturnTest, Breathing) { + std::string Code = R"( + #include "noreturn_test_defs.h" + + void target() { + trap(); + // [[p]] + } + )"; + runDataflow(Code, UnorderedElementsAre(IsStringMapEntry( + "p", HoldsFunctionCallLattice(HasCalledFunctions( + UnorderedElementsAre("trap")))))); +} + +TEST_F(AnalyzerNoreturnTest, DirectNoReturnCall) { + std::string Code = R"( + #include "noreturn_test_defs.h" + + void target() { + assertionHandler(); + trap(); + // [[p]] + } + )"; + runDataflow(Code, IsEmpty()); +} + +TEST_F(AnalyzerNoreturnTest, IndirectNoReturnCall) { + std::string Code = R"( + #include "noreturn_test_defs.h" + + void target() { + assertionTrampoline(); + trap(); + // [[p]] + } + )"; + runDataflow(Code, IsEmpty()); +} + +TEST_F(AnalyzerNoreturnTest, CanonicalDeclCallCheck) { + std::string Code = R"( + #include "noreturn_test_defs.h" + #include "noreturn_test_defs_canonical.h" + + void target() { + assertionSecondTrampoline(); + trap(); + // [[p]] + } + )"; + runDataflow(Code, IsEmpty()); +} + +TEST_F(AnalyzerNoreturnTest, NoReturnFromCFGCheck) { + std::string Code = R"( + #include "noreturn_test_defs_noretcfg.h" + + void target() { + assertionTrampoline(); + trap(); + // [[p]] + } + )"; + runDataflow(Code, IsEmpty()); +} + +TEST_F(AnalyzerNoreturnTest, InfiniteLoop) { + std::string Code = R"( + #include "noreturn_test_defs.h" + + void target() { + while(true){} + trap(); + // [[p]] + } + )"; + runDataflow(Code, IsEmpty()); +} + // Models an analysis that uses flow conditions. class SpecialBoolAnalysis final : public DataflowAnalysis {