[llvm] Avoid undefined behavior in shift operators during constant folding of DIExpressions. (PR #116466)

via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 15 21:48:00 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-ir

Author: Tom Honermann (tahonermann)

<details>
<summary>Changes</summary>

Bit shift operations with a shift operand greater than or equal to the bit width of the (promoted) value type result in undefined behavior according to C++ [\[expr.shift\]p1](https://eel.is/c++draft/expr.shift#<!-- -->1). This change adds checking for this situation and avoids attempts to constant fold `DIExpressions` that would otherwise provoke such behavior. An existing test that presumably intended to exercise shifts at the UB boundary has been updated; it now checks for shifts of 64 bits instead of 65. This issue was reported by a static analysis tool; no actual cases of shift operations that would result in undefined behavior in practice have been identified.

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


2 Files Affected:

- (modified) llvm/lib/IR/DIExpressionOptimizer.cpp (+4-2) 
- (modified) llvm/unittests/IR/MetadataTest.cpp (+6-6) 


``````````diff
diff --git a/llvm/lib/IR/DIExpressionOptimizer.cpp b/llvm/lib/IR/DIExpressionOptimizer.cpp
index 2bb8eac348c8e9..5bd18dcf049060 100644
--- a/llvm/lib/IR/DIExpressionOptimizer.cpp
+++ b/llvm/lib/IR/DIExpressionOptimizer.cpp
@@ -59,12 +59,14 @@ foldOperationIfPossible(uint64_t Const1, uint64_t Const2,
     return Const1 - Const2;
   }
   case dwarf::DW_OP_shl: {
-    if ((uint64_t)countl_zero(Const1) < Const2)
+    if (Const2 >= std::numeric_limits<uint64_t>::digits ||
+        (uint64_t)countl_zero(Const1) < Const2)
       return std::nullopt;
     return Const1 << Const2;
   }
   case dwarf::DW_OP_shr: {
-    if ((uint64_t)countr_zero(Const1) < Const2)
+    if (Const2 >= std::numeric_limits<uint64_t>::digits ||
+        (uint64_t)countr_zero(Const1) < Const2)
       return std::nullopt;
     return Const1 >> Const2;
   }
diff --git a/llvm/unittests/IR/MetadataTest.cpp b/llvm/unittests/IR/MetadataTest.cpp
index fbdab1975df725..628221339c89bf 100644
--- a/llvm/unittests/IR/MetadataTest.cpp
+++ b/llvm/unittests/IR/MetadataTest.cpp
@@ -3541,12 +3541,12 @@ TEST_F(DIExpressionTest, Fold) {
   ResExpr = DIExpression::get(Context, ResOps);
   EXPECT_EQ(E, ResExpr);
 
-  // Test a left shift greater than 64.
+  // Test a left shift greater than 63.
   Ops.clear();
   Ops.push_back(dwarf::DW_OP_constu);
   Ops.push_back(1);
   Ops.push_back(dwarf::DW_OP_constu);
-  Ops.push_back(65);
+  Ops.push_back(64);
   Ops.push_back(dwarf::DW_OP_shl);
   Expr = DIExpression::get(Context, Ops);
   E = Expr->foldConstantMath();
@@ -3554,17 +3554,17 @@ TEST_F(DIExpressionTest, Fold) {
   ResOps.push_back(dwarf::DW_OP_constu);
   ResOps.push_back(1);
   ResOps.push_back(dwarf::DW_OP_constu);
-  ResOps.push_back(65);
+  ResOps.push_back(64);
   ResOps.push_back(dwarf::DW_OP_shl);
   ResExpr = DIExpression::get(Context, ResOps);
   EXPECT_EQ(E, ResExpr);
 
-  // Test a right shift greater than 64.
+  // Test a right shift greater than 63.
   Ops.clear();
   Ops.push_back(dwarf::DW_OP_constu);
   Ops.push_back(1);
   Ops.push_back(dwarf::DW_OP_constu);
-  Ops.push_back(65);
+  Ops.push_back(64);
   Ops.push_back(dwarf::DW_OP_shr);
   Expr = DIExpression::get(Context, Ops);
   E = Expr->foldConstantMath();
@@ -3572,7 +3572,7 @@ TEST_F(DIExpressionTest, Fold) {
   ResOps.push_back(dwarf::DW_OP_constu);
   ResOps.push_back(1);
   ResOps.push_back(dwarf::DW_OP_constu);
-  ResOps.push_back(65);
+  ResOps.push_back(64);
   ResOps.push_back(dwarf::DW_OP_shr);
   ResExpr = DIExpression::get(Context, ResOps);
   EXPECT_EQ(E, ResExpr);

``````````

</details>


https://github.com/llvm/llvm-project/pull/116466


More information about the llvm-commits mailing list