[Mlir-commits] [mlir] [mlir] Fix assertion crash (#159673) (PR #173020)

[Jelle Schühmacher]

https://github.com/jschuhmacher created https://github.com/llvm/llvm-project/pull/173020

MLIR crashes with assertion 'inserted.second && "expected region to contain uniquely named symbol operations"' failed when constructing a symbol table from another symbol table that has duplicate symbols.

The API allowed a SymbolTable instance to be in this state in the first place, I think copying it should be allowed in that state as well. Simply removing the assert allows construction of the instance, and allows a subsequent run of the verifier to report the actual error.

>From 8e236a974d3adacf2b14644fd9a4ab7f024ce64f Mon Sep 17 00:00:00 2001
From: Jelle Schuhmacher <me at jschuhmacher.nl>
Date: Thu, 18 Dec 2025 20:22:17 +0100
Subject: [PATCH] [mlir] Fix assertion crash (#159673)

Crashes with assertion 'inserted.second && "expected region to contain
uniquely named symbol operations"' failed when constructing a symbol
table from another symbol table that has duplicate symbols.

Simply removing the assert allows construction of the instance, and
allows a subsequent run of the verifier to report the error.
---
 mlir/lib/IR/SymbolTable.cpp           |  2 --
 mlir/unittests/IR/SymbolTableTest.cpp | 30 +++++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/mlir/lib/IR/SymbolTable.cpp b/mlir/lib/IR/SymbolTable.cpp
index 9f5dd2c9e3b72..528b4f586d66f 100644
--- a/mlir/lib/IR/SymbolTable.cpp
+++ b/mlir/lib/IR/SymbolTable.cpp
@@ -132,8 +132,6 @@ SymbolTable::SymbolTable(Operation *symbolTableOp)
 
     auto inserted = symbolTable.insert({name, &op});
     (void)inserted;
-    assert(inserted.second &&
-           "expected region to contain uniquely named symbol operations");
   }
 }
 
diff --git a/mlir/unittests/IR/SymbolTableTest.cpp b/mlir/unittests/IR/SymbolTableTest.cpp
index 864eb40898335..45d48f62c10b3 100644
--- a/mlir/unittests/IR/SymbolTableTest.cpp
+++ b/mlir/unittests/IR/SymbolTableTest.cpp
@@ -166,4 +166,34 @@ TEST(SymbolOpInterface, Visibility) {
   ASSERT_EQ(diagStr, expectedDiag);
 }
 
+TEST(SymbolOpInterface, Duplicate) {
+  DialectRegistry registry;
+  ::test::registerTestDialect(registry);
+  MLIRContext context(registry);
+
+  constexpr static StringLiteral kInput = R"MLIR(
+    "test.overridden_symbol_visibility"() {sym_name = "duplicate_symbol_name"} : () -> ()
+    "test.overridden_symbol_visibility"() {sym_name = "duplicate_symbol_name"} : () -> ()
+  )MLIR";
+
+  constexpr bool verifyAfterParse = false;
+  ParserConfig parserConfig(&context, verifyAfterParse);
+  OwningOpRef<ModuleOp> module =
+      parseSourceString<ModuleOp>(kInput, parserConfig);
+
+  // copying an existing symboltable instance should not crash
+  SymbolTable symbolTable(module.get());
+
+  std::string diagStr;
+  constexpr static StringLiteral expectedDiag =
+      "redefinition of symbol named 'duplicate_symbol_name'";
+  context.getDiagEngine().registerHandler(
+      [&diagStr](Diagnostic &diag) { diagStr += diag.str(); });
+
+  LogicalResult res = mlir::verify(module.get(), true);
+
+  ASSERT_FALSE(succeeded(res));
+  ASSERT_EQ(diagStr, expectedDiag);
+}
+
 } // namespace