[lldb] Zero extend APInt when piece size is bigger than the bitwidth #150149

satyajanga · 2025-07-23T00:58:02Z

Summary

We have internally seen cases like this
DW_OP_lit0, DW_OP_stack_value, DW_OP_piece 0x28
where we have longer op pieces than what Scalar supports (32, 64 or 128 bits). In these cases LLDB is currently hitting the assertion assert(ap_int.getBitWidth() >= bit_size);

We are extending the generated APInt to the piece size by filling zeros.

Test plan

Added a unit to cover this case.

Reviewers

@clayborg , @jeffreytan81 , @Jlalond

llvmbot · 2025-07-23T05:55:20Z

@llvm/pr-subscribers-lldb

Author: satyanarayana reddy janga (satyajanga)

Changes

Summary

We have internally seen cases like this
DW_OP_lit0, DW_OP_stack_value, DW_OP_piece 0x28
where we have longer op pieces than what Scalar supports (32, 64 or 128 bits). In these cases LLDB is currently hitting the assertion assert(ap_int.getBitWidth() >= bit_size);

We are extending the generated APInt to the piece size by filling zeros.

Test plan

Added a unit to cover this case.

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

2 Files Affected:

(modified) lldb/source/Expression/DWARFExpression.cpp (+6-1)
(modified) lldb/unittests/Expression/DWARFExpressionTest.cpp (+21)

diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index 52891fcefd68b..c00795b97467b 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -1978,7 +1978,12 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
             // grows to the nearest host integer type.
             llvm::APInt fail_value(1, 0, false);
             llvm::APInt ap_int = scalar.UInt128(fail_value);
-            assert(ap_int.getBitWidth() >= bit_size);
+            // We have seen a case where we have expression like:
+            //      DW_OP_lit0, DW_OP_stack_value, DW_OP_piece 0x28
+            // here we are assuming the compiler was trying to zero
+            // extend the value that we should append to the buffer.
+            if (ap_int.getBitWidth() < bit_size)
+              ap_int = ap_int.zext(bit_size);
             llvm::ArrayRef<uint64_t> buf{ap_int.getRawData(),
                                          ap_int.getNumWords()};
             curr_piece.GetScalar() = Scalar(llvm::APInt(bit_size, buf));
diff --git a/lldb/unittests/Expression/DWARFExpressionTest.cpp b/lldb/unittests/Expression/DWARFExpressionTest.cpp
index fdc9bfae1876c..86c3b56e320fd 100644
--- a/lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ b/lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -358,6 +358,27 @@ TEST(DWARFExpression, DW_OP_piece) {
       llvm::HasValue(GetScalar(16, 0xff00, true)));
 }
 
+TEST(DWARFExpression, DW_OP_piece_host_address) {
+  static const uint8_t expr_data[] = {DW_OP_lit2, DW_OP_stack_value,
+                                      DW_OP_piece, 40};
+  llvm::ArrayRef<uint8_t> expr(expr_data, sizeof(expr_data));
+  DataExtractor extractor(expr.data(), expr.size(), lldb::eByteOrderLittle, 4);
+
+  // This tests if ap_int is extended to the right width.
+  // expect 40*8 = 320 bits size.
+  llvm::Expected<Value> result =
+      DWARFExpression::Evaluate(nullptr, nullptr, nullptr, extractor, nullptr,
+                                lldb::eRegisterKindDWARF, nullptr, nullptr);
+  ASSERT_THAT_EXPECTED(result, llvm::Succeeded());
+  ASSERT_EQ(result->GetValueType(), Value::ValueType::HostAddress);
+  ASSERT_EQ(result->GetBuffer().GetByteSize(), 40ul);
+  const uint8_t *data = result->GetBuffer().GetBytes();
+  ASSERT_EQ(data[0], 2);
+  for (int i = 1; i < 40; i++) {
+    ASSERT_EQ(data[i], 0);
+  }
+}
+
 TEST(DWARFExpression, DW_OP_implicit_value) {
   unsigned char bytes = 4;

Jlalond · 2025-07-23T16:41:39Z

lldb/source/Expression/DWARFExpression.cpp

+            //      DW_OP_lit0, DW_OP_stack_value, DW_OP_piece 0x28
+            // here we are assuming the compiler was trying to zero
+            // extend the value that we should append to the buffer.
+            if (ap_int.getBitWidth() < bit_size)


Nit, isn't this effectively just max(bit_Size, ap_int.getBitWidth())

No, APInt and APSInt classes can handle any sized integers.

This kind of expression seems to want to zero fill in the value.

clayborg

I am good with this. We should wait for anyone else to chime in for a day or two.

clayborg · 2025-07-24T17:20:05Z

lldb/source/Expression/DWARFExpression.cpp

+            //      DW_OP_lit0, DW_OP_stack_value, DW_OP_piece 0x28
+            // here we are assuming the compiler was trying to zero
+            // extend the value that we should append to the buffer.
+            if (ap_int.getBitWidth() < bit_size)


No, APInt and APSInt classes can handle any sized integers.

clayborg · 2025-07-24T17:20:49Z

lldb/source/Expression/DWARFExpression.cpp

+            //      DW_OP_lit0, DW_OP_stack_value, DW_OP_piece 0x28
+            // here we are assuming the compiler was trying to zero
+            // extend the value that we should append to the buffer.
+            if (ap_int.getBitWidth() < bit_size)


This kind of expression seems to want to zero fill in the value.

clayborg · 2025-07-24T17:25:31Z

I added Pavel Labath and David Blaikie in case they have any info on this.

The story if we have some RiscV code that is creating ___location values for variables that use DW_OP_piece and they are using:

DW_OP_lit0, DW_OP_stack_value, DW_OP_piece 0x28

to try and zero fill in the value. Not sure if this is common or if there is a better way to do the zero fill. But clang is producing this. as a series of DW_OP_constu XXX, DW_OP_stack_value, DW_OP_piece 0x8 followed by the above expression.

labath

I'm not sure that what clang is doing is completely compliant. According to the standard, DWARF expression values """can represent a value of any supported base type of the target machine. Instead of a base type, elements can have a generic type, which is an integral type that has the size of an address on the target machine and unspecified signedness""", so using a single value to represent 40 bytes is seems a bit dodgy. It's kind of obvious what you mean if the value is zero, but it gets a bit fuzzy for other values (where do you place that value, do you sign-extend it, etc.)

Nevertheless, looking at this from the consumer side, I think what you've done is a reasonable interpretation of this, and is better than crashing. I'm just wondering if there isn't a simpler way to express this. See the inline comment about a possible simplification.

labath · 2025-07-25T09:43:02Z

lldb/source/Expression/DWARFExpression.cpp

            llvm::APInt fail_value(1, 0, false);
            llvm::APInt ap_int = scalar.UInt128(fail_value);
-            assert(ap_int.getBitWidth() >= bit_size);
+            // We have seen a case where we have expression like:
+            //      DW_OP_lit0, DW_OP_stack_value, DW_OP_piece 0x28
+            // here we are assuming the compiler was trying to zero
+            // extend the value that we should append to the buffer.
+            if (ap_int.getBitWidth() < bit_size)
+              ap_int = ap_int.zext(bit_size);
            llvm::ArrayRef<uint64_t> buf{ap_int.getRawData(),
                                         ap_int.getNumWords()};
            curr_piece.GetScalar() = Scalar(llvm::APInt(bit_size, buf));


Isn't all of this equivalent to curr_piece.GetScalar() = scalar.TruncOrExtendTo(bit_size, /*sign=*/false)?

Isn't all of this equivalent to curr_piece.GetScalar() = scalar.TruncOrExtendTo(bit_size, /*sign=*/false)?

Yes. it seems so.
if the scalar has float value set then it wont work, but not sure if that case ever happens.
@labath do you know if the scalar contains a float value in the dwarf expression followed by DW_OP_Piece?

I don't know, but I'm not particularly worried by that given that the only other call of this function is also inside DWARFExpression (in DW_OP_convert). If that becomes an issue, we can address both cases together.

I don't know, but I'm not particularly worried by that given that the only other call of this function is also inside DWARFExpression (in DW_OP_convert). If that becomes an issue, we can address both cases together.

scalar.UInt128(fail_value) seems to robust to handle all the edge cases. So I am leaving the way it is. let me know if you think otherwise.

I think these two cases should be unified I don't see any reason why extension (or truncation) in DW_OP_piece should behave any differently from DW_OP_convert. And I don't like how this violates "campground rules" (leave the place in a better state than you found it in): instead of cleaning things up, it piles onto the existing dodgy implementation.

I agree. updated based on the recommendation.

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D78791142

labath

Thanks.

Jlalond · 2025-08-04T16:42:39Z

Merging for @satyajanga

satyajanga force-pushed the dw_op_piece_zest branch 4 times, most recently from 0431e9a to 3cec63f Compare July 23, 2025 05:30

satyajanga marked this pull request as ready for review July 23, 2025 05:54

satyajanga requested a review from JDevlieghere as a code owner July 23, 2025 05:54

llvmbot added the lldb label Jul 23, 2025

nikic changed the title ~~Zero extend APInt when piece size is bigger than the bitwidth~~ [lldb] Zero extend APInt when piece size is bigger than the bitwidth Jul 23, 2025

Jlalond reviewed Jul 23, 2025

View reviewed changes

clayborg approved these changes Jul 24, 2025

View reviewed changes

clayborg requested review from labath and dwblaikie July 24, 2025 17:21

labath reviewed Jul 25, 2025

View reviewed changes

clayborg approved these changes Jul 28, 2025

View reviewed changes

Zero extend APInt when piece size is bigger than the bitwidth

f369157

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D78791142

satyajanga force-pushed the dw_op_piece_zest branch from 3cec63f to f369157 Compare July 31, 2025 19:21

labath approved these changes Aug 4, 2025

View reviewed changes

Jlalond merged commit a0db29d into llvm:main Aug 4, 2025
9 checks passed

[lldb] Zero extend APInt when piece size is bigger than the bitwidth #150149

[lldb] Zero extend APInt when piece size is bigger than the bitwidth #150149

Uh oh!

Conversation

satyajanga commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Reviewers

Uh oh!

llvmbot commented Jul 23, 2025

Summary

Test plan

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clayborg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clayborg commented Jul 24, 2025

Uh oh!

labath left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

labath Jul 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

labath left a comment

Choose a reason for hiding this comment

Uh oh!

Jlalond commented Aug 4, 2025

Uh oh!

Uh oh!

Uh oh!

satyajanga commented Jul 23, 2025 •

edited

Loading

labath Jul 28, 2025 •

edited

Loading