[clang] [clang][dataflow] Remove buggy assertion. (PR #67311)

via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 25 04:05:29 PDT 2023


https://github.com/martinboehme created https://github.com/llvm/llvm-project/pull/67311

The assertion fails on the test TransferTest.EvaluateBlockWithUnreachablePreds
(which I think, ironically, was introuced in the same patch as the assertion).

This just wasn't obvious because the assertion is inside an `LLVM_DEBUG` block
and is thus only executed if the command-line flag `-debug` is passed. We don't
have any CI builds that do this, so it's almost guaranteed that assertions like
this will start failing over time (if they ever passed in the first place --
which I'm not sure about here).

It's not clear to me whether there's _some_ assertion we might be able to make
here -- I've looked at this for a while but haven't been able to come up with
anything obvious. For the time being, I think it's best to simply delete the
assertion.


>From f51c74eb8ec4f84f388dd25b8de724118a49f13c Mon Sep 17 00:00:00 2001
From: Martin Braenne <mboehme at google.com>
Date: Mon, 25 Sep 2023 11:04:53 +0000
Subject: [PATCH] [clang][dataflow] Remove buggy assertion.

The assertion fails on the test TransferTest.EvaluateBlockWithUnreachablePreds
(which I think, ironically, was introuced in the same patch as the assertion).

This just wasn't obvious because the assertion is inside an `LLVM_DEBUG` block
and is thus only executed if the command-line flag `-debug` is passed. We don't
have any CI builds that do this, so it's almost guaranteed that assertions like
this will start failing over time (if they ever passed in the first place --
which I'm not sure about here).

It's not clear to me whether there's _some_ assertion we might be able to make
here -- I've looked at this for a while but haven't been able to come up with
anything obvious. For the time being, I think it's best to simply delete the
assertion.
---
 clang/lib/Analysis/FlowSensitive/Transfer.cpp | 14 +-------------
 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 2414a1cc026af5f..2e4fa1ae9600f29 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -43,20 +43,8 @@ const Environment *StmtToEnvMap::getEnvironment(const Stmt &S) const {
   if (!CFCtx.isBlockReachable(*BlockIt->getSecond()))
     return nullptr;
   const auto &State = BlockToState[BlockIt->getSecond()->getBlockID()];
-  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);
-    });
+  if (!(State))
     return nullptr;
-  }
   return &State->Env;
 }
 



More information about the cfe-commits mailing list