[clang] [clang][dataflow] Don't crash when BlockToState is called from unreachable path (PR #65732)

[Kinuko Yasuda via cfe-commits]

https://github.com/kinu created https://github.com/llvm/llvm-project/pull/65732:

When we call `getEnvironment`, `BlockToState[BlockId]` for the block can return null even if CFCtx.isBlockReachable(B) returns true if it is called from a particular block that is marked unreachable to the block.

>From 1f805ef6a1c5697299eab149a9c0552e9195c259 Mon Sep 17 00:00:00 2001
From: Kinuko Yasuda <kinuko at chromium.org>
Date: Fri, 8 Sep 2023 09:03:35 +0000
Subject: [PATCH] [clang][dataflow] Don't crash when BlockToState doesn't have
 unreached block

When we call `getEnvironment`, `BlockToState[BlockId]` for the block can return
null even if CFCtx.isBlockReachable(B) returns true if it is called from a
particular block that is marked unreachable to the block.
(An example is in `EvaluateBlockWithUnreachablePreds` in TransferTest.cpp)
---
 clang/lib/Analysis/FlowSensitive/Transfer.cpp | 15 +++++++++++++-
 .../Analysis/FlowSensitive/TransferTest.cpp   | 20 +++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 67d8be392ae6053..b46c947c691b9b9 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -43,7 +43,20 @@ const Environment *StmtToEnvMap::getEnvironment(const Stmt &S) const {
   if (!CFCtx.isBlockReachable(*BlockIt->getSecond()))
     return nullptr;
   const auto &State = BlockToState[BlockIt->getSecond()->getBlockID()];
-  assert(State);
+  if (!(State)) {
+    LLVM_DEBUG({
+      // State can be null when this block is unreachable from the block that
+      // called this method.
+      bool hasUnreachableEdgeFromPred = false;
+      for (auto B : BlockIt->getSecond()->preds())
+        if (!B) {
+          hasUnreachableEdgeFromPred = true;
+          break;
+        }
+      assert(hasUnreachableEdgeFromPred);
+    });
+    return nullptr;
+  }
   return &State->Env;
 }
 
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 177970ac52a67eb..1fa800044c288d4 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -5853,4 +5853,24 @@ TEST(TransferTest, AnonymousStructWithReferenceField) {
       });
 }
 
+TEST(TransferTest, EvaluateBlockWithUnreachablePreds) {
+  // This is a crash repro.
+  // `false` block may not have been processed when we try to evalute the `||`
+  // after visiting `true`, because it is not necessary (and therefore the edge
+  // is marked unreachable). Trying to get the analysis state via
+  // `getEnvironment` for the subexpression should still not crash.
+  std::string Code = R"(
+    int cast(int i) {
+      if ((i < 0 && true) || false) {
+        return 0;
+      }
+      return 0;
+    }
+  )";
+  runDataflow(
+      Code,
+      [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+         ASTContext &ASTCtx) {});
+}
+
 } // namespace