[mlir][scf] Fix crash in ForOp verifier when body block has no arguments#183946
Conversation
|
@llvm/pr-subscribers-mlir-scf Author: Mehdi Amini (joker-eph) ChangesA malformed Fix by adding an explicit check in Fixes #159737 Full diff: https://github.com/llvm/llvm-project/pull/183946.diff 2 Files Affected:
diff --git a/mlir/lib/Dialect/SCF/IR/SCF.cpp b/mlir/lib/Dialect/SCF/IR/SCF.cpp
index c46a0577c4b96..ac212d3ae6dcd 100644
--- a/mlir/lib/Dialect/SCF/IR/SCF.cpp
+++ b/mlir/lib/Dialect/SCF/IR/SCF.cpp
@@ -348,6 +348,16 @@ void ForOp::build(OpBuilder &builder, OperationState &result, Value lb,
}
LogicalResult ForOp::verify() {
+ // Check that the body block has at least the induction variable argument.
+ // This must be checked before verifyRegions() and before any region trait
+ // verifiers (e.g. LoopLikeOpInterface) that call getRegionIterArgs(), to
+ // avoid crashing with an out-of-bounds drop_front on an empty arg list.
+ if (getBody()->getNumArguments() < getNumInductionVars())
+ return emitOpError("expected body to have at least ")
+ << getNumInductionVars()
+ << " argument(s) for the induction variable, but got "
+ << getBody()->getNumArguments();
+
// Check that the number of init args and op results is the same.
if (getInitArgs().size() != getNumResults())
return emitOpError(
diff --git a/mlir/test/Dialect/SCF/invalid.mlir b/mlir/test/Dialect/SCF/invalid.mlir
index 985e347fbf5ee..a61ef5c2952f8 100644
--- a/mlir/test/Dialect/SCF/invalid.mlir
+++ b/mlir/test/Dialect/SCF/invalid.mlir
@@ -835,3 +835,23 @@ func.func @invalid_reference(%a: index) {
}
return
}
+
+// -----
+
+// Regression test for https://github.com/llvm/llvm-project/issues/159737
+// A nested scf.for whose outer body block has no arguments used to crash in
+// getRegionIterArgs() via verifyLoopLikeOpInterface instead of producing a
+// proper diagnostic.
+func.func @for_missing_induction_var(%arg0: index, %arg1: index) {
+ %c1 = arith.constant 1 : index
+ %c2 = arith.constant 2 : index
+ // expected-error@+1 {{expected body to have at least 1 argument(s) for the induction variable, but got 0}}
+ "scf.for"(%arg0, %arg1, %c1) ({
+ "scf.for"(%arg0, %arg1, %c2) ({
+ ^bb0(%iv: index):
+ "scf.yield"() : () -> ()
+ }) : (index, index, index) -> ()
+ "scf.yield"() : () -> ()
+ }) : (index, index, index) -> ()
+ return
+}
|
|
@llvm/pr-subscribers-mlir Author: Mehdi Amini (joker-eph) ChangesA malformed Fix by adding an explicit check in Fixes #159737 Full diff: https://github.com/llvm/llvm-project/pull/183946.diff 2 Files Affected:
diff --git a/mlir/lib/Dialect/SCF/IR/SCF.cpp b/mlir/lib/Dialect/SCF/IR/SCF.cpp
index c46a0577c4b96..ac212d3ae6dcd 100644
--- a/mlir/lib/Dialect/SCF/IR/SCF.cpp
+++ b/mlir/lib/Dialect/SCF/IR/SCF.cpp
@@ -348,6 +348,16 @@ void ForOp::build(OpBuilder &builder, OperationState &result, Value lb,
}
LogicalResult ForOp::verify() {
+ // Check that the body block has at least the induction variable argument.
+ // This must be checked before verifyRegions() and before any region trait
+ // verifiers (e.g. LoopLikeOpInterface) that call getRegionIterArgs(), to
+ // avoid crashing with an out-of-bounds drop_front on an empty arg list.
+ if (getBody()->getNumArguments() < getNumInductionVars())
+ return emitOpError("expected body to have at least ")
+ << getNumInductionVars()
+ << " argument(s) for the induction variable, but got "
+ << getBody()->getNumArguments();
+
// Check that the number of init args and op results is the same.
if (getInitArgs().size() != getNumResults())
return emitOpError(
diff --git a/mlir/test/Dialect/SCF/invalid.mlir b/mlir/test/Dialect/SCF/invalid.mlir
index 985e347fbf5ee..a61ef5c2952f8 100644
--- a/mlir/test/Dialect/SCF/invalid.mlir
+++ b/mlir/test/Dialect/SCF/invalid.mlir
@@ -835,3 +835,23 @@ func.func @invalid_reference(%a: index) {
}
return
}
+
+// -----
+
+// Regression test for https://github.com/llvm/llvm-project/issues/159737
+// A nested scf.for whose outer body block has no arguments used to crash in
+// getRegionIterArgs() via verifyLoopLikeOpInterface instead of producing a
+// proper diagnostic.
+func.func @for_missing_induction_var(%arg0: index, %arg1: index) {
+ %c1 = arith.constant 1 : index
+ %c2 = arith.constant 2 : index
+ // expected-error@+1 {{expected body to have at least 1 argument(s) for the induction variable, but got 0}}
+ "scf.for"(%arg0, %arg1, %c1) ({
+ "scf.for"(%arg0, %arg1, %c2) ({
+ ^bb0(%iv: index):
+ "scf.yield"() : () -> ()
+ }) : (index, index, index) -> ()
+ "scf.yield"() : () -> ()
+ }) : (index, index, index) -> ()
+ return
+}
|
A malformed `scf.for` whose body block contains no arguments caused `getRegionIterArgs()` to crash via an out-of-bounds `drop_front(1)` call. This happened because `verifyLoopLikeOpInterface` (a `verifyRegionTrait`) invokes `getRegionIterArgs()` during `verifyRegionInvariants`, which runs before `verifyRegions()` has a chance to report a proper diagnostic. Fix by adding an explicit check in `ForOp::verify()` (which runs in `verifyInvariants`, before any region trait verifiers execute) that ensures the body block has at least as many arguments as there are induction variables. This prevents the crash and produces a clear error message. Fixes llvm#159737
71739a9 to
72a8a82
Compare
…nts (llvm#183946) A malformed `scf.for` whose body block contains no arguments caused `getRegionIterArgs()` to crash via an out-of-bounds `drop_front(1)` call. This happened because `verifyLoopLikeOpInterface` (a `verifyRegionTrait`) invokes `getRegionIterArgs()` during `verifyRegionInvariants`, which runs before `verifyRegions()` has a chance to report a proper diagnostic. Fix by adding an explicit check in `ForOp::verify()` (which runs in `verifyInvariants`, before any region trait verifiers execute) that ensures the body block has at least as many arguments as there are induction variables. This prevents the crash and produces a clear error message. Fixes llvm#159737
…nts (llvm#183946) A malformed `scf.for` whose body block contains no arguments caused `getRegionIterArgs()` to crash via an out-of-bounds `drop_front(1)` call. This happened because `verifyLoopLikeOpInterface` (a `verifyRegionTrait`) invokes `getRegionIterArgs()` during `verifyRegionInvariants`, which runs before `verifyRegions()` has a chance to report a proper diagnostic. Fix by adding an explicit check in `ForOp::verify()` (which runs in `verifyInvariants`, before any region trait verifiers execute) that ensures the body block has at least as many arguments as there are induction variables. This prevents the crash and produces a clear error message. Fixes llvm#159737
A malformed
scf.forwhose body block contains no arguments causedgetRegionIterArgs()to crash via an out-of-boundsdrop_front(1)call. This happened becauseverifyLoopLikeOpInterface(averifyRegionTrait) invokesgetRegionIterArgs()duringverifyRegionInvariants, which runs beforeverifyRegions()has a chance to report a proper diagnostic.Fix by adding an explicit check in
ForOp::verify()(which runs inverifyInvariants, before any region trait verifiers execute) that ensures the body block has at least as many arguments as there are induction variables. This prevents the crash and produces a clear error message.Fixes #159737