Skip to content

Conversation

@topperc
Copy link
Collaborator

@topperc topperc commented Mar 21, 2025

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

The C operator has undefined behavior for out of bounds shifts
so we should check this.
@topperc topperc requested a review from jurahul March 21, 2025 23:38
@llvmbot
Copy link
Member

llvmbot commented Mar 21, 2025

@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
Copy link

github-actions bot commented Mar 21, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@jurahul
Copy link
Contributor

jurahul commented Mar 22, 2025

Can you also add unit test?

@topperc topperc requested a review from preames August 12, 2025 22:05
@topperc
Copy link
Collaborator Author

topperc commented Aug 12, 2025

Can you also add unit test?

Done

@topperc
Copy link
Collaborator Author

topperc commented Aug 22, 2025

Ping

Copy link
Contributor

@s-barannikov s-barannikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@topperc topperc merged commit dee25a8 into llvm:main Aug 22, 2025
6 of 8 checks passed
@topperc topperc deleted the pr/tablegen-shift branch August 22, 2025 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants