[Lldb-commits] [lldb] 956f5c5 - [lldb] Use SmallVector for handling register data
David Spickett via lldb-commits
lldb-commits at lists.llvm.org
Tue Jun 27 02:15:21 PDT 2023
Author: David Spickett
Date: 2023-06-27T09:15:12Z
New Revision: 956f5c5f6de88af7d0c9844ab40979bb45a51ad4
URL: https://github.com/llvm/llvm-project/commit/956f5c5f6de88af7d0c9844ab40979bb45a51ad4
DIFF: https://github.com/llvm/llvm-project/commit/956f5c5f6de88af7d0c9844ab40979bb45a51ad4.diff
LOG: [lldb] Use SmallVector for handling register data
Previously lldb was using arrays of size kMaxRegisterByteSize to handle
registers. This was set to 256 because the largest possible register
we support is Arm's scalable vectors (SVE) which can be up to 256 bytes long.
This means for most operations aside from SVE, we're wasting 192 bytes
of it. Which is ok given that we don't have to pay the cost of a heap
alocation and 256 bytes isn't all that much overall.
With the introduction of the Arm Scalable Matrix extension there is a new
array storage register, ZA. This register is essentially a square made up of
SVE vectors. Therefore ZA could be up to 64kb in size.
https://developer.arm.com/documentation/ddi0616/latest/
"The Effective Streaming SVE vector length, SVL, is a power of two in the range 128 to 2048 bits inclusive."
"The ZA storage is architectural register state consisting of a two-dimensional ZA array of [SVLB × SVLB] bytes."
99% of operations will never touch ZA and making every stack frame 64kb+ just
for that slim chance is a bad idea.
Instead I'm switching register handling to use SmallVector with a stack allocation
size of kTypicalRegisterByteSize. kMaxRegisterByteSize will be used in places
where we can't predict the size of register we're reading (in the GDB remote client).
The result is that the 99% of small register operations can use the stack
as before and the actual ZA operations will move to the heap as needed.
I tested this by first working out -wframe-larger-than values for all the
libraries using the arrays previously. With this change I was able to increase
kMaxRegisterByteSize to 256*256 without hitting those limits. With the
exception of the GDB server which needs to use a max size buffer.
Reviewed By: JDevlieghere
Differential Revision: https://reviews.llvm.org/D153626
Added:
Modified:
lldb/include/lldb/Utility/RegisterValue.h
lldb/source/Host/common/NativeRegisterContext.cpp
lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/source/Target/RegisterContext.cpp
lldb/source/Utility/RegisterValue.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Utility/RegisterValue.h b/lldb/include/lldb/Utility/RegisterValue.h
index ff3dc9b3dda42..a86767dfaf512 100644
--- a/lldb/include/lldb/Utility/RegisterValue.h
+++ b/lldb/include/lldb/Utility/RegisterValue.h
@@ -15,6 +15,7 @@
#include "lldb/lldb-enumerations.h"
#include "lldb/lldb-types.h"
#include "llvm/ADT/APInt.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include <cstdint>
#include <cstring>
@@ -27,8 +28,15 @@ struct RegisterInfo;
class RegisterValue {
public:
- // big enough to support up to 256 byte AArch64 SVE
- enum { kMaxRegisterByteSize = 256u };
+ enum {
+ // What we can reasonably put on the stack, big enough to support up to 256
+ // byte AArch64 SVE.
+ kTypicalRegisterByteSize = 256u,
+ // Anything else we'll heap allocate storage for it.
+ kMaxRegisterByteSize = kTypicalRegisterByteSize,
+ };
+
+ typedef llvm::SmallVector<uint8_t, kTypicalRegisterByteSize> BytesContainer;
enum Type {
eTypeInvalid,
@@ -251,19 +259,17 @@ class RegisterValue {
uint32_t GetByteSize() const;
- static uint32_t GetMaxByteSize() { return kMaxRegisterByteSize; }
-
void Clear();
protected:
RegisterValue::Type m_type = eTypeInvalid;
Scalar m_scalar;
- struct {
- mutable uint8_t
- bytes[kMaxRegisterByteSize]; // This must be big enough to hold any
- // register for any supported target.
- uint16_t length = 0;
+ struct RegisterValueBuffer {
+ // Start at max stack storage size. Move to the heap for anything larger.
+ RegisterValueBuffer() : bytes(kTypicalRegisterByteSize) {}
+
+ mutable BytesContainer bytes;
lldb::ByteOrder byte_order = lldb::eByteOrderInvalid;
} buffer;
};
diff --git a/lldb/source/Host/common/NativeRegisterContext.cpp b/lldb/source/Host/common/NativeRegisterContext.cpp
index 411f5f5c92ed0..40a57f3d5c823 100644
--- a/lldb/source/Host/common/NativeRegisterContext.cpp
+++ b/lldb/source/Host/common/NativeRegisterContext.cpp
@@ -331,11 +331,6 @@ Status NativeRegisterContext::ReadRegisterValueFromMemory(
// |AABB| Address contents
// |AABB0000| Register contents [on little-endian hardware]
// |0000AABB| Register contents [on big-endian hardware]
- if (src_len > RegisterValue::kMaxRegisterByteSize) {
- error.SetErrorString("register too small to receive memory data");
- return error;
- }
-
const size_t dst_len = reg_info->byte_size;
if (src_len > dst_len) {
@@ -348,11 +343,11 @@ Status NativeRegisterContext::ReadRegisterValueFromMemory(
}
NativeProcessProtocol &process = m_thread.GetProcess();
- uint8_t src[RegisterValue::kMaxRegisterByteSize];
+ RegisterValue::BytesContainer src(src_len);
// Read the memory
size_t bytes_read;
- error = process.ReadMemory(src_addr, src, src_len, bytes_read);
+ error = process.ReadMemory(src_addr, src.data(), src_len, bytes_read);
if (error.Fail())
return error;
@@ -370,8 +365,8 @@ Status NativeRegisterContext::ReadRegisterValueFromMemory(
// TODO: we might need to add a parameter to this function in case the byte
// order of the memory data doesn't match the process. For now we are
// assuming they are the same.
- reg_value.SetFromMemoryData(*reg_info, src, src_len, process.GetByteOrder(),
- error);
+ reg_value.SetFromMemoryData(*reg_info, src.data(), src_len,
+ process.GetByteOrder(), error);
return error;
}
@@ -385,21 +380,22 @@ Status NativeRegisterContext::WriteRegisterValueToMemory(
return error;
}
- uint8_t dst[RegisterValue::kMaxRegisterByteSize];
+ RegisterValue::BytesContainer dst(dst_len);
NativeProcessProtocol &process = m_thread.GetProcess();
// TODO: we might need to add a parameter to this function in case the byte
// order of the memory data doesn't match the process. For now we are
// assuming they are the same.
const size_t bytes_copied = reg_value.GetAsMemoryData(
- *reg_info, dst, dst_len, process.GetByteOrder(), error);
+ *reg_info, dst.data(), dst_len, process.GetByteOrder(), error);
if (error.Success()) {
if (bytes_copied == 0) {
error.SetErrorString("byte copy failed.");
} else {
size_t bytes_written;
- error = process.WriteMemory(dst_addr, dst, bytes_copied, bytes_written);
+ error = process.WriteMemory(dst_addr, dst.data(), bytes_copied,
+ bytes_written);
if (error.Fail())
return error;
diff --git a/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp b/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
index b54f6846db3b6..f4ef9b4fc824b 100644
--- a/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
+++ b/lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp
@@ -303,26 +303,17 @@ ABIMacOSX_arm64::SetReturnValueObject(lldb::StackFrameSP &frame_sp,
if (v0_info) {
if (byte_size <= 16) {
- if (byte_size <= RegisterValue::GetMaxByteSize()) {
- RegisterValue reg_value;
- error = reg_value.SetValueFromData(*v0_info, data, 0, true);
- if (error.Success()) {
- if (!reg_ctx->WriteRegister(v0_info, reg_value))
- error.SetErrorString("failed to write register v0");
- }
- } else {
- error.SetErrorStringWithFormat(
- "returning float values with a byte size of %" PRIu64
- " are not supported",
- byte_size);
- }
+ RegisterValue reg_value;
+ error = reg_value.SetValueFromData(*v0_info, data, 0, true);
+ if (error.Success())
+ if (!reg_ctx->WriteRegister(v0_info, reg_value))
+ error.SetErrorString("failed to write register v0");
} else {
error.SetErrorString("returning float values longer than 128 "
"bits are not supported");
}
- } else {
+ } else
error.SetErrorString("v0 register is not available on this target");
- }
}
}
} else if (type_flags & eTypeIsVector) {
diff --git a/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp b/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
index 1bdf59a17fe83..bf3c5ddd58897 100644
--- a/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
+++ b/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
@@ -276,26 +276,17 @@ Status ABISysV_arm64::SetReturnValueObject(lldb::StackFrameSP &frame_sp,
if (v0_info) {
if (byte_size <= 16) {
- if (byte_size <= RegisterValue::GetMaxByteSize()) {
- RegisterValue reg_value;
- error = reg_value.SetValueFromData(*v0_info, data, 0, true);
- if (error.Success()) {
- if (!reg_ctx->WriteRegister(v0_info, reg_value))
- error.SetErrorString("failed to write register v0");
- }
- } else {
- error.SetErrorStringWithFormat(
- "returning float values with a byte size of %" PRIu64
- " are not supported",
- byte_size);
- }
+ RegisterValue reg_value;
+ error = reg_value.SetValueFromData(*v0_info, data, 0, true);
+ if (error.Success())
+ if (!reg_ctx->WriteRegister(v0_info, reg_value))
+ error.SetErrorString("failed to write register v0");
} else {
error.SetErrorString("returning float values longer than 128 "
"bits are not supported");
}
- } else {
+ } else
error.SetErrorString("v0 register is not available on this target");
- }
}
}
} else if (type_flags & eTypeIsVector) {
diff --git a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
index a0b02d6b848f7..6ca4fb052457e 100644
--- a/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
+++ b/lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp
@@ -21,6 +21,7 @@
#include "Plugins/Process/Utility/ARMUtils.h"
#include "Plugins/Process/Utility/lldb-arm64-register-enums.h"
+#include <algorithm>
#include <cstdlib>
#include <optional>
@@ -803,7 +804,7 @@ bool EmulateInstructionARM64::EmulateLDPSTP(const uint32_t opcode) {
Context context_t;
Context context_t2;
- uint8_t buffer[RegisterValue::kMaxRegisterByteSize];
+ RegisterValue::BytesContainer buffer;
Status error;
switch (memop) {
@@ -826,23 +827,27 @@ bool EmulateInstructionARM64::EmulateLDPSTP(const uint32_t opcode) {
if (!data_Rt)
return false;
- if (data_Rt->GetAsMemoryData(*reg_info_Rt, buffer, reg_info_Rt->byte_size,
- eByteOrderLittle, error) == 0)
+ buffer.resize(reg_info_Rt->byte_size);
+ if (data_Rt->GetAsMemoryData(*reg_info_Rt, buffer.data(),
+ reg_info_Rt->byte_size, eByteOrderLittle,
+ error) == 0)
return false;
- if (!WriteMemory(context_t, address + 0, buffer, reg_info_Rt->byte_size))
+ if (!WriteMemory(context_t, address + 0, buffer.data(),
+ reg_info_Rt->byte_size))
return false;
std::optional<RegisterValue> data_Rt2 = ReadRegister(*reg_info_Rt2);
if (!data_Rt2)
return false;
- if (data_Rt2->GetAsMemoryData(*reg_info_Rt2, buffer,
+ buffer.resize(reg_info_Rt2->byte_size);
+ if (data_Rt2->GetAsMemoryData(*reg_info_Rt2, buffer.data(),
reg_info_Rt2->byte_size, eByteOrderLittle,
error) == 0)
return false;
- if (!WriteMemory(context_t2, address + size, buffer,
+ if (!WriteMemory(context_t2, address + size, buffer.data(),
reg_info_Rt2->byte_size))
return false;
} break;
@@ -861,16 +866,19 @@ bool EmulateInstructionARM64::EmulateLDPSTP(const uint32_t opcode) {
context_t.SetAddress(address);
context_t2.SetAddress(address + size);
+ buffer.resize(reg_info_Rt->byte_size);
if (rt_unknown)
- memset(buffer, 'U', reg_info_Rt->byte_size);
+ std::fill(buffer.begin(), buffer.end(), 'U');
else {
- if (!ReadMemory(context_t, address, buffer, reg_info_Rt->byte_size))
+ if (!ReadMemory(context_t, address, buffer.data(),
+ reg_info_Rt->byte_size))
return false;
}
RegisterValue data_Rt;
- if (data_Rt.SetFromMemoryData(*reg_info_Rt, buffer, reg_info_Rt->byte_size,
- eByteOrderLittle, error) == 0)
+ if (data_Rt.SetFromMemoryData(*reg_info_Rt, buffer.data(),
+ reg_info_Rt->byte_size, eByteOrderLittle,
+ error) == 0)
return false;
if (!vector && is_signed && !data_Rt.SignExtend(datasize))
@@ -879,14 +887,14 @@ bool EmulateInstructionARM64::EmulateLDPSTP(const uint32_t opcode) {
if (!WriteRegister(context_t, *reg_info_Rt, data_Rt))
return false;
- if (!rt_unknown) {
- if (!ReadMemory(context_t2, address + size, buffer,
+ buffer.resize(reg_info_Rt2->byte_size);
+ if (!rt_unknown)
+ if (!ReadMemory(context_t2, address + size, buffer.data(),
reg_info_Rt2->byte_size))
return false;
- }
RegisterValue data_Rt2;
- if (data_Rt2.SetFromMemoryData(*reg_info_Rt2, buffer,
+ if (data_Rt2.SetFromMemoryData(*reg_info_Rt2, buffer.data(),
reg_info_Rt2->byte_size, eByteOrderLittle,
error) == 0)
return false;
@@ -958,7 +966,7 @@ bool EmulateInstructionARM64::EmulateLDRSTRImm(const uint32_t opcode) {
Status error;
bool success = false;
uint64_t address;
- uint8_t buffer[RegisterValue::kMaxRegisterByteSize];
+ RegisterValue::BytesContainer buffer;
if (n == 31)
address =
@@ -999,11 +1007,13 @@ bool EmulateInstructionARM64::EmulateLDRSTRImm(const uint32_t opcode) {
if (!data_Rt)
return false;
- if (data_Rt->GetAsMemoryData(*reg_info_Rt, buffer, reg_info_Rt->byte_size,
- eByteOrderLittle, error) == 0)
+ buffer.resize(reg_info_Rt->byte_size);
+ if (data_Rt->GetAsMemoryData(*reg_info_Rt, buffer.data(),
+ reg_info_Rt->byte_size, eByteOrderLittle,
+ error) == 0)
return false;
- if (!WriteMemory(context, address, buffer, reg_info_Rt->byte_size))
+ if (!WriteMemory(context, address, buffer.data(), reg_info_Rt->byte_size))
return false;
} break;
@@ -1016,12 +1026,14 @@ bool EmulateInstructionARM64::EmulateLDRSTRImm(const uint32_t opcode) {
context.type = eContextRegisterLoad;
context.SetAddress(address);
- if (!ReadMemory(context, address, buffer, reg_info_Rt->byte_size))
+ buffer.resize(reg_info_Rt->byte_size);
+ if (!ReadMemory(context, address, buffer.data(), reg_info_Rt->byte_size))
return false;
RegisterValue data_Rt;
- if (data_Rt.SetFromMemoryData(*reg_info_Rt, buffer, reg_info_Rt->byte_size,
- eByteOrderLittle, error) == 0)
+ if (data_Rt.SetFromMemoryData(*reg_info_Rt, buffer.data(),
+ reg_info_Rt->byte_size, eByteOrderLittle,
+ error) == 0)
return false;
if (!WriteRegister(context, *reg_info_Rt, data_Rt))
diff --git a/lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp b/lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
index 1be41e4a316bf..76d7a8272f3fc 100644
--- a/lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
+++ b/lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp
@@ -1263,19 +1263,19 @@ bool EmulateInstructionMIPS::Emulate_SW(llvm::MCInst &insn) {
context.type = eContextPushRegisterOnStack;
context.SetRegisterToRegisterPlusOffset(*reg_info_src, *reg_info_base, 0);
- uint8_t buffer[RegisterValue::kMaxRegisterByteSize];
+ RegisterValue::BytesContainer buffer(reg_info_src->byte_size);
Status error;
std::optional<RegisterValue> data_src = ReadRegister(*reg_info_base);
if (!data_src)
return false;
- if (data_src->GetAsMemoryData(*reg_info_src, buffer,
+ if (data_src->GetAsMemoryData(*reg_info_src, buffer.data(),
reg_info_src->byte_size, eByteOrderLittle,
error) == 0)
return false;
- if (!WriteMemory(context, address, buffer, reg_info_src->byte_size))
+ if (!WriteMemory(context, address, buffer.data(), reg_info_src->byte_size))
return false;
return true;
@@ -1523,18 +1523,19 @@ bool EmulateInstructionMIPS::Emulate_SWSP(llvm::MCInst &insn) {
context.type = eContextPushRegisterOnStack;
context.SetRegisterToRegisterPlusOffset(reg_info_src, *reg_info_base, 0);
- uint8_t buffer[RegisterValue::kMaxRegisterByteSize];
+ RegisterValue::BytesContainer buffer(reg_info_src.byte_size);
Status error;
std::optional<RegisterValue> data_src = ReadRegister(*reg_info_base);
if (!data_src)
return false;
- if (data_src->GetAsMemoryData(reg_info_src, buffer, reg_info_src.byte_size,
- eByteOrderLittle, error) == 0)
+ if (data_src->GetAsMemoryData(reg_info_src, buffer.data(),
+ reg_info_src.byte_size, eByteOrderLittle,
+ error) == 0)
return false;
- if (!WriteMemory(context, address, buffer, reg_info_src.byte_size))
+ if (!WriteMemory(context, address, buffer.data(), reg_info_src.byte_size))
return false;
return true;
@@ -1605,19 +1606,20 @@ bool EmulateInstructionMIPS::Emulate_SWM16_32(llvm::MCInst &insn) {
context.type = eContextPushRegisterOnStack;
context.SetRegisterToRegisterPlusOffset(*reg_info_src, *reg_info_base, 0);
- uint8_t buffer[RegisterValue::kMaxRegisterByteSize];
+ RegisterValue::BytesContainer buffer(reg_info_src->byte_size);
Status error;
std::optional<RegisterValue> data_src = ReadRegister(*reg_info_base);
if (!data_src)
return false;
- if (data_src->GetAsMemoryData(*reg_info_src, buffer,
+ if (data_src->GetAsMemoryData(*reg_info_src, buffer.data(),
reg_info_src->byte_size, eByteOrderLittle,
error) == 0)
return false;
- if (!WriteMemory(context, base_address, buffer, reg_info_src->byte_size))
+ if (!WriteMemory(context, base_address, buffer.data(),
+ reg_info_src->byte_size))
return false;
// Stack address for next register
diff --git a/lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp b/lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
index c472f90ea3ba2..e33ca95b523be 100644
--- a/lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
+++ b/lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp
@@ -1157,19 +1157,18 @@ bool EmulateInstructionMIPS64::Emulate_SD(llvm::MCInst &insn) {
context.type = eContextPushRegisterOnStack;
context.SetRegisterToRegisterPlusOffset(*reg_info_src, *reg_info_base, 0);
- uint8_t buffer[RegisterValue::kMaxRegisterByteSize];
- Status error;
-
std::optional<RegisterValue> data_src = ReadRegister(*reg_info_base);
if (!data_src)
return false;
- if (data_src->GetAsMemoryData(*reg_info_src, buffer,
+ Status error;
+ RegisterValue::BytesContainer buffer(reg_info_src->byte_size);
+ if (data_src->GetAsMemoryData(*reg_info_src, buffer.data(),
reg_info_src->byte_size, eByteOrderLittle,
error) == 0)
return false;
- if (!WriteMemory(context, address, buffer, reg_info_src->byte_size))
+ if (!WriteMemory(context, address, buffer.data(), reg_info_src->byte_size))
return false;
}
diff --git a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
index 9af795f228290..496779a436e95 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
@@ -59,22 +59,24 @@ NativeRegisterContextLinux::WriteRegisterRaw(uint32_t reg_index,
}
lldb::ByteOrder byte_order = GetByteOrder();
- uint8_t dst[RegisterValue::kMaxRegisterByteSize];
+ RegisterValue::BytesContainer dst(full_reg_info->byte_size);
// Get the bytes for the full register.
const uint32_t dest_size = full_value.GetAsMemoryData(
- *full_reg_info, dst, sizeof(dst), byte_order, error);
+ *full_reg_info, dst.data(), dst.size(), byte_order, error);
if (error.Success() && dest_size) {
- uint8_t src[RegisterValue::kMaxRegisterByteSize];
+ RegisterValue::BytesContainer src(reg_info->byte_size);
// Get the bytes for the source data.
const uint32_t src_size = reg_value.GetAsMemoryData(
- *reg_info, src, sizeof(src), byte_order, error);
+ *reg_info, src.data(), src.size(), byte_order, error);
if (error.Success() && src_size && (src_size < dest_size)) {
// Copy the src bytes to the destination.
- memcpy(dst + (reg_info->byte_offset & 0x1), src, src_size);
+ memcpy(dst.data() + (reg_info->byte_offset & 0x1), src.data(),
+ src_size);
// Set this full register as the value to write.
- value_to_write.SetBytes(dst, full_value.GetByteSize(), byte_order);
+ value_to_write.SetBytes(dst.data(), full_value.GetByteSize(),
+ byte_order);
value_to_write.SetType(*full_reg_info);
reg_to_write = full_reg;
}
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 026861bd1c35e..4efc454967a12 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -37,7 +37,6 @@
#include "lldb/Utility/LLDBAssert.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
-#include "lldb/Utility/RegisterValue.h"
#include "lldb/Utility/State.h"
#include "lldb/Utility/StreamString.h"
#include "lldb/Utility/UnimplementedError.h"
@@ -2266,8 +2265,7 @@ GDBRemoteCommunicationServerLLGS::Handle_P(StringExtractorGDBRemote &packet) {
packet, "P packet missing '=' char after register number");
// Parse out the value.
- uint8_t reg_bytes[RegisterValue::kMaxRegisterByteSize];
- size_t reg_size = packet.GetHexBytesAvail(reg_bytes);
+ size_t reg_size = packet.GetHexBytesAvail(m_reg_bytes);
// Get the thread to use.
NativeThreadProtocol *thread = GetThreadFromSuffix(packet);
@@ -2306,7 +2304,7 @@ GDBRemoteCommunicationServerLLGS::Handle_P(StringExtractorGDBRemote &packet) {
// Build the reginfos response.
StreamGDBRemote response;
- RegisterValue reg_value(ArrayRef(reg_bytes, reg_size),
+ RegisterValue reg_value(ArrayRef(m_reg_bytes, reg_size),
m_current_process->GetArchitecture().GetByteOrder());
Status error = reg_context.WriteRegister(reg_info, reg_value);
if (error.Fail()) {
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
index 244ef47814363..646b6a102abf6 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -16,6 +16,7 @@
#include "lldb/Core/Communication.h"
#include "lldb/Host/MainLoop.h"
#include "lldb/Host/common/NativeProcessProtocol.h"
+#include "lldb/Utility/RegisterValue.h"
#include "lldb/lldb-private-forward.h"
#include "GDBRemoteCommunicationServerCommon.h"
@@ -123,6 +124,11 @@ class GDBRemoteCommunicationServerLLGS
NativeProcessProtocol::Extension m_extensions_supported = {};
+ // Typically we would use a SmallVector for this data but in this context we
+ // don't know how much data we're recieving so we would have to heap allocate
+ // a lot, or have a very large stack frame. So it's a member instead.
+ uint8_t m_reg_bytes[RegisterValue::kMaxRegisterByteSize];
+
PacketResult SendONotification(const char *buffer, uint32_t len);
PacketResult SendWResponse(NativeProcessProtocol *process);
diff --git a/lldb/source/Target/RegisterContext.cpp b/lldb/source/Target/RegisterContext.cpp
index ee344b01c6b8a..7236a45bff3b9 100644
--- a/lldb/source/Target/RegisterContext.cpp
+++ b/lldb/source/Target/RegisterContext.cpp
@@ -320,11 +320,6 @@ Status RegisterContext::ReadRegisterValueFromMemory(
// |AABB| Address contents
// |AABB0000| Register contents [on little-endian hardware]
// |0000AABB| Register contents [on big-endian hardware]
- if (src_len > RegisterValue::kMaxRegisterByteSize) {
- error.SetErrorString("register too small to receive memory data");
- return error;
- }
-
const uint32_t dst_len = reg_info->byte_size;
if (src_len > dst_len) {
@@ -336,11 +331,11 @@ Status RegisterContext::ReadRegisterValueFromMemory(
ProcessSP process_sp(m_thread.GetProcess());
if (process_sp) {
- uint8_t src[RegisterValue::kMaxRegisterByteSize];
+ RegisterValue::BytesContainer src(src_len);
// Read the memory
const uint32_t bytes_read =
- process_sp->ReadMemory(src_addr, src, src_len, error);
+ process_sp->ReadMemory(src_addr, src.data(), src_len, error);
// Make sure the memory read succeeded...
if (bytes_read != src_len) {
@@ -357,7 +352,7 @@ Status RegisterContext::ReadRegisterValueFromMemory(
// TODO: we might need to add a parameter to this function in case the byte
// order of the memory data doesn't match the process. For now we are
// assuming they are the same.
- reg_value.SetFromMemoryData(*reg_info, src, src_len,
+ reg_value.SetFromMemoryData(*reg_info, src.data(), src_len,
process_sp->GetByteOrder(), error);
} else
error.SetErrorString("invalid process");
@@ -384,16 +379,16 @@ Status RegisterContext::WriteRegisterValueToMemory(
// TODO: we might need to add a parameter to this function in case the byte
// order of the memory data doesn't match the process. For now we are
// assuming they are the same.
- uint8_t dst[RegisterValue::kMaxRegisterByteSize];
+ RegisterValue::BytesContainer dst(dst_len);
const uint32_t bytes_copied = reg_value.GetAsMemoryData(
- *reg_info, dst, dst_len, process_sp->GetByteOrder(), error);
+ *reg_info, dst.data(), dst_len, process_sp->GetByteOrder(), error);
if (error.Success()) {
if (bytes_copied == 0) {
error.SetErrorString("byte copy failed.");
} else {
const uint32_t bytes_written =
- process_sp->WriteMemory(dst_addr, dst, bytes_copied, error);
+ process_sp->WriteMemory(dst_addr, dst.data(), bytes_copied, error);
if (bytes_written != bytes_copied) {
if (error.Success()) {
// This might happen if we read _some_ bytes but not all
diff --git a/lldb/source/Utility/RegisterValue.cpp b/lldb/source/Utility/RegisterValue.cpp
index b9ce9fd18ac48..9563599df1628 100644
--- a/lldb/source/Utility/RegisterValue.cpp
+++ b/lldb/source/Utility/RegisterValue.cpp
@@ -48,11 +48,6 @@ uint32_t RegisterValue::GetAsMemoryData(const RegisterInfo ®_info, void *dst,
return 0;
}
- if (dst_len > kMaxRegisterByteSize) {
- error.SetErrorString("destination is too big");
- return 0;
- }
-
const uint32_t src_len = reg_info.byte_size;
// Extract the register data into a data extractor
@@ -96,12 +91,6 @@ uint32_t RegisterValue::SetFromMemoryData(const RegisterInfo ®_info,
// |AABB| Address contents
// |AABB0000| Register contents [on little-endian hardware]
// |0000AABB| Register contents [on big-endian hardware]
- if (src_len > kMaxRegisterByteSize) {
- error.SetErrorStringWithFormat(
- "register buffer is too small to receive %u bytes of data.", src_len);
- return 0;
- }
-
const uint32_t dst_len = reg_info.byte_size;
if (src_len > dst_len) {
@@ -128,9 +117,10 @@ bool RegisterValue::GetScalarValue(Scalar &scalar) const {
case eTypeInvalid:
break;
case eTypeBytes: {
- DataExtractor data(buffer.bytes, buffer.length, buffer.byte_order, 1);
- if (scalar.SetValueFromData(data, lldb::eEncodingUint,
- buffer.length).Success())
+ DataExtractor data(buffer.bytes.data(), buffer.bytes.size(),
+ buffer.byte_order, 1);
+ if (scalar.SetValueFromData(data, lldb::eEncodingUint, buffer.bytes.size())
+ .Success())
return true;
} break;
case eTypeUInt8:
@@ -190,9 +180,6 @@ Status RegisterValue::SetValueFromData(const RegisterInfo ®_info,
if (src_len > reg_info.byte_size)
src_len = reg_info.byte_size;
- // Zero out the value in case we get partial data...
- memset(buffer.bytes, 0, sizeof(buffer.bytes));
-
type128 int128;
m_type = eTypeInvalid;
@@ -232,16 +219,14 @@ Status RegisterValue::SetValueFromData(const RegisterInfo ®_info,
break;
case eEncodingVector: {
m_type = eTypeBytes;
- buffer.length = reg_info.byte_size;
+ assert(reg_info.byte_size <= kMaxRegisterByteSize);
+ buffer.bytes.resize(reg_info.byte_size);
buffer.byte_order = src.GetByteOrder();
- assert(buffer.length <= kMaxRegisterByteSize);
- if (buffer.length > kMaxRegisterByteSize)
- buffer.length = kMaxRegisterByteSize;
if (src.CopyByteOrderedData(
- src_offset, // offset within "src" to start extracting data
- src_len, // src length
- buffer.bytes, // dst buffer
- buffer.length, // dst length
+ src_offset, // offset within "src" to start extracting data
+ src_len, // src length
+ buffer.bytes.data(), // dst buffer
+ buffer.bytes.size(), // dst length
buffer.byte_order) == 0) // dst byte order
{
error.SetErrorStringWithFormat(
@@ -488,9 +473,7 @@ bool RegisterValue::CopyValue(const RegisterValue &rhs) {
m_scalar = rhs.m_scalar;
break;
case eTypeBytes:
- assert(rhs.buffer.length <= kMaxRegisterByteSize);
- ::memcpy(buffer.bytes, rhs.buffer.bytes, kMaxRegisterByteSize);
- buffer.length = rhs.buffer.length;
+ buffer.bytes = rhs.buffer.bytes;
buffer.byte_order = rhs.buffer.byte_order;
break;
}
@@ -509,12 +492,12 @@ uint16_t RegisterValue::GetAsUInt16(uint16_t fail_value,
case eTypeUInt16:
return m_scalar.UShort(fail_value);
case eTypeBytes: {
- switch (buffer.length) {
+ switch (buffer.bytes.size()) {
default:
break;
case 1:
case 2:
- return *reinterpret_cast<const uint16_t *>(buffer.bytes);
+ return *reinterpret_cast<const uint16_t *>(buffer.bytes.data());
}
} break;
}
@@ -538,13 +521,13 @@ uint32_t RegisterValue::GetAsUInt32(uint32_t fail_value,
case eTypeLongDouble:
return m_scalar.UInt(fail_value);
case eTypeBytes: {
- switch (buffer.length) {
+ switch (buffer.bytes.size()) {
default:
break;
case 1:
case 2:
case 4:
- return *reinterpret_cast<const uint32_t *>(buffer.bytes);
+ return *reinterpret_cast<const uint32_t *>(buffer.bytes.data());
}
} break;
}
@@ -569,17 +552,17 @@ uint64_t RegisterValue::GetAsUInt64(uint64_t fail_value,
case eTypeLongDouble:
return m_scalar.ULongLong(fail_value);
case eTypeBytes: {
- switch (buffer.length) {
+ switch (buffer.bytes.size()) {
default:
break;
case 1:
- return *(const uint8_t *)buffer.bytes;
+ return *(const uint8_t *)buffer.bytes.data();
case 2:
- return *reinterpret_cast<const uint16_t *>(buffer.bytes);
+ return *reinterpret_cast<const uint16_t *>(buffer.bytes.data());
case 4:
- return *reinterpret_cast<const uint32_t *>(buffer.bytes);
+ return *reinterpret_cast<const uint32_t *>(buffer.bytes.data());
case 8:
- return *reinterpret_cast<const uint64_t *>(buffer.bytes);
+ return *reinterpret_cast<const uint64_t *>(buffer.bytes.data());
}
} break;
}
@@ -605,7 +588,7 @@ llvm::APInt RegisterValue::GetAsUInt128(const llvm::APInt &fail_value,
case eTypeLongDouble:
return m_scalar.UInt128(fail_value);
case eTypeBytes: {
- switch (buffer.length) {
+ switch (buffer.bytes.size()) {
default:
break;
case 1:
@@ -613,8 +596,9 @@ llvm::APInt RegisterValue::GetAsUInt128(const llvm::APInt &fail_value,
case 4:
case 8:
case 16:
- return llvm::APInt(BITWIDTH_INT128, NUM_OF_WORDS_INT128,
- (reinterpret_cast<const type128 *>(buffer.bytes))->x);
+ return llvm::APInt(
+ BITWIDTH_INT128, NUM_OF_WORDS_INT128,
+ (reinterpret_cast<const type128 *>(buffer.bytes.data()))->x);
}
} break;
}
@@ -696,9 +680,9 @@ const void *RegisterValue::GetBytes() const {
case eTypeDouble:
case eTypeLongDouble:
m_scalar.GetBytes(buffer.bytes);
- return buffer.bytes;
+ return buffer.bytes.data();
case eTypeBytes:
- return buffer.bytes;
+ return buffer.bytes.data();
}
return nullptr;
}
@@ -719,7 +703,7 @@ uint32_t RegisterValue::GetByteSize() const {
case eTypeLongDouble:
return m_scalar.GetByteSize();
case eTypeBytes:
- return buffer.length;
+ return buffer.bytes.size();
}
return 0;
}
@@ -744,19 +728,14 @@ bool RegisterValue::SetUInt(uint64_t uint, uint32_t byte_size) {
void RegisterValue::SetBytes(const void *bytes, size_t length,
lldb::ByteOrder byte_order) {
- // If this assertion fires off we need to increase the size of buffer.bytes,
- // or make it something that is allocated on the heap. Since the data buffer
- // is in a union, we can't make it a collection class like SmallVector...
if (bytes && length > 0) {
- assert(length <= sizeof(buffer.bytes) &&
- "Storing too many bytes in a RegisterValue.");
m_type = eTypeBytes;
- buffer.length = length;
- memcpy(buffer.bytes, bytes, length);
+ buffer.bytes.resize(length);
+ memcpy(buffer.bytes.data(), bytes, length);
buffer.byte_order = byte_order;
} else {
m_type = eTypeInvalid;
- buffer.length = 0;
+ buffer.bytes.resize(0);
}
}
@@ -775,15 +754,7 @@ bool RegisterValue::operator==(const RegisterValue &rhs) const {
case eTypeLongDouble:
return m_scalar == rhs.m_scalar;
case eTypeBytes:
- if (buffer.length != rhs.buffer.length)
- return false;
- else {
- uint16_t length = buffer.length;
- if (length > kMaxRegisterByteSize)
- length = kMaxRegisterByteSize;
- return memcmp(buffer.bytes, rhs.buffer.bytes, length) == 0;
- }
- break;
+ return buffer.bytes == rhs.buffer.bytes;
}
}
return false;
@@ -818,12 +789,12 @@ bool RegisterValue::ClearBit(uint32_t bit) {
buffer.byte_order == eByteOrderLittle) {
uint32_t byte_idx;
if (buffer.byte_order == eByteOrderBig)
- byte_idx = buffer.length - (bit / 8) - 1;
+ byte_idx = buffer.bytes.size() - (bit / 8) - 1;
else
byte_idx = bit / 8;
const uint32_t byte_bit = bit % 8;
- if (byte_idx < buffer.length) {
+ if (byte_idx < buffer.bytes.size()) {
buffer.bytes[byte_idx] &= ~(1u << byte_bit);
return true;
}
@@ -858,12 +829,12 @@ bool RegisterValue::SetBit(uint32_t bit) {
buffer.byte_order == eByteOrderLittle) {
uint32_t byte_idx;
if (buffer.byte_order == eByteOrderBig)
- byte_idx = buffer.length - (bit / 8) - 1;
+ byte_idx = buffer.bytes.size() - (bit / 8) - 1;
else
byte_idx = bit / 8;
const uint32_t byte_bit = bit % 8;
- if (byte_idx < buffer.length) {
+ if (byte_idx < buffer.bytes.size()) {
buffer.bytes[byte_idx] |= (1u << byte_bit);
return true;
}
More information about the lldb-commits
mailing list