[Mlir-commits] [mlir] [mlir][BytecodeReader] Fix crash reading FusedLoc with empty locations (PR #189228)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-ods

Author: Mehdi Amini (joker-eph)

<details>
<summary>Changes</summary>

FusedLoc::get(context, locs) may return UnknownLoc when locs is empty and no metadata is provided. The bytecode reader's cBuilder used cast<FusedLoc>() on this result, which crashes with an assertion failure.

Fix by giving the FusedLoc DialectAttribute its own cBuilder that passes Attribute() explicitly, causing getChecked<FusedLoc> to call the two-parameter storage constructor directly and always produce a FusedLoc.

Fixes #<!-- -->99626

Assisted-by: Claude Code

---
Full diff: https://github.com/llvm/llvm-project/pull/189228.diff


2 Files Affected:

- (modified) mlir/include/mlir/IR/BuiltinDialectBytecode.td (+6-2) 
- (modified) mlir/unittests/Bytecode/BytecodeTest.cpp (+26) 


``````````diff
diff --git a/mlir/include/mlir/IR/BuiltinDialectBytecode.td b/mlir/include/mlir/IR/BuiltinDialectBytecode.td
index 8a1f3d5e5b2e0..c97d093c84e51 100644
--- a/mlir/include/mlir/IR/BuiltinDialectBytecode.td
+++ b/mlir/include/mlir/IR/BuiltinDialectBytecode.td
@@ -116,12 +116,15 @@ def FileLineColLoc : DialectAttribute<(attr
 }
 }
 
-let cType = "FusedLoc",
-    cBuilder = "cast<FusedLoc>(getChecked<FusedLoc>([&]() { return reader.emitError(); }, context, $_args))" in {
+let cType = "FusedLoc" in {
 def FusedLoc : DialectAttribute<(attr
   Array<Location>:$locations
 )> {
   let printerPredicate = "!$_val.getMetadata()";
+  // FusedLoc::get(context, locs) may return UnknownLoc when locs is empty.
+  // Pass Attribute() explicitly to bypass the folding logic and always
+  // construct a FusedLoc via Base::getChecked.
+  let cBuilder = "cast<FusedLoc>(getChecked<FusedLoc>([&]() { return reader.emitError(); }, context, $_args, Attribute()))";
 }
 
 def FusedLocWithMetadata : DialectAttribute<(attr
@@ -129,6 +132,7 @@ def FusedLocWithMetadata : DialectAttribute<(attr
   Attribute:$metadata
 )> {
   let printerPredicate = "$_val.getMetadata()";
+  let cBuilder = "cast<FusedLoc>(getChecked<FusedLoc>([&]() { return reader.emitError(); }, context, $_args))";
 }
 }
 
diff --git a/mlir/unittests/Bytecode/BytecodeTest.cpp b/mlir/unittests/Bytecode/BytecodeTest.cpp
index 148868801da33..51eebc488d7f8 100644
--- a/mlir/unittests/Bytecode/BytecodeTest.cpp
+++ b/mlir/unittests/Bytecode/BytecodeTest.cpp
@@ -266,3 +266,29 @@ TEST(Bytecode, DeepCallSiteLoc) {
   // Verify the location matches.
   EXPECT_EQ(module.get()->getLoc(), roundTripped->getLoc());
 }
+
+// Regression test for https://github.com/llvm/llvm-project/issues/99626.
+// FusedLoc::get(context, locs) returns UnknownLoc when locs is empty, so the
+// bytecode reader must not use cast<FusedLoc>() on that result.
+TEST(Bytecode, EmptyFusedLocRoundtrip) {
+  MLIRContext context;
+
+  // FusedLoc with empty locations (no metadata). FusedLoc::get returns
+  // UnknownLoc in this case, but the bytecode writer stores a FusedLoc.
+  FusedLoc fusedLoc = FusedLoc::get(&context, /*locs=*/{}, /*metadata=*/{});
+  auto module = mlir::ModuleOp::create(fusedLoc, "test");
+
+  // Write the module to bytecode.
+  std::string buffer;
+  llvm::raw_string_ostream ostream(buffer);
+  ASSERT_TRUE(succeeded(writeBytecodeToFile(module, ostream)));
+  ostream.flush();
+
+  // Parse it back - this used to crash with an assertion failure in cast<>.
+  ParserConfig parseConfig(&context);
+  OwningOpRef<Operation *> roundTripModule =
+      parseSourceString<Operation *>(buffer, parseConfig);
+  ASSERT_TRUE(roundTripModule);
+
+  module.erase();
+}

``````````

</details>


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