[llvm] [TableGen] Fix operand constraint checking problem. (PR #85859)
Jason Eckhardt via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 20 11:16:13 PDT 2024
https://github.com/nvjle updated https://github.com/llvm/llvm-project/pull/85859
>From df6882388a0757fd6cd219cf069cf7b99514b68d Mon Sep 17 00:00:00 2001
From: Jason Eckhardt <jeckhardt at nvidia.com>
Date: Tue, 19 Mar 2024 14:58:40 -0500
Subject: [PATCH 1/3] [TableGen] Fix operand constraint checking problem.
Currently operand constraint checks on "$dest = $src" are inadvertently
accepting any token that contains "=". This has surprising results, e.g,
"$dest != $src" is accepted as a constraint but then treated as "=".
This patch ensures that only exactly the token "=" is accepted.
---
llvm/test/TableGen/ConstraintChecking3.td | 2 +-
llvm/test/TableGen/ConstraintChecking8.td | 8 ++++++++
llvm/test/TableGen/ConstraintChecking9.td | 8 ++++++++
llvm/utils/TableGen/CodeGenInstruction.cpp | 3 ++-
4 files changed, 19 insertions(+), 2 deletions(-)
create mode 100644 llvm/test/TableGen/ConstraintChecking8.td
create mode 100644 llvm/test/TableGen/ConstraintChecking9.td
diff --git a/llvm/test/TableGen/ConstraintChecking3.td b/llvm/test/TableGen/ConstraintChecking3.td
index 2d5fe6b7ef9699..886e6d52a039fe 100644
--- a/llvm/test/TableGen/ConstraintChecking3.td
+++ b/llvm/test/TableGen/ConstraintChecking3.td
@@ -4,5 +4,5 @@ include "ConstraintChecking.inc"
// (This is illegal because the '=' has to be surrounded by whitespace)
-// CHECK: [[FILE]]:[[@LINE+1]]:5: error: Illegal format for tied-to constraint in 'Foo'
+// CHECK: [[FILE]]:[[@LINE+1]]:5: error: Unrecognized constraint '$dest1=$dest2' in 'Foo'
def Foo : TestInstructionWithConstraints<"$dest1=$dest2">;
diff --git a/llvm/test/TableGen/ConstraintChecking8.td b/llvm/test/TableGen/ConstraintChecking8.td
new file mode 100644
index 00000000000000..52ecd23bf41044
--- /dev/null
+++ b/llvm/test/TableGen/ConstraintChecking8.td
@@ -0,0 +1,8 @@
+// RUN: not llvm-tblgen -gen-asm-writer -I %p -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s
+
+include "ConstraintChecking.inc"
+
+// Make sure exactly the token "=" appears.
+
+// CHECK: [[FILE]]:[[@LINE+1]]:5: error: Unrecognized constraint '$dest1 != $src2' in 'Foo'
+def Foo : TestInstructionWithConstraints<"$dest1 != $src2">;
diff --git a/llvm/test/TableGen/ConstraintChecking9.td b/llvm/test/TableGen/ConstraintChecking9.td
new file mode 100644
index 00000000000000..afd6706fdf4900
--- /dev/null
+++ b/llvm/test/TableGen/ConstraintChecking9.td
@@ -0,0 +1,8 @@
+// RUN: not llvm-tblgen -gen-asm-writer -I %p -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s
+
+include "ConstraintChecking.inc"
+
+// Make sure exactly the token "=" appears.
+
+// CHECK: [[FILE]]:[[@LINE+1]]:5: error: Unrecognized constraint '$dest1 == $src2' in 'Foo'
+def Foo : TestInstructionWithConstraints<"$dest1 == $src2">;
diff --git a/llvm/utils/TableGen/CodeGenInstruction.cpp b/llvm/utils/TableGen/CodeGenInstruction.cpp
index b00b95da5fc276..44b6f2dded681a 100644
--- a/llvm/utils/TableGen/CodeGenInstruction.cpp
+++ b/llvm/utils/TableGen/CodeGenInstruction.cpp
@@ -325,7 +325,8 @@ static void ParseConstraint(StringRef CStr, CGIOperandList &Ops, Record *Rec) {
// Only other constraint is "TIED_TO" for now.
StringRef::size_type pos = CStr.find_first_of('=');
- if (pos == StringRef::npos)
+ if (pos == StringRef::npos || CStr.find_first_of(" \t", pos) != (pos + 1) ||
+ CStr.find_last_of(" \t", pos) != (pos - 1))
PrintFatalError(Rec->getLoc(), "Unrecognized constraint '" + CStr +
"' in '" + Rec->getName() + "'");
start = CStr.find_first_not_of(" \t");
>From 335da3b8ac483ea59ede1b71ef2963598c1744c5 Mon Sep 17 00:00:00 2001
From: Jason Eckhardt <jeckhardt at nvidia.com>
Date: Wed, 20 Mar 2024 09:17:46 -0500
Subject: [PATCH 2/3] Add more corner cases to ConstraintChecking8.td.
And remove ConstraintChecking9.td.
---
llvm/test/TableGen/ConstraintChecking8.td | 28 ++++++++++++++++++++++-
llvm/test/TableGen/ConstraintChecking9.td | 8 -------
2 files changed, 27 insertions(+), 9 deletions(-)
delete mode 100644 llvm/test/TableGen/ConstraintChecking9.td
diff --git a/llvm/test/TableGen/ConstraintChecking8.td b/llvm/test/TableGen/ConstraintChecking8.td
index 52ecd23bf41044..699e7f5b6418ab 100644
--- a/llvm/test/TableGen/ConstraintChecking8.td
+++ b/llvm/test/TableGen/ConstraintChecking8.td
@@ -1,8 +1,34 @@
-// RUN: not llvm-tblgen -gen-asm-writer -I %p -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s
+// RUN: not llvm-tblgen -gen-asm-writer -DT0 -I %p -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s
+// RUN: not llvm-tblgen -gen-asm-writer -DT1 -I %p -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s --check-prefix=CHECK1
+// RUN: not llvm-tblgen -gen-asm-writer -DT2 -I %p -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s --check-prefix=CHECK2
+// RUN: not llvm-tblgen -gen-asm-writer -DT3 -I %p -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s --check-prefix=CHECK3
+// RUN: not llvm-tblgen -gen-asm-writer -DT4 -I %p -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s --check-prefix=CHECK4
include "ConstraintChecking.inc"
// Make sure exactly the token "=" appears.
+#ifdef T0
// CHECK: [[FILE]]:[[@LINE+1]]:5: error: Unrecognized constraint '$dest1 != $src2' in 'Foo'
def Foo : TestInstructionWithConstraints<"$dest1 != $src2">;
+#endif
+
+#ifdef T1
+// CHECK1: [[FILE]]:[[@LINE+1]]:5: error: Unrecognized constraint '$dest1 == $src2' in 'Foo'
+def Foo : TestInstructionWithConstraints<"$dest1 == $src2">;
+#endif
+
+#ifdef T2
+// CHECK2: [[FILE]]:[[@LINE+1]]:5: error: Illegal format for tied-to constraint in 'Foo': '= $rhs'
+def Foo : TestInstructionWithConstraints<"= $rhs">;
+#endif
+
+#ifdef T3
+// CHECK3: [[FILE]]:[[@LINE+1]]:5: error: Unrecognized constraint '$lhs =' in 'Foo'
+def Foo : TestInstructionWithConstraints<"$lhs =">;
+#endif
+
+#ifdef T4
+// CHECK4: [[FILE]]:[[@LINE+1]]:5: error: Unrecognized constraint '=' in 'Foo'
+def Foo : TestInstructionWithConstraints<"=">;
+#endif
diff --git a/llvm/test/TableGen/ConstraintChecking9.td b/llvm/test/TableGen/ConstraintChecking9.td
deleted file mode 100644
index afd6706fdf4900..00000000000000
--- a/llvm/test/TableGen/ConstraintChecking9.td
+++ /dev/null
@@ -1,8 +0,0 @@
-// RUN: not llvm-tblgen -gen-asm-writer -I %p -I %p/../../include %s 2>&1 | FileCheck %s -DFILE=%s
-
-include "ConstraintChecking.inc"
-
-// Make sure exactly the token "=" appears.
-
-// CHECK: [[FILE]]:[[@LINE+1]]:5: error: Unrecognized constraint '$dest1 == $src2' in 'Foo'
-def Foo : TestInstructionWithConstraints<"$dest1 == $src2">;
>From 937c7ac594886748a859858dd2360966c3b5d1c6 Mon Sep 17 00:00:00 2001
From: Jason Eckhardt <jeckhardt at nvidia.com>
Date: Wed, 20 Mar 2024 13:08:33 -0500
Subject: [PATCH 3/3] Address review comments-- add `pos == 0` check to avoid
relying on unsigned wraparound behavior for the `pos-1` check.
---
llvm/test/TableGen/ConstraintChecking8.td | 2 +-
llvm/utils/TableGen/CodeGenInstruction.cpp | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/llvm/test/TableGen/ConstraintChecking8.td b/llvm/test/TableGen/ConstraintChecking8.td
index 699e7f5b6418ab..37d35150911ef2 100644
--- a/llvm/test/TableGen/ConstraintChecking8.td
+++ b/llvm/test/TableGen/ConstraintChecking8.td
@@ -19,7 +19,7 @@ def Foo : TestInstructionWithConstraints<"$dest1 == $src2">;
#endif
#ifdef T2
-// CHECK2: [[FILE]]:[[@LINE+1]]:5: error: Illegal format for tied-to constraint in 'Foo': '= $rhs'
+// CHECK2: [[FILE]]:[[@LINE+1]]:5: error: Unrecognized constraint '= $rhs' in 'Foo'
def Foo : TestInstructionWithConstraints<"= $rhs">;
#endif
diff --git a/llvm/utils/TableGen/CodeGenInstruction.cpp b/llvm/utils/TableGen/CodeGenInstruction.cpp
index 44b6f2dded681a..18a4e7b0f18b23 100644
--- a/llvm/utils/TableGen/CodeGenInstruction.cpp
+++ b/llvm/utils/TableGen/CodeGenInstruction.cpp
@@ -325,7 +325,8 @@ static void ParseConstraint(StringRef CStr, CGIOperandList &Ops, Record *Rec) {
// Only other constraint is "TIED_TO" for now.
StringRef::size_type pos = CStr.find_first_of('=');
- if (pos == StringRef::npos || CStr.find_first_of(" \t", pos) != (pos + 1) ||
+ if (pos == StringRef::npos || pos == 0 ||
+ CStr.find_first_of(" \t", pos) != (pos + 1) ||
CStr.find_last_of(" \t", pos) != (pos - 1))
PrintFatalError(Rec->getLoc(), "Unrecognized constraint '" + CStr +
"' in '" + Rec->getName() + "'");
More information about the llvm-commits
mailing list