[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

[Jason Molenda via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6f4a0c762fe2: hi/low addr space bits can be sent in 
stop-rely packet (authored by jasonmolenda).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158041

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/include/lldb/Utility/AddressableBits.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
  lldb/source/Utility/AddressableBits.cpp

Index: lldb/source/Utility/AddressableBits.cpp
===
--- lldb/source/Utility/AddressableBits.cpp
+++ lldb/source/Utility/AddressableBits.cpp
@@ -23,28 +23,57 @@
   m_high_memory_addr_bits = highmem_addressing_bits;
 }
 
-void AddressableBits::Clear() {
-  m_low_memory_addr_bits = m_high_memory_addr_bits = 0;
+void AddressableBits::SetLowmemAddressableBits(
+uint32_t lowmem_addressing_bits) {
+  m_low_memory_addr_bits = lowmem_addressing_bits;
+}
+
+void AddressableBits::SetHighmemAddressableBits(
+uint32_t highmem_addressing_bits) {
+  m_high_memory_addr_bits = highmem_addressing_bits;
 }
 
 void AddressableBits::SetProcessMasks(Process ) {
-  // In case either value is set to 0, indicating it was not set, use the
-  // other value.
-  if (m_low_memory_addr_bits == 0)
-m_low_memory_addr_bits = m_high_memory_addr_bits;
-  if (m_high_memory_addr_bits == 0)
-m_high_memory_addr_bits = m_low_memory_addr_bits;
-
-  if (m_low_memory_addr_bits == 0)
+  if (m_low_memory_addr_bits == 0 && m_high_memory_addr_bits == 0)
 return;
 
-  addr_t address_mask = ~((1ULL << m_low_memory_addr_bits) - 1);
-  process.SetCodeAddressMask(address_mask);
-  process.SetDataAddressMask(address_mask);
+  // If we don't have an addressable bits value for low memory,
+  // see if we have a Code/Data mask already, and use that.
+  // Or use the high memory addressable bits value as a last
+  // resort.
+  addr_t low_addr_mask;
+  if (m_low_memory_addr_bits == 0) {
+if (process.GetCodeAddressMask() != UINT64_MAX)
+  low_addr_mask = process.GetCodeAddressMask();
+else if (process.GetDataAddressMask() != UINT64_MAX)
+  low_addr_mask = process.GetDataAddressMask();
+else
+  low_addr_mask = ~((1ULL << m_high_memory_addr_bits) - 1);
+  } else {
+low_addr_mask = ~((1ULL << m_low_memory_addr_bits) - 1);
+  }
+
+  // If we don't have an addressable bits value for high memory,
+  // see if we have a Code/Data mask already, and use that.
+  // Or use the low memory addressable bits value as a last
+  // resort.
+  addr_t hi_addr_mask;
+  if (m_high_memory_addr_bits == 0) {
+if (process.GetHighmemCodeAddressMask() != UINT64_MAX)
+  hi_addr_mask = process.GetHighmemCodeAddressMask();
+else if (process.GetHighmemDataAddressMask() != UINT64_MAX)
+  hi_addr_mask = process.GetHighmemDataAddressMask();
+else
+  hi_addr_mask = ~((1ULL << m_low_memory_addr_bits) - 1);
+  } else {
+hi_addr_mask = ~((1ULL << m_high_memory_addr_bits) - 1);
+  }
+
+  process.SetCodeAddressMask(low_addr_mask);
+  process.SetDataAddressMask(low_addr_mask);
 
-  if (m_low_memory_addr_bits != m_high_memory_addr_bits) {
-lldb::addr_t hi_address_mask = ~((1ULL << m_high_memory_addr_bits) - 1);
-process.SetHighmemCodeAddressMask(hi_address_mask);
-process.SetHighmemDataAddressMask(hi_address_mask);
+  if (low_addr_mask != hi_addr_mask) {
+process.SetHighmemCodeAddressMask(hi_addr_mask);
+process.SetHighmemDataAddressMask(hi_addr_mask);
   }
 }
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
@@ -574,10 +574,9 @@
 
   CleanupMemoryRegionPermissions();
 
-  AddressableBits addressable_bits;
-  if (core_objfile->GetAddressableBits(addressable_bits)) {
-addressable_bits.SetProcessMasks(*this);
-  }
+  AddressableBits addressable_bits = core_objfile->GetAddressableBits();
+  addressable_bits.SetProcessMasks(*this);
+
   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,8 @@
  process_arch.GetTriple().getTriple());
   }
 
-  AddressableBits addressable_bits;
-  if 

[Lldb-commits] [lldb] 6f4a0c7 - hi/low addr space bits can be sent in stop-rely packet

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2023-08-16T16:12:18-07:00
New Revision: 6f4a0c762fe2c0077865e0e30e3dfd67cd6287d1

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

LOG: hi/low addr space bits can be sent in stop-rely packet

Add support for the `low_mem_addressing_bits` and
`high_mem_addressing_bits` keys in the stop reply packet,
in addition to the existing `addressing_bits`.  Same
behavior as in the qHostInfo packet.

Clean up AddressableBits so we don't need to check if
any values have been set in the object before using it
to potentially update the Process address masks.

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

Added: 


Modified: 
lldb/docs/lldb-gdb-remote.txt
lldb/include/lldb/Symbol/ObjectFile.h
lldb/include/lldb/Utility/AddressableBits.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
lldb/source/Utility/AddressableBits.cpp

Removed: 




diff  --git a/lldb/docs/lldb-gdb-remote.txt b/lldb/docs/lldb-gdb-remote.txt
index bfc10133051a3a..6eed70eab043fe 100644
--- a/lldb/docs/lldb-gdb-remote.txt
+++ b/lldb/docs/lldb-gdb-remote.txt
@@ -1661,6 +1661,18 @@ for this region.
 //   start code that may be changing the
 //   page table setup, a dynamically set
 //   value may be needed.
+//  "low_mem_addressing_bits" unsigned optional, specifies how many bits in 
+//   addresses in low memory are 
significant 
+//   for addressing, base 10.  AArch64 can 
+//   have 
diff erent page table setups for low 
+//   and high memory, and therefore a 
diff erent 
+//   number of bits used for addressing.
+//  "high_mem_addressing_bits" unsigned optional, specifies how many bits in 
+//   addresses in high memory are 
significant 
+//   for addressing, base 10.  AArch64 can 
have 
+//   
diff erent page table setups for low and 
+//   high memory, and therefore a 
diff erent 
+//   number of bits used for addressing.
 //
 // BEST PRACTICES:
 //  Since register values can be supplied with this packet, it is often useful

diff  --git a/lldb/include/lldb/Symbol/ObjectFile.h 
b/lldb/include/lldb/Symbol/ObjectFile.h
index 02d1248e23c342..9519635c24f9ff 100644
--- a/lldb/include/lldb/Symbol/ObjectFile.h
+++ b/lldb/include/lldb/Symbol/ObjectFile.h
@@ -498,15 +498,10 @@ class ObjectFile : public 
std::enable_shared_from_this,
   /// object files can return an AddressableBits object that can can be
   /// used to set the address masks in the Process.
   ///
-  /// \param[out] address_bits
-  /// Can be used to set the Process address masks.
-  ///
   /// \return
-  /// Returns true if addressable bits metadata was found.
-  virtual bool GetAddressableBits(lldb_private::AddressableBits _bits) 
{
-address_bits.Clear();
-return false;
-  }
+  /// Returns an AddressableBits object which can be used to set
+  /// the address masks in the Process.
+  virtual lldb_private::AddressableBits GetAddressableBits() { return {}; }
 
   /// When the ObjectFile is a core file, lldb needs to locate the "binary" in
   /// the core file.  lldb can iterate over the pages looking for a valid

diff  --git a/lldb/include/lldb/Utility/AddressableBits.h 
b/lldb/include/lldb/Utility/AddressableBits.h
index d7e2e341569655..13c21329a8c617 100644
--- a/lldb/include/lldb/Utility/AddressableBits.h
+++ b/lldb/include/lldb/Utility/AddressableBits.h
@@ -29,9 +29,11 @@ class AddressableBits {
   void SetAddressableBits(uint32_t lowmem_addressing_bits,
   uint32_t highmem_addressing_bits);
 
-  void SetProcessMasks(lldb_private::Process );
+  void SetLowmemAddressableBits(uint32_t lowmem_addressing_bits);
+
+  void SetHighmemAddressableBits(uint32_t highmem_addressing_bits);
 
-  void Clear();
+  void SetProcessMasks(lldb_private::Process );
 
 private:
   uint32_t m_low_memory_addr_bits;

diff  --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index b737733c1b5148..3b52f718b6dab4 

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

[Jason Molenda via Phabricator via lldb-commits]

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

Updated back to my most recent version of this patch, thanks for the help all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155905

Files:
  lldb/include/lldb/API/SBProcess.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/API/SBProcess.cpp

Index: lldb/source/API/SBProcess.cpp
===
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -1244,6 +1244,73 @@
   return sb_proc_info;
 }
 
+addr_t SBProcess::GetAddressMask(lldb::AddressMaskType type) {
+  LLDB_INSTRUMENT_VA(this, type);
+  addr_t default_mask = 0;
+  if (ProcessSP process_sp = GetSP()) {
+switch (type) {
+case eMaskTypeCode:
+  return process_sp->GetCodeAddressMask();
+case eMaskTypeData:
+  return process_sp->GetDataAddressMask();
+case eMaskTypeHighmemCode:
+  return process_sp->GetHighmemCodeAddressMask();
+case eMaskTypeHighmemData:
+  return process_sp->GetHighmemDataAddressMask();
+case eMaskTypeAny:
+  return process_sp->GetDataAddressMask();
+}
+  }
+  return default_mask;
+}
+
+void SBProcess::SetAddressMask(lldb::AddressMaskType type, addr_t mask) {
+  LLDB_INSTRUMENT_VA(this, type, mask);
+  if (ProcessSP process_sp = GetSP()) {
+switch (type) {
+case eMaskTypeCode:
+  process_sp->SetCodeAddressMask(mask);
+  break;
+case eMaskTypeData:
+  process_sp->SetDataAddressMask(mask);
+  break;
+case eMaskTypeHighmemCode:
+  process_sp->SetHighmemCodeAddressMask(mask);
+  break;
+case eMaskTypeHighmemData:
+  process_sp->SetHighmemDataAddressMask(mask);
+  break;
+case eMaskTypeAll:
+  process_sp->SetCodeAddressMask(mask);
+  process_sp->SetDataAddressMask(mask);
+  process_sp->SetHighmemCodeAddressMask(mask);
+  process_sp->SetHighmemDataAddressMask(mask);
+  break;
+}
+  }
+}
+
+addr_t SBProcess::FixCodeAddress(addr_t addr) {
+  LLDB_INSTRUMENT_VA(this, addr);
+  if (ProcessSP process_sp = GetSP())
+return process_sp->FixCodeAddress(addr);
+  return addr;
+}
+
+addr_t SBProcess::FixDataAddress(addr_t addr) {
+  LLDB_INSTRUMENT_VA(this, addr);
+  if (ProcessSP process_sp = GetSP())
+return process_sp->FixDataAddress(addr);
+  return addr;
+}
+
+addr_t SBProcess::FixAnyAddress(addr_t addr) {
+  LLDB_INSTRUMENT_VA(this, addr);
+  if (ProcessSP process_sp = GetSP())
+return process_sp->FixAnyAddress(addr);
+  return addr;
+}
+
 lldb::addr_t SBProcess::AllocateMemory(size_t size, uint32_t permissions,
lldb::SBError _error) {
   LLDB_INSTRUMENT_VA(this, size, permissions, sb_error);
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -1283,6 +1283,15 @@
   eCustomCompletion = (1u << 25)
 };
 
+enum AddressMaskType {
+  eMaskTypeCode = 0,
+  eMaskTypeData,
+  eMaskTypeHighmemCode,
+  eMaskTypeHighmemData,
+  eMaskTypeAny,
+  eMaskTypeAll = eMaskTypeAny
+};
+
 } // namespace lldb
 
 #endif // LLDB_LLDB_ENUMERATIONS_H
Index: lldb/include/lldb/API/SBProcess.h
===
--- lldb/include/lldb/API/SBProcess.h
+++ lldb/include/lldb/API/SBProcess.h
@@ -398,6 +398,52 @@
   /// valid.
   lldb::SBProcessInfo GetProcessInfo();
 
+  /// Get the current address mask that may be applied to addresses
+  /// before reading from memory.
+  ///
+  /// \param[in] type
+  /// lldb may have different address masks for code and data
+  /// addresses.  And it may have different masks for code
+  /// and data in high memory, which differ from the low memory
+  /// code/data masks.  Each of these can be requested, or
+  /// most commonly, eMaskTypeAny can be requested, with the
+  /// assumption that the masks are all identical.
+  ///
+  /// \return
+  /// The address mask currently in use.  Bits which are not used
+  /// for addressing will be set to 1 in the mask.
+  lldb::addr_t GetAddressMask(lldb::AddressMaskType type);
+
+  /// Set the current address mask that may be applied to addresses
+  /// before reading from memory.
+  ///
+  /// \param[in] type
+  /// lldb can have different address masks for code and data
+  /// addresses.  And it can have different masks for code
+  /// and data in high memory, which differ from the low memory
+  /// code/data masks.  Each of these can be set, or
+  /// most commonly, eMaskTypeall can be set, when all masks are
+  /// identical.
+  ///
+  /// \param[in] mask
+  /// The address mask to set.  Bits which are not used for addressing
+  /// should be set to 1 in the mask.
+  void SetAddressMask(lldb::AddressMaskType type, 

[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

[Jonas Devlieghere via Phabricator via lldb-commits]

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

Thanks, this LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158041

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

[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

[Jason Molenda via Phabricator via lldb-commits]

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

Update patch to remove the HasValue/IsValue and Clear methods from 
AddressableBits. Change methods that may fill in an AddressableBits object with 
values to return an AddressableBits object unconditionally. Change 
`AddressableBits::SetProcessMasks` so it can be called unconditionally, and it 
only sets mask values in Process if values were set.

I wanted to allow for someone to create an AddressableBits object with only the 
high memory addressing bits specified (for instance) and the low memory address 
mask will not be changed. I updated `AddressableBits::SetProcessMasks` to use 
the current Process mask values if it doesn't have a value specified. This will 
be more important when I add the SB API to SBProcess to set address masks in 
https://reviews.llvm.org/D155905 I expect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158041

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/include/lldb/Utility/AddressableBits.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
  lldb/source/Utility/AddressableBits.cpp

Index: lldb/source/Utility/AddressableBits.cpp
===
--- lldb/source/Utility/AddressableBits.cpp
+++ lldb/source/Utility/AddressableBits.cpp
@@ -23,28 +23,57 @@
   m_high_memory_addr_bits = highmem_addressing_bits;
 }
 
-void AddressableBits::Clear() {
-  m_low_memory_addr_bits = m_high_memory_addr_bits = 0;
+void AddressableBits::SetLowmemAddressableBits(
+uint32_t lowmem_addressing_bits) {
+  m_low_memory_addr_bits = lowmem_addressing_bits;
+}
+
+void AddressableBits::SetHighmemAddressableBits(
+uint32_t highmem_addressing_bits) {
+  m_high_memory_addr_bits = highmem_addressing_bits;
 }
 
 void AddressableBits::SetProcessMasks(Process ) {
-  // In case either value is set to 0, indicating it was not set, use the
-  // other value.
-  if (m_low_memory_addr_bits == 0)
-m_low_memory_addr_bits = m_high_memory_addr_bits;
-  if (m_high_memory_addr_bits == 0)
-m_high_memory_addr_bits = m_low_memory_addr_bits;
-
-  if (m_low_memory_addr_bits == 0)
+  if (m_low_memory_addr_bits == 0 && m_high_memory_addr_bits == 0)
 return;
 
-  addr_t address_mask = ~((1ULL << m_low_memory_addr_bits) - 1);
-  process.SetCodeAddressMask(address_mask);
-  process.SetDataAddressMask(address_mask);
+  // If we don't have an addressable bits value for low memory,
+  // see if we have a Code/Data mask already, and use that.
+  // Or use the high memory addressable bits value as a last
+  // resort.
+  addr_t low_addr_mask;
+  if (m_low_memory_addr_bits == 0) {
+if (process.GetCodeAddressMask() != UINT64_MAX)
+  low_addr_mask = process.GetCodeAddressMask();
+else if (process.GetDataAddressMask() != UINT64_MAX)
+  low_addr_mask = process.GetDataAddressMask();
+else
+  low_addr_mask = ~((1ULL << m_high_memory_addr_bits) - 1);
+  } else {
+low_addr_mask = ~((1ULL << m_low_memory_addr_bits) - 1);
+  }
+
+  // If we don't have an addressable bits value for high memory,
+  // see if we have a Code/Data mask already, and use that.
+  // Or use the low memory addressable bits value as a last
+  // resort.
+  addr_t hi_addr_mask;
+  if (m_high_memory_addr_bits == 0) {
+if (process.GetHighmemCodeAddressMask() != UINT64_MAX)
+  hi_addr_mask = process.GetHighmemCodeAddressMask();
+else if (process.GetHighmemDataAddressMask() != UINT64_MAX)
+  hi_addr_mask = process.GetHighmemDataAddressMask();
+else
+  hi_addr_mask = ~((1ULL << m_low_memory_addr_bits) - 1);
+  } else {
+hi_addr_mask = ~((1ULL << m_high_memory_addr_bits) - 1);
+  }
+
+  process.SetCodeAddressMask(low_addr_mask);
+  process.SetDataAddressMask(low_addr_mask);
 
-  if (m_low_memory_addr_bits != m_high_memory_addr_bits) {
-lldb::addr_t hi_address_mask = ~((1ULL << m_high_memory_addr_bits) - 1);
-process.SetHighmemCodeAddressMask(hi_address_mask);
-process.SetHighmemDataAddressMask(hi_address_mask);
+  if (low_addr_mask != hi_addr_mask) {
+process.SetHighmemCodeAddressMask(hi_addr_mask);
+process.SetHighmemDataAddressMask(hi_addr_mask);
   }
 }
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
@@ -574,10 +574,9 @@
 
   CleanupMemoryRegionPermissions();
 
-  AddressableBits 

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

ah dangit, I meant to update the patch in https://reviews.llvm.org/D158041 and 
accidentally blew away the last patch I had for this one.  let's see if I can 
extract it from here somehow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155905

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

[Lldb-commits] [PATCH] D155905: lldb RFC: Exposing set/get address masks, Fix*Address methods in SBProcess

[Jason Molenda via Phabricator via lldb-commits]

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

Update patch to remove the HasValue/IsValue and Clear methods from 
AddressableBits.  Change methods that may fill in an AddressableBits object 
with values to return an AddressableBits object unconditionally.  Change 
`AddressableBits::SetProcessMasks` so it can be called unconditionally, and it 
only sets mask values in Process if values were set.

I wanted to allow for someone to create an AddressableBits object with only the 
high memory addressing bits specified (for instance) and the low memory address 
mask will not be changed.  I updated `AddressableBits::SetProcessMasks` to use 
the current Process mask values if it doesn't have a value specified.  This 
will be more important when I add the SB API to SBProcess to set address masks 
in https://reviews.llvm.org/D155905 I expect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155905

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/include/lldb/Utility/AddressableBits.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
  lldb/source/Utility/AddressableBits.cpp

Index: lldb/source/Utility/AddressableBits.cpp
===
--- lldb/source/Utility/AddressableBits.cpp
+++ lldb/source/Utility/AddressableBits.cpp
@@ -23,28 +23,57 @@
   m_high_memory_addr_bits = highmem_addressing_bits;
 }
 
-void AddressableBits::Clear() {
-  m_low_memory_addr_bits = m_high_memory_addr_bits = 0;
+void AddressableBits::SetLowmemAddressableBits(
+uint32_t lowmem_addressing_bits) {
+  m_low_memory_addr_bits = lowmem_addressing_bits;
+}
+
+void AddressableBits::SetHighmemAddressableBits(
+uint32_t highmem_addressing_bits) {
+  m_high_memory_addr_bits = highmem_addressing_bits;
 }
 
 void AddressableBits::SetProcessMasks(Process ) {
-  // In case either value is set to 0, indicating it was not set, use the
-  // other value.
-  if (m_low_memory_addr_bits == 0)
-m_low_memory_addr_bits = m_high_memory_addr_bits;
-  if (m_high_memory_addr_bits == 0)
-m_high_memory_addr_bits = m_low_memory_addr_bits;
-
-  if (m_low_memory_addr_bits == 0)
+  if (m_low_memory_addr_bits == 0 && m_high_memory_addr_bits == 0)
 return;
 
-  addr_t address_mask = ~((1ULL << m_low_memory_addr_bits) - 1);
-  process.SetCodeAddressMask(address_mask);
-  process.SetDataAddressMask(address_mask);
+  // If we don't have an addressable bits value for low memory,
+  // see if we have a Code/Data mask already, and use that.
+  // Or use the high memory addressable bits value as a last
+  // resort.
+  addr_t low_addr_mask;
+  if (m_low_memory_addr_bits == 0) {
+if (process.GetCodeAddressMask() != UINT64_MAX)
+  low_addr_mask = process.GetCodeAddressMask();
+else if (process.GetDataAddressMask() != UINT64_MAX)
+  low_addr_mask = process.GetDataAddressMask();
+else
+  low_addr_mask = ~((1ULL << m_high_memory_addr_bits) - 1);
+  } else {
+low_addr_mask = ~((1ULL << m_low_memory_addr_bits) - 1);
+  }
+
+  // If we don't have an addressable bits value for high memory,
+  // see if we have a Code/Data mask already, and use that.
+  // Or use the low memory addressable bits value as a last
+  // resort.
+  addr_t hi_addr_mask;
+  if (m_high_memory_addr_bits == 0) {
+if (process.GetHighmemCodeAddressMask() != UINT64_MAX)
+  hi_addr_mask = process.GetHighmemCodeAddressMask();
+else if (process.GetHighmemDataAddressMask() != UINT64_MAX)
+  hi_addr_mask = process.GetHighmemDataAddressMask();
+else
+  hi_addr_mask = ~((1ULL << m_low_memory_addr_bits) - 1);
+  } else {
+hi_addr_mask = ~((1ULL << m_high_memory_addr_bits) - 1);
+  }
+
+  process.SetCodeAddressMask(low_addr_mask);
+  process.SetDataAddressMask(low_addr_mask);
 
-  if (m_low_memory_addr_bits != m_high_memory_addr_bits) {
-lldb::addr_t hi_address_mask = ~((1ULL << m_high_memory_addr_bits) - 1);
-process.SetHighmemCodeAddressMask(hi_address_mask);
-process.SetHighmemDataAddressMask(hi_address_mask);
+  if (low_addr_mask != hi_addr_mask) {
+process.SetHighmemCodeAddressMask(hi_addr_mask);
+process.SetHighmemDataAddressMask(hi_addr_mask);
   }
 }
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
@@ -574,10 +574,9 @@
 
   CleanupMemoryRegionPermissions();
 
-  AddressableBits 

[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In D158041#4593205 , @jasonmolenda 
wrote:

> Update patch to change the `AddressableBits::IsValid` to 
> `AddressableBits::HasValue` which is a clearer description.  The `IsValid` 
> methods are used across the lldb_private classes everywhere.  I tried 
> changing this to `operator bool` but I thought that was less clear in use.  I 
> compromised by changing it to `HasValue` - this is a POD class so "IsValid" 
> seemed a little questionable, but I didn't like operator bool.  Let's try 
> this out.

Yeah that's fine. I don't feel strongly about `operator bool()`. My main 
concern is to not create another class that can be in an "invalid state" and 
requires you to check whether or not it's valid before doing an operation. As 
you point out, that's not really the case here anyway, and using different 
terminology helps convey that.




Comment at: 
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:2314-2316
+if (addressable_bits.HasValue()) {
+  addressable_bits.SetProcessMasks(*this);
+}

I would inline the check for `HasValue` in `SetProcessMasks`. You already do 
some kind of sanity checking there, might as well make that sound and do it 
unconditionally. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158041

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

[Lldb-commits] [PATCH] D158024: [lldb][NFCI] Remove unused method overload of ValueObject::GetChildAtNamePath

[Alex Langford via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2f382bfb14e3: [lldb][NFCI] Remove unused method overload of 
ValueObject::GetChildAtNamePath (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158024

Files:
  lldb/include/lldb/Core/ValueObject.h
  lldb/source/Core/ValueObject.cpp


Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -440,23 +440,6 @@
   return root;
 }
 
-lldb::ValueObjectSP ValueObject::GetChildAtNamePath(
-llvm::ArrayRef> names,
-ConstString *name_of_error) {
-  if (names.size() == 0)
-return GetSP();
-  ValueObjectSP root(GetSP());
-  for (std::pair name : names) {
-root = root->GetChildMemberWithName(name.first, name.second);
-if (!root) {
-  if (name_of_error)
-*name_of_error = name.first;
-  return root;
-}
-  }
-  return root;
-}
-
 size_t ValueObject::GetIndexOfChildWithName(llvm::StringRef name) {
   bool omit_empty_base_classes = true;
   return GetCompilerType().GetIndexOfChildWithName(name,
Index: lldb/include/lldb/Core/ValueObject.h
===
--- lldb/include/lldb/Core/ValueObject.h
+++ lldb/include/lldb/Core/ValueObject.h
@@ -479,10 +479,6 @@
   // this will always create the children if necessary
   lldb::ValueObjectSP GetChildAtNamePath(llvm::ArrayRef 
names);
 
-  lldb::ValueObjectSP
-  GetChildAtNamePath(llvm::ArrayRef> names,
- ConstString *name_of_error = nullptr);
-
   virtual lldb::ValueObjectSP GetChildMemberWithName(llvm::StringRef name,
  bool can_create = true);
 


Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -440,23 +440,6 @@
   return root;
 }
 
-lldb::ValueObjectSP ValueObject::GetChildAtNamePath(
-llvm::ArrayRef> names,
-ConstString *name_of_error) {
-  if (names.size() == 0)
-return GetSP();
-  ValueObjectSP root(GetSP());
-  for (std::pair name : names) {
-root = root->GetChildMemberWithName(name.first, name.second);
-if (!root) {
-  if (name_of_error)
-*name_of_error = name.first;
-  return root;
-}
-  }
-  return root;
-}
-
 size_t ValueObject::GetIndexOfChildWithName(llvm::StringRef name) {
   bool omit_empty_base_classes = true;
   return GetCompilerType().GetIndexOfChildWithName(name,
Index: lldb/include/lldb/Core/ValueObject.h
===
--- lldb/include/lldb/Core/ValueObject.h
+++ lldb/include/lldb/Core/ValueObject.h
@@ -479,10 +479,6 @@
   // this will always create the children if necessary
   lldb::ValueObjectSP GetChildAtNamePath(llvm::ArrayRef names);
 
-  lldb::ValueObjectSP
-  GetChildAtNamePath(llvm::ArrayRef> names,
- ConstString *name_of_error = nullptr);
-
   virtual lldb::ValueObjectSP GetChildMemberWithName(llvm::StringRef name,
  bool can_create = true);
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 2f382bf - [lldb][NFCI] Remove unused method overload of ValueObject::GetChildAtNamePath

[Alex Langford via lldb-commits]

Author: Alex Langford
Date: 2023-08-16T14:23:58-07:00
New Revision: 2f382bfb14e38429e69885406ae8cedc9bb3028e

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

LOG: [lldb][NFCI] Remove unused method overload of 
ValueObject::GetChildAtNamePath

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

Added: 


Modified: 
lldb/include/lldb/Core/ValueObject.h
lldb/source/Core/ValueObject.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/ValueObject.h 
b/lldb/include/lldb/Core/ValueObject.h
index 2d8036ab078718..3af94f0a86e2fc 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -479,10 +479,6 @@ class ValueObject {
   // this will always create the children if necessary
   lldb::ValueObjectSP GetChildAtNamePath(llvm::ArrayRef 
names);
 
-  lldb::ValueObjectSP
-  GetChildAtNamePath(llvm::ArrayRef> names,
- ConstString *name_of_error = nullptr);
-
   virtual lldb::ValueObjectSP GetChildMemberWithName(llvm::StringRef name,
  bool can_create = true);
 

diff  --git a/lldb/source/Core/ValueObject.cpp 
b/lldb/source/Core/ValueObject.cpp
index d60a1d6f7a1055..37d32e807fdbc2 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -440,23 +440,6 @@ 
ValueObject::GetChildAtNamePath(llvm::ArrayRef names) {
   return root;
 }
 
-lldb::ValueObjectSP ValueObject::GetChildAtNamePath(
-llvm::ArrayRef> names,
-ConstString *name_of_error) {
-  if (names.size() == 0)
-return GetSP();
-  ValueObjectSP root(GetSP());
-  for (std::pair name : names) {
-root = root->GetChildMemberWithName(name.first, name.second);
-if (!root) {
-  if (name_of_error)
-*name_of_error = name.first;
-  return root;
-}
-  }
-  return root;
-}
-
 size_t ValueObject::GetIndexOfChildWithName(llvm::StringRef name) {
   bool omit_empty_base_classes = true;
   return GetCompilerType().GetIndexOfChildWithName(name,



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

[Lldb-commits] [PATCH] D157059: [lldb][PECOFF] Exclude alignment padding when reading section data

[Hiroshi Yamauchi via Phabricator via lldb-commits]

hjyamauchi added a comment.

In D157059#4572809 , @Michael137 
wrote:

> Hi, this is failing on swift-ci (runs on x86 bots) with the following error:
>
>   
> /Users/ec2-user/jenkins/workspace/oss-lldb-incremental-macos-cmake/build/Ninja-ReleaseAssert+stdlib-Release/lldb-macosx-x86_64/unittests/ObjectFile/PECOFF/./ObjectFilePECOFFTests
>  --gtest_filter=SectionSizeTest.NoAlignmentPadding
>   --
>   YAML:12:5: error: unknown key 'SizeOfRawData'
>   SizeOfRawData:   512
>   ^
>   
> /Users/ec2-user/jenkins/workspace/oss-lldb-incremental-macos-cmake/llvm-project/lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp:49:
>  Failure
>   Value of: llvm::detail::TakeExpected(ExpectedFile)
>   Expected: succeeded
> Actual: failed  (convertYAML() failed)
>   
>   
> /Users/ec2-user/jenkins/workspace/oss-lldb-incremental-macos-cmake/llvm-project/lldb/unittests/ObjectFile/PECOFF/TestSectionSize.cpp:49
>   Value of: llvm::detail::TakeExpected(ExpectedFile)
>   Expected: succeeded
> Actual: failed  (convertYAML() failed)
>
> https://ci.swift.org/view/LLDB/job/oss-lldb-incremental-macos-cmake/
>
> Could you take a look please?

Fix: https://github.com/apple/llvm-project/pull/7218


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157059

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

[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added inline comments.



Comment at: lldb/source/Utility/AddressableBits.cpp:62-64
+void AddressableBits::Clear() {
+  m_low_memory_addr_bits = m_high_memory_addr_bits = 0;
+}

JDevlieghere wrote:
> jasonmolenda wrote:
> > jasonmolenda wrote:
> > > JDevlieghere wrote:
> > > > Where is this called?
> > > Hm.  When I thought of this patch last night, I was sure I needed a Clear 
> > > method for some reason, but you're right I never ended up using it.
> > AHA I did use it, but it's in the previous patch.  It's in the base class 
> > method of ObjectFile.
> > 
> > ```
> >   virtual bool GetAddressableBits(lldb_private::AddressableBits 
> > _bits) {
> > address_bits.Clear();
> > return false;
> >   } 
> > ```
> > 
> Why is this returning a bool and not `AddressableBits`?
The `addressable_bits` argument is a `[out]` variable to indicate if it has a 
value.  You're right, now that there's a `HasValue` method, 
`ObjectFile::GetAddressableBits` can return an `AddressableBits` and the caller 
can call `HasValue`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158041

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

[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Utility/AddressableBits.cpp:62-64
+void AddressableBits::Clear() {
+  m_low_memory_addr_bits = m_high_memory_addr_bits = 0;
+}

jasonmolenda wrote:
> jasonmolenda wrote:
> > JDevlieghere wrote:
> > > Where is this called?
> > Hm.  When I thought of this patch last night, I was sure I needed a Clear 
> > method for some reason, but you're right I never ended up using it.
> AHA I did use it, but it's in the previous patch.  It's in the base class 
> method of ObjectFile.
> 
> ```
>   virtual bool GetAddressableBits(lldb_private::AddressableBits 
> _bits) {
> address_bits.Clear();
> return false;
>   } 
> ```
> 
Why is this returning a bool and not `AddressableBits`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158041

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

[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

[Jason Molenda via Phabricator via lldb-commits]

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

Update patch to change the `AddressableBits::IsValid` to 
`AddressableBits::HasValue` which is a clearer description.  The `IsValid` 
methods are used across the lldb_private classes everywhere.  I tried changing 
this to `operator bool` but I thought that was less clear in use.  I 
compromised by changing it to `HasValue` - this is a POD class so "IsValid" 
seemed a little questionable, but I didn't like operator bool.  Let's try this 
out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158041

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Utility/AddressableBits.h
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Utility/AddressableBits.cpp

Index: lldb/source/Utility/AddressableBits.cpp
===
--- lldb/source/Utility/AddressableBits.cpp
+++ lldb/source/Utility/AddressableBits.cpp
@@ -23,8 +23,14 @@
   m_high_memory_addr_bits = highmem_addressing_bits;
 }
 
-void AddressableBits::Clear() {
-  m_low_memory_addr_bits = m_high_memory_addr_bits = 0;
+void AddressableBits::SetLowmemAddressableBits(
+uint32_t lowmem_addressing_bits) {
+  m_low_memory_addr_bits = lowmem_addressing_bits;
+}
+
+void AddressableBits::SetHighmemAddressableBits(
+uint32_t highmem_addressing_bits) {
+  m_high_memory_addr_bits = highmem_addressing_bits;
 }
 
 void AddressableBits::SetProcessMasks(Process ) {
@@ -48,3 +54,11 @@
 process.SetHighmemDataAddressMask(hi_address_mask);
   }
 }
+
+bool AddressableBits::HasValue() const {
+  return m_low_memory_addr_bits != 0 || m_high_memory_addr_bits != 0;
+}
+
+void AddressableBits::Clear() {
+  m_low_memory_addr_bits = m_high_memory_addr_bits = 0;
+}
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
@@ -2124,6 +2124,7 @@
 QueueKind queue_kind = eQueueKindUnknown;
 uint64_t queue_serial_number = 0;
 ExpeditedRegisterMap expedited_register_map;
+AddressableBits addressable_bits;
 while (stop_packet.GetNameColonValue(key, value)) {
   if (key.compare("metype") == 0) {
 // exception type in big endian hex
@@ -2271,9 +2272,17 @@
   } else if (key.compare("addressing_bits") == 0) {
 uint64_t addressing_bits;
 if (!value.getAsInteger(0, addressing_bits)) {
-  addr_t address_mask = ~((1ULL << addressing_bits) - 1);
-  SetCodeAddressMask(address_mask);
-  SetDataAddressMask(address_mask);
+  addressable_bits.SetAddressableBits(addressing_bits);
+}
+  } else if (key.compare("low_mem_addressing_bits") == 0) {
+uint64_t addressing_bits;
+if (!value.getAsInteger(0, addressing_bits)) {
+  addressable_bits.SetLowmemAddressableBits(addressing_bits);
+}
+  } else if (key.compare("high_mem_addressing_bits") == 0) {
+uint64_t addressing_bits;
+if (!value.getAsInteger(0, addressing_bits)) {
+  addressable_bits.SetHighmemAddressableBits(addressing_bits);
 }
   } else if (key.size() == 2 && ::isxdigit(key[0]) && ::isxdigit(key[1])) {
 uint32_t reg = UINT32_MAX;
@@ -2302,6 +2311,10 @@
   }
 }
 
+if (addressable_bits.HasValue()) {
+  addressable_bits.SetProcessMasks(*this);
+}
+
 ThreadSP thread_sp = SetThreadStopInfo(
 tid, expedited_register_map, signo, thread_name, reason, description,
 exc_type, exc_data, thread_dispatch_qaddr, queue_vars_valid,
Index: lldb/include/lldb/Utility/AddressableBits.h
===
--- lldb/include/lldb/Utility/AddressableBits.h
+++ lldb/include/lldb/Utility/AddressableBits.h
@@ -29,8 +29,14 @@
   void SetAddressableBits(uint32_t lowmem_addressing_bits,
   uint32_t highmem_addressing_bits);
 
+  void SetLowmemAddressableBits(uint32_t lowmem_addressing_bits);
+
+  void SetHighmemAddressableBits(uint32_t highmem_addressing_bits);
+
   void SetProcessMasks(lldb_private::Process );
 
+  bool HasValue() const;
+
   void Clear();
 
 private:
Index: lldb/docs/lldb-gdb-remote.txt
===
--- lldb/docs/lldb-gdb-remote.txt
+++ lldb/docs/lldb-gdb-remote.txt
@@ -1661,6 +1661,18 @@
 //   start code that may be changing the
 //   page table setup, a dynamically set
 //   value may be needed.
+//  "low_mem_addressing_bits" unsigned optional, specifies how many bits in 
+//   addresses in low memory are significant 
+// 

[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added inline comments.



Comment at: lldb/source/Utility/AddressableBits.cpp:62-64
+void AddressableBits::Clear() {
+  m_low_memory_addr_bits = m_high_memory_addr_bits = 0;
+}

jasonmolenda wrote:
> JDevlieghere wrote:
> > Where is this called?
> Hm.  When I thought of this patch last night, I was sure I needed a Clear 
> method for some reason, but you're right I never ended up using it.
AHA I did use it, but it's in the previous patch.  It's in the base class 
method of ObjectFile.

```
  virtual bool GetAddressableBits(lldb_private::AddressableBits _bits) {
address_bits.Clear();
return false;
  } 
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158041

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

[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added inline comments.



Comment at: lldb/source/Utility/AddressableBits.cpp:58-60
+bool AddressableBits::IsValid() {
+  return m_low_memory_addr_bits != 0 || m_high_memory_addr_bits != 0;
+}

JDevlieghere wrote:
> Please implement `operator bool()` instead. 
ok.



Comment at: lldb/source/Utility/AddressableBits.cpp:62-64
+void AddressableBits::Clear() {
+  m_low_memory_addr_bits = m_high_memory_addr_bits = 0;
+}

JDevlieghere wrote:
> Where is this called?
Hm.  When I thought of this patch last night, I was sure I needed a Clear 
method for some reason, but you're right I never ended up using it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158041

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

[Lldb-commits] [PATCH] D158041: [lldb] [gdb-remote] Also handle high/low memory addressable bits setting in the stop info packet

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.



Comment at: lldb/source/Utility/AddressableBits.cpp:58-60
+bool AddressableBits::IsValid() {
+  return m_low_memory_addr_bits != 0 || m_high_memory_addr_bits != 0;
+}

Please implement `operator bool()` instead. 



Comment at: lldb/source/Utility/AddressableBits.cpp:62-64
+void AddressableBits::Clear() {
+  m_low_memory_addr_bits = m_high_memory_addr_bits = 0;
+}

Where is this called?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158041

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

[Lldb-commits] [lldb] d268ba3 - Test follow-up to 2e7aa2ee34eb53347396731dc8a3b2dbc6a3df45

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2023-08-16T12:19:07-07:00
New Revision: d268ba38083cd7c9c4f55cb0792d4a0db1d4768b

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

LOG: Test follow-up to 2e7aa2ee34eb53347396731dc8a3b2dbc6a3df45

The TestEvents.py test I added for ShadowListeners fails on Windows.
Since there's no reason to believe the ShadowListeners feature has
different behavior from the other event-based tests here, I copied
the skips & expected_flakey's from the other tests in that file to
this one.

Added: 


Modified: 
lldb/test/API/python_api/event/TestEvents.py

Removed: 




diff  --git a/lldb/test/API/python_api/event/TestEvents.py 
b/lldb/test/API/python_api/event/TestEvents.py
index a8a67a94a84345..9c933d440847c5 100644
--- a/lldb/test/API/python_api/event/TestEvents.py
+++ b/lldb/test/API/python_api/event/TestEvents.py
@@ -347,6 +347,9 @@ def wait_for_next_event(self, expected_state, test_shadow = 
False):
 
 return state, restart
 
+@expectedFlakeyLinux("llvm.org/pr23730")  # Flaky, fails ~1/100 cases
+@skipIfWindows  # This is flakey on Windows AND when it fails, it hangs: 
llvm.org/pr38373
+@skipIfNetBSD
 def test_shadow_listener(self):
 self.build()
 exe = self.getBuildArtifact("a.out")



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

[Lldb-commits] [PATCH] D158043: [lldb][NFCI] Module constructor should take ConstString by value

[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/D158043/new/

https://reviews.llvm.org/D158043

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

[Lldb-commits] [PATCH] D158043: [lldb][NFCI] Module constructor should take ConstString by value

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

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

LGTM! Good catch :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158043

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

[Lldb-commits] [PATCH] D158043: [lldb][NFCI] Module constructor should take ConstString by value

[Alex Langford via Phabricator via lldb-commits]

bulbazord updated this revision to Diff 550824.
bulbazord added a comment.

Remove `const` from parameter in DebugMapModule


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158043

Files:
  lldb/include/lldb/Core/Module.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -170,7 +170,7 @@
 public:
   DebugMapModule(const ModuleSP _module_sp, uint32_t cu_idx,
  const FileSpec _spec, const ArchSpec ,
- const ConstString *object_name, off_t object_offset,
+ ConstString object_name, off_t object_offset,
  const llvm::sys::TimePoint<> object_mod_time)
   : Module(file_spec, arch, object_name, object_offset, object_mod_time),
 m_exe_module_wp(exe_module_sp), m_cu_idx(cu_idx) {}
@@ -459,7 +459,7 @@
  .c_str());
   comp_unit_info->oso_sp->module_sp = std::make_shared(
   obj_file->GetModule(), GetCompUnitInfoIndex(comp_unit_info), 
oso_file,
-  oso_arch, oso_object ? _object : nullptr, 0,
+  oso_arch, oso_object, 0,
   oso_object ? comp_unit_info->oso_mod_time : 
llvm::sys::TimePoint<>());
 
   if (oso_object && !comp_unit_info->oso_sp->module_sp->GetObjectFile() &&
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -233,12 +233,12 @@
 }
 
 Module::Module(const FileSpec _spec, const ArchSpec ,
-   const ConstString *object_name, lldb::offset_t object_offset,
+   ConstString object_name, lldb::offset_t object_offset,
const llvm::sys::TimePoint<> _mod_time)
 : m_mod_time(FileSystem::Instance().GetModificationTime(file_spec)),
-  m_arch(arch), m_file(file_spec), m_object_offset(object_offset),
-  m_object_mod_time(object_mod_time), m_file_has_changed(false),
-  m_first_file_changed_log(false) {
+  m_arch(arch), m_file(file_spec), m_object_name(object_name),
+  m_object_offset(object_offset), m_object_mod_time(object_mod_time),
+  m_file_has_changed(false), m_first_file_changed_log(false) {
   // Scope for locker below...
   {
 std::lock_guard guard(
@@ -246,9 +246,6 @@
 GetModuleCollection().push_back(this);
   }
 
-  if (object_name)
-m_object_name = *object_name;
-
   Log *log(GetLog(LLDBLog::Object | LLDBLog::Modules));
   if (log != nullptr)
 LLDB_LOGF(log, "%p Module::Module((%s) '%s%s%s%s')",
Index: lldb/include/lldb/Core/Module.h
===
--- lldb/include/lldb/Core/Module.h
+++ lldb/include/lldb/Core/Module.h
@@ -124,8 +124,7 @@
   /// multiple architectures).
   Module(
   const FileSpec _spec, const ArchSpec ,
-  const ConstString *object_name = nullptr,
-  lldb::offset_t object_offset = 0,
+  ConstString object_name = ConstString(), lldb::offset_t object_offset = 
0,
   const llvm::sys::TimePoint<> _mod_time = 
llvm::sys::TimePoint<>());
 
   Module(const ModuleSpec _spec);


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -170,7 +170,7 @@
 public:
   DebugMapModule(const ModuleSP _module_sp, uint32_t cu_idx,
  const FileSpec _spec, const ArchSpec ,
- const ConstString *object_name, off_t object_offset,
+ ConstString object_name, off_t object_offset,
  const llvm::sys::TimePoint<> object_mod_time)
   : Module(file_spec, arch, object_name, object_offset, object_mod_time),
 m_exe_module_wp(exe_module_sp), m_cu_idx(cu_idx) {}
@@ -459,7 +459,7 @@
  .c_str());
   comp_unit_info->oso_sp->module_sp = std::make_shared(
   obj_file->GetModule(), GetCompUnitInfoIndex(comp_unit_info), oso_file,
-  oso_arch, oso_object ? _object : nullptr, 0,
+  oso_arch, oso_object, 0,
   oso_object ? comp_unit_info->oso_mod_time : llvm::sys::TimePoint<>());
 
   if (oso_object && !comp_unit_info->oso_sp->module_sp->GetObjectFile() &&
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -233,12 +233,12 @@
 }
 
 Module::Module(const FileSpec _spec, const ArchSpec ,
-   const ConstString *object_name, 

[Lldb-commits] [PATCH] D158043: [lldb][NFCI] Module constructor should take ConstString by value

[Alex Langford via Phabricator via lldb-commits]

bulbazord updated this revision to Diff 550822.
bulbazord added a comment.

Give the parameter a default value


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158043

Files:
  lldb/include/lldb/Core/Module.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -170,7 +170,7 @@
 public:
   DebugMapModule(const ModuleSP _module_sp, uint32_t cu_idx,
  const FileSpec _spec, const ArchSpec ,
- const ConstString *object_name, off_t object_offset,
+ const ConstString object_name, off_t object_offset,
  const llvm::sys::TimePoint<> object_mod_time)
   : Module(file_spec, arch, object_name, object_offset, object_mod_time),
 m_exe_module_wp(exe_module_sp), m_cu_idx(cu_idx) {}
@@ -459,7 +459,7 @@
  .c_str());
   comp_unit_info->oso_sp->module_sp = std::make_shared(
   obj_file->GetModule(), GetCompUnitInfoIndex(comp_unit_info), 
oso_file,
-  oso_arch, oso_object ? _object : nullptr, 0,
+  oso_arch, oso_object, 0,
   oso_object ? comp_unit_info->oso_mod_time : 
llvm::sys::TimePoint<>());
 
   if (oso_object && !comp_unit_info->oso_sp->module_sp->GetObjectFile() &&
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -233,12 +233,12 @@
 }
 
 Module::Module(const FileSpec _spec, const ArchSpec ,
-   const ConstString *object_name, lldb::offset_t object_offset,
+   ConstString object_name, lldb::offset_t object_offset,
const llvm::sys::TimePoint<> _mod_time)
 : m_mod_time(FileSystem::Instance().GetModificationTime(file_spec)),
-  m_arch(arch), m_file(file_spec), m_object_offset(object_offset),
-  m_object_mod_time(object_mod_time), m_file_has_changed(false),
-  m_first_file_changed_log(false) {
+  m_arch(arch), m_file(file_spec), m_object_name(object_name),
+  m_object_offset(object_offset), m_object_mod_time(object_mod_time),
+  m_file_has_changed(false), m_first_file_changed_log(false) {
   // Scope for locker below...
   {
 std::lock_guard guard(
@@ -246,9 +246,6 @@
 GetModuleCollection().push_back(this);
   }
 
-  if (object_name)
-m_object_name = *object_name;
-
   Log *log(GetLog(LLDBLog::Object | LLDBLog::Modules));
   if (log != nullptr)
 LLDB_LOGF(log, "%p Module::Module((%s) '%s%s%s%s')",
Index: lldb/include/lldb/Core/Module.h
===
--- lldb/include/lldb/Core/Module.h
+++ lldb/include/lldb/Core/Module.h
@@ -124,8 +124,7 @@
   /// multiple architectures).
   Module(
   const FileSpec _spec, const ArchSpec ,
-  const ConstString *object_name = nullptr,
-  lldb::offset_t object_offset = 0,
+  ConstString object_name = ConstString(), lldb::offset_t object_offset = 
0,
   const llvm::sys::TimePoint<> _mod_time = 
llvm::sys::TimePoint<>());
 
   Module(const ModuleSpec _spec);


Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -170,7 +170,7 @@
 public:
   DebugMapModule(const ModuleSP _module_sp, uint32_t cu_idx,
  const FileSpec _spec, const ArchSpec ,
- const ConstString *object_name, off_t object_offset,
+ const ConstString object_name, off_t object_offset,
  const llvm::sys::TimePoint<> object_mod_time)
   : Module(file_spec, arch, object_name, object_offset, object_mod_time),
 m_exe_module_wp(exe_module_sp), m_cu_idx(cu_idx) {}
@@ -459,7 +459,7 @@
  .c_str());
   comp_unit_info->oso_sp->module_sp = std::make_shared(
   obj_file->GetModule(), GetCompUnitInfoIndex(comp_unit_info), oso_file,
-  oso_arch, oso_object ? _object : nullptr, 0,
+  oso_arch, oso_object, 0,
   oso_object ? comp_unit_info->oso_mod_time : llvm::sys::TimePoint<>());
 
   if (oso_object && !comp_unit_info->oso_sp->module_sp->GetObjectFile() &&
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -233,12 +233,12 @@
 }
 
 Module::Module(const FileSpec _spec, const ArchSpec ,
-   const ConstString *object_name, 

[Lldb-commits] [PATCH] D158043: [lldb][NFCI] Module constructor should take ConstString by value

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/include/lldb/Core/Module.h:126
   Module(
-  const FileSpec _spec, const ArchSpec ,
-  const ConstString *object_name = nullptr,
+  const FileSpec _spec, const ArchSpec , ConstString object_name,
   lldb::offset_t object_offset = 0,

fdeazeve wrote:
> Out of curiosity, what was the rationale for removing the default parameter 
> here?
I didn't really even think about it, I probably should have just done 
`ConstString object_name = ConstString()` or something.



Comment at: 
lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp:806
+   target.GetArchitecture(),
+   ConstString());
   }

mib wrote:
> Can this be `{}` ?
Yes, although I'll probably make the default value of the parameter be an empty 
ConstString instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158043

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

[Lldb-commits] [PATCH] D158043: [lldb][NFCI] Module constructor should take ConstString by value

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

fdeazeve added inline comments.



Comment at: lldb/include/lldb/Core/Module.h:126
   Module(
-  const FileSpec _spec, const ArchSpec ,
-  const ConstString *object_name = nullptr,
+  const FileSpec _spec, const ArchSpec , ConstString object_name,
   lldb::offset_t object_offset = 0,

Out of curiosity, what was the rationale for removing the default parameter 
here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158043

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

[Lldb-commits] [PATCH] D158043: [lldb][NFCI] Module constructor should take ConstString by value

[Med Ismail Bennani via Phabricator via lldb-commits]

mib added inline comments.



Comment at: 
lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp:806
+   target.GetArchitecture(),
+   ConstString());
   }

Can this be `{}` ?



Comment at: lldb/source/Target/Process.cpp:2389
   }
-  ModuleSP module_sp(new Module(file_spec, ArchSpec()));
+  ModuleSP module_sp(new Module(file_spec, ArchSpec(), ConstString()));
   if (module_sp) {

Should the module constructor have this as a default value for the 
`ConstString` parameter ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158043

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

[Lldb-commits] [PATCH] D158043: [lldb][NFCI] Module constructor should take ConstString by value

[Alex Langford via Phabricator via lldb-commits]

bulbazord updated this revision to Diff 550812.
bulbazord added a comment.

Addressing @aprantl's feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158043

Files:
  lldb/include/lldb/Core/Module.h
  lldb/source/Core/Module.cpp
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Target/Process.cpp
  lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp

Index: lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
===
--- lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
+++ lldb/unittests/SymbolFile/DWARF/SymbolFileDWARFTests.cpp
@@ -57,7 +57,7 @@
   // Test that when we have Dwarf debug info, SymbolFileDWARF is used.
   FileSpec fspec(m_dwarf_test_exe);
   ArchSpec aspec("i686-pc-windows");
-  lldb::ModuleSP module = std::make_shared(fspec, aspec);
+  lldb::ModuleSP module = std::make_shared(fspec, aspec, ConstString());
 
   SymbolFile *symfile = module->GetSymbolFile();
   ASSERT_NE(nullptr, symfile);
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2386,7 +2386,7 @@
   "Process::ReadModuleFromMemory reading %s binary from memory",
   file_spec.GetPath().c_str());
   }
-  ModuleSP module_sp(new Module(file_spec, ArchSpec()));
+  ModuleSP module_sp(new Module(file_spec, ArchSpec(), ConstString()));
   if (module_sp) {
 Status error;
 ObjectFile *objfile = module_sp->GetMemoryObjectFile(
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -170,7 +170,7 @@
 public:
   DebugMapModule(const ModuleSP _module_sp, uint32_t cu_idx,
  const FileSpec _spec, const ArchSpec ,
- const ConstString *object_name, off_t object_offset,
+ const ConstString object_name, off_t object_offset,
  const llvm::sys::TimePoint<> object_mod_time)
   : Module(file_spec, arch, object_name, object_offset, object_mod_time),
 m_exe_module_wp(exe_module_sp), m_cu_idx(cu_idx) {}
@@ -459,7 +459,7 @@
  .c_str());
   comp_unit_info->oso_sp->module_sp = std::make_shared(
   obj_file->GetModule(), GetCompUnitInfoIndex(comp_unit_info), oso_file,
-  oso_arch, oso_object ? _object : nullptr, 0,
+  oso_arch, oso_object, 0,
   oso_object ? comp_unit_info->oso_mod_time : llvm::sys::TimePoint<>());
 
   if (oso_object && !comp_unit_info->oso_sp->module_sp->GetObjectFile() &&
Index: lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
===
--- lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
+++ lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
@@ -802,7 +802,8 @@
  kernel_search_error, true)) {
   if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
 m_module_sp = std::make_shared(module_spec.GetFileSpec(),
-   target.GetArchitecture());
+   target.GetArchitecture(),
+   ConstString());
   }
 }
   }
Index: lldb/source/Core/Module.cpp
===
--- lldb/source/Core/Module.cpp
+++ lldb/source/Core/Module.cpp
@@ -233,12 +233,12 @@
 }
 
 Module::Module(const FileSpec _spec, const ArchSpec ,
-   const ConstString *object_name, lldb::offset_t object_offset,
+   ConstString object_name, lldb::offset_t object_offset,
const llvm::sys::TimePoint<> _mod_time)
 : m_mod_time(FileSystem::Instance().GetModificationTime(file_spec)),
-  m_arch(arch), m_file(file_spec), m_object_offset(object_offset),
-  m_object_mod_time(object_mod_time), m_file_has_changed(false),
-  m_first_file_changed_log(false) {
+  m_arch(arch), m_file(file_spec), m_object_name(object_name),
+  m_object_offset(object_offset), m_object_mod_time(object_mod_time),
+  m_file_has_changed(false), m_first_file_changed_log(false) {
   // Scope for locker below...
   {
 std::lock_guard guard(
@@ -246,9 +246,6 @@
 GetModuleCollection().push_back(this);
   }
 
-  if (object_name)
-m_object_name = *object_name;
-
   Log *log(GetLog(LLDBLog::Object | LLDBLog::Modules));
   if (log != nullptr)
 

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

[Jim Ingham via Phabricator via lldb-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Revision".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2e7aa2ee34eb: Replace the singleton 
ShadowListener with a primary and N secondary Listeners (authored 
by jingham).

Changed prior to commit:
  https://reviews.llvm.org/D157556?vs=550495=550806#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157556

Files:
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Utility/Broadcaster.h
  lldb/include/lldb/Utility/Event.h
  lldb/source/Target/Process.cpp
  lldb/source/Utility/Broadcaster.cpp
  lldb/source/Utility/Event.cpp
  lldb/source/Utility/Listener.cpp
  lldb/test/API/api/listeners/TestListener.py
  
lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py
  lldb/test/API/python_api/event/TestEvents.py

Index: lldb/test/API/python_api/event/TestEvents.py
===
--- lldb/test/API/python_api/event/TestEvents.py
+++ lldb/test/API/python_api/event/TestEvents.py
@@ -313,3 +313,121 @@
 self.assertEqual(
 self.state, "stopped", "Both expected state changed events received"
 )
+
+def wait_for_next_event(self, expected_state, test_shadow = False):
+"""Wait for an event from self.primary & self.shadow listener.
+   If test_shadow is true, we also check that the shadow listener only 
+   receives events AFTER the primary listener does."""
+# Waiting on the shadow listener shouldn't have events yet because
+# we haven't fetched them for the primary listener yet:
+event = lldb.SBEvent()
+
+if test_shadow:
+success = self.shadow_listener.WaitForEvent(1, event)
+self.assertFalse(success, "Shadow listener doesn't pull events")
+
+# But there should be an event for the primary listener:
+success = self.primary_listener.WaitForEvent(5, event)
+self.assertTrue(success, "Primary listener got the event")
+
+state = lldb.SBProcess.GetStateFromEvent(event)
+restart = False
+if state == lldb.eStateStopped:
+restart = lldb.SBProcess.GetRestartedFromEvent(event)
+
+if expected_state != None:
+self.assertEqual(state, expected_state, "Primary thread got the correct event")
+
+# And after pulling that one there should be an equivalent event for the shadow
+# listener:
+success = self.shadow_listener.WaitForEvent(5, event)
+self.assertTrue(success, "Shadow listener got event too")
+self.assertEqual(state, lldb.SBProcess.GetStateFromEvent(event), "It was the same event")
+self.assertEqual(restart, lldb.SBProcess.GetRestartedFromEvent(event), "It was the same restarted")
+
+return state, restart
+
+def test_shadow_listener(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+
+# Create a target by the debugger.
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target, VALID_TARGET)
+
+# Now create a breakpoint on main.c by name 'c'.
+bkpt1 = target.BreakpointCreateByName("c", "a.out")
+self.trace("breakpoint:", bkpt1)
+self.assertTrue(bkpt1.GetNumLocations() == 1, VALID_BREAKPOINT)
+
+self.primary_listener = lldb.SBListener("my listener")
+self.shadow_listener = lldb.SBListener("shadow listener")
+
+self.cur_thread = None
+
+error = lldb.SBError()
+launch_info = target.GetLaunchInfo()
+launch_info.SetListener(self.primary_listener)
+launch_info.SetShadowListener(self.shadow_listener)
+
+self.runCmd("settings set target.process.extra-startup-command QSetLogging:bitmask=LOG_PROCESS|LOG_EXCEPTIONS|LOG_RNB_PACKETS|LOG_STEP;")
+self.dbg.SetAsync(True)
+
+self.process = target.Launch(launch_info, error)
+self.assertSuccess(error, "Process launched successfully")
+
+# Keep fetching events from the primary to trigger the do on removal and
+# then from the shadow listener, and make sure they match:
+
+# Events in the launch sequence might be platform dependent, so don't
+# expect any particular event till we get the stopped:
+state = lldb.eStateInvalid
+while state != lldb.eStateStopped:
+state, restart = self.wait_for_next_event(None, False)
+
+# Okay, we're now at a good stop, so try a next:
+self.cur_thread = self.process.threads[0]
+
+# Make sure we're at our expected breakpoint:
+self.assertTrue(self.cur_thread.IsValid(), "Got a zeroth thread")
+self.assertEqual(self.cur_thread.stop_reason, lldb.eStopReasonBreakpoint)
+self.assertEqual(self.cur_thread.GetStopReasonDataCount(), 

[Lldb-commits] [lldb] 2e7aa2e - Replace the singleton "ShadowListener" with a primary and N secondary Listeners

[Jim Ingham via lldb-commits]

Author: Jim Ingham
Date: 2023-08-16T10:35:32-07:00
New Revision: 2e7aa2ee34eb53347396731dc8a3b2dbc6a3df45

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

LOG: Replace the singleton "ShadowListener" with a primary and N secondary 
Listeners

Before the addition of the process "Shadow Listener" you could only have one
Listener observing the Process Broadcaster.  That was necessary because 
fetching the
Process event is what switches the public process state, and for the execution
control logic to be manageable you needed to keep other listeners from causing
this to happen before the main process control engine was ready.

Ismail added the notion of a "ShadowListener" - which allowed you ONE
extra process listener.  This patch inverts that setup by designating the
first listener as primary - and giving it priority in fetching events.

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

Added: 


Modified: 
lldb/include/lldb/Target/Process.h
lldb/include/lldb/Utility/Broadcaster.h
lldb/include/lldb/Utility/Event.h
lldb/source/Target/Process.cpp
lldb/source/Utility/Broadcaster.cpp
lldb/source/Utility/Event.cpp
lldb/source/Utility/Listener.cpp
lldb/test/API/api/listeners/TestListener.py

lldb/test/API/functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py
lldb/test/API/python_api/event/TestEvents.py

Removed: 




diff  --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index 6ea6f82548aaf8..2186eea7ef5054 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -352,6 +352,14 @@ class Process : public 
std::enable_shared_from_this,
 eBroadcastBitProfileData = (1 << 4),
 eBroadcastBitStructuredData = (1 << 5),
   };
+  // This is all the event bits the public process broadcaster broadcasts.
+  // The process shadow listener signs up for all these bits...
+  static constexpr int g_all_event_bits = eBroadcastBitStateChanged 
+| eBroadcastBitInterrupt
+| eBroadcastBitSTDOUT 
+| eBroadcastBitSTDERR
+| eBroadcastBitProfileData 
+| eBroadcastBitStructuredData;
 
   enum {
 eBroadcastInternalStateControlStop = (1 << 0),
@@ -382,11 +390,7 @@ class Process : public 
std::enable_shared_from_this,
   ConstString () const override {
 return GetStaticBroadcasterClass();
   }
-
-  void SetShadowListener(lldb::ListenerSP listener_sp) override {
-Broadcaster::SetShadowListener(listener_sp);
-  }
-
+  
 /// A notification structure that can be used by clients to listen
 /// for changes in a process's lifetime.
 ///
@@ -610,6 +614,15 @@ class Process : public 
std::enable_shared_from_this,
 return error;
   }
 
+  /// The "ShadowListener" for a process is just an ordinary Listener that 
+  /// listens for all the Process event bits.  It's convenient because you can
+  /// specify it in the LaunchInfo or AttachInfo, so it will get events from
+  /// the very start of the process.
+  void SetShadowListener(lldb::ListenerSP shadow_listener_sp) {
+if (shadow_listener_sp)
+  AddListener(shadow_listener_sp, g_all_event_bits);
+  }
+
   // FUTURE WORK: GetLoadImageUtilityFunction are the first use we've
   // had of having other plugins cache data in the Process.  This is handy for
   // long-living plugins - like the Platform - which manage interactions whose
@@ -2979,8 +2992,6 @@ void PruneThreadPlans();
   std::vector m_notifications; ///< The list of notifications
   ///that this process can deliver.
   std::vector m_image_tokens;
-  lldb::ListenerSP m_listener_sp; ///< Shared pointer to the listener used for
-  ///public events.  Can not be empty.
   BreakpointSiteList m_breakpoint_site_list; ///< This is the list of 
breakpoint
  ///locations we intend to insert 
in
  ///the target.

diff  --git a/lldb/include/lldb/Utility/Broadcaster.h 
b/lldb/include/lldb/Utility/Broadcaster.h
index f9887566a1609d..8444c38f6ecc65 100644
--- a/lldb/include/lldb/Utility/Broadcaster.h
+++ b/lldb/include/lldb/Utility/Broadcaster.h
@@ -312,8 +312,12 @@ class Broadcaster {
 
   lldb::BroadcasterManagerSP GetManager();
 
-  virtual void SetShadowListener(lldb::ListenerSP listener_sp) {
-m_broadcaster_sp->m_shadow_listener = listener_sp;
+  void SetPrimaryListener(lldb::ListenerSP listener_sp) {
+m_broadcaster_sp->SetPrimaryListener(listener_sp);
+  }
+
+  lldb::ListenerSP GetPrimaryListener() {
+  

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

[Alex Langford via Phabricator via lldb-commits]

bulbazord requested changes to this revision.
bulbazord added a comment.
This revision now requires changes to proceed.

Thanks for all the work to clean up some of those interfaces! I know that was 
not straightforward. :)

I left a few more comments about minor things I noticed while re-reading this. 
I'm going to request changes to put this back into your queue.




Comment at: lldb/include/lldb/Utility/Event.h:233
+m_pending_listeners.push_back(pending_listener_sp);
+  };
+

nit: trailing semicolon



Comment at: lldb/source/Target/Process.cpp:620-621
 
-  if (m_listener_sp->GetEventForBroadcaster(this, event_sp,
+  if (GetPrimaryListener()->GetEventForBroadcaster(this, event_sp,
 std::chrono::seconds(0)) &&
   event_sp)

Do you need to check for the validity of the primary listener before using it? 
It can point to nullptr from what I can tell



Comment at: lldb/source/Utility/Broadcaster.cpp:59
+max_count++;
+  listeners.reserve(max_count);
 

I wonder if we should be reserving at all considering we're using an 
`llvm::SmallVector<$type, 4>`.  The point of a SmallVector is that on average 
it should be small and only sometimes should we be exceeding that hardcoded 
size. This means that if there are 8 listeners but only 2 are relevant, we're 
taking a SmallVector of size 4 and allocating space for 8 listeners (always 
triggering a memory allocation) even if it will only ever have 2 listeners in 
it.

I know that's the existing behavior and you're just preserving it, but 
something to think about for the future.



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

This check is a bit redundant since `HasListeners` performs the exact same check



Comment at: lldb/source/Utility/Broadcaster.cpp:198
+
   std::lock_guard guard(m_listeners_mutex);
+  for (auto it = m_listeners.begin(); it != m_listeners.end();) {




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] D158010: [lldb] Allow synthetic providers in C++ and fix linking problems

[Lily Orth-Smith via Phabricator via lldb-commits]

electriclilies updated this revision to Diff 550795.
electriclilies added a comment.

Fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158010

Files:
  lldb/include/lldb/DataFormatters/TypeSynthetic.h
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/DataFormatters/TypeSynthetic.cpp

Index: lldb/source/DataFormatters/TypeSynthetic.cpp
===
--- lldb/source/DataFormatters/TypeSynthetic.cpp
+++ lldb/source/DataFormatters/TypeSynthetic.cpp
@@ -84,6 +84,27 @@
   return std::string(sstr.GetString());
 }
 
+SyntheticChildren::SyntheticChildren(const Flags ) : m_flags(flags) {}
+
+SyntheticChildren::~SyntheticChildren() = default;
+
+CXXSyntheticChildren::CXXSyntheticChildren(
+const SyntheticChildren::Flags , const char *description,
+CreateFrontEndCallback callback)
+: SyntheticChildren(flags), m_create_callback(std::move(callback)),
+  m_description(description ? description : "") {}
+
+CXXSyntheticChildren::~CXXSyntheticChildren() = default;
+
+bool SyntheticChildren::IsScripted() { return false; }
+
+std::string SyntheticChildren::GetDescription() { return ""; }
+
+SyntheticChildrenFrontEnd::AutoPointer
+SyntheticChildren::GetFrontEnd(ValueObject ) {
+  return nullptr;
+}
+
 std::string CXXSyntheticChildren::GetDescription() {
   StreamString sstr;
   sstr.Printf("%s%s%s %s", Cascades() ? "" : " (not cascading)",
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -218,10 +218,8 @@
 SetValueFormat(DataVisualization::GetFormat(*this, eNoDynamicValues));
 SetSummaryFormat(
 DataVisualization::GetSummaryFormat(*this, GetDynamicValueType()));
-#if LLDB_ENABLE_PYTHON
 SetSyntheticChildren(
 DataVisualization::GetSyntheticChildren(*this, GetDynamicValueType()));
-#endif
   }
 
   return any_change;
@@ -1170,7 +1168,7 @@
 Stream , ValueObjectRepresentationStyle val_obj_display,
 Format custom_format, PrintableRepresentationSpecialCases special,
 bool do_dump_error) {
-
+
   // If the ValueObject has an error, we might end up dumping the type, which
   // is useful, but if we don't even have a type, then don't examine the object
   // further as that's not meaningful, only the error is.
@@ -2785,15 +2783,15 @@
 
 ValueObjectSP ValueObject::Cast(const CompilerType _type) {
   // Only allow casts if the original type is equal or larger than the cast
-  // type.  We don't know how to fetch more data for all the ConstResult types, 
+  // type.  We don't know how to fetch more data for all the ConstResult types,
   // so we can't guarantee this will work:
   Status error;
   CompilerType my_type = GetCompilerType();
 
-  ExecutionContextScope *exe_scope 
+  ExecutionContextScope *exe_scope
   = ExecutionContext(GetExecutionContextRef())
   .GetBestExecutionContextScope();
-  if (compiler_type.GetByteSize(exe_scope) 
+  if (compiler_type.GetByteSize(exe_scope)
   <= GetCompilerType().GetByteSize(exe_scope)) {
 return DoCast(compiler_type);
   }
Index: lldb/source/Commands/CommandObjectType.cpp
===
--- lldb/source/Commands/CommandObjectType.cpp
+++ lldb/source/Commands/CommandObjectType.cpp
@@ -2171,8 +2171,6 @@
"Show a list of current filters.") {}
 };
 
-#if LLDB_ENABLE_PYTHON
-
 // CommandObjectTypeSynthList
 
 class CommandObjectTypeSynthList
@@ -2184,8 +2182,6 @@
 "Show a list of current synthetic providers.") {}
 };
 
-#endif
-
 // CommandObjectTypeFilterDelete
 
 class CommandObjectTypeFilterDelete : public CommandObjectTypeFormatterDelete {
@@ -2197,8 +2193,6 @@
   ~CommandObjectTypeFilterDelete() override = default;
 };
 
-#if LLDB_ENABLE_PYTHON
-
 // CommandObjectTypeSynthDelete
 
 class CommandObjectTypeSynthDelete : public CommandObjectTypeFormatterDelete {
@@ -2210,7 +2204,6 @@
   ~CommandObjectTypeSynthDelete() override = default;
 };
 
-#endif
 
 // CommandObjectTypeFilterClear
 
@@ -,7 +2215,6 @@
 "Delete all existing filter.") {}
 };
 
-#if LLDB_ENABLE_PYTHON
 // CommandObjectTypeSynthClear
 
 class CommandObjectTypeSynthClear : public CommandObjectTypeFormatterClear {
@@ -2393,7 +2385,6 @@
   return true;
 }
 
-#endif
 #define LLDB_OPTIONS_type_filter_add
 #include "CommandOptions.inc"
 
@@ -2941,8 +2932,6 @@
   ~CommandObjectTypeFormat() override = default;
 };
 
-#if LLDB_ENABLE_PYTHON
-
 class CommandObjectTypeSynth : public CommandObjectMultiword {
 public:
   CommandObjectTypeSynth(CommandInterpreter )
@@ -2970,8 +2959,6 @@
   ~CommandObjectTypeSynth() override = default;
 };
 
-#endif
-
 class CommandObjectTypeFilter : public 

[Lldb-commits] [PATCH] D158085: [LLDB][Docs] Update cross compilation docs and examples

[Alex Langford via Phabricator via lldb-commits]

bulbazord accepted this revision.
bulbazord added a comment.

This is in line with my understanding of cross-compiling LLDB. It could be 
useful to also document some other interesting cross-compilation options like 
`CMAKE_TOOLCHAIN_FILE` or mention the use of CMake caches, but if you're 
unfamiliar or aren't comfortable listing those that's ok too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158085

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

[Lldb-commits] [PATCH] D157851: [lldb/crashlog] Add support for Last Exception Backtrace

[Jonas Devlieghere via Phabricator via lldb-commits]

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

LGTM with Alex' comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157851

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

[Lldb-commits] [PATCH] D158023: [lldb] Simplify the LLDB website structure

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 550791.
JDevlieghere added a comment.

- Rename "Extending LLDB" to "Scripting LLDB".
- Move "DWARF Extensions" to the developer section.


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

https://reviews.llvm.org/D158023

Files:
  lldb/docs/.htaccess
  lldb/docs/index.rst
  lldb/docs/resources/extensions.rst
  lldb/docs/resources/fuzzing.rst
  lldb/docs/status/features.rst
  lldb/docs/status/goals.rst
  lldb/docs/status/releases.rst
  lldb/docs/status/status.rst
  lldb/docs/use/extensions.rst

Index: lldb/docs/status/status.rst
===
--- lldb/docs/status/status.rst
+++ lldb/docs/status/status.rst
@@ -1,68 +0,0 @@
-Status
-==
-
-FreeBSD

-
-LLDB on FreeBSD lags behind the Linux implementation but is improving rapidly.
-For more details, see the Features by OS section below.
-
-Linux
--
-
-LLDB is improving on Linux. Linux is nearing feature completeness with Darwin
-to debug x86_64, i386, ARM, AArch64, IBM POWER (ppc64), and IBM Z (s390x)
-programs. For more details, see the Features by OS section below.
-
-macOS
--
-
-LLDB is the system debugger on macOS, iOS, tvOS, and watchOS and
-can be used for C, C++, Objective-C and Swift development for x86_64,
-i386, ARM, and AArch64 debugging. The entire public API is exposed
-through a macOS framework which is used by Xcode and the `lldb`
-command line tool. It can also be imported from Python. The entire public API is
-exposed through script bridging which allows LLDB to use an embedded Python
-script interpreter, as well as having a Python module named "lldb" which can be
-used from Python on the command line. This allows debug sessions to be
-scripted. It also allows powerful debugging actions to be created and attached
-to a variety of debugging workflows.
-
-NetBSD
---
-
-LLDB is improving on NetBSD and reaching feature completeness with Linux.
-
-Windows

-
-LLDB on Windows is still under development, but already useful for i386
-programs (x86_64 untested) built with DWARF debug information, including
-postmortem analysis of minidumps. For more details, see the Features by OS
-section below.
-
-Features Matrix

-+---++-+---++--+
-| Feature   | FreeBSD| Linux   | macOS | NetBSD | Windows  |
-+===++=+===++==+
-| Backtracing   | YES| YES | YES   | YES| YES  |
-+---++-+---++--+
-| Breakpoints   | YES| YES | YES   | YES| YES  |
-+---++-+---++--+
-| C++11:| YES| YES | YES   | YES| Unknown  |
-+---++-+---++--+
-| Commandline tool  | YES| YES | YES   | YES| YES  |
-+---++-+---++--+
-| Core file debugging   | YES (ELF)  | YES (ELF)   | YES (MachO)   | YES (ELF)  | YES (Minidump)   |
-+---++-+---++--+
-| Remote debugging  | YES (lldb-server)  | YES (lldb-server)   | YES (debugserver) | YES (lldb-server)  | NO   |
-+---++-+---++--+
-| Disassembly   | YES| YES | YES   | YES| YES  |
-+---++-+---++--+
-| Expression evaluation | YES (known issues) | YES (known issues)  | YES   | YES (known issues) | YES (known issues)   |
-+---++-+---++--+
-| JIT debugging | Unknown| Symbolic debugging only | Untested  | Work In 

[Lldb-commits] [PATCH] D158022: [lldb] Print an actionable error message when sphinx_automodapi is not installed

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5afa519c1ae9: [lldb] Print better error message when 
sphinx_automodapi is not installed (authored by JDevlieghere).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158022

Files:
  lldb/docs/conf.py


Index: lldb/docs/conf.py
===
--- lldb/docs/conf.py
+++ lldb/docs/conf.py
@@ -49,6 +49,12 @@
 # Unless we only generate the basic manpage we need the plugin for generating
 # the Python API documentation.
 if not building_man_page:
+try:
+import sphinx_automodapi.automodapi
+except ModuleNotFoundError:
+print(
+f"install sphinx_automodapi with {sys.executable} -m pip install 
sphinx_automodapi"
+)
 extensions.append("sphinx_automodapi.automodapi")
 
 # Add any paths that contain templates here, relative to this directory.


Index: lldb/docs/conf.py
===
--- lldb/docs/conf.py
+++ lldb/docs/conf.py
@@ -49,6 +49,12 @@
 # Unless we only generate the basic manpage we need the plugin for generating
 # the Python API documentation.
 if not building_man_page:
+try:
+import sphinx_automodapi.automodapi
+except ModuleNotFoundError:
+print(
+f"install sphinx_automodapi with {sys.executable} -m pip install sphinx_automodapi"
+)
 extensions.append("sphinx_automodapi.automodapi")
 
 # Add any paths that contain templates here, relative to this directory.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 5afa519 - [lldb] Print better error message when sphinx_automodapi is not installed

[Jonas Devlieghere via lldb-commits]

Author: Jonas Devlieghere
Date: 2023-08-16T09:14:42-07:00
New Revision: 5afa519c1ae9e17c3ac556b7ed42ab96b487ef0f

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

LOG: [lldb] Print better error message when sphinx_automodapi is not installed

Print an error message with instructions on how to install
sphinx_automodapi.

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

Added: 


Modified: 
lldb/docs/conf.py

Removed: 




diff  --git a/lldb/docs/conf.py b/lldb/docs/conf.py
index 7c4c945f25babe..d326a253b0a012 100644
--- a/lldb/docs/conf.py
+++ b/lldb/docs/conf.py
@@ -49,6 +49,12 @@
 # Unless we only generate the basic manpage we need the plugin for generating
 # the Python API documentation.
 if not building_man_page:
+try:
+import sphinx_automodapi.automodapi
+except ModuleNotFoundError:
+print(
+f"install sphinx_automodapi with {sys.executable} -m pip install 
sphinx_automodapi"
+)
 extensions.append("sphinx_automodapi.automodapi")
 
 # Add any paths that contain templates here, relative to this directory.



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

[Lldb-commits] [PATCH] D158043: [lldb][NFCI] Module constructor should take ConstString by value

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.



Comment at: lldb/include/lldb/Core/Module.h:127
   const FileSpec _spec, const ArchSpec ,
-  const ConstString *object_name = nullptr,
-  lldb::offset_t object_offset = 0,
+  const ConstString object_name, lldb::offset_t object_offset = 0,
   const llvm::sys::TimePoint<> _mod_time = 
llvm::sys::TimePoint<>());

If we're passing it by value, why the const qualifier?



Comment at: lldb/source/Core/Module.cpp:249
 
-  if (object_name)
-m_object_name = *object_name;
+  m_object_name = object_name;
 

why not `m_object_name(object_name)` above?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158043

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

[Lldb-commits] [PATCH] D157497: feat: Migrate isArch16Bit

[Evgeniy Makarev via Phabricator via lldb-commits]

Pivnoy added a comment.

At the moment, the TargetParser architecture is extensible. This complicates 
the addition of new architectures, operating systems, and so on.
In the main discussion, it was proposed to redesign the architecture of this 
module in order to increase the possibility of adding new architectures, etc.
The main idea of   the rework was to separate the Triple entity into a 
data-class, and create a number of interfaces from components that would 
include Triple, through the implementation of which it would be possible to 
represent various data bindings of the components. At the moment, Triple is 
overflowing with various methods that do not fully meet the ideas of OOP.
Since the TargetParser module is quite large and has many dependencies 
throughout the llvm-project, it was first of all supposed to remove these 
methods from Triple, since they would not correspond to OOP ideas.
This would help to gradually rid Triple of unnecessary dependencies, and 
gradually change the architecture inside Triple, without breaking code of 
another LLVM developers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157497

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

[Lldb-commits] [PATCH] D158085: [LLDB][Docs] Update cross compilation docs and examples

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere accepted this revision.
JDevlieghere 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/D158085/new/

https://reviews.llvm.org/D158085

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

[Lldb-commits] [PATCH] D154927: [lldb][AArch64] Add SME's streaming vector control register

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 550769.
DavidSpickett added a comment.

Correct regiter -> register in comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154927

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  
lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c
  
lldb/test/API/commands/register/register/aarch64_za_reg/za_save_restore/TestZARegisterSaveRestore.py

Index: lldb/test/API/commands/register/register/aarch64_za_reg/za_save_restore/TestZARegisterSaveRestore.py
===
--- lldb/test/API/commands/register/register/aarch64_za_reg/za_save_restore/TestZARegisterSaveRestore.py
+++ lldb/test/API/commands/register/register/aarch64_za_reg/za_save_restore/TestZARegisterSaveRestore.py
@@ -149,6 +149,10 @@
 self.runCmd("register read " + sve_reg_names)
 sve_values = self.res.GetOutput()
 
+svcr_value = 1 if sve_mode == Mode.SSVE else 0
+if za_state == ZA.Enabled:
+svcr_value += 2
+
 def check_regs():
 if za_state == ZA.Enabled:
 self.check_za(start_vl)
@@ -160,6 +164,7 @@
 self.assertEqual(start_vg, self.read_vg())
 
 self.expect("register read " + sve_reg_names, substrs=[sve_values])
+self.expect("register read svcr", substrs=["0x{:016x}".format(svcr_value)])
 
 for expr in exprs:
 expr_cmd = "expression {}()".format(expr)
Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c
===
--- lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c
@@ -5,7 +5,7 @@
 #define PR_SME_SET_VL 63
 #endif
 
-#define SMSTART() asm volatile("msr  s0_3_c4_c7_3, xzr" /*smstart*/)
+#define SMSTART_SM() asm volatile("msr  s0_3_c4_c3_3, xzr" /*smstart sm*/)
 
 void write_sve_regs() {
   // We assume the smefa64 feature is present, which allows ffr access
@@ -126,18 +126,18 @@
   // Note that doing a syscall brings you back to non-streaming mode, so we
   // don't need to SMSTOP here.
   if (streaming)
-SMSTART();
+SMSTART_SM();
   write_sve_regs_expr();
   prctl(SET_VL_OPT, 8 * 4);
   if (streaming)
-SMSTART();
+SMSTART_SM();
   write_sve_regs_expr();
   return 1;
 }
 
 int main() {
 #ifdef START_SSVE
-  SMSTART();
+  SMSTART_SM();
 #endif
   write_sve_regs();
 
Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
===
--- lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
@@ -24,7 +24,14 @@
 reg_value.GetByteSize(), expected, 'Verify "%s" == %i' % (name, expected)
 )
 
-def check_sve_regs_read(self, z_reg_size):
+def check_sve_regs_read(self, z_reg_size, expected_mode):
+if self.isAArch64SME():
+# This test uses SMSTART SM, which only enables streaming mode,
+# leaving ZA disabled.
+expected_value = "1" if expected_mode == Mode.SSVE else "0"
+self.expect("register read svcr", substrs=[
+"0x000" + expected_value])
+
 p_reg_size = int(z_reg_size / 8)
 
 for i in range(32):
@@ -168,7 +175,7 @@
 
 vg_reg_value = sve_registers.GetChildMemberWithName("vg").GetValueAsUnsigned()
 z_reg_size = vg_reg_value * 8
-self.check_sve_regs_read(z_reg_size)
+self.check_sve_regs_read(z_reg_size, start_mode)
 
 # Evaluate simple expression and print function expr_eval_func address.
 self.expect("expression expr_eval_func", substrs=["= 0x"])
@@ -184,7 +191,7 @@
 
 # We called a jitted function above which must not have changed SVE
 # vector length or register values.
-self.check_sve_regs_read(z_reg_size)
+self.check_sve_regs_read(z_reg_size, start_mode)
 
 self.check_sve_regs_read_after_write(z_reg_size)
 
Index: lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py
===
--- 

[Lldb-commits] [PATCH] D158085: [LLDB][Docs] Update cross compilation docs and examples

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added reviewers: JDevlieghere, bulbazord.
DavidSpickett added a comment.

I've confirmed the instructions work for x86 -> AArch64. The original issue was 
x86 -> Arm 32 bit, so I hope to get the issue reporter to confirm whether the 
instructions make sense with this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158085

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

[Lldb-commits] [PATCH] D158085: [LLDB][Docs] Update cross compilation docs and examples

[David Spickett via Phabricator via lldb-commits]

DavidSpickett created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

To address https://github.com/llvm/llvm-project/issues/64616, this change
does the following:

- Links back to the "Optional Dependencies" section from the cross compilation 
instructions where we talk about removing libraries. In the reported issue, the 
host libXML was found by CMAke somehow and disabling libXML fixed it, so this 
link should encourage users to try as many of those options as needed.

- Removes the use of CMAKE_CROSS_COMPILING. In older CMake versions it seems 
you could set this manually but now the advice is to set CMAKE_SYSTEM_NAME and 
CMAKE_SYSTEM_PROCESSOR then let CMake figure it out.

  There is also CMAKE_SYSTEM_VERSION which the docs say you have to set too, 
but I can only find examples of Windows and Android builds using this. It might 
be needed to "cross" compile from one version of AArch64 Linux to another, but 
I can't confirm that.

- Removes the tablegen tools paths in favour of DLLVM_NATIVE_TOOL_DIR. This one 
setting deals with all build tools in llvm that must be host versions.

- Updates the explanations of the common options and orders them in some rough 
order of relevance with the "might be needed" later.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158085

Files:
  lldb/docs/resources/build.rst


Index: lldb/docs/resources/build.rst
===
--- lldb/docs/resources/build.rst
+++ lldb/docs/resources/build.rst
@@ -36,6 +36,8 @@
 * `Python `_
 * `SWIG `_ 4 or later.
 
+.. _Optional Dependencies:
+
 Optional Dependencies
 *
 
@@ -458,6 +460,8 @@
   -DLLDB_ENABLE_CURSES=0
   -DLLVM_ENABLE_TERMINFO=0
 
+(see :ref:`Optional Dependencies` for more)
+
 In this case you, will often not need anything other than the standard C and
 C++ libraries.
 
@@ -465,26 +469,29 @@
 the build system with the locations and arguments of all the necessary tools.
 The most important cmake options here are:
 
-* ``CMAKE_CROSSCOMPILING`` : Set to 1 to enable cross-compilation.
-* ``CMAKE_LIBRARY_ARCHITECTURE`` : Affects the cmake search path when looking
-  for libraries. You may need to set this to your architecture triple if you do
-  not specify all your include and library paths explicitly.
+* ``CMAKE_SYSTEM_NAME`` and ``CMAKE_SYSTEM_PROCESSOR``: This tells CMake what
+  the build target is and from this it will infer that you are cross compiling.
 * ``CMAKE_C_COMPILER``, ``CMAKE_CXX_COMPILER`` : C and C++ compilers for the
-  target architecture
+  target architecture.
 * ``CMAKE_C_FLAGS``, ``CMAKE_CXX_FLAGS`` : The flags for the C and C++ target
-  compilers. You may need to specify the exact target cpu and abi besides the
+  compilers. You may need to specify the exact target cpu and ABI besides the
   include paths for the target headers.
 * ``CMAKE_EXE_LINKER_FLAGS`` : The flags to be passed to the linker. Usually
   just a list of library search paths referencing the target libraries.
-* ``LLVM_TABLEGEN``, ``CLANG_TABLEGEN`` : Paths to llvm-tblgen and clang-tblgen
-  for the host architecture. If you already have built clang for the host, you
-  can point these variables to the executables in your build directory. If not,
-  you will need to build the llvm-tblgen and clang-tblgen host targets at
-  least.
 * ``LLVM_HOST_TRIPLE`` : The triple of the system that lldb (or lldb-server)
   will run on. Not setting this (or setting it incorrectly) can cause a lot of
   issues with remote debugging as a lot of the choices lldb makes depend on the
   triple reported by the remote platform.
+* ``LLVM_NATIVE_TOOL_DIR`` : Is a path to the llvm tools compiled for the host.
+  Any tool that must be run on the host during a cross build will be configured
+  from this path, so you do not need to set them all individually. If you are
+  doing a host build just for the purpose of a cross build, you will need it
+  to include at least ``llvm-tblgen``, ``clang-tblgen`` and ``lldb-tblgen``.
+  Please be aware that that list may grow over time.
+* ``CMAKE_LIBRARY_ARCHITECTURE`` : Affects the cmake search path when looking
+  for libraries. You may need to set this to your architecture triple if you do
+  not specify all your include and library paths explicitly.
+
 
 You can of course also specify the usual cmake options like
 ``CMAKE_BUILD_TYPE``, etc.
@@ -499,12 +506,12 @@
 
 ::
 
-  -DCMAKE_CROSSCOMPILING=1 \
+  -DCMAKE_SYSTEM_NAME=Linux \
+  -DCMAKE_SYSTEM_PROCESSOR=AArch64 \
   -DCMAKE_C_COMPILER=aarch64-linux-gnu-gcc \
   -DCMAKE_CXX_COMPILER=aarch64-linux-gnu-g++ \
   -DLLVM_HOST_TRIPLE=aarch64-unknown-linux-gnu \
-  -DLLVM_TABLEGEN=/bin/llvm-tblgen \
-  -DCLANG_TABLEGEN=/bin/clang-tblgen \
+  -DLLVM_NATIVE_TOOL_DIR=/bin/ \
   

[Lldb-commits] [PATCH] D158023: [lldb] Simplify the LLDB website structure

[Felipe de Azevedo Piovezan via Phabricator via lldb-commits]

fdeazeve added a comment.

In D158023#4590502 , @bulbazord wrote:

> I think the categories you've set are the same ones I would have created. May 
> I suggest renaming `Extending LLDB` to `Scripting LLDB` or something similar? 
> I think "Extending" and "Developing" are close enough in meaning that it may 
> be confusing to end users.

+1 to scripting; this is the word we use all the time, so it makes sense to 
highlight it as the name of a category.

Btw, what do you think of the extensions page? This patch currently keeps 
"extensions" under "using lldb", but if we read that page it contains no 
"using" information. It explains at length how an extension is *implemented* in 
terms of DWARF. Because of that, I think this page is aimed at developers.


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

https://reviews.llvm.org/D158023

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

[Lldb-commits] [PATCH] D157960: [lldb][gui] Update TreeItem children's m_parent on move

[Pavel Kosov via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG83695d45d621: [lldb][gui] Update TreeItem childrens 
m_parent on move (authored by kpdev42).

Changed prior to commit:
  https://reviews.llvm.org/D157960?vs=550240=550655#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157960

Files:
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/test/API/commands/gui/spawn-threads/Makefile
  lldb/test/API/commands/gui/spawn-threads/TestGuiSpawnThreads.py
  lldb/test/API/commands/gui/spawn-threads/main.cpp

Index: lldb/test/API/commands/gui/spawn-threads/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/gui/spawn-threads/main.cpp
@@ -0,0 +1,25 @@
+#include 
+#include 
+#include 
+
+#include "pseudo_barrier.h"
+
+pseudo_barrier_t barrier_inside;
+
+void thread_func() { pseudo_barrier_wait(barrier_inside); }
+
+void test_thread() {
+  std::vector thrs;
+  for (int i = 0; i < 5; i++)
+thrs.push_back(std::thread(thread_func)); // break here
+
+  pseudo_barrier_wait(barrier_inside); // break before join
+  for (auto  : thrs)
+t.join();
+}
+
+int main() {
+  pseudo_barrier_init(barrier_inside, 6);
+  test_thread();
+  return 0;
+}
Index: lldb/test/API/commands/gui/spawn-threads/TestGuiSpawnThreads.py
===
--- /dev/null
+++ lldb/test/API/commands/gui/spawn-threads/TestGuiSpawnThreads.py
@@ -0,0 +1,47 @@
+"""
+Test that 'gui' does not crash when adding new threads, which
+populate TreeItem's children and may be reallocated elsewhere.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.lldbpexpect import PExpectTest
+
+import sys
+
+class TestGuiSpawnThreadsTest(PExpectTest):
+# PExpect uses many timeouts internally and doesn't play well
+# under ASAN on a loaded machine..
+@skipIfAsan
+@skipIfCursesSupportMissing
+def test_gui(self):
+self.build()
+
+self.launch(executable=self.getBuildArtifact('a.out'), dimensions=(100, 500))
+self.expect(
+'breakpoint set -f main.cpp -p "break here"', substrs=['Breakpoint 1', 'address =']
+)
+self.expect(
+'breakpoint set -f main.cpp -p "before join"', substrs=['Breakpoint 2', 'address =']
+)
+self.expect("run", substrs=["stop reason ="])
+
+escape_key = chr(27).encode()
+
+# Start the GUI
+self.child.sendline("gui")
+self.child.expect_exact("Threads")
+self.child.expect_exact(f"thread #1: tid =")
+
+for i in range(5):
+# Stopped at the breakpoint, continue over the thread creation
+self.child.send("c")
+# Check the newly created thread
+self.child.expect_exact(f"thread #{i + 2}: tid =")
+
+# Exit GUI.
+self.child.send(escape_key)
+self.expect_prompt()
+
+self.quit()
Index: lldb/test/API/commands/gui/spawn-threads/Makefile
===
--- /dev/null
+++ lldb/test/API/commands/gui/spawn-threads/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+ENABLE_THREADS := YES
+include Makefile.rules
Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -4614,30 +4614,48 @@
 
 typedef std::shared_ptr TreeDelegateSP;
 
-class TreeItem {
+struct TreeItemData {
+  TreeItemData(TreeItem *parent, TreeDelegate ,
+   bool might_have_children, bool is_expanded)
+  : m_parent(parent), m_delegate(),
+m_might_have_children(might_have_children), m_is_expanded(is_expanded) {
+  }
+
+protected:
+  TreeItem *m_parent;
+  TreeDelegate *m_delegate;
+  void *m_user_data = nullptr;
+  uint64_t m_identifier = 0;
+  std::string m_text;
+  int m_row_idx = -1; // Zero based visible row index, -1 if not visible or for
+  // the root item
+  bool m_might_have_children;
+  bool m_is_expanded = false;
+};
+
+class TreeItem : public TreeItemData {
 public:
   TreeItem(TreeItem *parent, TreeDelegate , bool might_have_children)
-  : m_parent(parent), m_delegate(delegate), m_children(),
-m_might_have_children(might_have_children) {
-if (m_parent == nullptr)
-  m_is_expanded = m_delegate.TreeDelegateExpandRootByDefault();
-  }
+  : TreeItemData(parent, delegate, might_have_children,
+ parent == nullptr
+ ? delegate.TreeDelegateExpandRootByDefault()
+ : false),
+m_children() {}
+
+  TreeItem(const TreeItem &) = delete;
+  TreeItem =(const TreeItem ) = delete;
 
-  TreeItem =(const TreeItem ) {
+  TreeItem =(TreeItem &) {
 if (this 

[Lldb-commits] [lldb] 83695d4 - [lldb][gui] Update TreeItem children's m_parent on move

[Pavel Kosov via lldb-commits]

Author: Pavel Kosov
Date: 2023-08-16T11:10:00+03:00
New Revision: 83695d45d62121ab306d0dc108b549d9056a2f28

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

LOG: [lldb][gui] Update TreeItem children's m_parent on move

Before this patch, any time TreeItem is copied in Resize method, its
parent is not updated, which can cause crashes when, for example, thread
window with multiple hierarchy levels is updated. Makes TreeItem
move-only, removes TreeItem's m_delegate extra self-assignment by making
it a pointer, adds code to fix up children's parent on move constructor
and operator=
Patch prepared by NH5pml30

~~~

Huawei RRI, OS Lab

Reviewed By: clayborg

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

Added: 
lldb/test/API/commands/gui/spawn-threads/Makefile
lldb/test/API/commands/gui/spawn-threads/TestGuiSpawnThreads.py
lldb/test/API/commands/gui/spawn-threads/main.cpp

Modified: 
lldb/source/Core/IOHandlerCursesGUI.cpp

Removed: 




diff  --git a/lldb/source/Core/IOHandlerCursesGUI.cpp 
b/lldb/source/Core/IOHandlerCursesGUI.cpp
index 92f62b725ce385..22b8cc3582eae7 100644
--- a/lldb/source/Core/IOHandlerCursesGUI.cpp
+++ b/lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -4614,30 +4614,48 @@ class TreeDelegate {
 
 typedef std::shared_ptr TreeDelegateSP;
 
-class TreeItem {
+struct TreeItemData {
+  TreeItemData(TreeItem *parent, TreeDelegate ,
+   bool might_have_children, bool is_expanded)
+  : m_parent(parent), m_delegate(),
+m_might_have_children(might_have_children), m_is_expanded(is_expanded) 
{
+  }
+
+protected:
+  TreeItem *m_parent;
+  TreeDelegate *m_delegate;
+  void *m_user_data = nullptr;
+  uint64_t m_identifier = 0;
+  std::string m_text;
+  int m_row_idx = -1; // Zero based visible row index, -1 if not visible or for
+  // the root item
+  bool m_might_have_children;
+  bool m_is_expanded = false;
+};
+
+class TreeItem : public TreeItemData {
 public:
   TreeItem(TreeItem *parent, TreeDelegate , bool might_have_children)
-  : m_parent(parent), m_delegate(delegate), m_children(),
-m_might_have_children(might_have_children) {
-if (m_parent == nullptr)
-  m_is_expanded = m_delegate.TreeDelegateExpandRootByDefault();
-  }
+  : TreeItemData(parent, delegate, might_have_children,
+ parent == nullptr
+ ? delegate.TreeDelegateExpandRootByDefault()
+ : false),
+m_children() {}
+
+  TreeItem(const TreeItem &) = delete;
+  TreeItem =(const TreeItem ) = delete;
 
-  TreeItem =(const TreeItem ) {
+  TreeItem =(TreeItem &) {
 if (this != ) {
-  m_parent = rhs.m_parent;
-  m_delegate = rhs.m_delegate;
-  m_user_data = rhs.m_user_data;
-  m_identifier = rhs.m_identifier;
-  m_row_idx = rhs.m_row_idx;
-  m_children = rhs.m_children;
-  m_might_have_children = rhs.m_might_have_children;
-  m_is_expanded = rhs.m_is_expanded;
+  TreeItemData::operator=(std::move(rhs));
+  AdoptChildren(rhs.m_children);
 }
 return *this;
   }
 
-  TreeItem(const TreeItem &) = default;
+  TreeItem(TreeItem &) : TreeItemData(std::move(rhs)) {
+AdoptChildren(rhs.m_children);
+  }
 
   size_t GetDepth() const {
 if (m_parent)
@@ -4649,18 +4667,28 @@ class TreeItem {
 
   void ClearChildren() { m_children.clear(); }
 
-  void Resize(size_t n, const TreeItem ) { m_children.resize(n, t); }
+  void Resize(size_t n, TreeDelegate , bool might_have_children) {
+if (m_children.size() >= n) {
+  m_children.erase(m_children.begin() + n, m_children.end());
+  return;
+}
+m_children.reserve(n);
+std::generate_n(std::back_inserter(m_children), n - m_children.size(),
+[&, parent = this]() {
+  return TreeItem(parent, delegate, might_have_children);
+});
+  }
 
   TreeItem [](size_t i) { return m_children[i]; }
 
   void SetRowIndex(int row_idx) { m_row_idx = row_idx; }
 
   size_t GetNumChildren() {
-m_delegate.TreeDelegateGenerateChildren(*this);
+m_delegate->TreeDelegateGenerateChildren(*this);
 return m_children.size();
   }
 
-  void ItemWasSelected() { m_delegate.TreeDelegateItemSelected(*this); }
+  void ItemWasSelected() { m_delegate->TreeDelegateItemSelected(*this); }
 
   void CalculateRowIndexes(int _idx) {
 SetRowIndex(row_idx);
@@ -4727,7 +4755,7 @@ class TreeItem {
   if (highlight)
 window.AttributeOn(A_REVERSE);
 
-  m_delegate.TreeDelegateDrawTreeItem(*this, window);
+  m_delegate->TreeDelegateDrawTreeItem(*this, window);
 
   if (highlight)
 window.AttributeOff(A_REVERSE);
@@ -4811,16 +4839,13 @@ class TreeItem {
   void SetMightHaveChildren(bool b) {