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

Tom Honermann via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 15 19:39:14 PST 2024


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

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.

>From 57f9ef9ed97c2031aacd74655f160d16e3770081 Mon Sep 17 00:00:00 2001
From: Tom Honermann <tom.honermann at intel.com>
Date: Fri, 15 Nov 2024 19:19:24 -0800
Subject: [PATCH] Avoid undefined behavior in shift operators during constant
 folding of DIExpressions.

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. 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.
---
 llvm/lib/IR/DIExpressionOptimizer.cpp |  6 ++++--
 llvm/unittests/IR/MetadataTest.cpp    | 12 ++++++------
 2 files changed, 10 insertions(+), 8 deletions(-)

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);



More information about the llvm-commits mailing list