Skip to content

Commit 1d9b860

Browse files
adrian-prantladrian-prantl
committed
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
1 parent c09a108 commit 1d9b860

24 files changed

+62
-57
lines changed

‎lldb/include/lldb/Core/ValueObject.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ class ValueObject : public UserID {
358358
virtual bool CanProvideValue();
359359

360360
// Subclasses must implement the functions below.
361-
virtual uint64_t GetByteSize() = 0;
361+
virtual llvm::Optional<uint64_t> GetByteSize() = 0;
362362

363363
virtual lldb::ValueType GetValueType() const = 0;
364364

‎lldb/include/lldb/Core/ValueObjectCast.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class ValueObjectCast : public ValueObject {
3030
ConstString name,
3131
const CompilerType &cast_type);
3232

33-
uint64_t GetByteSize() override;
33+
llvm::Optional<uint64_t> GetByteSize() override;
3434

3535
size_t CalculateNumChildren(uint32_t max) override;
3636

‎lldb/include/lldb/Core/ValueObjectChild.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ class ValueObjectChild : public ValueObject {
3030
public:
3131
~ValueObjectChild() override;
3232

33-
uint64_t GetByteSize() override { return m_byte_size; }
33+
llvm::Optional<uint64_t> GetByteSize() override { return m_byte_size; }
3434

3535
lldb::offset_t GetByteOffset() override { return m_byte_offset; }
3636

‎lldb/include/lldb/Core/ValueObjectConstResult.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ class ValueObjectConstResult : public ValueObject {
6262
static lldb::ValueObjectSP Create(ExecutionContextScope *exe_scope,
6363
const Status &error);
6464

65-
uint64_t GetByteSize() override;
65+
llvm::Optional<uint64_t> GetByteSize() override;
6666

6767
lldb::ValueType GetValueType() const override;
6868

@@ -113,7 +113,7 @@ class ValueObjectConstResult : public ValueObject {
113113
CompilerType GetCompilerTypeImpl() override;
114114

115115
ConstString m_type_name;
116-
uint64_t m_byte_size;
116+
llvm::Optional<uint64_t> m_byte_size;
117117

118118
ValueObjectConstResultImpl m_impl;
119119

‎lldb/include/lldb/Core/ValueObjectDynamicValue.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class ValueObjectDynamicValue : public ValueObject {
3434
public:
3535
~ValueObjectDynamicValue() override;
3636

37-
uint64_t GetByteSize() override;
37+
llvm::Optional<uint64_t> GetByteSize() override;
3838

3939
ConstString GetTypeName() override;
4040

‎lldb/include/lldb/Core/ValueObjectMemory.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class ValueObjectMemory : public ValueObject {
4040
const Address &address,
4141
const CompilerType &ast_type);
4242

43-
uint64_t GetByteSize() override;
43+
llvm::Optional<uint64_t> GetByteSize() override;
4444

4545
ConstString GetTypeName() override;
4646

‎lldb/include/lldb/Core/ValueObjectRegister.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class ValueObjectRegisterSet : public ValueObject {
3636
lldb::RegisterContextSP &reg_ctx_sp,
3737
uint32_t set_idx);
3838

39-
uint64_t GetByteSize() override;
39+
llvm::Optional<uint64_t> GetByteSize() override;
4040

4141
lldb::ValueType GetValueType() const override {
4242
return lldb::eValueTypeRegisterSet;
@@ -86,7 +86,7 @@ class ValueObjectRegister : public ValueObject {
8686
lldb::RegisterContextSP &reg_ctx_sp,
8787
uint32_t reg_num);
8888

89-
uint64_t GetByteSize() override;
89+
llvm::Optional<uint64_t> GetByteSize() override;
9090

9191
lldb::ValueType GetValueType() const override {
9292
return lldb::eValueTypeRegister;

‎lldb/include/lldb/Core/ValueObjectSyntheticFilter.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class ValueObjectSynthetic : public ValueObject {
3636
public:
3737
~ValueObjectSynthetic() override;
3838

39-
uint64_t GetByteSize() override;
39+
llvm::Optional<uint64_t> GetByteSize() override;
4040

4141
ConstString GetTypeName() override;
4242

‎lldb/include/lldb/Core/ValueObjectVariable.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class ValueObjectVariable : public ValueObject {
3737
static lldb::ValueObjectSP Create(ExecutionContextScope *exe_scope,
3838
const lldb::VariableSP &var_sp);
3939

40-
uint64_t GetByteSize() override;
40+
llvm::Optional<uint64_t> GetByteSize() override;
4141

4242
ConstString GetTypeName() override;
4343

‎lldb/include/lldb/Expression/ExpressionVariable.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class ExpressionVariable
3232

3333
virtual ~ExpressionVariable();
3434

35-
size_t GetByteSize() { return m_frozen_sp->GetByteSize(); }
35+
llvm::Optional<uint64_t> GetByteSize() { return m_frozen_sp->GetByteSize(); }
3636

3737
ConstString GetName() { return m_frozen_sp->GetName(); }
3838

‎lldb/include/lldb/Target/StackFrameRecognizer.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,9 @@ class ValueObjectRecognizerSynthesizedValue : public ValueObject {
154154
SetName(parent.GetName());
155155
}
156156

157-
uint64_t GetByteSize() override { return m_parent->GetByteSize(); }
157+
llvm::Optional<uint64_t> GetByteSize() override {
158+
return m_parent->GetByteSize();
159+
}
158160
lldb::ValueType GetValueType() const override { return m_type; }
159161
bool UpdateValue() override {
160162
if (!m_parent->UpdateValueIfNeeded()) return false;

‎lldb/source/API/SBValue.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,7 @@ size_t SBValue::GetByteSize() {
333333
ValueLocker locker;
334334
lldb::ValueObjectSP value_sp(GetSP(locker));
335335
if (value_sp) {
336-
result = value_sp->GetByteSize();
336+
result = value_sp->GetByteSize().getValueOr(0);
337337
}
338338

339339
return result;

‎lldb/source/Commands/CommandObjectWatchpoint.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -905,7 +905,7 @@ corresponding to the byte size of the data type.");
905905
// We're in business.
906906
// Find out the size of this variable.
907907
size = m_option_watchpoint.watch_size == 0
908-
? valobj_sp->GetByteSize()
908+
? valobj_sp->GetByteSize().getValueOr(0)
909909
: m_option_watchpoint.watch_size;
910910
}
911911
compiler_type = valobj_sp->GetCompilerType();

‎lldb/source/Core/ValueObject.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -849,7 +849,7 @@ bool ValueObject::SetData(DataExtractor &data, Status &error) {
849849
uint64_t count = 0;
850850
const Encoding encoding = GetCompilerType().GetEncoding(count);
851851

852-
const size_t byte_size = GetByteSize();
852+
const size_t byte_size = GetByteSize().getValueOr(0);
853853

854854
Value::ValueType value_type = m_value.GetValueType();
855855

@@ -1524,7 +1524,7 @@ bool ValueObject::SetValueFromCString(const char *value_str, Status &error) {
15241524
uint64_t count = 0;
15251525
const Encoding encoding = GetCompilerType().GetEncoding(count);
15261526

1527-
const size_t byte_size = GetByteSize();
1527+
const size_t byte_size = GetByteSize().getValueOr(0);
15281528

15291529
Value::ValueType value_type = m_value.GetValueType();
15301530

@@ -1741,13 +1741,13 @@ ValueObjectSP ValueObject::GetSyntheticBitFieldChild(uint32_t from, uint32_t to,
17411741
uint32_t bit_field_offset = from;
17421742
if (GetDataExtractor().GetByteOrder() == eByteOrderBig)
17431743
bit_field_offset =
1744-
GetByteSize() * 8 - bit_field_size - bit_field_offset;
1744+
GetByteSize().getValueOr(0) * 8 - bit_field_size - bit_field_offset;
17451745
// We haven't made a synthetic array member for INDEX yet, so lets make
17461746
// one and cache it for any future reference.
17471747
ValueObjectChild *synthetic_child = new ValueObjectChild(
1748-
*this, GetCompilerType(), index_const_str, GetByteSize(), 0,
1749-
bit_field_size, bit_field_offset, false, false, eAddressTypeInvalid,
1750-
0);
1748+
*this, GetCompilerType(), index_const_str,
1749+
GetByteSize().getValueOr(0), 0, bit_field_size, bit_field_offset,
1750+
false, false, eAddressTypeInvalid, 0);
17511751

17521752
// Cache the value if we got one back...
17531753
if (synthetic_child) {

‎lldb/source/Core/ValueObjectCast.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ size_t ValueObjectCast::CalculateNumChildren(uint32_t max) {
4747
return children_count <= max ? children_count : max;
4848
}
4949

50-
uint64_t ValueObjectCast::GetByteSize() {
50+
llvm::Optional<uint64_t> ValueObjectCast::GetByteSize() {
5151
ExecutionContext exe_ctx(GetExecutionContextRef());
5252
return m_value.GetValueByteSize(nullptr, &exe_ctx);
5353
}

‎lldb/source/Core/ValueObjectConstResult.cpp

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,7 @@ ValueObjectSP ValueObjectConstResult::Create(ExecutionContextScope *exe_scope,
179179
ValueObjectConstResult::ValueObjectConstResult(ExecutionContextScope *exe_scope,
180180
ValueObjectManager &manager,
181181
const Status &error)
182-
: ValueObject(exe_scope, manager), m_type_name(), m_byte_size(0),
183-
m_impl(this) {
182+
: ValueObject(exe_scope, manager), m_impl(this) {
184183
m_error = error;
185184
SetIsConstant();
186185
}
@@ -189,8 +188,7 @@ ValueObjectConstResult::ValueObjectConstResult(ExecutionContextScope *exe_scope,
189188
ValueObjectManager &manager,
190189
const Value &value,
191190
ConstString name, Module *module)
192-
: ValueObject(exe_scope, manager), m_type_name(), m_byte_size(0),
193-
m_impl(this) {
191+
: ValueObject(exe_scope, manager), m_impl(this) {
194192
m_value = value;
195193
m_name = name;
196194
ExecutionContext exe_ctx;
@@ -208,9 +206,9 @@ lldb::ValueType ValueObjectConstResult::GetValueType() const {
208206
return eValueTypeConstResult;
209207
}
210208

211-
uint64_t ValueObjectConstResult::GetByteSize() {
209+
llvm::Optional<uint64_t> ValueObjectConstResult::GetByteSize() {
212210
ExecutionContext exe_ctx(GetExecutionContextRef());
213-
if (m_byte_size == 0) {
211+
if (!m_byte_size) {
214212
if (auto size =
215213
GetCompilerType().GetByteSize(exe_ctx.GetBestExecutionContextScope()))
216214
SetByteSize(*size);

‎lldb/source/Core/ValueObjectDynamicValue.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ size_t ValueObjectDynamicValue::CalculateNumChildren(uint32_t max) {
9898
return m_parent->GetNumChildren(max);
9999
}
100100

101-
uint64_t ValueObjectDynamicValue::GetByteSize() {
101+
llvm::Optional<uint64_t> ValueObjectDynamicValue::GetByteSize() {
102102
const bool success = UpdateValueIfNeeded(false);
103103
if (success && m_dynamic_type_info.HasType()) {
104104
ExecutionContext exe_ctx(GetExecutionContextRef());

‎lldb/source/Core/ValueObjectMemory.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -139,13 +139,11 @@ size_t ValueObjectMemory::CalculateNumChildren(uint32_t max) {
139139
return child_count <= max ? child_count : max;
140140
}
141141

142-
uint64_t ValueObjectMemory::GetByteSize() {
142+
llvm::Optional<uint64_t> ValueObjectMemory::GetByteSize() {
143143
ExecutionContext exe_ctx(GetExecutionContextRef());
144144
if (m_type_sp)
145-
return m_type_sp->GetByteSize(exe_ctx.GetBestExecutionContextScope())
146-
.getValueOr(0);
147-
return m_compiler_type.GetByteSize(exe_ctx.GetBestExecutionContextScope())
148-
.getValueOr(0);
145+
return m_type_sp->GetByteSize(exe_ctx.GetBestExecutionContextScope());
146+
return m_compiler_type.GetByteSize(exe_ctx.GetBestExecutionContextScope());
149147
}
150148

151149
lldb::ValueType ValueObjectMemory::GetValueType() const {

‎lldb/source/Core/ValueObjectRegister.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ size_t ValueObjectRegisterSet::CalculateNumChildren(uint32_t max) {
8181
return 0;
8282
}
8383

84-
uint64_t ValueObjectRegisterSet::GetByteSize() { return 0; }
84+
llvm::Optional<uint64_t> ValueObjectRegisterSet::GetByteSize() { return 0; }
8585

8686
bool ValueObjectRegisterSet::UpdateValue() {
8787
m_error.Clear();
@@ -229,7 +229,9 @@ size_t ValueObjectRegister::CalculateNumChildren(uint32_t max) {
229229
return children_count <= max ? children_count : max;
230230
}
231231

232-
uint64_t ValueObjectRegister::GetByteSize() { return m_reg_info.byte_size; }
232+
llvm::Optional<uint64_t> ValueObjectRegister::GetByteSize() {
233+
return m_reg_info.byte_size;
234+
}
233235

234236
bool ValueObjectRegister::UpdateValue() {
235237
m_error.Clear();

‎lldb/source/Core/ValueObjectSyntheticFilter.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,9 @@ bool ValueObjectSynthetic::MightHaveChildren() {
121121
return (m_might_have_children != eLazyBoolNo);
122122
}
123123

124-
uint64_t ValueObjectSynthetic::GetByteSize() { return m_parent->GetByteSize(); }
124+
llvm::Optional<uint64_t> ValueObjectSynthetic::GetByteSize() {
125+
return m_parent->GetByteSize();
126+
}
125127

126128
lldb::ValueType ValueObjectSynthetic::GetValueType() const {
127129
return m_parent->GetValueType();

‎lldb/source/Core/ValueObjectVariable.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,15 +105,15 @@ size_t ValueObjectVariable::CalculateNumChildren(uint32_t max) {
105105
return child_count <= max ? child_count : max;
106106
}
107107

108-
uint64_t ValueObjectVariable::GetByteSize() {
108+
llvm::Optional<uint64_t> ValueObjectVariable::GetByteSize() {
109109
ExecutionContext exe_ctx(GetExecutionContextRef());
110110

111111
CompilerType type(GetCompilerType());
112112

113113
if (!type.IsValid())
114-
return 0;
114+
return {};
115115

116-
return type.GetByteSize(exe_ctx.GetBestExecutionContextScope()).getValueOr(0);
116+
return type.GetByteSize(exe_ctx.GetBestExecutionContextScope());
117117
}
118118

119119
lldb::ValueType ValueObjectVariable::GetValueType() const {

‎lldb/source/Expression/ExpressionVariable.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@ using namespace lldb_private;
1616
ExpressionVariable::~ExpressionVariable() {}
1717

1818
uint8_t *ExpressionVariable::GetValueBytes() {
19-
const size_t byte_size = m_frozen_sp->GetByteSize();
20-
if (byte_size > 0) {
21-
if (m_frozen_sp->GetDataExtractor().GetByteSize() < byte_size) {
22-
m_frozen_sp->GetValue().ResizeData(byte_size);
19+
llvm::Optional<uint64_t> byte_size = m_frozen_sp->GetByteSize();
20+
if (byte_size && *byte_size) {
21+
if (m_frozen_sp->GetDataExtractor().GetByteSize() < *byte_size) {
22+
m_frozen_sp->GetValue().ResizeData(*byte_size);
2323
m_frozen_sp->GetValue().GetData(m_frozen_sp->GetDataExtractor());
2424
}
2525
return const_cast<uint8_t *>(

‎lldb/source/Expression/Materializer.cpp

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ class EntityPersistentVariable : public Materializer::Entity {
6767
const bool zero_memory = false;
6868

6969
lldb::addr_t mem = map.Malloc(
70-
m_persistent_variable_sp->GetByteSize(), 8,
70+
m_persistent_variable_sp->GetByteSize().getValueOr(0), 8,
7171
lldb::ePermissionsReadable | lldb::ePermissionsWritable,
7272
IRMemoryMap::eAllocationPolicyMirror, zero_memory, allocate_error);
7373

@@ -106,7 +106,8 @@ class EntityPersistentVariable : public Materializer::Entity {
106106
Status write_error;
107107

108108
map.WriteMemory(mem, m_persistent_variable_sp->GetValueBytes(),
109-
m_persistent_variable_sp->GetByteSize(), write_error);
109+
m_persistent_variable_sp->GetByteSize().getValueOr(0),
110+
write_error);
110111

111112
if (!write_error.Success()) {
112113
err.SetErrorStringWithFormat(
@@ -234,7 +235,7 @@ class EntityPersistentVariable : public Materializer::Entity {
234235
map.GetBestExecutionContextScope(),
235236
m_persistent_variable_sp.get()->GetCompilerType(),
236237
m_persistent_variable_sp->GetName(), location, eAddressTypeLoad,
237-
m_persistent_variable_sp->GetByteSize());
238+
m_persistent_variable_sp->GetByteSize().getValueOr(0));
238239

239240
if (frame_top != LLDB_INVALID_ADDRESS &&
240241
frame_bottom != LLDB_INVALID_ADDRESS && location >= frame_bottom &&
@@ -279,7 +280,8 @@ class EntityPersistentVariable : public Materializer::Entity {
279280
LLDB_LOGF(log, "Dematerializing %s from 0x%" PRIx64 " (size = %llu)",
280281
m_persistent_variable_sp->GetName().GetCString(),
281282
(uint64_t)mem,
282-
(unsigned long long)m_persistent_variable_sp->GetByteSize());
283+
(unsigned long long)m_persistent_variable_sp->GetByteSize()
284+
.getValueOr(0));
283285

284286
// Read the contents of the spare memory area
285287

@@ -288,7 +290,7 @@ class EntityPersistentVariable : public Materializer::Entity {
288290
Status read_error;
289291

290292
map.ReadMemory(m_persistent_variable_sp->GetValueBytes(), mem,
291-
m_persistent_variable_sp->GetByteSize(), read_error);
293+
m_persistent_variable_sp->GetByteSize().getValueOr(0), read_error);
292294

293295
if (!read_error.Success()) {
294296
err.SetErrorStringWithFormat(
@@ -369,10 +371,11 @@ class EntityPersistentVariable : public Materializer::Entity {
369371
if (!err.Success()) {
370372
dump_stream.Printf(" <could not be read>\n");
371373
} else {
372-
DataBufferHeap data(m_persistent_variable_sp->GetByteSize(), 0);
374+
DataBufferHeap data(
375+
m_persistent_variable_sp->GetByteSize().getValueOr(0), 0);
373376

374377
map.ReadMemory(data.GetBytes(), target_address,
375-
m_persistent_variable_sp->GetByteSize(), err);
378+
m_persistent_variable_sp->GetByteSize().getValueOr(0), err);
376379

377380
if (!err.Success()) {
378381
dump_stream.Printf(" <could not be read>\n");
@@ -621,8 +624,8 @@ class EntityVariable : public Materializer::Entity {
621624

622625
Status extract_error;
623626

624-
map.GetMemoryData(data, m_temporary_allocation, valobj_sp->GetByteSize(),
625-
extract_error);
627+
map.GetMemoryData(data, m_temporary_allocation,
628+
valobj_sp->GetByteSize().getValueOr(0), extract_error);
626629

627630
if (!extract_error.Success()) {
628631
err.SetErrorStringWithFormat("couldn't get the data for variable %s",
@@ -919,7 +922,7 @@ class EntityResultVariable : public Materializer::Entity {
919922

920923
ret->ValueUpdated();
921924

922-
const size_t pvar_byte_size = ret->GetByteSize();
925+
const size_t pvar_byte_size = ret->GetByteSize().getValueOr(0);
923926
uint8_t *pvar_data = ret->GetValueBytes();
924927

925928
map.ReadMemory(pvar_data, address, pvar_byte_size, read_error);

0 commit comments

Comments
 (0)