[Lldb-commits] [lldb] [LLDB] Enable TLS Variable Debugging Without Location Info on AArch64 (PR #110822)

Kamlesh Kumar via lldb-commits lldb-commits at lists.llvm.org
Wed Oct 2 07:38:29 PDT 2024


https://github.com/kamleshbhalui updated https://github.com/llvm/llvm-project/pull/110822

>From ba3071566ac697e750606c7fc941e5f6fc738367 Mon Sep 17 00:00:00 2001
From: Kamlesh Kumar <kamleshbhalui at 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       | 15 +++++----
 .../Process/Utility/RegisterInfos_arm64.h     | 11 +++++--
 .../SymbolFile/DWARF/ManualDWARFIndex.cpp     |  7 ++--
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp      |  9 +++++
 lldb/source/Symbol/Variable.cpp               | 13 ++++++++
 8 files changed, 85 insertions(+), 19 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..320b9cf4a68634 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, KIND_ALL_INVALID),
+    DEFINE_EXTENSION_REG(code_mask, KIND_ALL_INVALID)};
 
 static lldb_private::RegisterInfo g_register_infos_mte[] = {
-    DEFINE_EXTENSION_REG(mte_ctrl)};
+    DEFINE_EXTENSION_REG(mte_ctrl, KIND_ALL_INVALID)};
 
 static lldb_private::RegisterInfo g_register_infos_tls[] = {
-    DEFINE_EXTENSION_REG(tpidr),
+    DEFINE_EXTENSION_REG(tpidr, GENERIC_KIND(LLDB_REGNUM_GENERIC_TP)),
     // Only present when SME is present
-    DEFINE_EXTENSION_REG(tpidr2)};
+    DEFINE_EXTENSION_REG(tpidr2, KIND_ALL_INVALID)};
 
 static lldb_private::RegisterInfo g_register_infos_sme[] = {
-    DEFINE_EXTENSION_REG(svcr),
-    DEFINE_EXTENSION_REG(svg),
+    DEFINE_EXTENSION_REG(svcr, KIND_ALL_INVALID),
+    DEFINE_EXTENSION_REG(svg, KIND_ALL_INVALID),
     // 16 is a default size we will change later.
     {"za", nullptr, 16, 0, lldb::eEncodingVector, lldb::eFormatVectorOfUInt8,
      KIND_ALL_INVALID, nullptr, nullptr, nullptr}};
@@ -95,7 +96,7 @@ static lldb_private::RegisterInfo g_register_infos_sme2[] = {
      KIND_ALL_INVALID, nullptr, nullptr, nullptr}};
 
 static lldb_private::RegisterInfo g_register_infos_fpmr[] = {
-    DEFINE_EXTENSION_REG(fpmr)};
+    DEFINE_EXTENSION_REG(fpmr, KIND_ALL_INVALID)};
 
 // Number of register sets provided by this context.
 enum {
diff --git a/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h b/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
index c9c4d7ceae5573..b1a0a1957effb5 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfos_arm64.h
@@ -470,6 +470,13 @@ static uint32_t g_d31_invalidates[] = {fpu_v31, fpu_s31, LLDB_INVALID_REGNUM};
         LLDB_INVALID_REGNUM, lldb_kind                                         \
   }
 
+// Generates register kinds array for registers with only generic kind
+#define GENERIC_KIND(generic_kind)                                             \
+  {                                                                            \
+    LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, generic_kind,                    \
+        LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM                               \
+  }
+
 // Generates register kinds array for registers with only lldb kind
 #define KIND_ALL_INVALID                                                       \
   {                                                                            \
@@ -535,10 +542,10 @@ 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,                           \
+        kind, 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..d52757a6d255c4 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -230,7 +230,6 @@ void ManualDWARFIndex::IndexUnitImpl(DWARFUnit &unit,
     const char *mangled_cstr = nullptr;
     bool is_declaration = false;
     bool has_address = false;
-    bool has_location_or_const_value = false;
     bool is_global_or_static_variable = false;
 
     DWARFFormValue specification_die_form;
@@ -269,9 +268,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 +359,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..4a1f4d42b66271 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,8 @@ VariableSP SymbolFileDWARF::ParseVariableDIE(const SymbolContext &sc,
       is_artificial = form_value.Boolean();
       break;
     case DW_AT_declaration:
+      is_declaration = form_value.Boolean();
+      break;
     case DW_AT_description:
     case DW_AT_endianity:
     case DW_AT_segment:
@@ -3557,6 +3560,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..bae8313d613d05 100644
--- a/lldb/source/Symbol/Variable.cpp
+++ b/lldb/source/Symbol/Variable.cpp
@@ -438,6 +438,19 @@ 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);
+  if (symbol_indexes.empty())
+    return false;
+  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);



More information about the lldb-commits mailing list