[LLDB] Change RegisterValue::GetAsMemoryData to const RegisterInfo&

Most of the paths to this never passed nullptr intentionally. Those
that possibly could have were assuming it was not null elsehwere,
so would have crashed.

I've added asserts in those cases.

At least one case was relying on GetAsMemoryData to return an error
when it was given nullptr. So I've hoisted that error setting code
out into the caller.

Depends on D134963

Reviewed By: clayborg

Differential Revision: https://reviews.llvm.org/D134965
This commit is contained in:
David Spickett 2022-09-28 14:00:02 +00:00
parent 7ec604a317
commit c7ddbd62d8
18 changed files with 91 additions and 89 deletions

View File

@ -95,7 +95,7 @@ public:
// value, or pad the destination with zeroes if the register byte size is
// shorter that "dst_len" (all while correctly abiding the "dst_byte_order").
// Returns the number of bytes copied into "dst".
uint32_t GetAsMemoryData(const RegisterInfo *reg_info, void *dst,
uint32_t GetAsMemoryData(const RegisterInfo &reg_info, void *dst,
uint32_t dst_len, lldb::ByteOrder dst_byte_order,
Status &error) const;

View File

@ -385,18 +385,20 @@ Status NativeRegisterContext::ReadRegisterValueFromMemory(
Status NativeRegisterContext::WriteRegisterValueToMemory(
const RegisterInfo *reg_info, lldb::addr_t dst_addr, size_t dst_len,
const RegisterValue &reg_value) {
Status error;
if (reg_info == nullptr) {
error.SetErrorString("Invalid register info argument.");
return error;
}
uint8_t dst[RegisterValue::kMaxRegisterByteSize];
Status error;
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, dst_len, process.GetByteOrder(), error);
if (error.Success()) {
if (bytes_copied == 0) {

View File

@ -537,8 +537,8 @@ static bool LoadValueFromConsecutiveGPRRegisters(
// Make sure we have enough room in "heap_data_up"
if ((data_offset + *base_byte_size) <= heap_data_up->GetByteSize()) {
const size_t bytes_copied = reg_value.GetAsMemoryData(
reg_info, heap_data_up->GetBytes() + data_offset, *base_byte_size,
byte_order, error);
*reg_info, heap_data_up->GetBytes() + data_offset,
*base_byte_size, byte_order, error);
if (bytes_copied != *base_byte_size)
return false;
data_offset += bytes_copied;
@ -577,7 +577,7 @@ static bool LoadValueFromConsecutiveGPRRegisters(
const size_t curr_byte_size = std::min<size_t>(8, bytes_left);
const size_t bytes_copied = reg_value.GetAsMemoryData(
reg_info, heap_data_up->GetBytes() + data_offset, curr_byte_size,
*reg_info, heap_data_up->GetBytes() + data_offset, curr_byte_size,
byte_order, error);
if (bytes_copied == 0)
return false;
@ -689,10 +689,10 @@ ValueObjectSP ABIMacOSX_arm64::GetReturnValueObjectImpl(
reg_ctx->ReadRegister(x1_reg_info, x1_reg_value)) {
Status error;
if (x0_reg_value.GetAsMemoryData(
x0_reg_info, heap_data_up->GetBytes() + 0, 8,
*x0_reg_info, heap_data_up->GetBytes() + 0, 8,
byte_order, error) &&
x1_reg_value.GetAsMemoryData(
x1_reg_info, heap_data_up->GetBytes() + 8, 8,
*x1_reg_info, heap_data_up->GetBytes() + 8, 8,
byte_order, error)) {
DataExtractor data(
DataBufferSP(heap_data_up.release()), byte_order,
@ -785,7 +785,7 @@ ValueObjectSP ABIMacOSX_arm64::GetReturnValueObjectImpl(
RegisterValue reg_value;
if (reg_ctx->ReadRegister(v0_info, reg_value)) {
Status error;
if (reg_value.GetAsMemoryData(v0_info, heap_data_up->GetBytes(),
if (reg_value.GetAsMemoryData(*v0_info, heap_data_up->GetBytes(),
heap_data_up->GetByteSize(),
byte_order, error)) {
DataExtractor data(DataBufferSP(heap_data_up.release()),

View File

@ -512,8 +512,8 @@ static bool LoadValueFromConsecutiveGPRRegisters(
// Make sure we have enough room in "heap_data_up"
if ((data_offset + *base_byte_size) <= heap_data_up->GetByteSize()) {
const size_t bytes_copied = reg_value.GetAsMemoryData(
reg_info, heap_data_up->GetBytes() + data_offset, *base_byte_size,
byte_order, error);
*reg_info, heap_data_up->GetBytes() + data_offset,
*base_byte_size, byte_order, error);
if (bytes_copied != *base_byte_size)
return false;
data_offset += bytes_copied;
@ -548,7 +548,7 @@ static bool LoadValueFromConsecutiveGPRRegisters(
const size_t curr_byte_size = std::min<size_t>(8, bytes_left);
const size_t bytes_copied = reg_value.GetAsMemoryData(
reg_info, heap_data_up->GetBytes() + data_offset, curr_byte_size,
*reg_info, heap_data_up->GetBytes() + data_offset, curr_byte_size,
byte_order, error);
if (bytes_copied == 0)
return false;
@ -661,10 +661,10 @@ ValueObjectSP ABISysV_arm64::GetReturnValueObjectImpl(
reg_ctx->ReadRegister(x1_reg_info, x1_reg_value)) {
Status error;
if (x0_reg_value.GetAsMemoryData(
x0_reg_info, heap_data_up->GetBytes() + 0, 8,
*x0_reg_info, heap_data_up->GetBytes() + 0, 8,
byte_order, error) &&
x1_reg_value.GetAsMemoryData(
x1_reg_info, heap_data_up->GetBytes() + 8, 8,
*x1_reg_info, heap_data_up->GetBytes() + 8, 8,
byte_order, error)) {
DataExtractor data(
DataBufferSP(heap_data_up.release()), byte_order,
@ -755,7 +755,7 @@ ValueObjectSP ABISysV_arm64::GetReturnValueObjectImpl(
RegisterValue reg_value;
if (reg_ctx->ReadRegister(v0_info, reg_value)) {
Status error;
if (reg_value.GetAsMemoryData(v0_info, heap_data_up->GetBytes(),
if (reg_value.GetAsMemoryData(*v0_info, heap_data_up->GetBytes(),
heap_data_up->GetByteSize(), byte_order,
error)) {
DataExtractor data(DataBufferSP(heap_data_up.release()), byte_order,

View File

@ -1495,16 +1495,16 @@ ValueObjectSP ABIMacOSX_arm::GetReturnValueObjectImpl(
reg_ctx->ReadRegister(r2_reg_info, r2_reg_value) &&
reg_ctx->ReadRegister(r3_reg_info, r3_reg_value)) {
Status error;
if (r0_reg_value.GetAsMemoryData(r0_reg_info,
if (r0_reg_value.GetAsMemoryData(*r0_reg_info,
heap_data_up->GetBytes() + 0,
4, byte_order, error) &&
r1_reg_value.GetAsMemoryData(r1_reg_info,
r1_reg_value.GetAsMemoryData(*r1_reg_info,
heap_data_up->GetBytes() + 4,
4, byte_order, error) &&
r2_reg_value.GetAsMemoryData(r2_reg_info,
r2_reg_value.GetAsMemoryData(*r2_reg_info,
heap_data_up->GetBytes() + 8,
4, byte_order, error) &&
r3_reg_value.GetAsMemoryData(r3_reg_info,
r3_reg_value.GetAsMemoryData(*r3_reg_info,
heap_data_up->GetBytes() + 12,
4, byte_order, error)) {
DataExtractor data(DataBufferSP(heap_data_up.release()),

View File

@ -1693,7 +1693,7 @@ ValueObjectSP ABISysV_arm::GetReturnValueObjectImpl(
if ((data_offset + vfp_byte_size) <= data_sp->GetByteSize()) {
Status error;
const size_t bytes_copied = reg_value.GetAsMemoryData(
reg_info, data_sp->GetBytes() + data_offset, vfp_byte_size,
*reg_info, data_sp->GetBytes() + data_offset, vfp_byte_size,
byte_order, error);
if (bytes_copied != vfp_byte_size)
break;

View File

@ -724,6 +724,7 @@ ValueObjectSP ABISysV_mips64::GetReturnValueObjectImpl(
const RegisterInfo *r2_info = reg_ctx->GetRegisterInfoByName("r2", 0);
const RegisterInfo *r3_info = reg_ctx->GetRegisterInfoByName("r3", 0);
assert(r2_info && r3_info && "Basic registers should always be present.");
if (type_flags & eTypeIsScalar || type_flags & eTypeIsPointer) {
value.SetValueType(Value::ValueType::Scalar);
@ -1054,8 +1055,8 @@ ValueObjectSP ABISysV_mips64::GetReturnValueObjectImpl(
reg_ctx->ReadRegister(r2_info, r2_value);
const size_t bytes_copied = r2_value.GetAsMemoryData(
r2_info, data_sp->GetBytes(), r2_info->byte_size, target_byte_order,
error);
*r2_info, data_sp->GetBytes(), r2_info->byte_size,
target_byte_order, error);
if (bytes_copied != r2_info->byte_size)
return return_valobj_sp;
sucess = true;
@ -1063,7 +1064,7 @@ ValueObjectSP ABISysV_mips64::GetReturnValueObjectImpl(
if (use_r3) {
reg_ctx->ReadRegister(r3_info, r3_value);
const size_t bytes_copied = r3_value.GetAsMemoryData(
r3_info, data_sp->GetBytes() + r2_info->byte_size,
*r3_info, data_sp->GetBytes() + r2_info->byte_size,
r3_info->byte_size, target_byte_order, error);
if (bytes_copied != r3_info->byte_size)

View File

@ -621,7 +621,7 @@ ValueObjectSP ABISysV_ppc::GetReturnValueObjectSimple(
if (reg_ctx->ReadRegister(altivec_reg, reg_value)) {
Status error;
if (reg_value.GetAsMemoryData(
altivec_reg, heap_data_up->GetBytes(),
*altivec_reg, heap_data_up->GetBytes(),
heap_data_up->GetByteSize(), byte_order, error)) {
DataExtractor data(DataBufferSP(heap_data_up.release()),
byte_order,

View File

@ -463,7 +463,7 @@ class ReturnValueExtractor {
Status error;
uint32_t rc = reg_val.GetAsMemoryData(
reg_info, &raw_data, sizeof(raw_data), m_byte_order, error);
*reg_info, &raw_data, sizeof(raw_data), m_byte_order, error);
if (rc != sizeof(raw_data)) {
LLDB_LOG(m_log, LOG_PREFIX "GetAsMemoryData() failed");
return false;
@ -727,7 +727,7 @@ private:
LLDB_LOG(m_log, LOG_PREFIX "Failed to read vector register contents");
return ValueObjectSP();
}
if (!vr_val[i].GetAsMemoryData(vr[i], vr_data->GetBytes() + i * vr_size,
if (!vr_val[i].GetAsMemoryData(*vr[i], vr_data->GetBytes() + i * vr_size,
vr_size, m_byte_order, error)) {
LLDB_LOG(m_log, LOG_PREFIX "Failed to extract vector register bytes");
return ValueObjectSP();

View File

@ -528,7 +528,7 @@ ValueObjectSP ABISysV_i386::GetReturnValueObjectSimple(
RegisterValue reg_value;
if (reg_ctx->ReadRegister(vec_reg, reg_value)) {
Status error;
if (reg_value.GetAsMemoryData(vec_reg, heap_data_up->GetBytes(),
if (reg_value.GetAsMemoryData(*vec_reg, heap_data_up->GetBytes(),
heap_data_up->GetByteSize(),
byte_order, error)) {
DataExtractor data(DataBufferSP(heap_data_up.release()),
@ -556,11 +556,12 @@ ValueObjectSP ABISysV_i386::GetReturnValueObjectSimple(
reg_ctx->ReadRegister(vec_reg2, reg_value2)) {
Status error;
if (reg_value.GetAsMemoryData(vec_reg, heap_data_up->GetBytes(),
vec_reg->byte_size, byte_order,
error) &&
if (reg_value.GetAsMemoryData(
*vec_reg, heap_data_up->GetBytes(), vec_reg->byte_size,
byte_order, error) &&
reg_value2.GetAsMemoryData(
vec_reg2, heap_data_up->GetBytes() + vec_reg->byte_size,
*vec_reg2,
heap_data_up->GetBytes() + vec_reg->byte_size,
heap_data_up->GetByteSize() - vec_reg->byte_size,
byte_order, error)) {
DataExtractor data(DataBufferSP(heap_data_up.release()),

View File

@ -511,7 +511,7 @@ ValueObjectSP ABISysV_x86_64::GetReturnValueObjectSimple(
if (reg_ctx->ReadRegister(altivec_reg, reg_value)) {
Status error;
if (reg_value.GetAsMemoryData(
altivec_reg, heap_data_up->GetBytes(),
*altivec_reg, heap_data_up->GetBytes(),
heap_data_up->GetByteSize(), byte_order, error)) {
DataExtractor data(DataBufferSP(heap_data_up.release()),
byte_order,
@ -539,10 +539,10 @@ ValueObjectSP ABISysV_x86_64::GetReturnValueObjectSimple(
Status error;
if (reg_value.GetAsMemoryData(
altivec_reg, heap_data_up->GetBytes(),
*altivec_reg, heap_data_up->GetBytes(),
altivec_reg->byte_size, byte_order, error) &&
reg_value2.GetAsMemoryData(
altivec_reg2,
*altivec_reg2,
heap_data_up->GetBytes() + altivec_reg->byte_size,
heap_data_up->GetByteSize() - altivec_reg->byte_size,
byte_order, error)) {

View File

@ -517,9 +517,9 @@ ValueObjectSP ABIWindows_x86_64::GetReturnValueObjectSimple(
RegisterValue reg_value;
if (reg_ctx->ReadRegister(xmm_reg, reg_value)) {
Status error;
if (reg_value.GetAsMemoryData(
xmm_reg, heap_data_up->GetBytes(),
heap_data_up->GetByteSize(), byte_order, error)) {
if (reg_value.GetAsMemoryData(*xmm_reg, heap_data_up->GetBytes(),
heap_data_up->GetByteSize(),
byte_order, error)) {
DataExtractor data(DataBufferSP(heap_data_up.release()),
byte_order,
process_sp->GetTarget()

View File

@ -827,7 +827,7 @@ bool EmulateInstructionARM64::EmulateLDPSTP(const uint32_t opcode) {
if (!ReadRegister(*reg_info_Rt, data_Rt))
return false;
if (data_Rt.GetAsMemoryData(&(*reg_info_Rt), buffer, reg_info_Rt->byte_size,
if (data_Rt.GetAsMemoryData(*reg_info_Rt, buffer, reg_info_Rt->byte_size,
eByteOrderLittle, error) == 0)
return false;
@ -837,9 +837,8 @@ bool EmulateInstructionARM64::EmulateLDPSTP(const uint32_t opcode) {
if (!ReadRegister(*reg_info_Rt2, data_Rt2))
return false;
if (data_Rt2.GetAsMemoryData(&(*reg_info_Rt2), buffer,
reg_info_Rt2->byte_size, eByteOrderLittle,
error) == 0)
if (data_Rt2.GetAsMemoryData(*reg_info_Rt2, buffer, reg_info_Rt2->byte_size,
eByteOrderLittle, error) == 0)
return false;
if (!WriteMemory(context_t2, address + size, buffer,
@ -998,7 +997,7 @@ bool EmulateInstructionARM64::EmulateLDRSTRImm(const uint32_t opcode) {
if (!ReadRegister(*reg_info_Rt, data_Rt))
return false;
if (data_Rt.GetAsMemoryData(&(*reg_info_Rt), buffer, reg_info_Rt->byte_size,
if (data_Rt.GetAsMemoryData(*reg_info_Rt, buffer, reg_info_Rt->byte_size,
eByteOrderLittle, error) == 0)
return false;

View File

@ -1269,9 +1269,8 @@ bool EmulateInstructionMIPS::Emulate_SW(llvm::MCInst &insn) {
if (!ReadRegister(*reg_info_base, 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, reg_info_src->byte_size,
eByteOrderLittle, error) == 0)
return false;
if (!WriteMemory(context, address, buffer, reg_info_src->byte_size))
@ -1529,7 +1528,7 @@ bool EmulateInstructionMIPS::Emulate_SWSP(llvm::MCInst &insn) {
if (!ReadRegister(*reg_info_base, data_src))
return false;
if (data_src.GetAsMemoryData(&reg_info_src, buffer, reg_info_src.byte_size,
if (data_src.GetAsMemoryData(reg_info_src, buffer, reg_info_src.byte_size,
eByteOrderLittle, error) == 0)
return false;
@ -1611,9 +1610,8 @@ bool EmulateInstructionMIPS::Emulate_SWM16_32(llvm::MCInst &insn) {
if (!ReadRegister(*reg_info_base, 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, reg_info_src->byte_size,
eByteOrderLittle, error) == 0)
return false;
if (!WriteMemory(context, base_address, buffer, reg_info_src->byte_size))

View File

@ -1163,9 +1163,8 @@ bool EmulateInstructionMIPS64::Emulate_SD(llvm::MCInst &insn) {
if (!ReadRegister(*reg_info_base, 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, reg_info_src->byte_size,
eByteOrderLittle, error) == 0)
return false;
if (!WriteMemory(context, address, buffer, reg_info_src->byte_size))

View File

@ -42,6 +42,7 @@ NativeRegisterContextLinux::WriteRegisterRaw(uint32_t reg_index,
// Check if this is a subregister of a full register.
const RegisterInfo *reg_info = GetRegisterInfoAtIndex(reg_index);
assert(reg_info && "Expected valid register info for reg_index.");
if (reg_info->invalidate_regs &&
(reg_info->invalidate_regs[0] != LLDB_INVALID_REGNUM)) {
Status error;
@ -52,21 +53,23 @@ NativeRegisterContextLinux::WriteRegisterRaw(uint32_t reg_index,
// Read the full register.
error = ReadRegister(full_reg_info, full_value);
if (error.Fail())
if (error.Fail()) {
// full_reg_info was nullptr, or we couldn't read the register.
return error;
}
lldb::ByteOrder byte_order = GetByteOrder();
uint8_t dst[RegisterValue::kMaxRegisterByteSize];
// 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, sizeof(dst), byte_order, error);
if (error.Success() && dest_size) {
uint8_t src[RegisterValue::kMaxRegisterByteSize];
// 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, sizeof(src), 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);

View File

@ -368,37 +368,41 @@ Status RegisterContext::ReadRegisterValueFromMemory(
Status RegisterContext::WriteRegisterValueToMemory(
const RegisterInfo *reg_info, lldb::addr_t dst_addr, uint32_t dst_len,
const RegisterValue &reg_value) {
uint8_t dst[RegisterValue::kMaxRegisterByteSize];
Status error;
ProcessSP process_sp(m_thread.GetProcess());
if (process_sp) {
// 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.
if (!process_sp) {
error.SetErrorString("invalid process");
return error;
}
const uint32_t bytes_copied = reg_value.GetAsMemoryData(
reg_info, dst, dst_len, process_sp->GetByteOrder(), error);
if (reg_info == nullptr) {
error.SetErrorString("Invalid register info argument.");
return 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);
if (bytes_written != bytes_copied) {
if (error.Success()) {
// This might happen if we read _some_ bytes but not all
error.SetErrorStringWithFormat("only wrote %u of %u bytes",
bytes_written, bytes_copied);
}
// 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];
const uint32_t bytes_copied = reg_value.GetAsMemoryData(
*reg_info, dst, 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);
if (bytes_written != bytes_copied) {
if (error.Success()) {
// This might happen if we read _some_ bytes but not all
error.SetErrorStringWithFormat("only wrote %u of %u bytes",
bytes_written, bytes_copied);
}
}
}
} else
error.SetErrorString("invalid process");
}
return error;
}

View File

@ -35,21 +35,16 @@ bool RegisterValue::GetData(DataExtractor &data) const {
return data.SetData(GetBytes(), GetByteSize(), GetByteOrder()) > 0;
}
uint32_t RegisterValue::GetAsMemoryData(const RegisterInfo *reg_info, void *dst,
uint32_t RegisterValue::GetAsMemoryData(const RegisterInfo &reg_info, void *dst,
uint32_t dst_len,
lldb::ByteOrder dst_byte_order,
Status &error) const {
if (reg_info == nullptr) {
error.SetErrorString("invalid register info argument.");
return 0;
}
// ReadRegister should have already been called on this object prior to
// calling this.
if (GetType() == eTypeInvalid) {
// No value has been read into this object...
error.SetErrorStringWithFormat(
"invalid register value type for register %s", reg_info->name);
"invalid register value type for register %s", reg_info.name);
return 0;
}
@ -58,7 +53,7 @@ uint32_t RegisterValue::GetAsMemoryData(const RegisterInfo *reg_info, void *dst,
return 0;
}
const uint32_t src_len = reg_info->byte_size;
const uint32_t src_len = reg_info.byte_size;
// Extract the register data into a data extractor
DataExtractor reg_data;
@ -76,7 +71,7 @@ uint32_t RegisterValue::GetAsMemoryData(const RegisterInfo *reg_info, void *dst,
dst_byte_order); // dst byte order
if (bytes_copied == 0)
error.SetErrorStringWithFormat(
"failed to copy data for register write of %s", reg_info->name);
"failed to copy data for register write of %s", reg_info.name);
return bytes_copied;
}