[Mlir-commits] [mlir] 493fbc4 - [MLIR][Test] Avoid buffer overflow for `test.verifiers`

[llvmlistbot at llvm.org]

Author: rikhuijzer
Date: 2023-07-10T19:15:29+02:00
New Revision: 493fbc4c8e3f4303e5660e335f169c55462c9618

URL: https://github.com/llvm/llvm-project/commit/493fbc4c8e3f4303e5660e335f169c55462c9618
DIFF: https://github.com/llvm/llvm-project/commit/493fbc4c8e3f4303e5660e335f169c55462c9618.diff

LOG: [MLIR][Test] Avoid buffer overflow for `test.verifiers`

Fixes buffer overflows which occurred anytime a "test.verifiers" op was used.

Fixes #61378, fixes #61379, fixes #61381.

Reviewed By: mehdi_amini

Differential Revision: https://reviews.llvm.org/D154792

Added: 
    mlir/test/IR/test-verifiers-op.mlir

Modified: 
    mlir/test/lib/Dialect/Test/TestDialect.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/test/IR/test-verifiers-op.mlir b/mlir/test/IR/test-verifiers-op.mlir
new file mode 100644
index 00000000000000..a6ae016244885a
--- /dev/null
+++ b/mlir/test/IR/test-verifiers-op.mlir
@@ -0,0 +1,13 @@
+// RUN: mlir-opt %s | FileCheck %s
+
+// CHECK-LABEL: func @no_overflow_on_test_verifiers_op
+func.func @no_overflow_on_test_verifiers_op() {
+  %0 = arith.constant 1 : i32
+  "test.verifiers"(%0) ({
+    %1 = arith.constant 2 : i32
+    "test.verifiers"(%1) ({
+      %2 = arith.constant 3 : index
+    }) : (i32) -> ()
+  }) : (i32) -> ()
+  return
+}

diff  --git a/mlir/test/lib/Dialect/Test/TestDialect.cpp b/mlir/test/lib/Dialect/Test/TestDialect.cpp
index 1ec769847a1c44..a12a5a302672b1 100644
--- a/mlir/test/lib/Dialect/Test/TestDialect.cpp
+++ b/mlir/test/lib/Dialect/Test/TestDialect.cpp
@@ -1229,8 +1229,8 @@ void PrettyPrintedRegionOp::print(OpAsmPrinter &p) {
   // Assuming that region has a single non-terminator inner-op, if the inner-op
   // meets some criteria (which in this case is a simple one  based on the name
   // of inner-op), then we can print the entire region in a succinct way.
-  // Here we assume that the prototype of "test.special.op" can be trivially derived
-  // while parsing it back.
+  // Here we assume that the prototype of "test.special.op" can be trivially
+  // derived while parsing it back.
   if (innerOp.getName().getStringRef().equals("test.special.op")) {
     p << " start test.special.op end";
   } else {
@@ -1788,7 +1788,9 @@ LogicalResult TestVerifiersOp::verify() {
   if (definingOp && failed(mlir::verify(definingOp)))
     return emitOpError("operand hasn't been verified");
 
-  emitRemark("success run of verifier");
+  // Avoid using `emitRemark(msg)` since that will trigger an infinite verifier
+  // loop.
+  mlir::emitRemark(getLoc(), "success run of verifier");
 
   return success();
 }
@@ -1802,7 +1804,9 @@ LogicalResult TestVerifiersOp::verifyRegions() {
       if (failed(mlir::verify(&op)))
         return emitOpError("nested op hasn't been verified");
 
-  emitRemark("success run of region verifier");
+  // Avoid using `emitRemark(msg)` since that will trigger an infinite verifier
+  // loop.
+  mlir::emitRemark(getLoc(), "success run of region verifier");
 
   return success();
 }