Unify the return value of GetByteSize to an llvm::Optional<uint64_t> (NFC-ish)

This cleanup patch unifies all methods called GetByteSize() in the
ValueObject hierarchy to return an optional, like the methods in
CompilerType do. This means fewer magic 0 values, which could fix bugs
down the road in languages where types can have a size of zero, such
as Swift and C (but not C++).

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

This re-lands the patch with bogus :m_byte_size(0) initalizations removed.
This commit is contained in:
Adrian Prantl 2020-07-25 08:27:21 -07:00
parent 145acacaea
commit 113f56fbb8
24 changed files with 66 additions and 64 deletions

View File

@ -358,7 +358,7 @@ public:
virtual bool CanProvideValue();
// Subclasses must implement the functions below.
virtual uint64_t GetByteSize() = 0;
virtual llvm::Optional<uint64_t> GetByteSize() = 0;
virtual lldb::ValueType GetValueType() const = 0;

View File

@ -30,7 +30,7 @@ public:
ConstString name,
const CompilerType &cast_type);
uint64_t GetByteSize() override;
llvm::Optional<uint64_t> GetByteSize() override;
size_t CalculateNumChildren(uint32_t max) override;

View File

@ -30,7 +30,7 @@ class ValueObjectChild : public ValueObject {
public:
~ValueObjectChild() override;
uint64_t GetByteSize() override { return m_byte_size; }
llvm::Optional<uint64_t> GetByteSize() override { return m_byte_size; }
lldb::offset_t GetByteOffset() override { return m_byte_offset; }

View File

@ -62,7 +62,7 @@ public:
static lldb::ValueObjectSP Create(ExecutionContextScope *exe_scope,
const Status &error);
uint64_t GetByteSize() override;
llvm::Optional<uint64_t> GetByteSize() override;
lldb::ValueType GetValueType() const override;
@ -113,7 +113,7 @@ protected:
CompilerType GetCompilerTypeImpl() override;
ConstString m_type_name;
uint64_t m_byte_size;
llvm::Optional<uint64_t> m_byte_size;
ValueObjectConstResultImpl m_impl;

View File

@ -34,7 +34,7 @@ class ValueObjectDynamicValue : public ValueObject {
public:
~ValueObjectDynamicValue() override;
uint64_t GetByteSize() override;
llvm::Optional<uint64_t> GetByteSize() override;
ConstString GetTypeName() override;

View File

@ -40,7 +40,7 @@ public:
const Address &address,
const CompilerType &ast_type);
uint64_t GetByteSize() override;
llvm::Optional<uint64_t> GetByteSize() override;
ConstString GetTypeName() override;

View File

@ -36,7 +36,7 @@ public:
lldb::RegisterContextSP &reg_ctx_sp,
uint32_t set_idx);
uint64_t GetByteSize() override;
llvm::Optional<uint64_t> GetByteSize() override;
lldb::ValueType GetValueType() const override {
return lldb::eValueTypeRegisterSet;
@ -86,7 +86,7 @@ public:
lldb::RegisterContextSP &reg_ctx_sp,
uint32_t reg_num);
uint64_t GetByteSize() override;
llvm::Optional<uint64_t> GetByteSize() override;
lldb::ValueType GetValueType() const override {
return lldb::eValueTypeRegister;

View File

@ -36,7 +36,7 @@ class ValueObjectSynthetic : public ValueObject {
public:
~ValueObjectSynthetic() override;
uint64_t GetByteSize() override;
llvm::Optional<uint64_t> GetByteSize() override;
ConstString GetTypeName() override;

View File

@ -37,7 +37,7 @@ public:
static lldb::ValueObjectSP Create(ExecutionContextScope *exe_scope,
const lldb::VariableSP &var_sp);
uint64_t GetByteSize() override;
llvm::Optional<uint64_t> GetByteSize() override;
ConstString GetTypeName() override;

View File

@ -32,7 +32,7 @@ public:
virtual ~ExpressionVariable();
size_t GetByteSize() { return m_frozen_sp->GetByteSize(); }
llvm::Optional<uint64_t> GetByteSize() { return m_frozen_sp->GetByteSize(); }
ConstString GetName() { return m_frozen_sp->GetName(); }

View File

@ -154,7 +154,9 @@ class ValueObjectRecognizerSynthesizedValue : public ValueObject {
SetName(parent.GetName());
}
uint64_t GetByteSize() override { return m_parent->GetByteSize(); }
llvm::Optional<uint64_t> GetByteSize() override {
return m_parent->GetByteSize();
}
lldb::ValueType GetValueType() const override { return m_type; }
bool UpdateValue() override {
if (!m_parent->UpdateValueIfNeeded()) return false;

View File

@ -333,7 +333,7 @@ size_t SBValue::GetByteSize() {
ValueLocker locker;
lldb::ValueObjectSP value_sp(GetSP(locker));
if (value_sp) {
result = value_sp->GetByteSize();
result = value_sp->GetByteSize().getValueOr(0);
}
return result;

View File

@ -905,7 +905,7 @@ protected:
// We're in business.
// Find out the size of this variable.
size = m_option_watchpoint.watch_size == 0
? valobj_sp->GetByteSize()
? valobj_sp->GetByteSize().getValueOr(0)
: m_option_watchpoint.watch_size;
}
compiler_type = valobj_sp->GetCompilerType();

View File

@ -849,7 +849,7 @@ bool ValueObject::SetData(DataExtractor &data, Status &error) {
uint64_t count = 0;
const Encoding encoding = GetCompilerType().GetEncoding(count);
const size_t byte_size = GetByteSize();
const size_t byte_size = GetByteSize().getValueOr(0);
Value::ValueType value_type = m_value.GetValueType();
@ -1524,7 +1524,7 @@ bool ValueObject::SetValueFromCString(const char *value_str, Status &error) {
uint64_t count = 0;
const Encoding encoding = GetCompilerType().GetEncoding(count);
const size_t byte_size = GetByteSize();
const size_t byte_size = GetByteSize().getValueOr(0);
Value::ValueType value_type = m_value.GetValueType();
@ -1739,13 +1739,13 @@ ValueObjectSP ValueObject::GetSyntheticBitFieldChild(uint32_t from, uint32_t to,
uint32_t bit_field_offset = from;
if (GetDataExtractor().GetByteOrder() == eByteOrderBig)
bit_field_offset =
GetByteSize() * 8 - bit_field_size - bit_field_offset;
GetByteSize().getValueOr(0) * 8 - bit_field_size - bit_field_offset;
// We haven't made a synthetic array member for INDEX yet, so lets make
// one and cache it for any future reference.
ValueObjectChild *synthetic_child = new ValueObjectChild(
*this, GetCompilerType(), index_const_str, GetByteSize(), 0,
bit_field_size, bit_field_offset, false, false, eAddressTypeInvalid,
0);
*this, GetCompilerType(), index_const_str,
GetByteSize().getValueOr(0), 0, bit_field_size, bit_field_offset,
false, false, eAddressTypeInvalid, 0);
// Cache the value if we got one back...
if (synthetic_child) {

View File

@ -47,7 +47,7 @@ size_t ValueObjectCast::CalculateNumChildren(uint32_t max) {
return children_count <= max ? children_count : max;
}
uint64_t ValueObjectCast::GetByteSize() {
llvm::Optional<uint64_t> ValueObjectCast::GetByteSize() {
ExecutionContext exe_ctx(GetExecutionContextRef());
return m_value.GetValueByteSize(nullptr, &exe_ctx);
}

View File

@ -40,8 +40,7 @@ ValueObjectConstResult::ValueObjectConstResult(ExecutionContextScope *exe_scope,
ByteOrder byte_order,
uint32_t addr_byte_size,
lldb::addr_t address)
: ValueObject(exe_scope, manager), m_type_name(), m_byte_size(0),
m_impl(this, address) {
: ValueObject(exe_scope, manager), m_impl(this, address) {
SetIsConstant();
SetValueIsValid(true);
m_data.SetByteOrder(byte_order);
@ -64,8 +63,7 @@ ValueObjectConstResult::ValueObjectConstResult(
ExecutionContextScope *exe_scope, ValueObjectManager &manager,
const CompilerType &compiler_type, ConstString name,
const DataExtractor &data, lldb::addr_t address)
: ValueObject(exe_scope, manager), m_type_name(), m_byte_size(0),
m_impl(this, address) {
: ValueObject(exe_scope, manager), m_impl(this, address) {
m_data = data;
if (!m_data.GetSharedDataBuffer()) {
@ -112,8 +110,7 @@ ValueObjectConstResult::ValueObjectConstResult(
const CompilerType &compiler_type, ConstString name,
const lldb::DataBufferSP &data_sp, lldb::ByteOrder data_byte_order,
uint32_t data_addr_size, lldb::addr_t address)
: ValueObject(exe_scope, manager), m_type_name(), m_byte_size(0),
m_impl(this, address) {
: ValueObject(exe_scope, manager), m_impl(this, address) {
m_data.SetByteOrder(data_byte_order);
m_data.SetAddressByteSize(data_addr_size);
m_data.SetData(data_sp);
@ -143,7 +140,7 @@ ValueObjectConstResult::ValueObjectConstResult(
ExecutionContextScope *exe_scope, ValueObjectManager &manager,
const CompilerType &compiler_type, ConstString name, lldb::addr_t address,
AddressType address_type, uint32_t addr_byte_size)
: ValueObject(exe_scope, manager), m_type_name(), m_byte_size(0),
: ValueObject(exe_scope, manager), m_type_name(),
m_impl(this, address) {
m_value.GetScalar() = address;
m_data.SetAddressByteSize(addr_byte_size);
@ -179,8 +176,7 @@ ValueObjectSP ValueObjectConstResult::Create(ExecutionContextScope *exe_scope,
ValueObjectConstResult::ValueObjectConstResult(ExecutionContextScope *exe_scope,
ValueObjectManager &manager,
const Status &error)
: ValueObject(exe_scope, manager), m_type_name(), m_byte_size(0),
m_impl(this) {
: ValueObject(exe_scope, manager), m_impl(this) {
m_error = error;
SetIsConstant();
}
@ -189,8 +185,7 @@ ValueObjectConstResult::ValueObjectConstResult(ExecutionContextScope *exe_scope,
ValueObjectManager &manager,
const Value &value,
ConstString name, Module *module)
: ValueObject(exe_scope, manager), m_type_name(), m_byte_size(0),
m_impl(this) {
: ValueObject(exe_scope, manager), m_impl(this) {
m_value = value;
m_name = name;
ExecutionContext exe_ctx;
@ -208,9 +203,9 @@ lldb::ValueType ValueObjectConstResult::GetValueType() const {
return eValueTypeConstResult;
}
uint64_t ValueObjectConstResult::GetByteSize() {
llvm::Optional<uint64_t> ValueObjectConstResult::GetByteSize() {
ExecutionContext exe_ctx(GetExecutionContextRef());
if (m_byte_size == 0) {
if (!m_byte_size) {
if (auto size =
GetCompilerType().GetByteSize(exe_ctx.GetBestExecutionContextScope()))
SetByteSize(*size);

View File

@ -98,7 +98,7 @@ size_t ValueObjectDynamicValue::CalculateNumChildren(uint32_t max) {
return m_parent->GetNumChildren(max);
}
uint64_t ValueObjectDynamicValue::GetByteSize() {
llvm::Optional<uint64_t> ValueObjectDynamicValue::GetByteSize() {
const bool success = UpdateValueIfNeeded(false);
if (success && m_dynamic_type_info.HasType()) {
ExecutionContext exe_ctx(GetExecutionContextRef());

View File

@ -139,13 +139,11 @@ size_t ValueObjectMemory::CalculateNumChildren(uint32_t max) {
return child_count <= max ? child_count : max;
}
uint64_t ValueObjectMemory::GetByteSize() {
llvm::Optional<uint64_t> ValueObjectMemory::GetByteSize() {
ExecutionContext exe_ctx(GetExecutionContextRef());
if (m_type_sp)
return m_type_sp->GetByteSize(exe_ctx.GetBestExecutionContextScope())
.getValueOr(0);
return m_compiler_type.GetByteSize(exe_ctx.GetBestExecutionContextScope())
.getValueOr(0);
return m_type_sp->GetByteSize(exe_ctx.GetBestExecutionContextScope());
return m_compiler_type.GetByteSize(exe_ctx.GetBestExecutionContextScope());
}
lldb::ValueType ValueObjectMemory::GetValueType() const {

View File

@ -81,7 +81,7 @@ size_t ValueObjectRegisterSet::CalculateNumChildren(uint32_t max) {
return 0;
}
uint64_t ValueObjectRegisterSet::GetByteSize() { return 0; }
llvm::Optional<uint64_t> ValueObjectRegisterSet::GetByteSize() { return 0; }
bool ValueObjectRegisterSet::UpdateValue() {
m_error.Clear();
@ -229,7 +229,9 @@ size_t ValueObjectRegister::CalculateNumChildren(uint32_t max) {
return children_count <= max ? children_count : max;
}
uint64_t ValueObjectRegister::GetByteSize() { return m_reg_info.byte_size; }
llvm::Optional<uint64_t> ValueObjectRegister::GetByteSize() {
return m_reg_info.byte_size;
}
bool ValueObjectRegister::UpdateValue() {
m_error.Clear();

View File

@ -121,7 +121,9 @@ bool ValueObjectSynthetic::MightHaveChildren() {
return (m_might_have_children != eLazyBoolNo);
}
uint64_t ValueObjectSynthetic::GetByteSize() { return m_parent->GetByteSize(); }
llvm::Optional<uint64_t> ValueObjectSynthetic::GetByteSize() {
return m_parent->GetByteSize();
}
lldb::ValueType ValueObjectSynthetic::GetValueType() const {
return m_parent->GetValueType();

View File

@ -105,15 +105,15 @@ size_t ValueObjectVariable::CalculateNumChildren(uint32_t max) {
return child_count <= max ? child_count : max;
}
uint64_t ValueObjectVariable::GetByteSize() {
llvm::Optional<uint64_t> ValueObjectVariable::GetByteSize() {
ExecutionContext exe_ctx(GetExecutionContextRef());
CompilerType type(GetCompilerType());
if (!type.IsValid())
return 0;
return {};
return type.GetByteSize(exe_ctx.GetBestExecutionContextScope()).getValueOr(0);
return type.GetByteSize(exe_ctx.GetBestExecutionContextScope());
}
lldb::ValueType ValueObjectVariable::GetValueType() const {

View File

@ -16,10 +16,10 @@ using namespace lldb_private;
ExpressionVariable::~ExpressionVariable() {}
uint8_t *ExpressionVariable::GetValueBytes() {
const size_t byte_size = m_frozen_sp->GetByteSize();
if (byte_size > 0) {
if (m_frozen_sp->GetDataExtractor().GetByteSize() < byte_size) {
m_frozen_sp->GetValue().ResizeData(byte_size);
llvm::Optional<uint64_t> byte_size = m_frozen_sp->GetByteSize();
if (byte_size && *byte_size) {
if (m_frozen_sp->GetDataExtractor().GetByteSize() < *byte_size) {
m_frozen_sp->GetValue().ResizeData(*byte_size);
m_frozen_sp->GetValue().GetData(m_frozen_sp->GetDataExtractor());
}
return const_cast<uint8_t *>(

View File

@ -67,7 +67,7 @@ public:
const bool zero_memory = false;
lldb::addr_t mem = map.Malloc(
m_persistent_variable_sp->GetByteSize(), 8,
m_persistent_variable_sp->GetByteSize().getValueOr(0), 8,
lldb::ePermissionsReadable | lldb::ePermissionsWritable,
IRMemoryMap::eAllocationPolicyMirror, zero_memory, allocate_error);
@ -106,7 +106,8 @@ public:
Status write_error;
map.WriteMemory(mem, m_persistent_variable_sp->GetValueBytes(),
m_persistent_variable_sp->GetByteSize(), write_error);
m_persistent_variable_sp->GetByteSize().getValueOr(0),
write_error);
if (!write_error.Success()) {
err.SetErrorStringWithFormat(
@ -234,7 +235,7 @@ public:
map.GetBestExecutionContextScope(),
m_persistent_variable_sp.get()->GetCompilerType(),
m_persistent_variable_sp->GetName(), location, eAddressTypeLoad,
m_persistent_variable_sp->GetByteSize());
m_persistent_variable_sp->GetByteSize().getValueOr(0));
if (frame_top != LLDB_INVALID_ADDRESS &&
frame_bottom != LLDB_INVALID_ADDRESS && location >= frame_bottom &&
@ -279,7 +280,8 @@ public:
LLDB_LOGF(log, "Dematerializing %s from 0x%" PRIx64 " (size = %llu)",
m_persistent_variable_sp->GetName().GetCString(),
(uint64_t)mem,
(unsigned long long)m_persistent_variable_sp->GetByteSize());
(unsigned long long)m_persistent_variable_sp->GetByteSize()
.getValueOr(0));
// Read the contents of the spare memory area
@ -288,7 +290,7 @@ public:
Status read_error;
map.ReadMemory(m_persistent_variable_sp->GetValueBytes(), mem,
m_persistent_variable_sp->GetByteSize(), read_error);
m_persistent_variable_sp->GetByteSize().getValueOr(0), read_error);
if (!read_error.Success()) {
err.SetErrorStringWithFormat(
@ -369,10 +371,11 @@ public:
if (!err.Success()) {
dump_stream.Printf(" <could not be read>\n");
} else {
DataBufferHeap data(m_persistent_variable_sp->GetByteSize(), 0);
DataBufferHeap data(
m_persistent_variable_sp->GetByteSize().getValueOr(0), 0);
map.ReadMemory(data.GetBytes(), target_address,
m_persistent_variable_sp->GetByteSize(), err);
m_persistent_variable_sp->GetByteSize().getValueOr(0), err);
if (!err.Success()) {
dump_stream.Printf(" <could not be read>\n");
@ -621,8 +624,8 @@ public:
Status extract_error;
map.GetMemoryData(data, m_temporary_allocation, valobj_sp->GetByteSize(),
extract_error);
map.GetMemoryData(data, m_temporary_allocation,
valobj_sp->GetByteSize().getValueOr(0), extract_error);
if (!extract_error.Success()) {
err.SetErrorStringWithFormat("couldn't get the data for variable %s",
@ -919,7 +922,7 @@ public:
ret->ValueUpdated();
const size_t pvar_byte_size = ret->GetByteSize();
const size_t pvar_byte_size = ret->GetByteSize().getValueOr(0);
uint8_t *pvar_data = ret->GetValueBytes();
map.ReadMemory(pvar_data, address, pvar_byte_size, read_error);

View File

@ -1408,7 +1408,7 @@ ValueObjectSP GetValueForOffset(StackFrame &frame, ValueObjectSP &parent,
}
int64_t child_offset = child_sp->GetByteOffset();
int64_t child_size = child_sp->GetByteSize();
int64_t child_size = child_sp->GetByteSize().getValueOr(0);
if (offset >= child_offset && offset < (child_offset + child_size)) {
return GetValueForOffset(frame, child_sp, offset - child_offset);
@ -1441,8 +1441,8 @@ ValueObjectSP GetValueForDereferincingOffset(StackFrame &frame,
}
if (offset >= 0 && uint64_t(offset) >= pointee->GetByteSize()) {
int64_t index = offset / pointee->GetByteSize();
offset = offset % pointee->GetByteSize();
int64_t index = offset / pointee->GetByteSize().getValueOr(1);
offset = offset % pointee->GetByteSize().getValueOr(1);
const bool can_create = true;
pointee = base->GetSyntheticArrayMember(index, can_create);
}