[Lldb-commits] [PATCH] D72879: Add testing for DW_OP_piece and fix a bug with small Scalar values.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl created this revision.
aprantl added reviewers: davide, friss, jasonmolenda.
Herald added a project: LLDB.

By switching to Scalars that are backed by explicitly-sized APInts we can avoid a bug that increases the buffer reserved for a small piece to the next-largest host integer type.

      

This manifests as "DW_OP_piece for offset foo but top of stack is of size bar".


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72879

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/unittests/Expression/DWARFExpressionTest.cpp


Index: lldb/unittests/Expression/DWARFExpressionTest.cpp
===================================================================
--- lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -37,7 +37,22 @@
           /*object_address_ptr*/ nullptr, result, &status))
     return status.ToError();
 
-  return result.GetScalar();
+  switch (result.GetValueType()) {
+  case Value::eValueTypeScalar:
+    return result.GetScalar();
+  case Value::eValueTypeHostAddress: {
+    // Convert small buffers to scalars to simplify the tests.
+    DataBufferHeap &buf = result.GetBuffer();
+    if (buf.GetByteSize() <= 8) {
+      uint64_t val = 0;
+      memcpy(&val, buf.GetBytes(), buf.GetByteSize());
+      return Scalar(llvm::APInt(buf.GetByteSize()*8, val, false));
+    }
+  }
+    LLVM_FALLTHROUGH;
+  default:
+    return status.ToError();
+  }
 }
 
 /// A mock module holding an object file parsed from YAML.
@@ -335,3 +350,9 @@
       t.Eval({DW_OP_const1s, 'X', DW_OP_convert, 0x1d}).takeError(),
       llvm::Failed());
 }
+
+TEST(DWARFExpression, DW_OP_piece) {
+  EXPECT_THAT_EXPECTED(Evaluate({DW_OP_const2u, 0x11, 0x22, DW_OP_piece, 2,
+                                 DW_OP_const2u, 0x33, 0x44, DW_OP_piece, 2}),
+                       llvm::HasValue(GetScalar(32, 0x44332211, true)));
+}
Index: lldb/source/Expression/DWARFExpression.cpp
===================================================================
--- lldb/source/Expression/DWARFExpression.cpp
+++ lldb/source/Expression/DWARFExpression.cpp
@@ -2128,7 +2128,8 @@
           case Value::eValueTypeScalar: {
             uint32_t bit_size = piece_byte_size * 8;
             uint32_t bit_offset = 0;
-            if (!curr_piece_source_value.GetScalar().ExtractBitfield(
+            Scalar &scalar = curr_piece_source_value.GetScalar();
+            if (!scalar.ExtractBitfield(
                     bit_size, bit_offset)) {
               if (error_ptr)
                 error_ptr->SetErrorStringWithFormat(
@@ -2139,7 +2140,14 @@
                         .GetByteSize());
               return false;
             }
-            curr_piece = curr_piece_source_value;
+            // Create curr_piece with bit_size. By default Scalar
+            // 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);
+            llvm::ArrayRef<uint64_t> buf{ap_int.getRawData(),
+                                         ap_int.getNumWords()};
+            curr_piece.GetScalar() = Scalar(llvm::APInt(bit_size, buf));
           } break;
 
           case Value::eValueTypeVector: {
@@ -2161,7 +2169,7 @@
           if (op_piece_offset == 0) {
             // This is the first piece, we should push it back onto the stack
             // so subsequent pieces will be able to access this piece and add
-            // to it
+            // to it.
             if (pieces.AppendDataToHostBuffer(curr_piece) == 0) {
               if (error_ptr)
                 error_ptr->SetErrorString("failed to append piece data");
@@ -2169,7 +2177,7 @@
             }
           } else {
             // If this is the second or later piece there should be a value on
-            // the stack
+            // the stack.
             if (pieces.GetBuffer().GetByteSize() != op_piece_offset) {
               if (error_ptr)
                 error_ptr->SetErrorStringWithFormat(


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72879.238623.patch
Type: text/x-patch
Size: 3526 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200116/dd0692ca/attachment-0001.bin>