[Lldb-commits] [lldb] 96601ec - ValueObject: Fix a crash related to children address type computation
Pavel Labath via lldb-commits
lldb-commits at lists.llvm.org
Fri Oct 25 10:49:24 PDT 2019
Author: Pavel Labath
Date: 2019-10-25T17:49:06Z
New Revision: 96601ec28b7efe5abf3479a1aa91bcedb235bbbd
URL: https://github.com/llvm/llvm-project/commit/96601ec28b7efe5abf3479a1aa91bcedb235bbbd
DIFF: https://github.com/llvm/llvm-project/commit/96601ec28b7efe5abf3479a1aa91bcedb235bbbd.diff
LOG: 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
Added:
lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-struct.s
Modified:
lldb/include/lldb/Core/ValueObject.h
lldb/source/Core/ValueObject.cpp
lldb/source/Core/ValueObjectVariable.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Core/ValueObject.h b/lldb/include/lldb/Core/ValueObject.h
index 0808b86c67a1..3ad17364fd81 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -985,6 +985,7 @@ class ValueObject : public UserID {
private:
virtual CompilerType MaybeCalculateCompleteType();
+ void UpdateChildrenAddressType();
lldb::ValueObjectSP GetValueForExpressionPath_Impl(
llvm::StringRef expression_cstr,
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 74176eeace3a..aa03e3b9f499 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -133,6 +133,58 @@ ValueObject::ValueObject(ExecutionContextScope *exe_scope,
// Destructor
ValueObject::~ValueObject() {}
+void ValueObject::UpdateChildrenAddressType() {
+ Value::ValueType value_type = m_value.GetValueType();
+ ExecutionContext exe_ctx(GetExecutionContextRef());
+ Process *process = exe_ctx.GetProcessPtr();
+ const bool process_is_alive = process && process->IsAlive();
+ const uint32_t type_info = GetCompilerType().GetTypeInfo();
+ const bool is_pointer_or_ref =
+ (type_info & (lldb::eTypeIsPointer | lldb::eTypeIsReference)) != 0;
+
+ switch (value_type) {
+ case Value::eValueTypeFileAddress:
+ // If this type is a pointer, then its children will be considered load
+ // addresses if the pointer or reference is dereferenced, but only if
+ // the process is alive.
+ //
+ // There could be global variables like in the following code:
+ // struct LinkedListNode { Foo* foo; LinkedListNode* next; };
+ // Foo g_foo1;
+ // Foo g_foo2;
+ // LinkedListNode g_second_node = { &g_foo2, NULL };
+ // LinkedListNode g_first_node = { &g_foo1, &g_second_node };
+ //
+ // When we aren't running, we should be able to look at these variables
+ // using the "target variable" command. Children of the "g_first_node"
+ // always will be of the same address type as the parent. But children
+ // of the "next" member of LinkedListNode will become load addresses if
+ // we have a live process, or remain a file address if it was a file
+ // address.
+ if (process_is_alive && is_pointer_or_ref)
+ SetAddressTypeOfChildren(eAddressTypeLoad);
+ else
+ SetAddressTypeOfChildren(eAddressTypeFile);
+ break;
+ case Value::eValueTypeHostAddress:
+ // Same as above for load addresses, except children of pointer or refs
+ // are always load addresses. Host addresses are used to store freeze
+ // dried variables. If this type is a struct, the entire struct
+ // contents will be copied into the heap of the
+ // LLDB process, but we do not currently follow any pointers.
+ if (is_pointer_or_ref)
+ SetAddressTypeOfChildren(eAddressTypeLoad);
+ else
+ SetAddressTypeOfChildren(eAddressTypeHost);
+ break;
+ case Value::eValueTypeLoadAddress:
+ case Value::eValueTypeScalar:
+ case Value::eValueTypeVector:
+ SetAddressTypeOfChildren(eAddressTypeLoad);
+ break;
+ }
+}
+
bool ValueObject::UpdateValueIfNeeded(bool update_format) {
bool did_change_formats = false;
@@ -195,6 +247,7 @@ bool ValueObject::UpdateValueIfNeeded(bool update_format) {
SetValueIsValid(success);
if (success) {
+ UpdateChildrenAddressType();
const uint64_t max_checksum_size = 128;
m_data.Checksum(m_value_checksum, max_checksum_size);
} else {
diff --git a/lldb/source/Core/ValueObjectVariable.cpp b/lldb/source/Core/ValueObjectVariable.cpp
index 33f9d5410843..240ebe1fdff3 100644
--- a/lldb/source/Core/ValueObjectVariable.cpp
+++ b/lldb/source/Core/ValueObjectVariable.cpp
@@ -168,51 +168,6 @@ bool ValueObjectVariable::UpdateValue() {
Process *process = exe_ctx.GetProcessPtr();
const bool process_is_alive = process && process->IsAlive();
- const uint32_t type_info = compiler_type.GetTypeInfo();
- const bool is_pointer_or_ref =
- (type_info & (lldb::eTypeIsPointer | lldb::eTypeIsReference)) != 0;
-
- switch (value_type) {
- case Value::eValueTypeFileAddress:
- // If this type is a pointer, then its children will be considered load
- // addresses if the pointer or reference is dereferenced, but only if
- // the process is alive.
- //
- // There could be global variables like in the following code:
- // struct LinkedListNode { Foo* foo; LinkedListNode* next; };
- // Foo g_foo1;
- // Foo g_foo2;
- // LinkedListNode g_second_node = { &g_foo2, NULL };
- // LinkedListNode g_first_node = { &g_foo1, &g_second_node };
- //
- // When we aren't running, we should be able to look at these variables
- // using the "target variable" command. Children of the "g_first_node"
- // always will be of the same address type as the parent. But children
- // of the "next" member of LinkedListNode will become load addresses if
- // we have a live process, or remain what a file address if it what a
- // file address.
- if (process_is_alive && is_pointer_or_ref)
- SetAddressTypeOfChildren(eAddressTypeLoad);
- else
- SetAddressTypeOfChildren(eAddressTypeFile);
- break;
- case Value::eValueTypeHostAddress:
- // Same as above for load addresses, except children of pointer or refs
- // are always load addresses. Host addresses are used to store freeze
- // dried variables. If this type is a struct, the entire struct
- // contents will be copied into the heap of the
- // LLDB process, but we do not currently follow any pointers.
- if (is_pointer_or_ref)
- SetAddressTypeOfChildren(eAddressTypeLoad);
- else
- SetAddressTypeOfChildren(eAddressTypeHost);
- break;
- case Value::eValueTypeLoadAddress:
- case Value::eValueTypeScalar:
- case Value::eValueTypeVector:
- SetAddressTypeOfChildren(eAddressTypeLoad);
- break;
- }
switch (value_type) {
case Value::eValueTypeVector:
diff --git a/lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-struct.s b/lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-struct.s
new file mode 100644
index 000000000000..1809cf94b8bc
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-struct.s
@@ -0,0 +1,113 @@
+# RUN: llvm-mc -filetype=obj -o %t -triple x86_64-pc-linux %s
+# RUN: %lldb %t -o "target variable reset" -b | FileCheck %s
+
+# CHECK: (lldb) target variable reset
+# CHECK: (auto_reset) reset = {
+# CHECK: ptr = 0xdeadbeefbaadf00d
+# CHECK: prev = false
+# CHECK: }
+
+ .section .debug_abbrev,"", at progbits
+ .byte 1 # Abbreviation Code
+ .byte 17 # DW_TAG_compile_unit
+ .byte 1 # DW_CHILDREN_yes
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 2 # Abbreviation Code
+ .byte 52 # DW_TAG_variable
+ .byte 0 # DW_CHILDREN_no
+ .byte 3 # DW_AT_name
+ .byte 8 # DW_FORM_string
+ .byte 73 # DW_AT_type
+ .byte 19 # DW_FORM_ref4
+ .byte 2 # DW_AT_location
+ .byte 24 # DW_FORM_exprloc
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 3 # Abbreviation Code
+ .byte 36 # DW_TAG_base_type
+ .byte 0 # DW_CHILDREN_no
+ .byte 3 # DW_AT_name
+ .byte 8 # DW_FORM_string
+ .byte 62 # DW_AT_encoding
+ .byte 11 # DW_FORM_data1
+ .byte 11 # DW_AT_byte_size
+ .byte 11 # DW_FORM_data1
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 4 # Abbreviation Code
+ .byte 19 # DW_TAG_structure_type
+ .byte 1 # DW_CHILDREN_yes
+ .byte 3 # DW_AT_name
+ .byte 8 # DW_FORM_string
+ .byte 11 # DW_AT_byte_size
+ .byte 11 # DW_FORM_data1
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 5 # Abbreviation Code
+ .byte 13 # DW_TAG_member
+ .byte 0 # DW_CHILDREN_no
+ .byte 3 # DW_AT_name
+ .byte 8 # DW_FORM_string
+ .byte 73 # DW_AT_type
+ .byte 19 # DW_FORM_ref4
+ .byte 56 # DW_AT_data_member_location
+ .byte 11 # DW_FORM_data1
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 6 # Abbreviation Code
+ .byte 15 # DW_TAG_pointer_type
+ .byte 0 # DW_CHILDREN_no
+ .byte 73 # DW_AT_type
+ .byte 19 # DW_FORM_ref4
+ .byte 0 # EOM(1)
+ .byte 0 # EOM(2)
+ .byte 0 # EOM(3)
+
+ .section .debug_info,"", at progbits
+.Lcu_begin0:
+ .long .Lcu_end-.Lcu_start # Length of Unit
+.Lcu_start:
+ .short 4 # DWARF version number
+ .long .debug_abbrev # Offset Into Abbrev. Section
+ .byte 8 # Address Size (in bytes)
+ .byte 1 # Abbrev [1] 0xb:0x6c DW_TAG_compile_unit
+.Lbool:
+ .byte 3 # Abbrev [3] 0x33:0x7 DW_TAG_base_type
+ .asciz "bool" # DW_AT_name
+ .byte 2 # DW_AT_encoding
+ .byte 1 # DW_AT_byte_size
+ .byte 2 # Abbrev [2] 0x3a:0x15 DW_TAG_variable
+ .asciz "reset" # DW_AT_name
+ .long .Lstruct # DW_AT_type
+ .byte 2f-1f # DW_AT_location
+1:
+ .byte 0xe # DW_OP_constu
+ .quad 0xdeadbeefbaadf00d
+ .byte 0x9f # DW_OP_stack_value
+ .byte 0x93 # DW_OP_piece
+ .uleb128 8
+ .byte 0xe # DW_OP_constu
+ .quad 0
+ .byte 0x9f # DW_OP_stack_value
+ .byte 0x93 # DW_OP_piece
+ .uleb128 8
+2:
+.Lstruct:
+ .byte 4 # Abbrev [4] 0x4f:0x22 DW_TAG_structure_type
+ .asciz "auto_reset" # DW_AT_name
+ .byte 16 # DW_AT_byte_size
+ .byte 5 # Abbrev [5] 0x58:0xc DW_TAG_member
+ .asciz "ptr" # DW_AT_name
+ .long .Lbool_ptr # DW_AT_type
+ .byte 0 # DW_AT_data_member_location
+ .byte 5 # Abbrev [5] 0x64:0xc DW_TAG_member
+ .asciz "prev" # DW_AT_name
+ .long .Lbool # DW_AT_type
+ .byte 8 # DW_AT_data_member_location
+ .byte 0 # End Of Children Mark
+.Lbool_ptr:
+ .byte 6 # Abbrev [6] 0x71:0x5 DW_TAG_pointer_type
+ .long .Lbool # DW_AT_type
+ .byte 0 # End Of Children Mark
+.Lcu_end:
More information about the lldb-commits
mailing list