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

[via cfe-commits]

================
@@ -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);
----------------
martinboehme wrote:

Post-submit comment on this: We're now going to check this assertion only if the `-debug` command line flag is passed (because that's required to run the contents of an `LLVM_DEBUG` block) whereas previously we would check the assertion unless `NDEBUG` is defined (because that's what governs `assert()`).

The bottom line is: We used to check this assertion when we run tests, but now we don't, because tests don't set the `-debug` command line flag by default. This seems like it will make it really likely that we will start violating this assertion in tests without knowing.

Is this check really too costly to do it whenever `NDEBUG` is not defined? IOW, couldn't we turn the `LLVM_DEBUG` into an `#ifndef NDEBUG`?

Raising this as a point for discussion, but no need to make a change here -- I expect to be touching this method anyway as part of a different patch.

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