[Mlir-commits] [mlir] 282d501 - [mlir][Transform] Fix crash with invalid ir for transform libraries (#75649)

[llvmlistbot at llvm.org]

Author: Kunwar Grover
Date: 2023-12-19T23:16:19+05:30
New Revision: 282d501476284c46fd943dcbae87494cb08e2c5f

URL: https://github.com/llvm/llvm-project/commit/282d501476284c46fd943dcbae87494cb08e2c5f
DIFF: https://github.com/llvm/llvm-project/commit/282d501476284c46fd943dcbae87494cb08e2c5f.diff

LOG: [mlir][Transform] Fix crash with invalid ir for transform libraries (#75649)

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

Added: 
    mlir/test/Dialect/Transform/include/test-interpreter-library-invalid/definitions-invalid.mlir
    mlir/test/Dialect/Transform/preload-library-invalid.mlir

Modified: 
    mlir/lib/Dialect/Transform/Transforms/TransformInterpreterUtils.cpp

Removed: 
    


################################################################################
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-library-invalid/definitions-invalid.mlir b/mlir/test/Dialect/Transform/include/test-interpreter-library-invalid/definitions-invalid.mlir
new file mode 100644
index 00000000000000..08e53a8d553c36
--- /dev/null
+++ b/mlir/test/Dialect/Transform/include/test-interpreter-library-invalid/definitions-invalid.mlir
@@ -0,0 +1,11 @@
+// RUN: mlir-opt %s --verify-diagnostics
+
+// The only thing we check here is that it should fail to parse. The other
+// check is in the preload test.
+
+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 "should have ',' prior to this" : !transform.any_op
+  }
+}

diff  --git a/mlir/test/Dialect/Transform/preload-library-invalid.mlir b/mlir/test/Dialect/Transform/preload-library-invalid.mlir
new file mode 100644
index 00000000000000..9abb849e4d27eb
--- /dev/null
+++ b/mlir/test/Dialect/Transform/preload-library-invalid.mlir
@@ -0,0 +1,7 @@
+// RUN: mlir-opt %s \
+// RUN:   -transform-preload-library=transform-library-paths=%p%{fs-sep}include%{fs-sep}test-interpreter-library-invalid \
+// RUN:   -transform-interpreter=entry-point=private_helper \
+// RUN:   -verify-diagnostics
+
+// This test checks if the preload mechanism fails gracefully when passed an
+// invalid transform file.