[Mlir-commits] [mlir] [MLIR][SymbolTable] Fix crash when SymbolTable is built on unverified IR (PR #183945)

[Mehdi Amini]

https://github.com/joker-eph created https://github.com/llvm/llvm-project/pull/183945

The SymbolTable::SymbolTable constructor asserted that all symbol names in the region were unique. This could cause mlir-opt to crash instead of producing a proper diagnostic when the IR contained both:

1. An IsolatedFromAbove op (e.g., irdl.dialect) with a symbol user that looks up symbols in an ancestor symbol table, and
2. Duplicate symbols in that ancestor (e.g., two func.func @test).

The crash occurred because IsolatedFromAbove ops are verified in verifyOnExit() before verifyRegionInvariants() runs the SymbolTable trait's verifyRegionTrait (which produces the proper duplicate-symbol diagnostic). When the isolated op's symbol use verification triggered SymbolTableCollection::getSymbolTable() on the ancestor, the constructor would assert instead of gracefully handling the invalid-but-not-yet- reported duplicate symbols.

The fix removes the assertion and silently skips duplicate symbol insertions (keeping the first definition). The proper diagnostic is still produced by the SymbolTable trait's verifyRegionTrait, which runs after all children have been verified.

Add a regression test in invalid.irdl.mlir using a self-referencing IRDL parametric type combined with duplicate function symbols.

Fixes #159673

>From f2c26b88e165186aad63c16f2d50a8a141710502 Mon Sep 17 00:00:00 2001
From: Mehdi Amini <joker.eph at gmail.com>
Date: Sat, 28 Feb 2026 11:33:17 -0800
Subject: [PATCH] [MLIR][SymbolTable] Fix crash when SymbolTable is built on
 unverified IR

The SymbolTable::SymbolTable constructor asserted that all symbol names
in the region were unique. This could cause mlir-opt to crash instead of
producing a proper diagnostic when the IR contained both:

1. An IsolatedFromAbove op (e.g., irdl.dialect) with a symbol user
   that looks up symbols in an ancestor symbol table, and
2. Duplicate symbols in that ancestor (e.g., two func.func @test).

The crash occurred because IsolatedFromAbove ops are verified in
verifyOnExit() before verifyRegionInvariants() runs the SymbolTable
trait's verifyRegionTrait (which produces the proper duplicate-symbol
diagnostic). When the isolated op's symbol use verification triggered
SymbolTableCollection::getSymbolTable() on the ancestor, the constructor
would assert instead of gracefully handling the invalid-but-not-yet-
reported duplicate symbols.

The fix removes the assertion and silently skips duplicate symbol
insertions (keeping the first definition). The proper diagnostic is
still produced by the SymbolTable trait's verifyRegionTrait, which
runs after all children have been verified.

Add a regression test in invalid.irdl.mlir using a self-referencing
IRDL parametric type combined with duplicate function symbols.

Fixes #159673
---
 mlir/lib/IR/SymbolTable.cpp              | 11 +++++++----
 mlir/test/Dialect/IRDL/invalid.irdl.mlir | 20 ++++++++++++++++++++
 2 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/mlir/lib/IR/SymbolTable.cpp b/mlir/lib/IR/SymbolTable.cpp
index 4e191e7d612ad..078401c8380f4 100644
--- a/mlir/lib/IR/SymbolTable.cpp
+++ b/mlir/lib/IR/SymbolTable.cpp
@@ -130,10 +130,13 @@ SymbolTable::SymbolTable(Operation *symbolTableOp)
     if (!name)
       continue;
 
-    auto inserted = symbolTable.insert({name, &op});
-    (void)inserted;
-    assert(inserted.second &&
-           "expected region to contain uniquely named symbol operations");
+    // Silently skip duplicate symbol names. Duplicate symbols are an
+    // invalid IR condition diagnosed by the SymbolTable trait's
+    // verifyRegionTrait. The constructor may be called before verification
+    // completes (e.g., when IsolatedFromAbove ops look up symbols in an
+    // ancestor symbol table during verification), so an assert here would
+    // crash instead of producing a proper diagnostic.
+    symbolTable.try_emplace(name, &op);
   }
 }
 
diff --git a/mlir/test/Dialect/IRDL/invalid.irdl.mlir b/mlir/test/Dialect/IRDL/invalid.irdl.mlir
index 8a7fffe1a9cbd..4ff7445c80de8 100644
--- a/mlir/test/Dialect/IRDL/invalid.irdl.mlir
+++ b/mlir/test/Dialect/IRDL/invalid.irdl.mlir
@@ -179,3 +179,23 @@ irdl.dialect @invalid_parametric {
     irdl.results(foo: %param)
   }
 }
+
+// -----
+
+// Regression test for https://github.com/llvm/llvm-project/issues/159673
+// A self-referencing IRDL parametric type combined with duplicate function
+// symbols in the enclosing module used to crash with a SymbolTable assertion
+// instead of producing a proper "redefinition of symbol" diagnostic.
+
+irdl.dialect @testd {
+  irdl.type @self_referencing {
+    %0 = irdl.any
+    %1 = irdl.parametric @testd::@self_referencing<%0>
+    irdl.parameters(foo: %1)
+  }
+}
+
+// expected-note at +1 {{see existing symbol definition here}}
+func.func @test() { return }
+// expected-error at +1 {{redefinition of symbol named 'test'}}
+func.func @test() { return }