[clang] 9b0651f - [clang][dataflow] Don't propagate result objects in nested declarations. (#89903)

[via cfe-commits]

Author: martinboehme
Date: 2024-04-25T09:22:14+02:00
New Revision: 9b0651f5ae6510577302ea527b2cc79e80ec9ccc

URL: https://github.com/llvm/llvm-project/commit/9b0651f5ae6510577302ea527b2cc79e80ec9ccc
DIFF: https://github.com/llvm/llvm-project/commit/9b0651f5ae6510577302ea527b2cc79e80ec9ccc.diff

LOG: [clang][dataflow] Don't propagate result objects in nested declarations. (#89903)

Trying to do so can cause crashes -- see newly added test and the
comments in
the fix.

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 3cb656adcbdc0c..636d2302093983 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -333,6 +333,18 @@ class ResultObjectVisitor : public RecursiveASTVisitor<ResultObjectVisitor> {
     }
   }
 
+  bool TraverseDecl(Decl *D) {
+    // Don't traverse nested record or function declarations.
+    // - We won't be analyzing code contained in these anyway
+    // - We don't model fields that are used only in these nested declaration,
+    //   so trying to propagate a result object to initializers of such fields
+    //   would cause an error.
+    if (isa_and_nonnull<RecordDecl>(D) || isa_and_nonnull<FunctionDecl>(D))
+      return true;
+
+    return RecursiveASTVisitor<ResultObjectVisitor>::TraverseDecl(D);
+  }
+
   bool TraverseBindingDecl(BindingDecl *BD) {
     // `RecursiveASTVisitor` doesn't traverse holding variables for
     // `BindingDecl`s by itself, so we need to tell it to.

diff  --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 215e208615ac23..5c7c39e52612ec 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -3309,6 +3309,28 @@ TEST(TransferTest, ResultObjectLocationPropagatesThroughConditionalOperator) {
       });
 }
 
+TEST(TransferTest, ResultObjectLocationDontVisitNestedRecordDecl) {
+  // This is a crash repro.
+  // We used to crash because when propagating result objects, we would visit
+  // nested record and function declarations, but we don't model fields used
+  // only in these.
+  std::string Code = R"(
+    struct S1 {};
+    struct S2 { S1 s1; };
+    void target() {
+      struct Nested {
+        void f() {
+          S2 s2 = { S1() };
+        }
+      };
+    }
+  )";
+  runDataflow(
+      Code,
+      [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+         ASTContext &ASTCtx) {});
+}
+
 TEST(TransferTest, StaticCast) {
   std::string Code = R"(
     void target(int Foo) {