[mlir][Transform] Fix crash with invalid ir for transform libraries

[Groverkss]

This patch fixes a crash caused when the transform library interpreter is given an IR that fails to parse.

[llvmbot]

@llvm/pr-subscribers-mlir

Author: Kunwar Grover (Groverkss)

Changes

This patch fixes a crash caused when the transform library interpreter is given an IR that fails to parse.

---

Full diff: https://github.com/llvm/llvm-project/pull/75649.diff

2 Files Affected:

• (modified) mlir/lib/Dialect/Transform/Transforms/TransformInterpreterUtils.cpp (+6)
• (added) mlir/test/Dialect/Transform/include/test-interpreter-invalid.mlir (+8)

diff --git a/mlir/lib/Dialect/Transform/Transforms/TransformInterpreterUtils.cpp b/mlir/lib/Dialect/Transform/Transforms/TransformInterpreterUtils.cpp
index 3fa26bce150992..2f74b76f07b77b 100644
--- a/mlir/lib/Dialect/Transform/Transforms/TransformInterpreterUtils.cpp
+++ b/mlir/lib/Dialect/Transform/Transforms/TransformInterpreterUtils.cpp
@@ -109,6 +109,12 @@ LogicalResult transform::detail::parseTransformModuleFromFile(
   sourceMgr.AddNewSourceBuffer(std::move(memoryBuffer), llvm::SMLoc());
   transformModule =
       OwningOpRef<ModuleOp>(parseSourceFile<ModuleOp>(sourceMgr, context));
+  if (!transformModule) {
+    // Failed to parse the transform module.
+    // Don't need to emit an error here as the parsing should have already done
+    // that.
+    return failure();
+  }
   return mlir::verify(*transformModule);
 }
 
diff --git a/mlir/test/Dialect/Transform/include/test-interpreter-invalid.mlir b/mlir/test/Dialect/Transform/include/test-interpreter-invalid.mlir
new file mode 100644
index 00000000000000..8b5641293d27e6
--- /dev/null
+++ b/mlir/test/Dialect/Transform/include/test-interpreter-invalid.mlir
@@ -0,0 +1,8 @@
+// RUN: mlir-opt %s --verify-diagnostics
+
+module attributes {transform.with_named_sequence} {
+  transform.named_sequence private @private_helper(%arg0: !transform.any_op {transform.readonly}) {
+    // expected-error @below {{expected ','}}
+    transform.test_print_remark_at_operand %arg0 "message" : !transform.any_op
+  }
+}

[ingomueller-net]

Awesome, thanks for the fix!

[ftynse]

Thanks for the patch. Please add the test that actually tests the behavior the patch is fixing. Removing the changes in .cpp doesn't make the test fail. This file should likely be passed as "library" via one of the mechanisms that ultimately invokes this function.

[Groverkss]

Thanks for the patch. Please add the test that actually tests the behavior the patch is fixing. Removing the changes in .cpp doesn't make the test fail. This file should likely be passed as "library" via one of the mechanisms that ultimately invokes this function.

Thank you for checking. I pushed a test that uses the library mechanisms to check for failure. Also confirmed that the tests fail without the .cpp change now.