[Lldb-commits] [lldb] [lldb] Implement thread local storage for linux (PR #67470)

via lldb-commits lldb-commits at lists.llvm.org
Tue Sep 26 17:44:01 PDT 2023


https://github.com/jeffreytan81 updated https://github.com/llvm/llvm-project/pull/67470

>From 5e8b4a44bf48216785f5ecb412e145a7ac4d3a55 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Tue, 26 Sep 2023 11:04:08 -0700
Subject: [PATCH 1/3] Implement thread local storage for linux

---
 lldb/include/lldb/Target/RegisterContext.h    |  2 +
 lldb/include/lldb/lldb-defines.h              |  1 +
 .../POSIX-DYLD/DYLDRendezvous.cpp             | 11 ++---
 .../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp     | 49 +++++++++++++++----
 .../Utility/RegisterInfos_x86_64_with_base.h  | 13 +----
 .../GDBRemoteCommunicationServerLLGS.cpp      |  2 +
 lldb/source/Target/RegisterContext.cpp        |  6 +++
 lldb/source/Target/Thread.cpp                 |  6 ++-
 lldb/source/Utility/Args.cpp                  |  1 +
 .../API/lang/c/tls_globals/TestTlsGlobals.py  |  5 +-
 10 files changed, 65 insertions(+), 31 deletions(-)

diff --git a/lldb/include/lldb/Target/RegisterContext.h b/lldb/include/lldb/Target/RegisterContext.h
index de0efd982daaa71..b2626928f142604 100644
--- a/lldb/include/lldb/Target/RegisterContext.h
+++ b/lldb/include/lldb/Target/RegisterContext.h
@@ -144,6 +144,8 @@ class RegisterContext : public std::enable_shared_from_this<RegisterContext>,
 
   uint64_t GetPC(uint64_t fail_value = LLDB_INVALID_ADDRESS);
 
+  uint64_t GetThreadPointer(uint64_t fail_value = LLDB_INVALID_ADDRESS);
+
   /// Get an address suitable for symbolication.
   /// When symbolicating -- computing line, block, function --
   /// for a function in the middle of the stack, using the return
diff --git a/lldb/include/lldb/lldb-defines.h b/lldb/include/lldb/lldb-defines.h
index ce7fd6f3754516e..aaf0b04e4fb86e9 100644
--- a/lldb/include/lldb/lldb-defines.h
+++ b/lldb/include/lldb/lldb-defines.h
@@ -71,6 +71,7 @@
   11 // The register that would contain pointer size or less argument 7 (if any)
 #define LLDB_REGNUM_GENERIC_ARG8                                               \
   12 // The register that would contain pointer size or less argument 8 (if any)
+#define LLDB_REGNUM_GENERIC_TP 13 // Thread pointer
 /// Invalid value definitions
 #define LLDB_INVALID_STOP_ID 0
 #define LLDB_INVALID_ADDRESS UINT64_MAX
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
index 68e4ac0cc4007c4..cb174d31b86dfe6 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
@@ -710,16 +710,13 @@ bool DYLDRendezvous::FindMetadata(const char *name, PThreadField field,
   target.GetImages().FindSymbolsWithNameAndType(ConstString(name),
                                                 eSymbolTypeAny, list);
   if (list.IsEmpty())
-  return false;
-
-  Address address = list[0].symbol->GetAddress();
-  addr_t addr = address.GetLoadAddress(&target);
-  if (addr == LLDB_INVALID_ADDRESS)
     return false;
 
+  Address address = list[0].symbol->GetAddress();
+  address.SetOffset(address.GetOffset() + field * sizeof(uint32_t));
   Status error;
-  value = (uint32_t)m_process->ReadUnsignedIntegerFromMemory(
-      addr + field * sizeof(uint32_t), sizeof(uint32_t), 0, error);
+  value =
+      target.ReadUnsignedIntegerFromMemory(address, sizeof(uint32_t), 0, error);
   if (error.Fail())
     return false;
 
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
index b4b38a88e1b9c7a..85d7ae9dac75d1e 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
@@ -420,6 +420,11 @@ void DynamicLoaderPOSIXDYLD::RefreshModules() {
   if (!m_rendezvous.Resolve())
     return;
 
+  // The rendezvous class doesn't enumerate the main module, so track that
+  // ourselves here.
+  ModuleSP executable = GetTargetExecutable();
+  m_loaded_modules[executable] = m_rendezvous.GetLinkMapAddress();
+
   DYLDRendezvous::iterator I;
   DYLDRendezvous::iterator E;
 
@@ -727,41 +732,66 @@ lldb::addr_t
 DynamicLoaderPOSIXDYLD::GetThreadLocalData(const lldb::ModuleSP module_sp,
                                            const lldb::ThreadSP thread,
                                            lldb::addr_t tls_file_addr) {
+  Log *log = GetLog(LLDBLog::DynamicLoader);
   auto it = m_loaded_modules.find(module_sp);
-  if (it == m_loaded_modules.end())
+  if (it == m_loaded_modules.end()) {
+    LLDB_LOGF(
+        log, "GetThreadLocalData error: module(%s) not found in loaded modules",
+        module_sp->GetObjectName().AsCString());
     return LLDB_INVALID_ADDRESS;
+  }
 
   addr_t link_map = it->second;
-  if (link_map == LLDB_INVALID_ADDRESS)
+  if (link_map == LLDB_INVALID_ADDRESS || link_map == 0) {
+    LLDB_LOGF(log,
+              "GetThreadLocalData error: invalid link map address=0x%" PRIx64,
+              link_map);
     return LLDB_INVALID_ADDRESS;
+  }
 
   const DYLDRendezvous::ThreadInfo &metadata = m_rendezvous.GetThreadInfo();
-  if (!metadata.valid)
+  if (!metadata.valid) {
+    LLDB_LOGF(log,
+              "GetThreadLocalData error: fail to read thread info metadata");
     return LLDB_INVALID_ADDRESS;
+  }
+
+  LLDB_LOGF(log,
+            "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",
+            link_map, metadata.modid_offset, metadata.dtv_offset,
+            metadata.tls_offset, metadata.dtv_slot_size);
 
   // Get the thread pointer.
   addr_t tp = thread->GetThreadPointer();
-  if (tp == LLDB_INVALID_ADDRESS)
+  if (tp == LLDB_INVALID_ADDRESS) {
+    LLDB_LOGF(log, "GetThreadLocalData error: fail to read thread pointer");
     return LLDB_INVALID_ADDRESS;
+  }
 
   // Find the module's modid.
   int modid_size = 4; // FIXME(spucci): This isn't right for big-endian 64-bit
   int64_t modid = ReadUnsignedIntWithSizeInBytes(
       link_map + metadata.modid_offset, modid_size);
-  if (modid == -1)
+  if (modid == -1) {
+    LLDB_LOGF(log, "GetThreadLocalData error: fail to read modid");
     return LLDB_INVALID_ADDRESS;
+  }
 
   // Lookup the DTV structure for this thread.
   addr_t dtv_ptr = tp + metadata.dtv_offset;
   addr_t dtv = ReadPointer(dtv_ptr);
-  if (dtv == LLDB_INVALID_ADDRESS)
+  if (dtv == LLDB_INVALID_ADDRESS) {
+    LLDB_LOGF(log, "GetThreadLocalData error: fail to read dtv");
     return LLDB_INVALID_ADDRESS;
+  }
 
   // Find the TLS block for this module.
   addr_t dtv_slot = dtv + metadata.dtv_slot_size * modid;
   addr_t tls_block = ReadPointer(dtv_slot + metadata.tls_offset);
 
-  Log *log = GetLog(LLDBLog::DynamicLoader);
   LLDB_LOGF(log,
             "DynamicLoaderPOSIXDYLD::Performed TLS lookup: "
             "module=%s, link_map=0x%" PRIx64 ", tp=0x%" PRIx64
@@ -769,9 +799,10 @@ DynamicLoaderPOSIXDYLD::GetThreadLocalData(const lldb::ModuleSP module_sp,
             module_sp->GetObjectName().AsCString(""), link_map, tp,
             (int64_t)modid, tls_block);
 
-  if (tls_block == LLDB_INVALID_ADDRESS)
+  if (tls_block == LLDB_INVALID_ADDRESS) {
+    LLDB_LOGF(log, "GetThreadLocalData error: fail to read tls_block");
     return LLDB_INVALID_ADDRESS;
-  else
+  } else
     return tls_block + tls_file_addr;
 }
 
diff --git a/lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64_with_base.h b/lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64_with_base.h
index 39428bdd0a08640..b111d8f62d1f3d7 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64_with_base.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64_with_base.h
@@ -71,15 +71,6 @@
         nullptr, nullptr,                                                      \
   }
 
-// Note that the size and offset will be updated by platform-specific classes.
-#define DEFINE_GPR_WITH_BASE(reg, alt, kind1, kind2, kind3, kind4)             \
-  {                                                                            \
-    #reg, alt, sizeof(((GPR *)nullptr)->reg), GPR_OFFSET(reg), eEncodingUint,  \
-        eFormatHex,                                                            \
-        {kind1, kind2, kind3, kind4, x86_64_with_base::lldb_##reg}, nullptr,   \
-        nullptr, nullptr,                                                      \
-  }
-
 #define DEFINE_FPR(name, reg, kind1, kind2, kind3, kind4)                      \
   {                                                                            \
     #name, nullptr, FPR_SIZE(reg), FPR_OFFSET(reg), eEncodingUint, eFormatHex, \
@@ -224,8 +215,8 @@ static RegisterInfo g_register_infos_x86_64_with_base[] = {
     DEFINE_GPR(fs,     nullptr,  dwarf_fs_x86_64,      dwarf_fs_x86_64,      LLDB_INVALID_REGNUM,        LLDB_INVALID_REGNUM),
     DEFINE_GPR(gs,     nullptr,  dwarf_gs_x86_64,      dwarf_gs_x86_64,      LLDB_INVALID_REGNUM,        LLDB_INVALID_REGNUM),
     DEFINE_GPR(ss,     nullptr,  dwarf_ss_x86_64,      dwarf_ss_x86_64,      LLDB_INVALID_REGNUM,        LLDB_INVALID_REGNUM),
-    DEFINE_GPR(fs_base,     nullptr,  dwarf_fs_base_x86_64,      dwarf_fs_base_x86_64,      LLDB_INVALID_REGNUM,        LLDB_INVALID_REGNUM),
-    DEFINE_GPR(gs_base,     nullptr,  dwarf_gs_base_x86_64,      dwarf_gs_base_x86_64,      LLDB_INVALID_REGNUM,        LLDB_INVALID_REGNUM),
+    DEFINE_GPR(fs_base,nullptr,  dwarf_fs_base_x86_64, dwarf_fs_base_x86_64, LLDB_REGNUM_GENERIC_TP,     LLDB_INVALID_REGNUM),
+    DEFINE_GPR(gs_base,nullptr,  dwarf_gs_base_x86_64, dwarf_gs_base_x86_64, LLDB_INVALID_REGNUM,        LLDB_INVALID_REGNUM),
     DEFINE_GPR(ds,     nullptr,  dwarf_ds_x86_64,      dwarf_ds_x86_64,      LLDB_INVALID_REGNUM,        LLDB_INVALID_REGNUM),
     DEFINE_GPR(es,     nullptr,  dwarf_es_x86_64,      dwarf_es_x86_64,      LLDB_INVALID_REGNUM,        LLDB_INVALID_REGNUM),
 
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 65c7a0fabe90ffc..23c2f18cd388a86 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -595,6 +595,8 @@ static llvm::StringRef GetKindGenericOrEmpty(const RegisterInfo &reg_info) {
     return "arg7";
   case LLDB_REGNUM_GENERIC_ARG8:
     return "arg8";
+  case LLDB_REGNUM_GENERIC_TP:
+    return "tp";
   default:
     return "";
   }
diff --git a/lldb/source/Target/RegisterContext.cpp b/lldb/source/Target/RegisterContext.cpp
index 7236a45bff3b948..47c50b8a0154852 100644
--- a/lldb/source/Target/RegisterContext.cpp
+++ b/lldb/source/Target/RegisterContext.cpp
@@ -109,6 +109,12 @@ uint64_t RegisterContext::GetPC(uint64_t fail_value) {
   return pc;
 }
 
+uint64_t RegisterContext::GetThreadPointer(uint64_t fail_value) {
+  uint32_t reg = ConvertRegisterKindToRegisterNumber(eRegisterKindGeneric,
+                                                     LLDB_REGNUM_GENERIC_TP);
+  return ReadRegisterAsUnsigned(reg, fail_value);
+}
+
 bool RegisterContext::SetPC(uint64_t pc) {
   uint32_t reg = ConvertRegisterKindToRegisterNumber(eRegisterKindGeneric,
                                                      LLDB_REGNUM_GENERIC_PC);
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index b6c19a2e0b3626a..6731b76c87b88b6 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -1622,7 +1622,11 @@ void Thread::SettingsInitialize() {}
 
 void Thread::SettingsTerminate() {}
 
-lldb::addr_t Thread::GetThreadPointer() { return LLDB_INVALID_ADDRESS; }
+lldb::addr_t Thread::GetThreadPointer() {
+  if (m_reg_context_sp)
+    return m_reg_context_sp->GetThreadPointer();
+  return LLDB_INVALID_ADDRESS;
+}
 
 addr_t Thread::GetThreadLocalData(const ModuleSP module,
                                   lldb::addr_t tls_file_addr) {
diff --git a/lldb/source/Utility/Args.cpp b/lldb/source/Utility/Args.cpp
index 000321b73694524..152be96a22128b7 100644
--- a/lldb/source/Utility/Args.cpp
+++ b/lldb/source/Utility/Args.cpp
@@ -445,6 +445,7 @@ uint32_t Args::StringToGenericRegister(llvm::StringRef s) {
                         .Case("arg6", LLDB_REGNUM_GENERIC_ARG6)
                         .Case("arg7", LLDB_REGNUM_GENERIC_ARG7)
                         .Case("arg8", LLDB_REGNUM_GENERIC_ARG8)
+                        .Case("tp", LLDB_REGNUM_GENERIC_TP)
                         .Default(LLDB_INVALID_REGNUM);
   return result;
 }
diff --git a/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py b/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
index 863477f9f211051..571b0eac2b46c1a 100644
--- a/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
+++ b/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
@@ -38,9 +38,8 @@ def setUp(self):
     # TLS works differently on Windows, this would need to be implemented
     # separately.
     @skipIfWindows
-    @expectedFailureAll(
-        bugnumber="llvm.org/pr28392",
-        oslist=no_match(lldbplatformutil.getDarwinOSTriples()),
+    @skipIf(
+        oslist=no_match([lldbplatformutil.getDarwinOSTriples(), "linux"])
     )
     def test(self):
         """Test thread-local storage."""

>From ea6704050e6e6004d470a5bfafb8fdc1b8111f94 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Tue, 26 Sep 2023 11:04:08 -0700
Subject: [PATCH 2/3] Implement thread local storage for linux

---
 lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py b/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
index 571b0eac2b46c1a..a853b284c2cce4d 100644
--- a/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
+++ b/lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
@@ -38,9 +38,7 @@ def setUp(self):
     # TLS works differently on Windows, this would need to be implemented
     # separately.
     @skipIfWindows
-    @skipIf(
-        oslist=no_match([lldbplatformutil.getDarwinOSTriples(), "linux"])
-    )
+    @skipIf(oslist=no_match([lldbplatformutil.getDarwinOSTriples(), "linux"]))
     def test(self):
         """Test thread-local storage."""
         self.build()

>From aefd62988ad809b3415ccf56bb5232a2b2fa5597 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Tue, 26 Sep 2023 17:43:35 -0700
Subject: [PATCH 3/3] Add header comments for thread pointer/tls register

---
 lldb/include/lldb/Target/RegisterContext.h                  | 2 ++
 lldb/include/lldb/lldb-defines.h                            | 4 +++-
 .../Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp     | 6 ++++++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/lldb/include/lldb/Target/RegisterContext.h b/lldb/include/lldb/Target/RegisterContext.h
index b2626928f142604..893569a98dbd8b3 100644
--- a/lldb/include/lldb/Target/RegisterContext.h
+++ b/lldb/include/lldb/Target/RegisterContext.h
@@ -144,6 +144,8 @@ class RegisterContext : public std::enable_shared_from_this<RegisterContext>,
 
   uint64_t GetPC(uint64_t fail_value = LLDB_INVALID_ADDRESS);
 
+  // Returns the register value containing thread specific data, like TLS data
+  // and other thread specific stuff.
   uint64_t GetThreadPointer(uint64_t fail_value = LLDB_INVALID_ADDRESS);
 
   /// Get an address suitable for symbolication.
diff --git a/lldb/include/lldb/lldb-defines.h b/lldb/include/lldb/lldb-defines.h
index aaf0b04e4fb86e9..6950a4f3a496acf 100644
--- a/lldb/include/lldb/lldb-defines.h
+++ b/lldb/include/lldb/lldb-defines.h
@@ -71,7 +71,9 @@
   11 // The register that would contain pointer size or less argument 7 (if any)
 #define LLDB_REGNUM_GENERIC_ARG8                                               \
   12 // The register that would contain pointer size or less argument 8 (if any)
-#define LLDB_REGNUM_GENERIC_TP 13 // Thread pointer
+#define LLDB_REGNUM_GENERIC_TP                                                 \
+  13 // The register that would contain thread specific data, like TLS data and
+     // thread control block pointer
 /// Invalid value definitions
 #define LLDB_INVALID_STOP_ID 0
 #define LLDB_INVALID_ADDRESS UINT64_MAX
diff --git a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
index cb174d31b86dfe6..a0b6f44bed0e73b 100644
--- a/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
+++ b/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DYLDRendezvous.cpp
@@ -714,6 +714,12 @@ bool DYLDRendezvous::FindMetadata(const char *name, PThreadField field,
 
   Address address = list[0].symbol->GetAddress();
   address.SetOffset(address.GetOffset() + field * sizeof(uint32_t));
+
+  // Read from target memory as this allows us to try process memory and
+  // fallback to reading from read only sections from the object files. Here we
+  // are reading read only data from libpthread.so to find data in the thread
+  // specific area for the data we want and this won't be saved into process
+  // memory due to it being read only.
   Status error;
   value =
       target.ReadUnsignedIntegerFromMemory(address, sizeof(uint32_t), 0, error);



More information about the lldb-commits mailing list