[Lldb-commits] [PATCH] D143698: Support Debugging TLS variable with lldb

2023-09-02 Thread Kamlesh Kumar via Phabricator via lldb-commits 
kkcode0 updated this revision to Diff 72.
kkcode0 added a comment.

formatting


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143698/new/

https://reviews.llvm.org/D143698

Files:
  lldb/include/lldb/Host/common/NativeRegisterContext.h
  lldb/include/lldb/Target/RegisterContext.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Host/common/NativeRegisterContext.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Target/RegisterContext.cpp
  lldb/source/Target/Thread.cpp
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py

Index: lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
===
--- lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
+++ lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
@@ -37,11 +37,7 @@
 
 # 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()
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -210,6 +210,8 @@
 return eServerPacketType_qGetProfileData;
   if (PACKET_MATCHES("qGDBServerVersion"))
 return eServerPacketType_qGDBServerVersion;
+  if (PACKET_STARTS_WITH("qGetTLSAddr"))
+	  return eServerPacketType_qGetTLSAddr;
   break;
 
 case 'H':
Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -1623,7 +1623,11 @@
 
 void Thread::SettingsTerminate() {}
 
-lldb::addr_t Thread::GetThreadPointer() { return LLDB_INVALID_ADDRESS; }
+lldb::addr_t Thread::GetThreadPointer() {
+  RegisterContext *reg_ctx = this->GetRegisterContext().get();
+  auto tp = reg_ctx->GetThreadPointer();
+  return tp;
+}
 
 addr_t Thread::GetThreadLocalData(const ModuleSP module,
   lldb::addr_t tls_file_addr) {
Index: lldb/source/Target/RegisterContext.cpp
===
--- lldb/source/Target/RegisterContext.cpp
+++ lldb/source/Target/RegisterContext.cpp
@@ -29,6 +29,10 @@
 
 RegisterContext::~RegisterContext() = default;
 
+uint64_t RegisterContext::GetThreadPointer() {
+  return UINT64_MAX;
+}
+
 void RegisterContext::InvalidateIfNeeded(bool force) {
   ProcessSP process_sp(m_thread.GetProcess());
   bool invalidate = force;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
@@ -55,6 +55,8 @@
 
   const RegisterInfo *GetRegisterInfoAtIndex(size_t reg) override;
 
+  uint64_t GetThreadPointer() override;
+
   size_t GetRegisterSetCount() override;
 
   const RegisterSet *GetRegisterSet(size_t reg_set) override;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -80,6 +80,19 @@
   return m_reg_info_sp->GetRegisterSet(reg_set);
 }
 
+uint64_t GDBRemoteRegisterContext::GetThreadPointer() {
+  ExecutionContext exe_ctx(CalculateThread());
+  Process *process = exe_ctx.GetProcessPtr();
+  Thread *thread = exe_ctx.GetThreadPtr();
+  if (process == nullptr || thread == nullptr)
+return LLDB_INVALID_ADDRESS;
+  GDBRemoteCommunicationClient &gdb_comm(
+  ((ProcessGDBRemote *)process)->GetGDBRemote());
+  uint64_t tid = thread->GetProtocolID();
+  // Return thread poi

[Lldb-commits] [PATCH] D143698: Support Debugging TLS variable with lldb

2023-09-02 Thread Kamlesh Kumar via Phabricator via lldb-commits 
kkcode0 updated this revision to Diff 73.
kkcode0 added a comment.

formatting


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143698/new/

https://reviews.llvm.org/D143698

Files:
  lldb/include/lldb/Host/common/NativeRegisterContext.h
  lldb/include/lldb/Target/RegisterContext.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Host/common/NativeRegisterContext.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Target/RegisterContext.cpp
  lldb/source/Target/Thread.cpp
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py

Index: lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
===
--- lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
+++ lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
@@ -37,11 +37,7 @@
 
 # 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()
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -210,6 +210,8 @@
 return eServerPacketType_qGetProfileData;
   if (PACKET_MATCHES("qGDBServerVersion"))
 return eServerPacketType_qGDBServerVersion;
+  if (PACKET_STARTS_WITH("qGetTLSAddr"))
+	  return eServerPacketType_qGetTLSAddr;
   break;
 
 case 'H':
Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -1623,7 +1623,11 @@
 
 void Thread::SettingsTerminate() {}
 
-lldb::addr_t Thread::GetThreadPointer() { return LLDB_INVALID_ADDRESS; }
+lldb::addr_t Thread::GetThreadPointer() {
+  RegisterContext *reg_ctx = this->GetRegisterContext().get();
+  auto tp = reg_ctx->GetThreadPointer();
+  return tp;
+}
 
 addr_t Thread::GetThreadLocalData(const ModuleSP module,
   lldb::addr_t tls_file_addr) {
Index: lldb/source/Target/RegisterContext.cpp
===
--- lldb/source/Target/RegisterContext.cpp
+++ lldb/source/Target/RegisterContext.cpp
@@ -29,6 +29,10 @@
 
 RegisterContext::~RegisterContext() = default;
 
+uint64_t RegisterContext::GetThreadPointer() {
+  return UINT64_MAX;
+}
+
 void RegisterContext::InvalidateIfNeeded(bool force) {
   ProcessSP process_sp(m_thread.GetProcess());
   bool invalidate = force;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
@@ -55,6 +55,8 @@
 
   const RegisterInfo *GetRegisterInfoAtIndex(size_t reg) override;
 
+  uint64_t GetThreadPointer() override;
+
   size_t GetRegisterSetCount() override;
 
   const RegisterSet *GetRegisterSet(size_t reg_set) override;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -80,6 +80,19 @@
   return m_reg_info_sp->GetRegisterSet(reg_set);
 }
 
+uint64_t GDBRemoteRegisterContext::GetThreadPointer() {
+  ExecutionContext exe_ctx(CalculateThread());
+  Process *process = exe_ctx.GetProcessPtr();
+  Thread *thread = exe_ctx.GetThreadPtr();
+  if (process == nullptr || thread == nullptr)
+return LLDB_INVALID_ADDRESS;
+  GDBRemoteCommunicationClient &gdb_comm(
+  ((ProcessGDBRemote *)process)->GetGDBRemote());
+  uint64_t tid = thread->GetProtocolID();
+  // Return thread poi

[Lldb-commits] [PATCH] D143698: Support Debugging TLS variable with lldb

2023-09-02 Thread Kamlesh Kumar via Phabricator via lldb-commits 
kkcode0 updated this revision to Diff 74.
kkcode0 marked an inline comment as done.
kkcode0 added a comment.

formatting


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143698/new/

https://reviews.llvm.org/D143698

Files:
  lldb/include/lldb/Host/common/NativeRegisterContext.h
  lldb/include/lldb/Target/RegisterContext.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Host/common/NativeRegisterContext.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Target/RegisterContext.cpp
  lldb/source/Target/Thread.cpp
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py

Index: lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
===
--- lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
+++ lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
@@ -37,11 +37,7 @@
 
 # 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()
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -210,6 +210,8 @@
 return eServerPacketType_qGetProfileData;
   if (PACKET_MATCHES("qGDBServerVersion"))
 return eServerPacketType_qGDBServerVersion;
+  if (PACKET_STARTS_WITH("qGetTLSAddr"))
+	  return eServerPacketType_qGetTLSAddr;
   break;
 
 case 'H':
Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -1623,7 +1623,11 @@
 
 void Thread::SettingsTerminate() {}
 
-lldb::addr_t Thread::GetThreadPointer() { return LLDB_INVALID_ADDRESS; }
+lldb::addr_t Thread::GetThreadPointer() {
+  RegisterContext *reg_ctx = this->GetRegisterContext().get();
+  auto tp = reg_ctx->GetThreadPointer();
+  return tp;
+}
 
 addr_t Thread::GetThreadLocalData(const ModuleSP module,
   lldb::addr_t tls_file_addr) {
Index: lldb/source/Target/RegisterContext.cpp
===
--- lldb/source/Target/RegisterContext.cpp
+++ lldb/source/Target/RegisterContext.cpp
@@ -29,6 +29,10 @@
 
 RegisterContext::~RegisterContext() = default;
 
+uint64_t RegisterContext::GetThreadPointer() {
+  return UINT64_MAX;
+}
+
 void RegisterContext::InvalidateIfNeeded(bool force) {
   ProcessSP process_sp(m_thread.GetProcess());
   bool invalidate = force;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
@@ -55,6 +55,8 @@
 
   const RegisterInfo *GetRegisterInfoAtIndex(size_t reg) override;
 
+  uint64_t GetThreadPointer() override;
+
   size_t GetRegisterSetCount() override;
 
   const RegisterSet *GetRegisterSet(size_t reg_set) override;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -80,6 +80,19 @@
   return m_reg_info_sp->GetRegisterSet(reg_set);
 }
 
+uint64_t GDBRemoteRegisterContext::GetThreadPointer() {
+  ExecutionContext exe_ctx(CalculateThread());
+  Process *process = exe_ctx.GetProcessPtr();
+  Thread *thread = exe_ctx.GetThreadPtr();
+  if (process == nullptr || thread == nullptr)
+return LLDB_INVALID_ADDRESS;
+  GDBRemoteCommunicationClient &gdb_comm(
+  ((ProcessGDBRemote *)process)->GetGDBRemote());
+  uint64_t tid = thread

[Lldb-commits] [PATCH] D143698: Support Debugging TLS variable with lldb

2023-09-02 Thread Kamlesh Kumar via Phabricator via lldb-commits 
kkcode0 updated this revision to Diff 75.
kkcode0 added a comment.

formatting


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143698/new/

https://reviews.llvm.org/D143698

Files:
  lldb/include/lldb/Host/common/NativeRegisterContext.h
  lldb/include/lldb/Target/RegisterContext.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Host/common/NativeRegisterContext.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
  lldb/source/Target/RegisterContext.cpp
  lldb/source/Target/Thread.cpp
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py

Index: lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
===
--- lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
+++ lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
@@ -37,11 +37,7 @@
 
 # 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()
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -210,6 +210,8 @@
 return eServerPacketType_qGetProfileData;
   if (PACKET_MATCHES("qGDBServerVersion"))
 return eServerPacketType_qGDBServerVersion;
+  if (PACKET_STARTS_WITH("qGetTLSAddr"))
+	  return eServerPacketType_qGetTLSAddr;
   break;
 
 case 'H':
Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -1623,7 +1623,11 @@
 
 void Thread::SettingsTerminate() {}
 
-lldb::addr_t Thread::GetThreadPointer() { return LLDB_INVALID_ADDRESS; }
+lldb::addr_t Thread::GetThreadPointer() {
+  RegisterContext *reg_ctx = this->GetRegisterContext().get();
+  auto tp = reg_ctx->GetThreadPointer();
+  return tp;
+}
 
 addr_t Thread::GetThreadLocalData(const ModuleSP module,
   lldb::addr_t tls_file_addr) {
Index: lldb/source/Target/RegisterContext.cpp
===
--- lldb/source/Target/RegisterContext.cpp
+++ lldb/source/Target/RegisterContext.cpp
@@ -29,6 +29,10 @@
 
 RegisterContext::~RegisterContext() = default;
 
+uint64_t RegisterContext::GetThreadPointer() {
+  return UINT64_MAX;
+}
+
 void RegisterContext::InvalidateIfNeeded(bool force) {
   ProcessSP process_sp(m_thread.GetProcess());
   bool invalidate = force;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.h
@@ -55,6 +55,8 @@
 
   const RegisterInfo *GetRegisterInfoAtIndex(size_t reg) override;
 
+  uint64_t GetThreadPointer() override;
+
   size_t GetRegisterSetCount() override;
 
   const RegisterSet *GetRegisterSet(size_t reg_set) override;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp
@@ -80,6 +80,19 @@
   return m_reg_info_sp->GetRegisterSet(reg_set);
 }
 
+uint64_t GDBRemoteRegisterContext::GetThreadPointer() {
+  ExecutionContext exe_ctx(CalculateThread());
+  Process *process = exe_ctx.GetProcessPtr();
+  Thread *thread = exe_ctx.GetThreadPtr();
+  if (process == nullptr || thread == nullptr)
+return LLDB_INVALID_ADDRESS;
+  GDBRemoteCommunicationClient &gdb_comm(
+  ((ProcessGDBRemote *)process)->GetGDBRemote());
+  uint64_t tid = thread->GetProtocolID();
+  // Return thread poi

lldb-commits@lists.llvm.org

2023-09-02 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib accepted this revision.
mib added a comment.

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159387/new/

https://reviews.llvm.org/D159387

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D143698: Support Debugging TLS variable with lldb

2023-09-02 Thread Kamlesh Kumar via Phabricator via lldb-commits 
kkcode0 updated this revision to Diff 91.
kkcode0 marked 3 inline comments as done.
kkcode0 added a comment.

address review comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143698/new/

https://reviews.llvm.org/D143698

Files:
  lldb/include/lldb/Host/common/NativeRegisterContext.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Host/common/NativeRegisterContext.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py

Index: lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
===
--- lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
+++ lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
@@ -37,11 +37,7 @@
 
 # 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()
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -210,6 +210,8 @@
 return eServerPacketType_qGetProfileData;
   if (PACKET_MATCHES("qGDBServerVersion"))
 return eServerPacketType_qGDBServerVersion;
+  if (PACKET_STARTS_WITH("qGetTLSAddr"))
+	  return eServerPacketType_qGetTLSAddr;
   break;
 
 case 'H':
Index: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
@@ -71,6 +71,8 @@
   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) {
Index: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
@@ -66,6 +66,15 @@
   return m_thread_name.c_str();
 }
 
+lldb::addr_t ThreadGDBRemote::GetThreadPointer() {
+  ProcessSP process_sp(GetProcess());
+  ProcessGDBRemote *gdb_process =
+static_cast(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;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -185,6 +185,8 @@
 
   PacketResult Handle_qsThreadInfo(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_qGetTLSAddr(StringExtractorGDBRemote &packet);
+  
   PacketResult Handle_p(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_P(StringExtractorGDBRemote &packet);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -148,6 +148,9 @@
   RegisterMemberFunctionHandler(
   StringExtractorGDBRemote::eServerPacketType_qsThreadInfo,
   &GDBRemoteCommunicationServerLLGS::Handle_qsThreadInfo);
+  RegisterMemberFunctionHandler(
+  StringExtractorGDBRemote::eServerPacketType_qGetTLSAddr,
+

[Lldb-commits] [PATCH] D143698: Support Debugging TLS variable with lldb

2023-09-02 Thread Kamlesh Kumar via Phabricator via lldb-commits 
kkcode0 updated this revision to Diff 92.
kkcode0 marked an inline comment as done.
kkcode0 added a comment.

spacing


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143698/new/

https://reviews.llvm.org/D143698

Files:
  lldb/include/lldb/Host/common/NativeRegisterContext.h
  lldb/include/lldb/Utility/StringExtractorGDBRemote.h
  lldb/source/Host/common/NativeRegisterContext.cpp
  lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
  lldb/source/Utility/StringExtractorGDBRemote.cpp
  lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py

Index: lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
===
--- lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
+++ lldb/test/API/lang/c/tls_globals/TestTlsGlobals.py
@@ -37,11 +37,7 @@
 
 # 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()
Index: lldb/source/Utility/StringExtractorGDBRemote.cpp
===
--- lldb/source/Utility/StringExtractorGDBRemote.cpp
+++ lldb/source/Utility/StringExtractorGDBRemote.cpp
@@ -210,6 +210,8 @@
 return eServerPacketType_qGetProfileData;
   if (PACKET_MATCHES("qGDBServerVersion"))
 return eServerPacketType_qGDBServerVersion;
+  if (PACKET_STARTS_WITH("qGetTLSAddr"))
+	  return eServerPacketType_qGetTLSAddr;
   break;
 
 case 'H':
Index: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
===
--- lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
+++ lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.h
@@ -71,6 +71,8 @@
   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) {
Index: lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ThreadGDBRemote.cpp
@@ -66,6 +66,15 @@
   return m_thread_name.c_str();
 }
 
+lldb::addr_t ThreadGDBRemote::GetThreadPointer() {
+  ProcessSP process_sp(GetProcess());
+  ProcessGDBRemote *gdb_process =
+static_cast(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;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -185,6 +185,8 @@
 
   PacketResult Handle_qsThreadInfo(StringExtractorGDBRemote &packet);
 
+  PacketResult Handle_qGetTLSAddr(StringExtractorGDBRemote &packet);
+  
   PacketResult Handle_p(StringExtractorGDBRemote &packet);
 
   PacketResult Handle_P(StringExtractorGDBRemote &packet);
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -148,6 +148,9 @@
   RegisterMemberFunctionHandler(
   StringExtractorGDBRemote::eServerPacketType_qsThreadInfo,
   &GDBRemoteCommunicationServerLLGS::Handle_qsThreadInfo);
+  RegisterMemberFunctionHandler(
+  StringExtractorGDBRemote::eServerPacketType_qGetTLSAddr,
+  &GDBRemoteCommunicationServerLLGS::Handle_qGetTLSAddr);
   RegisterMemberFun

[Lldb-commits] [PATCH] D143698: Support Debugging TLS variable with lldb

2023-09-02 Thread Kamlesh Kumar via Phabricator via lldb-commits 
kkcode0 added inline comments.



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux.cpp:114-117
+  tp = LLDB_INVALID_ADDRESS;
+  Status error;
+  return error;
+}

labath wrote:
> Is this necessary, given that the function is already stubbed out in 
> `NativeRegisterContext`?
Thanks, it can be removed.



Comment at: lldb/source/Target/Thread.cpp:1651-1653
+  RegisterContext *reg_ctx = this->GetRegisterContext().get();
+  auto tp = reg_ctx->GetThreadPointer();
+  return tp;

labath wrote:
> Could this be implemented in ThreadGDBRemote directly (without going through 
> the GDBRemoteRegisterContext)?
Yes, Thanks moved it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143698/new/

https://reviews.llvm.org/D143698

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D143698: Support Debugging TLS variable with lldb

2023-09-02 Thread Kamlesh Kumar via Phabricator via lldb-commits 
kkcode0 added inline comments.



Comment at: 
lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:186-193
-
-if (!SetRendezvousBreakpoint()) {
-  // If we cannot establish rendezvous breakpoint right now we'll try again
-  // at entry point.
-  ProbeEntry();
-}
-

labath wrote:
> How is this related to the rest of the patch?
LoadVDSO is redundant , it is called via 
ProbeEntry->EntryBreakpointHit->LoadAllCurrentModules->LoadVDSO


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143698/new/

https://reviews.llvm.org/D143698

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 5dd9568 - Fix typos in documentation

2023-09-02 Thread Kazu Hirata via lldb-commits 

Author: Kazu Hirata
Date: 2023-09-02T09:32:48-07:00
New Revision: 5dd956871785a5b04903d6da85109c9e1052a8a8

URL: 
https://github.com/llvm/llvm-project/commit/5dd956871785a5b04903d6da85109c9e1052a8a8
DIFF: 
https://github.com/llvm/llvm-project/commit/5dd956871785a5b04903d6da85109c9e1052a8a8.diff

LOG: Fix typos in documentation

Added: 


Modified: 
clang/docs/analyzer/checkers.rst
libc/docs/dev/code_style.rst
lldb/docs/use/tutorial.rst
llvm/docs/LangRef.rst

Removed: 




diff  --git a/clang/docs/analyzer/checkers.rst 
b/clang/docs/analyzer/checkers.rst
index ac6919e4c98297..54ea49e7426cc8 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -2481,13 +2481,13 @@ input refers to a valid file and removing any invalid 
user input.
 if (!filename[0])
   return -1;
 strcat(cmd, filename);
-system(cmd); // Superflous Warning: Untrusted data is passed to a system 
call
+system(cmd); // Superfluous Warning: Untrusted data is passed to a system 
call
   }
 
 Unfortunately, the checker cannot discover automatically that the programmer
 have performed data sanitation, so it still emits the warning.
 
-One can get rid of this superflous warning by telling by specifying the
+One can get rid of this superfluous warning by telling by specifying the
 sanitation functions in the taint configuration file (see
 :doc:`user-docs/TaintAnalysisConfiguration`).
 

diff  --git a/libc/docs/dev/code_style.rst b/libc/docs/dev/code_style.rst
index 0c3171b7c95027..4b03217b18c33f 100644
--- a/libc/docs/dev/code_style.rst
+++ b/libc/docs/dev/code_style.rst
@@ -39,7 +39,7 @@ We define two kinds of macros:
 
* ``src/__support/macros/properties/`` - Build related properties like
  target architecture or enabled CPU features defined by introspecting
- compiler defined preprocessor defininitions.
+ compiler defined preprocessor definitions.
 
  * ``architectures.h`` - Target architecture properties.
e.g., ``LIBC_TARGET_ARCH_IS_ARM``.

diff  --git a/lldb/docs/use/tutorial.rst b/lldb/docs/use/tutorial.rst
index ef268fff775d59..85dc173fa37cd3 100644
--- a/lldb/docs/use/tutorial.rst
+++ b/lldb/docs/use/tutorial.rst
@@ -360,7 +360,7 @@ That is better, but suffers from the problem that when new 
breakpoints get
 added, they don't pick up these modifications, and the options only exist in
 the context of actual breakpoints, so they are hard to store & reuse.
 
-A even better solution is to make a fully configured breakpoint name:
+An even better solution is to make a fully configured breakpoint name:
 
 ::
 

diff  --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index cddb20ca6a6f06..d7fde35f63a3ee 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -7159,7 +7159,7 @@ It is illegal for the list node to be empty since it 
might be confused
 with an access group.
 
 The access group metadata node must be 'distinct' to avoid collapsing
-multiple access groups by content. A access group metadata node must
+multiple access groups by content. An access group metadata node must
 always be empty which can be used to distinguish an access group
 metadata node from a list of access groups. Being empty avoids the
 situation that the content must be updated which, because metadata is



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D159408: Switch over to using the LLVM archive parser for BSD archives.

2023-09-02 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg created this revision.
clayborg added reviewers: labath, JDevlieghere, GeorgeHuyubo, yinghuitan, 
kusmour.
Herald added a project: All.
clayborg requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Our LLDB parser didn't correctly handle archives of all flavors on different 
systems, it currently only correctly handled BSD archives, normal and thin, on 
macOS, but I noticed that it was getting incorrect information when decoding a 
variety of archives on linux. There were subtle changes to how names were 
encoded that we didn't handle correctly and we also didn't set the result of 
GetObjectSize() correctly as there was some bad math. This didn't matter when 
exracting .o files from .a files for LLDB because the size was always way too 
big, but it was big enough to at least read enough bytes for each object within 
the archive.

This patch does the following:

- switch over to use LLVM's archive parser and avoids previous code duplication
- remove values from ObjectContainerBSDArchive::Object that we don't use like:
  - uid
  - gid
  - mode
- fix ths ObjectContainerBSDArchive::Object::file_size value to be correct
- adds tests to test that we get the correct module specifications


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159408

Files:
  lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.cpp
  lldb/source/Plugins/ObjectContainer/BSD-Archive/ObjectContainerBSDArchive.h
  lldb/test/API/functionalities/archives/TestBSDArchives.py
  lldb/test/API/functionalities/archives/b.c
  lldb/test/API/functionalities/archives/c.c

Index: lldb/test/API/functionalities/archives/c.c
===
--- lldb/test/API/functionalities/archives/c.c
+++ lldb/test/API/functionalities/archives/c.c
@@ -1,5 +1,6 @@
 static int __c_global = 3;
-
+char __extra[4096]; // Make sure sizeof b.o differs from a.o and c.o
+char __extra2[4096]; // Make sure sizeof b.o differs from a.o and c.o
 int c(int arg) {
   int result = arg + __c_global;
   return result;
Index: lldb/test/API/functionalities/archives/b.c
===
--- lldb/test/API/functionalities/archives/b.c
+++ lldb/test/API/functionalities/archives/b.c
@@ -1,4 +1,5 @@
 static int __b_global = 2;
+char __extra[4096]; // Make sure sizeof b.o differs from a.o and c.o
 
 int b(int arg) {
 int result = arg + __b_global;
Index: lldb/test/API/functionalities/archives/TestBSDArchives.py
===
--- lldb/test/API/functionalities/archives/TestBSDArchives.py
+++ lldb/test/API/functionalities/archives/TestBSDArchives.py
@@ -70,12 +70,6 @@
 )
 self.expect_var_path("__b_global", type="int", value="2")
 
-# Test loading thin archives
-archive_path = self.getBuildArtifact("libbar.a")
-module_specs = lldb.SBModuleSpecList.GetModuleSpecifications(archive_path)
-num_specs = module_specs.GetSize()
-self.assertEqual(num_specs, 1)
-self.assertEqual(module_specs.GetSpecAtIndex(0).GetObjectName(), "c.o")
 
 def check_frame_variable_errors(self, thread, error_strings):
 command_result = lldb.SBCommandReturnObject()
@@ -129,6 +123,53 @@
 ]
 self.check_frame_variable_errors(thread, error_strings)
 
+@skipIfRemote
+def test_archive_specifications(self):
+"""
+Create archives and make sure the information we get when retrieving
+the modules specifications is correct.
+"""
+self.build()
+libbar_path = self.getBuildArtifact("libbar.a")
+libfoo_path = self.getBuildArtifact("libfoo.a")
+libfoothin_path = self.getBuildArtifact("libfoo-thin.a")
+objfile_a = self.getBuildArtifact("a.o")
+objfile_b = self.getBuildArtifact("b.o")
+objfile_c = self.getBuildArtifact("c.o")
+size_a = os.path.getsize(objfile_a)
+size_b = os.path.getsize(objfile_b)
+size_c = os.path.getsize(objfile_c)
+
+# Test loading normal archives
+module_specs = lldb.SBModuleSpecList.GetModuleSpecifications(libfoo_path)
+num_specs = module_specs.GetSize()
+self.assertEqual(num_specs, 2)
+spec = module_specs.GetSpecAtIndex(0)
+self.assertEqual(spec.GetObjectName(), "a.o")
+self.assertEqual(spec.GetObjectSize(), size_a)
+spec = module_specs.GetSpecAtIndex(1)
+self.assertEqual(spec.GetObjectName(), "b.o")
+self.assertEqual(spec.GetObjectSize(), size_b)
+
+# Test loading thin archives
+module_specs = lldb.SBModuleSpecList.GetModuleSpecifications(libbar_path)
+num_specs = module_specs.GetSize()
+self.assertEqual(num_specs, 1)
+spec = module_specs.GetSpecAtIndex(0)
+self.assertEqual(spec.GetObjectName(), "c.o")
+self.assertEqual(spec.Get