[TableGen] Validate the shift amount for !srl, !shl, and !sra operators.

[topperc]

The C operator has undefined behavior for out of bounds shifts so we should check this.

[llvmbot]

@llvm/pr-subscribers-tablegen

Author: Craig Topper (topperc)

Changes

The C operator has undefined behavior for out of bounds shifts so we should check this.

---

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

1 Files Affected:

• (modified) llvm/lib/TableGen/Record.cpp (+8-2)

diff --git a/llvm/lib/TableGen/Record.cpp b/llvm/lib/TableGen/Record.cpp
index c5b9b670b6f42..655c4078697f3 100644
--- a/llvm/lib/TableGen/Record.cpp
+++ b/llvm/lib/TableGen/Record.cpp
@@ -1537,7 +1537,13 @@ const Init *BinOpInit::Fold(const Record *CurRec) const {
     if (LHSi && RHSi) {
       int64_t LHSv = LHSi->getValue(), RHSv = RHSi->getValue();
       int64_t Result;
-      switch (getOpcode()) {
+
+      unsigned Opc = getOpcode();
+      if ((Opc == SHL || Opc == SRA || Opc == SRL) && (RHSv < 0 || RHSv >= 64))
+        PrintFatalError(CurRec->getLoc(),
+                        "Illegal operation: out of bounds shift");
+
+      switch (Opc) {
       default: llvm_unreachable("Bad opcode!");
       case ADD: Result = LHSv + RHSv; break;
       case SUB: Result = LHSv - RHSv; break;
@@ -1556,7 +1562,7 @@ const Init *BinOpInit::Fold(const Record *CurRec) const {
       case OR:  Result = LHSv | RHSv; break;
       case XOR: Result = LHSv ^ RHSv; break;
       case SHL: Result = (uint64_t)LHSv << (uint64_t)RHSv; break;
-      case SRA: Result = LHSv >> RHSv; break;
+      case SRA: Result = LHSv >> (uint64_t)RHSv; break;
       case SRL: Result = (uint64_t)LHSv >> (uint64_t)RHSv; break;
       }
       return IntInit::get(getRecordKeeper(), Result);

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff b858ba0f6597c66e5c276ca9e2564ca27e7e28e7 4277124e8acb8efed73dee9bc32a1744c807dcdf --extensions cpp -- llvm/lib/TableGen/Record.cpp

View the diff from clang-format here.

diff --git a/llvm/lib/TableGen/Record.cpp b/llvm/lib/TableGen/Record.cpp
index 655c407869..78f9f2368d 100644
--- a/llvm/lib/TableGen/Record.cpp
+++ b/llvm/lib/TableGen/Record.cpp
@@ -1562,7 +1562,9 @@ const Init *BinOpInit::Fold(const Record *CurRec) const {
       case OR:  Result = LHSv | RHSv; break;
       case XOR: Result = LHSv ^ RHSv; break;
       case SHL: Result = (uint64_t)LHSv << (uint64_t)RHSv; break;
-      case SRA: Result = LHSv >> (uint64_t)RHSv; break;
+      case SRA:
+        Result = LHSv >> (uint64_t)RHSv;
+        break;
       case SRL: Result = (uint64_t)LHSv >> (uint64_t)RHSv; break;
       }
       return IntInit::get(getRecordKeeper(), Result);

[jurahul]

Can you also add unit test?

[preames]

While the duplication is mildly ugly, this would seem more natural if sunk into the case blocks below.

[preames]

This change should be a nop given the newly added bounds check, right?