[Mlir-commits] [mlir] [mlir] Document and assert Liveness coverage preconditions (PR #208142)

[Jacky Li]

https://github.com/JPL11 created https://github.com/llvm/llvm-project/pull/208142

`Liveness::getLiveIn`, `getLiveOut`, `isDeadAfter` and `resolveLiveness` dereference `getLiveness(block)` without a null check, so querying a block that was created after the analysis was constructed (e.g. by splitting a critical edge, or by signature conversion during dialect conversion) crashes with a plain nullptr dereference. I hit this twice downstream in IREE while replacing its own liveness implementation with `mlir::Liveness` (iree-org/iree#24687); in both cases the symptom was a segfault deep inside the query with no hint that the analysis was stale.

This PR:

- documents the precondition on the class and the affected methods: queries expect blocks that were covered by the analysis at construction time, and `getLiveness` (which returns nullptr for unknown blocks) can be used to check coverage;
- turns the nullptr dereferences into assertions with an actionable message;
- adds a unit test covering `getLiveness`'s nullptr contract for blocks created after construction;
- fixes an `isDeafAfter` typo in the class doc comment.

I deliberately did not make the queries gracefully return empty/conservative answers for unknown blocks: that is a semantic change that can silently hide stale-analysis bugs (IREE's old implementation did exactly that and it masked a real bug for years). If there's appetite for recomputing on demand instead, happy to discuss on the issue.

Fixes #208140

Per the LLVM AI tools policy: I used Claude Code to help draft this patch; I reviewed and tested everything myself (`check-mlir` and the new `MLIRAnalysisTests` locally).


>From f03e9be7ecfaf8800acc79eda3fd2c25842522cf Mon Sep 17 00:00:00 2001
From: Jacky Li <jackydli95 at gmail.com>
Date: Tue, 7 Jul 2026 21:53:07 -0700
Subject: [PATCH] [mlir] Document and assert Liveness coverage preconditions

Liveness::getLiveIn, getLiveOut, isDeadAfter and resolveLiveness
dereference getLiveness(block) without a null check, so querying a
block that was created after the analysis was constructed (e.g. by
splitting a critical edge, or by signature conversion during dialect
conversion) crashes with a plain nullptr dereference.

Document the precondition on the class and the affected methods, turn
the crashes into assertions with an actionable message, and add a unit
test for getLiveness's nullptr contract. Also fix an isDeafAfter typo
in the class doc comment.

Fixes #208140

Assisted-by: Claude Code
---
 mlir/include/mlir/Analysis/Liveness.h    | 17 +++++-
 mlir/lib/Analysis/Liveness.cpp           | 16 +++--
 mlir/unittests/Analysis/CMakeLists.txt   |  9 +++
 mlir/unittests/Analysis/LivenessTest.cpp | 74 ++++++++++++++++++++++++
 4 files changed, 110 insertions(+), 6 deletions(-)
 create mode 100644 mlir/unittests/Analysis/LivenessTest.cpp

diff --git a/mlir/include/mlir/Analysis/Liveness.h b/mlir/include/mlir/Analysis/Liveness.h
index ca4d8dfab3c91..c56762586251a 100644
--- a/mlir/include/mlir/Analysis/Liveness.h
+++ b/mlir/include/mlir/Analysis/Liveness.h
@@ -43,7 +43,13 @@ class Value;
 ///   auto &allInValues = liveness.getLiveIn(block);
 ///   auto &allOutValues = liveness.getLiveOut(block);
 ///   auto allOperationsInWhichValueIsLive = liveness.resolveLiveness(value);
-///   bool isDeafAfter = liveness.isDeadAfter(value, operation);
+///   bool isDeadAfter = liveness.isDeadAfter(value, operation);
+///
+/// The analysis is computed once at construction time. All queries expect
+/// blocks and operations that were part of the analyzed operation when the
+/// analysis was constructed; querying blocks created afterwards (e.g. by a
+/// rewrite) is not supported. `getLiveness` returns nullptr for such blocks
+/// and can be used to check whether a block is covered.
 class Liveness {
 public:
   using OperationListT = std::vector<Operation *>;
@@ -63,18 +69,25 @@ class Liveness {
   /// Note that the operations in this list are not ordered and the current
   /// implementation is computationally expensive (as it iterates over all
   /// blocks in which the given value is live).
+  /// All blocks in which the value is live must be covered by the analysis
+  /// (see `getLiveness`).
   OperationListT resolveLiveness(Value value) const;
 
-  /// Gets liveness info (if any) for the block.
+  /// Gets liveness info (if any) for the block. Returns nullptr if the block
+  /// was not covered by the analysis at construction time.
   const LivenessBlockInfo *getLiveness(Block *block) const;
 
   /// Returns a reference to a set containing live-in values (unordered).
+  /// The block must be covered by the analysis (see `getLiveness`).
   const ValueSetT &getLiveIn(Block *block) const;
 
   /// Returns a reference to a set containing live-out values (unordered).
+  /// The block must be covered by the analysis (see `getLiveness`).
   const ValueSetT &getLiveOut(Block *block) const;
 
   /// Returns true if `value` is not live after `operation`.
+  /// The operation's block must be covered by the analysis (see
+  /// `getLiveness`).
   bool isDeadAfter(Value value, Operation *operation) const;
 
   /// Dumps the liveness information in a human readable format.
diff --git a/mlir/lib/Analysis/Liveness.cpp b/mlir/lib/Analysis/Liveness.cpp
index 3fcc0b5a6ffef..e784b97bcaf84 100644
--- a/mlir/lib/Analysis/Liveness.cpp
+++ b/mlir/lib/Analysis/Liveness.cpp
@@ -200,6 +200,7 @@ Liveness::OperationListT Liveness::resolveLiveness(Value value) const {
     // Get block and block liveness information.
     Block *block = toProcess.pop_back_val();
     const LivenessBlockInfo *blockInfo = getLiveness(block);
+    assert(blockInfo && "expected block to be covered by the analysis");
 
     // Note that start and end will be in the same block.
     Operation *start = blockInfo->getStartOperation(value);
@@ -212,8 +213,10 @@ Liveness::OperationListT Liveness::resolveLiveness(Value value) const {
     }
 
     for (Block *successor : block->getSuccessors()) {
-      if (getLiveness(successor)->isLiveIn(value) &&
-          visited.insert(successor).second)
+      const LivenessBlockInfo *successorInfo = getLiveness(successor);
+      assert(successorInfo &&
+             "expected successor to be covered by the analysis");
+      if (successorInfo->isLiveIn(value) && visited.insert(successor).second)
         toProcess.push_back(successor);
     }
   }
@@ -229,18 +232,23 @@ const LivenessBlockInfo *Liveness::getLiveness(Block *block) const {
 
 /// Returns a reference to a set containing live-in values.
 const Liveness::ValueSetT &Liveness::getLiveIn(Block *block) const {
-  return getLiveness(block)->in();
+  const LivenessBlockInfo *blockInfo = getLiveness(block);
+  assert(blockInfo && "expected block to be covered by the analysis");
+  return blockInfo->in();
 }
 
 /// Returns a reference to a set containing live-out values.
 const Liveness::ValueSetT &Liveness::getLiveOut(Block *block) const {
-  return getLiveness(block)->out();
+  const LivenessBlockInfo *blockInfo = getLiveness(block);
+  assert(blockInfo && "expected block to be covered by the analysis");
+  return blockInfo->out();
 }
 
 /// Returns true if `value` is not live after `operation`.
 bool Liveness::isDeadAfter(Value value, Operation *operation) const {
   Block *block = operation->getBlock();
   const LivenessBlockInfo *blockInfo = getLiveness(block);
+  assert(blockInfo && "expected block to be covered by the analysis");
 
   // The given value escapes the associated block.
   if (blockInfo->isLiveOut(value))
diff --git a/mlir/unittests/Analysis/CMakeLists.txt b/mlir/unittests/Analysis/CMakeLists.txt
index a05d09f89070c..2af936fdc855a 100644
--- a/mlir/unittests/Analysis/CMakeLists.txt
+++ b/mlir/unittests/Analysis/CMakeLists.txt
@@ -1 +1,10 @@
+add_mlir_unittest(MLIRAnalysisTests
+  LivenessTest.cpp
+)
+
+mlir_target_link_libraries(MLIRAnalysisTests
+  PRIVATE MLIRAnalysis
+  MLIRParser
+  )
+
 add_subdirectory(Presburger)
diff --git a/mlir/unittests/Analysis/LivenessTest.cpp b/mlir/unittests/Analysis/LivenessTest.cpp
new file mode 100644
index 0000000000000..075252cb2d961
--- /dev/null
+++ b/mlir/unittests/Analysis/LivenessTest.cpp
@@ -0,0 +1,74 @@
+//===- LivenessTest.cpp - Liveness analysis unit tests --------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "mlir/Analysis/Liveness.h"
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/Parser/Parser.h"
+
+#include "gtest/gtest.h"
+
+using namespace mlir;
+
+namespace {
+
+const StringLiteral moduleStr = R"mlir(
+"test.func"() ({
+^bb0:
+  %0 = "test.def"() : () -> i32
+  "test.br"()[^bb1] : () -> ()
+^bb1:
+  "test.use"(%0) : (i32) -> ()
+  "test.ret"() : () -> ()
+}) : () -> ()
+)mlir";
+
+TEST(LivenessTest, CoveredBlocks) {
+  MLIRContext context;
+  context.allowUnregisteredDialects();
+  OwningOpRef<ModuleOp> module =
+      parseSourceString<ModuleOp>(moduleStr, &context);
+  ASSERT_TRUE(module);
+
+  Region &region = module->getBody()->getOperations().front().getRegion(0);
+  Block &entryBlock = region.front();
+  Block &secondBlock = region.back();
+
+  Liveness liveness(module.get());
+
+  // Blocks that existed when the analysis was constructed are covered.
+  EXPECT_NE(liveness.getLiveness(&entryBlock), nullptr);
+  EXPECT_NE(liveness.getLiveness(&secondBlock), nullptr);
+
+  Value def = entryBlock.front().getResult(0);
+  EXPECT_TRUE(liveness.getLiveOut(&entryBlock).contains(def));
+  EXPECT_TRUE(liveness.getLiveIn(&secondBlock).contains(def));
+  EXPECT_FALSE(liveness.isDeadAfter(def, &entryBlock.front()));
+  EXPECT_TRUE(liveness.isDeadAfter(def, &secondBlock.front()));
+  EXPECT_EQ(liveness.resolveLiveness(def).size(), 3u);
+}
+
+TEST(LivenessTest, BlockCreatedAfterConstructionIsNotCovered) {
+  MLIRContext context;
+  context.allowUnregisteredDialects();
+  OwningOpRef<ModuleOp> module =
+      parseSourceString<ModuleOp>(moduleStr, &context);
+  ASSERT_TRUE(module);
+
+  Region &region = module->getBody()->getOperations().front().getRegion(0);
+  Block &secondBlock = region.back();
+
+  Liveness liveness(module.get());
+
+  // Blocks created after the analysis was constructed (e.g. by a rewrite)
+  // are not covered; `getLiveness` returns nullptr for them.
+  Block *newBlock = secondBlock.splitBlock(&secondBlock.front());
+  EXPECT_EQ(liveness.getLiveness(newBlock), nullptr);
+  EXPECT_NE(liveness.getLiveness(&secondBlock), nullptr);
+}
+
+} // namespace