[Lldb-commits] [lldb] [lldb] Produce generic results for DWARF relational operations (PR #210122)

[via lldb-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: firmiana (firmiana402)

<details>
<summary>Changes</summary>

DWARF relational operations produce a generic result, whose width is the target address size. LLDB currently assigns the C++ `bool` comparison result directly to `Scalar`; because `Scalar` has no `bool` constructor, this creates a 32-bit integer even on 64-bit targets.

The stricter binary operand checks added in #<!-- -->201288 exposed this in Apple's x86-64 `_sigtramp` CFI. A later `DW_OP_plus` combines a 64-bit generic address with the 32-bit relational result, so evaluation fails and signal-frame unwinding stops. The generic-operand relaxation in #<!-- -->209641 does not cover this case because the relational result is narrower than the address size.

Convert the results of `DW_OP_eq`, `DW_OP_ge`, `DW_OP_gt`, `DW_OP_le`, `DW_OP_lt`, and `DW_OP_ne` through the existing `to_generic` helper. This fixes the incorrect result producer without further relaxing binary type checking.

## Testing

Add a unit test that evaluates each relational opcode with an 8-byte address size and then consumes its result with `DW_OP_plus`.

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


2 Files Affected:

- (modified) lldb/source/Expression/DWARFExpression.cpp (+12-6) 
- (modified) lldb/unittests/Expression/DWARFExpressionTest.cpp (+33) 


``````````diff
diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index 5d5c03d81ef1f..299dbe97d6176 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -1724,7 +1724,8 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
         return err;
       tmp = stack.back();
       stack.pop_back();
-      stack.back().GetScalar() = stack.back().GetScalar() == tmp.GetScalar();
+      stack.back().GetScalar() =
+          to_generic(stack.back().GetScalar() == tmp.GetScalar());
       break;
 
     case DW_OP_ge:
@@ -1734,7 +1735,8 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
         return err;
       tmp = stack.back();
       stack.pop_back();
-      stack.back().GetScalar() = stack.back().GetScalar() >= tmp.GetScalar();
+      stack.back().GetScalar() =
+          to_generic(stack.back().GetScalar() >= tmp.GetScalar());
       break;
 
     case DW_OP_gt:
@@ -1744,7 +1746,8 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
         return err;
       tmp = stack.back();
       stack.pop_back();
-      stack.back().GetScalar() = stack.back().GetScalar() > tmp.GetScalar();
+      stack.back().GetScalar() =
+          to_generic(stack.back().GetScalar() > tmp.GetScalar());
       break;
 
     case DW_OP_le:
@@ -1754,7 +1757,8 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
         return err;
       tmp = stack.back();
       stack.pop_back();
-      stack.back().GetScalar() = stack.back().GetScalar() <= tmp.GetScalar();
+      stack.back().GetScalar() =
+          to_generic(stack.back().GetScalar() <= tmp.GetScalar());
       break;
 
     case DW_OP_lt:
@@ -1764,7 +1768,8 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
         return err;
       tmp = stack.back();
       stack.pop_back();
-      stack.back().GetScalar() = stack.back().GetScalar() < tmp.GetScalar();
+      stack.back().GetScalar() =
+          to_generic(stack.back().GetScalar() < tmp.GetScalar());
       break;
 
     case DW_OP_ne:
@@ -1774,7 +1779,8 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
         return err;
       tmp = stack.back();
       stack.pop_back();
-      stack.back().GetScalar() = stack.back().GetScalar() != tmp.GetScalar();
+      stack.back().GetScalar() =
+          to_generic(stack.back().GetScalar() != tmp.GetScalar());
       break;
 
     case DW_OP_lit0:
diff --git a/lldb/unittests/Expression/DWARFExpressionTest.cpp b/lldb/unittests/Expression/DWARFExpressionTest.cpp
index 7dc707ac29a70..6df4553757de4 100644
--- a/lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ b/lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -708,6 +708,39 @@ TEST(DWARFExpression, GenericBinaryOpsAllowDifferentSignedness) {
       ExpectScalar(4));
 }
 
+TEST(DWARFExpression, RelationalOpsProduceGenericResult) {
+  struct TestCase {
+    uint8_t opcode;
+    uint8_t lhs;
+    uint8_t rhs;
+  };
+  constexpr TestCase test_cases[] = {
+      {DW_OP_eq, 3, 3}, {DW_OP_ge, 4, 3}, {DW_OP_gt, 4, 3},
+      {DW_OP_le, 3, 4}, {DW_OP_lt, 3, 4}, {DW_OP_ne, 3, 4},
+  };
+
+  for (const TestCase &test : test_cases) {
+    const std::vector<uint8_t> expr = {
+        DW_OP_lit8,
+        static_cast<uint8_t>(DW_OP_lit0 + test.lhs),
+        static_cast<uint8_t>(DW_OP_lit0 + test.rhs),
+        test.opcode,
+        DW_OP_plus,
+        DW_OP_stack_value,
+    };
+    DataExtractor extractor(expr.data(), expr.size(), lldb::eByteOrderLittle,
+                            /*addr_size=*/8);
+
+    EXPECT_THAT_EXPECTED(
+        DWARFExpression::Evaluate(
+            /*exe_ctx=*/nullptr, /*reg_ctx=*/nullptr, /*module_sp=*/{},
+            extractor, /*unit=*/nullptr, lldb::eRegisterKindLLDB,
+            /*initial_value_ptr=*/nullptr, /*object_address_ptr=*/nullptr),
+        ExpectScalar(64, 9, false))
+        << "opcode 0x" << llvm::utohexstr(test.opcode);
+  }
+}
+
 TEST(DWARFExpression, DW_OP_stack_value) {
   EXPECT_THAT_EXPECTED(Evaluate({DW_OP_stack_value}), llvm::Failed());
 }

``````````

</details>


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