[Lldb-commits] [lldb] [lldb] Accept DW_OP_bit_piece with an empty location description (PR #238206)

[via lldb-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: firmiana (firmiana402)

<details>
<summary>Changes</summary>

DWARF 5 section 2.6.1.2 defines an empty location followed by `DW_OP_bit_piece` as an unavailable fragment, with the source offset ignored. LLDB currently rejects it as a stack underflow.

Append the fragment using the existing composite assembler, retaining the same zero-filled placeholder used by `DW_OP_piece`. Per-bit availability tracking remains a separate issue (#<!-- -->202897). Add regression coverage for empty fragments, ignored offsets, zero-sized fragments, and unaligned composites in both byte orders.

Fixes #<!-- -->208888.

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


2 Files Affected:

- (modified) lldb/source/Expression/DWARFExpression.cpp (+14-2) 
- (modified) lldb/unittests/Expression/DWARFExpressionTest.cpp (+40-1) 


``````````diff
diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index 9857a4481220f..8b0d6e2e62131 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -2160,8 +2160,20 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
                                                LocationDescriptionKind::Empty);
         // Reset for the next piece.
         eval_ctx.loc_desc_kind = Memory;
-        return llvm::createStringError(
-            "expression stack needs at least 1 item for DW_OP_bit_piece");
+        const uint64_t piece_bit_size = op->getRawOperand(0);
+        std::optional<uint64_t> piece_byte_size = BitsToBytes(piece_bit_size);
+        if (!piece_byte_size ||
+            *piece_byte_size > std::numeric_limits<size_t>::max())
+          return llvm::createStringError("DW_OP_bit_piece size overflow");
+
+        // Match DW_OP_piece's zero-filled placeholder for unavailable bits.
+        Value curr_piece;
+        if (curr_piece.ResizeData(*piece_byte_size) != *piece_byte_size)
+          return llvm::createStringError(
+              "failed to resize DW_OP_bit_piece buffer");
+        if (llvm::Error err =
+                AppendPieceToComposite(eval_ctx, curr_piece, piece_bit_size))
+          return err;
       } else {
         const LocationDescriptionKind piece_locdesc = eval_ctx.loc_desc_kind;
         UpdateValueTypeFromLocationDescription(eval_ctx, piece_locdesc,
diff --git a/lldb/unittests/Expression/DWARFExpressionTest.cpp b/lldb/unittests/Expression/DWARFExpressionTest.cpp
index be2268f7bed79..5f060493b2c94 100644
--- a/lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ b/lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -1526,7 +1526,46 @@ TEST(DWARFExpression, DW_OP_bit_piece_offset_overflow) {
 }
 
 TEST(DWARFExpression, DW_OP_bit_piece_empty_stack) {
-  EXPECT_THAT_EXPECTED(Evaluate({DW_OP_bit_piece, 0x08, 0x00}), llvm::Failed());
+  // As with DW_OP_piece, zeros stand in for unavailable bits for now.
+  EXPECT_THAT_EXPECTED(Evaluate({DW_OP_bit_piece, 8, 0}),
+                       ExpectHostAddress({0x00}));
+
+  // The source offset is ignored, and the piece need not fit in a scalar.
+  EXPECT_THAT_EXPECTED(Evaluate({DW_OP_bit_piece, 99, 127}),
+                       ExpectHostAddress(std::vector<uint8_t>(13, 0)));
+
+  // Empty zero-sized pieces do not contribute any data.
+  EXPECT_THAT_EXPECTED(
+      Evaluate({DW_OP_bit_piece, 0, 127, DW_OP_lit7, DW_OP_stack_value,
+                DW_OP_bit_piece, 8, 0, DW_OP_bit_piece, 0, 127}),
+      ExpectHostAddress({0x07}));
+
+  MockDwarfDelegate dwarf5 = MockDwarfDelegate::Dwarf5();
+  for (lldb::ByteOrder byte_order :
+       {lldb::eByteOrderLittle, lldb::eByteOrderBig}) {
+    // Unavailable nibbles surround a byte-sized piece at an unaligned offset.
+    const uint8_t expr[] = {DW_OP_bit_piece,
+                            4,
+                            127,
+                            DW_OP_const1u,
+                            0xab,
+                            DW_OP_stack_value,
+                            DW_OP_piece,
+                            1,
+                            DW_OP_bit_piece,
+                            4,
+                            127};
+    DataExtractor extractor(expr, sizeof(expr), byte_order, /*addr_size=*/4);
+    const std::vector<uint8_t> expected =
+        byte_order == lldb::eByteOrderLittle ? std::vector<uint8_t>{0xb0, 0x0a}
+                                             : std::vector<uint8_t>{0x0a, 0xb0};
+    EXPECT_THAT_EXPECTED(
+        DWARFExpression::Evaluate(
+            /*exe_ctx=*/nullptr, /*reg_ctx=*/nullptr, /*module_sp=*/{},
+            extractor, /*dwarf_cu=*/&dwarf5, lldb::eRegisterKindLLDB,
+            /*initial_value_ptr=*/nullptr, /*object_address_ptr=*/nullptr),
+        ExpectHostAddress(expected));
+  }
 }
 
 TEST(DWARFExpression, DW_OP_deref_size_exceeds_address_size) {

``````````

</details>


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