Support Debugging TLS variable with lldb

[kkcode0]

Summary

Implement qGetTLSAddr for accessing thread pointer and utilises existing facility to calculate TLS variable address (GetThreadLocalData in DYLD).

[kkcode0]

rebase all changes

[kkcode0]

mark some vars used

[kkcode0]

rebased with main

[kkcode0]

enabled the tls test

[jingham]

A tiny quibble: GetThreadPointer seems like it should return a pointer to the thread that has this register set. I would not guess it's a pointer to the Thread's local storage. Maybe GetThreadLSPointer + a doc string saying what it does would clear this up?

[kkcode0]

added comments

[kkcode0]

added comments

[kkcode0]

ping?

[labath]

There's one thing that's not clear to me about this patch. What is the relationship between qGetTLSAddr, as implemented here, and its implementation in GDB? I guess they're not compatible since we're not sending or using the offset and link map fields, but it's not clear to me whether we're at least implementing some subset of gdb functionality (e.g., what would gdbserver do if it received one of the packets that we're sending here). If we're not, then I don't think we should be using the same packet for this purpose.

lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

186–193

How is this related to the rest of the patch?

196

revert whitespace

768

I'm not sure what these marks are (tabs maybe), but could you figure out how to revert them?

lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp

114–117 ↗

(On Diff #496655)

Is this necessary, given that the function is already stubbed out in NativeRegisterContext?

lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h

85 ↗

(On Diff #496655)

Return llvm::Expected<addr_t> perhaps?

lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp

381

Any reason to not use the PtraceWrapper function?

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

2123–2124

Might as well not bother parsing them, if they're going to be thrown away anyway.

lldb/source/Target/Thread.cpp

1651–1653 ↗

(On Diff #496655)

Could this be implemented in ThreadGDBRemote directly (without going through the GDBRemoteRegisterContext)?

[kkcode0]

formatting

[kkcode0]

formatting

[kkcode0]

formatting

[kkcode0]

formatting

[kkcode0]

address review comments

[kkcode0]

spacing

[kkcode0]

lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp

114–117 ↗

(On Diff #496655)

Thanks, it can be removed.

lldb/source/Target/Thread.cpp

1651–1653 ↗

(On Diff #496655)

Yes, Thanks moved it.

[kkcode0]

lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

186–193

LoadVDSO is redundant , it is called via ProbeEntry->EntryBreakpointHit->LoadAllCurrentModules->LoadVDSO

[kkcode0]

use ptracewrapper

lldb/include/lldb/Host/common/NativeRegisterContext.h

public:
 
   virtual uint32_t GetRegisterSetCount() const = 0;
 
   virtual const RegisterSet *GetRegisterSet(uint32_t set_index) const = 0;
 
   virtual Status ReadRegister(const RegisterInfo *reg_info,
                               RegisterValue &reg_value) = 0;
 
+  virtual llvm::Expected<uint64_t> ReadThreadPointer();
+
   virtual Status WriteRegister(const RegisterInfo *reg_info,
                                const RegisterValue &reg_value) = 0;
 
   virtual Status ReadAllRegisterValues(lldb::WritableDataBufferSP &data_sp) = 0;
 
   virtual Status WriteAllRegisterValues(const lldb::DataBufferSP &data_sp) = 0;
 
   uint32_t ConvertRegisterKindToRegisterNumber(uint32_t kind,

lldb/include/lldb/Utility/StringExtractorGDBRemote.h

enum ServerPacketType {
     eServerPacketType_QSetMaxPayloadSize,
     eServerPacketType_QSetEnableAsyncProfiling,
     eServerPacketType_QSyncThreadState,
     eServerPacketType_QThreadSuffixSupported,
 
     eServerPacketType_jThreadsInfo,
     eServerPacketType_qsThreadInfo,
     eServerPacketType_qfThreadInfo,
+    eServerPacketType_qGetTLSAddr,
     eServerPacketType_qGetPid,
     eServerPacketType_qGetProfileData,
     eServerPacketType_qGDBServerVersion,
     eServerPacketType_qMemoryRegionInfo,
     eServerPacketType_qMemoryRegionInfoSupported,
     eServerPacketType_qProcessInfo,
     eServerPacketType_qRcmd,
     eServerPacketType_qRegisterInfo,

lldb/source/Host/common/NativeRegisterContext.cpp

for (uint32_t reg_num_index = 0; reg_num_index < reg_set->num_registers;
       }
     }
   }
 
   // Didn't find it.
   return nullptr;
 }
 
+llvm::Expected<uint64_t> NativeRegisterContext::ReadThreadPointer() {
+  return llvm::createStringError(
+            llvm::inconvertibleErrorCode(),
+            "Architecture does not implement Reading Thread Pointer");
+}
+
 lldb::addr_t NativeRegisterContext::GetPC(lldb::addr_t fail_value) {
   Log *log = GetLog(LLDBLog::Thread);
 
   uint32_t reg = ConvertRegisterKindToRegisterNumber(eRegisterKindGeneric,
                                                      LLDB_REGNUM_GENERIC_PC);
   LLDB_LOGF(log, "Using reg index %" PRIu32 " (default %" PRIu64 ")", reg,
             fail_value);
 

lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp

void DynamicLoaderPOSIXDYLD::DidLaunch() {
 
   if (executable.get() && load_offset != LLDB_INVALID_ADDRESS) {
     ModuleList module_list;
     module_list.Append(executable);
     UpdateLoadedSections(executable, LLDB_INVALID_ADDRESS, load_offset, true);
 
     LLDB_LOGF(log, "DynamicLoaderPOSIXDYLD::%s about to call ProbeEntry()",
               __FUNCTION__);
-
-    if (!SetRendezvousBreakpoint()) {
-      // If we cannot establish rendezvous breakpoint right now we'll try again
-      // at entry point.
-      ProbeEntry();
-    }
-
-    LoadVDSO();
     m_process->GetTarget().ModulesDidLoad(module_list);
   }
 }
 
 Status DynamicLoaderPOSIXDYLD::CanLoadImage() { return Status(); }
 
 void DynamicLoaderPOSIXDYLD::UpdateLoadedSections(ModuleSP module,
                                                   addr_t link_map_addr,
                                                   addr_t base_addr,
                                                   bool base_addr_is_offset) {
   m_loaded_modules[module] = link_map_addr;
   UpdateLoadedSectionsCommon(module, base_addr, base_addr_is_offset);
 }
 
 void DynamicLoaderPOSIXDYLD::UnloadSections(const ModuleSP module) {
   m_loaded_modules.erase(module);
 
   UnloadSectionsCommon(module);
LLDB_LOGF(log,
             "module=%s, link_map=0x%" PRIx64 ", tp=0x%" PRIx64
             ", modid=%" PRId64 ", tls_block=0x%" PRIx64 "\n",
             module_sp->GetObjectName().AsCString(""), link_map, tp,
             (int64_t)modid, tls_block);
 
   if (tls_block == LLDB_INVALID_ADDRESS)
     return LLDB_INVALID_ADDRESS;
   else
     return tls_block + tls_file_addr;
 }
 
 void DynamicLoaderPOSIXDYLD::ResolveExecutableModule(
     lldb::ModuleSP &module_sp) {
   Log *log = GetLog(LLDBLog::DynamicLoader);
 
   if (m_process == nullptr)
     return;

lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h

NativeRegisterContextLinux_x86_64(const ArchSpec &target_arch,
                                     NativeThreadProtocol &native_thread);
 
   uint32_t GetRegisterSetCount() const override;
 
   const RegisterSet *GetRegisterSet(uint32_t set_index) const override;
 
   uint32_t GetUserRegisterCount() const override;
 
+  llvm::Expected<uint64_t> ReadThreadPointer() override;
+
   Status ReadRegister(const RegisterInfo *reg_info,
                       RegisterValue &reg_value) override;
 
   Status WriteRegister(const RegisterInfo *reg_info,
                        const RegisterValue &reg_value) override;
 
   Status ReadAllRegisterValues(lldb::WritableDataBufferSP &data_sp) override;
 

lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp

 //===-- NativeRegisterContextLinux_x86_64.cpp -----------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #if defined(__i386__) || defined(__x86_64__)
 
+#include "Plugins/Process/Linux/NativeProcessLinux.h"
 #include "NativeRegisterContextLinux_x86_64.h"
 #include "Plugins/Process/Linux/NativeThreadLinux.h"
 #include "Plugins/Process/Utility/RegisterContextLinux_i386.h"
 #include "Plugins/Process/Utility/RegisterContextLinux_x86_64.h"
 #include "lldb/Host/HostInfo.h"
+#include "lldb/Host/linux/Ptrace.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/RegisterValue.h"
 #include "lldb/Utility/Status.h"
 #include <cpuid.h>
 #include <linux/elf.h>
 #include <optional>
 
NativeRegisterContextLinux_x86_64::NativeRegisterContextLinux_x86_64(
   // Clear out the FPR state.
   ::memset(m_xstate.get(), 0, xstate_size);
 
   // Store byte offset of fctrl (i.e. first register of FPR)
   const RegisterInfo *reg_info_fctrl = GetRegisterInfoByName("fctrl");
   m_fctrl_offset_in_userarea = reg_info_fctrl->byte_offset;
 }
 
+llvm::Expected<uint64_t> NativeRegisterContextLinux_x86_64::ReadThreadPointer() {
+  Status error;
+  uint64_t tp;
+  error = NativeProcessLinux::PtraceWrapper(
+      PTRACE_ARCH_PRCTL, m_thread.GetID(), (void *)&tp, (void *)ARCH_GET_FS, 0);
+  if (error.Fail())
+    return error.ToError();
+  return tp;
+}
+
 // CONSIDER after local and llgs debugging are merged, register set support can
 // be moved into a base x86-64 class with IsRegisterSetAvailable made virtual.
 uint32_t NativeRegisterContextLinux_x86_64::GetRegisterSetCount() const {
   uint32_t sets = 0;
   for (uint32_t set_index = 0; set_index < k_num_register_sets; ++set_index) {
     if (IsRegisterSetAvailable(set_index))
       ++sets;
   }

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

public:
   Status WriteMemoryTags(lldb::addr_t addr, size_t len, int32_t type,
                          const std::vector<uint8_t> &tags);
 
   /// Use qOffsets to query the offset used when relocating the target
   /// executable. If successful, the returned structure will contain at least
   /// one value in the offsets field.
   std::optional<QOffsets> GetQOffsets();
 
+  lldb::addr_t GetQGetTLSAddr(lldb::tid_t tid, lldb::addr_t offset, lldb::addr_t lms);
+  
   bool GetModuleInfo(const FileSpec &module_file_spec,
                      const ArchSpec &arch_spec, ModuleSpec &module_spec);
 
   std::optional<std::vector<ModuleSpec>>
   GetModulesInfo(llvm::ArrayRef<FileSpec> module_file_specs,
                  const llvm::Triple &triple);
 
   llvm::Expected<std::string> ReadExtFeature(llvm::StringRef object,

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

if (ref.consume_front("Text=")) {
     if (ref.empty())
       return result;
     if (ref.consume_front(";DataSeg=") && GetOffset() && ref.empty())
       return result;
   }
   return std::nullopt;
 }
 
+lldb::addr_t GDBRemoteCommunicationClient::GetQGetTLSAddr(lldb::tid_t tid, lldb::addr_t offset, lldb::addr_t lm) {
+  StreamString packet;
+  packet.PutCString("qGetTLSAddr:");
+  packet.PutHex64(tid, lldb::eByteOrderBig);
+  packet.PutCString(",");
+  packet.PutHex64(offset, lldb::eByteOrderBig);
+  packet.PutCString(",");
+  packet.PutHex64(lm, lldb::eByteOrderBig);
+  StringExtractorGDBRemote response;
+  if (SendPacketAndWaitForResponse(packet.GetString(), response) !=
+      PacketResult::Success)
+    return LLDB_INVALID_ADDRESS;
+  if (response.IsErrorResponse())
+    return LLDB_INVALID_ADDRESS;
+  if (response.IsUnsupportedResponse())
+    return LLDB_INVALID_ADDRESS;
+  llvm::StringRef ref = response.GetStringRef();
+  uint64_t addr = LLDB_INVALID_ADDRESS;
+  ref.consumeInteger(16, addr);
+  return addr;
+}
+
 bool GDBRemoteCommunicationClient::GetModuleInfo(
     const FileSpec &module_file_spec, const lldb_private::ArchSpec &arch_spec,
     ModuleSpec &module_spec) {
   if (!m_supports_qModuleInfo)
     return false;
 
   std::string module_path = module_file_spec.GetPath(false);
   if (module_path.empty())

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

protected:
 
   void AddProcessThreads(StreamGDBRemote &response,
                          NativeProcessProtocol &process, bool &had_any);
 
   PacketResult Handle_qfThreadInfo(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_qsThreadInfo(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_qGetTLSAddr(StringExtractorGDBRemote &packet);
+  
   PacketResult Handle_p(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_P(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_H(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_I(StringExtractorGDBRemote &packet);
 

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

RegisterMemberFunctionHandler(
       &GDBRemoteCommunicationServerLLGS::Handle_QSetDisableASLR);
   RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_QSetWorkingDir,
       &GDBRemoteCommunicationServerLLGS::Handle_QSetWorkingDir);
   RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_qsThreadInfo,
       &GDBRemoteCommunicationServerLLGS::Handle_qsThreadInfo);
   RegisterMemberFunctionHandler(
+      StringExtractorGDBRemote::eServerPacketType_qGetTLSAddr,
+      &GDBRemoteCommunicationServerLLGS::Handle_qGetTLSAddr);
+  RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_qThreadStopInfo,
       &GDBRemoteCommunicationServerLLGS::Handle_qThreadStopInfo);
   RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_jThreadsInfo,
       &GDBRemoteCommunicationServerLLGS::Handle_jThreadsInfo);
   RegisterMemberFunctionHandler(
       StringExtractorGDBRemote::eServerPacketType_qWatchpointSupportInfo,
       &GDBRemoteCommunicationServerLLGS::Handle_qWatchpointSupportInfo);

 GDBRemoteCommunicationServerLLGS::Handle_qsThreadInfo(
     StringExtractorGDBRemote &packet) {
   // FIXME for now we return the full thread list in the initial packet and
   // always do nothing here.
   return SendPacketNoLock("l");
 }
 
 GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerLLGS::Handle_qGetTLSAddr(
+    StringExtractorGDBRemote &packet) {
+  llvm::StringRef s = packet.GetStringRef();
+  if (!s.consume_front("qGetTLSAddr:"))
+    return SendErrorResponse(8);
+  llvm::SmallVector<llvm::StringRef, 16> argv;
+  s.split(argv, ',');
+  lldb::tid_t tid =  StringExtractor(argv[0]).GetU64(LLDB_INVALID_ADDRESS, 16);
+  lldb::addr_t offset = StringExtractor(argv[1]).GetU64(LLDB_INVALID_ADDRESS, 16);
+  lldb::addr_t lm = StringExtractor(argv[2]).GetU64(LLDB_INVALID_ADDRESS, 16);
+  (void) offset;
+  (void) lm;
+  NativeThreadProtocol *ntp = m_current_process->GetThreadByID(tid);
+  NativeRegisterContext &reg_ctx = ntp->GetRegisterContext();
+  StreamGDBRemote response;
+  llvm::Expected<lldb::addr_t> tp = reg_ctx.ReadThreadPointer();
+  if (llvm::Error E = tp.takeError()) {
+    std::string Str;
+    raw_string_ostream OS(Str);
+    OS << E << "\n";
+    Log *log = GetLog(LLDBLog::Thread);
+    LLDB_LOG(log, "{0}", Str);
+    return SendErrorResponse(8);
+  } else {
+    response.PutHex64(*tp, lldb::eByteOrderBig);
+    return SendPacketNoLock(response.GetString());
+  }
+}
+
+GDBRemoteCommunication::PacketResult
 GDBRemoteCommunicationServerLLGS::Handle_g(StringExtractorGDBRemote &packet) {
   Log *log = GetLog(LLDBLog::Thread);
 
   // Move past packet name.
   packet.SetFilePos(strlen("g"));
 
   // Get the thread to use.
   NativeThreadProtocol *thread = GetThreadFromSuffix(packet);

lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h

public:
 
   void SetName(const char *name) override {
     if (name && name[0])
       m_thread_name.assign(name);
     else
       m_thread_name.clear();
   }
 
+  lldb::addr_t GetThreadPointer() override;
+
   lldb::addr_t GetThreadDispatchQAddr() { return m_thread_dispatch_qaddr; }
 
   void SetThreadDispatchQAddr(lldb::addr_t thread_dispatch_qaddr) {
     m_thread_dispatch_qaddr = thread_dispatch_qaddr;
   }
 
   void ClearQueueInfo();
 

lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp

 }
 
 const char *ThreadGDBRemote::GetName() {
   if (m_thread_name.empty())
     return nullptr;
   return m_thread_name.c_str();
 }
 
+lldb::addr_t ThreadGDBRemote::GetThreadPointer() {
+  ProcessSP process_sp(GetProcess());
+  ProcessGDBRemote *gdb_process =
+    static_cast<ProcessGDBRemote *>(process_sp.get());
+  uint64_t tid = this->GetProtocolID();
+  // Return thread pointer here, offset and link_map will be filled by GetThreadLocalData in DYLD
+  return gdb_process->m_gdb_comm.GetQGetTLSAddr(tid, LLDB_INVALID_ADDRESS /* offset */, LLDB_INVALID_ADDRESS /* lm */);
+}
+
 void ThreadGDBRemote::ClearQueueInfo() {
   m_dispatch_queue_name.clear();
   m_queue_kind = eQueueKindUnknown;
   m_queue_serial_number = 0;
   m_dispatch_queue_t = LLDB_INVALID_ADDRESS;
   m_associated_with_libdispatch_queue = eLazyBoolCalculate;
 }
 

lldb/source/Utility/StringExtractorGDBRemote.cpp

case 'G':
       if (PACKET_MATCHES("qGetWorkingDir"))
         return eServerPacketType_qGetWorkingDir;
       if (PACKET_MATCHES("qGetPid"))
         return eServerPacketType_qGetPid;
       if (PACKET_STARTS_WITH("qGetProfileData;"))
         return eServerPacketType_qGetProfileData;
       if (PACKET_MATCHES("qGDBServerVersion"))
         return eServerPacketType_qGDBServerVersion;
+      if (PACKET_STARTS_WITH("qGetTLSAddr"))
+        return eServerPacketType_qGetTLSAddr;
       break;
 
     case 'H':
       if (PACKET_MATCHES("qHostInfo"))
         return eServerPacketType_qHostInfo;
       break;
 
     case 'K':

lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py

def setUp(self):
                     + self.getBuildDir()
                 )
             self.addTearDownHook(
                 lambda: self.runCmd("settings remove target.env-vars " + self.dylibPath)
             )
 
     # TLS works differently on Windows, this would need to be implemented
     # separately.
-    @skipIfWindows
-    @expectedFailureAll(
-        bugnumber="llvm.org/pr28392",
-        oslist=no_match(lldbplatformutil.getDarwinOSTriples()),
-    )
+    @skipIf(archs=no_match(['x86_64']), oslist=no_match(['linux']))
     def test(self):
         """Test thread-local storage."""
         self.build()
         exe = self.getBuildArtifact("a.out")
         target = self.dbg.CreateTarget(exe)
         if self.platformIsDarwin():
             self.registerSharedLibrariesWithTarget(target, ["liba.dylib"])