From 57cd1f6f7ac29604a72e5055a520bc908afd0a56 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Tue, 29 Oct 2019 10:13:43 -0700 Subject: [PATCH] Revert "ValueObject: Fix a crash related to children address type computation" This reverts commit 3116987c0f5a6c88604bae6b16912651e409fa8a. --- lldb/include/lldb/Core/ValueObject.h | 1 - .../lit/SymbolFile/DWARF/DW_OP_piece-struct.s | 113 ------------------ lldb/source/Core/ValueObject.cpp | 53 -------- lldb/source/Core/ValueObjectVariable.cpp | 45 +++++++ 4 files changed, 45 insertions(+), 167 deletions(-) delete mode 100644 lldb/lit/SymbolFile/DWARF/DW_OP_piece-struct.s diff --git a/lldb/include/lldb/Core/ValueObject.h b/lldb/include/lldb/Core/ValueObject.h index 94416eebebb0f..739145bb411fb 100644 --- a/lldb/include/lldb/Core/ValueObject.h +++ b/lldb/include/lldb/Core/ValueObject.h @@ -990,7 +990,6 @@ class ValueObject : public UserID { private: virtual CompilerType MaybeCalculateCompleteType(); - void UpdateChildrenAddressType(); lldb::ValueObjectSP GetValueForExpressionPath_Impl( llvm::StringRef expression_cstr, diff --git a/lldb/lit/SymbolFile/DWARF/DW_OP_piece-struct.s b/lldb/lit/SymbolFile/DWARF/DW_OP_piece-struct.s deleted file mode 100644 index 1809cf94b8bc5..0000000000000 --- a/lldb/lit/SymbolFile/DWARF/DW_OP_piece-struct.s +++ /dev/null @@ -1,113 +0,0 @@ -# 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,"",@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,"",@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: diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp index 95268905764a6..0a643c7d8f119 100644 --- a/lldb/source/Core/ValueObject.cpp +++ b/lldb/source/Core/ValueObject.cpp @@ -152,58 +152,6 @@ 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; @@ -275,7 +223,6 @@ 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 42dc20bf44b6f..e5462fe5e8094 100644 --- a/lldb/source/Core/ValueObjectVariable.cpp +++ b/lldb/source/Core/ValueObjectVariable.cpp @@ -190,6 +190,51 @@ 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; + } // BEGIN Swift if (variable->GetType() && variable->GetType()->IsSwiftFixedValueBuffer())