[MLIR] Fix crash in alias printer when encountering null operands#188581
Conversation
|
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-mlir-core Author: Mehdi Amini (joker-eph) ChangesWhen the IR contains operations with null operand values, the DummyAliasOperationPrinter used during alias collection crashed when iterating over operand types via op->getOperandTypes(), because ValueTypeIterator::mapElement calls value.getType() on each operand, which dereferences null for a null Value. Fix the crash by:
The test requires using --mlir-very-unsafe-disable-verifier-on-parsing to construct the invalid IR. Fixes #182747 Assisted-by: Claude Code Full diff: https://github.com/llvm/llvm-project/pull/188581.diff 2 Files Affected:
diff --git a/mlir/lib/IR/AsmPrinter.cpp b/mlir/lib/IR/AsmPrinter.cpp
index bc81720551a12..75008d6cc2591 100644
--- a/mlir/lib/IR/AsmPrinter.cpp
+++ b/mlir/lib/IR/AsmPrinter.cpp
@@ -752,9 +752,12 @@ class DummyAliasOperationPrinter : private OpAsmPrinter {
/*printBlockTerminators=*/true);
}
- // Visit all the types used in the operation.
- for (Type type : op->getOperandTypes())
- printType(type);
+ // Visit all the types used in the operation. Null operands/types can
+ // occur when operating on invalid IR (e.g., with
+ // --mlir-very-unsafe-disable-verifier-on-parsing), so guard against them.
+ for (Value operand : op->getOperands())
+ if (operand && operand.getType())
+ printType(operand.getType());
for (Type type : op->getResultTypes())
printType(type);
@@ -820,7 +823,10 @@ class DummyAliasOperationPrinter : private OpAsmPrinter {
}
/// Consider the given type to be printed for an alias.
- void printType(Type type) override { initializer.visit(type); }
+ void printType(Type type) override {
+ if (type)
+ initializer.visit(type);
+ }
/// Consider the given attribute to be printed for an alias.
void printAttribute(Attribute attr) override { initializer.visit(attr); }
@@ -970,6 +976,8 @@ class DummyAliasDialectAsmPrinter : public DialectAsmPrinter {
}
}
void printAndVisitNestedAliasesImpl(Type type) {
+ if (!type)
+ return;
if (!isa<BuiltinDialect>(type.getDialect()))
return type.getDialect().printType(type, *this);
diff --git a/mlir/test/IR/print-unsafe-null-operand.mlir b/mlir/test/IR/print-unsafe-null-operand.mlir
new file mode 100644
index 0000000000000..3715e4e46a892
--- /dev/null
+++ b/mlir/test/IR/print-unsafe-null-operand.mlir
@@ -0,0 +1,23 @@
+// RUN: mlir-opt %s --mlir-very-unsafe-disable-verifier-on-parsing 2>&1 | FileCheck %s
+//
+// Regression test for https://github.com/llvm/llvm-project/issues/182747
+// Verify that printing does not crash when an operation has a null operand
+// due to an unresolvable forward reference created with
+// --mlir-very-unsafe-disable-verifier-on-parsing.
+
+// CHECK: "scf.while"(<<NULL VALUE>>)
+// CHECK: <<NULL TYPE>>
+func.func @test(%arg0: memref<?xi32>, %arg1: i32) {
+ %0 = arith.constant 0 : index
+ %cond = arith.constant true
+ scf.while (%iter = %1) : (i32) -> i32 {
+ %1 = memref.load %arg0[%0] : memref<?xi32>
+ %2 = arith.addi %iter, %arg1 : i32
+ scf.condition(%cond) %2 : i32
+ } do {
+ ^bb0(%arg2: i32):
+ memref.store %arg2, %arg0[%0] : memref<?xi32>
+ scf.yield
+ }
+ return
+}
|
| // --mlir-very-unsafe-disable-verifier-on-parsing), so guard against them. | ||
| for (Value operand : op->getOperands()) | ||
| if (operand && operand.getType()) | ||
| printType(operand.getType()); |
There was a problem hiding this comment.
So we can just plain skip printing here as this is only for alias while the NULL would be printed still?
|
nit: something as simple as this i think would crash this too |
When using --mlir-very-unsafe-disable-verifier-on-parsing on IR with unresolvable forward SSA references (e.g., an scf.while whose init value is defined inside its own body region), the parsed IR can contain operations with null operand values. The DummyAliasOperationPrinter used during alias collection crashed when iterating over operand types via op->getOperandTypes(), because ValueTypeIterator::mapElement calls value.getType() on each operand, which dereferences null for a null Value. Fix the crash by: 1. Iterating over operand values directly in printGenericOp, skipping any null operands. 2. Adding a null-type guard in printType so that null types are silently skipped. 3. Adding a null-type guard at the top of printAndVisitNestedAliasesImpl to avoid crashing when visiting sub-elements of a null type. Fixes llvm#182747 Assisted-by: Claude Code
2be61b0 to
ac42b6b
Compare
…vm#188581) When the IR contains operations with null operand values, the DummyAliasOperationPrinter used during alias collection crashed when iterating over operand types via op->getOperandTypes(), because ValueTypeIterator::mapElement calls value.getType() on each operand, which dereferences null for a null Value. Fix the crash by: 1. Iterating over operand values directly in printGenericOp, skipping any null operands. 2. Adding a null-type guard in printType so that null types are silently skipped. 3. Adding a null-type guard at the top of printAndVisitNestedAliasesImpl to avoid crashing when visiting sub-elements of a null type. The test requires using --mlir-very-unsafe-disable-verifier-on-parsing to construct the invalid IR. Fixes llvm#182747 Assisted-by: Claude Code
…vm#188581) When the IR contains operations with null operand values, the DummyAliasOperationPrinter used during alias collection crashed when iterating over operand types via op->getOperandTypes(), because ValueTypeIterator::mapElement calls value.getType() on each operand, which dereferences null for a null Value. Fix the crash by: 1. Iterating over operand values directly in printGenericOp, skipping any null operands. 2. Adding a null-type guard in printType so that null types are silently skipped. 3. Adding a null-type guard at the top of printAndVisitNestedAliasesImpl to avoid crashing when visiting sub-elements of a null type. The test requires using --mlir-very-unsafe-disable-verifier-on-parsing to construct the invalid IR. Fixes llvm#182747 Assisted-by: Claude Code
When the IR contains operations with null operand values, the DummyAliasOperationPrinter used during alias collection crashed when iterating over operand types via op->getOperandTypes(), because ValueTypeIterator::mapElement calls value.getType() on each operand, which dereferences null for a null Value.
Fix the crash by:
The test requires using --mlir-very-unsafe-disable-verifier-on-parsing to construct the invalid IR.
Fixes #182747
Assisted-by: Claude Code