[Lldb-commits] [lldb] 7b0d58e - Add testing for DW_OP_piece and fix a bug with small Scalar values.

[Adrian Prantl via lldb-commits]

Author: Adrian Prantl
Date: 2020-01-16T16:47:36-08:00
New Revision: 7b0d58e339b271e3b1d9dc14b781b57fa0262e3a

URL: https://github.com/llvm/llvm-project/commit/7b0d58e339b271e3b1d9dc14b781b57fa0262e3a
DIFF: https://github.com/llvm/llvm-project/commit/7b0d58e339b271e3b1d9dc14b781b57fa0262e3a.diff

LOG: Add testing for DW_OP_piece and fix a bug with small Scalar values.

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".

Differential Revision: https://reviews.llvm.org/D72879

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index 5dc0a7f876f4..6af0f371d00e 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -2128,7 +2128,8 @@ bool DWARFExpression::Evaluate(
           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 @@ bool DWARFExpression::Evaluate(
                         .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 @@ bool DWARFExpression::Evaluate(
           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 @@ bool DWARFExpression::Evaluate(
             }
           } 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(

diff  --git a/lldb/unittests/Expression/DWARFExpressionTest.cpp b/lldb/unittests/Expression/DWARFExpressionTest.cpp
index 8fad88a93e11..fe5e9c957ba0 100644
--- a/lldb/unittests/Expression/DWARFExpressionTest.cpp
+++ b/lldb/unittests/Expression/DWARFExpressionTest.cpp
@@ -37,7 +37,22 @@ static llvm::Expected<Scalar> Evaluate(llvm::ArrayRef<uint8_t> expr,
           /*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 @@ TEST(DWARFExpression, DW_OP_convert) {
       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)));
+}