[clang] 5520c58 - [clang][dataflow] Fix incorrect CXXThisExpr pointee for lambdas

[Eric Li via cfe-commits]

Author: Eric Li
Date: 2022-05-25T20:58:02Z
New Revision: 5520c5839016d1d98d8e01789eb17c910381bd6f

URL: https://github.com/llvm/llvm-project/commit/5520c5839016d1d98d8e01789eb17c910381bd6f
DIFF: https://github.com/llvm/llvm-project/commit/5520c5839016d1d98d8e01789eb17c910381bd6f.diff

LOG: [clang][dataflow] Fix incorrect CXXThisExpr pointee for lambdas

When constructing the `Environment`, the `this` pointee is established
for a `CXXMethodDecl` by looking at its parent. However, inside of
lambdas, a `CXXThisExpr` refers to the captured `this` coming from the
enclosing member function.

When establishing the `this` pointee for a function, we check whether
the function is a lambda, and check for an enclosing member function
to establish the `this` pointee storage location.

Differential Revision: https://reviews.llvm.org/D126413

Added: 
    

Modified: 
    clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
    clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index e555bee0e8bf..fe573bbf9f37 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -216,7 +216,12 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
   }
 
   if (const auto *MethodDecl = dyn_cast<CXXMethodDecl>(&DeclCtx)) {
-    if (!MethodDecl->isStatic()) {
+    auto *Parent = MethodDecl->getParent();
+    assert(Parent != nullptr);
+    if (Parent->isLambda())
+      MethodDecl = dyn_cast<CXXMethodDecl>(Parent->getDeclContext());
+
+    if (MethodDecl && !MethodDecl->isStatic()) {
       QualType ThisPointeeType = MethodDecl->getThisObjectType();
       // FIXME: Add support for union types.
       if (!ThisPointeeType->isUnionType()) {

diff  --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index bb6e269969da..bed6b975974c 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1476,6 +1476,112 @@ TEST_F(TransferTest, ClassThisMember) {
       });
 }
 
+TEST_F(TransferTest, StructThisInLambda) {
+  std::string ThisCaptureCode = R"(
+    struct A {
+      void frob() {
+        [this]() {
+          int Foo = Bar;
+          // [[p1]]
+        }();
+      }
+
+      int Bar;
+    };
+  )";
+  runDataflow(
+      ThisCaptureCode,
+      [](llvm::ArrayRef<
+             std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+             Results,
+         ASTContext &ASTCtx) {
+        ASSERT_THAT(Results, ElementsAre(Pair("p1", _)));
+        const Environment &Env = Results[0].second.Env;
+
+        const auto *ThisLoc = dyn_cast<AggregateStorageLocation>(
+            Env.getThisPointeeStorageLocation());
+        ASSERT_THAT(ThisLoc, NotNull());
+
+        const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+        ASSERT_THAT(BarDecl, NotNull());
+
+        const auto *BarLoc =
+            cast<ScalarStorageLocation>(&ThisLoc->getChild(*BarDecl));
+        ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(BarLoc));
+
+        const Value *BarVal = Env.getValue(*BarLoc);
+        ASSERT_TRUE(isa_and_nonnull<IntegerValue>(BarVal));
+
+        const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+        ASSERT_THAT(FooDecl, NotNull());
+        EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None), BarVal);
+      },
+      LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator()");
+
+  std::string RefCaptureDefaultCode = R"(
+    struct A {
+      void frob() {
+        [&]() {
+          int Foo = Bar;
+          // [[p2]]
+        }();
+      }
+
+      int Bar;
+    };
+  )";
+  runDataflow(
+      RefCaptureDefaultCode,
+      [](llvm::ArrayRef<
+             std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+             Results,
+         ASTContext &ASTCtx) {
+        ASSERT_THAT(Results, ElementsAre(Pair("p2", _)));
+        const Environment &Env = Results[0].second.Env;
+
+        const auto *ThisLoc = dyn_cast<AggregateStorageLocation>(
+            Env.getThisPointeeStorageLocation());
+        ASSERT_THAT(ThisLoc, NotNull());
+
+        const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+        ASSERT_THAT(BarDecl, NotNull());
+
+        const auto *BarLoc =
+            cast<ScalarStorageLocation>(&ThisLoc->getChild(*BarDecl));
+        ASSERT_TRUE(isa_and_nonnull<ScalarStorageLocation>(BarLoc));
+
+        const Value *BarVal = Env.getValue(*BarLoc);
+        ASSERT_TRUE(isa_and_nonnull<IntegerValue>(BarVal));
+
+        const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+        ASSERT_THAT(FooDecl, NotNull());
+        EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None), BarVal);
+      },
+      LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator()");
+
+  std::string FreeFunctionLambdaCode = R"(
+    void foo() {
+      int Bar;
+      [&]() {
+        int Foo = Bar;
+        // [[p3]]
+      }();
+    }
+  )";
+  runDataflow(
+      FreeFunctionLambdaCode,
+      [](llvm::ArrayRef<
+             std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+             Results,
+         ASTContext &ASTCtx) {
+        ASSERT_THAT(Results, ElementsAre(Pair("p3", _)));
+        const Environment &Env = Results[0].second.Env;
+
+        EXPECT_THAT(Env.getThisPointeeStorageLocation(), IsNull());
+      },
+      LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator()");
+}
+
 TEST_F(TransferTest, ConstructorInitializer) {
   std::string Code = R"(
     struct target {