date:20230810

[Lldb-commits] [PATCH] D157662: [lldb][NFCI] Change parameter type in Process::SetExitStatus

2023-08-10 Thread Med Ismail Bennani via Phabricator via lldb-commits

mib accepted this revision.
mib added a comment.
This revision is now accepted and ready to land.

Sounds reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157662

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

[Lldb-commits] [lldb] 989f250 - [lldb] 'scoped_lock' may not intend to support class template argument deduction (NFC)

2023-08-10 Thread Jie Fu via lldb-commits


Author: Jie Fu
Date: 2023-08-11T09:06:11+08:00
New Revision: 989f25001aee3457738c431ba1e977a8140726a0

URL: 
https://github.com/llvm/llvm-project/commit/989f25001aee3457738c431ba1e977a8140726a0
DIFF: 
https://github.com/llvm/llvm-project/commit/989f25001aee3457738c431ba1e977a8140726a0.diff

LOG: [lldb] 'scoped_lock' may not intend to support class template argument 
deduction (NFC)

/data/home/jiefu/llvm-project/lldb/source/Host/common/File.cpp:251:3: error: 
'scoped_lock' may not intend to support class template argument deduction 
[-Werror,-Wctad-maybe-unsupported]
  std::scoped_lock lock(m_descriptor_mutex, m_stream_mutex);
  ^
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/mutex:692:11:
 note: add a deduction guide to suppress this warning
class scoped_lock
  ^
/data/home/jiefu/llvm-project/lldb/source/Host/common/File.cpp:316:3: error: 
'scoped_lock' may not intend to support class template argument deduction 
[-Werror,-Wctad-maybe-unsupported]
  std::scoped_lock lock(m_descriptor_mutex, m_stream_mutex);
  ^
/opt/rh/gcc-toolset-12/root/usr/lib/gcc/x86_64-redhat-linux/12/../../../../include/c++/12/mutex:692:11:
 note: add a deduction guide to suppress this warning
class scoped_lock
  ^
2 errors generated.

Added: 


Modified: 
lldb/source/Host/common/File.cpp

Removed: 




diff  --git a/lldb/source/Host/common/File.cpp 
b/lldb/source/Host/common/File.cpp
index 82cfcedd616fc2..174feecf2de0ad 100644
--- a/lldb/source/Host/common/File.cpp
+++ b/lldb/source/Host/common/File.cpp
@@ -248,7 +248,7 @@ uint32_t File::GetPermissions(Status ) const {
 }
 
 bool NativeFile::IsValid() const {
-  std::scoped_lock lock(m_descriptor_mutex, m_stream_mutex);
+  std::scoped_lock lock(m_descriptor_mutex, 
m_stream_mutex);
   return DescriptorIsValidUnlocked() || StreamIsValidUnlocked();
 }
 
@@ -313,7 +313,7 @@ FILE *NativeFile::GetStream() {
 }
 
 Status NativeFile::Close() {
-  std::scoped_lock lock(m_descriptor_mutex, m_stream_mutex);
+  std::scoped_lock lock(m_descriptor_mutex, 
m_stream_mutex);
 
   Status error;
 



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

[Lldb-commits] [PATCH] D157667: Define qHostInfo and Mach-O LC_NOTE "addrable bits" methods to describe high and low memory addressing bits

2023-08-10 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda updated this revision to Diff 549214.
jasonmolenda added a comment.

update comments a tiny bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157667

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp

Index: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
===
--- lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -509,10 +509,14 @@
 
   CleanupMemoryRegionPermissions();
 
-  addr_t address_mask = core_objfile->GetAddressMask();
-  if (address_mask != 0) {
-SetCodeAddressMask(address_mask);
-SetDataAddressMask(address_mask);
+  addr_t lo_addr_mask, hi_addr_mask;
+  if (core_objfile->GetAddressMask(lo_addr_mask, hi_addr_mask)) {
+SetCodeAddressMask(lo_addr_mask);
+SetDataAddressMask(lo_addr_mask);
+if (lo_addr_mask != hi_addr_mask) {
+  SetHighmemCodeAddressMask(hi_addr_mask);
+  SetHighmemDataAddressMask(hi_addr_mask);
+}
   }
   return error;
 }
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -899,10 +899,24 @@
  process_arch.GetTriple().getTriple());
   }
 
-  if (int addressable_bits = m_gdb_comm.GetAddressingBits()) {
-lldb::addr_t address_mask = ~((1ULL << addressable_bits) - 1);
+  uint32_t low_mem_addressable_bits, high_mem_addressable_bits;
+  m_gdb_comm.GetAddressingBits(low_mem_addressable_bits,
+   high_mem_addressable_bits);
+  // In case we were only given a low or high memory addressing bits,
+  // copy the value to the other.
+  if (low_mem_addressable_bits == 0)
+low_mem_addressable_bits = high_mem_addressable_bits;
+  if (high_mem_addressable_bits == 0)
+high_mem_addressable_bits = low_mem_addressable_bits;
+  if (low_mem_addressable_bits != 0) {
+lldb::addr_t address_mask = ~((1ULL << low_mem_addressable_bits) - 1);
 SetCodeAddressMask(address_mask);
 SetDataAddressMask(address_mask);
+if (low_mem_addressable_bits != high_mem_addressable_bits) {
+  lldb::addr_t hi_address_mask = ~((1ULL << high_mem_addressable_bits) - 1);
+  SetCodeAddressMask(hi_address_mask);
+  SetDataAddressMask(hi_address_mask);
+}
   }
 
   if (process_arch.IsValid()) {
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -237,7 +237,8 @@
 
   ArchSpec GetSystemArchitecture();
 
-  uint32_t GetAddressingBits();
+  void GetAddressingBits(uint32_t _mem_addr_bits,
+ uint32_t _mem_addr_bits);
 
   bool GetHostname(std::string );
 
@@ -580,7 +581,8 @@
   lldb::tid_t m_curr_tid_run = LLDB_INVALID_THREAD_ID;
 
   uint32_t m_num_supported_hardware_watchpoints = 0;
-  uint32_t m_addressing_bits = 0;
+  uint32_t m_low_mem_addressing_bits = 0;
+  uint32_t m_high_mem_addressing_bits = 0;
 
   ArchSpec m_host_arch;
   std::string m_host_distribution_id;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1265,7 +1265,15 @@
 if (!value.getAsInteger(0, pointer_byte_size))
   ++num_keys_decoded;
   } else if (name.equals("addressing_bits")) {
-if (!value.getAsInteger(0, m_addressing_bits))
+if (!value.getAsInteger(0, m_low_mem_addressing_bits)) {
+  m_high_mem_addressing_bits = m_low_mem_addressing_bits;
+  ++num_keys_decoded;
+}
+  } else if (name.equals("high_mem_addressing_bits")) {
+if (!value.getAsInteger(0, m_high_mem_addressing_bits))
+  ++num_keys_decoded;
+  } else if (name.equals("low_mem_addressing_bits")) {
+if (!value.getAsInteger(0, m_low_mem_addressing_bits))
   ++num_keys_decoded;
   } else if (name.equals("os_version") ||
  name.equals("version"))

[Lldb-commits] [PATCH] D157667: Define qHostInfo and Mach-O LC_NOTE "addrable bits" methods to describe high and low memory addressing bits

2023-08-10 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda created this revision.
jasonmolenda added reviewers: JDevlieghere, bulbazord.
jasonmolenda added a project: LLDB.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

To recap, on AArch64 where we need to strip pointer authentication bits off of 
addresses, it can have a different number of bits in use for high and low 
memory (you can have a different page table setup for high and low memory, so 
different number of addressing bits for virtual addresses).  I added this to 
Process and defined a new setting so these can be done manually in 
https://reviews.llvm.org/D151292 a few months ago.

This patch defines keys for qHostInfo and a new "addrable bits" LC_NOTE format 
for mach-o corefiles to define both of them.

qHostInfo gains `low_mem_addressing_bits` and `high_mem_addressing_bits` keys.  
The previous `addressing_bits` is still accepted for a value that is used for 
both regions of memory (this is by far the most common case).

For Mach-O corefile, I've needed to update the "addrable bits" LC_NOTE.  The 
previous definition was

  struct addressing_bit_count_v3// ** DEPRECATED **
  {
  uint32_t version; // currently 3
  uint32_t addressing_bits; // # of bits used for addressing
  uint64_t unused;
  };

The new definition is

  struct addressing_bit_count
  {
  // currently 4
  uint32_t version;
  
  // Number of bits used for addressing in low
  // memory (addresses starting with 0)
  uint32_t low_memory_addressing_bits;
  
  // Number of bits used for addressing in high
  // memory (addresses starting with f)
  uint32_t high_memory_addressing_bits;
  
  // set to zero
  uint32_t reserved;
  };

The changes in this patchset are to GDBRemoteCommunicationClient (read the new 
qHostInfo keys), ProcessGDBRemote (set both in the Process when they are 
specified).  And ObjectFileMachO (read and write the new LC_NOTE in mach-o 
corefiles), ProcessMachCore (set both in Process when they are specified).

I still accept the previous formats of a single value, and only initialize the 
base CodeAddressMask and DataAddressMask.  Only when both high and low memory 
addressing bits are specified and are different, do we set the Process' 
HighmemCodeAddressMask, HighmemDataAddressMask.

This is the final part of adding support for the different address masks that 
can be used on AArch64 targets.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157667

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
  lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.h
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp

Index: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
===
--- lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
+++ lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp
@@ -509,10 +509,14 @@
 
   CleanupMemoryRegionPermissions();
 
-  addr_t address_mask = core_objfile->GetAddressMask();
-  if (address_mask != 0) {
-SetCodeAddressMask(address_mask);
-SetDataAddressMask(address_mask);
+  addr_t lo_addr_mask, hi_addr_mask;
+  if (core_objfile->GetAddressMask(lo_addr_mask, hi_addr_mask)) {
+SetCodeAddressMask(lo_addr_mask);
+SetDataAddressMask(lo_addr_mask);
+if (lo_addr_mask != hi_addr_mask) {
+  SetHighmemCodeAddressMask(hi_addr_mask);
+  SetHighmemDataAddressMask(hi_addr_mask);
+}
   }
   return error;
 }
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -899,10 +899,21 @@
  process_arch.GetTriple().getTriple());
   }
 
-  if (int addressable_bits = m_gdb_comm.GetAddressingBits()) {
-lldb::addr_t address_mask = ~((1ULL << addressable_bits) - 1);
+  uint32_t low_mem_addressable_bits, high_mem_addressable_bits;
+  m_gdb_comm.GetAddressingBits(low_mem_addressable_bits,
+   high_mem_addressable_bits);
+  // In case we were only given a high memory addressing bits
+  if (low_mem_addressable_bits == 0)
+low_mem_addressable_bits = high_mem_addressable_bits;
+  if (low_mem_addressable_bits != 0) {
+lldb::addr_t address_mask = ~((1ULL << low_mem_addressable_bits) - 1);
 SetCodeAddressMask(address_mask);
 SetDataAddressMask(address_mask);
+if (low_mem_addressable_bits != high_mem_addressable_bits) {
+  lldb::addr_t

[Lldb-commits] [PATCH] D157659: Have SBTarget::AddModule force a possibly-slow search for the binary, and if the Target has no arch, initialize it with the binary's arch

2023-08-10 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added a comment.

One more bit of explanation about the changes:  There is an SBTarget::AddModule 
which takes individual parts of a module spec, and an SBTarget::AddModule which 
takes an SBModuleSpec.  I was going to need to duplicate my code to force the 
expensive search & update the Target's arch if it didn't have one in both, so I 
had the first method create an SBModuleSpec with its information and call the 
second.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157659

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

[Lldb-commits] [PATCH] D157648: [lldb] Fix data race in Process

2023-08-10 Thread Augusto Noronha via Phabricator via lldb-commits

augusto2112 added a comment.

In D157648#4578420 , @JDevlieghere 
wrote:

> Is the reported race specifically about the shared pointer being accessed 
> concurrently or operations on the `IOHandler`?

I believe it's the shared pointer being accessed. Here's an example of what's 
being reported:

  WARNING: ThreadSanitizer: data race (pid=52249)
Read of size 8 at 0x00010731ce38 by thread T3:
  #0 lldb_private::Process::PopProcessIOHandler() Process.cpp:4559 
(liblldb.18.0.0git.dylib:arm64+0x53a5a4) (BuildId: 
6e77321523b936d2955d63d4d25df7cd320021000e00)
  #1 
lldb_private::Process::HandlePrivateEvent(std::__1::shared_ptr&)
 Process.cpp:3775 (liblldb.18.0.0git.dylib:arm64+0x54204c) (BuildId: 
6e77321523b936d2955d63d4d25df7cd320021000e00)
  #2 lldb_private::Process::RunPrivateStateThread(bool) Process.cpp:3904 
(liblldb.18.0.0git.dylib:arm64+0x54874c) (BuildId: 
6e77321523b936d2955d63d4d25df7cd320021000e00)
  #3 
std::__1::__function::__func, 
void* ()>::operator()() function.h:356 (liblldb.18.0.0git.dylib:arm64+0x555de4) 
(BuildId: 6e77321523b936d2955d63d4d25df7cd320021000e00)
  #4 lldb_private::HostNativeThreadBase::ThreadCreateTrampoline(void*) 
HostNativeThreadBase.cpp:62 (liblldb.18.0.0git.dylib:arm64+0x3e83fc) (BuildId: 
6e77321523b936d2955d63d4d25df7cd320021000e00)
  #5 lldb_private::HostThreadMacOSX::ThreadCreateTrampoline(void*) 
HostThreadMacOSX.mm:18 (liblldb.18.0.0git.dylib:arm64+0x175056c) (BuildId: 
6e77321523b936d2955d63d4d25df7cd320021000e00)
  
Previous write of size 8 at 0x00010731ce38 by main thread (mutexes: write 
M0):
  #0 lldb_private::Process::SetSTDIOFileDescriptor(int) Process.cpp:4528 
(liblldb.18.0.0git.dylib:arm64+0x54aa5c) (BuildId: 
6e77321523b936d2955d63d4d25df7cd320021000e00)
  #1 lldb_private::Platform::DebugProcess(lldb_private::ProcessLaunchInfo&, 
lldb_private::Debugger&, lldb_private::Target&, lldb_private::Status&) 
Platform.cpp:1120 (liblldb.18.0.0git.dylib:arm64+0x52bc54) (BuildId: 
6e77321523b936d2955d63d4d25df7cd320021000e00)
  #2 
lldb_private::PlatformDarwin::DebugProcess(lldb_private::ProcessLaunchInfo&, 
lldb_private::Debugger&, lldb_private::Target&, lldb_private::Status&) 
PlatformDarwin.cpp:711 (liblldb.18.0.0git.dylib:arm64+0x872cc8) (BuildId: 
6e77321523b936d2955d63d4d25df7cd320021000e00)
  #3 lldb_private::Target::Launch(lldb_private::ProcessLaunchInfo&, 
lldb_private::Stream*) Target.cpp:3235 (liblldb.18.0.0git.dylib:arm64+0x5b118c) 
(BuildId: 6e77321523b936d2955d63d4d25df7cd320021000e00)

Line 4559 is

  IOHandlerSP io_handler_sp(m_process_input_reader);

Line 4528 is

  m_process_input_reader =
  std::make_shared(this, fd);


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157648

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

[Lldb-commits] [PATCH] D157662: [lldb][NFCI] Change parameter type in Process::SetExitStatus

2023-08-10 Thread Alex Langford via Phabricator via lldb-commits

bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, mib, jingham, jasonmolenda.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

My primary motivation here is actually to change something in
UnixSignals, but this change is a necesary precondition.

I've also updated the documentation and rewritten the log statements to
use `formatv` instead of `printf` (printf-style formatting and
llvm::StringRef don't mix well).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157662

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Target/Process.cpp


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -1053,27 +1053,27 @@
   return nullptr;
 }
 
-bool Process::SetExitStatus(int status, const char *cstr) {
+bool Process::SetExitStatus(int status, llvm::StringRef exit_string) {
   // Use a mutex to protect setting the exit status.
   std::lock_guard guard(m_exit_status_mutex);
 
   Log *log(GetLog(LLDBLog::State | LLDBLog::Process));
-  LLDB_LOGF(log, "(plugin = %s status=%i (0x%8.8x), description=%s%s%s)",
-   GetPluginName().data(), status, status, cstr ? "\"" : "",
-   cstr ? cstr : "NULL", cstr ? "\"" : "");
+  LLDB_LOG(log, "(plugin = {0} status = {1} ({1:x8}), description=\"{2}\")",
+   GetPluginName(), status, exit_string);
 
   // We were already in the exited state
   if (m_private_state.GetValue() == eStateExited) {
-LLDB_LOGF(log,
- "(plugin = %s) ignoring exit status because state was already set 
"
- "to eStateExited",
- GetPluginName().data());
+LLDB_LOG(
+log,
+"(plugin = {0}) ignoring exit status because state was already set "
+"to eStateExited",
+GetPluginName());
 return false;
   }
 
   m_exit_status = status;
-  if (cstr)
-m_exit_string = cstr;
+  if (!exit_string.empty())
+m_exit_string = exit_string.str();
   else
 m_exit_string.clear();
 
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -1439,8 +1439,13 @@
   /// \param[in] exit_status
   /// The value for the process's return code.
   ///
-  /// \see lldb::StateType
-  virtual bool SetExitStatus(int exit_status, const char *cstr);
+  /// \param[in] exit_string
+  /// A StringRef containing the reason for exiting. May be empty.
+  ///
+  /// \return
+  /// Returns \b false if the process was already in an exited state, \b
+  /// true otherwise.
+  virtual bool SetExitStatus(int exit_status, llvm::StringRef exit_string);
 
   /// Check if a process is still alive.
   ///


Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -1053,27 +1053,27 @@
   return nullptr;
 }
 
-bool Process::SetExitStatus(int status, const char *cstr) {
+bool Process::SetExitStatus(int status, llvm::StringRef exit_string) {
   // Use a mutex to protect setting the exit status.
   std::lock_guard guard(m_exit_status_mutex);
 
   Log *log(GetLog(LLDBLog::State | LLDBLog::Process));
-  LLDB_LOGF(log, "(plugin = %s status=%i (0x%8.8x), description=%s%s%s)",
-   GetPluginName().data(), status, status, cstr ? "\"" : "",
-   cstr ? cstr : "NULL", cstr ? "\"" : "");
+  LLDB_LOG(log, "(plugin = {0} status = {1} ({1:x8}), description=\"{2}\")",
+   GetPluginName(), status, exit_string);
 
   // We were already in the exited state
   if (m_private_state.GetValue() == eStateExited) {
-LLDB_LOGF(log,
- "(plugin = %s) ignoring exit status because state was already set "
- "to eStateExited",
- GetPluginName().data());
+LLDB_LOG(
+log,
+"(plugin = {0}) ignoring exit status because state was already set "
+"to eStateExited",
+GetPluginName());
 return false;
   }
 
   m_exit_status = status;
-  if (cstr)
-m_exit_string = cstr;
+  if (!exit_string.empty())
+m_exit_string = exit_string.str();
   else
 m_exit_string.clear();
 
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -1439,8 +1439,13 @@
   /// \param[in] exit_status
   /// The value for the process's return code.
   ///
-  /// \see lldb::StateType
-  virtual bool SetExitStatus(int exit_status, const char *cstr);
+  /// \param[in] exit_string
+  /// A StringRef containing the reason for exiting. May be empty.
+  ///
+  /// \return
+  /// Returns \b false if the process was already in an exited state, \b
+  ///

[Lldb-commits] [PATCH] D157648: [lldb] Fix data race in Process

2023-08-10 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

Is the reported race specifically about the shared pointer being accessed 
concurrently or operations on the `IOHandler`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157648

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

[Lldb-commits] [PATCH] D157654: [lldb] Fix data race in PipePosix

2023-08-10 Thread Alex Langford via Phabricator via lldb-commits

bulbazord added inline comments.



Comment at: lldb/source/Host/posix/PipePosix.cpp:69
   PipeBase::operator=(std::move(pipe_posix));
-  m_fds[READ] = pipe_posix.ReleaseReadFileDescriptor();
-  m_fds[WRITE] = pipe_posix.ReleaseWriteFileDescriptor();
+  std::lock_guard guard(m_lock);
+  m_fds[READ] = pipe_posix.ReleaseReadFileDescriptorUnlocked();

I think this lock needs to be at the beginning. Scenario:

Thread 1 calls the move assign operator and performs the base move assign. Gets 
unscheduled before acquiring lock.
Thread 2 calls the move assign operator and performs the base move assign. 
Acquires the lock, fills in `m_fds`, and returns.
Thread 1 is scheduled again, grabs the lock, and fills in `m_fds`.
`this` will have its `PipeBase` members filled in by the Thread 2 call while 
`m_fds` will be filled in by the Thread 1 call. Granted, one thread may be 
surprised that suddenly the thing didn't get moved in as expected, but at least 
`this` will be in a consistent state.

I think you also need to lock the mutex of `pipe_posix` at the same time. 
Imagine Thread 1 is trying to access something from `pipe_posix` while Thread 2 
is trying to move it into another `PipePosix` object. (Not sure how likely this 
scenario is but I'd rather be safe than sorry).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157654

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

[Lldb-commits] [PATCH] D157659: Have SBTarget::AddModule force a possibly-slow search for the binary, and if the Target has no arch, initialize it with the binary's arch

2023-08-10 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda created this revision.
jasonmolenda added reviewers: JDevlieghere, bulbazord.
jasonmolenda added a project: LLDB.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch addresses two issues I found while looking into a problem yesterday. 
 First, SBTarget::AddModule calls Target::GetOrCreate which will find the 
module in the global module list, the current target, and will call the 
DebugSymbols framework on macOS and look in specified directories, and do a 
Spotlight search of the local computer.  But it doesn't call 
`Symbols::DownloadObjectAndSymbolFile()` with a "force expensive lookup" which 
we should assume the program is trying to do if they gave us a UUID to look up. 
 This matches the behavior of `target modules add -u ` today in the lldb 
command line interface.

Also, if the SB API user has created a Target which has no architecture, we 
normally will set the Target's arch as soon as we find a hint about the arch -- 
e.g. when we connect to a gdb remote serial protocol stub, or when we get a 
corefile.  But in this case, they are creating a Target with no arch, adding a 
module to it.  We should use the same scheme where a Target inherits the first 
likely ArchSpec we see when it has none of its own.

Add a test for these two things, using a fake dsym-for-uuid.sh shell (which 
Jonas has pointed out in the past, I really need to create a utility function 
that creates these given an array of uuid/binary/dsyms, which I agree with but 
haven't done yet. :/)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157659

Files:
  lldb/source/API/SBTarget.cpp
  lldb/test/API/python_api/target-arch-from-module/Makefile
  lldb/test/API/python_api/target-arch-from-module/TestTargetArchFromModule.py
  lldb/test/API/python_api/target-arch-from-module/main.c

Index: lldb/test/API/python_api/target-arch-from-module/main.c
===
--- /dev/null
+++ lldb/test/API/python_api/target-arch-from-module/main.c
@@ -0,0 +1,5 @@
+#include 
+int main() {
+  puts("HI i am a program");
+  return 0;
+}
Index: lldb/test/API/python_api/target-arch-from-module/TestTargetArchFromModule.py
===
--- /dev/null
+++ lldb/test/API/python_api/target-arch-from-module/TestTargetArchFromModule.py
@@ -0,0 +1,107 @@
+"""
+An SBTarget with no arch, call AddModule, SBTarget's arch should be set.
+"""
+
+import os
+import subprocess
+import re
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TargetArchFromModule(TestBase):
+@skipIf(
+debug_info=no_match(["dsym"]),
+bugnumber="This test is looking explicitly for a dSYM",
+)
+@skipUnlessDarwin
+@skipIfRemote
+def test_target_arch_init(self):
+self.build()
+aout_exe = self.getBuildArtifact("a.out")
+aout_dsym = self.getBuildArtifact("a.out.dSYM")
+hidden_dir = self.getBuildArtifact("hide.noindex")
+hidden_aout_exe = self.getBuildArtifact("hide.noindex/a.out")
+hidden_aout_dsym = self.getBuildArtifact("hide.noindex/a.out.dSYM")
+dsym_for_uuid = self.getBuildArtifact("dsym-for-uuid.sh")
+
+# We can hook in our dsym-for-uuid shell script to lldb with
+# this env var instead of requiring a defaults write.
+os.environ["LLDB_APPLE_DSYMFORUUID_EXECUTABLE"] = dsym_for_uuid
+self.addTearDownHook(
+lambda: os.environ.pop("LLDB_APPLE_DSYMFORUUID_EXECUTABLE", None)
+)
+
+dwarfdump_uuid_regex = re.compile("UUID: ([-0-9a-fA-F]+) \(([^\(]+)\) .*")
+dwarfdump_cmd_output = subprocess.check_output(
+('/usr/bin/dwarfdump --uuid "%s"' % aout_exe), shell=True
+).decode("utf-8")
+aout_uuid = None
+for line in dwarfdump_cmd_output.splitlines():
+match = dwarfdump_uuid_regex.search(line)
+if match:
+aout_uuid = match.group(1)
+self.assertNotEqual(aout_uuid, None, "Could not get uuid of built a.out")
+
+###  Create our dsym-for-uuid shell script which returns self.hidden_aout_exe.
+shell_cmds = [
+"#! /bin/sh",
+"# the last argument is the uuid",
+"while [ $# -gt 1 ]",
+"do",
+"  shift",
+"done",
+"ret=0",
+'echo ""',
+'echo "http://www.apple.com/DTDs/PropertyList-1.0.dtd\\;>"',
+'echo ""',
+"",
+'if [ "$1" = "%s" ]' % aout_uuid,
+"then",
+"  uuid=%s" % aout_uuid,
+"  bin=%s" % hidden_aout_exe,
+"  dsym=%s.dSYM/Contents/Resources/DWARF/%s"
+% (hidden_aout_exe, os.path.basename(hidden_aout_exe)),
+"fi",
+'echo "  "',
+

[Lldb-commits] [PATCH] D157654: [lldb] Fix data race in PipePosix

2023-08-10 Thread Augusto Noronha via Phabricator via lldb-commits

augusto2112 created this revision.
augusto2112 added reviewers: JDevlieghere, bulbazord.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
augusto2112 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Thread sanitizer reports the following data race:

  WARNING: ThreadSanitizer: data race (pid=43201)
Write of size 4 at 0x00010520c474 by thread T1 (mutexes: write M0, write 
M1):
  #0 lldb_private::PipePosix::CloseWriteFileDescriptor() PipePosix.cpp:242 
(liblldb.18.0.0git.dylib:arm64+0x414700) (BuildId: 
2983976beb2637b5943bff32fd12eb89320021000e00)
  #1 lldb_private::PipePosix::Close() PipePosix.cpp:217 
(liblldb.18.0.0git.dylib:arm64+0x4144e8) (BuildId: 
2983976beb2637b5943bff32fd12eb89320021000e00)
  #2 
lldb_private::ConnectionFileDescriptor::Disconnect(lldb_private::Status*) 
ConnectionFileDescriptorPosix.cpp:239 (liblldb.18.0.0git.dylib:arm64+0x40a620) 
(BuildId: 2983976beb2637b5943bff32fd12eb89320021000e00)
  #3 lldb_private::Communication::Disconnect(lldb_private::Status*) 
Communication.cpp:61 (liblldb.18.0.0git.dylib:arm64+0x2a9318) (BuildId: 
2983976beb2637b5943bff32fd12eb89320021000e00)
  #4 lldb_private::process_gdb_remote::ProcessGDBRemote::DidExit() 
ProcessGDBRemote.cpp:1167 (liblldb.18.0.0git.dylib:arm64+0x8ed984) (BuildId: 
2983976beb2637b5943bff32fd12eb89320021000e00)
  
Previous read of size 4 at 0x00010520c474 by main thread (mutexes: write 
M2, write M3):
  #0 lldb_private::PipePosix::CanWrite() const PipePosix.cpp:229 
(liblldb.18.0.0git.dylib:arm64+0x4145e4) (BuildId: 
2983976beb2637b5943bff32fd12eb89320021000e00)
  #1 
lldb_private::ConnectionFileDescriptor::Disconnect(lldb_private::Status*) 
ConnectionFileDescriptorPosix.cpp:212 (liblldb.18.0.0git.dylib:arm64+0x40a4a8) 
(BuildId: 2983976beb2637b5943bff32fd12eb89320021000e00)
  #2 lldb_private::Communication::Disconnect(lldb_private::Status*) 
Communication.cpp:61 (liblldb.18.0.0git.dylib:arm64+0x2a9318) (BuildId: 
2983976beb2637b5943bff32fd12eb89320021000e00)
  #3 
lldb_private::process_gdb_remote::GDBRemoteCommunication::WaitForPacketNoLock(StringExtractorGDBRemote&,
 lldb_private::Timeout>, bool) 
GDBRemoteCommunication.cpp:373 (liblldb.18.0.0git.dylib:arm64+0x8b9c48) 
(BuildId: 2983976beb2637b5943bff32fd12eb89320021000e00)
  #4 
lldb_private::process_gdb_remote::GDBRemoteCommunication::WaitForPacketNoLock(StringExtractorGDBRemote&,
 lldb_private::Timeout>, bool) 
GDBRemoteCommunication.cpp:243 (liblldb.18.0.0git.dylib:arm64+0x8b9904) 
(BuildId: 2983976beb2637b5943bff32fd12eb89320021000e00)

Fix this by adding a mutex to PipePosix.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157654

Files:
  lldb/include/lldb/Host/posix/PipePosix.h
  lldb/source/Host/posix/PipePosix.cpp

Index: lldb/source/Host/posix/PipePosix.cpp
===
--- lldb/source/Host/posix/PipePosix.cpp
+++ lldb/source/Host/posix/PipePosix.cpp
@@ -66,15 +66,17 @@
 
 PipePosix ::operator=(PipePosix &_posix) {
   PipeBase::operator=(std::move(pipe_posix));
-  m_fds[READ] = pipe_posix.ReleaseReadFileDescriptor();
-  m_fds[WRITE] = pipe_posix.ReleaseWriteFileDescriptor();
+  std::lock_guard guard(m_lock);
+  m_fds[READ] = pipe_posix.ReleaseReadFileDescriptorUnlocked();
+  m_fds[WRITE] = pipe_posix.ReleaseWriteFileDescriptorUnlocked();
   return *this;
 }
 
 PipePosix::~PipePosix() { Close(); }
 
 Status PipePosix::CreateNew(bool child_processes_inherit) {
-  if (CanRead() || CanWrite())
+  std::lock_guard guard(m_lock);
+  if (CanReadUnlocked() || CanWriteUnlocked())
 return Status(EINVAL, eErrorTypePOSIX);
 
   Status error;
@@ -87,7 +89,7 @@
 if (!child_processes_inherit) {
   if (!SetCloexecFlag(m_fds[0]) || !SetCloexecFlag(m_fds[1])) {
 error.SetErrorToErrno();
-Close();
+CloseUnlocked();
 return error;
   }
 }
@@ -103,7 +105,8 @@
 }
 
 Status PipePosix::CreateNew(llvm::StringRef name, bool child_process_inherit) {
-  if (CanRead() || CanWrite())
+  std::lock_guard guard(m_lock);
+  if (CanReadUnlocked() || CanWriteUnlocked())
 return Status("Pipe is already opened");
 
   Status error;
@@ -140,7 +143,8 @@
 
 Status PipePosix::OpenAsReader(llvm::StringRef name,
bool child_process_inherit) {
-  if (CanRead() || CanWrite())
+  std::lock_guard guard(m_lock);
+  if (CanReadUnlocked() || CanWriteUnlocked())
 return Status("Pipe is already opened");
 
   int flags = O_RDONLY | O_NONBLOCK;
@@ -161,7 +165,8 @@
 PipePosix::OpenAsWriterWithTimeout(llvm::StringRef name,
bool child_process_inherit,
const

[Lldb-commits] [PATCH] D157640: [lldb] Improve error message when trying to debug a non-debuggable process

2023-08-10 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

Not sure if codesign exists on iOS devices, but at least we could do this on 
macOS.

I thought we'd be able to tell from the Team Identifier whether it was a 
platform binary, but for /bin/ls & Co I get "not set" for the Team Identifier.


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

https://reviews.llvm.org/D157640

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

[Lldb-commits] [PATCH] D157640: [lldb] Improve error message when trying to debug a non-debuggable process

2023-08-10 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

In this function we have the path to the binary.  We could spawn `codesign -d 
-entitlements -` and then we would know whether it had that entitlement.

Maybe that's more work than you wanted to do here, however.


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

https://reviews.llvm.org/D157640

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

[Lldb-commits] [PATCH] D157648: [lldb] Fix data race in Process

2023-08-10 Thread Augusto Noronha via Phabricator via lldb-commits

augusto2112 created this revision.
augusto2112 added reviewers: bulbazord, JDevlieghere.
Herald added a project: All.
augusto2112 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Thread sanitizer reports a data race in Process.cpp in the usage of
m_process_input_reader. Fix this data race by introducing a mutex
guarding the access to this variable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157648

Files:
  lldb/include/lldb/Target/Process.h
  lldb/source/Target/Process.cpp

Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -630,6 +630,7 @@
 const Timeout ) {
   // don't sync (potentially context switch) in case where there is no process
   // IO
+  std::lock_guard guard(m_process_input_reader_mutex);
   if (!m_process_input_reader)
 return;
 
@@ -2505,7 +2506,11 @@
   m_jit_loaders_up.reset();
   m_system_runtime_up.reset();
   m_os_up.reset();
-  m_process_input_reader.reset();
+
+  {
+std::lock_guard guard(m_process_input_reader_mutex);
+m_process_input_reader.reset();
+  }
 
   Module *exe_module = GetTarget().GetExecutableModulePointer();
 
@@ -2803,7 +2808,10 @@
 
 Status Process::Attach(ProcessAttachInfo _info) {
   m_abi_sp.reset();
-  m_process_input_reader.reset();
+  {
+std::lock_guard guard(m_process_input_reader_mutex);
+m_process_input_reader.reset();
+  }
   m_dyld_up.reset();
   m_jit_loaders_up.reset();
   m_system_runtime_up.reset();
@@ -3054,7 +3062,10 @@
 
 Status Process::ConnectRemote(llvm::StringRef remote_url) {
   m_abi_sp.reset();
-  m_process_input_reader.reset();
+  {
+std::lock_guard guard(m_process_input_reader_mutex);
+m_process_input_reader.reset();
+  }
 
   // Find the process and its architecture.  Make sure it matches the
   // architecture of the current Target, and if not adjust it.
@@ -3342,10 +3353,13 @@
 m_stdio_communication.Disconnect();
 m_stdin_forward = false;
 
-if (m_process_input_reader) {
-  m_process_input_reader->SetIsDone(true);
-  m_process_input_reader->Cancel();
-  m_process_input_reader.reset();
+{
+  std::lock_guard guard(m_process_input_reader_mutex);
+  if (m_process_input_reader) {
+m_process_input_reader->SetIsDone(true);
+m_process_input_reader->Cancel();
+m_process_input_reader.reset();
+  }
 }
 
 // If we exited when we were waiting for a process to stop, then forward
@@ -4523,20 +4537,25 @@
 m_stdio_communication.StartReadThread();
 
 // Now read thread is set up, set up input reader.
-
-if (!m_process_input_reader)
-  m_process_input_reader =
-  std::make_shared(this, fd);
+{
+  std::lock_guard guard(m_process_input_reader_mutex);
+  if (!m_process_input_reader)
+m_process_input_reader =
+std::make_shared(this, fd);
+}
   }
 }
 
 bool Process::ProcessIOHandlerIsActive() {
+  std::lock_guard guard(m_process_input_reader_mutex);
   IOHandlerSP io_handler_sp(m_process_input_reader);
   if (io_handler_sp)
 return GetTarget().GetDebugger().IsTopIOHandler(io_handler_sp);
   return false;
 }
+
 bool Process::PushProcessIOHandler() {
+  std::lock_guard guard(m_process_input_reader_mutex);
   IOHandlerSP io_handler_sp(m_process_input_reader);
   if (io_handler_sp) {
 Log *log = GetLog(LLDBLog::Process);
@@ -4556,6 +4575,7 @@
 }
 
 bool Process::PopProcessIOHandler() {
+  std::lock_guard guard(m_process_input_reader_mutex);
   IOHandlerSP io_handler_sp(m_process_input_reader);
   if (io_handler_sp)
 return GetTarget().GetDebugger().RemoveIOHandler(io_handler_sp);
Index: lldb/include/lldb/Target/Process.h
===
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -2992,6 +2992,7 @@
   m_unix_signals_sp; /// This is the current signal set for this process.
   lldb::ABISP m_abi_sp;
   lldb::IOHandlerSP m_process_input_reader;
+  mutable std::mutex m_process_input_reader_mutex;
   ThreadedCommunication m_stdio_communication;
   std::recursive_mutex m_stdio_communication_mutex;
   bool m_stdin_forward; /// Remember if stdin must be forwarded to remote debug
@@ -3117,6 +3118,7 @@
   bool ProcessIOHandlerIsActive();
 
   bool ProcessIOHandlerExists() const {
+std::lock_guard guard(m_process_input_reader_mutex);
 return static_cast(m_process_input_reader);
   }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157636: [lldb][test] Remove tests relying on deprecated std::char_traits specializations

2023-08-10 Thread Dmitri Gribenko via Phabricator via lldb-commits

gribozavr2 added a comment.

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157636

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

[Lldb-commits] [PATCH] D157640: [lldb] Improve error message when trying to debug a non-debuggable process

2023-08-10 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added a comment.

Maybe

  const char *ent_name = 
  #if TARGET_OS_OSX
  "com.apple.security.get-task-allow";
  #else
  "get-task-allow";
  #endif

debugserver is running on the target device so compile time checks are fine.


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

https://reviews.llvm.org/D157640

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

[Lldb-commits] [PATCH] D157640: [lldb] Improve error message when trying to debug a non-debuggable process

2023-08-10 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added a comment.

Maybe get-task-allow is allowed (lol) on macOS too.


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

https://reviews.llvm.org/D157640

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

[Lldb-commits] [PATCH] D157640: [lldb] Improve error message when trying to debug a non-debuggable process

2023-08-10 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added a comment.

I'm fine with this but on macOS I believe it's 
com.apple.security.get-task-allow v. "Entitlements on macOS"
https://developer.apple.com/documentation/technotes/tn3125-inside-code-signing-provisioning-profiles


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

https://reviews.llvm.org/D157640

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

[Lldb-commits] [PATCH] D157556: Replace the singleton "ShadowListener" with a primary and N secondary listeners

2023-08-10 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added inline comments.



Comment at: lldb/source/Target/Process.cpp:372
 
+const int Process::g_all_event_bits = eBroadcastBitStateChanged 
+  | eBroadcastBitInterrupt

mib wrote:
> nit: this could probably be `constexpr`
yes, this can be a constexpr and be defined in only in the header file. No need 
to create storage for it by making it a real static variable



Comment at: lldb/source/Utility/Broadcaster.cpp:154-157
+  // The primary listener listens for all event bits:
+  if (m_primary_listener_sp)
+return true;
+

Does the primary listener need to listen to all event bits?



Comment at: lldb/source/Utility/Broadcaster.cpp:229-235
+  ListenerSP primary_listener_sp = hijacking_listener_sp;
+  bool is_hijack = false;
+  
+  if (primary_listener_sp)
+is_hijack = true;
+  else
+primary_listener_sp = m_primary_listener_sp;

mib wrote:
> nit: We could simplify the logic here a little bit
I agree this would clean this up nicely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157556

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

[Lldb-commits] [lldb] 487ab39 - [lldb][test] Remove tests relying on deprecated std::char_traits specializations

2023-08-10 Thread Michael Buch via lldb-commits


Author: Michael Buch
Date: 2023-08-10T19:48:44+01:00
New Revision: 487ab39a5082098f92e886de606505f25031a22a

URL: 
https://github.com/llvm/llvm-project/commit/487ab39a5082098f92e886de606505f25031a22a
DIFF: 
https://github.com/llvm/llvm-project/commit/487ab39a5082098f92e886de606505f25031a22a.diff

LOG: [lldb][test] Remove tests relying on deprecated std::char_traits 
specializations

(motivated by test failures after D157058)

With D157058 the base template for `std::char_traits` was removed from
libc++. Quoting the release notes:

```
The base template for ``std::char_traits`` has been removed. If you are using
``std::char_traits`` with types other than ``char``, ``wchar_t``, ``char8_t``,
``char16_t``, ``char32_t`` or a custom character type for which you
specialized ``std::char_traits``, your code will no longer work.
```

This patch simply removes all such instantiations to make sure the
tests that run against the latest libc++ version pass.

One could try testing the existence of this base template from within
the test source files but this doesn't seem like something we want
support.

Differential Revision: https://reviews.llvm.org/D157636

Added: 


Modified: 

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp

Removed: 




diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
index 16f93bc8acb0ee..e2d2e67f1e885e 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
@@ -50,16 +50,6 @@ def cleanup():
 
 ns = self.namespace
 
-if self.expectedCompiler(["clang"]) and self.expectedCompilerVersion(
-[">", "16.0"]
-):
-expected_basic_string = "%s::basic_string" % ns
-else:
-expected_basic_string = (
-"%s::basic_string, "
-"%s::allocator >" % (ns, ns, ns)
-)
-
 self.expect(
 "frame variable",
 substrs=[
@@ -81,7 +71,6 @@ def cleanup():
 '(%s::u32string) u32_string = U""' % ns,
 # FIXME: This should have a 'U' prefix.
 '(%s::u32string) u32_empty = ""' % ns,
-'(%s) uchar = "a"' % expected_basic_string,
 "(%s::string *) null_str = nullptr" % ns,
 ],
 )
@@ -126,7 +115,6 @@ def cleanup():
 '(%s::u16string) u16_string = u"ß水氶"' % ns,
 '(%s::u32string) u32_string = U""' % ns,
 '(%s::u32string) u32_empty = ""' % ns,
-'(%s) uchar = "a"' % expected_basic_string,
 "(%s::string *) null_str = nullptr" % ns,
 ],
 )

diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
index 02c55487c41d27..f9f1c0802e5189 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
@@ -113,7 +113,6 @@ int main()
 std::u16string u16_empty(u"");
 std::u32string u32_string(U"");
 std::u32string u32_empty(U"");
-std::basic_string uchar(5, 'a');
 std::string *null_str = nullptr;
 
 std::string garbage1, garbage2, garbage3, garbage4, garbage5;

diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
index 6a77358b9bd2bf..660eb09db20da2 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
@@ -60,15 +60,6 @@ def cleanup():
 # Execute the cleanup

[Lldb-commits] [PATCH] D157636: [lldb][test] Remove tests relying on deprecated std::char_traits specializations

2023-08-10 Thread Michael Buch via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG487ab39a5082: [lldb][test] Remove tests relying on 
deprecated std::char_traits specializations (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157636

Files:
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
@@ -9,7 +9,6 @@
 std::string empty("");
 std::string q("hello world");
 std::string Q("quite a long std::strin with lots of info inside it");
-std::basic_string uchar(5, 'a');
 auto  = q,  = Q;
 std::string *pq = , *pQ = 
 S.assign(L"!"); // Set break point at this line.
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
@@ -61,8 +61,6 @@
 var_pq = self.frame().FindVariable("pq")
 var_pQ = self.frame().FindVariable("pQ")
 
-var_uchar = self.frame().FindVariable("uchar")
-
 self.assertEqual(var_wempty.GetSummary(), 'L""', "wempty summary wrong")
 self.assertEqual(
 var_s.GetSummary(), 'L"hello world! ××× ×××!"', "s summary wrong"
@@ -78,7 +76,6 @@
 '"quite a long std::strin with lots of info inside it"',
 "Q summary wrong",
 )
-self.assertEqual(var_uchar.GetSummary(), '"a"', "u summary wrong")
 self.assertEqual(var_rq.GetSummary(), '"hello world"', "rq summary wrong")
 self.assertEqual(
 var_rQ.GetSummary(),
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp
@@ -92,8 +92,6 @@
   std::u16string_view u16_empty(u"");
   std::u32string_view u32_string(U"ðððð");
   std::u32string_view u32_empty(U"");
-  std::basic_string uchar_source(10, 'a');
-  std::basic_string_view uchar(uchar_source.data(), 5);
   std::string_view *null_str = nullptr;
 
   std::string hello = "Hellooo ";
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
@@ -60,15 +60,6 @@
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
 
-if self.expectedCompiler(["clang"]) and self.expectedCompilerVersion(
-[">", "16.0"]
-):
-expected_basic_string = "std::basic_string"
-expected_basic_string_view = "std::basic_string_view"
-else:
-expected_basic_string = "std::basic_string, std::allocator >"
-expected_basic_string_view = "std::basic_string_view >"
-
 self.expect_var_path("wempty", type="std::wstring_view", summary='L""')
 self.expect_var_path(
 "s", type="std::wstring_view", summary='L"hello world! ××× ×××!"'
@@ -96,12 +87,6 @@
 "u32_string", type="std::u32string_view", summary='U"ðððð"'
 )
 self.expect_var_path("u32_empty", type="std::u32string_view", summary='""')
-self.expect_var_path(
-"uchar_source", type=expected_basic_string, summary='"aa"'
-)
-

[Lldb-commits] [lldb] d21ec35 - [lldb][NFCI] Remove use of ifdef __cpluplus where unneeded

2023-08-10 Thread Alex Langford via lldb-commits


Author: Alex Langford
Date: 2023-08-10T11:24:40-07:00
New Revision: d21ec35dcf3c351395522807beac0f45b7bcf6b9

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

LOG: [lldb][NFCI] Remove use of ifdef __cpluplus where unneeded

Just about every file in the lldb project is built with C++ enabled.
Unless I've missed something, these macro guards don't really accomplish
very much.

Differential Revision: https://reviews.llvm.org/D157538

Added: 


Modified: 
lldb/include/lldb/Core/Mangled.h
lldb/include/lldb/Host/Editline.h
lldb/include/lldb/Host/Terminal.h
lldb/include/lldb/Host/posix/PipePosix.h
lldb/include/lldb/Utility/DataBuffer.h
lldb/include/lldb/Utility/DataEncoder.h
lldb/include/lldb/lldb-forward.h
lldb/include/lldb/lldb-private-interfaces.h
lldb/include/lldb/lldb-private-types.h
lldb/include/lldb/lldb-private.h
lldb/source/Host/macosx/cfcpp/CFCReleaser.h
lldb/source/Host/macosx/objcxx/PosixSpawnResponsible.h
lldb/tools/debugserver/source/DNBLog.h
lldb/tools/debugserver/source/DNBRegisterInfo.h
lldb/tools/debugserver/source/MacOSX/CFUtils.h

Removed: 




diff  --git a/lldb/include/lldb/Core/Mangled.h 
b/lldb/include/lldb/Core/Mangled.h
index b7813313d45975..9892426161e439 100644
--- a/lldb/include/lldb/Core/Mangled.h
+++ b/lldb/include/lldb/Core/Mangled.h
@@ -8,7 +8,6 @@
 
 #ifndef LLDB_CORE_MANGLED_H
 #define LLDB_CORE_MANGLED_H
-#if defined(__cplusplus)
 
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-forward.h"
@@ -286,5 +285,4 @@ Stream <<(Stream , const Mangled );
 
 } // namespace lldb_private
 
-#endif // #if defined(__cplusplus)
 #endif // LLDB_CORE_MANGLED_H

diff  --git a/lldb/include/lldb/Host/Editline.h 
b/lldb/include/lldb/Host/Editline.h
index bda3f06d8946c3..4e2a26e04fa672 100644
--- a/lldb/include/lldb/Host/Editline.h
+++ b/lldb/include/lldb/Host/Editline.h
@@ -27,7 +27,6 @@
 
 #ifndef LLDB_HOST_EDITLINE_H
 #define LLDB_HOST_EDITLINE_H
-#if defined(__cplusplus)
 
 #include "lldb/Host/Config.h"
 
@@ -408,5 +407,4 @@ class Editline {
 };
 }
 
-#endif // #if defined(__cplusplus)
 #endif // LLDB_HOST_EDITLINE_H

diff  --git a/lldb/include/lldb/Host/Terminal.h 
b/lldb/include/lldb/Host/Terminal.h
index 8ff6d75657a74b..da0d05e8bd2651 100644
--- a/lldb/include/lldb/Host/Terminal.h
+++ b/lldb/include/lldb/Host/Terminal.h
@@ -8,7 +8,6 @@
 
 #ifndef LLDB_HOST_TERMINAL_H
 #define LLDB_HOST_TERMINAL_H
-#if defined(__cplusplus)
 
 #include "lldb/lldb-private.h"
 #include "llvm/Support/Error.h"
@@ -172,5 +171,4 @@ class TerminalState {
 
 } // namespace lldb_private
 
-#endif // #if defined(__cplusplus)
 #endif // LLDB_HOST_TERMINAL_H

diff  --git a/lldb/include/lldb/Host/posix/PipePosix.h 
b/lldb/include/lldb/Host/posix/PipePosix.h
index 77c0e2f7ef49e9..3bdb7431d8c770 100644
--- a/lldb/include/lldb/Host/posix/PipePosix.h
+++ b/lldb/include/lldb/Host/posix/PipePosix.h
@@ -8,7 +8,6 @@
 
 #ifndef LLDB_HOST_POSIX_PIPEPOSIX_H
 #define LLDB_HOST_POSIX_PIPEPOSIX_H
-#if defined(__cplusplus)
 
 #include "lldb/Host/PipeBase.h"
 
@@ -76,5 +75,4 @@ class PipePosix : public PipeBase {
 
 } // namespace lldb_private
 
-#endif // #if defined(__cplusplus)
 #endif // LLDB_HOST_POSIX_PIPEPOSIX_H

diff  --git a/lldb/include/lldb/Utility/DataBuffer.h 
b/lldb/include/lldb/Utility/DataBuffer.h
index e1c66fae7453a1..5529afec553031 100644
--- a/lldb/include/lldb/Utility/DataBuffer.h
+++ b/lldb/include/lldb/Utility/DataBuffer.h
@@ -8,7 +8,6 @@
 
 #ifndef LLDB_UTILITY_DATABUFFER_H
 #define LLDB_UTILITY_DATABUFFER_H
-#if defined(__cplusplus)
 
 #include 
 #include 
@@ -149,5 +148,4 @@ class DataBufferUnowned : public WritableDataBuffer {
 
 } // namespace lldb_private
 
-#endif /// #if defined(__cplusplus)
 #endif // LLDB_UTILITY_DATABUFFER_H

diff  --git a/lldb/include/lldb/Utility/DataEncoder.h 
b/lldb/include/lldb/Utility/DataEncoder.h
index 7e1dec6992b2e0..0b78161d87ef10 100644
--- a/lldb/include/lldb/Utility/DataEncoder.h
+++ b/lldb/include/lldb/Utility/DataEncoder.h
@@ -9,8 +9,6 @@
 #ifndef LLDB_UTILITY_DATAENCODER_H
 #define LLDB_UTILITY_DATAENCODER_H
 
-#if defined(__cplusplus)
-
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-enumerations.h"
 #include "lldb/lldb-forward.h"
@@ -301,5 +299,4 @@ class DataEncoder {
 
 } // namespace lldb_private
 
-#endif // #if defined (__cplusplus)
 #endif // LLDB_UTILITY_DATAENCODER_H

diff  --git a/lldb/include/lldb/lldb-forward.h 
b/lldb/include/lldb/lldb-forward.h
index 262afd2c0cd177..c159d82911a04a 100644
--- a/lldb/include/lldb/lldb-forward.h
+++ b/lldb/include/lldb/lldb-forward.h
@@ -9,8 +9,6 @@
 #ifndef LLDB_LLDB_FORWARD_H
 #define LLDB_LLDB_FORWARD_H
 
-#if defined(__cplusplus)
-
 #include 
 
 // lldb forward declarations
@@ -465,5 +463,4 @@ typedef std::shared_ptr

[Lldb-commits] [PATCH] D157636: [lldb][test] Remove tests relying on deprecated std::char_traits specializations

2023-08-10 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

If this was important we could put this behind a define, break up the test and 
conditionally skip it based on the libc++ version. But I personally don't think 
that's worth it for this. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157636

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

[Lldb-commits] [PATCH] D157538: [lldb][NFCI] Remove use of ifdef __cpluplus where unneeded

2023-08-10 Thread Alex Langford via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd21ec35dcf3c: [lldb][NFCI] Remove use of ifdef __cpluplus 
where unneeded (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157538

Files:
  lldb/include/lldb/Core/Mangled.h
  lldb/include/lldb/Host/Editline.h
  lldb/include/lldb/Host/Terminal.h
  lldb/include/lldb/Host/posix/PipePosix.h
  lldb/include/lldb/Utility/DataBuffer.h
  lldb/include/lldb/Utility/DataEncoder.h
  lldb/include/lldb/lldb-forward.h
  lldb/include/lldb/lldb-private-interfaces.h
  lldb/include/lldb/lldb-private-types.h
  lldb/include/lldb/lldb-private.h
  lldb/source/Host/macosx/cfcpp/CFCReleaser.h
  lldb/source/Host/macosx/objcxx/PosixSpawnResponsible.h
  lldb/tools/debugserver/source/DNBLog.h
  lldb/tools/debugserver/source/DNBRegisterInfo.h
  lldb/tools/debugserver/source/MacOSX/CFUtils.h

Index: lldb/tools/debugserver/source/MacOSX/CFUtils.h
===
--- lldb/tools/debugserver/source/MacOSX/CFUtils.h
+++ lldb/tools/debugserver/source/MacOSX/CFUtils.h
@@ -15,8 +15,6 @@
 
 #include 
 
-#ifdef __cplusplus
-
 // Templatized CF helper class that can own any CF pointer and will
 // call CFRelease() on any valid pointer it owns unless that pointer is
 // explicitly released using the release() member function.
@@ -71,5 +69,4 @@
   element_type _ptr;
 };
 
-#endif // #ifdef __cplusplus
 #endif // LLDB_TOOLS_DEBUGSERVER_SOURCE_MACOSX_CFUTILS_H
Index: lldb/tools/debugserver/source/DNBRegisterInfo.h
===
--- lldb/tools/debugserver/source/DNBRegisterInfo.h
+++ lldb/tools/debugserver/source/DNBRegisterInfo.h
@@ -18,12 +18,10 @@
 #include 
 
 struct DNBRegisterValueClass : public DNBRegisterValue {
-#ifdef __cplusplus
   DNBRegisterValueClass(const DNBRegisterInfo *regInfo = NULL);
   void Clear();
   void Dump(const char *pre, const char *post) const;
   bool IsValid() const;
-#endif
 };
 
 #endif
Index: lldb/tools/debugserver/source/DNBLog.h
===
--- lldb/tools/debugserver/source/DNBLog.h
+++ lldb/tools/debugserver/source/DNBLog.h
@@ -17,9 +17,7 @@
 #include 
 #include 
 
-#ifdef __cplusplus
 extern "C" {
-#endif
 
 // Flags that get filled in automatically before calling the log callback
 // function
@@ -144,9 +142,6 @@
 #define DNBLogCloseLogFile() ((void)0)
 
 #endif // #else defined(DNBLOG_ENABLED)
-
-#ifdef __cplusplus
 }
-#endif
 
 #endif // LLDB_TOOLS_DEBUGSERVER_SOURCE_DNBLOG_H
Index: lldb/source/Host/macosx/objcxx/PosixSpawnResponsible.h
===
--- lldb/source/Host/macosx/objcxx/PosixSpawnResponsible.h
+++ lldb/source/Host/macosx/objcxx/PosixSpawnResponsible.h
@@ -31,9 +31,7 @@
 static dispatch_once_t pred;
 dispatch_once(, ^{
   responsibility_spawnattrs_setdisclaim_ptr =
-#ifdef __cplusplus
   reinterpret_cast<__typeof__(_spawnattrs_setdisclaim)>
-#endif
   (dlsym(RTLD_DEFAULT, "responsibility_spawnattrs_setdisclaim"));
 });
 if (responsibility_spawnattrs_setdisclaim_ptr)
Index: lldb/source/Host/macosx/cfcpp/CFCReleaser.h
===
--- lldb/source/Host/macosx/cfcpp/CFCReleaser.h
+++ lldb/source/Host/macosx/cfcpp/CFCReleaser.h
@@ -11,8 +11,6 @@
 
 #include 
 
-#ifdef __cplusplus
-
 #include 
 
 // Templatized CF helper class that can own any CF pointer and will
@@ -105,5 +103,4 @@
   T _ptr;
 };
 
-#endif // #ifdef __cplusplus
 #endif // LLDB_SOURCE_HOST_MACOSX_CFCPP_CFCRELEASER_H
Index: lldb/include/lldb/lldb-private.h
===
--- lldb/include/lldb/lldb-private.h
+++ lldb/include/lldb/lldb-private.h
@@ -9,13 +9,9 @@
 #ifndef LLDB_LLDB_PRIVATE_H
 #define LLDB_LLDB_PRIVATE_H
 
-#if defined(__cplusplus)
-
 #include "lldb/lldb-private-enumerations.h"
 #include "lldb/lldb-private-interfaces.h"
 #include "lldb/lldb-private-types.h"
 #include "lldb/lldb-public.h"
 
-#endif // defined(__cplusplus)
-
 #endif // LLDB_LLDB_PRIVATE_H
Index: lldb/include/lldb/lldb-private-types.h
===
--- lldb/include/lldb/lldb-private-types.h
+++ lldb/include/lldb/lldb-private-types.h
@@ -9,8 +9,6 @@
 #ifndef LLDB_LLDB_PRIVATE_TYPES_H
 #define LLDB_LLDB_PRIVATE_TYPES_H
 
-#if defined(__cplusplus)
-
 #include "lldb/lldb-private.h"
 
 #include "llvm/ADT/ArrayRef.h"
@@ -125,6 +123,4 @@
 void *baton, const char **argv, lldb_private::CommandReturnObject );
 } // namespace lldb_private
 
-#endif // #if defined(__cplusplus)
-
 #endif // LLDB_LLDB_PRIVATE_TYPES_H
Index: lldb/include/lldb/lldb-private-interfaces.h
===
---

[Lldb-commits] [PATCH] D157640: [lldb] Improve error message when trying to debug a non-debuggable process

2023-08-10 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: jasonmolenda, aprantl.
Herald added a project: All.
JDevlieghere requested review of this revision.

On the Swift forums [1][2], people are disabling SIP in order to debug process 
that are missing the get-task-allow entitlement. Improve the error to give 
developers a hint at the potential issues.

[1] 
https://forums.swift.org/t/yo-apple-xcode-debugging-swift-is-still-horribly-broken/62702/26
[2] 
https://forums.swift.org/t/finding-cycles-with-the-memory-graph-debugger-with-swift-pm-projects/62769/7

rdar://113704200


https://reviews.llvm.org/D157640

Files:
  lldb/tools/debugserver/source/DNB.cpp


Index: lldb/tools/debugserver/source/DNB.cpp
===
--- lldb/tools/debugserver/source/DNB.cpp
+++ lldb/tools/debugserver/source/DNB.cpp
@@ -382,11 +382,15 @@
 if (err_str && err_len > 0) {
   if (launch_err.AsString()) {
 ::snprintf(err_str, err_len,
-   "failed to get the task for process %i (%s)", pid,
+   "failed to get the task for process %i: %s", pid,
launch_err.AsString());
   } else {
 ::snprintf(err_str, err_len,
-   "failed to get the task for process %i", pid);
+   "failed to get the task for process %i: this likely "
+   "means the process cannot be debugged, either because "
+   "it's a system process or because the process is "
+   "missing the get-task-allow entitlement.",
+   pid);
   }
 }
   } else {


Index: lldb/tools/debugserver/source/DNB.cpp
===
--- lldb/tools/debugserver/source/DNB.cpp
+++ lldb/tools/debugserver/source/DNB.cpp
@@ -382,11 +382,15 @@
 if (err_str && err_len > 0) {
   if (launch_err.AsString()) {
 ::snprintf(err_str, err_len,
-   "failed to get the task for process %i (%s)", pid,
+   "failed to get the task for process %i: %s", pid,
launch_err.AsString());
   } else {
 ::snprintf(err_str, err_len,
-   "failed to get the task for process %i", pid);
+   "failed to get the task for process %i: this likely "
+   "means the process cannot be debugged, either because "
+   "it's a system process or because the process is "
+   "missing the get-task-allow entitlement.",
+   pid);
   }
 }
   } else {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 9c135f1 - [lldb] Fix data race in NativeFile

2023-08-10 Thread Jonas Devlieghere via lldb-commits


Author: Jonas Devlieghere
Date: 2023-08-10T11:13:47-07:00
New Revision: 9c135f1478cd9d7c81c11c6a6c9ac85b16d68d74

URL: 
https://github.com/llvm/llvm-project/commit/9c135f1478cd9d7c81c11c6a6c9ac85b16d68d74
DIFF: 
https://github.com/llvm/llvm-project/commit/9c135f1478cd9d7c81c11c6a6c9ac85b16d68d74.diff

LOG: [lldb] Fix data race in NativeFile

TSan reports the following data race:

  Write of size 4 at 0x000109e0b160 by thread T2 (...):
#0 lldb_private::NativeFile::Close() File.cpp:329
#1 lldb_private::ConnectionFileDescriptor::Disconnect(...) 
ConnectionFileDescriptorPosix.cpp:232
#2 lldb_private::Communication::Disconnect(...) Communication.cpp:61
#3 lldb_private::process_gdb_remote::ProcessGDBRemote::DidExit() 
ProcessGDBRemote.cpp:1164
#4 lldb_private::Process::SetExitStatus(...) Process.cpp:1097
#5 
lldb_private::process_gdb_remote::ProcessGDBRemote::MonitorDebugserverProcess(...)
 ProcessGDBRemote.cpp:3387

  Previous read of size 4 at 0x000109e0b160 by main thread (...):
#0 lldb_private::NativeFile::IsValid() const File.h:393
#1 lldb_private::ConnectionFileDescriptor::IsConnected() const 
ConnectionFileDescriptorPosix.cpp:121
#2 lldb_private::Communication::IsConnected() const Communication.cpp:79
#3 
lldb_private::process_gdb_remote::GDBRemoteCommunication::WaitForPacketNoLock(...)
 GDBRemoteCommunication.cpp:256
#4 
lldb_private::process_gdb_remote::GDBRemoteCommunication::WaitForPacketNoLock(...)
 GDBRemoteCommunication.cpp:244
#5 
lldb_private::process_gdb_remote::GDBRemoteClientBase::SendPacketAndWaitForResponseNoLock(...)
 GDBRemoteClientBase.cpp:246

I originally tried fixing the problem at the ConnectionFileDescriptor
level, but that operates on an IOObject which can have different thread
safety guarantees depending on its implementation.

For this particular issue, the problem is specific to NativeFile.
NativeFile can hold a file descriptor and/or a file stream. Throughout
its implementation, it checks if the descriptor or stream is valid and
do some operation on it if it is. While that works in a single threaded
environment, nothing prevents another thread from modifying the
descriptor or stream between the IsValid check and when it's actually
being used.

This patch prevents such issues by returning a ValueGuard RAII object.
As long as the object is in scope, the value is guaranteed by a lock.

Differential revision: https://reviews.llvm.org/D157347

Added: 


Modified: 
lldb/include/lldb/Host/File.h
lldb/source/Host/common/File.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/File.h b/lldb/include/lldb/Host/File.h
index a1199a51b8a691..5ce53c93b1b910 100644
--- a/lldb/include/lldb/Host/File.h
+++ b/lldb/include/lldb/Host/File.h
@@ -389,9 +389,7 @@ class NativeFile : public File {
 
   ~NativeFile() override { Close(); }
 
-  bool IsValid() const override {
-return DescriptorIsValid() || StreamIsValid();
-  }
+  bool IsValid() const override;
 
   Status Read(void *buf, size_t _bytes) override;
   Status Write(const void *buf, size_t _bytes) override;
@@ -417,15 +415,37 @@ class NativeFile : public File {
   static bool classof(const File *file) { return file->isA(); }
 
 protected:
-  bool DescriptorIsValid() const {
+  struct ValueGuard {
+ValueGuard(std::mutex , bool b) : guard(m, std::adopt_lock), value(b) {}
+std::lock_guard guard;
+bool value;
+operator bool() { return value; }
+  };
+
+  bool DescriptorIsValidUnlocked() const {
+
 return File::DescriptorIsValid(m_descriptor);
   }
-  bool StreamIsValid() const { return m_stream != kInvalidStream; }
 
-  // Member variables
+  bool StreamIsValidUnlocked() const { return m_stream != kInvalidStream; }
+
+  ValueGuard DescriptorIsValid() const {
+m_descriptor_mutex.lock();
+return ValueGuard(m_descriptor_mutex, DescriptorIsValidUnlocked());
+  }
+
+  ValueGuard StreamIsValid() const {
+m_stream_mutex.lock();
+return ValueGuard(m_stream_mutex, StreamIsValidUnlocked());
+  }
+
   int m_descriptor;
   bool m_own_descriptor = false;
+  mutable std::mutex m_descriptor_mutex;
+
   FILE *m_stream;
+  mutable std::mutex m_stream_mutex;
+
   OpenOptions m_options{};
   bool m_own_stream = false;
   std::mutex offset_access_mutex;

diff  --git a/lldb/source/Host/common/File.cpp 
b/lldb/source/Host/common/File.cpp
index 7c5d71d9426ead..82cfcedd616fc2 100644
--- a/lldb/source/Host/common/File.cpp
+++ b/lldb/source/Host/common/File.cpp
@@ -219,7 +219,7 @@ size_t File::Printf(const char *format, ...) {
 size_t File::PrintfVarArg(const char *format, va_list args) {
   llvm::SmallString<0> s;
   if (VASprintf(s, format, args)) {
-size_t written = s.size();;
+size_t written = s.size();
 Write(s.data(), written);
 return written;
   }
@@ -247,15 +247,21 @@ uint32_t File::GetPermissions(Status ) const {
   return file_stats.st_mode & (S_IRWXU |

[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in NativeFile

2023-08-10 Thread Jonas Devlieghere via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9c135f1478cd: [lldb] Fix data race in NativeFile (authored 
by JDevlieghere).

Changed prior to commit:
  https://reviews.llvm.org/D157347?vs=548849=549116#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157347

Files:
  lldb/include/lldb/Host/File.h
  lldb/source/Host/common/File.cpp

Index: lldb/source/Host/common/File.cpp
===
--- lldb/source/Host/common/File.cpp
+++ lldb/source/Host/common/File.cpp
@@ -219,7 +219,7 @@
 size_t File::PrintfVarArg(const char *format, va_list args) {
   llvm::SmallString<0> s;
   if (VASprintf(s, format, args)) {
-size_t written = s.size();;
+size_t written = s.size();
 Write(s.data(), written);
 return written;
   }
@@ -247,15 +247,21 @@
   return file_stats.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO);
 }
 
+bool NativeFile::IsValid() const {
+  std::scoped_lock lock(m_descriptor_mutex, m_stream_mutex);
+  return DescriptorIsValidUnlocked() || StreamIsValidUnlocked();
+}
+
 Expected NativeFile::GetOptions() const { return m_options; }
 
 int NativeFile::GetDescriptor() const {
-  if (DescriptorIsValid())
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 return m_descriptor;
+  }
 
   // Don't open the file descriptor if we don't need to, just get it from the
   // stream if we have one.
-  if (StreamIsValid()) {
+  if (ValueGuard stream_guard = StreamIsValid()) {
 #if defined(_WIN32)
 return _fileno(m_stream);
 #else
@@ -272,8 +278,9 @@
 }
 
 FILE *NativeFile::GetStream() {
-  if (!StreamIsValid()) {
-if (DescriptorIsValid()) {
+  ValueGuard stream_guard = StreamIsValid();
+  if (!stream_guard) {
+if (ValueGuard descriptor_guard = DescriptorIsValid()) {
   auto mode = GetStreamOpenModeFromOptions(m_options);
   if (!mode)
 llvm::consumeError(mode.takeError());
@@ -282,9 +289,9 @@
 // We must duplicate the file descriptor if we don't own it because when you
 // call fdopen, the stream will own the fd
 #ifdef _WIN32
-  m_descriptor = ::_dup(GetDescriptor());
+  m_descriptor = ::_dup(m_descriptor);
 #else
-  m_descriptor = dup(GetDescriptor());
+  m_descriptor = dup(m_descriptor);
 #endif
   m_own_descriptor = true;
 }
@@ -306,8 +313,11 @@
 }
 
 Status NativeFile::Close() {
+  std::scoped_lock lock(m_descriptor_mutex, m_stream_mutex);
+
   Status error;
-  if (StreamIsValid()) {
+
+  if (StreamIsValidUnlocked()) {
 if (m_own_stream) {
   if (::fclose(m_stream) == EOF)
 error.SetErrorToErrno();
@@ -322,15 +332,17 @@
   }
 }
   }
-  if (DescriptorIsValid() && m_own_descriptor) {
+
+  if (DescriptorIsValidUnlocked() && m_own_descriptor) {
 if (::close(m_descriptor) != 0)
   error.SetErrorToErrno();
   }
-  m_descriptor = kInvalidDescriptor;
+
   m_stream = kInvalidStream;
-  m_options = OpenOptions(0);
   m_own_stream = false;
+  m_descriptor = kInvalidDescriptor;
   m_own_descriptor = false;
+  m_options = OpenOptions(0);
   m_is_interactive = eLazyBoolCalculate;
   m_is_real_terminal = eLazyBoolCalculate;
   return error;
@@ -374,7 +386,7 @@
 
 off_t NativeFile::SeekFromStart(off_t offset, Status *error_ptr) {
   off_t result = 0;
-  if (DescriptorIsValid()) {
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 result = ::lseek(m_descriptor, offset, SEEK_SET);
 
 if (error_ptr) {
@@ -383,7 +395,10 @@
   else
 error_ptr->Clear();
 }
-  } else if (StreamIsValid()) {
+return result;
+  }
+
+  if (ValueGuard stream_guard = StreamIsValid()) {
 result = ::fseek(m_stream, offset, SEEK_SET);
 
 if (error_ptr) {
@@ -392,15 +407,17 @@
   else
 error_ptr->Clear();
 }
-  } else if (error_ptr) {
-error_ptr->SetErrorString("invalid file handle");
+return result;
   }
+
+  if (error_ptr)
+error_ptr->SetErrorString("invalid file handle");
   return result;
 }
 
 off_t NativeFile::SeekFromCurrent(off_t offset, Status *error_ptr) {
   off_t result = -1;
-  if (DescriptorIsValid()) {
+  if (ValueGuard descriptor_guard = DescriptorIsValid()) {
 result = ::lseek(m_descriptor, offset, SEEK_CUR);
 
 if (error_ptr) {
@@ -409,7 +426,10 @@
   else
 error_ptr->Clear();
 }
-  } else if (StreamIsValid()) {
+return result;
+  }
+
+  if (ValueGuard stream_guard = StreamIsValid()) {
 result = ::fseek(m_stream, offset, SEEK_CUR);
 
 if (error_ptr) {
@@ -418,15 +438,17 @@
   else
 error_ptr->Clear();
 }
-  } else if (error_ptr) {
-error_ptr->SetErrorString("invalid file handle");
+return result;
   }
+
+  if (error_ptr)
+error_ptr->SetErrorString("invalid file handle");
   return result;
 }
 
 off_t NativeFile::SeekFromEnd(off_t offset, Status *error_ptr) {
   off_t result = -1;
-  if

[Lldb-commits] [PATCH] D157636: [lldb][test] Remove tests relying on deprecated std::char_traits specializations

2023-08-10 Thread Michael Buch via Phabricator via lldb-commits

Michael137 created this revision.
Michael137 added reviewers: gribozavr2, JDevlieghere.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

(motivated by test failures after D157058 )

With D157058  the base template for 
`std::char_traits` was removed from
libc++. Quoting the release notes:

  The base template for ``std::char_traits`` has been removed. If you are using
  ``std::char_traits`` with types other than ``char``, ``wchar_t``, ``char8_t``,
  ``char16_t``, ``char32_t`` or a custom character type for which you
  specialized ``std::char_traits``, your code will no longer work.

This patch simply removes all such instantiations to make sure the
tests that run against the latest libc++ version pass.

One could try testing the existence of this base template from within
the test source files but this doesn't seem like something we want
support.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157636

Files:
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
  
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp

Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/main.cpp
@@ -9,7 +9,6 @@
 std::string empty("");
 std::string q("hello world");
 std::string Q("quite a long std::strin with lots of info inside it");
-std::basic_string uchar(5, 'a');
 auto  = q,  = Q;
 std::string *pq = , *pQ = 
 S.assign(L"!"); // Set break point at this line.
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/string/TestDataFormatterStdString.py
@@ -61,8 +61,6 @@
 var_pq = self.frame().FindVariable("pq")
 var_pQ = self.frame().FindVariable("pQ")
 
-var_uchar = self.frame().FindVariable("uchar")
-
 self.assertEqual(var_wempty.GetSummary(), 'L""', "wempty summary wrong")
 self.assertEqual(
 var_s.GetSummary(), 'L"hello world! ××× ×××!"', "s summary wrong"
@@ -78,7 +76,6 @@
 '"quite a long std::strin with lots of info inside it"',
 "Q summary wrong",
 )
-self.assertEqual(var_uchar.GetSummary(), '"a"', "u summary wrong")
 self.assertEqual(var_rq.GetSummary(), '"hello world"', "rq summary wrong")
 self.assertEqual(
 var_rQ.GetSummary(),
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/main.cpp
@@ -92,8 +92,6 @@
   std::u16string_view u16_empty(u"");
   std::u32string_view u32_string(U"ðððð");
   std::u32string_view u32_empty(U"");
-  std::basic_string uchar_source(10, 'a');
-  std::basic_string_view uchar(uchar_source.data(), 5);
   std::string_view *null_str = nullptr;
 
   std::string hello = "Hellooo ";
Index: lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
===
--- lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
+++ lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/string_view/TestDataFormatterLibcxxStringView.py
@@ -60,15 +60,6 @@
 # Execute the cleanup function during test case tear down.
 self.addTearDownHook(cleanup)
 
-if self.expectedCompiler(["clang"]) and self.expectedCompilerVersion(
-[">", "16.0"]
-):
-expected_basic_string = "std::basic_string"
-

[Lldb-commits] [PATCH] D157347: [lldb] Fix data race in NativeFile

2023-08-10 Thread Alex Langford via Phabricator via lldb-commits

bulbazord accepted this revision.
bulbazord added a comment.
This revision is now accepted and ready to land.

LGTM!


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

https://reviews.llvm.org/D157347

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

[Lldb-commits] [PATCH] D141702: [lldb/crashlog] Make module loading use Scripted Process affordance

2023-08-10 Thread Alex Langford via Phabricator via lldb-commits

bulbazord accepted this revision.
bulbazord added a comment.

Sorry, this fell off my radar for a while. I re-read through it and it looks 
fine to me, the lambda vs static methods function thing doesn't really matter 
much. LGTM.


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

https://reviews.llvm.org/D141702

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

[Lldb-commits] [PATCH] D157556: Replace the singleton "ShadowListener" with a primary and N secondary listeners

2023-08-10 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/include/lldb/Utility/Event.h:228
   void Clear() { m_data_sp.reset(); }
+  
+  /// This is used by Broadcasters with Primary Listeners to store the other

Nit: trailing whitespace



Comment at: 
lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py:26
 @skipUnlessDarwin
-@skipIfDarwin
+#@skipIfDarwin
 def test_passthrough_launch(self):

bulbazord wrote:
> Remove this comment?
Remove?



Comment at: 
lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py:56
 @skipUnlessDarwin
-@skipIfDarwin
+#@skipIfDarwin
 def test_multiplexed_launch(self):

bulbazord wrote:
> delete?
Remove?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157556

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

[Lldb-commits] [PATCH] D157609: WIP: [lldb] Search debug file paths when looking for split DWARF files

2023-08-10 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett added a comment.

For https://github.com/llvm/llvm-project/issues/28667


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157609

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

[Lldb-commits] [PATCH] D157609: WIP: [lldb] Search debug file paths when looking for split DWARF files

2023-08-10 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett created this revision.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: lldb-commits, jplehr, sstefan1.
Herald added a project: LLDB.
DavidSpickett planned changes to this revision.
Herald added a subscriber: JDevlieghere.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157609

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1731,7 +1731,11 @@
   const char *comp_dir = nullptr;
   FileSpec dwo_file(dwo_name);
   FileSystem::Instance().Resolve(dwo_file);
-  if (dwo_file.IsRelative()) {
+  bool found = false;
+
+  if (!dwo_file.IsRelative()) {
+found = FileSystem::Instance().Exists(dwo_file);
+  } else {
 comp_dir = cu_die.GetAttributeValueAsString(dwarf_cu, DW_AT_comp_dir,
 nullptr);
 if (!comp_dir) {
@@ -1744,18 +1748,52 @@
 }
 
 dwo_file.SetFile(comp_dir, FileSpec::Style::native);
-if (dwo_file.IsRelative()) {
+
+if (!dwo_file.IsRelative()) {
+  // TODO: should we also search debug file paths here?
+  FileSystem::Instance().Resolve(dwo_file);
+  dwo_file.AppendPathComponent(dwo_name);
+  found = FileSystem::Instance().Exists(dwo_file);
+} else {
+  FileSpecList dwo_paths;
+
   // if DW_AT_comp_dir is relative, it should be relative to the location
   // of the executable, not to the location from which the debugger was
   // launched.
-  dwo_file.PrependPathComponent(
+  FileSpec relative_to_binary = dwo_file;
+  relative_to_binary.PrependPathComponent(
   m_objfile_sp->GetFileSpec().GetDirectory().GetStringRef());
+  FileSystem::Instance().Resolve(relative_to_binary);
+  relative_to_binary.AppendPathComponent(dwo_name);
+  dwo_paths.Append(relative_to_binary);
+
+  // Or it's relative to one of the user specified debug directories.
+  const FileSpecList _file_search_paths =
+  Target::GetDefaultDebugFileSearchPaths();
+  size_t num_directories = debug_file_search_paths.GetSize();
+  for (size_t idx = 0; idx < num_directories; ++idx) {
+FileSpec dirspec = debug_file_search_paths.GetFileSpecAtIndex(idx);
+dirspec.AppendPathComponent(comp_dir);
+FileSystem::Instance().Resolve(dirspec);
+if (!FileSystem::Instance().IsDirectory(dirspec))
+  continue;
+
+dirspec.AppendPathComponent(dwo_name);
+dwo_paths.Append(dirspec);
+  }
+
+  size_t num_possible = dwo_paths.GetSize();
+  for (size_t idx = 0; idx < num_possible && !found; ++idx) {
+FileSpec dwo_spec = dwo_paths.GetFileSpecAtIndex(idx);
+if (FileSystem::Instance().Exists(dwo_spec)) {
+  dwo_file = dwo_spec;
+  found = true;
+}
+  }
 }
-FileSystem::Instance().Resolve(dwo_file);
-dwo_file.AppendPathComponent(dwo_name);
   }
 
-  if (!FileSystem::Instance().Exists(dwo_file)) {
+  if (!found) {
 unit.SetDwoError(Status::createWithFormat(
 "unable to locate .dwo debug file \"{0}\" for skeleton DIE "
 "{1:x16}",


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1731,7 +1731,11 @@
   const char *comp_dir = nullptr;
   FileSpec dwo_file(dwo_name);
   FileSystem::Instance().Resolve(dwo_file);
-  if (dwo_file.IsRelative()) {
+  bool found = false;
+
+  if (!dwo_file.IsRelative()) {
+found = FileSystem::Instance().Exists(dwo_file);
+  } else {
 comp_dir = cu_die.GetAttributeValueAsString(dwarf_cu, DW_AT_comp_dir,
 nullptr);
 if (!comp_dir) {
@@ -1744,18 +1748,52 @@
 }
 
 dwo_file.SetFile(comp_dir, FileSpec::Style::native);
-if (dwo_file.IsRelative()) {
+
+if (!dwo_file.IsRelative()) {
+  // TODO: should we also search debug file paths here?
+  FileSystem::Instance().Resolve(dwo_file);
+  dwo_file.AppendPathComponent(dwo_name);
+  found = FileSystem::Instance().Exists(dwo_file);
+} else {
+  FileSpecList dwo_paths;
+
   // if DW_AT_comp_dir is relative, it should be relative to the location
   // of the executable, not to the location from which the debugger was
   // launched.
-  dwo_file.PrependPathComponent(
+  FileSpec relative_to_binary = dwo_file;
+  relative_to_binary.PrependPathComponent(
   m_objfile_sp->GetFileSpec().GetDirectory().GetStringRef());
+  FileSystem::Instance().Resolve(relative_to_binary);
+

[Lldb-commits] [lldb] 68744ff - [lldb] Fix building with latest libc++

2023-08-10 Thread Martin Storsjö via lldb-commits


Author: Martin Storsjö
Date: 2023-08-10T12:59:35+03:00
New Revision: 68744ffbdd7daac41da274eef9ac0d191e11c16d

URL: 
https://github.com/llvm/llvm-project/commit/68744ffbdd7daac41da274eef9ac0d191e11c16d
DIFF: 
https://github.com/llvm/llvm-project/commit/68744ffbdd7daac41da274eef9ac0d191e11c16d.diff

LOG: [lldb] Fix building with latest libc++

Since https://reviews.llvm.org/D157058 in libc++,
the base template for char_traits has been removed - it is only
provided for char, wchar_t, char8_t, char16_t and char32_t.
(Thus, to use basic_string with a type other than those, we'd need
to supply suitable traits ourselves.)

For this particular use, a vector works just as well as basic_string.

Differential Revision: https://reviews.llvm.org/D157589

Added: 


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

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
index 4efc454967a123..f9d95fc5d2a66d 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -631,7 +631,7 @@ static void WriteRegisterValueInHexFixedWidth(
   } else {
 // Zero-out any unreadable values.
 if (reg_info.byte_size > 0) {
-  std::basic_string zeros(reg_info.byte_size, '\0');
+  std::vector zeros(reg_info.byte_size, '\0');
   AppendHexValue(response, zeros.data(), zeros.size(), false);
 }
   }



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

[Lldb-commits] [PATCH] D157589: [lldb] Fix building with latest libc++

2023-08-10 Thread Martin Storsjö via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG68744ffbdd7d: [lldb] Fix building with latest libc++ 
(authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157589

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


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
@@ -631,7 +631,7 @@
   } else {
 // Zero-out any unreadable values.
 if (reg_info.byte_size > 0) {
-  std::basic_string zeros(reg_info.byte_size, '\0');
+  std::vector zeros(reg_info.byte_size, '\0');
   AppendHexValue(response, zeros.data(), zeros.size(), false);
 }
   }


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
@@ -631,7 +631,7 @@
   } else {
 // Zero-out any unreadable values.
 if (reg_info.byte_size > 0) {
-  std::basic_string zeros(reg_info.byte_size, '\0');
+  std::vector zeros(reg_info.byte_size, '\0');
   AppendHexValue(response, zeros.data(), zeros.size(), false);
 }
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D157589: [lldb] Fix building with latest libc++

2023-08-10 Thread David Spickett via Phabricator via lldb-commits

DavidSpickett accepted this revision.
DavidSpickett added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157589

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

[Lldb-commits] [PATCH] D157589: [lldb] Fix building with latest libc++

2023-08-10 Thread Martin Storsjö via Phabricator via lldb-commits

mstorsjo created this revision.
mstorsjo added reviewers: JDevlieghere, mib, jingham.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: LLDB.

Since https://reviews.llvm.org/D157058 in libc++,
the base template for char_traits has been removed - it is only
provided for char, wchar_t, char8_t, char16_t and char32_t.
(Thus, to use basic_string with a type other than those, we'd need
to supply suitable traits ourselves.)

For this particular use, a vector works just as well as basic_string.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157589

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


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
@@ -631,7 +631,7 @@
   } else {
 // Zero-out any unreadable values.
 if (reg_info.byte_size > 0) {
-  std::basic_string zeros(reg_info.byte_size, '\0');
+  std::vector zeros(reg_info.byte_size, '\0');
   AppendHexValue(response, zeros.data(), zeros.size(), false);
 }
   }


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
@@ -631,7 +631,7 @@
   } else {
 // Zero-out any unreadable values.
 if (reg_info.byte_size > 0) {
-  std::basic_string zeros(reg_info.byte_size, '\0');
+  std::vector zeros(reg_info.byte_size, '\0');
   AppendHexValue(response, zeros.data(), zeros.size(), false);
 }
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

37 matches

Mail list logo