Skip to content

Commit 3116987

Browse files
labathDavide Italiano
labath
authored and
Davide Italiano
committed
ValueObject: Fix a crash related to children address type computation
Summary: This patch fixes a crash encountered when debugging optimized code. If some variable has been completely optimized out, but it's value is nonetheless known, the compiler can replace it with a DWARF expression computing its value. The evaluating these expressions results in a eValueTypeHostAddress Value object, as it's contents are computed into an lldb buffer. However, any value that is obtained by dereferencing pointers in this object should no longer have the "host" address type. Lldb had code to account for this, but it was only present in the ValueObjectVariable class. This wasn't enough when the object being described was a struct, as then the object holding the actual pointer was a ValueObjectChild. This caused lldb to dereference the contained pointer in the context of the host process and crash. Though I am not an expert on ValueObjects, it seems to me that this children address type logic should apply to all types of objects (and indeed, applying applying the same logic to ValueObjectChild fixes the crash). Therefore, I move this code to the base class, and arrange it to be run everytime the value is updated. The test case is a reduced and simplified version of the original debug info triggering the crash. Originally we were dealing with a local variable, but as these require a running process to display, I changed it to use a global one instead. Reviewers: jingham, clayborg Subscribers: aprantl, lldb-commits Differential Revision: https://reviews.llvm.org/D69273
1 parent 506e24c commit 3116987

File tree

4 files changed

+167
-45
lines changed

4 files changed

+167
-45
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -985,6 +985,7 @@ class ValueObject : public UserID {
985985

986986
private:
987987
virtual CompilerType MaybeCalculateCompleteType();
988+
void UpdateChildrenAddressType();
988989

989990
lldb::ValueObjectSP GetValueForExpressionPath_Impl(
990991
llvm::StringRef expression_cstr,

‎lldb/lit/SymbolFile/DWARF/DW_OP_piece-struct.s

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-pc-linux %s
2+
# RUN: %lldb %t -o "target variable reset" -b | FileCheck %s
3+
4+
# CHECK: (lldb) target variable reset
5+
# CHECK: (auto_reset) reset = {
6+
# CHECK: ptr = 0xdeadbeefbaadf00d
7+
# CHECK: prev = false
8+
# CHECK: }
9+
10+
.section .debug_abbrev,"",@progbits
11+
.byte 1 # Abbreviation Code
12+
.byte 17 # DW_TAG_compile_unit
13+
.byte 1 # DW_CHILDREN_yes
14+
.byte 0 # EOM(1)
15+
.byte 0 # EOM(2)
16+
.byte 2 # Abbreviation Code
17+
.byte 52 # DW_TAG_variable
18+
.byte 0 # DW_CHILDREN_no
19+
.byte 3 # DW_AT_name
20+
.byte 8 # DW_FORM_string
21+
.byte 73 # DW_AT_type
22+
.byte 19 # DW_FORM_ref4
23+
.byte 2 # DW_AT_location
24+
.byte 24 # DW_FORM_exprloc
25+
.byte 0 # EOM(1)
26+
.byte 0 # EOM(2)
27+
.byte 3 # Abbreviation Code
28+
.byte 36 # DW_TAG_base_type
29+
.byte 0 # DW_CHILDREN_no
30+
.byte 3 # DW_AT_name
31+
.byte 8 # DW_FORM_string
32+
.byte 62 # DW_AT_encoding
33+
.byte 11 # DW_FORM_data1
34+
.byte 11 # DW_AT_byte_size
35+
.byte 11 # DW_FORM_data1
36+
.byte 0 # EOM(1)
37+
.byte 0 # EOM(2)
38+
.byte 4 # Abbreviation Code
39+
.byte 19 # DW_TAG_structure_type
40+
.byte 1 # DW_CHILDREN_yes
41+
.byte 3 # DW_AT_name
42+
.byte 8 # DW_FORM_string
43+
.byte 11 # DW_AT_byte_size
44+
.byte 11 # DW_FORM_data1
45+
.byte 0 # EOM(1)
46+
.byte 0 # EOM(2)
47+
.byte 5 # Abbreviation Code
48+
.byte 13 # DW_TAG_member
49+
.byte 0 # DW_CHILDREN_no
50+
.byte 3 # DW_AT_name
51+
.byte 8 # DW_FORM_string
52+
.byte 73 # DW_AT_type
53+
.byte 19 # DW_FORM_ref4
54+
.byte 56 # DW_AT_data_member_location
55+
.byte 11 # DW_FORM_data1
56+
.byte 0 # EOM(1)
57+
.byte 0 # EOM(2)
58+
.byte 6 # Abbreviation Code
59+
.byte 15 # DW_TAG_pointer_type
60+
.byte 0 # DW_CHILDREN_no
61+
.byte 73 # DW_AT_type
62+
.byte 19 # DW_FORM_ref4
63+
.byte 0 # EOM(1)
64+
.byte 0 # EOM(2)
65+
.byte 0 # EOM(3)
66+
67+
.section .debug_info,"",@progbits
68+
.Lcu_begin0:
69+
.long .Lcu_end-.Lcu_start # Length of Unit
70+
.Lcu_start:
71+
.short 4 # DWARF version number
72+
.long .debug_abbrev # Offset Into Abbrev. Section
73+
.byte 8 # Address Size (in bytes)
74+
.byte 1 # Abbrev [1] 0xb:0x6c DW_TAG_compile_unit
75+
.Lbool:
76+
.byte 3 # Abbrev [3] 0x33:0x7 DW_TAG_base_type
77+
.asciz "bool" # DW_AT_name
78+
.byte 2 # DW_AT_encoding
79+
.byte 1 # DW_AT_byte_size
80+
.byte 2 # Abbrev [2] 0x3a:0x15 DW_TAG_variable
81+
.asciz "reset" # DW_AT_name
82+
.long .Lstruct # DW_AT_type
83+
.byte 2f-1f # DW_AT_location
84+
1:
85+
.byte 0xe # DW_OP_constu
86+
.quad 0xdeadbeefbaadf00d
87+
.byte 0x9f # DW_OP_stack_value
88+
.byte 0x93 # DW_OP_piece
89+
.uleb128 8
90+
.byte 0xe # DW_OP_constu
91+
.quad 0
92+
.byte 0x9f # DW_OP_stack_value
93+
.byte 0x93 # DW_OP_piece
94+
.uleb128 8
95+
2:
96+
.Lstruct:
97+
.byte 4 # Abbrev [4] 0x4f:0x22 DW_TAG_structure_type
98+
.asciz "auto_reset" # DW_AT_name
99+
.byte 16 # DW_AT_byte_size
100+
.byte 5 # Abbrev [5] 0x58:0xc DW_TAG_member
101+
.asciz "ptr" # DW_AT_name
102+
.long .Lbool_ptr # DW_AT_type
103+
.byte 0 # DW_AT_data_member_location
104+
.byte 5 # Abbrev [5] 0x64:0xc DW_TAG_member
105+
.asciz "prev" # DW_AT_name
106+
.long .Lbool # DW_AT_type
107+
.byte 8 # DW_AT_data_member_location
108+
.byte 0 # End Of Children Mark
109+
.Lbool_ptr:
110+
.byte 6 # Abbrev [6] 0x71:0x5 DW_TAG_pointer_type
111+
.long .Lbool # DW_AT_type
112+
.byte 0 # End Of Children Mark
113+
.Lcu_end:

‎lldb/source/Core/ValueObject.cpp

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,58 @@ ValueObject::ValueObject(ExecutionContextScope *exe_scope,
135135
// Destructor
136136
ValueObject::~ValueObject() {}
137137

138+
void ValueObject::UpdateChildrenAddressType() {
139+
Value::ValueType value_type = m_value.GetValueType();
140+
ExecutionContext exe_ctx(GetExecutionContextRef());
141+
Process *process = exe_ctx.GetProcessPtr();
142+
const bool process_is_alive = process && process->IsAlive();
143+
const uint32_t type_info = GetCompilerType().GetTypeInfo();
144+
const bool is_pointer_or_ref =
145+
(type_info & (lldb::eTypeIsPointer | lldb::eTypeIsReference)) != 0;
146+
147+
switch (value_type) {
148+
case Value::eValueTypeFileAddress:
149+
// If this type is a pointer, then its children will be considered load
150+
// addresses if the pointer or reference is dereferenced, but only if
151+
// the process is alive.
152+
//
153+
// There could be global variables like in the following code:
154+
// struct LinkedListNode { Foo* foo; LinkedListNode* next; };
155+
// Foo g_foo1;
156+
// Foo g_foo2;
157+
// LinkedListNode g_second_node = { &g_foo2, NULL };
158+
// LinkedListNode g_first_node = { &g_foo1, &g_second_node };
159+
//
160+
// When we aren't running, we should be able to look at these variables
161+
// using the "target variable" command. Children of the "g_first_node"
162+
// always will be of the same address type as the parent. But children
163+
// of the "next" member of LinkedListNode will become load addresses if
164+
// we have a live process, or remain a file address if it was a file
165+
// address.
166+
if (process_is_alive && is_pointer_or_ref)
167+
SetAddressTypeOfChildren(eAddressTypeLoad);
168+
else
169+
SetAddressTypeOfChildren(eAddressTypeFile);
170+
break;
171+
case Value::eValueTypeHostAddress:
172+
// Same as above for load addresses, except children of pointer or refs
173+
// are always load addresses. Host addresses are used to store freeze
174+
// dried variables. If this type is a struct, the entire struct
175+
// contents will be copied into the heap of the
176+
// LLDB process, but we do not currently follow any pointers.
177+
if (is_pointer_or_ref)
178+
SetAddressTypeOfChildren(eAddressTypeLoad);
179+
else
180+
SetAddressTypeOfChildren(eAddressTypeHost);
181+
break;
182+
case Value::eValueTypeLoadAddress:
183+
case Value::eValueTypeScalar:
184+
case Value::eValueTypeVector:
185+
SetAddressTypeOfChildren(eAddressTypeLoad);
186+
break;
187+
}
188+
}
189+
138190
bool ValueObject::UpdateValueIfNeeded(bool update_format) {
139191

140192
bool did_change_formats = false;
@@ -197,6 +249,7 @@ bool ValueObject::UpdateValueIfNeeded(bool update_format) {
197249
SetValueIsValid(success);
198250

199251
if (success) {
252+
UpdateChildrenAddressType();
200253
const uint64_t max_checksum_size = 128;
201254
m_data.Checksum(m_value_checksum, max_checksum_size);
202255
} else {

‎lldb/source/Core/ValueObjectVariable.cpp

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -168,51 +168,6 @@ bool ValueObjectVariable::UpdateValue() {
168168

169169
Process *process = exe_ctx.GetProcessPtr();
170170
const bool process_is_alive = process && process->IsAlive();
171-
const uint32_t type_info = compiler_type.GetTypeInfo();
172-
const bool is_pointer_or_ref =
173-
(type_info & (lldb::eTypeIsPointer | lldb::eTypeIsReference)) != 0;
174-
175-
switch (value_type) {
176-
case Value::eValueTypeFileAddress:
177-
// If this type is a pointer, then its children will be considered load
178-
// addresses if the pointer or reference is dereferenced, but only if
179-
// the process is alive.
180-
//
181-
// There could be global variables like in the following code:
182-
// struct LinkedListNode { Foo* foo; LinkedListNode* next; };
183-
// Foo g_foo1;
184-
// Foo g_foo2;
185-
// LinkedListNode g_second_node = { &g_foo2, NULL };
186-
// LinkedListNode g_first_node = { &g_foo1, &g_second_node };
187-
//
188-
// When we aren't running, we should be able to look at these variables
189-
// using the "target variable" command. Children of the "g_first_node"
190-
// always will be of the same address type as the parent. But children
191-
// of the "next" member of LinkedListNode will become load addresses if
192-
// we have a live process, or remain what a file address if it what a
193-
// file address.
194-
if (process_is_alive && is_pointer_or_ref)
195-
SetAddressTypeOfChildren(eAddressTypeLoad);
196-
else
197-
SetAddressTypeOfChildren(eAddressTypeFile);
198-
break;
199-
case Value::eValueTypeHostAddress:
200-
// Same as above for load addresses, except children of pointer or refs
201-
// are always load addresses. Host addresses are used to store freeze
202-
// dried variables. If this type is a struct, the entire struct
203-
// contents will be copied into the heap of the
204-
// LLDB process, but we do not currently follow any pointers.
205-
if (is_pointer_or_ref)
206-
SetAddressTypeOfChildren(eAddressTypeLoad);
207-
else
208-
SetAddressTypeOfChildren(eAddressTypeHost);
209-
break;
210-
case Value::eValueTypeLoadAddress:
211-
case Value::eValueTypeScalar:
212-
case Value::eValueTypeVector:
213-
SetAddressTypeOfChildren(eAddressTypeLoad);
214-
break;
215-
}
216171

217172
switch (value_type) {
218173
case Value::eValueTypeVector:

0 commit comments

Comments
 (0)