[clang] [clang][dataflow] Only propagate past CXXDefaultInitExpr if init is (PR #99236)
https://github.com/bazuzi closed https://github.com/llvm/llvm-project/pull/99236 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Collect local variables referenced within a functio… (PR #104459)
bazuzi wrote: Added a test. https://github.com/llvm/llvm-project/pull/104459 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Collect local variables referenced within a functio… (PR #104459)
https://github.com/bazuzi updated https://github.com/llvm/llvm-project/pull/104459 >From 5aaa8c5e65b59b0f21775811f48dec350ae210c7 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Thu, 15 Aug 2024 11:11:30 -0400 Subject: [PATCH 1/2] [clang][dataflow] Collect local variables referenced within a function/statement. --- clang/include/clang/Analysis/FlowSensitive/ASTOps.h | 3 +++ clang/lib/Analysis/FlowSensitive/ASTOps.cpp | 9 + 2 files changed, 12 insertions(+) diff --git a/clang/include/clang/Analysis/FlowSensitive/ASTOps.h b/clang/include/clang/Analysis/FlowSensitive/ASTOps.h index f9c923a36ad229..ec4d041254877f 100644 --- a/clang/include/clang/Analysis/FlowSensitive/ASTOps.h +++ b/clang/include/clang/Analysis/FlowSensitive/ASTOps.h @@ -139,6 +139,9 @@ struct ReferencedDecls { /// All variables with static storage duration, notably including static /// member variables and static variables declared within a function. llvm::DenseSet Globals; + /// Local variables, not including parameters or static variables declared + /// within a function. + llvm::DenseSet Locals; /// Free functions and member functions which are referenced (but not /// necessarily called). llvm::DenseSet Functions; diff --git a/clang/lib/Analysis/FlowSensitive/ASTOps.cpp b/clang/lib/Analysis/FlowSensitive/ASTOps.cpp index 27d42a7b508562..fdba139628d8ff 100644 --- a/clang/lib/Analysis/FlowSensitive/ASTOps.cpp +++ b/clang/lib/Analysis/FlowSensitive/ASTOps.cpp @@ -170,6 +170,13 @@ static void insertIfGlobal(const Decl &D, Globals.insert(V); } +static void insertIfLocal(const Decl &D, + llvm::DenseSet &Locals) { + if (auto *V = dyn_cast(&D)) +if (V->hasLocalStorage() && !isa(V)) + Locals.insert(V); +} + static void insertIfFunction(const Decl &D, llvm::DenseSet &Funcs) { if (auto *FD = dyn_cast(&D)) @@ -220,12 +227,14 @@ class ReferencedDeclsVisitor bool VisitDecl(Decl *D) { insertIfGlobal(*D, Referenced.Globals); +insertIfLocal(*D, Referenced.Locals); insertIfFunction(*D, Referenced.Functions); return true; } bool VisitDeclRefExpr(DeclRefExpr *E) { insertIfGlobal(*E->getDecl(), Referenced.Globals); +insertIfLocal(*E->getDecl(), Referenced.Locals); insertIfFunction(*E->getDecl(), Referenced.Functions); return true; } >From 0852bb9cea4916a94c7c168d99458f19abab5ee8 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Mon, 19 Aug 2024 11:06:24 -0400 Subject: [PATCH 2/2] Add test confirming local non-static, non-param variables are included. --- .../Analysis/FlowSensitive/ASTOpsTest.cpp | 22 +++ 1 file changed, 22 insertions(+) diff --git a/clang/unittests/Analysis/FlowSensitive/ASTOpsTest.cpp b/clang/unittests/Analysis/FlowSensitive/ASTOpsTest.cpp index cd1c076ab09e6b..834aa7f4c2ac2e 100644 --- a/clang/unittests/Analysis/FlowSensitive/ASTOpsTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/ASTOpsTest.cpp @@ -85,4 +85,26 @@ TEST(ASTOpsTest, ReferencedDeclsOnUnionInitList) { UnorderedElementsAre(IDecl)); } +TEST(ASTOpsTest, ReferencedDeclsLocalsNotParamsOrStatics) { + std::string Code = R"cc( +void func(int Param) { + static int Static = 0; + int Local = Param; + Local = Static; +} + )cc"; + std::unique_ptr Unit = + tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++17"}); + auto &ASTCtx = Unit->getASTContext(); + + ASSERT_EQ(ASTCtx.getDiagnostics().getClient()->getNumErrors(), 0U); + + auto *Func = cast(findValueDecl(ASTCtx, "func")); + ASSERT_NE(Func, nullptr); + auto *LocalDecl = cast(findValueDecl(ASTCtx, "Local")); + + EXPECT_THAT(getReferencedDecls(*Func).Locals, + UnorderedElementsAre(LocalDecl)); +} + } // namespace ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Collect local variables referenced within a functio… (PR #104459)
bazuzi wrote: The new `Locals` field can be, and is by default, ignored, as the existing uses iterate individually through each of the sets in `ReferencedDecls`. https://github.com/llvm/llvm-project/pull/104459 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Collect local variables referenced within a functio… (PR #104459)
bazuzi wrote: @ymand Can you review and eventually merge for me? https://github.com/llvm/llvm-project/pull/104459 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Collect local variables referenced within a functio… (PR #104459)
https://github.com/bazuzi created https://github.com/llvm/llvm-project/pull/104459 …n/statement. We don't need these for the same in-tree purposes as the other sets, i.e. for making sure we model these Decls that are declared outside the function, but we have an out-of-tree use for these sets that would benefit from this simple addition and would avoid duplicating so much of this code. >From 5aaa8c5e65b59b0f21775811f48dec350ae210c7 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Thu, 15 Aug 2024 11:11:30 -0400 Subject: [PATCH] [clang][dataflow] Collect local variables referenced within a function/statement. --- clang/include/clang/Analysis/FlowSensitive/ASTOps.h | 3 +++ clang/lib/Analysis/FlowSensitive/ASTOps.cpp | 9 + 2 files changed, 12 insertions(+) diff --git a/clang/include/clang/Analysis/FlowSensitive/ASTOps.h b/clang/include/clang/Analysis/FlowSensitive/ASTOps.h index f9c923a36ad229..ec4d041254877f 100644 --- a/clang/include/clang/Analysis/FlowSensitive/ASTOps.h +++ b/clang/include/clang/Analysis/FlowSensitive/ASTOps.h @@ -139,6 +139,9 @@ struct ReferencedDecls { /// All variables with static storage duration, notably including static /// member variables and static variables declared within a function. llvm::DenseSet Globals; + /// Local variables, not including parameters or static variables declared + /// within a function. + llvm::DenseSet Locals; /// Free functions and member functions which are referenced (but not /// necessarily called). llvm::DenseSet Functions; diff --git a/clang/lib/Analysis/FlowSensitive/ASTOps.cpp b/clang/lib/Analysis/FlowSensitive/ASTOps.cpp index 27d42a7b508562..fdba139628d8ff 100644 --- a/clang/lib/Analysis/FlowSensitive/ASTOps.cpp +++ b/clang/lib/Analysis/FlowSensitive/ASTOps.cpp @@ -170,6 +170,13 @@ static void insertIfGlobal(const Decl &D, Globals.insert(V); } +static void insertIfLocal(const Decl &D, + llvm::DenseSet &Locals) { + if (auto *V = dyn_cast(&D)) +if (V->hasLocalStorage() && !isa(V)) + Locals.insert(V); +} + static void insertIfFunction(const Decl &D, llvm::DenseSet &Funcs) { if (auto *FD = dyn_cast(&D)) @@ -220,12 +227,14 @@ class ReferencedDeclsVisitor bool VisitDecl(Decl *D) { insertIfGlobal(*D, Referenced.Globals); +insertIfLocal(*D, Referenced.Locals); insertIfFunction(*D, Referenced.Functions); return true; } bool VisitDeclRefExpr(DeclRefExpr *E) { insertIfGlobal(*E->getDecl(), Referenced.Globals); +insertIfLocal(*E->getDecl(), Referenced.Locals); insertIfFunction(*E->getDecl(), Referenced.Functions); return true; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Return available function types for BindingDecls. (PR #102196)
bazuzi wrote: Yes, thanks! https://github.com/llvm/llvm-project/pull/102196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Return available function types for BindingDecls. (PR #102196)
bazuzi wrote: Any additional questions on this @mizvekov? https://github.com/llvm/llvm-project/pull/102196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Return available function types for BindingDecls. (PR #102196)
bazuzi wrote: The fact that the code that revealed this bug was in a dataflow analysis isn't really relevant to the fix. I've added a unit test file with a new test limited only to the modified behavior of getFunctionType for BindingDecls. https://github.com/llvm/llvm-project/pull/102196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Return available function types for BindingDecls. (PR #102196)
https://github.com/bazuzi updated https://github.com/llvm/llvm-project/pull/102196 >From 052eff82638bdfbc8c7b8a2b90bcd9a0c46e52c4 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Tue, 6 Aug 2024 14:11:46 -0400 Subject: [PATCH 1/2] Return available function types for BindingDecls. Only return nullptr when we don't have an available QualType. --- clang/lib/AST/DeclBase.cpp | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index 98a7746b7d997..129a119267890 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -1161,15 +1161,20 @@ int64_t Decl::getID() const { const FunctionType *Decl::getFunctionType(bool BlocksToo) const { QualType Ty; - if (isa(this)) -return nullptr; - else if (const auto *D = dyn_cast(this)) + if (const auto *D = dyn_cast(this)) Ty = D->getType(); else if (const auto *D = dyn_cast(this)) Ty = D->getUnderlyingType(); else return nullptr; + if (Ty.isNull()) { +// BindingDecls do not have types during parsing, so return nullptr. This is +// the only known case where `Ty` is null. +assert(isa(this)); +return nullptr; + } + if (Ty->isFunctionPointerType()) Ty = Ty->castAs()->getPointeeType(); else if (Ty->isFunctionReferenceType()) >From 56a9d3d2e29289323f30b6dbed2b43a6a76a6b08 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Wed, 7 Aug 2024 12:37:24 -0400 Subject: [PATCH 2/2] Add unit test for BindingDecl.getFunctionType(). --- clang/unittests/AST/CMakeLists.txt| 1 + clang/unittests/AST/DeclBaseTest.cpp | 59 +++ .../gn/secondary/clang/unittests/AST/BUILD.gn | 1 + 3 files changed, 61 insertions(+) create mode 100644 clang/unittests/AST/DeclBaseTest.cpp diff --git a/clang/unittests/AST/CMakeLists.txt b/clang/unittests/AST/CMakeLists.txt index dcc9bc0f39ac2..51245599736cf 100644 --- a/clang/unittests/AST/CMakeLists.txt +++ b/clang/unittests/AST/CMakeLists.txt @@ -26,6 +26,7 @@ add_clang_unittest(ASTTests CommentTextTest.cpp ConceptPrinterTest.cpp DataCollectionTest.cpp + DeclBaseTest.cpp DeclPrinterTest.cpp DeclTest.cpp EvaluateAsRValueTest.cpp diff --git a/clang/unittests/AST/DeclBaseTest.cpp b/clang/unittests/AST/DeclBaseTest.cpp new file mode 100644 index 0..39e97aa731196 --- /dev/null +++ b/clang/unittests/AST/DeclBaseTest.cpp @@ -0,0 +1,59 @@ +//===- unittests/AST/DeclBaseTest.cpp --- Declaration tests===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +// +// Unit tests for Decl class in the AST. +// +//===--===// + +#include "clang/AST/DeclBase.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/DeclCXX.h" +#include "clang/AST/Type.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/LLVM.h" +#include "clang/Tooling/Tooling.h" +#include "gtest/gtest.h" + +using ::clang::BindingDecl; +using ::clang::ast_matchers::bindingDecl; +using ::clang::ast_matchers::hasName; +using ::clang::ast_matchers::match; +using ::clang::ast_matchers::selectFirst; + +TEST(DeclGetFunctionType, BindingDecl) { + llvm::StringRef Code = R"cpp( +template +struct Pair { + A AnA; + B AB; +}; + +void target(int *i) { + Pair P; + auto [FunctionPointer, B] = P; + FunctionPointer(i); +} + )cpp"; + + auto AST = + clang::tooling::buildASTFromCodeWithArgs(Code, /*Args=*/{"-std=c++20"}); + clang::ASTContext &Ctx = AST->getASTContext(); + + auto *BD = selectFirst( + "FunctionPointer", + match(bindingDecl(hasName("FunctionPointer")).bind("FunctionPointer"), +Ctx)); + ASSERT_NE(BD, nullptr); + + EXPECT_NE(BD->getFunctionType(), nullptr); + + // Emulate a call before the BindingDecl has a bound type. + const_cast(BD)->setBinding(clang::QualType(), nullptr); + EXPECT_EQ(BD->getFunctionType(), nullptr); +} diff --git a/llvm/utils/gn/secondary/clang/unittests/AST/BUILD.gn b/llvm/utils/gn/secondary/clang/unittests/AST/BUILD.gn index 7451c406d2ac8..4b905c17eea44 100644 --- a/llvm/utils/gn/secondary/clang/unittests/AST/BUILD.gn +++ b/llvm/utils/gn/secondary/clang/unittests/AST/BUILD.gn @@ -34,6 +34,7 @@ unittest("ASTTests") { "CommentTextTest.cpp", "ConceptPrinterTest.cpp", "DataCollectionTest.cpp", +"DeclBaseTest.cpp", "DeclPrinterTest.cpp", "DeclTest.cpp", "EvaluateAsRValueTest.cpp", ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Return available function types for BindingDecls. (PR #102196)
bazuzi wrote: This is a pointer nullability analysis being developed outside of clang. The relevant analysis code, including our workaround, can be seen at https://github.com/google/crubit/blob/main/nullability/inference/collect_evidence.cc#L757. https://github.com/llvm/llvm-project/pull/102196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Return available function types for BindingDecls. (PR #102196)
bazuzi wrote: For the following: ``` template struct Pair { Pair(); A a; B b; }; void target(int *i) { Pair p; auto [fp, b] = p; fp(i); } ``` Running or compiling this code presents no misbehavior that I know of, but when running a dataflow analysis that queries `getFunctionType()` for `fp`, `nullptr` was returned, when in fact there was a valid type expected, roughly `void (*)(int*)`. https://github.com/llvm/llvm-project/pull/102196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Return available function types for BindingDecls. (PR #102196)
bazuzi wrote: This is a bug fix, yes, but there are currently no tests at all for this function that I can find (and no test file corresponding to this file or its header), so I'm not sure what the scope of a new unit test should be. I also don't know how I would most realistically trigger the circumstance of a BindingDecl without a QualType set. The recent changes to this function that introduced the issue, along with the previous two changes to this function (though 5 and 6 years ago now), do not include any tests or any mention of the changes in release notes (#89906 and #90495). I'm not aware of what magnitude of change qualifies for release notes, but this change seems pretty small to me. https://github.com/llvm/llvm-project/pull/102196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Return available function types for BindingDecls. (PR #102196)
bazuzi wrote: Thanks for the quick review! Could you merge for me? I don't have write access. https://github.com/llvm/llvm-project/pull/102196 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Return available function types for BindingDecls. (PR #102196)
https://github.com/bazuzi created https://github.com/llvm/llvm-project/pull/102196 Only return nullptr when we don't have an available QualType. >From 052eff82638bdfbc8c7b8a2b90bcd9a0c46e52c4 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Tue, 6 Aug 2024 14:11:46 -0400 Subject: [PATCH] Return available function types for BindingDecls. Only return nullptr when we don't have an available QualType. --- clang/lib/AST/DeclBase.cpp | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index 98a7746b7d997..129a119267890 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -1161,15 +1161,20 @@ int64_t Decl::getID() const { const FunctionType *Decl::getFunctionType(bool BlocksToo) const { QualType Ty; - if (isa(this)) -return nullptr; - else if (const auto *D = dyn_cast(this)) + if (const auto *D = dyn_cast(this)) Ty = D->getType(); else if (const auto *D = dyn_cast(this)) Ty = D->getUnderlyingType(); else return nullptr; + if (Ty.isNull()) { +// BindingDecls do not have types during parsing, so return nullptr. This is +// the only known case where `Ty` is null. +assert(isa(this)); +return nullptr; + } + if (Ty->isFunctionPointerType()) Ty = Ty->castAs()->getPointeeType(); else if (Ty->isFunctionReferenceType()) ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Handle this-capturing lambdas in field initializers. (PR #99519)
bazuzi wrote: @ymand Can you review and eventually merge for me? https://github.com/llvm/llvm-project/pull/99519 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Handle this-capturing lambdas in field initializers. (PR #99519)
https://github.com/bazuzi created https://github.com/llvm/llvm-project/pull/99519 We previously would assume these were inside a method definition and end up crashing. >From 23f74d7f2e0864d4cc667fa191a8511b2759428f Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Thu, 18 Jul 2024 11:07:03 -0400 Subject: [PATCH] [clang][dataflow] Handle this-capturing lambdas in field initializers. We previously would assume these were inside a method definition and end up crashing. --- .../FlowSensitive/DataflowEnvironment.cpp | 21 ++ .../FlowSensitive/DataflowEnvironmentTest.cpp | 28 +++ 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index 7c88917faf9c6..6c0e2f44cd5ae 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -518,12 +518,21 @@ void Environment::initialize() { assert(VarDecl != nullptr); setStorageLocation(*VarDecl, createObject(*VarDecl, nullptr)); } else if (Capture.capturesThis()) { - const auto *SurroundingMethodDecl = - cast(InitialTargetFunc->getNonClosureAncestor()); - QualType ThisPointeeType = - SurroundingMethodDecl->getFunctionObjectParameterType(); - setThisPointeeStorageLocation( - cast(createObject(ThisPointeeType))); + if (auto *Ancestor = InitialTargetFunc->getNonClosureAncestor()) { +const auto *SurroundingMethodDecl = cast(Ancestor); +QualType ThisPointeeType = +SurroundingMethodDecl->getFunctionObjectParameterType(); +setThisPointeeStorageLocation( +cast(createObject(ThisPointeeType))); + } else if (auto *FieldBeingInitialized = + dyn_cast(Parent->getLambdaContextDecl())) { +// This is in a field initializer, rather than a method. +setThisPointeeStorageLocation( +cast(createObject(QualType( +FieldBeingInitialized->getParent()->getTypeForDecl(), 0; + } else { +assert(false && "Unexpected this-capturing lambda context."); + } } } } else if (MethodDecl->isImplicitObjectMemberFunction()) { diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp index bd710a00c47ce..296ea5a3b386b 100644 --- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp @@ -436,4 +436,32 @@ TEST_F(EnvironmentTest, Stmt) { Env.getResultObjectLocation(*Init); } +// This is a crash repro. +TEST_F(EnvironmentTest, LambdaCapturingThisInFieldInitializer) { + using namespace ast_matchers; + std::string Code = R"cc( + struct S { +int f{[this]() { return 1; }()}; + }; +)cc"; + + auto Unit = + tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++11"}); + auto &Context = Unit->getASTContext(); + + ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U); + + auto *LambdaCallOperator = selectFirst( + "method", match(cxxMethodDecl(hasName("operator()"), +ofClass(cxxRecordDecl(isLambda( + .bind("method"), + Context)); + + Environment Env(DAContext, *LambdaCallOperator); + // Don't crash when initializing. + Env.initialize(); + // And initialize the captured `this` pointee. + ASSERT_NE(nullptr, Env.getThisPointeeStorageLocation()); +} + } // namespace ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Only propagate past CXXDefaultInitExpr if init is (PR #99236)
@@ -465,7 +465,12 @@ class ResultObjectVisitor : public AnalysisASTVisitor { } if (auto *DIE = dyn_cast(E)) { - PropagateResultObject(DIE->getExpr(), Loc); + // If it has a rewritten init, we should propagate to that. If it doesn't, + // then the CXXDefaultInitExpr is the only initializer available during + // the analysis as the underlying Expr is only traversed as a child of the + // Decl being initialized, which is not usually in the CFG. + if (DIE->hasRewrittenInit()) bazuzi wrote: I was working off of a now-out-of-date understanding of CFG-building (from this version: https://github.com/llvm/llvm-project/blob/905b402a5d8f1490d668f40942390ebd6e87aa8f/clang/lib/Analysis/CFG.cpp) where the underlying initializer was added to the CFG only when the init was rewritten. That functionality was then reverted. Additional complexity comes in from the CFG-building option AddCXXDefaultInitExprInCtors, which is set true for dataflow analyses, causing the underlying initializers to be included in CFGs for constructors only. It seemed to me that we don't want to propagate if the underlying initializer is not included in the CFG, but detection of that characteristic is not directly available. @martinboehme and @ymand this is a continuation of our discussions. https://github.com/llvm/llvm-project/pull/99236 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Only propagate past CXXDefaultInitExpr if init is (PR #99236)
https://github.com/bazuzi created https://github.com/llvm/llvm-project/pull/99236 rewritten. >From 7c75181c590f30cc439af6fa523f3ca78f8a933d Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Tue, 16 Jul 2024 16:25:32 -0400 Subject: [PATCH] [clang][dataflow] Only propagate past CXXDefaultInitExpr if init is rewritten. --- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp | 7 ++- .../Analysis/FlowSensitive/DataflowEnvironmentTest.cpp | 2 ++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index f734168e647bd..e6be6f9aa1dd0 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -465,7 +465,12 @@ class ResultObjectVisitor : public AnalysisASTVisitor { } if (auto *DIE = dyn_cast(E)) { - PropagateResultObject(DIE->getExpr(), Loc); + // If it has a rewritten init, we should propagate to that. If it doesn't, + // then the CXXDefaultInitExpr is the only initializer available during + // the analysis as the underlying Expr is only traversed as a child of the + // Decl being initialized, which is not usually in the CFG. + if (DIE->hasRewrittenInit()) +PropagateResultObject(DIE->getExpr(), Loc); return; } diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp index a4ac597bb06d6..f93e5d9ec527a 100644 --- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp @@ -435,6 +435,8 @@ TEST_F(EnvironmentTest, CXXDefaultInitExprResultObjIsWrappedExprResultObj) { const auto *Constructor = selectFirst("ctor", Results); const auto *DefaultInit = selectFirst("default_init_expr", Results); + // We only propagate past the `CXXDefaultInitExpr` if it has a rewritten init. + ASSERT_TRUE(DefaultInit->hasRewrittenInit()); Environment Env(DAContext, *Constructor); Env.initialize(); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow]Propagate the result object location for CXXDefaultInitExpr. (PR #98490)
bazuzi wrote: Thanks for the review. Could you merge for me? I don't have commit access. https://github.com/llvm/llvm-project/pull/98490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow]Propagate the result object location for CXXDefaultInitExpr. (PR #98490)
https://github.com/bazuzi edited https://github.com/llvm/llvm-project/pull/98490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Propagate the result object location for CXXDefaultInitExpr. (PR #98490)
https://github.com/bazuzi updated https://github.com/llvm/llvm-project/pull/98490 >From 8f51730d1f4bbca740a01b89780b6ed9bee3c3d7 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Thu, 11 Jul 2024 09:56:17 -0400 Subject: [PATCH 1/2] Propagate the result object location for CXXDefaultInitExpr. These are not "original initializers"; the single node underneath represents the initializing node. --- .../lib/Analysis/FlowSensitive/DataflowEnvironment.cpp | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index 7c88917faf9c6..f734168e647bd 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -15,6 +15,7 @@ #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" +#include "clang/AST/ExprCXX.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/Stmt.h" #include "clang/AST/Type.h" @@ -414,8 +415,8 @@ class ResultObjectVisitor : public AnalysisASTVisitor { // lowest-level AST node that initializes a given object, and nothing // below them can initialize the same object (or part of it). if (isa(E) || isa(E) || isa(E) || -isa(E) || isa(E) || -isa(E) || isa(E) || +isa(E) || isa(E) || +isa(E) || // We treat `BuiltinBitCastExpr` as an "original initializer" too as // it may not even be casting from a record type -- and even if it is, // the two objects are in general of unrelated type. @@ -463,6 +464,11 @@ class ResultObjectVisitor : public AnalysisASTVisitor { return; } +if (auto *DIE = dyn_cast(E)) { + PropagateResultObject(DIE->getExpr(), Loc); + return; +} + // All other expression nodes that propagate a record prvalue should have // exactly one child. SmallVector Children(E->child_begin(), E->child_end()); >From 94efb2ff85bdd95cf74b2e0969715c368d3cc293 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Fri, 12 Jul 2024 12:09:45 -0400 Subject: [PATCH 2/2] Add test for matching result objects. --- .../FlowSensitive/DataflowEnvironmentTest.cpp | 37 +++ 1 file changed, 37 insertions(+) diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp index bd710a00c47ce..a4ac597bb06d6 100644 --- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp @@ -21,6 +21,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" #include +#include namespace { @@ -405,6 +406,42 @@ TEST_F(EnvironmentTest, Contains(Member)); } +// This is a repro of a failure case seen in the wild. +TEST_F(EnvironmentTest, CXXDefaultInitExprResultObjIsWrappedExprResultObj) { + using namespace ast_matchers; + + std::string Code = R"cc( + struct Inner {}; + + struct S { +S() {} + +Inner i = {}; + }; + )cc"; + + auto Unit = + tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++11"}); + auto &Context = Unit->getASTContext(); + + ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U); + + auto Results = + match(cxxConstructorDecl( +hasAnyConstructorInitializer(cxxCtorInitializer( +withInitializer(expr().bind("default_init_expr") +.bind("ctor"), +Context); + const auto *Constructor = selectFirst("ctor", Results); + const auto *DefaultInit = + selectFirst("default_init_expr", Results); + + Environment Env(DAContext, *Constructor); + Env.initialize(); + EXPECT_EQ(&Env.getResultObjectLocation(*DefaultInit), +&Env.getResultObjectLocation(*DefaultInit->getExpr())); +} + TEST_F(EnvironmentTest, Stmt) { using namespace ast_matchers; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Propagate the result object location for CXXDefaultInitExpr. (PR #98490)
https://github.com/bazuzi created https://github.com/llvm/llvm-project/pull/98490 These are not "original initializers"; the single node underneath represents the initializing node. >From 8f51730d1f4bbca740a01b89780b6ed9bee3c3d7 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Thu, 11 Jul 2024 09:56:17 -0400 Subject: [PATCH] Propagate the result object location for CXXDefaultInitExpr. These are not "original initializers"; the single node underneath represents the initializing node. --- .../lib/Analysis/FlowSensitive/DataflowEnvironment.cpp | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index 7c88917faf9c6..f734168e647bd 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -15,6 +15,7 @@ #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" +#include "clang/AST/ExprCXX.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/Stmt.h" #include "clang/AST/Type.h" @@ -414,8 +415,8 @@ class ResultObjectVisitor : public AnalysisASTVisitor { // lowest-level AST node that initializes a given object, and nothing // below them can initialize the same object (or part of it). if (isa(E) || isa(E) || isa(E) || -isa(E) || isa(E) || -isa(E) || isa(E) || +isa(E) || isa(E) || +isa(E) || // We treat `BuiltinBitCastExpr` as an "original initializer" too as // it may not even be casting from a record type -- and even if it is, // the two objects are in general of unrelated type. @@ -463,6 +464,11 @@ class ResultObjectVisitor : public AnalysisASTVisitor { return; } +if (auto *DIE = dyn_cast(E)) { + PropagateResultObject(DIE->getExpr(), Loc); + return; +} + // All other expression nodes that propagate a record prvalue should have // exactly one child. SmallVector Children(E->child_begin(), E->child_end()); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Fully support Environment construction for Stmt analysis. (PR #91616)
@@ -718,16 +730,24 @@ class Environment { void pushCallInternal(const FunctionDecl *FuncDecl, ArrayRef Args); + // FIXME: Add support for resetting globals after function calls to enable + // the implementation of sound analyses. bazuzi wrote: Thanks for catching, there was some iteration on the placement of the implementation of `initFieldsGlobalsAndFuncs` and I forgot to move this back. https://github.com/llvm/llvm-project/pull/91616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Fully support Environment construction for Stmt analysis. (PR #91616)
@@ -164,23 +169,28 @@ class Environment { Environment &operator=(Environment &&Other) = default; /// Creates an environment that uses `DACtx` to store objects that encompass - /// the state of a program. - /// - /// If `DeclCtx` is a function, initializes the environment with symbolic - /// representations of the function parameters. - /// - /// If `DeclCtx` is a non-static member function, initializes the environment - /// with a symbolic representation of the `this` pointee. - Environment(DataflowAnalysisContext &DACtx, const DeclContext &DeclCtx); + /// the state of a program, with `S` as the initial analysis target. + Environment(DataflowAnalysisContext &DACtx, Stmt &S) : Environment(DACtx) { +InitialTargetStmt = &S; + } - /// Assigns storage locations and values to all parameters, captures, global - /// variables, fields and functions referenced in the function currently being - /// analyzed. + /// Creates an environment that uses `DACtx` to store objects that encompass + /// the state of a program, with `FD` as the initial analysis target. /// /// Requirements: /// /// The function must have a body, i.e. /// `FunctionDecl::doesThisDecalarationHaveABody()` must be true. + Environment(DataflowAnalysisContext &DACtx, const FunctionDecl &FD) + : Environment(DACtx, *FD.getBody()) { +InitialTargetFunc = &FD; + } + + /// Assigns storage locations and values to all parameters, captures, global + /// variables, fields and functions referenced in the initial analysis target. + /// + /// If the target is a non-static member function, initializes the environment + /// with a symbolic representation of the `this` pointee. void initialize(); bazuzi wrote: The call stack being assumed non-empty was the case previously. Now these two are not technically on the call stack, and the function will no longer crash if called without InitialTargetStmt being non-null. It just doesn't do anything. Updated the comment to indicate this. https://github.com/llvm/llvm-project/pull/91616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Fully support Environment construction for Stmt analysis. (PR #91616)
@@ -290,13 +295,13 @@ widenKeyToValueMap(const llvm::MapVector &CurMap, namespace { // Visitor that builds a map from record prvalues to result objects. -// This traverses the body of the function to be analyzed; for each result -// object that it encounters, it propagates the storage location of the result -// object to all record prvalues that can initialize it. +// This traverses the AST sub-tree to be visited; for each result object that bazuzi wrote: I found "analyzed" to be confusing because it seemed to imply that this Visitor was doing the analyzing, which within Environment typically refers to a dataflow analysis, not any generic analysis of something. Looks like all starting calls to use this Visitor use a Traverse* method, so the fact that it traverses seems obvious enough that we can just remove this clause. https://github.com/llvm/llvm-project/pull/91616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Fully support Environment construction for Stmt analysis. (PR #91616)
@@ -290,13 +295,13 @@ widenKeyToValueMap(const llvm::MapVector &CurMap, namespace { // Visitor that builds a map from record prvalues to result objects. -// This traverses the body of the function to be analyzed; for each result -// object that it encounters, it propagates the storage location of the result -// object to all record prvalues that can initialize it. +// This traverses the AST sub-tree to be visited; for each result object that +// it encounters, it propagates the storage location of the result object to all +// record prvalues that can initialize it. class ResultObjectVisitor : public RecursiveASTVisitor { public: // `ResultObjectMap` will be filled with a map from record prvalues to result - // object. If the function being analyzed returns a record by value, + // object. If the sub-tree to be visited returns a record by value, bazuzi wrote: Same confusion around analysis as above. I'm also having difficulty with the wording for this because this is a comment on the constructor, not a Visit/Traverse function. Tried another version. https://github.com/llvm/llvm-project/pull/91616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Fully support Environment construction for Stmt analysis. (PR #91616)
@@ -403,4 +405,35 @@ TEST_F(EnvironmentTest, Contains(Member)); } +TEST_F(EnvironmentTest, Stmt) { + using namespace ast_matchers; + + std::string Code = R"cc( + struct S {int i;}; + void foo() { +S AnS = S{1}; + } bazuzi wrote: A variable declaration outside a function doesn't actually exist within a Stmt, nor is it a Stmt. In order to analyze a global variable declaration, one option is to synthesize a DeclStmt that holds the VarDecl. I was trying to keep this test simple, but could add an additional test for this synthesis pattern? https://github.com/llvm/llvm-project/pull/91616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Fully support Environment construction for Stmt analysis. (PR #91616)
@@ -403,4 +405,35 @@ TEST_F(EnvironmentTest, Contains(Member)); } +TEST_F(EnvironmentTest, Stmt) { + using namespace ast_matchers; + + std::string Code = R"cc( + struct S {int i;}; + void foo() { +S AnS = S{1}; + } +)cc"; + auto Unit = + tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++11"}); + auto &Context = Unit->getASTContext(); + + ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U); + + auto *DeclStatement = const_cast( + selectFirst("d", match(declStmt().bind("d"), Context))); + ASSERT_THAT(DeclStatement, NotNull()); + auto *ConstructExpr = selectFirst( + "c", match(cxxConstructExpr().bind("c"), Context)); + ASSERT_THAT(ConstructExpr, NotNull()); + + // Verify that we can retrieve the result object location for the construction + // expression when we analyze the DeclStmt for `AnS`. + Environment Env(DAContext, *DeclStatement); + // Don't crash when initializing. + Env.initialize(); + // And don't crash when retrieving the result object location. + Env.getResultObjectLocation(*ConstructExpr); bazuzi wrote: Correct, only result objects and referenced decls. I didn't see much opportunity to check the value of anything in particular. https://github.com/llvm/llvm-project/pull/91616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Fully support Environment construction for Stmt analysis. (PR #91616)
@@ -146,6 +146,30 @@ TEST_F(DataflowAnalysisTest, DiagnoseFunctionDiagnoserCalledOnEachElement) { " (Lifetime ends)\n"))); } +TEST_F(DataflowAnalysisTest, CanAnalyzeStmt) { + std::string Code = R"cc( + struct S {int i;}; + S getAnS() {return S{1};}; + void foo() { +S AnS = getAnS(); + } bazuzi wrote: Same reason not to use a global variable as above. Additionally, `AdornedCFG::build` expects to take a `FunctionDecl` in which the provided `Stmt` resides. In the case of `DeclStmt`s, this `FunctionDecl` doesn't seem to actually be used at all, but support may not be guaranteed. https://github.com/llvm/llvm-project/pull/91616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Fully support Environment construction for Stmt analysis. (PR #91616)
@@ -146,6 +146,30 @@ TEST_F(DataflowAnalysisTest, DiagnoseFunctionDiagnoserCalledOnEachElement) { " (Lifetime ends)\n"))); } +TEST_F(DataflowAnalysisTest, CanAnalyzeStmt) { + std::string Code = R"cc( + struct S {int i;}; + S getAnS() {return S{1};}; bazuzi wrote: Just an ever so slightly more interesting analysis to test. Simplified now so that we can check the field value. https://github.com/llvm/llvm-project/pull/91616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Fully support Environment construction for Stmt analysis. (PR #91616)
@@ -146,6 +146,30 @@ TEST_F(DataflowAnalysisTest, DiagnoseFunctionDiagnoserCalledOnEachElement) { " (Lifetime ends)\n"))); } +TEST_F(DataflowAnalysisTest, CanAnalyzeStmt) { + std::string Code = R"cc( + struct S {int i;}; + S getAnS() {return S{1};}; + void foo() { +S AnS = getAnS(); + } +)cc"; + AST = tooling::buildASTFromCodeWithArgs(Code, {"-std=c++11"}); + const auto &DeclStatement = matchNode(declStmt()); + const auto &Func = matchNode(functionDecl(hasName("foo"))); + + ACFG = std::make_unique(llvm::cantFail(AdornedCFG::build( + Func, const_cast(DeclStatement), AST->getASTContext(; + + NoopAnalysis Analysis = NoopAnalysis(AST->getASTContext()); + DACtx = std::make_unique( + std::make_unique()); + Environment Env(*DACtx, const_cast(DeclStatement)); + + ASSERT_THAT_ERROR(runDataflowAnalysis(*ACFG, Analysis, Env).takeError(), +llvm::Succeeded()); bazuzi wrote: Changed to bool and added a check. https://github.com/llvm/llvm-project/pull/91616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Fully support Environment construction for Stmt analysis. (PR #91616)
https://github.com/bazuzi updated https://github.com/llvm/llvm-project/pull/91616 >From b62a95001e32a0c1d63204950481eb500c9d0275 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Thu, 9 May 2024 12:00:06 -0400 Subject: [PATCH 1/7] [clang][dataflow] Fully support Environment construction for Stmt analysis. Assume in fewer places that the analysis is of a FunctionDecl, and initialize the Environment properly for Stmts. Moves constructors for Environment to header to make it more obvious that there are only minor differences between them and very little initialization in the constructors. Tested check-clang-tooling. --- .../FlowSensitive/DataflowEnvironment.h | 133 -- .../FlowSensitive/DataflowEnvironment.cpp | 113 +++ .../TypeErasedDataflowAnalysis.cpp| 2 +- .../FlowSensitive/DataflowEnvironmentTest.cpp | 33 + .../Analysis/FlowSensitive/TestingSupport.h | 4 +- 5 files changed, 176 insertions(+), 109 deletions(-) diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h index cdf89c7def2c9..dfc3f6a35e4e0 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -19,6 +19,7 @@ #include "clang/AST/DeclBase.h" #include "clang/AST/Expr.h" #include "clang/AST/Type.h" +#include "clang/Analysis/FlowSensitive/ASTOps.h" #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h" #include "clang/Analysis/FlowSensitive/DataflowLattice.h" #include "clang/Analysis/FlowSensitive/Formula.h" @@ -28,8 +29,10 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/MapVector.h" +#include "llvm/ADT/PointerUnion.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/ErrorHandling.h" +#include #include #include #include @@ -155,7 +158,9 @@ class Environment { /// Creates an environment that uses `DACtx` to store objects that encompass /// the state of a program. - explicit Environment(DataflowAnalysisContext &DACtx); + explicit Environment(DataflowAnalysisContext &DACtx) + : DACtx(&DACtx), +FlowConditionToken(DACtx.arena().makeFlowConditionToken()) {} // Copy-constructor is private, Environments should not be copied. See fork(). Environment &operator=(const Environment &Other) = delete; @@ -164,23 +169,25 @@ class Environment { Environment &operator=(Environment &&Other) = default; /// Creates an environment that uses `DACtx` to store objects that encompass - /// the state of a program. - /// - /// If `DeclCtx` is a function, initializes the environment with symbolic - /// representations of the function parameters. - /// - /// If `DeclCtx` is a non-static member function, initializes the environment - /// with a symbolic representation of the `this` pointee. - Environment(DataflowAnalysisContext &DACtx, const DeclContext &DeclCtx); + /// the state of a program, with `FD` as the current analysis target. + Environment(DataflowAnalysisContext &DACtx, const FunctionDecl &FD) + : Environment(DACtx) { +TargetStack.push_back(&FD); + } + + /// Creates an environment that uses `DACtx` to store objects that encompass + /// the state of a program, with `S` as the current analysis target. + Environment(DataflowAnalysisContext &DACtx, Stmt &S) : Environment(DACtx) { +TargetStack.push_back(&S); + } /// Assigns storage locations and values to all parameters, captures, global - /// variables, fields and functions referenced in the function currently being - /// analyzed. + /// variables, fields and functions referenced in the target currently + /// being analyzed. /// - /// Requirements: - /// - /// The function must have a body, i.e. - /// `FunctionDecl::doesThisDecalarationHaveABody()` must be true. + /// If the current analysis target is a non-static member function, + /// initializes the environment with a symbolic representation of the `this` + /// pointee. void initialize(); /// Returns a new environment that is a copy of this one. @@ -365,46 +372,51 @@ class Environment { RecordStorageLocation & getResultObjectLocation(const Expr &RecordPRValue) const; - /// Returns the return value of the current function. This can be null if: + /// Returns the return value of the function currently being analyzed. + /// This can be null if: /// - The function has a void return type /// - No return value could be determined for the function, for example /// because it calls a function without a body. /// /// Requirements: - /// The current function must have a non-reference return type. + /// The current analysis target must be a function and must have a + /// non-reference return type. Value *getReturnValue() const { assert(getCurrentFunc() != nullptr && !getCurrentFunc()->getReturnType()->is
[clang] [clang][dataflow] Fully support Environment construction for Stmt analysis. (PR #91616)
https://github.com/bazuzi updated https://github.com/llvm/llvm-project/pull/91616 >From b62a95001e32a0c1d63204950481eb500c9d0275 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Thu, 9 May 2024 12:00:06 -0400 Subject: [PATCH 1/6] [clang][dataflow] Fully support Environment construction for Stmt analysis. Assume in fewer places that the analysis is of a FunctionDecl, and initialize the Environment properly for Stmts. Moves constructors for Environment to header to make it more obvious that there are only minor differences between them and very little initialization in the constructors. Tested check-clang-tooling. --- .../FlowSensitive/DataflowEnvironment.h | 133 -- .../FlowSensitive/DataflowEnvironment.cpp | 113 +++ .../TypeErasedDataflowAnalysis.cpp| 2 +- .../FlowSensitive/DataflowEnvironmentTest.cpp | 33 + .../Analysis/FlowSensitive/TestingSupport.h | 4 +- 5 files changed, 176 insertions(+), 109 deletions(-) diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h index cdf89c7def2c9..dfc3f6a35e4e0 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -19,6 +19,7 @@ #include "clang/AST/DeclBase.h" #include "clang/AST/Expr.h" #include "clang/AST/Type.h" +#include "clang/Analysis/FlowSensitive/ASTOps.h" #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h" #include "clang/Analysis/FlowSensitive/DataflowLattice.h" #include "clang/Analysis/FlowSensitive/Formula.h" @@ -28,8 +29,10 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/MapVector.h" +#include "llvm/ADT/PointerUnion.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/ErrorHandling.h" +#include #include #include #include @@ -155,7 +158,9 @@ class Environment { /// Creates an environment that uses `DACtx` to store objects that encompass /// the state of a program. - explicit Environment(DataflowAnalysisContext &DACtx); + explicit Environment(DataflowAnalysisContext &DACtx) + : DACtx(&DACtx), +FlowConditionToken(DACtx.arena().makeFlowConditionToken()) {} // Copy-constructor is private, Environments should not be copied. See fork(). Environment &operator=(const Environment &Other) = delete; @@ -164,23 +169,25 @@ class Environment { Environment &operator=(Environment &&Other) = default; /// Creates an environment that uses `DACtx` to store objects that encompass - /// the state of a program. - /// - /// If `DeclCtx` is a function, initializes the environment with symbolic - /// representations of the function parameters. - /// - /// If `DeclCtx` is a non-static member function, initializes the environment - /// with a symbolic representation of the `this` pointee. - Environment(DataflowAnalysisContext &DACtx, const DeclContext &DeclCtx); + /// the state of a program, with `FD` as the current analysis target. + Environment(DataflowAnalysisContext &DACtx, const FunctionDecl &FD) + : Environment(DACtx) { +TargetStack.push_back(&FD); + } + + /// Creates an environment that uses `DACtx` to store objects that encompass + /// the state of a program, with `S` as the current analysis target. + Environment(DataflowAnalysisContext &DACtx, Stmt &S) : Environment(DACtx) { +TargetStack.push_back(&S); + } /// Assigns storage locations and values to all parameters, captures, global - /// variables, fields and functions referenced in the function currently being - /// analyzed. + /// variables, fields and functions referenced in the target currently + /// being analyzed. /// - /// Requirements: - /// - /// The function must have a body, i.e. - /// `FunctionDecl::doesThisDecalarationHaveABody()` must be true. + /// If the current analysis target is a non-static member function, + /// initializes the environment with a symbolic representation of the `this` + /// pointee. void initialize(); /// Returns a new environment that is a copy of this one. @@ -365,46 +372,51 @@ class Environment { RecordStorageLocation & getResultObjectLocation(const Expr &RecordPRValue) const; - /// Returns the return value of the current function. This can be null if: + /// Returns the return value of the function currently being analyzed. + /// This can be null if: /// - The function has a void return type /// - No return value could be determined for the function, for example /// because it calls a function without a body. /// /// Requirements: - /// The current function must have a non-reference return type. + /// The current analysis target must be a function and must have a + /// non-reference return type. Value *getReturnValue() const { assert(getCurrentFunc() != nullptr && !getCurrentFunc()->getReturnType()->is
[clang] [clang][dataflow] Fully support Environment construction for Stmt analysis. (PR #91616)
https://github.com/bazuzi updated https://github.com/llvm/llvm-project/pull/91616 >From b62a95001e32a0c1d63204950481eb500c9d0275 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Thu, 9 May 2024 12:00:06 -0400 Subject: [PATCH 1/5] [clang][dataflow] Fully support Environment construction for Stmt analysis. Assume in fewer places that the analysis is of a FunctionDecl, and initialize the Environment properly for Stmts. Moves constructors for Environment to header to make it more obvious that there are only minor differences between them and very little initialization in the constructors. Tested check-clang-tooling. --- .../FlowSensitive/DataflowEnvironment.h | 133 -- .../FlowSensitive/DataflowEnvironment.cpp | 113 +++ .../TypeErasedDataflowAnalysis.cpp| 2 +- .../FlowSensitive/DataflowEnvironmentTest.cpp | 33 + .../Analysis/FlowSensitive/TestingSupport.h | 4 +- 5 files changed, 176 insertions(+), 109 deletions(-) diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h index cdf89c7def2c9..dfc3f6a35e4e0 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -19,6 +19,7 @@ #include "clang/AST/DeclBase.h" #include "clang/AST/Expr.h" #include "clang/AST/Type.h" +#include "clang/Analysis/FlowSensitive/ASTOps.h" #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h" #include "clang/Analysis/FlowSensitive/DataflowLattice.h" #include "clang/Analysis/FlowSensitive/Formula.h" @@ -28,8 +29,10 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/MapVector.h" +#include "llvm/ADT/PointerUnion.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/ErrorHandling.h" +#include #include #include #include @@ -155,7 +158,9 @@ class Environment { /// Creates an environment that uses `DACtx` to store objects that encompass /// the state of a program. - explicit Environment(DataflowAnalysisContext &DACtx); + explicit Environment(DataflowAnalysisContext &DACtx) + : DACtx(&DACtx), +FlowConditionToken(DACtx.arena().makeFlowConditionToken()) {} // Copy-constructor is private, Environments should not be copied. See fork(). Environment &operator=(const Environment &Other) = delete; @@ -164,23 +169,25 @@ class Environment { Environment &operator=(Environment &&Other) = default; /// Creates an environment that uses `DACtx` to store objects that encompass - /// the state of a program. - /// - /// If `DeclCtx` is a function, initializes the environment with symbolic - /// representations of the function parameters. - /// - /// If `DeclCtx` is a non-static member function, initializes the environment - /// with a symbolic representation of the `this` pointee. - Environment(DataflowAnalysisContext &DACtx, const DeclContext &DeclCtx); + /// the state of a program, with `FD` as the current analysis target. + Environment(DataflowAnalysisContext &DACtx, const FunctionDecl &FD) + : Environment(DACtx) { +TargetStack.push_back(&FD); + } + + /// Creates an environment that uses `DACtx` to store objects that encompass + /// the state of a program, with `S` as the current analysis target. + Environment(DataflowAnalysisContext &DACtx, Stmt &S) : Environment(DACtx) { +TargetStack.push_back(&S); + } /// Assigns storage locations and values to all parameters, captures, global - /// variables, fields and functions referenced in the function currently being - /// analyzed. + /// variables, fields and functions referenced in the target currently + /// being analyzed. /// - /// Requirements: - /// - /// The function must have a body, i.e. - /// `FunctionDecl::doesThisDecalarationHaveABody()` must be true. + /// If the current analysis target is a non-static member function, + /// initializes the environment with a symbolic representation of the `this` + /// pointee. void initialize(); /// Returns a new environment that is a copy of this one. @@ -365,46 +372,51 @@ class Environment { RecordStorageLocation & getResultObjectLocation(const Expr &RecordPRValue) const; - /// Returns the return value of the current function. This can be null if: + /// Returns the return value of the function currently being analyzed. + /// This can be null if: /// - The function has a void return type /// - No return value could be determined for the function, for example /// because it calls a function without a body. /// /// Requirements: - /// The current function must have a non-reference return type. + /// The current analysis target must be a function and must have a + /// non-reference return type. Value *getReturnValue() const { assert(getCurrentFunc() != nullptr && !getCurrentFunc()->getReturnType()->is
[clang] [clang][dataflow] Fully support Environment construction for Stmt analysis. (PR #91616)
https://github.com/bazuzi updated https://github.com/llvm/llvm-project/pull/91616 >From b62a95001e32a0c1d63204950481eb500c9d0275 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Thu, 9 May 2024 12:00:06 -0400 Subject: [PATCH 1/4] [clang][dataflow] Fully support Environment construction for Stmt analysis. Assume in fewer places that the analysis is of a FunctionDecl, and initialize the Environment properly for Stmts. Moves constructors for Environment to header to make it more obvious that there are only minor differences between them and very little initialization in the constructors. Tested check-clang-tooling. --- .../FlowSensitive/DataflowEnvironment.h | 133 -- .../FlowSensitive/DataflowEnvironment.cpp | 113 +++ .../TypeErasedDataflowAnalysis.cpp| 2 +- .../FlowSensitive/DataflowEnvironmentTest.cpp | 33 + .../Analysis/FlowSensitive/TestingSupport.h | 4 +- 5 files changed, 176 insertions(+), 109 deletions(-) diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h index cdf89c7def2c9..dfc3f6a35e4e0 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -19,6 +19,7 @@ #include "clang/AST/DeclBase.h" #include "clang/AST/Expr.h" #include "clang/AST/Type.h" +#include "clang/Analysis/FlowSensitive/ASTOps.h" #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h" #include "clang/Analysis/FlowSensitive/DataflowLattice.h" #include "clang/Analysis/FlowSensitive/Formula.h" @@ -28,8 +29,10 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/MapVector.h" +#include "llvm/ADT/PointerUnion.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/ErrorHandling.h" +#include #include #include #include @@ -155,7 +158,9 @@ class Environment { /// Creates an environment that uses `DACtx` to store objects that encompass /// the state of a program. - explicit Environment(DataflowAnalysisContext &DACtx); + explicit Environment(DataflowAnalysisContext &DACtx) + : DACtx(&DACtx), +FlowConditionToken(DACtx.arena().makeFlowConditionToken()) {} // Copy-constructor is private, Environments should not be copied. See fork(). Environment &operator=(const Environment &Other) = delete; @@ -164,23 +169,25 @@ class Environment { Environment &operator=(Environment &&Other) = default; /// Creates an environment that uses `DACtx` to store objects that encompass - /// the state of a program. - /// - /// If `DeclCtx` is a function, initializes the environment with symbolic - /// representations of the function parameters. - /// - /// If `DeclCtx` is a non-static member function, initializes the environment - /// with a symbolic representation of the `this` pointee. - Environment(DataflowAnalysisContext &DACtx, const DeclContext &DeclCtx); + /// the state of a program, with `FD` as the current analysis target. + Environment(DataflowAnalysisContext &DACtx, const FunctionDecl &FD) + : Environment(DACtx) { +TargetStack.push_back(&FD); + } + + /// Creates an environment that uses `DACtx` to store objects that encompass + /// the state of a program, with `S` as the current analysis target. + Environment(DataflowAnalysisContext &DACtx, Stmt &S) : Environment(DACtx) { +TargetStack.push_back(&S); + } /// Assigns storage locations and values to all parameters, captures, global - /// variables, fields and functions referenced in the function currently being - /// analyzed. + /// variables, fields and functions referenced in the target currently + /// being analyzed. /// - /// Requirements: - /// - /// The function must have a body, i.e. - /// `FunctionDecl::doesThisDecalarationHaveABody()` must be true. + /// If the current analysis target is a non-static member function, + /// initializes the environment with a symbolic representation of the `this` + /// pointee. void initialize(); /// Returns a new environment that is a copy of this one. @@ -365,46 +372,51 @@ class Environment { RecordStorageLocation & getResultObjectLocation(const Expr &RecordPRValue) const; - /// Returns the return value of the current function. This can be null if: + /// Returns the return value of the function currently being analyzed. + /// This can be null if: /// - The function has a void return type /// - No return value could be determined for the function, for example /// because it calls a function without a body. /// /// Requirements: - /// The current function must have a non-reference return type. + /// The current analysis target must be a function and must have a + /// non-reference return type. Value *getReturnValue() const { assert(getCurrentFunc() != nullptr && !getCurrentFunc()->getReturnType()->is
[clang] [clang][dataflow] Fully support Environment construction for Stmt analysis. (PR #91616)
https://github.com/bazuzi updated https://github.com/llvm/llvm-project/pull/91616 >From b62a95001e32a0c1d63204950481eb500c9d0275 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Thu, 9 May 2024 12:00:06 -0400 Subject: [PATCH 1/3] [clang][dataflow] Fully support Environment construction for Stmt analysis. Assume in fewer places that the analysis is of a FunctionDecl, and initialize the Environment properly for Stmts. Moves constructors for Environment to header to make it more obvious that there are only minor differences between them and very little initialization in the constructors. Tested check-clang-tooling. --- .../FlowSensitive/DataflowEnvironment.h | 133 -- .../FlowSensitive/DataflowEnvironment.cpp | 113 +++ .../TypeErasedDataflowAnalysis.cpp| 2 +- .../FlowSensitive/DataflowEnvironmentTest.cpp | 33 + .../Analysis/FlowSensitive/TestingSupport.h | 4 +- 5 files changed, 176 insertions(+), 109 deletions(-) diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h index cdf89c7def2c9..dfc3f6a35e4e0 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -19,6 +19,7 @@ #include "clang/AST/DeclBase.h" #include "clang/AST/Expr.h" #include "clang/AST/Type.h" +#include "clang/Analysis/FlowSensitive/ASTOps.h" #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h" #include "clang/Analysis/FlowSensitive/DataflowLattice.h" #include "clang/Analysis/FlowSensitive/Formula.h" @@ -28,8 +29,10 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/MapVector.h" +#include "llvm/ADT/PointerUnion.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/ErrorHandling.h" +#include #include #include #include @@ -155,7 +158,9 @@ class Environment { /// Creates an environment that uses `DACtx` to store objects that encompass /// the state of a program. - explicit Environment(DataflowAnalysisContext &DACtx); + explicit Environment(DataflowAnalysisContext &DACtx) + : DACtx(&DACtx), +FlowConditionToken(DACtx.arena().makeFlowConditionToken()) {} // Copy-constructor is private, Environments should not be copied. See fork(). Environment &operator=(const Environment &Other) = delete; @@ -164,23 +169,25 @@ class Environment { Environment &operator=(Environment &&Other) = default; /// Creates an environment that uses `DACtx` to store objects that encompass - /// the state of a program. - /// - /// If `DeclCtx` is a function, initializes the environment with symbolic - /// representations of the function parameters. - /// - /// If `DeclCtx` is a non-static member function, initializes the environment - /// with a symbolic representation of the `this` pointee. - Environment(DataflowAnalysisContext &DACtx, const DeclContext &DeclCtx); + /// the state of a program, with `FD` as the current analysis target. + Environment(DataflowAnalysisContext &DACtx, const FunctionDecl &FD) + : Environment(DACtx) { +TargetStack.push_back(&FD); + } + + /// Creates an environment that uses `DACtx` to store objects that encompass + /// the state of a program, with `S` as the current analysis target. + Environment(DataflowAnalysisContext &DACtx, Stmt &S) : Environment(DACtx) { +TargetStack.push_back(&S); + } /// Assigns storage locations and values to all parameters, captures, global - /// variables, fields and functions referenced in the function currently being - /// analyzed. + /// variables, fields and functions referenced in the target currently + /// being analyzed. /// - /// Requirements: - /// - /// The function must have a body, i.e. - /// `FunctionDecl::doesThisDecalarationHaveABody()` must be true. + /// If the current analysis target is a non-static member function, + /// initializes the environment with a symbolic representation of the `this` + /// pointee. void initialize(); /// Returns a new environment that is a copy of this one. @@ -365,46 +372,51 @@ class Environment { RecordStorageLocation & getResultObjectLocation(const Expr &RecordPRValue) const; - /// Returns the return value of the current function. This can be null if: + /// Returns the return value of the function currently being analyzed. + /// This can be null if: /// - The function has a void return type /// - No return value could be determined for the function, for example /// because it calls a function without a body. /// /// Requirements: - /// The current function must have a non-reference return type. + /// The current analysis target must be a function and must have a + /// non-reference return type. Value *getReturnValue() const { assert(getCurrentFunc() != nullptr && !getCurrentFunc()->getReturnType()->is
[clang] [clang][dataflow] Fully support Environment construction for Stmt analysis. (PR #91616)
bazuzi wrote: I agree with the usefulness of storing the initial target separately, reducing checks and branching based on whether the current target is a `Stmt` or `Function`. Since I was touching everywhere `CallStack` was used anyway, I went ahead with not pushing the starting `FunctionDecl` onto that stack. I wasn't immediately able to reduce the `initialize` duplication for statements and function bodies since `buildResultObjectMap` and `initFieldsGlobalsAndFuncs`, via `getReferencedDecls`, both handle `FunctionDecl`s and `Stmt`s differently, pulling info from a `FunctionDecl` beyond just its body. If templating `initFieldsGlobalsAndFuncs` is really undesirable, I could either pass in the `ReferencedDecls` from current callers or insert an intermediate with two overloads that takes the `Stmt`/`FunctionDecl` and passes along the `ReferencedDecls`. https://github.com/llvm/llvm-project/pull/91616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Fully support Environment construction for Stmt analysis. (PR #91616)
https://github.com/bazuzi updated https://github.com/llvm/llvm-project/pull/91616 >From b62a95001e32a0c1d63204950481eb500c9d0275 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Thu, 9 May 2024 12:00:06 -0400 Subject: [PATCH 1/2] [clang][dataflow] Fully support Environment construction for Stmt analysis. Assume in fewer places that the analysis is of a FunctionDecl, and initialize the Environment properly for Stmts. Moves constructors for Environment to header to make it more obvious that there are only minor differences between them and very little initialization in the constructors. Tested check-clang-tooling. --- .../FlowSensitive/DataflowEnvironment.h | 133 -- .../FlowSensitive/DataflowEnvironment.cpp | 113 +++ .../TypeErasedDataflowAnalysis.cpp| 2 +- .../FlowSensitive/DataflowEnvironmentTest.cpp | 33 + .../Analysis/FlowSensitive/TestingSupport.h | 4 +- 5 files changed, 176 insertions(+), 109 deletions(-) diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h index cdf89c7def2c9..dfc3f6a35e4e0 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -19,6 +19,7 @@ #include "clang/AST/DeclBase.h" #include "clang/AST/Expr.h" #include "clang/AST/Type.h" +#include "clang/Analysis/FlowSensitive/ASTOps.h" #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h" #include "clang/Analysis/FlowSensitive/DataflowLattice.h" #include "clang/Analysis/FlowSensitive/Formula.h" @@ -28,8 +29,10 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/MapVector.h" +#include "llvm/ADT/PointerUnion.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/ErrorHandling.h" +#include #include #include #include @@ -155,7 +158,9 @@ class Environment { /// Creates an environment that uses `DACtx` to store objects that encompass /// the state of a program. - explicit Environment(DataflowAnalysisContext &DACtx); + explicit Environment(DataflowAnalysisContext &DACtx) + : DACtx(&DACtx), +FlowConditionToken(DACtx.arena().makeFlowConditionToken()) {} // Copy-constructor is private, Environments should not be copied. See fork(). Environment &operator=(const Environment &Other) = delete; @@ -164,23 +169,25 @@ class Environment { Environment &operator=(Environment &&Other) = default; /// Creates an environment that uses `DACtx` to store objects that encompass - /// the state of a program. - /// - /// If `DeclCtx` is a function, initializes the environment with symbolic - /// representations of the function parameters. - /// - /// If `DeclCtx` is a non-static member function, initializes the environment - /// with a symbolic representation of the `this` pointee. - Environment(DataflowAnalysisContext &DACtx, const DeclContext &DeclCtx); + /// the state of a program, with `FD` as the current analysis target. + Environment(DataflowAnalysisContext &DACtx, const FunctionDecl &FD) + : Environment(DACtx) { +TargetStack.push_back(&FD); + } + + /// Creates an environment that uses `DACtx` to store objects that encompass + /// the state of a program, with `S` as the current analysis target. + Environment(DataflowAnalysisContext &DACtx, Stmt &S) : Environment(DACtx) { +TargetStack.push_back(&S); + } /// Assigns storage locations and values to all parameters, captures, global - /// variables, fields and functions referenced in the function currently being - /// analyzed. + /// variables, fields and functions referenced in the target currently + /// being analyzed. /// - /// Requirements: - /// - /// The function must have a body, i.e. - /// `FunctionDecl::doesThisDecalarationHaveABody()` must be true. + /// If the current analysis target is a non-static member function, + /// initializes the environment with a symbolic representation of the `this` + /// pointee. void initialize(); /// Returns a new environment that is a copy of this one. @@ -365,46 +372,51 @@ class Environment { RecordStorageLocation & getResultObjectLocation(const Expr &RecordPRValue) const; - /// Returns the return value of the current function. This can be null if: + /// Returns the return value of the function currently being analyzed. + /// This can be null if: /// - The function has a void return type /// - No return value could be determined for the function, for example /// because it calls a function without a body. /// /// Requirements: - /// The current function must have a non-reference return type. + /// The current analysis target must be a function and must have a + /// non-reference return type. Value *getReturnValue() const { assert(getCurrentFunc() != nullptr && !getCurrentFunc()->getReturnType()->is
[clang] [clang][dataflow] Fully support Environment construction for Stmt analysis. (PR #91616)
bazuzi wrote: @ymand Can you review and eventually merge for me? https://github.com/llvm/llvm-project/pull/91616 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Fully support Environment construction for Stmt analysis. (PR #91616)
https://github.com/bazuzi created https://github.com/llvm/llvm-project/pull/91616 Assume in fewer places that the analysis is of a FunctionDecl, and initialize the Environment properly for Stmts. Moves constructors for Environment to header to make it more obvious that there are only minor differences between them and very little initialization in the constructors. Tested with check-clang-tooling. >From b62a95001e32a0c1d63204950481eb500c9d0275 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Thu, 9 May 2024 12:00:06 -0400 Subject: [PATCH] [clang][dataflow] Fully support Environment construction for Stmt analysis. Assume in fewer places that the analysis is of a FunctionDecl, and initialize the Environment properly for Stmts. Moves constructors for Environment to header to make it more obvious that there are only minor differences between them and very little initialization in the constructors. Tested check-clang-tooling. --- .../FlowSensitive/DataflowEnvironment.h | 133 -- .../FlowSensitive/DataflowEnvironment.cpp | 113 +++ .../TypeErasedDataflowAnalysis.cpp| 2 +- .../FlowSensitive/DataflowEnvironmentTest.cpp | 33 + .../Analysis/FlowSensitive/TestingSupport.h | 4 +- 5 files changed, 176 insertions(+), 109 deletions(-) diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h index cdf89c7def2c..dfc3f6a35e4e 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -19,6 +19,7 @@ #include "clang/AST/DeclBase.h" #include "clang/AST/Expr.h" #include "clang/AST/Type.h" +#include "clang/Analysis/FlowSensitive/ASTOps.h" #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h" #include "clang/Analysis/FlowSensitive/DataflowLattice.h" #include "clang/Analysis/FlowSensitive/Formula.h" @@ -28,8 +29,10 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/MapVector.h" +#include "llvm/ADT/PointerUnion.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/ErrorHandling.h" +#include #include #include #include @@ -155,7 +158,9 @@ class Environment { /// Creates an environment that uses `DACtx` to store objects that encompass /// the state of a program. - explicit Environment(DataflowAnalysisContext &DACtx); + explicit Environment(DataflowAnalysisContext &DACtx) + : DACtx(&DACtx), +FlowConditionToken(DACtx.arena().makeFlowConditionToken()) {} // Copy-constructor is private, Environments should not be copied. See fork(). Environment &operator=(const Environment &Other) = delete; @@ -164,23 +169,25 @@ class Environment { Environment &operator=(Environment &&Other) = default; /// Creates an environment that uses `DACtx` to store objects that encompass - /// the state of a program. - /// - /// If `DeclCtx` is a function, initializes the environment with symbolic - /// representations of the function parameters. - /// - /// If `DeclCtx` is a non-static member function, initializes the environment - /// with a symbolic representation of the `this` pointee. - Environment(DataflowAnalysisContext &DACtx, const DeclContext &DeclCtx); + /// the state of a program, with `FD` as the current analysis target. + Environment(DataflowAnalysisContext &DACtx, const FunctionDecl &FD) + : Environment(DACtx) { +TargetStack.push_back(&FD); + } + + /// Creates an environment that uses `DACtx` to store objects that encompass + /// the state of a program, with `S` as the current analysis target. + Environment(DataflowAnalysisContext &DACtx, Stmt &S) : Environment(DACtx) { +TargetStack.push_back(&S); + } /// Assigns storage locations and values to all parameters, captures, global - /// variables, fields and functions referenced in the function currently being - /// analyzed. + /// variables, fields and functions referenced in the target currently + /// being analyzed. /// - /// Requirements: - /// - /// The function must have a body, i.e. - /// `FunctionDecl::doesThisDecalarationHaveABody()` must be true. + /// If the current analysis target is a non-static member function, + /// initializes the environment with a symbolic representation of the `this` + /// pointee. void initialize(); /// Returns a new environment that is a copy of this one. @@ -365,46 +372,51 @@ class Environment { RecordStorageLocation & getResultObjectLocation(const Expr &RecordPRValue) const; - /// Returns the return value of the current function. This can be null if: + /// Returns the return value of the function currently being analyzed. + /// This can be null if: /// - The function has a void return type /// - No return value could be determined for the function, for example /// because it calls a function without a body. /// /
[clang] [clang][dataflow] Expose getReferencedDecls for a Stmt. (PR #89444)
bazuzi wrote: Ha, so fast I hadn't even hit send to ask for you to review yet. Can you merge for me? https://github.com/llvm/llvm-project/pull/89444 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Expose getReferencedDecls for a Stmt. (PR #89444)
https://github.com/bazuzi created https://github.com/llvm/llvm-project/pull/89444 None >From e7b1540984306bfb36a59fa7692208bfaf2bdc1a Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Fri, 19 Apr 2024 15:09:32 -0400 Subject: [PATCH] [clang][dataflow] Expose getReferencedDecls for a Stmt. --- clang/include/clang/Analysis/FlowSensitive/ASTOps.h | 3 +++ clang/lib/Analysis/FlowSensitive/ASTOps.cpp | 6 ++ 2 files changed, 9 insertions(+) diff --git a/clang/include/clang/Analysis/FlowSensitive/ASTOps.h b/clang/include/clang/Analysis/FlowSensitive/ASTOps.h index 27ad32c1694f77..ebc250e64fc33a 100644 --- a/clang/include/clang/Analysis/FlowSensitive/ASTOps.h +++ b/clang/include/clang/Analysis/FlowSensitive/ASTOps.h @@ -92,6 +92,9 @@ struct ReferencedDecls { /// Returns declarations that are declared in or referenced from `FD`. ReferencedDecls getReferencedDecls(const FunctionDecl &FD); +/// Returns declarations that are declared in or referenced from `S`. +ReferencedDecls getReferencedDecls(const Stmt &S); + } // namespace dataflow } // namespace clang diff --git a/clang/lib/Analysis/FlowSensitive/ASTOps.cpp b/clang/lib/Analysis/FlowSensitive/ASTOps.cpp index 75188aef4d1a43..329d4d8837a9a0 100644 --- a/clang/lib/Analysis/FlowSensitive/ASTOps.cpp +++ b/clang/lib/Analysis/FlowSensitive/ASTOps.cpp @@ -246,4 +246,10 @@ ReferencedDecls getReferencedDecls(const FunctionDecl &FD) { return Result; } +ReferencedDecls getReferencedDecls(const Stmt &S) { + ReferencedDecls Result; + getReferencedDecls(S, Result); + return Result; +} + } // namespace clang::dataflow ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Expose getReferencedDecls and relocate free functions. (PR #88754)
https://github.com/bazuzi updated https://github.com/llvm/llvm-project/pull/88754 >From 223d4c48abf27c8b0949ac1520b66ef32cbd63c1 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Fri, 12 Apr 2024 12:18:44 -0400 Subject: [PATCH 1/4] [clang][dataflow] Expose fields, globals, and functions referenced. Exposes the collection functionality, but does not alter it beyond using a return value instead of output parameters. Also relocates underlying and related functions and a class from DataflowEnvironment's files to DataflowAnalysisContext's files, as no Environment is needed. --- .../FlowSensitive/DataflowAnalysisContext.h | 46 + .../FlowSensitive/DataflowEnvironment.h | 36 .../FlowSensitive/DataflowAnalysisContext.cpp | 174 + .../FlowSensitive/DataflowEnvironment.cpp | 176 +- clang/lib/Analysis/FlowSensitive/Transfer.cpp | 1 + 5 files changed, 225 insertions(+), 208 deletions(-) diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h index 909a91059438ca..a34e5f603eb396 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h @@ -62,6 +62,52 @@ FieldSet getObjectFields(QualType Type); bool containsSameFields(const FieldSet &Fields, const RecordStorageLocation::FieldToLoc &FieldLocs); +/// Returns the fields of a `RecordDecl` that are initialized by an +/// `InitListExpr`, in the order in which they appear in +/// `InitListExpr::inits()`. +/// `Init->getType()` must be a record type. +std::vector +getFieldsForInitListExpr(const InitListExpr *InitList); + +/// Helper class for initialization of a record with an `InitListExpr`. +/// `InitListExpr::inits()` contains the initializers for both the base classes +/// and the fields of the record; this helper class separates these out into two +/// different lists. In addition, it deals with special cases associated with +/// unions. +class RecordInitListHelper { +public: + // `InitList` must have record type. + RecordInitListHelper(const InitListExpr *InitList); + + // Base classes with their associated initializer expressions. + ArrayRef> base_inits() const { +return BaseInits; + } + + // Fields with their associated initializer expressions. + ArrayRef> field_inits() const { +return FieldInits; + } + +private: + SmallVector> BaseInits; + SmallVector> FieldInits; + + // We potentially synthesize an `ImplicitValueInitExpr` for unions. It's a + // member variable because we store a pointer to it in `FieldInits`. + std::optional ImplicitValueInitForUnion; +}; + +struct FieldsGlobalsAndFuncs { + FieldSet Fields; + // Globals includes all variables with global storage, notably including + // static data members and static variables declared within a function. + llvm::DenseSet Globals; + llvm::DenseSet Funcs; +}; + +FieldsGlobalsAndFuncs getFieldsGlobalsAndFuncs(const FunctionDecl &FD); + struct ContextSensitiveOptions { /// The maximum depth to analyze. A value of zero is equivalent to disabling /// context-sensitive analysis entirely. diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h index 706664d7db1c25..4277792219c0af 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -775,42 +775,6 @@ RecordStorageLocation *getImplicitObjectLocation(const CXXMemberCallExpr &MCE, RecordStorageLocation *getBaseObjectLocation(const MemberExpr &ME, const Environment &Env); -/// Returns the fields of a `RecordDecl` that are initialized by an -/// `InitListExpr`, in the order in which they appear in -/// `InitListExpr::inits()`. -/// `Init->getType()` must be a record type. -std::vector -getFieldsForInitListExpr(const InitListExpr *InitList); - -/// Helper class for initialization of a record with an `InitListExpr`. -/// `InitListExpr::inits()` contains the initializers for both the base classes -/// and the fields of the record; this helper class separates these out into two -/// different lists. In addition, it deals with special cases associated with -/// unions. -class RecordInitListHelper { -public: - // `InitList` must have record type. - RecordInitListHelper(const InitListExpr *InitList); - - // Base classes with their associated initializer expressions. - ArrayRef> base_inits() const { -return BaseInits; - } - - // Fields with their associated initializer expressions. - ArrayRef> field_inits() const { -return FieldInits; - } - -private: - SmallVector> BaseInits; - SmallVector> FieldInits; - - // We potentially synthesize an `ImplicitValueInitExpr` for unions. It's a - // member variable because we
[clang] [clang][dataflow] Expose getReferencedDecls and relocate free functions. (PR #88754)
bazuzi wrote: @ymand @martinboehme Apologies for the broken chains of comments; can you take another look? https://github.com/llvm/llvm-project/pull/88754 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Expose getReferencedDecls and relocate free functions. (PR #88754)
https://github.com/bazuzi created https://github.com/llvm/llvm-project/pull/88754 Moves free functions from DataflowEnvironment.h/cc and DataflowAnalysisContext.h/cc to RecordOps and a new ASTOps and exposes them as needed for current use and to expose getReferencedDecls for out-of-tree use. Minimal change in functionality, only to modify the return type of getReferenceDecls to return the collected decls instead of using output params. Tested with `ninja check-clang-tooling`. This started as #88534, but I screwed up the push of changes after a first round of comments and needed to close that PR to avoid spam for recent commit authors. I believe all of the first round comments have been addressed and should be considered marked "Done" and resolved. >From 223d4c48abf27c8b0949ac1520b66ef32cbd63c1 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Fri, 12 Apr 2024 12:18:44 -0400 Subject: [PATCH 1/3] [clang][dataflow] Expose fields, globals, and functions referenced. Exposes the collection functionality, but does not alter it beyond using a return value instead of output parameters. Also relocates underlying and related functions and a class from DataflowEnvironment's files to DataflowAnalysisContext's files, as no Environment is needed. --- .../FlowSensitive/DataflowAnalysisContext.h | 46 + .../FlowSensitive/DataflowEnvironment.h | 36 .../FlowSensitive/DataflowAnalysisContext.cpp | 174 + .../FlowSensitive/DataflowEnvironment.cpp | 176 +- clang/lib/Analysis/FlowSensitive/Transfer.cpp | 1 + 5 files changed, 225 insertions(+), 208 deletions(-) diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h index 909a91059438ca..a34e5f603eb396 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h @@ -62,6 +62,52 @@ FieldSet getObjectFields(QualType Type); bool containsSameFields(const FieldSet &Fields, const RecordStorageLocation::FieldToLoc &FieldLocs); +/// Returns the fields of a `RecordDecl` that are initialized by an +/// `InitListExpr`, in the order in which they appear in +/// `InitListExpr::inits()`. +/// `Init->getType()` must be a record type. +std::vector +getFieldsForInitListExpr(const InitListExpr *InitList); + +/// Helper class for initialization of a record with an `InitListExpr`. +/// `InitListExpr::inits()` contains the initializers for both the base classes +/// and the fields of the record; this helper class separates these out into two +/// different lists. In addition, it deals with special cases associated with +/// unions. +class RecordInitListHelper { +public: + // `InitList` must have record type. + RecordInitListHelper(const InitListExpr *InitList); + + // Base classes with their associated initializer expressions. + ArrayRef> base_inits() const { +return BaseInits; + } + + // Fields with their associated initializer expressions. + ArrayRef> field_inits() const { +return FieldInits; + } + +private: + SmallVector> BaseInits; + SmallVector> FieldInits; + + // We potentially synthesize an `ImplicitValueInitExpr` for unions. It's a + // member variable because we store a pointer to it in `FieldInits`. + std::optional ImplicitValueInitForUnion; +}; + +struct FieldsGlobalsAndFuncs { + FieldSet Fields; + // Globals includes all variables with global storage, notably including + // static data members and static variables declared within a function. + llvm::DenseSet Globals; + llvm::DenseSet Funcs; +}; + +FieldsGlobalsAndFuncs getFieldsGlobalsAndFuncs(const FunctionDecl &FD); + struct ContextSensitiveOptions { /// The maximum depth to analyze. A value of zero is equivalent to disabling /// context-sensitive analysis entirely. diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h index 706664d7db1c25..4277792219c0af 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -775,42 +775,6 @@ RecordStorageLocation *getImplicitObjectLocation(const CXXMemberCallExpr &MCE, RecordStorageLocation *getBaseObjectLocation(const MemberExpr &ME, const Environment &Env); -/// Returns the fields of a `RecordDecl` that are initialized by an -/// `InitListExpr`, in the order in which they appear in -/// `InitListExpr::inits()`. -/// `Init->getType()` must be a record type. -std::vector -getFieldsForInitListExpr(const InitListExpr *InitList); - -/// Helper class for initialization of a record with an `InitListExpr`. -/// `InitListExpr::inits()` contains the initializers for both the base classes -/// and the fields of the record; this helper class sepa
[clang] [clang][dataflow] Expose fields, globals, and functions referenced. (PR #88534)
bazuzi wrote: @ymand Can you review and merge? https://github.com/llvm/llvm-project/pull/88534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Expose fields, globals, and functions referenced. (PR #88534)
https://github.com/bazuzi created https://github.com/llvm/llvm-project/pull/88534 Exposes the collection functionality, but does not alter it beyond using a return value instead of output parameters. Also relocates underlying and related functions and a class from DataflowEnvironment's files to DataflowAnalysisContext's files, as no Environment is needed. >From 869ddaf62b7db9145bc005e05d04387348654b12 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Fri, 12 Apr 2024 12:18:44 -0400 Subject: [PATCH] [clang][dataflow] Expose fields, globals, and functions referenced. Exposes the collection functionality, but does not alter it beyond using a return value instead of output parameters. Also relocates underlying and related functions and a class from DataflowEnvironment's files to DataflowAnalysisContext's files, as no Environment is needed. --- .../FlowSensitive/DataflowAnalysisContext.h | 46 + .../FlowSensitive/DataflowEnvironment.h | 36 .../FlowSensitive/DataflowAnalysisContext.cpp | 174 + .../FlowSensitive/DataflowEnvironment.cpp | 176 +- clang/lib/Analysis/FlowSensitive/Transfer.cpp | 1 + 5 files changed, 225 insertions(+), 208 deletions(-) diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h index 909a91059438ca..a34e5f603eb396 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h @@ -62,6 +62,52 @@ FieldSet getObjectFields(QualType Type); bool containsSameFields(const FieldSet &Fields, const RecordStorageLocation::FieldToLoc &FieldLocs); +/// Returns the fields of a `RecordDecl` that are initialized by an +/// `InitListExpr`, in the order in which they appear in +/// `InitListExpr::inits()`. +/// `Init->getType()` must be a record type. +std::vector +getFieldsForInitListExpr(const InitListExpr *InitList); + +/// Helper class for initialization of a record with an `InitListExpr`. +/// `InitListExpr::inits()` contains the initializers for both the base classes +/// and the fields of the record; this helper class separates these out into two +/// different lists. In addition, it deals with special cases associated with +/// unions. +class RecordInitListHelper { +public: + // `InitList` must have record type. + RecordInitListHelper(const InitListExpr *InitList); + + // Base classes with their associated initializer expressions. + ArrayRef> base_inits() const { +return BaseInits; + } + + // Fields with their associated initializer expressions. + ArrayRef> field_inits() const { +return FieldInits; + } + +private: + SmallVector> BaseInits; + SmallVector> FieldInits; + + // We potentially synthesize an `ImplicitValueInitExpr` for unions. It's a + // member variable because we store a pointer to it in `FieldInits`. + std::optional ImplicitValueInitForUnion; +}; + +struct FieldsGlobalsAndFuncs { + FieldSet Fields; + // Globals includes all variables with global storage, notably including + // static data members and static variables declared within a function. + llvm::DenseSet Globals; + llvm::DenseSet Funcs; +}; + +FieldsGlobalsAndFuncs getFieldsGlobalsAndFuncs(const FunctionDecl &FD); + struct ContextSensitiveOptions { /// The maximum depth to analyze. A value of zero is equivalent to disabling /// context-sensitive analysis entirely. diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h index 706664d7db1c25..4277792219c0af 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -775,42 +775,6 @@ RecordStorageLocation *getImplicitObjectLocation(const CXXMemberCallExpr &MCE, RecordStorageLocation *getBaseObjectLocation(const MemberExpr &ME, const Environment &Env); -/// Returns the fields of a `RecordDecl` that are initialized by an -/// `InitListExpr`, in the order in which they appear in -/// `InitListExpr::inits()`. -/// `Init->getType()` must be a record type. -std::vector -getFieldsForInitListExpr(const InitListExpr *InitList); - -/// Helper class for initialization of a record with an `InitListExpr`. -/// `InitListExpr::inits()` contains the initializers for both the base classes -/// and the fields of the record; this helper class separates these out into two -/// different lists. In addition, it deals with special cases associated with -/// unions. -class RecordInitListHelper { -public: - // `InitList` must have record type. - RecordInitListHelper(const InitListExpr *InitList); - - // Base classes with their associated initializer expressions. - ArrayRef> base_inits() const { -return BaseInits; - } - - // Fields with their ass
[clang] [clang][dataflow] Skip array types when handling InitListExprs. (PR #83013)
https://github.com/bazuzi updated https://github.com/llvm/llvm-project/pull/83013 >From ee395ff3555efa5cbeae4d874f3ad39c52b85faf Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Mon, 26 Feb 2024 10:00:48 -0500 Subject: [PATCH 1/2] [clang][dataflow] Skip array types when handling InitListExprs. Crashes resulted from single-element InitListExprs for arrays with elements of a record type after #80970. --- clang/lib/Analysis/FlowSensitive/Transfer.cpp | 6 +++--- .../Analysis/FlowSensitive/TransferTest.cpp | 17 - 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index fe13e919bddcd8..a5b8e9cbc18e64 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -671,9 +671,9 @@ class TransferVisitor : public ConstStmtVisitor { } if (!Type->isStructureOrClassType()) { - // Until array initialization is implemented, we don't need to care about - // cases where `getNumInits() > 1`. - if (S->getNumInits() == 1) + // Until array initialization is implemented, we skip arrays and don't need + // to care about cases where `getNumInits() > 1`. + if (!Type->isArrayType() && S->getNumInits() == 1) propagateValueOrStorageLocation(*S->getInit(0), *S, Env); return; } diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index a65b0446ac7818..2be899f5b6da91 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -2367,6 +2367,21 @@ TEST(TransferTest, InitListExprAsXValue) { }); } +TEST(TransferTest, ArrayInitListExprOneRecordElement) { + // This is a crash repro. + std::string Code = R"cc( +struct S {}; + +void target() { S foo[] = {S()}; } + )cc"; + runDataflow( + Code, + [](const llvm::StringMap> &Results, + ASTContext &ASTCtx) { +// Just verify that it doesn't crash. + }); +} + TEST(TransferTest, InitListExprAsUnion) { // This is a crash repro. std::string Code = R"cc( @@ -3414,7 +3429,7 @@ TEST(TransferTest, AggregateInitializationFunctionPointer) { struct S { void (*const Field)(); }; - + void target() { S s{nullptr}; } >From 00d1d53a1c52850042e1d09250833355a140ea4c Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Mon, 26 Feb 2024 10:00:48 -0500 Subject: [PATCH 2/2] [clang][dataflow] Skip array types when handling InitListExprs. Crashes resulted from single-element InitListExprs for arrays with elements of a record type after #80970. --- clang/lib/Analysis/FlowSensitive/Transfer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index a5b8e9cbc18e64..089854264f483a 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -671,8 +671,8 @@ class TransferVisitor : public ConstStmtVisitor { } if (!Type->isStructureOrClassType()) { - // Until array initialization is implemented, we skip arrays and don't need - // to care about cases where `getNumInits() > 1`. + // Until array initialization is implemented, we skip arrays and don't + // need to care about cases where `getNumInits() > 1`. if (!Type->isArrayType() && S->getNumInits() == 1) propagateValueOrStorageLocation(*S->getInit(0), *S, Env); return; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Skip array types when handling InitListExprs. (PR #83013)
https://github.com/bazuzi created https://github.com/llvm/llvm-project/pull/83013 Crashes resulted from single-element InitListExprs for arrays with elements of a record type after #80970. >From ee395ff3555efa5cbeae4d874f3ad39c52b85faf Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Mon, 26 Feb 2024 10:00:48 -0500 Subject: [PATCH] [clang][dataflow] Skip array types when handling InitListExprs. Crashes resulted from single-element InitListExprs for arrays with elements of a record type after #80970. --- clang/lib/Analysis/FlowSensitive/Transfer.cpp | 6 +++--- .../Analysis/FlowSensitive/TransferTest.cpp | 17 - 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index fe13e919bddcd8..a5b8e9cbc18e64 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -671,9 +671,9 @@ class TransferVisitor : public ConstStmtVisitor { } if (!Type->isStructureOrClassType()) { - // Until array initialization is implemented, we don't need to care about - // cases where `getNumInits() > 1`. - if (S->getNumInits() == 1) + // Until array initialization is implemented, we skip arrays and don't need + // to care about cases where `getNumInits() > 1`. + if (!Type->isArrayType() && S->getNumInits() == 1) propagateValueOrStorageLocation(*S->getInit(0), *S, Env); return; } diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index a65b0446ac7818..2be899f5b6da91 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -2367,6 +2367,21 @@ TEST(TransferTest, InitListExprAsXValue) { }); } +TEST(TransferTest, ArrayInitListExprOneRecordElement) { + // This is a crash repro. + std::string Code = R"cc( +struct S {}; + +void target() { S foo[] = {S()}; } + )cc"; + runDataflow( + Code, + [](const llvm::StringMap> &Results, + ASTContext &ASTCtx) { +// Just verify that it doesn't crash. + }); +} + TEST(TransferTest, InitListExprAsUnion) { // This is a crash repro. std::string Code = R"cc( @@ -3414,7 +3429,7 @@ TEST(TransferTest, AggregateInitializationFunctionPointer) { struct S { void (*const Field)(); }; - + void target() { S s{nullptr}; } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)
https://github.com/bazuzi edited https://github.com/llvm/llvm-project/pull/82856 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)
bazuzi wrote: @ymand Can you merge this? https://github.com/llvm/llvm-project/pull/82856 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)
https://github.com/bazuzi created https://github.com/llvm/llvm-project/pull/82856 Reverts llvm/llvm-project#82348, which caused crashes when analyzing empty InitListExprs for unions. >From ae17f89f38d56c2aa9feaa1ebfc7a641b41dc068 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Fri, 23 Feb 2024 20:12:33 -0500 Subject: [PATCH] =?UTF-8?q?Revert=20"[clang][dataflow]=20Correctly=20handl?= =?UTF-8?q?e=20`InitListExpr`=20of=20union=20type.=20(#82=E2=80=A6"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 4725993f1a812c86b9ad79d229a015d0216ff550. --- .../FlowSensitive/DataflowEnvironment.h | 9 +++ .../FlowSensitive/DataflowEnvironment.cpp | 18 +++-- clang/lib/Analysis/FlowSensitive/Transfer.cpp | 25 --- .../Analysis/FlowSensitive/TestingSupport.h | 19 -- .../Analysis/FlowSensitive/TransferTest.cpp | 14 ++- 5 files changed, 20 insertions(+), 65 deletions(-) diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h index b3dc940705f870..0aecc749bf415c 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -753,12 +753,9 @@ RecordStorageLocation *getImplicitObjectLocation(const CXXMemberCallExpr &MCE, RecordStorageLocation *getBaseObjectLocation(const MemberExpr &ME, const Environment &Env); -/// Returns the fields of a `RecordDecl` that are initialized by an -/// `InitListExpr`, in the order in which they appear in -/// `InitListExpr::inits()`. -/// `Init->getType()` must be a record type. -std::vector -getFieldsForInitListExpr(const InitListExpr *InitList); +/// Returns the fields of `RD` that are initialized by an `InitListExpr`, in the +/// order in which they appear in `InitListExpr::inits()`. +std::vector getFieldsForInitListExpr(const RecordDecl *RD); /// Associates a new `RecordValue` with `Loc` and returns the new value. RecordValue &refreshRecordValue(RecordStorageLocation &Loc, Environment &Env); diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index 0cfc26ea952cda..d487944ce92111 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -361,8 +361,8 @@ getFieldsGlobalsAndFuncs(const Stmt &S, FieldSet &Fields, if (const auto *FD = dyn_cast(VD)) Fields.insert(FD); } else if (auto *InitList = dyn_cast(&S)) { -if (InitList->getType()->isRecordType()) - for (const auto *FD : getFieldsForInitListExpr(InitList)) +if (RecordDecl *RD = InitList->getType()->getAsRecordDecl()) + for (const auto *FD : getFieldsForInitListExpr(RD)) Fields.insert(FD); } } @@ -1104,22 +1104,12 @@ RecordStorageLocation *getBaseObjectLocation(const MemberExpr &ME, return Env.get(*Base); } -std::vector -getFieldsForInitListExpr(const InitListExpr *InitList) { - const RecordDecl *RD = InitList->getType()->getAsRecordDecl(); - assert(RD != nullptr); - - std::vector Fields; - - if (InitList->getType()->isUnionType()) { -Fields.push_back(InitList->getInitializedFieldInUnion()); -return Fields; - } - +std::vector getFieldsForInitListExpr(const RecordDecl *RD) { // Unnamed bitfields are only used for padding and do not appear in // `InitListExpr`'s inits. However, those fields do appear in `RecordDecl`'s // field list, and we thus need to remove them before mapping inits to // fields to avoid mapping inits to the wrongs fields. + std::vector Fields; llvm::copy_if( RD->fields(), std::back_inserter(Fields), [](const FieldDecl *Field) { return !Field->isUnnamedBitfield(); }); diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index cd1f04e53cff68..fe13e919bddcd8 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -663,7 +663,14 @@ class TransferVisitor : public ConstStmtVisitor { void VisitInitListExpr(const InitListExpr *S) { QualType Type = S->getType(); -if (!Type->isRecordType()) { +if (Type->isUnionType()) { + // FIXME: Initialize unions properly. + if (auto *Val = Env.createValue(Type)) +Env.setValue(*S, *Val); + return; +} + +if (!Type->isStructureOrClassType()) { // Until array initialization is implemented, we don't need to care about // cases where `getNumInits() > 1`. if (S->getNumInits() == 1) @@ -681,9 +688,10 @@ class TransferVisitor : public ConstStmtVisitor { llvm::DenseMap FieldLocs; // This only contains the direct fields for the given type. -std::vector FieldsForInit = getFieldsForInitListExpr(S); +
[clang] [dataflow] Fix crash when InitListExpr is not a prvalue (PR #80970)
@@ -648,6 +648,12 @@ class TransferVisitor : public ConstStmtVisitor { QualType Type = S->getType(); if (!Type->isStructureOrClassType()) { + // It is possible that InitListExpr is not a prvalue, in which case + // `setValue` will fail. In this case, we can just let the next + // transfer function handle the value creation. + if (!S->isPRValue()) +return; + if (auto *Val = Env.createValue(Type)) Env.setValue(*S, *Val); bazuzi wrote: This actually causes crashes in the case of array initialization with one element, where the element type is a RecordType. Minimal repro: ``` struct S {}; void target() { S foo[] = {S()}; } ``` https://github.com/llvm/llvm-project/pull/80970 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Re-land: Retrieve members from accessors called usi… (PR #74336)
bazuzi wrote: @martinboehme or @ymand Could you take a look and merge? The only modification from https://github.com/llvm/llvm-project/pull/73978 is the `Env.initialize()` in the test. https://github.com/llvm/llvm-project/pull/74336 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Re-land: Retrieve members from accessors called usi… (PR #74336)
https://github.com/bazuzi created https://github.com/llvm/llvm-project/pull/74336 …ng member pointers. This initially landed with a broken test due to a mid-air collision with a new requirement for Environment initialization before field modeling. Have added that initialization in the test. >From first landing: getMethodDecl does not handle pointers to members and returns nullptr for them. getMethodDecl contains a decade-plus-old FIXME to handle pointers to members, but two approaches I looked at for fixing it are more invasive or complex than simply swapping to getCalleeDecl. The first, have getMethodDecl call getCalleeDecl, creates a large tree of const-ness mismatches due to getMethodDecl returning a non-const value while being a const member function and getCalleeDecl only being a const member function when it returns a const value. The second, implementing an AST walk to match how CXXMemberCallExpr::getImplicitObjectArgument grabs the LHS of the binary operator, is basically reimplementing Expr::getReferencedDeclOfCallee, which is used by Expr::getCalleeDecl. We don't need another copy of that code. >From 0a25a1a60247b25d199ee0fa0a45fb0547d715a9 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Mon, 4 Dec 2023 04:10:07 -0500 Subject: [PATCH] =?UTF-8?q?[clang][dataflow]=20Re-land:=20Retrieve=20membe?= =?UTF-8?q?rs=20from=20accessors=20called=20using=20member=E2=80=A6=20(#73?= =?UTF-8?q?978)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit … pointers. This initially landed with a broken test due to a mid-air collision with a new requirement for Environment initialization before field modeling. Have added that initialization in the test. >From first landing: getMethodDecl does not handle pointers to members and returns nullptr for them. getMethodDecl contains a decade-plus-old FIXME to handle pointers to members, but two approaches I looked at for fixing it are more invasive or complex than simply swapping to getCalleeDecl. The first, have getMethodDecl call getCalleeDecl, creates a large tree of const-ness mismatches due to getMethodDecl returning a non-const value while being a const member function and getCalleeDecl only being a const member function when it returns a const value. The second, implementing an AST walk to match how CXXMemberCallExpr::getImplicitObjectArgument grabs the LHS of the binary operator, is basically reimplementing Expr::getReferencedDeclOfCallee, which is used by Expr::getCalleeDecl. We don't need another copy of that code. --- .../FlowSensitive/DataflowEnvironment.cpp | 7 ++- .../FlowSensitive/DataflowEnvironmentTest.cpp | 52 +++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index 042402a129d10..b98037b736452 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -300,9 +300,12 @@ static void insertIfFunction(const Decl &D, } static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) { - if (!C.getMethodDecl()) + // Use getCalleeDecl instead of getMethodDecl in order to handle + // pointer-to-member calls. + const auto *MethodDecl = dyn_cast_or_null(C.getCalleeDecl()); + if (!MethodDecl) return nullptr; - auto *Body = dyn_cast_or_null(C.getMethodDecl()->getBody()); + auto *Body = dyn_cast_or_null(MethodDecl->getBody()); if (!Body || Body->size() != 1) return nullptr; if (auto *RS = dyn_cast(*Body->body_begin())) diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp index 3569b0eac7005..003434a58b107 100644 --- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp @@ -25,6 +25,7 @@ namespace { using namespace clang; using namespace dataflow; using ::clang::dataflow::test::getFieldValue; +using ::testing::Contains; using ::testing::IsNull; using ::testing::NotNull; @@ -311,6 +312,57 @@ TEST_F(EnvironmentTest, InitGlobalVarsConstructor) { EXPECT_THAT(Env.getValue(*Var), NotNull()); } +// Pointers to Members are a tricky case of accessor calls, complicated further +// when using templates where the pointer to the member is a template argument. +// This is a repro of a failure case seen in the wild. +TEST_F(EnvironmentTest, + ModelMemberForAccessorUsingMethodPointerThroughTemplate) { + using namespace ast_matchers; + + std::string Code = R"cc( + struct S { +int accessor() {return member;} + +int member = 0; + }; + + template + int Target(S* S) { +return (S->*method)(); + } + + // We want to analyze the instantiation of Target for the accessor. + int Instantiator () {S S; return Tar
[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)
bazuzi wrote: Can you merge for me, now that I've finally remembered to format everything? https://github.com/llvm/llvm-project/pull/73978 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)
https://github.com/bazuzi updated https://github.com/llvm/llvm-project/pull/73978 >From aab1069636896c4a8289545ba01d2fdf4f1715c0 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Thu, 30 Nov 2023 14:18:04 -0500 Subject: [PATCH 1/4] [clang][dataflow] Retrieve members from accessors called using member pointers. getMethodDecl does not handle pointers to members and returns nullptr. --- .../FlowSensitive/DataflowEnvironment.cpp | 5 +- .../FlowSensitive/DataflowEnvironmentTest.cpp | 50 +++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index 525ab188b01b8aa..5a37fa0f02f5d21 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -300,9 +300,10 @@ static void insertIfFunction(const Decl &D, } static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) { - if (!C.getMethodDecl()) + const auto *MethodDecl = dyn_cast_or_null(C.getCalleeDecl()); + if (!MethodDecl) return nullptr; - auto *Body = dyn_cast_or_null(C.getMethodDecl()->getBody()); + auto *Body = dyn_cast_or_null(MethodDecl->getBody()); if (!Body || Body->size() != 1) return nullptr; if (auto *RS = dyn_cast(*Body->body_begin())) diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp index ff6cbec5d20b744..55fb6549621737a 100644 --- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp @@ -306,6 +306,56 @@ TEST_F(EnvironmentTest, InitGlobalVarsConstructor) { EXPECT_THAT(Env.getValue(*Var), NotNull()); } +// Pointers to Members are a tricky case of accessor calls, complicated further +// when using templates where the pointer to the member is a template argument. +// This is a repro of a failure case seen in the wild. +TEST_F(EnvironmentTest, + ModelMemberForAccessorUsingMethodPointerThroughTemplate) { + using namespace ast_matchers; + + std::string Code = R"cc( + struct S { +int accessor() {return member;} + +int member = 0; + }; + + template + int Target(S* S) { +return (S->*method)(); + } + + // We want to analyze the instantiation of Target for the accessor. + int Instantiator () {S S; return Target<&S::accessor>(&S); } + )cc"; + + auto Unit = + // C++17 for the simplifying use of auto in the template declaration. + tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++17"}); + auto &Context = Unit->getASTContext(); + + ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U); + + auto Results = match( + decl(anyOf(functionDecl(hasName("Target"), isTemplateInstantiation()) + .bind("target"), + fieldDecl(hasName("member")).bind("member"), + recordDecl(hasName("S")).bind("struct"))), + Context); + const auto *Fun = selectFirst("target", Results); + const auto *Struct = selectFirst("struct", Results); + const auto *Member = selectFirst("member", Results); + ASSERT_THAT(Fun, NotNull()); + ASSERT_THAT(Struct, NotNull()); + ASSERT_THAT(Member, NotNull()); + + // Verify that `member` is modeled for `S` when we analyze + // `Target<&S::accessor>`. + Environment Env(DAContext, *Fun); + EXPECT_THAT(DAContext.getModeledFields(QualType(Struct->getTypeForDecl(), 0)), + Contains(Member)); +} + TEST_F(EnvironmentTest, RefreshRecordValue) { using namespace ast_matchers; >From 120bdd51496dd69c601181bccae072effb547920 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Fri, 1 Dec 2023 10:33:54 -0500 Subject: [PATCH 2/4] Add missing using declaration to test. And add comment. --- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp | 1 + .../unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index 5a37fa0f02f5d21..0b962bff4c183bf 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -300,6 +300,7 @@ static void insertIfFunction(const Decl &D, } static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) { + // Use getCalleeDecl instead of getMethodDecl in order to handle pointer-to-member calls. const auto *MethodDecl = dyn_cast_or_null(C.getCalleeDecl()); if (!MethodDecl) return nullptr; diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp index 55fb6549621737a..abebd362bec471f 100644 --- a/clang/unittests/Analysis/FlowSensiti
[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)
https://github.com/bazuzi updated https://github.com/llvm/llvm-project/pull/73978 >From aab1069636896c4a8289545ba01d2fdf4f1715c0 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Thu, 30 Nov 2023 14:18:04 -0500 Subject: [PATCH 1/3] [clang][dataflow] Retrieve members from accessors called using member pointers. getMethodDecl does not handle pointers to members and returns nullptr. --- .../FlowSensitive/DataflowEnvironment.cpp | 5 +- .../FlowSensitive/DataflowEnvironmentTest.cpp | 50 +++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index 525ab188b01b8aa..5a37fa0f02f5d21 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -300,9 +300,10 @@ static void insertIfFunction(const Decl &D, } static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) { - if (!C.getMethodDecl()) + const auto *MethodDecl = dyn_cast_or_null(C.getCalleeDecl()); + if (!MethodDecl) return nullptr; - auto *Body = dyn_cast_or_null(C.getMethodDecl()->getBody()); + auto *Body = dyn_cast_or_null(MethodDecl->getBody()); if (!Body || Body->size() != 1) return nullptr; if (auto *RS = dyn_cast(*Body->body_begin())) diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp index ff6cbec5d20b744..55fb6549621737a 100644 --- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp @@ -306,6 +306,56 @@ TEST_F(EnvironmentTest, InitGlobalVarsConstructor) { EXPECT_THAT(Env.getValue(*Var), NotNull()); } +// Pointers to Members are a tricky case of accessor calls, complicated further +// when using templates where the pointer to the member is a template argument. +// This is a repro of a failure case seen in the wild. +TEST_F(EnvironmentTest, + ModelMemberForAccessorUsingMethodPointerThroughTemplate) { + using namespace ast_matchers; + + std::string Code = R"cc( + struct S { +int accessor() {return member;} + +int member = 0; + }; + + template + int Target(S* S) { +return (S->*method)(); + } + + // We want to analyze the instantiation of Target for the accessor. + int Instantiator () {S S; return Target<&S::accessor>(&S); } + )cc"; + + auto Unit = + // C++17 for the simplifying use of auto in the template declaration. + tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++17"}); + auto &Context = Unit->getASTContext(); + + ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U); + + auto Results = match( + decl(anyOf(functionDecl(hasName("Target"), isTemplateInstantiation()) + .bind("target"), + fieldDecl(hasName("member")).bind("member"), + recordDecl(hasName("S")).bind("struct"))), + Context); + const auto *Fun = selectFirst("target", Results); + const auto *Struct = selectFirst("struct", Results); + const auto *Member = selectFirst("member", Results); + ASSERT_THAT(Fun, NotNull()); + ASSERT_THAT(Struct, NotNull()); + ASSERT_THAT(Member, NotNull()); + + // Verify that `member` is modeled for `S` when we analyze + // `Target<&S::accessor>`. + Environment Env(DAContext, *Fun); + EXPECT_THAT(DAContext.getModeledFields(QualType(Struct->getTypeForDecl(), 0)), + Contains(Member)); +} + TEST_F(EnvironmentTest, RefreshRecordValue) { using namespace ast_matchers; >From 120bdd51496dd69c601181bccae072effb547920 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Fri, 1 Dec 2023 10:33:54 -0500 Subject: [PATCH 2/3] Add missing using declaration to test. And add comment. --- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp | 1 + .../unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index 5a37fa0f02f5d21..0b962bff4c183bf 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -300,6 +300,7 @@ static void insertIfFunction(const Decl &D, } static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) { + // Use getCalleeDecl instead of getMethodDecl in order to handle pointer-to-member calls. const auto *MethodDecl = dyn_cast_or_null(C.getCalleeDecl()); if (!MethodDecl) return nullptr; diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp index 55fb6549621737a..abebd362bec471f 100644 --- a/clang/unittests/Analysis/FlowSensiti
[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)
https://github.com/bazuzi updated https://github.com/llvm/llvm-project/pull/73978 >From aab1069636896c4a8289545ba01d2fdf4f1715c0 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Thu, 30 Nov 2023 14:18:04 -0500 Subject: [PATCH 1/2] [clang][dataflow] Retrieve members from accessors called using member pointers. getMethodDecl does not handle pointers to members and returns nullptr. --- .../FlowSensitive/DataflowEnvironment.cpp | 5 +- .../FlowSensitive/DataflowEnvironmentTest.cpp | 50 +++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index 525ab188b01b8aa..5a37fa0f02f5d21 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -300,9 +300,10 @@ static void insertIfFunction(const Decl &D, } static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) { - if (!C.getMethodDecl()) + const auto *MethodDecl = dyn_cast_or_null(C.getCalleeDecl()); + if (!MethodDecl) return nullptr; - auto *Body = dyn_cast_or_null(C.getMethodDecl()->getBody()); + auto *Body = dyn_cast_or_null(MethodDecl->getBody()); if (!Body || Body->size() != 1) return nullptr; if (auto *RS = dyn_cast(*Body->body_begin())) diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp index ff6cbec5d20b744..55fb6549621737a 100644 --- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp @@ -306,6 +306,56 @@ TEST_F(EnvironmentTest, InitGlobalVarsConstructor) { EXPECT_THAT(Env.getValue(*Var), NotNull()); } +// Pointers to Members are a tricky case of accessor calls, complicated further +// when using templates where the pointer to the member is a template argument. +// This is a repro of a failure case seen in the wild. +TEST_F(EnvironmentTest, + ModelMemberForAccessorUsingMethodPointerThroughTemplate) { + using namespace ast_matchers; + + std::string Code = R"cc( + struct S { +int accessor() {return member;} + +int member = 0; + }; + + template + int Target(S* S) { +return (S->*method)(); + } + + // We want to analyze the instantiation of Target for the accessor. + int Instantiator () {S S; return Target<&S::accessor>(&S); } + )cc"; + + auto Unit = + // C++17 for the simplifying use of auto in the template declaration. + tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++17"}); + auto &Context = Unit->getASTContext(); + + ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U); + + auto Results = match( + decl(anyOf(functionDecl(hasName("Target"), isTemplateInstantiation()) + .bind("target"), + fieldDecl(hasName("member")).bind("member"), + recordDecl(hasName("S")).bind("struct"))), + Context); + const auto *Fun = selectFirst("target", Results); + const auto *Struct = selectFirst("struct", Results); + const auto *Member = selectFirst("member", Results); + ASSERT_THAT(Fun, NotNull()); + ASSERT_THAT(Struct, NotNull()); + ASSERT_THAT(Member, NotNull()); + + // Verify that `member` is modeled for `S` when we analyze + // `Target<&S::accessor>`. + Environment Env(DAContext, *Fun); + EXPECT_THAT(DAContext.getModeledFields(QualType(Struct->getTypeForDecl(), 0)), + Contains(Member)); +} + TEST_F(EnvironmentTest, RefreshRecordValue) { using namespace ast_matchers; >From 120bdd51496dd69c601181bccae072effb547920 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Fri, 1 Dec 2023 10:33:54 -0500 Subject: [PATCH 2/2] Add missing using declaration to test. And add comment. --- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp | 1 + .../unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index 5a37fa0f02f5d21..0b962bff4c183bf 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -300,6 +300,7 @@ static void insertIfFunction(const Decl &D, } static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) { + // Use getCalleeDecl instead of getMethodDecl in order to handle pointer-to-member calls. const auto *MethodDecl = dyn_cast_or_null(C.getCalleeDecl()); if (!MethodDecl) return nullptr; diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp index 55fb6549621737a..abebd362bec471f 100644 --- a/clang/unittests/Analysis/FlowSensiti
[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)
@@ -300,9 +300,10 @@ static void insertIfFunction(const Decl &D, } static MemberExpr *getMemberForAccessor(const CXXMemberCallExpr &C) { - if (!C.getMethodDecl()) + const auto *MethodDecl = dyn_cast_or_null(C.getCalleeDecl()); bazuzi wrote: Done https://github.com/llvm/llvm-project/pull/73978 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Retrieve members from accessors called using member… (PR #73978)
bazuzi wrote: @ymand Can you review? https://github.com/llvm/llvm-project/pull/73978 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Fix buggy assertion: Compare an unqualified type to an unqualified type. (PR #71573)
bazuzi wrote: Could you merge for me? I don't have write access. https://github.com/llvm/llvm-project/pull/71573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Fix buggy assertion: Compare an unqualified type to an unqualified type. (PR #71573)
https://github.com/bazuzi updated https://github.com/llvm/llvm-project/pull/71573 >From d6b87c3ff427d6425d2559e9731d88b89f2206c8 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Tue, 7 Nov 2023 13:44:51 -0500 Subject: [PATCH 1/2] [clang][dataflow] Compare an unqualified type to an unqualified type. Includes crash-reproducing test case. --- clang/lib/Analysis/FlowSensitive/Transfer.cpp | 6 +++--- .../Analysis/FlowSensitive/TransferTest.cpp | 20 +++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index 8b2f8ecc5027e8a..839c04c65e39e7c 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -683,11 +683,11 @@ class TransferVisitor : public ConstStmtVisitor { assert( // The types are same, or Field->getType().getCanonicalType().getUnqualifiedType() == - Init->getType().getCanonicalType() || + Init->getType().getCanonicalType().getUnqualifiedType() || // The field's type is T&, and initializer is T (Field->getType()->isReferenceType() && - Field->getType().getCanonicalType()->getPointeeType() == - Init->getType().getCanonicalType())); + Field->getType().getCanonicalType()->getPointeeType() == + Init->getType().getCanonicalType())); auto& Loc = Env.createObject(Field->getType(), Init); FieldLocs.insert({Field, &Loc}); } diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index bd9b98178b5d4e3..19136f24d666b66 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -3197,6 +3197,26 @@ TEST(TransferTest, AggregateInitialization_NotExplicitlyInitializedField) { }); } +TEST(TransferTest, AggregateInitializationFunctionPointer) { + // This is a crash repro. + // nullptr takes on the type of a const function pointer, but its type was + // asserted to be equal to the *unqualified* type of Field, which no longer + // included the const. + std::string Code = R"( +struct S { + void (*const Field)(); +}; + +void target() { + S s{nullptr}; +} + )"; + runDataflow( + Code, + [](const llvm::StringMap> &Results, + ASTContext &ASTCtx) {}); +} + TEST(TransferTest, AssignToUnionMember) { std::string Code = R"( union A { >From 212eb3faf63525f87c11f229b03141b66b0f Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Wed, 8 Nov 2023 09:54:34 -0500 Subject: [PATCH 2/2] Update test comment. Co-authored-by: martinboehme --- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index 19136f24d666b66..ade0d202ced2f37 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -3198,7 +3198,7 @@ TEST(TransferTest, AggregateInitialization_NotExplicitlyInitializedField) { } TEST(TransferTest, AggregateInitializationFunctionPointer) { - // This is a crash repro. + // This is a repro for an assertion failure. // nullptr takes on the type of a const function pointer, but its type was // asserted to be equal to the *unqualified* type of Field, which no longer // included the const. ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Compare an unqualified type to an unqualified type. (PR #71573)
bazuzi wrote: @martinboehme Could you review? https://github.com/llvm/llvm-project/pull/71573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Compare an unqualified type to an unqualified type. (PR #71573)
https://github.com/bazuzi created https://github.com/llvm/llvm-project/pull/71573 Includes crash-reproducing test case. >From d6b87c3ff427d6425d2559e9731d88b89f2206c8 Mon Sep 17 00:00:00 2001 From: Samira Bazuzi Date: Tue, 7 Nov 2023 13:44:51 -0500 Subject: [PATCH] [clang][dataflow] Compare an unqualified type to an unqualified type. Includes crash-reproducing test case. --- clang/lib/Analysis/FlowSensitive/Transfer.cpp | 6 +++--- .../Analysis/FlowSensitive/TransferTest.cpp | 20 +++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index 8b2f8ecc5027e8a..839c04c65e39e7c 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -683,11 +683,11 @@ class TransferVisitor : public ConstStmtVisitor { assert( // The types are same, or Field->getType().getCanonicalType().getUnqualifiedType() == - Init->getType().getCanonicalType() || + Init->getType().getCanonicalType().getUnqualifiedType() || // The field's type is T&, and initializer is T (Field->getType()->isReferenceType() && - Field->getType().getCanonicalType()->getPointeeType() == - Init->getType().getCanonicalType())); + Field->getType().getCanonicalType()->getPointeeType() == + Init->getType().getCanonicalType())); auto& Loc = Env.createObject(Field->getType(), Init); FieldLocs.insert({Field, &Loc}); } diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index bd9b98178b5d4e3..19136f24d666b66 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -3197,6 +3197,26 @@ TEST(TransferTest, AggregateInitialization_NotExplicitlyInitializedField) { }); } +TEST(TransferTest, AggregateInitializationFunctionPointer) { + // This is a crash repro. + // nullptr takes on the type of a const function pointer, but its type was + // asserted to be equal to the *unqualified* type of Field, which no longer + // included the const. + std::string Code = R"( +struct S { + void (*const Field)(); +}; + +void target() { + S s{nullptr}; +} + )"; + runDataflow( + Code, + [](const llvm::StringMap> &Results, + ASTContext &ASTCtx) {}); +} + TEST(TransferTest, AssignToUnionMember) { std::string Code = R"( union A { ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits