From: Martin Braenne Date: Mon, 15 May 2023 09:23:44 +0000 (+0000) Subject: [clang][dataflow] Don't analyze templated declarations. X-Git-Tag: upstream/17.0.6~8537 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=1a42f795587b9d57291d009989aace6efd0a7a7f;p=platform%2Fupstream%2Fllvm.git [clang][dataflow] Don't analyze templated declarations. Attempting to analyze templated code doesn't have a good cost-benefit ratio. We have so far done a best-effort attempt at this, but maintaining this support has an ongoing high maintenance cost because the AST for templates can violate a lot of the invariants that otherwise hold for the AST of concrete code. As just one example, in concrete code the operand of a UnaryOperator '*' is always a prvalue (https://godbolt.org/z/s3e5xxMd1), but in templates this isn't true (https://godbolt.org/z/6W9xxGvoM). Further rationale for not analyzing templates: * The semantics of a template itself are weakly defined; semantics can depend strongly on the concrete template arguments. Analyzing the template itself (as opposed to an instantiation) therefore has limited value. * Analyzing templates requires a lot of special-case code that isn't necessary for concrete code because dependent types are hard to deal with and the AST violates invariants that otherwise hold for concrete code (see above). * There's precedent in that neither Clang Static Analyzer nor the flow-sensitive warnings in Clang (such as uninitialized variables) support analyzing templates. Reviewed By: gribozavr2, xazax.hun Differential Revision: https://reviews.llvm.org/D150352 --- diff --git a/clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h b/clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h index 3495bdf..b51e2cb 100644 --- a/clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h +++ b/clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h @@ -32,7 +32,13 @@ namespace dataflow { class ControlFlowContext { public: /// Builds a ControlFlowContext from an AST node. `D` is the function in which - /// `S` resides and must not be null. + /// `S` resides. `D.isTemplated()` must be false. + static llvm::Expected build(const Decl &D, Stmt &S, + ASTContext &C); + + /// Builds a ControlFlowContext from an AST node. `D` is the function in which + /// `S` resides. `D` must not be null and `D->isTemplated()` must be false. + LLVM_DEPRECATED("Use the version that takes a const Decl & instead", "") static llvm::Expected build(const Decl *D, Stmt &S, ASTContext &C); diff --git a/clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp b/clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp index 5520633..4556787 100644 --- a/clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp +++ b/clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp @@ -68,7 +68,12 @@ static llvm::BitVector findReachableBlocks(const CFG &Cfg) { } llvm::Expected -ControlFlowContext::build(const Decl *D, Stmt &S, ASTContext &C) { +ControlFlowContext::build(const Decl &D, Stmt &S, ASTContext &C) { + if (D.isTemplated()) + return llvm::createStringError( + std::make_error_code(std::errc::invalid_argument), + "Cannot analyze templated declarations"); + CFG::BuildOptions Options; Options.PruneTriviallyFalseEdges = true; Options.AddImplicitDtors = true; @@ -79,7 +84,7 @@ ControlFlowContext::build(const Decl *D, Stmt &S, ASTContext &C) { // Ensure that all sub-expressions in basic blocks are evaluated. Options.setAllAlwaysAdd(); - auto Cfg = CFG::buildCFG(D, &S, &C, Options); + auto Cfg = CFG::buildCFG(&D, &S, &C, Options); if (Cfg == nullptr) return llvm::createStringError( std::make_error_code(std::errc::invalid_argument), @@ -90,9 +95,19 @@ ControlFlowContext::build(const Decl *D, Stmt &S, ASTContext &C) { llvm::BitVector BlockReachable = findReachableBlocks(*Cfg); - return ControlFlowContext(D, std::move(Cfg), std::move(StmtToBlock), + return ControlFlowContext(&D, std::move(Cfg), std::move(StmtToBlock), std::move(BlockReachable)); } +llvm::Expected +ControlFlowContext::build(const Decl *D, Stmt &S, ASTContext &C) { + if (D == nullptr) + return llvm::createStringError( + std::make_error_code(std::errc::invalid_argument), + "Declaration must not be null"); + + return build(*D, S, C); +} + } // namespace dataflow } // namespace clang diff --git a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp index 5dd390e..73428ac 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp @@ -211,7 +211,7 @@ DataflowAnalysisContext::getControlFlowContext(const FunctionDecl *F) { return &It->second; if (Stmt *Body = F->getBody()) { - auto CFCtx = ControlFlowContext::build(F, *Body, F->getASTContext()); + auto CFCtx = ControlFlowContext::build(*F, *Body, F->getASTContext()); // FIXME: Handle errors. assert(CFCtx); auto Result = FunctionContexts.insert({F, std::move(*CFCtx)}); diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index 142dd2f..95810a4 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -424,9 +424,8 @@ public: switch (S->getOpcode()) { case UO_Deref: { - // Skip past a reference to handle dereference of a dependent pointer. - const auto *SubExprVal = cast_or_null( - Env.getValue(*SubExpr, SkipPast::Reference)); + const auto *SubExprVal = + cast_or_null(Env.getValue(*SubExpr, SkipPast::None)); if (SubExprVal == nullptr) break; diff --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h index e1a56b0..ff7d27d 100644 --- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h +++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h @@ -242,7 +242,7 @@ checkDataflow(AnalysisInputs AI, // Build the control flow graph for the target function. auto MaybeCFCtx = - ControlFlowContext::build(Target, *Target->getBody(), Context); + ControlFlowContext::build(*Target, *Target->getBody(), Context); if (!MaybeCFCtx) return MaybeCFCtx.takeError(); auto &CFCtx = *MaybeCFCtx; diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index 54d3c57..7c2a701 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -39,12 +39,16 @@ using ::testing::UnorderedElementsAre; using BuiltinOptions = DataflowAnalysisContext::Options; template -void runDataflow(llvm::StringRef Code, Matcher Match, - DataflowAnalysisOptions Options, - LangStandard::Kind Std = LangStandard::lang_cxx17, - llvm::StringRef TargetFun = "target") { +llvm::Error +runDataflowReturnError(llvm::StringRef Code, Matcher Match, + DataflowAnalysisOptions Options, + LangStandard::Kind Std = LangStandard::lang_cxx17, + llvm::StringRef TargetFun = "target") { using ast_matchers::hasName; llvm::SmallVector ASTBuildArgs = { + // -fnodelayed-template-parsing is the default everywhere but on Windows. + // Set it explicitly so that tests behave the same on Windows as on other + // platforms. "-fsyntax-only", "-fno-delayed-template-parsing", "-std=" + std::string(LangStandard::getLangStandardForKind(Std).getName())}; @@ -61,13 +65,21 @@ void runDataflow(llvm::StringRef Code, Matcher Match, AI.ASTBuildArgs = ASTBuildArgs; if (Options.BuiltinOpts) AI.BuiltinOptions = *Options.BuiltinOpts; + return checkDataflow( + std::move(AI), + /*VerifyResults=*/ + [&Match]( + const llvm::StringMap> &Results, + const AnalysisOutputs &AO) { Match(Results, AO.ASTCtx); }); +} + +template +void runDataflow(llvm::StringRef Code, Matcher Match, + DataflowAnalysisOptions Options, + LangStandard::Kind Std = LangStandard::lang_cxx17, + llvm::StringRef TargetFun = "target") { ASSERT_THAT_ERROR( - checkDataflow( - std::move(AI), - /*VerifyResults=*/ - [&Match](const llvm::StringMap> - &Results, - const AnalysisOutputs &AO) { Match(Results, AO.ASTCtx); }), + runDataflowReturnError(Code, Match, Options, Std, TargetFun), llvm::Succeeded()); } @@ -2534,31 +2546,34 @@ TEST(TransferTest, AddrOfReference) { }); } -TEST(TransferTest, DerefDependentPtr) { +TEST(TransferTest, CannotAnalyzeFunctionTemplate) { std::string Code = R"( template - void target(T *Foo) { - T &Bar = *Foo; - /*[[p]]*/ - } + void target() {} )"; - runDataflow( - Code, - [](const llvm::StringMap> &Results, - ASTContext &ASTCtx) { - ASSERT_THAT(Results.keys(), UnorderedElementsAre("p")); - const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); - - const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); - ASSERT_THAT(FooDecl, NotNull()); - - const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar"); - ASSERT_THAT(BarDecl, NotNull()); + ASSERT_THAT_ERROR( + runDataflowReturnError( + Code, + [](const llvm::StringMap> &Results, + ASTContext &ASTCtx) {}, + {BuiltinOptions()}), + llvm::FailedWithMessage("Cannot analyze templated declarations")); +} - const auto *FooVal = cast(Env.getValue(*FooDecl)); - const auto *BarLoc = Env.getStorageLocation(*BarDecl); - EXPECT_EQ(BarLoc, &FooVal->getPointeeLoc()); - }); +TEST(TransferTest, CannotAnalyzeMethodOfClassTemplate) { + std::string Code = R"( + template + struct A { + void target() {} + }; + )"; + ASSERT_THAT_ERROR( + runDataflowReturnError( + Code, + [](const llvm::StringMap> &Results, + ASTContext &ASTCtx) {}, + {BuiltinOptions()}), + llvm::FailedWithMessage("Cannot analyze templated declarations")); } TEST(TransferTest, VarDeclInitAssignConditionalOperator) { diff --git a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp index 83b9f33..30aef86e 100644 --- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp @@ -68,7 +68,7 @@ runAnalysis(llvm::StringRef Code, AnalysisT (*MakeAnalysis)(ASTContext &)) { assert(Body != nullptr); auto CFCtx = llvm::cantFail( - ControlFlowContext::build(nullptr, *Body, AST->getASTContext())); + ControlFlowContext::build(*Func, *Body, AST->getASTContext())); AnalysisT Analysis = MakeAnalysis(AST->getASTContext()); DataflowAnalysisContext DACtx(std::make_unique());