[clang] 33b598a - [clang][dataflow] Relax assert on existence of `this` pointee storage

[Eric Li via cfe-commits]

Author: Eric Li
Date: 2022-05-25T20:58:02Z
New Revision: 33b598a808b9ef1a019e798f19855f203e09ad48

URL: https://github.com/llvm/llvm-project/commit/33b598a808b9ef1a019e798f19855f203e09ad48
DIFF: https://github.com/llvm/llvm-project/commit/33b598a808b9ef1a019e798f19855f203e09ad48.diff

LOG: [clang][dataflow] Relax assert on existence of `this` pointee storage

Support for unions is incomplete (per 99f7d55e) and the `this` pointee
storage location is not set for unions. The assert in
`VisitCXXThisExpr` is then guaranteed to trigger when analyzing member
functions of a union.

This commit changes the assert to an early-return. Any expression may
be undefined, and so having a value for the `CXXThisExpr` is not a
postcondition of the transfer function.

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index f88406595d728..fc04e0b55ffc7 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -279,7 +279,10 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
 
   void VisitCXXThisExpr(const CXXThisExpr *S) {
     auto *ThisPointeeLoc = Env.getThisPointeeStorageLocation();
-    assert(ThisPointeeLoc != nullptr);
+    if (ThisPointeeLoc == nullptr)
+      // Unions are not supported yet, and will not have a location for the
+      // `this` expression's pointee.
+      return;
 
     auto &Loc = Env.createStorageLocation(*S);
     Env.setStorageLocation(*S, Loc);

diff  --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index b6d2940a98daa..bb6e269969da7 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -42,10 +42,11 @@ class TransferTest : public ::testing::Test {
   template <typename Matcher>
   void runDataflow(llvm::StringRef Code, Matcher Match,
                    LangStandard::Kind Std = LangStandard::lang_cxx17,
-                   bool ApplyBuiltinTransfer = true) {
+                   bool ApplyBuiltinTransfer = true,
+                   llvm::StringRef TargetFun = "target") {
     ASSERT_THAT_ERROR(
         test::checkDataflow<NoopAnalysis>(
-            Code, "target",
+            Code, TargetFun,
             [ApplyBuiltinTransfer](ASTContext &C, Environment &) {
               return NoopAnalysis(C, ApplyBuiltinTransfer);
             },
@@ -3175,4 +3176,27 @@ TEST_F(TransferTest, LoopWithStructReferenceAssignmentConverges) {
 });
 }
 
+TEST_F(TransferTest, DoesNotCrashOnUnionThisExpr) {
+  std::string Code = R"(
+    union Union {
+      int A;
+      float B;
+    };
+
+    void foo() {
+      Union A;
+      Union B;
+      A = B;
+    }
+  )";
+  // This is a crash regression test when calling the transfer function on a
+  // `CXXThisExpr` that refers to a union.
+  runDataflow(
+      Code,
+      [](llvm::ArrayRef<
+             std::pair<std::string, DataflowAnalysisState<NoopLattice>>>,
+         ASTContext &) {},
+      LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator=");
+}
+
 } // namespace