From 7b0d58e339b271e3b1d9dc14b781b57fa0262e3a Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Thu, 16 Jan 2020 14:21:03 -0800 Subject: [PATCH] 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 --- lldb/source/Expression/DWARFExpression.cpp | 16 ++++++++++++---- lldb/unittests/Expression/DWARFExpressionTest.cpp | 23 ++++++++++++++++++++++- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp index 5dc0a7f..6af0f37 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 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 8fad88a..fe5e9c9 100644 --- a/lldb/unittests/Expression/DWARFExpressionTest.cpp +++ b/lldb/unittests/Expression/DWARFExpressionTest.cpp @@ -37,7 +37,22 @@ static llvm::Expected Evaluate(llvm::ArrayRef 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))); +} -- 2.7.4