DavidSpickett created this revision. Herald added subscribers: ctetreau, atanasyan, jrtc27, kristof.beyls. Herald added a project: All. DavidSpickett requested review of this revision. Herald added subscribers: lldb-commits, alextsao1999. Herald added a project: LLDB.
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. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D153626 Files: 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
Index: lldb/source/Utility/RegisterValue.cpp =================================================================== --- lldb/source/Utility/RegisterValue.cpp +++ lldb/source/Utility/RegisterValue.cpp @@ -48,11 +48,6 @@ 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 @@ // |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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ case eTypeLongDouble: return m_scalar.GetByteSize(); case eTypeBytes: - return buffer.length; + return buffer.bytes.size(); } return 0; } @@ -744,19 +728,14 @@ 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 @@ 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 @@ 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 @@ 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; } Index: lldb/source/Target/RegisterContext.cpp =================================================================== --- lldb/source/Target/RegisterContext.cpp +++ lldb/source/Target/RegisterContext.cpp @@ -320,11 +320,6 @@ // |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 @@ 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 @@ // 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 @@ // 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 Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h +++ 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 @@ 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); Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp =================================================================== --- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp +++ 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 @@ 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 @@ // 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()) { Index: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp =================================================================== --- lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp +++ lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp @@ -59,22 +59,24 @@ } 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; } Index: lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp =================================================================== --- lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp +++ lldb/source/Plugins/Instruction/MIPS64/EmulateInstructionMIPS64.cpp @@ -1157,19 +1157,18 @@ 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; } Index: lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp =================================================================== --- lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp +++ lldb/source/Plugins/Instruction/MIPS/EmulateInstructionMIPS.cpp @@ -1263,19 +1263,19 @@ 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 @@ 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 @@ 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 Index: lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp =================================================================== --- lldb/source/Plugins/Instruction/ARM64/EmulateInstructionARM64.cpp +++ 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 @@ Context context_t; Context context_t2; - uint8_t buffer[RegisterValue::kMaxRegisterByteSize]; + RegisterValue::BytesContainer buffer; Status error; switch (memop) { @@ -826,23 +827,27 @@ 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 @@ 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)) @@ -880,13 +888,14 @@ return false; if (!rt_unknown) { - if (!ReadMemory(context_t2, address + size, buffer, + buffer.resize(reg_info_Rt2->byte_size); + 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 +967,7 @@ Status error; bool success = false; uint64_t address; - uint8_t buffer[RegisterValue::kMaxRegisterByteSize]; + RegisterValue::BytesContainer buffer; if (n == 31) address = @@ -999,11 +1008,13 @@ 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 +1027,14 @@ 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)) Index: lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp =================================================================== --- lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp +++ lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp @@ -276,26 +276,17 @@ 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) { Index: lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp =================================================================== --- lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp +++ lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp @@ -303,26 +303,17 @@ 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) { Index: lldb/source/Host/common/NativeRegisterContext.cpp =================================================================== --- lldb/source/Host/common/NativeRegisterContext.cpp +++ lldb/source/Host/common/NativeRegisterContext.cpp @@ -331,11 +331,6 @@ // |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 @@ } 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 @@ // 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 @@ 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; Index: lldb/include/lldb/Utility/RegisterValue.h =================================================================== --- lldb/include/lldb/Utility/RegisterValue.h +++ 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 @@ 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 @@ 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; };
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits