https://github.com/kamleshbhalui created https://github.com/llvm/llvm-project/pull/110822
On AArch64, TLS variables do not have DT_Location entries generated in the debug information due to the lack of dtpoff relocation support, unlike on x86_64. LLDB relies on this location info to calculate the TLS address, leading to issues when debugging TLS variables on AArch64. However, GDB can successfully calculate the TLS address without relying on this debug info, by using the symbol’s address and manually calculating the offset. We adopt a similar approach for LLDB. Fixes #71666 >From 899c26d1a70dd332c3a0d5cd1887fac205752888 Mon Sep 17 00:00:00 2001 From: Kamlesh Kumar <kamleshbha...@gmail.com> Date: Thu, 19 Sep 2024 18:57:14 +0200 Subject: [PATCH] [LLDB] Enable TLS Variable Debugging Without Location Info on AArch64 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On AArch64, TLS variables do not have DT_Location entries generated in the debug information due to the lack of dtpoff relocation support, unlike on x86_64. LLDB relies on this location info to calculate the TLS address, leading to issues when debugging TLS variables on AArch64. However, GDB can successfully calculate the TLS address without relying on this debug info, by using the symbol’s address and manually calculating the offset. We adopt a similar approach for LLDB. --- lldb/include/lldb/Symbol/Variable.h | 2 ++ lldb/source/Core/ValueObjectVariable.cpp | 33 ++++++++++++++++++- .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 14 +++++--- .../Utility/RegisterInfoPOSIX_arm64.cpp | 13 ++++---- .../Process/Utility/RegisterInfos_arm64.h | 5 +-- .../SymbolFile/DWARF/ManualDWARFIndex.cpp | 5 ++- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 8 +++++ lldb/source/Symbol/Variable.cpp | 11 +++++++ 8 files changed, 75 insertions(+), 16 deletions(-) diff --git a/lldb/include/lldb/Symbol/Variable.h b/lldb/include/lldb/Symbol/Variable.h index c437624d1ea6d7..4ad06baeb684a4 100644 --- a/lldb/include/lldb/Symbol/Variable.h +++ b/lldb/include/lldb/Symbol/Variable.h @@ -79,6 +79,8 @@ class Variable : public UserID, public std::enable_shared_from_this<Variable> { return m_location_list; } + bool IsThreadLocal() const; + // When given invalid address, it dumps all locations. Otherwise it only dumps // the location that contains this address. bool DumpLocations(Stream *s, const Address &address); diff --git a/lldb/source/Core/ValueObjectVariable.cpp b/lldb/source/Core/ValueObjectVariable.cpp index 29aefb270c92c8..393ddc1960ab47 100644 --- a/lldb/source/Core/ValueObjectVariable.cpp +++ b/lldb/source/Core/ValueObjectVariable.cpp @@ -254,7 +254,38 @@ bool ValueObjectVariable::UpdateValue() { m_resolved_value.SetContext(Value::ContextType::Invalid, nullptr); } } - + if (m_error.Fail() && variable->IsThreadLocal()) { + ExecutionContext exe_ctx(GetExecutionContextRef()); + Thread *thread = exe_ctx.GetThreadPtr(); + lldb::ModuleSP module_sp = GetModule(); + if (!thread) { + m_error = Status::FromErrorString("no thread to evaluate TLS within"); + return m_error.Success(); + } + std::vector<uint32_t> symbol_indexes; + module_sp->GetSymtab()->FindAllSymbolsWithNameAndType( + ConstString(variable->GetName()), lldb::SymbolType::eSymbolTypeAny, + symbol_indexes); + Symbol *symbol = module_sp->GetSymtab()->SymbolAtIndex(symbol_indexes[0]); + lldb::addr_t tls_file_addr = + symbol->GetAddress().GetOffset() + + symbol->GetAddress().GetSection()->GetFileAddress(); + const lldb::addr_t tls_load_addr = + thread->GetThreadLocalData(module_sp, tls_file_addr); + if (tls_load_addr == LLDB_INVALID_ADDRESS) + m_error = Status::FromErrorString( + "no TLS data currently exists for this thread"); + else { + Value old_value(m_value); + m_value.GetScalar() = tls_load_addr; + m_value.SetContext(Value::ContextType::Variable, variable); + m_value.SetValueType(Value::ValueType::LoadAddress); + m_error = m_value.GetValueAsData(&exe_ctx, m_data, GetModule().get()); + SetValueDidChange(m_value.GetValueType() != old_value.GetValueType() || + m_value.GetScalar() != old_value.GetScalar()); + SetValueIsValid(m_error.Success()); + } + } return m_error.Success(); } diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp index 51e4b3e6728f23..5a78ad2286f87a 100644 --- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp +++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp @@ -771,9 +771,12 @@ DynamicLoaderPOSIXDYLD::GetThreadLocalData(const lldb::ModuleSP module_sp, "GetThreadLocalData info: link_map=0x%" PRIx64 ", thread info metadata: " "modid_offset=0x%" PRIx32 ", dtv_offset=0x%" PRIx32 - ", tls_offset=0x%" PRIx32 ", dtv_slot_size=%" PRIx32 "\n", + ", tls_offset=0x%" PRIx32 ", dtv_slot_size=%" PRIx32 + ", tls_file_addr=0x%" PRIx64 ", module name=%s " + "\n", link_map, metadata.modid_offset, metadata.dtv_offset, - metadata.tls_offset, metadata.dtv_slot_size); + metadata.tls_offset, metadata.dtv_slot_size, tls_file_addr, + module_sp->GetFileSpec().GetFilename().AsCString()); // Get the thread pointer. addr_t tp = thread->GetThreadPointer(); @@ -790,9 +793,12 @@ DynamicLoaderPOSIXDYLD::GetThreadLocalData(const lldb::ModuleSP module_sp, LLDB_LOGF(log, "GetThreadLocalData error: fail to read modid"); return LLDB_INVALID_ADDRESS; } - + const llvm::Triple &triple_ref = + m_process->GetTarget().GetArchitecture().GetTriple(); // Lookup the DTV structure for this thread. - addr_t dtv_ptr = tp + metadata.dtv_offset; + addr_t dtv_ptr = tp; + if (triple_ref.getArch() != llvm::Triple::aarch64) + dtv_ptr = dtv_ptr + metadata.dtv_offset; addr_t dtv = ReadPointer(dtv_ptr); if (dtv == LLDB_INVALID_ADDRESS) { LLDB_LOGF(log, "GetThreadLocalData error: fail to read dtv"); diff --git a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp index f51a93e1b2dcbd..5f80265feda70c 100644 --- a/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp +++ b/lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp @@ -73,19 +73,20 @@ #undef DECLARE_REGISTER_INFOS_ARM64_STRUCT static lldb_private::RegisterInfo g_register_infos_pauth[] = { - DEFINE_EXTENSION_REG(data_mask), DEFINE_EXTENSION_REG(code_mask)}; + DEFINE_EXTENSION_REG(data_mask, LLDB_INVALID_REGNUM), + DEFINE_EXTENSION_REG(code_mask, LLDB_INVALID_REGNUM)}; static lldb_private::RegisterInfo g_register_infos_mte[] = { - DEFINE_EXTENSION_REG(mte_ctrl)}; + DEFINE_EXTENSION_REG(mte_ctrl, LLDB_INVALID_REGNUM)}; static lldb_private::RegisterInfo g_register_infos_tls[] = { - DEFINE_EXTENSION_REG(tpidr), + DEFINE_EXTENSION_REG(tpidr, LLDB_REGNUM_GENERIC_TP), // Only present when SME is present - DEFINE_EXTENSION_REG(tpidr2)}; + DEFINE_EXTENSION_REG(tpidr2, LLDB_INVALID_REGNUM)}; static lldb_private::RegisterInfo g_register_infos_sme[] = { - DEFINE_EXTENSION_REG(svcr), - DEFINE_EXTENSION_REG(svg), + DEFINE_EXTENSION_REG(svcr, LLDB_INVALID_REGNUM), + DEFINE_EXTENSION_REG(svg, LLDB_INVALID_REGNUM), // 16 is a default size we will change later. {"za", nullptr, 16, 0, lldb::eEncodingVector, lldb::eFormatVectorOfUInt8, KIND_ALL_INVALID, nullptr, nullptr, nullptr}}; diff --git a/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h b/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h index c9c4d7ceae5573..29f3bddef317f7 100644 --- a/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h +++ b/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h @@ -535,10 +535,11 @@ static uint32_t g_d31_invalidates[] = {fpu_v31, fpu_s31, LLDB_INVALID_REGNUM}; } // Defines pointer authentication mask registers -#define DEFINE_EXTENSION_REG(reg) \ +#define DEFINE_EXTENSION_REG(reg, kind) \ { \ #reg, nullptr, 8, 0, lldb::eEncodingUint, lldb::eFormatHex, \ - KIND_ALL_INVALID, nullptr, nullptr, nullptr, \ + LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, kind, \ + LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM , nullptr, nullptr, nullptr, \ } static lldb_private::RegisterInfo g_register_infos_arm64_le[] = { diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp index 887983de2e8516..af63706c9e9e78 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp @@ -270,8 +270,6 @@ void ManualDWARFIndex::IndexUnitImpl(DWARFUnit &unit, case DW_AT_location: case DW_AT_const_value: has_location_or_const_value = true; - is_global_or_static_variable = die.IsGlobalOrStaticScopeVariable(); - break; case DW_AT_specification: @@ -363,7 +361,8 @@ void ManualDWARFIndex::IndexUnitImpl(DWARFUnit &unit, break; case DW_TAG_variable: - if (name && has_location_or_const_value && is_global_or_static_variable) { + is_global_or_static_variable = die.IsGlobalOrStaticScopeVariable(); + if (name && is_global_or_static_variable) { set.globals.Insert(ConstString(name), ref); // Be sure to include variables by their mangled and demangled names if // they have any since a variable can have a basename "i", a mangled diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index a40d6d9e37978b..db8d0d5e889b1a 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -3499,6 +3499,7 @@ VariableSP SymbolFileDWARF::ParseVariableDIE(const SymbolContext &sc, DWARFFormValue type_die_form; bool is_external = false; bool is_artificial = false; + bool is_declaration = false; DWARFFormValue const_value_form, location_form; Variable::RangeList scope_ranges; @@ -3545,6 +3546,7 @@ VariableSP SymbolFileDWARF::ParseVariableDIE(const SymbolContext &sc, is_artificial = form_value.Boolean(); break; case DW_AT_declaration: + is_declaration = form_value.Boolean(); case DW_AT_description: case DW_AT_endianity: case DW_AT_segment: @@ -3557,6 +3559,12 @@ VariableSP SymbolFileDWARF::ParseVariableDIE(const SymbolContext &sc, } } + // If It's a declaration then symbol not present in this symbolfile + // return early to try other linked objects. + if (is_declaration) { + return nullptr; + } + // Prefer DW_AT_location over DW_AT_const_value. Both can be emitted e.g. // for static constexpr member variables -- DW_AT_const_value and // DW_AT_location will both be present in the DIE defining the member. diff --git a/lldb/source/Symbol/Variable.cpp b/lldb/source/Symbol/Variable.cpp index d4e1ce43ef1f16..8996846d42a6d8 100644 --- a/lldb/source/Symbol/Variable.cpp +++ b/lldb/source/Symbol/Variable.cpp @@ -438,6 +438,17 @@ Status Variable::GetValuesForVariableExpressionPath( return error; } +bool Variable::IsThreadLocal() const { + ModuleSP module_sp(m_owner_scope->CalculateSymbolContextModule()); + // Give the symbol vendor a chance to add to the unified section list. + module_sp->GetSymbolFile(); + std::vector<uint32_t> symbol_indexes; + module_sp->GetSymtab()->FindAllSymbolsWithNameAndType( + ConstString(GetName()), lldb::SymbolType::eSymbolTypeAny, symbol_indexes); + Symbol *symbol = module_sp->GetSymtab()->SymbolAtIndex(symbol_indexes[0]); + return symbol->GetAddress().GetSection()->IsThreadSpecific(); +} + bool Variable::DumpLocations(Stream *s, const Address &address) { SymbolContext sc; CalculateSymbolContext(&sc); _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits