[Mlir-commits] [mlir] [mlir-tblgen] trim method body to empty with only spaces to avoid crash (PR #139568)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Mon May 12 08:53:33 PDT 2025
https://github.com/fengxie created https://github.com/llvm/llvm-project/pull/139568
method body or default impl must be true empty. Even they contain only spaces, ``mlir-tblgen`` considers they are non-empty and generates invalid code lead to segment fault. It's very hard to debug.
```c++
InterfaceMethod<
...
/*methodBody=*/ [{ }], // This must be true empty. Leaving a space here can lead to segment fault which is hard to figure out why
/*defaultImpl=*/ [{
...
}]
```
This PR trim spaces when method body or default implementation of interface method is not empty. Now ``mlir-tblgen`` generates valid code even when they contain only spaces.
>From 1dfa667491a0b6249685779b6fb3835255266c31 Mon Sep 17 00:00:00 2001
From: Fung Xie <ftse at nvidia.com>
Date: Mon, 12 May 2025 23:27:57 +0800
Subject: [PATCH 1/2] trim method body with spaces to avoid crash
---
mlir/lib/TableGen/Interfaces.cpp | 6 +++
.../mlir-tblgen/method-body-has-spaces.td | 45 +++++++++++++++++++
2 files changed, 51 insertions(+)
create mode 100644 mlir/test/mlir-tblgen/method-body-has-spaces.td
diff --git a/mlir/lib/TableGen/Interfaces.cpp b/mlir/lib/TableGen/Interfaces.cpp
index a209b003b0f3b..9e9d4e4d3b13a 100644
--- a/mlir/lib/TableGen/Interfaces.cpp
+++ b/mlir/lib/TableGen/Interfaces.cpp
@@ -47,12 +47,18 @@ bool InterfaceMethod::isStatic() const {
// Return the body for this method if it has one.
std::optional<StringRef> InterfaceMethod::getBody() const {
auto value = def->getValueAsString("body");
+ // Trim leading and trailing spaces from the default implementation.
+ if (!value.empty())
+ value = value.trim();
return value.empty() ? std::optional<StringRef>() : value;
}
// Return the default implementation for this method if it has one.
std::optional<StringRef> InterfaceMethod::getDefaultImplementation() const {
auto value = def->getValueAsString("defaultBody");
+ // Trim leading and trailing spaces from the default implementation.
+ if (!value.empty())
+ value = value.trim();
return value.empty() ? std::optional<StringRef>() : value;
}
diff --git a/mlir/test/mlir-tblgen/method-body-has-spaces.td b/mlir/test/mlir-tblgen/method-body-has-spaces.td
new file mode 100644
index 0000000000000..41ff84e9b480a
--- /dev/null
+++ b/mlir/test/mlir-tblgen/method-body-has-spaces.td
@@ -0,0 +1,45 @@
+// RUN: mlir-tblgen -gen-op-defs -I %S/../../include %s | FileCheck %s
+
+include "mlir/IR/OpBase.td"
+
+def TestEmptyMethodBodyTypeInterface : TypeInterface<"TestEmptyMethodBodyTypeInterface"> {
+ let cppNamespace = "::TestEmptyMethodBodyTypeInterface";
+ let description = [{
+ Treat method body with only spaces as empty.
+ }];
+ let methods = [
+ InterfaceMethod<
+ /*desc=*/ [{ Trim spaces of method body and default implementation }],
+ /*returnType=*/ "StringRef",
+ /*methodName=*/ "trimSpaces",
+ /*args=*/ (ins),
+ // CHECK-LABEL: StringRef detail::TestEmptyMethodBodyTypeInterfaceInterfaceTraits::Model<ConcreteType>::trimSpaces
+ // CHECK-SAME: {
+ // CHECK-NEXT: return (::llvm::cast<ConcreteType>(tablegen_opaque_val)).trimSpaces();
+ // CHECK-NEXT: }
+ /*methodBody=*/ [{ }],
+ /*defaultImpl=*/ [{ return StringRef(); }]
+ >
+ ];
+}
+
+def TestEmptyDefaultImplTypeInterface : TypeInterface<"TestEmptyDefaultImplTypeInterface"> {
+ let cppNamespace = "::TestEmptyDefaultImplTypeInterface";
+ let description = [{
+ Treat default implementation with only spaces as empty.
+ }];
+
+ let methods = [
+ InterfaceMethod<
+ /*desc=*/ [{ Trim spaces of default implementation }],
+ /*returnType=*/ "StringRef",
+ /*methodName=*/ "trimSpaces",
+ /*args=*/ (ins),
+ /*methodBody=*/ [{ return StringRef(); }],
+ // COM: Don't generate default implementation
+ // CHECK-NOT: StringRef detail::TestEmptyDefaultImplTypeInterfaceInterfaceTraits::ExternalModel<ConcreteModel, ConcreteType>::trimSpaces
+ /*defaultImpl=*/ [{
+ }]
+ >
+ ];
+}
>From 7b59fe547301e5c19b81aee8d1401a2c4c0abed5 Mon Sep 17 00:00:00 2001
From: Fung Xie <ftse at nvidia.com>
Date: Mon, 12 May 2025 23:51:27 +0800
Subject: [PATCH 2/2] fix test and rename
---
...ethod-body-has-spaces.td => method-body-with-only-spaces.td} | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
rename mlir/test/mlir-tblgen/{method-body-has-spaces.td => method-body-with-only-spaces.td} (94%)
diff --git a/mlir/test/mlir-tblgen/method-body-has-spaces.td b/mlir/test/mlir-tblgen/method-body-with-only-spaces.td
similarity index 94%
rename from mlir/test/mlir-tblgen/method-body-has-spaces.td
rename to mlir/test/mlir-tblgen/method-body-with-only-spaces.td
index 41ff84e9b480a..daa0067e062d5 100644
--- a/mlir/test/mlir-tblgen/method-body-has-spaces.td
+++ b/mlir/test/mlir-tblgen/method-body-with-only-spaces.td
@@ -1,4 +1,4 @@
-// RUN: mlir-tblgen -gen-op-defs -I %S/../../include %s | FileCheck %s
+// RUN: mlir-tblgen --gen-type-interface-decls -I %S/../../include %s | FileCheck %s
include "mlir/IR/OpBase.td"
More information about the Mlir-commits
mailing list