[Lldb-commits] [PATCH] D158251: [lldb] Propagate destination type's byte order to memory writer

2023-08-17 Thread Kamlesh Kumar via Phabricator via lldb-commits 
kkcode0 updated this revision to Diff 551398.
kkcode0 added a comment.

typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158251

Files:
  lldb/include/lldb/Symbol/CompilerType.h
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/include/lldb/Target/Process.h
  lldb/source/Core/ValueObject.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/source/Symbol/CompilerType.cpp
  lldb/source/Target/Process.cpp

Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2111,14 +2111,14 @@
 }
 
 size_t Process::WriteMemoryPrivate(addr_t addr, const void *buf, size_t size,
-   Status &error) {
+   Status &error, ByteOrder byte_order) {
   size_t bytes_written = 0;
   const uint8_t *bytes = (const uint8_t *)buf;
 
   while (bytes_written < size) {
 const size_t curr_size = size - bytes_written;
 const size_t curr_bytes_written = DoWriteMemory(
-addr + bytes_written, bytes + bytes_written, curr_size, error);
+addr + bytes_written, bytes + bytes_written, curr_size, error, byte_order);
 bytes_written += curr_bytes_written;
 if (curr_bytes_written == curr_size || curr_bytes_written == 0)
   break;
@@ -2127,7 +2127,7 @@
 }
 
 size_t Process::WriteMemory(addr_t addr, const void *buf, size_t size,
-Status &error) {
+Status &error, ByteOrder byte_order) {
   if (ABISP abi_sp = GetABI())
 addr = abi_sp->FixAnyAddress(addr);
 
@@ -2146,17 +2146,17 @@
 
   BreakpointSiteList bp_sites_in_range;
   if (!m_breakpoint_site_list.FindInRange(addr, addr + size, bp_sites_in_range))
-return WriteMemoryPrivate(addr, buf, size, error);
+return WriteMemoryPrivate(addr, buf, size, error, byte_order);
 
   // No breakpoint sites overlap
   if (bp_sites_in_range.IsEmpty())
-return WriteMemoryPrivate(addr, buf, size, error);
+return WriteMemoryPrivate(addr, buf, size, error, byte_order);
 
   const uint8_t *ubuf = (const uint8_t *)buf;
   uint64_t bytes_written = 0;
 
   bp_sites_in_range.ForEach([this, addr, size, &bytes_written, &ubuf,
- &error](BreakpointSite *bp) -> void {
+ &error, byte_order](BreakpointSite *bp) -> void {
 if (error.Fail())
   return;
 
@@ -2182,7 +2182,7 @@
   // write to memory
   size_t curr_size = intersect_addr - curr_addr;
   size_t curr_bytes_written =
-  WriteMemoryPrivate(curr_addr, ubuf + bytes_written, curr_size, error);
+  WriteMemoryPrivate(curr_addr, ubuf + bytes_written, curr_size, error, byte_order);
   bytes_written += curr_bytes_written;
   if (curr_bytes_written != curr_size) {
 // We weren't able to write all of the requested bytes, we are
@@ -2203,13 +2203,14 @@
   if (bytes_written < size)
 bytes_written +=
 WriteMemoryPrivate(addr + bytes_written, ubuf + bytes_written,
-   size - bytes_written, error);
+   size - bytes_written, error, byte_order);
 
   return bytes_written;
 }
 
 size_t Process::WriteScalarToMemory(addr_t addr, const Scalar &scalar,
-size_t byte_size, Status &error) {
+size_t byte_size, Status &error,
+ByteOrder byte_order) {
   if (byte_size == UINT32_MAX)
 byte_size = scalar.GetByteSize();
   if (byte_size > 0) {
@@ -2217,7 +2218,7 @@
 const size_t mem_size =
 scalar.GetAsMemoryData(buf, byte_size, GetByteOrder(), error);
 if (mem_size > 0)
-  return WriteMemory(addr, buf, mem_size, error);
+  return WriteMemory(addr, buf, mem_size, error, byte_order);
 else
   error.SetErrorString("failed to get scalar as memory data");
   } else {
Index: lldb/source/Symbol/CompilerType.cpp
===
--- lldb/source/Symbol/CompilerType.cpp
+++ lldb/source/Symbol/CompilerType.cpp
@@ -584,6 +584,13 @@
   return lldb::eEncodingInvalid;
 }
 
+lldb::ByteOrder CompilerType::GetByteOrder() const {
+  if (IsValid())
+if (auto type_system_sp = GetTypeSystem())
+  return type_system_sp->GetByteOrder(m_type);
+  return endian::InlHostByteOrder();
+}
+
 lldb::Format CompilerType::GetFormat() const {
   if (IsValid())
 if (auto type_system_sp = GetTypeSystem())
Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
=

[Lldb-commits] [PATCH] D158251: [lldb] Propagate destination type's byte order to memory writer

2023-08-17 Thread Kamlesh Kumar via Phabricator via lldb-commits 
kkcode0 updated this revision to Diff 551397.
kkcode0 added a comment.
Herald added a subscriber: JDevlieghere.

clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158251

Files:
  lldb/include/lldb/Symbol/CompilerType.h
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/include/lldb/Target/Process.h
  lldb/source/Core/ValueObject.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/source/Symbol/CompilerType.cpp
  lldb/source/Target/Process.cpp

Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2111,14 +2111,14 @@
 }
 
 size_t Process::WriteMemoryPrivate(addr_t addr, const void *buf, size_t size,
-   Status &error) {
+   Status &error, ByteOrder byte_order) {
   size_t bytes_written = 0;
   const uint8_t *bytes = (const uint8_t *)buf;
 
   while (bytes_written < size) {
 const size_t curr_size = size - bytes_written;
 const size_t curr_bytes_written = DoWriteMemory(
-addr + bytes_written, bytes + bytes_written, curr_size, error);
+addr + bytes_written, bytes + bytes_written, curr_size, error, byte_order);
 bytes_written += curr_bytes_written;
 if (curr_bytes_written == curr_size || curr_bytes_written == 0)
   break;
@@ -2127,7 +2127,7 @@
 }
 
 size_t Process::WriteMemory(addr_t addr, const void *buf, size_t size,
-Status &error) {
+Status &error, ByteOrder byte_order) {
   if (ABISP abi_sp = GetABI())
 addr = abi_sp->FixAnyAddress(addr);
 
@@ -2146,17 +2146,17 @@
 
   BreakpointSiteList bp_sites_in_range;
   if (!m_breakpoint_site_list.FindInRange(addr, addr + size, bp_sites_in_range))
-return WriteMemoryPrivate(addr, buf, size, error);
+return WriteMemoryPrivate(addr, buf, size, error, byte_order);
 
   // No breakpoint sites overlap
   if (bp_sites_in_range.IsEmpty())
-return WriteMemoryPrivate(addr, buf, size, error);
+return WriteMemoryPrivate(addr, buf, size, error, byte_order);
 
   const uint8_t *ubuf = (const uint8_t *)buf;
   uint64_t bytes_written = 0;
 
   bp_sites_in_range.ForEach([this, addr, size, &bytes_written, &ubuf,
- &error](BreakpointSite *bp) -> void {
+ &error, byte_order](BreakpointSite *bp) -> void {
 if (error.Fail())
   return;
 
@@ -2182,7 +2182,7 @@
   // write to memory
   size_t curr_size = intersect_addr - curr_addr;
   size_t curr_bytes_written =
-  WriteMemoryPrivate(curr_addr, ubuf + bytes_written, curr_size, error);
+  WriteMemoryPrivate(curr_addr, ubuf + bytes_written, curr_size, error, byte_order);
   bytes_written += curr_bytes_written;
   if (curr_bytes_written != curr_size) {
 // We weren't able to write all of the requested bytes, we are
@@ -2203,13 +2203,14 @@
   if (bytes_written < size)
 bytes_written +=
 WriteMemoryPrivate(addr + bytes_written, ubuf + bytes_written,
-   size - bytes_written, error);
+   size - bytes_written, error, byte_order);
 
   return bytes_written;
 }
 
 size_t Process::WriteScalarToMemory(addr_t addr, const Scalar &scalar,
-size_t byte_size, Status &error) {
+size_t byte_size, Status &error,
+ByteOrder byte_order) {
   if (byte_size == UINT32_MAX)
 byte_size = scalar.GetByteSize();
   if (byte_size > 0) {
@@ -2217,7 +2218,7 @@
 const size_t mem_size =
 scalar.GetAsMemoryData(buf, byte_size, GetByteOrder(), error);
 if (mem_size > 0)
-  return WriteMemory(addr, buf, mem_size, error);
+  return WriteMemory(addr, buf, mem_size, error, byte_order);
 else
   error.SetErrorString("failed to get scalar as memory data");
   } else {
Index: lldb/source/Symbol/CompilerType.cpp
===
--- lldb/source/Symbol/CompilerType.cpp
+++ lldb/source/Symbol/CompilerType.cpp
@@ -584,6 +584,13 @@
   return lldb::eEncodingInvalid;
 }
 
+lldb::ByteOrder CompilerType::GetByteOrder() const {
+  if (IsValid())
+if (auto type_system_sp = GetTypeSystem())
+  return type_system_sp->GetByteOrder(m_type);
+  return endian::InlHostByteOrder();
+}
+
 lldb::Format CompilerType::GetFormat() const {
   if (IsValid())
 if (auto type_system_sp = GetTypeSystem())
Index: lldb/source/Plugins/TypeSystem/Clan

[Lldb-commits] [PATCH] D158251: [lldb] Propagate destination type's byte order to memory writer

2023-08-17 Thread Kamlesh Kumar via Phabricator via lldb-commits 
kkcode0 created this revision.
Herald added a project: All.
kkcode0 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

When a compiler attaches endianess to type, debugger should honor that when 
writing to variables of that variable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158251

Files:
  lldb/include/lldb/Symbol/CompilerType.h
  lldb/include/lldb/Symbol/TypeSystem.h
  lldb/include/lldb/Target/Process.h
  lldb/source/Core/ValueObject.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  lldb/source/Symbol/CompilerType.cpp
  lldb/source/Target/Process.cpp

Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -2111,14 +2111,14 @@
 }
 
 size_t Process::WriteMemoryPrivate(addr_t addr, const void *buf, size_t size,
-   Status &error) {
+   Status &error, ByteOrder byte_order) {
   size_t bytes_written = 0;
   const uint8_t *bytes = (const uint8_t *)buf;
 
   while (bytes_written < size) {
 const size_t curr_size = size - bytes_written;
 const size_t curr_bytes_written = DoWriteMemory(
-addr + bytes_written, bytes + bytes_written, curr_size, error);
+addr + bytes_written, bytes + bytes_written, curr_size, error, byte_order);
 bytes_written += curr_bytes_written;
 if (curr_bytes_written == curr_size || curr_bytes_written == 0)
   break;
@@ -2127,7 +2127,7 @@
 }
 
 size_t Process::WriteMemory(addr_t addr, const void *buf, size_t size,
-Status &error) {
+Status &error, ByteOrder byte_order) {
   if (ABISP abi_sp = GetABI())
 addr = abi_sp->FixAnyAddress(addr);
 
@@ -2146,17 +2146,17 @@
 
   BreakpointSiteList bp_sites_in_range;
   if (!m_breakpoint_site_list.FindInRange(addr, addr + size, bp_sites_in_range))
-return WriteMemoryPrivate(addr, buf, size, error);
+return WriteMemoryPrivate(addr, buf, size, error, byte_order);
 
   // No breakpoint sites overlap
   if (bp_sites_in_range.IsEmpty())
-return WriteMemoryPrivate(addr, buf, size, error);
+return WriteMemoryPrivate(addr, buf, size, error, byte_order);
 
   const uint8_t *ubuf = (const uint8_t *)buf;
   uint64_t bytes_written = 0;
 
   bp_sites_in_range.ForEach([this, addr, size, &bytes_written, &ubuf,
- &error](BreakpointSite *bp) -> void {
+ &error, byte_order](BreakpointSite *bp) -> void {
 if (error.Fail())
   return;
 
@@ -2182,7 +2182,7 @@
   // write to memory
   size_t curr_size = intersect_addr - curr_addr;
   size_t curr_bytes_written =
-  WriteMemoryPrivate(curr_addr, ubuf + bytes_written, curr_size, error);
+  WriteMemoryPrivate(curr_addr, ubuf + bytes_written, curr_size, error, byte_order);
   bytes_written += curr_bytes_written;
   if (curr_bytes_written != curr_size) {
 // We weren't able to write all of the requested bytes, we are
@@ -2203,13 +2203,14 @@
   if (bytes_written < size)
 bytes_written +=
 WriteMemoryPrivate(addr + bytes_written, ubuf + bytes_written,
-   size - bytes_written, error);
+   size - bytes_written, error, byte_order);
 
   return bytes_written;
 }
 
 size_t Process::WriteScalarToMemory(addr_t addr, const Scalar &scalar,
-size_t byte_size, Status &error) {
+size_t byte_size, Status &error,
+ByteOrder byte_order) {
   if (byte_size == UINT32_MAX)
 byte_size = scalar.GetByteSize();
   if (byte_size > 0) {
@@ -2217,7 +2218,7 @@
 const size_t mem_size =
 scalar.GetAsMemoryData(buf, byte_size, GetByteOrder(), error);
 if (mem_size > 0)
-  return WriteMemory(addr, buf, mem_size, error);
+  return WriteMemory(addr, buf, mem_size, error, byte_order);
 else
   error.SetErrorString("failed to get scalar as memory data");
   } else {
Index: lldb/source/Symbol/CompilerType.cpp
===
--- lldb/source/Symbol/CompilerType.cpp
+++ lldb/source/Symbol/CompilerType.cpp
@@ -584,6 +584,13 @@
   return lldb::eEncodingInvalid;
 }
 
+lldb::ByteOrder CompilerType::GetByteOrder() const {
+  if (IsValid())
+if (auto type_system_sp = GetTypeSystem())
+  return type_system_sp->GetByteOrder(m_type);
+  return endian::InlHostByteOrder();
+}
+
 lldb::Format CompilerType::GetFormat() const {
   if (IsV

[Lldb-commits] [PATCH] D156270: [lldb][NFCI] Change logic to find clang resource dir in standalone builds

2023-08-17 Thread Junchang Liu via Phabricator via lldb-commits 
paperchalice added a comment.

When I made D141907 , I want 
`get_clang_resource_dir` in cmake and GetResourcesPath 
 in 
clang to have the same behavior and it also respects the doc string of 
`CLANG_RESOURCE_DIR`, but the behavior of `GetResourcesPath` itself is 
unintuitive when custom resource path is empty.

In D156270#4557902 , @mgorny wrote:

> I'm not sure if it was really intended but the new API is kinda horrible, at 
> least for us in Gentoo.
>
> Our install prefix is `/usr/lib/llvm/NN`, whereas clang resource dir is 
> `/usr/lib/clang/NN`.
>
> If I don't override `CLANG_RESOURCE_DIR`, it infers the wrong directory:
> `/usr/lib/llvm/18/lib64/cmake/clang/../../..//lib64/clang/18`.

IIUC, the resource dir should be `/usr/lib/llvm/18/lib64/clang/18` when 
`CLANG_RESOURCE_DIR` is empty.

> To get the correct directory, I need to pass:
> `-DCLANG_RESOURCE_DIR="../../../clang/${LLVM_MAJOR}"`
> which is absolutely counterintuitive (the path gets appended to:
> `/usr/lib/llvm/18/lib64/cmake/clang/../../../bin`
> ).
>
> Not saying it's not workable but I'd hardly call that an improvement over the 
> previous API.
>
> CC @tstellar, @paperchalice.

The API here is indeed user unfriendly, a possible improvement is to let 
`CLANG_RESOURCE_DIR` aslo accept absolute path.
Also, `CLANG_RESOURCE_DIR` doesn't seem to work in standalone mode, some extra 
variables should be exported to `ClangConfig.cmake` e.g. the install prefix and 
bin dir of clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156270

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

[Lldb-commits] [lldb] 10f494d - Only set the "low" address masks when only one adressable bits specified

2023-08-17 Thread Jason Molenda via lldb-commits 

Author: Jason Molenda
Date: 2023-08-17T18:16:31-07:00
New Revision: 10f494d2896b8a58f69cc47d0db35d2b84c2fb44

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

LOG: Only set the "low" address masks when only one adressable bits specified

qHostInfo / stop-reply packet / LC_NOTE "addrable bits" can all
specify either a single value for all address masks, or separate
masks for low and high memory addresses.

When the same number of addressing bits are used for all addresses,
we use the "low memory" address masks for everything. (or another
way, if the high address masks are not set, we use the low address
masks with the assumption that all memory is using the same mask --
the most common situation).

I was setting low and high address masks when I had a single value
from these metadata, but that gave the impression that the high
address mask was specified explicitly.  After living on the code
a bit, it's clearly better to only set the high address masks when
we have a distinct high address mask value.

This patch is the minor adjustment to behave that way.

Added: 


Modified: 
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 3b52f718b6dab4..3e52c9e3c04281 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -5514,9 +5514,7 @@ AddressableBits ObjectFileMachO::GetAddressableBits() {
   if (m_data.GetU32(&offset, &version, 1) != nullptr) {
 if (version == 3) {
   uint32_t num_addr_bits = m_data.GetU32_unchecked(&offset);
-  if (num_addr_bits != 0) {
-addressable_bits.SetAddressableBits(num_addr_bits);
-  }
+  addressable_bits.SetAddressableBits(num_addr_bits);
   LLDB_LOGF(log,
 "LC_NOTE 'addrable bits' v3 found, value %d "
 "bits",
@@ -5527,7 +5525,10 @@ AddressableBits ObjectFileMachO::GetAddressableBits() {
   uint32_t lo_addr_bits = m_data.GetU32_unchecked(&offset);
   uint32_t hi_addr_bits = m_data.GetU32_unchecked(&offset);
 
-  addressable_bits.SetAddressableBits(lo_addr_bits, hi_addr_bits);
+  if (lo_addr_bits == hi_addr_bits)
+addressable_bits.SetAddressableBits(lo_addr_bits);
+  else
+addressable_bits.SetAddressableBits(lo_addr_bits, 
hi_addr_bits);
   LLDB_LOGF(log,
 "LC_NOTE 'addrable bits' v4 found, value %d & %d bits",
 lo_addr_bits, hi_addr_bits);

diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 1fcd850643ce07..04d98b96acd683 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1266,7 +1266,6 @@ bool GDBRemoteCommunicationClient::GetHostInfo(bool 
force) {
   ++num_keys_decoded;
   } else if (name.equals("addressing_bits")) {
 if (!value.getAsInteger(0, m_low_mem_addressing_bits)) {
-  m_high_mem_addressing_bits = m_low_mem_addressing_bits;
   ++num_keys_decoded;
 }
   } else if (name.equals("high_mem_addressing_bits")) {
@@ -1420,11 +1419,11 @@ AddressableBits 
GDBRemoteCommunicationClient::GetAddressableBits() {
   if (m_qHostInfo_is_valid == eLazyBoolCalculate)
 GetHostInfo();
 
-  // m_low_mem_addressing_bits and m_high_mem_addressing_bits
-  // will be 0 if we did not receive values; AddressableBits
-  // treats 0 as "unspecified".
-  addressable_bits.SetAddressableBits(m_low_mem_addressing_bits,
-  m_high_mem_addressing_bits);
+  if (m_low_mem_addressing_bits == m_high_mem_addressing_bits)
+addressable_bits.SetAddressableBits(m_low_mem_addressing_bits);
+  else
+addressable_bits.SetAddressableBits(m_low_mem_addressing_bits,
+m_high_mem_addressing_bits);
   return addressable_bits;
 }
 



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

[Lldb-commits] [PATCH] D158237: Change LLGSTest.cpp to run environment_check inferior with stdin/stdout disabled, to work around ASAN CI bot issue

2023-08-17 Thread Jim Ingham via Phabricator via lldb-commits 
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

This seems good to me.  Tests that aren't using stdio for a purpose shouldn't 
have to worry about random output.

But wait on my acceptance a bit to see if Pavel (or anyone else) has a stronger 
opinion...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158237

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

[Lldb-commits] [PATCH] D158237: Change LLGSTest.cpp to run environment_check inferior with stdin/stdout disabled, to work around ASAN CI bot issue

2023-08-17 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda created this revision.
jasonmolenda added a reviewer: labath.
jasonmolenda added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

We have an ASAN+UBSAN bot for lldb with swift supported added on swift.org - 
https://ci.swift.org/view/LLDB/job/oss-lldb-asan-macos/ - and it has been 
failing for a few months, it seems.  The failing test is the LLDBServerTests.  
There is a test that is setting an environment variable and running 
lldb/unittests/tools/lldb-server/inferior/environment_check.cpp to confirm (via 
the exit value) that the env var was set.  The test in 
lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp launches the app, continues 
and expects it to exit:

  ASSERT_THAT_ERROR(Client.ContinueAll(), Succeeded());
  ASSERT_THAT_EXPECTED(
  Client.GetLatestStopReplyAs(),
  HasValue(testing::Property(&StopReply::getKind,
 WaitStatus{WaitStatus::Exit, 0})));

This ContinueAll ends up in unittests/tools/lldb-server/tests/TestClient.cpp 
`TestClient::Continue()` which sends a continue packet, and expects to see 
either a stop reply packet or a process-exited response,

  [...]
auto StopReplyOr = SendMessage(
message, m_process_info->GetEndian(), m_register_infos);
if (!StopReplyOr)
  return StopReplyOr.takeError();
  
m_stop_reply = std::move(*StopReplyOr);
if (!isa(m_stop_reply)) {
  StringExtractorGDBRemote R;
  PacketResult result = ReadPacket(R, GetPacketTimeout(), false);
  [...]

When run on the UBSan bot, the inferior (environment_check) is also built ASAN 
and while it is running, asan or a malloc library prints a note to stderr, 
`environment_check(41292,0x113e7a600) malloc: nano zone abandoned due to 
inability to preallocate reserved vm space.`.  When we look at the bot logging, 
we see the problem:

  [  INFO ] 
/Users/ec2-user/jenkins/workspace/oss-lldb-asan-macos/llvm-project/lldb/unittests/tools/lldb-server/tests/TestClient.cpp:201::
 Send Packet: vCont;c
  
  [  INFO ] 
/Users/ec2-user/jenkins/workspace/oss-lldb-asan-macos/llvm-project/lldb/unittests/tools/lldb-server/tests/TestClient.cpp:204::
 Read Packet: 
O656e7669726f6e6d656e745f636865636b2833323338352c307831313535623336303029206d616c6c6f633a206e616e6f207a6f6e65206162616e646f6e65642064756520746f20696e6162696c69747920746f20707265616c6c6f6361746520726573657276656420766d2073706163652e0d0a
  
/Users/ec2-user/jenkins/workspace/oss-lldb-asan-macos/llvm-project/lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp:50:
 Failure
  Value of: llvm::detail::TakeError(Client.ContinueAll())
  Expected: succeeded
Actual: failed  (Unable to parse StopReply: Invalid packet)

We sent a `vCont;c` to continue, and some stdout/stderr text was received while 
it was running.  And the next packet we receive is almost certainly the 
"process has exited successfully" result, and the check in 
`TestClient::Continue()` would have succeeded.

There's no stdin/stdout/stderr needed with environment_check, so the easiest 
fix I'd like to try is to have it launched with these channels not sent up to 
lldb at all.  We should see the vCont;c followed by the "process exited 
successfully" packet and everyone will be happy.

This test is originally @labath but I don't want to assume he'll have time to 
look at this.  Let's see if anyone has an opinion, Pavel don't feel obligated 
to weigh in unless you have time/want to.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158237

Files:
  lldb/unittests/tools/lldb-server/tests/LLGSTest.cpp
  lldb/unittests/tools/lldb-server/tests/TestClient.cpp
  lldb/unittests/tools/lldb-server/tests/TestClient.h


Index: lldb/unittests/tools/lldb-server/tests/TestClient.h
===
--- lldb/unittests/tools/lldb-server/tests/TestClient.h
+++ lldb/unittests/tools/lldb-server/tests/TestClient.h
@@ -48,8 +48,9 @@
   /// using this for generic tests, as the two stubs have different
   /// command-line interfaces.
   static llvm::Expected>
-  launchCustom(llvm::StringRef Log, llvm::ArrayRef 
ServerArgs, llvm::ArrayRef InferiorArgs);
-
+  launchCustom(llvm::StringRef Log, bool disable_stdio,
+   llvm::ArrayRef ServerArgs,
+   llvm::ArrayRef InferiorArgs);
 
   ~TestClient() override;
   llvm::Error SetInferior(llvm::ArrayRef inferior_args);
Index: lldb/unittests/tools/lldb-server/tests/TestClient.cpp
===
--- lldb/unittests/tools/lldb-server/tests/TestClient.cpp
+++ lldb/unittests/tools/lldb-server/tests/TestClient.cpp
@@ -59,10 +59,13 @@
 }
 
 Expected> TestClient::launch(StringRef Log, 
ArrayRef InferiorArgs) {
-  return launchCustom(Log, {}, InferiorArgs);
+  return launchCustom(Log, false, {}, InferiorArgs);
 }
 
-Expected> TestClient::launchCustom(StringRef Log,

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

2023-08-17 Thread 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!


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] [lldb] d37642b - Simplify address mask setting logic in AddressableBits

2023-08-17 Thread Jason Molenda via lldb-commits 

Author: Jason Molenda
Date: 2023-08-17T16:22:39-07:00
New Revision: d37642b4a261b5b5687725fd60f7da5dc5ec4782

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

LOG: Simplify address mask setting logic in AddressableBits

I wrote some complicated conditionals for how to handle a partially
specified AddressableBits object in https://reviews.llvm.org/D158041 ,
and how to reuse existing masks if they were set and we had an
unspecified value.

I don't think this logic is the right thing to start
with.  Simplify back to the most straightforward
setting, where only the bits that have been specified
are set in the Process.

Added: 


Modified: 
lldb/source/Utility/AddressableBits.cpp

Removed: 




diff  --git a/lldb/source/Utility/AddressableBits.cpp 
b/lldb/source/Utility/AddressableBits.cpp
index e4479b855b1403..c6e25f608da73d 100644
--- a/lldb/source/Utility/AddressableBits.cpp
+++ b/lldb/source/Utility/AddressableBits.cpp
@@ -37,42 +37,14 @@ void AddressableBits::SetProcessMasks(Process &process) {
   if (m_low_memory_addr_bits == 0 && m_high_memory_addr_bits == 0)
 return;
 
-  // 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 (m_low_memory_addr_bits != 0) {
+addr_t low_addr_mask = ~((1ULL << m_low_memory_addr_bits) - 1);
+process.SetCodeAddressMask(low_addr_mask);
+process.SetDataAddressMask(low_addr_mask);
   }
 
-  // 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 (low_addr_mask != hi_addr_mask) {
+  if (m_high_memory_addr_bits != 0) {
+addr_t hi_addr_mask = ~((1ULL << m_high_memory_addr_bits) - 1);
 process.SetHighmemCodeAddressMask(hi_addr_mask);
 process.SetHighmemDataAddressMask(hi_addr_mask);
   }



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

[Lldb-commits] [PATCH] D158205: [lldb] Fix performance regression after adding GNUstep ObjC runtime

2023-08-17 Thread Jim Ingham via Phabricator via lldb-commits 
jingham accepted this revision.
jingham added inline comments.



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp:49
+  if (TT.isOSBinFormatELF())
+return filename.starts_with("libobjc.so");
+  if (TT.isOSWindows())

bulbazord wrote:
> jasonmolenda wrote:
> > theraven wrote:
> > > This is a bit unfortunate.  I know some downstream users that link the 
> > > Objective-C runtime components into another .so, so we can't really rely 
> > > on the name.  It would be nice if there were some mechanism for the user 
> > > to specify the name of the runtime if they're using something 
> > > non-standard.
> > If the runtime was merged in to a library with a known name, we could 
> > search for a name in the runtime (if it were the Darwin runtime, 
> > objc_msgSend would be a good candidate) in the list of libraries that might 
> > have the runtime in them.  Doing a "search for a symbol name in any binary 
> > that is added" is expensive, doing "search these three solibs for a symbol 
> > if they're loaded" is much less expensive, doing "is a solib with this name 
> > loaded" is free.
> That would be a nice follow-up!
If we can't know up front what library contains the runtime, we could also add 
a setting for the library name, so we don't have to guess.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158205

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

[Lldb-commits] [PATCH] D158205: [lldb] Fix performance regression after adding GNUstep ObjC runtime

2023-08-17 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord accepted this revision.
bulbazord added a comment.

This looks fine to me. Did you have the chance to verify that it improves 
performance of non-objc inferiors on Linux? We'll also probably want to make 
sure this gets back ported into llvm-17.




Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp:49
+  if (TT.isOSBinFormatELF())
+return filename.starts_with("libobjc.so");
+  if (TT.isOSWindows())

jasonmolenda wrote:
> theraven wrote:
> > This is a bit unfortunate.  I know some downstream users that link the 
> > Objective-C runtime components into another .so, so we can't really rely on 
> > the name.  It would be nice if there were some mechanism for the user to 
> > specify the name of the runtime if they're using something non-standard.
> If the runtime was merged in to a library with a known name, we could search 
> for a name in the runtime (if it were the Darwin runtime, objc_msgSend would 
> be a good candidate) in the list of libraries that might have the runtime in 
> them.  Doing a "search for a symbol name in any binary that is added" is 
> expensive, doing "search these three solibs for a symbol if they're loaded" 
> is much less expensive, doing "is a solib with this name loaded" is free.
That would be a nice follow-up!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158205

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

[Lldb-commits] [PATCH] D158205: [lldb] Fix performance regression after adding GNUstep ObjC runtime

2023-08-17 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda added inline comments.



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp:49
+  if (TT.isOSBinFormatELF())
+return filename.starts_with("libobjc.so");
+  if (TT.isOSWindows())

theraven wrote:
> This is a bit unfortunate.  I know some downstream users that link the 
> Objective-C runtime components into another .so, so we can't really rely on 
> the name.  It would be nice if there were some mechanism for the user to 
> specify the name of the runtime if they're using something non-standard.
If the runtime was merged in to a library with a known name, we could search 
for a name in the runtime (if it were the Darwin runtime, objc_msgSend would be 
a good candidate) in the list of libraries that might have the runtime in them. 
 Doing a "search for a symbol name in any binary that is added" is expensive, 
doing "search these three solibs for a symbol if they're loaded" is much less 
expensive, doing "is a solib with this name loaded" is free.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158205

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

[Lldb-commits] [PATCH] D158209: [lldb] Change UnixSignals::GetSignalAsCString to GetSignalAsStringRef

2023-08-17 Thread Alex Langford via Phabricator via lldb-commits 
bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, mib, jasonmolenda, jingham.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

This is in preparation to remove the uses of ConstString from
UnixSignals.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158209

Files:
  lldb/include/lldb/Target/UnixSignals.h
  lldb/source/API/SBUnixSignals.cpp
  lldb/source/Commands/CommandObjectProcess.cpp
  
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/source/Target/UnixSignals.cpp
  lldb/unittests/Signals/UnixSignalsTest.cpp

Index: lldb/unittests/Signals/UnixSignalsTest.cpp
===
--- lldb/unittests/Signals/UnixSignalsTest.cpp
+++ lldb/unittests/Signals/UnixSignalsTest.cpp
@@ -84,7 +84,7 @@
 
   bool should_suppress = false, should_stop = false, should_notify = false;
   int32_t signo = 4;
-  std::string name =
+  llvm::StringRef name =
   signals.GetSignalInfo(signo, should_suppress, should_stop, should_notify);
   EXPECT_EQ("SIG4", name);
   EXPECT_EQ(true, should_suppress);
@@ -94,15 +94,14 @@
   EXPECT_EQ(true, signals.GetShouldSuppress(signo));
   EXPECT_EQ(false, signals.GetShouldStop(signo));
   EXPECT_EQ(true, signals.GetShouldNotify(signo));
-  EXPECT_EQ(name, signals.GetSignalAsCString(signo));
+  EXPECT_EQ(name, signals.GetSignalAsStringRef(signo));
 }
 
-TEST(UnixSignalsTest, GetAsCString) {
+TEST(UnixSignalsTest, GetAsStringRef) {
   TestSignals signals;
 
-  ASSERT_EQ(nullptr, signals.GetSignalAsCString(100));
-  std::string name = signals.GetSignalAsCString(16);
-  ASSERT_EQ("SIG16", name);
+  ASSERT_EQ(llvm::StringRef(), signals.GetSignalAsStringRef(100));
+  ASSERT_EQ("SIG16", signals.GetSignalAsStringRef(16));
 }
 
 TEST(UnixSignalsTest, GetAsString) {
Index: lldb/source/Target/UnixSignals.cpp
===
--- lldb/source/Target/UnixSignals.cpp
+++ lldb/source/Target/UnixSignals.cpp
@@ -131,12 +131,11 @@
   ++m_version;
 }
 
-const char *UnixSignals::GetSignalAsCString(int signo) const {
-  collection::const_iterator pos = m_signals.find(signo);
+llvm::StringRef UnixSignals::GetSignalAsStringRef(int32_t signo) const {
+  const auto pos = m_signals.find(signo);
   if (pos == m_signals.end())
-return nullptr;
-  else
-return pos->second.m_name.GetCString();
+return {};
+  return pos->second.m_name.GetStringRef();
 }
 
 std::string
Index: lldb/source/Target/StopInfo.cpp
===
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -1067,9 +1067,9 @@
   thread_sp->GetProcess()->GetUnixSignals()->GetShouldNotify(m_value);
   if (should_notify) {
 StreamString strm;
-strm.Printf(
-"thread %d received signal: %s", thread_sp->GetIndexID(),
-thread_sp->GetProcess()->GetUnixSignals()->GetSignalAsCString(
+strm.Format(
+"thread {0:d} received signal: {1}", thread_sp->GetIndexID(),
+thread_sp->GetProcess()->GetUnixSignals()->GetSignalAsStringRef(
 m_value));
 Process::ProcessEventData::AddRestartedReason(event_ptr,
   strm.GetData());
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -1124,11 +1124,9 @@
 if (target_sp) {
   ProcessSP process_sp(target_sp->GetProcessSP());
   if (process_sp) {
-const char *signal_cstr = nullptr;
-if (signo)
-  signal_cstr = process_sp->GetUnixSignals()->GetSignalAsCString(signo);
-
-process_sp->SetExitStatus(exit_status, signal_cstr);
+llvm::StringRef signal_str =
+process_sp->GetUnixSignals()->GetSignalAsStringRef(signo);
+process_sp->SetExitStatus(exit_status, signal_str);
   }
 }
 return true;
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
@@ -3384,10 +3384,10 @@
   stream.Format(DEBUGSERVER_BASENAME " died with an exit status of {0:x8}",
 exit_status);
 else {
-  const char *signal_name =
-  process_sp->GetUnixSignals()->GetSignalAsCString(signo);
+  llvm::StringRef signal_name =
+  process_sp->GetUnixSignals()->GetSignalAsStringRef(signo);
   const char *format_str = DEBUGSERVER_BASENAME " died with signal {0}";
-  if (signal_nam

[Lldb-commits] [PATCH] D158205: [lldb] Fix performance regression after adding GNUstep ObjC runtime

2023-08-17 Thread David Chisnall via Phabricator via lldb-commits 
theraven added inline comments.



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp:49
+  if (TT.isOSBinFormatELF())
+return filename.starts_with("libobjc.so");
+  if (TT.isOSWindows())

This is a bit unfortunate.  I know some downstream users that link the 
Objective-C runtime components into another .so, so we can't really rely on the 
name.  It would be nice if there were some mechanism for the user to specify 
the name of the runtime if they're using something non-standard.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158205

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

[Lldb-commits] [PATCH] D158205: [lldb] Fix performance regression after adding GNUstep ObjC runtime

2023-08-17 Thread Jason Molenda via Phabricator via lldb-commits 
jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.

Looks good!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158205

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

[Lldb-commits] [PATCH] D158205: [lldb] Fix performance regression after adding GNUstep ObjC runtime

2023-08-17 Thread Stefan Gränitz via Phabricator via lldb-commits 
sgraenitz added a comment.
Herald added a subscriber: JDevlieghere.

The issue was reported in https://github.com/llvm/llvm-project/issues/64582


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158205

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

[Lldb-commits] [PATCH] D158205: [lldb] Fix performance regression after adding GNUstep ObjC runtime

2023-08-17 Thread Stefan Gränitz via Phabricator via lldb-commits 
sgraenitz created this revision.
sgraenitz added reviewers: jasonmolenda, jingham, bulbazord, aprantl.
Herald added a project: All.
sgraenitz requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

We added support for the GNUstep ObjC runtime in 0b6264738f3d 
. In order 
to check if the target process uses
GNUstep we run an expensive symbol lookup in `CreateInstance()`. This turned 
out to cause a heavy performance
regression for non-GNUstep inferiors.

This patch puts a cheaper check in front, so that the vast majority of requests 
should return early. This
should fix the symptom for the moment. The conceptual question is why 
`LanguageRuntime::FindPlugin` invokes 
`create_callback` for each available runtime unconditionally in every 
`Process::ModulesDidLoad`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158205

Files:
  
lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp


Index: 
lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp
===
--- 
lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp
+++ 
lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp
@@ -37,6 +37,33 @@
   PluginManager::UnregisterPlugin(CreateInstance);
 }
 
+static bool CanModuleBeGNUstepObjCLibrary(const ModuleSP &module_sp,
+  const llvm::Triple &TT) {
+  if (!module_sp)
+return false;
+  const FileSpec &module_file_spec = module_sp->GetFileSpec();
+  if (!module_file_spec)
+return false;
+  llvm::StringRef filename = module_file_spec.GetFilename().GetStringRef();
+  if (TT.isOSBinFormatELF())
+return filename.starts_with("libobjc.so");
+  if (TT.isOSWindows())
+return filename == "objc.dll";
+  return false;
+}
+
+static bool ScanForGNUstepObjCLibraryCandidate(const ModuleList &modules,
+   const llvm::Triple &TT) {
+  std::lock_guard guard(modules.GetMutex());
+  size_t num_modules = modules.GetSize();
+  for (size_t i = 0; i < num_modules; i++) {
+auto mod = modules.GetModuleAtIndex(i);
+if (CanModuleBeGNUstepObjCLibrary(mod, TT))
+  return true;
+  }
+  return false;
+}
+
 LanguageRuntime *GNUstepObjCRuntime::CreateInstance(Process *process,
 LanguageType language) {
   if (language != eLanguageTypeObjC)
@@ -50,6 +77,9 @@
 return nullptr;
 
   const ModuleList &images = target.GetImages();
+  if (!ScanForGNUstepObjCLibraryCandidate(images, TT))
+return nullptr;
+
   if (TT.isOSBinFormatELF()) {
 SymbolContextList eh_pers;
 RegularExpression regex("__gnustep_objc[x]*_personality_v[0-9]+");
@@ -176,18 +206,8 @@
 }
 
 bool GNUstepObjCRuntime::IsModuleObjCLibrary(const ModuleSP &module_sp) {
-  if (!module_sp)
-return false;
-  const FileSpec &module_file_spec = module_sp->GetFileSpec();
-  if (!module_file_spec)
-return false;
-  llvm::StringRef filename = module_file_spec.GetFilename().GetStringRef();
   const llvm::Triple &TT = GetTargetRef().GetArchitecture().GetTriple();
-  if (TT.isOSBinFormatELF())
-return filename.starts_with("libobjc.so");
-  if (TT.isOSWindows())
-return filename == "objc.dll";
-  return false;
+  return CanModuleBeGNUstepObjCLibrary(module_sp, TT);
 }
 
 bool GNUstepObjCRuntime::ReadObjCLibrary(const ModuleSP &module_sp) {


Index: lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp
@@ -37,6 +37,33 @@
   PluginManager::UnregisterPlugin(CreateInstance);
 }
 
+static bool CanModuleBeGNUstepObjCLibrary(const ModuleSP &module_sp,
+  const llvm::Triple &TT) {
+  if (!module_sp)
+return false;
+  const FileSpec &module_file_spec = module_sp->GetFileSpec();
+  if (!module_file_spec)
+return false;
+  llvm::StringRef filename = module_file_spec.GetFilename().GetStringRef();
+  if (TT.isOSBinFormatELF())
+return filename.starts_with("libobjc.so");
+  if (TT.isOSWindows())
+return filename == "objc.dll";
+  return false;
+}
+
+static bool ScanForGNUstepObjCLibraryCandidate(const ModuleList &modules,
+   const llvm::Triple &TT) {
+  std::lock_guard guard(modules.GetMutex());
+  size_t num_modules = modules.GetSize();
+  for (size_t i = 0; i < num_modules; i++) {
+auto mod = modules.GetModuleAtIndex(i);
+if (CanModuleBeGNUstepObjCLibrary(mod, TT))
+  return true;
+  }
+  return false;
+}
+
 LanguageRuntime *GNUstepObjCRuntime

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

2023-08-17 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg accepted this revision.
clayborg added a comment.

Looks good


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] D157609: [lldb] Search debug file paths when looking for split DWARF files

2023-08-17 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg added a comment.

Just one question if we should still try all of your new code if we have a full 
path to a DW_AT_comp_dir, but we don't find the .dwo file, should be allow your 
code to run and still try to just append the relative "dwo_name" (without comp 
dir) to each of the search paths?




Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1755
+  dwo_file.AppendPathComponent(dwo_name);
+  found = FileSystem::Instance().Exists(dwo_file);
+} else {

If "found == false" here, do we want to fall through to the "else" clause 
below? This would mean we have a full path to a DW_AT_comp_dir, but we didn't 
find the .dwo file and "dwo_name" is relative, so maybe we should try to use 
the relative "dwo_name" below without the comp dir if not found?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1770-1771
+  // Or it's relative to one of the user specified debug directories.
+  const FileSpecList &debug_file_search_paths =
+  Target::GetDefaultDebugFileSearchPaths();
+  size_t num_directories = debug_file_search_paths.GetSize();

On major issue we have here is if someone creates a target first, then sets the 
debug file search paths, then this will always fail because 
Target::GetDefaultDebugFileSearchPaths() gets the global target settings. This 
is because before we have a target, if any target settings get set, they get 
set in this global target settings object. Then each time you create a target, 
it will get a copy of these global settings. So this would work:
```
(lldb) settings set target.debug-file-search-paths /my/objfiles
# Global target settings are set above
(lldb) target create a.out
(lldb) run
...
```

This wouldn't:
```
(lldb) target create a.out
(lldb) settings set target.debug-file-search-paths /my/objfiles
# Current target settings are set above, not the global settings
(lldb) run
...
```
Because the global settings have nothing for the 
"target.debug-file-search-paths", but the current lldb_private::Target that was 
created by "target create a.out" does have it set, but that isn't what we are 
getting here.

We don't have access to the current target anywhere in lldb_private::Module and 
its lldb_private::ObjectFile or lldb_private::SymbolFile because 
lldb_private::Module can be used in multiple targets, so the module doesn't 
know what target it is in. This isn't something that needs to be solved here, 
but we should understand the limitations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157609

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

[Lldb-commits] [PATCH] D158182: [lldb] Try to find relative DWO files by file name only, as a last resort

2023-08-17 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1784-1804
+  FileSpec spec_to_get_name(dwo_name);
+  llvm::StringRef filename_only = spec_to_get_name.GetFilename();
+
+  // Try binary dir plus DWO filename only.
+  FileSpec filename_next_to_binary(
+  m_objfile_sp->GetFileSpec().GetDirectory().GetStringRef());
+  FileSystem::Instance().Resolve(filename_next_to_binary);

Would it be better to move this out of the "if (!dwo_file.IsRelative())" and 
else statement?



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1815
   }
 
   if (!found) {

Maybe move it here? That way even if we have a non-relative path to a .dwo 
file, it could find it locally or in the search paths?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158182

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

[Lldb-commits] [PATCH] D158026: [lldb][NFCI] Remove unneeded ConstString from ValueObject::GetValueForExpressionPath_Impl

2023-08-17 Thread Alex Langford via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3415798f7993: [lldb][NFCI] Remove unneeded ConstString from 
ValueObject… (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158026

Files:
  lldb/source/Core/ValueObject.cpp


Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -2131,11 +2131,10 @@
   temp_expression = temp_expression.drop_front(); // skip . or >
 
   size_t next_sep_pos = temp_expression.find_first_of("-.[", 1);
-  ConstString child_name;
   if (next_sep_pos == llvm::StringRef::npos) // if no other separator just
  // expand this last layer
   {
-child_name.SetString(temp_expression);
+llvm::StringRef child_name = temp_expression;
 ValueObjectSP child_valobj_sp =
 root->GetChildMemberWithName(child_name);
 
@@ -2203,8 +2202,7 @@
   } else // other layers do expand
   {
 llvm::StringRef next_separator = temp_expression.substr(next_sep_pos);
-
-child_name.SetString(temp_expression.slice(0, next_sep_pos));
+llvm::StringRef child_name = temp_expression.slice(0, next_sep_pos);
 
 ValueObjectSP child_valobj_sp =
 root->GetChildMemberWithName(child_name);


Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -2131,11 +2131,10 @@
   temp_expression = temp_expression.drop_front(); // skip . or >
 
   size_t next_sep_pos = temp_expression.find_first_of("-.[", 1);
-  ConstString child_name;
   if (next_sep_pos == llvm::StringRef::npos) // if no other separator just
  // expand this last layer
   {
-child_name.SetString(temp_expression);
+llvm::StringRef child_name = temp_expression;
 ValueObjectSP child_valobj_sp =
 root->GetChildMemberWithName(child_name);
 
@@ -2203,8 +2202,7 @@
   } else // other layers do expand
   {
 llvm::StringRef next_separator = temp_expression.substr(next_sep_pos);
-
-child_name.SetString(temp_expression.slice(0, next_sep_pos));
+llvm::StringRef child_name = temp_expression.slice(0, next_sep_pos);
 
 ValueObjectSP child_valobj_sp =
 root->GetChildMemberWithName(child_name);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 3415798 - [lldb][NFCI] Remove unneeded ConstString from ValueObject::GetValueForExpressionPath_Impl

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

Author: Alex Langford
Date: 2023-08-17T10:44:01-07:00
New Revision: 3415798f7993974a19bd22b0481f2f6a71e4a2ab

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

LOG: [lldb][NFCI] Remove unneeded ConstString from 
ValueObject::GetValueForExpressionPath_Impl

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

Added: 


Modified: 
lldb/source/Core/ValueObject.cpp

Removed: 




diff  --git a/lldb/source/Core/ValueObject.cpp 
b/lldb/source/Core/ValueObject.cpp
index 37d32e807fdbc2..ed9e26aad0f30b 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -2131,11 +2131,10 @@ ValueObjectSP 
ValueObject::GetValueForExpressionPath_Impl(
   temp_expression = temp_expression.drop_front(); // skip . or >
 
   size_t next_sep_pos = temp_expression.find_first_of("-.[", 1);
-  ConstString child_name;
   if (next_sep_pos == llvm::StringRef::npos) // if no other separator just
  // expand this last layer
   {
-child_name.SetString(temp_expression);
+llvm::StringRef child_name = temp_expression;
 ValueObjectSP child_valobj_sp =
 root->GetChildMemberWithName(child_name);
 
@@ -2203,8 +2202,7 @@ ValueObjectSP ValueObject::GetValueForExpressionPath_Impl(
   } else // other layers do expand
   {
 llvm::StringRef next_separator = temp_expression.substr(next_sep_pos);
-
-child_name.SetString(temp_expression.slice(0, next_sep_pos));
+llvm::StringRef child_name = temp_expression.slice(0, next_sep_pos);
 
 ValueObjectSP child_valobj_sp =
 root->GetChildMemberWithName(child_name);



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

[Lldb-commits] [PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

2023-08-17 Thread Michał Górny via Phabricator via lldb-commits 
mgorny added a comment.

This is the final version that was pushed, correct? (the commits don't link the 
revision)

I've raised some concerns about the resulting API in 
https://reviews.llvm.org/D156270#4557902. I'd appreciate if you could take a 
look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141907

___
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

2023-08-17 Thread Alex Langford via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9e6d48ef6085: [lldb][NFCI] Module constructor should take 
ConstString by value (authored by bulbazord).

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 &exe_module_sp, uint32_t cu_idx,
  const FileSpec &file_spec, const ArchSpec &arch,
- 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 ? &oso_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 &file_spec, const ArchSpec &arch,
-   const ConstString *object_name, lldb::offset_t object_offset,
+   ConstString object_name, lldb::offset_t object_offset,
const llvm::sys::TimePoint<> &object_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 &file_spec, const ArchSpec &arch,
-  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<> &object_mod_time = 
llvm::sys::TimePoint<>());
 
   Module(const ModuleSpec &module_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 &exe_module_sp, uint32_t cu_idx,
  const FileSpec &file_spec, const ArchSpec &arch,
- 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 ? &oso_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/Mo

[Lldb-commits] [lldb] 9e6d48e - [lldb][NFCI] Module constructor should take ConstString by value

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

Author: Alex Langford
Date: 2023-08-17T10:34:57-07:00
New Revision: 9e6d48ef6085b762d8eb2531bbb481f57446da1c

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

LOG: [lldb][NFCI] Module constructor should take ConstString by value

ConstStrings are super cheap to copy around. It is often more expensive
to pass a pointer and potentially dereference it than just to always copy it.

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

Added: 


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

Removed: 




diff  --git a/lldb/include/lldb/Core/Module.h b/lldb/include/lldb/Core/Module.h
index 67e15120f5e786..51f01305b7d50f 100644
--- a/lldb/include/lldb/Core/Module.h
+++ b/lldb/include/lldb/Core/Module.h
@@ -124,8 +124,7 @@ class Module : public std::enable_shared_from_this,
   /// multiple architectures).
   Module(
   const FileSpec &file_spec, const ArchSpec &arch,
-  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<> &object_mod_time = 
llvm::sys::TimePoint<>());
 
   Module(const ModuleSpec &module_spec);

diff  --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 4b74929d671467..96c1411010b073 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -233,12 +233,12 @@ Module::Module(const ModuleSpec &module_spec)
 }
 
 Module::Module(const FileSpec &file_spec, const ArchSpec &arch,
-   const ConstString *object_name, lldb::offset_t object_offset,
+   ConstString object_name, lldb::offset_t object_offset,
const llvm::sys::TimePoint<> &object_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 @@ Module::Module(const FileSpec &file_spec, const ArchSpec 
&arch,
 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')",

diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
index 59b8f167f7c611..eadedd32e1a4aa 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -170,7 +170,7 @@ class DebugMapModule : public Module {
 public:
   DebugMapModule(const ModuleSP &exe_module_sp, uint32_t cu_idx,
  const FileSpec &file_spec, const ArchSpec &arch,
- 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 @@ Module *SymbolFileDWARFDebugMap::GetModuleByCompUnitInfo(
  .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 ? &oso_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() &&



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

[Lldb-commits] [lldb] 90c5675 - [lldb][NFCI] Rewrite error-handling code in ProcessGDBRemote::MonitorDebugserverProcess

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

Author: Alex Langford
Date: 2023-08-17T10:32:23-07:00
New Revision: 90c5675a3db2319dd449a1753c79568355dfaaed

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

LOG: [lldb][NFCI] Rewrite error-handling code in 
ProcessGDBRemote::MonitorDebugserverProcess

I'm rewriting this for 2 reasons:
1) Although a 1024 char buffer is probably enough space, the error
   string may grow beyond that and we could lose some information. Using
   StringStream (as we do in the rest of ProcessGDBRemote) seems like a
   sensible solution.
2) I am planning on changing `UnixSignals::GetSignalAsCString`,
   rewriting things with `llvm::formatv` will make that process easier.

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

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 7eb29ee7baa11b..68c343903f3dc8 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -3379,23 +3379,20 @@ void ProcessGDBRemote::MonitorDebugserverProcess(
 
   if (state != eStateInvalid && state != eStateUnloaded &&
   state != eStateExited && state != eStateDetached) {
-char error_str[1024];
-if (signo) {
-  const char *signal_cstr =
+StreamString stream;
+if (signo == 0)
+  stream.Format(DEBUGSERVER_BASENAME " died with an exit status of {0:x8}",
+exit_status);
+else {
+  const char *signal_name =
   process_sp->GetUnixSignals()->GetSignalAsCString(signo);
-  if (signal_cstr)
-::snprintf(error_str, sizeof(error_str),
-   DEBUGSERVER_BASENAME " died with signal %s", signal_cstr);
+  const char *format_str = DEBUGSERVER_BASENAME " died with signal {0}";
+  if (signal_name)
+stream.Format(format_str, signal_name);
   else
-::snprintf(error_str, sizeof(error_str),
-   DEBUGSERVER_BASENAME " died with signal %i", signo);
-} else {
-  ::snprintf(error_str, sizeof(error_str),
- DEBUGSERVER_BASENAME " died with an exit status of 0x%8.8x",
- exit_status);
+stream.Format(format_str, signo);
 }
-
-process_sp->SetExitStatus(-1, error_str);
+process_sp->SetExitStatus(-1, stream.GetString());
   }
   // Debugserver has exited we need to let our ProcessGDBRemote know that it no
   // longer has a debugserver instance



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

[Lldb-commits] [PATCH] D158017: [lldb][NFCI] Rewrite error-handling code in ProcessGDBRemote::MonitorDebugserverProcess

2023-08-17 Thread Alex Langford via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG90c5675a3db2: [lldb][NFCI] Rewrite error-handling code in 
ProcessGDBRemote… (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158017

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


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
@@ -3379,23 +3379,20 @@
 
   if (state != eStateInvalid && state != eStateUnloaded &&
   state != eStateExited && state != eStateDetached) {
-char error_str[1024];
-if (signo) {
-  const char *signal_cstr =
+StreamString stream;
+if (signo == 0)
+  stream.Format(DEBUGSERVER_BASENAME " died with an exit status of {0:x8}",
+exit_status);
+else {
+  const char *signal_name =
   process_sp->GetUnixSignals()->GetSignalAsCString(signo);
-  if (signal_cstr)
-::snprintf(error_str, sizeof(error_str),
-   DEBUGSERVER_BASENAME " died with signal %s", signal_cstr);
+  const char *format_str = DEBUGSERVER_BASENAME " died with signal {0}";
+  if (signal_name)
+stream.Format(format_str, signal_name);
   else
-::snprintf(error_str, sizeof(error_str),
-   DEBUGSERVER_BASENAME " died with signal %i", signo);
-} else {
-  ::snprintf(error_str, sizeof(error_str),
- DEBUGSERVER_BASENAME " died with an exit status of 0x%8.8x",
- exit_status);
+stream.Format(format_str, signo);
 }
-
-process_sp->SetExitStatus(-1, error_str);
+process_sp->SetExitStatus(-1, stream.GetString());
   }
   // Debugserver has exited we need to let our ProcessGDBRemote know that it no
   // longer has a debugserver instance


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
@@ -3379,23 +3379,20 @@
 
   if (state != eStateInvalid && state != eStateUnloaded &&
   state != eStateExited && state != eStateDetached) {
-char error_str[1024];
-if (signo) {
-  const char *signal_cstr =
+StreamString stream;
+if (signo == 0)
+  stream.Format(DEBUGSERVER_BASENAME " died with an exit status of {0:x8}",
+exit_status);
+else {
+  const char *signal_name =
   process_sp->GetUnixSignals()->GetSignalAsCString(signo);
-  if (signal_cstr)
-::snprintf(error_str, sizeof(error_str),
-   DEBUGSERVER_BASENAME " died with signal %s", signal_cstr);
+  const char *format_str = DEBUGSERVER_BASENAME " died with signal {0}";
+  if (signal_name)
+stream.Format(format_str, signal_name);
   else
-::snprintf(error_str, sizeof(error_str),
-   DEBUGSERVER_BASENAME " died with signal %i", signo);
-} else {
-  ::snprintf(error_str, sizeof(error_str),
- DEBUGSERVER_BASENAME " died with an exit status of 0x%8.8x",
- exit_status);
+stream.Format(format_str, signo);
 }
-
-process_sp->SetExitStatus(-1, error_str);
+process_sp->SetExitStatus(-1, stream.GetString());
   }
   // Debugserver has exited we need to let our ProcessGDBRemote know that it no
   // longer has a debugserver instance
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 9c3f1f4 - [lldb][ClangASTImporter][NFC] Remove redundant calls to ASTImporter::Imported

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

Author: Michael Buch
Date: 2023-08-17T17:54:51+01:00
New Revision: 9c3f1f42cbed0fa6184cd2047b34ab5090503e9e

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

LOG: [lldb][ClangASTImporter][NFC] Remove redundant calls to 
ASTImporter::Imported

The ASTImporter::Imported base method has been made a no-op in
26f72a96559f2acd6799c363f1ca88ef3238c601. So all calls to it from
a base-class are now redundant. The API is now only used to notify
subclasses that an import occurred and not for any other
bookkeeping (which is done in MapImported which we call properly).

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

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
index 2826b102625fe5..5d109feb3d39fa 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -902,7 +902,6 @@ void 
ClangASTImporter::ASTImporterDelegate::ImportDefinitionTo(
   // We want that 'to' is actually complete after this function so let's
   // tell the ASTImporter that 'to' was imported from 'from'.
   MapImported(from, to);
-  ASTImporter::Imported(from, to);
 
   Log *log = GetLog(LLDBLog::Expressions);
 
@@ -1028,7 +1027,7 @@ void 
ClangASTImporter::ASTImporterDelegate::Imported(clang::Decl *from,
   // Some decls shouldn't be tracked here because they were not created by
   // copying 'from' to 'to'. Just exit early for those.
   if (m_decls_to_ignore.count(to))
-return clang::ASTImporter::Imported(from, to);
+return;
 
   // Transfer module ownership information.
   auto *from_source = llvm::dyn_cast_or_null(
@@ -1081,12 +1080,6 @@ void 
ClangASTImporter::ASTImporterDelegate::Imported(clang::Decl *from,
 if (!to_context_md->hasOrigin(to) || user_id != LLDB_INVALID_UID)
   to_context_md->setOrigin(to, origin);
 
-ImporterDelegateSP direct_completer =
-m_main.GetDelegate(&to->getASTContext(), origin.ctx);
-
-if (direct_completer.get() != this)
-  direct_completer->ASTImporter::Imported(origin.decl, to);
-
 LLDB_LOG(log,
  "[ClangASTImporter] Propagated origin "
  "(Decl*){0}/(ASTContext*){1} from (ASTContext*){2} to "



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

[Lldb-commits] [PATCH] D158172: [lldb][ClangASTImporter][NFC] Remove redundant calls to ASTImporter::Imported

2023-08-17 Thread Michael Buch via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9c3f1f42cbed: [lldb][ClangASTImporter][NFC] Remove redundant 
calls to ASTImporter::Imported (authored by Michael137).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158172

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -902,7 +902,6 @@
   // We want that 'to' is actually complete after this function so let's
   // tell the ASTImporter that 'to' was imported from 'from'.
   MapImported(from, to);
-  ASTImporter::Imported(from, to);
 
   Log *log = GetLog(LLDBLog::Expressions);
 
@@ -1028,7 +1027,7 @@
   // Some decls shouldn't be tracked here because they were not created by
   // copying 'from' to 'to'. Just exit early for those.
   if (m_decls_to_ignore.count(to))
-return clang::ASTImporter::Imported(from, to);
+return;
 
   // Transfer module ownership information.
   auto *from_source = llvm::dyn_cast_or_null(
@@ -1081,12 +1080,6 @@
 if (!to_context_md->hasOrigin(to) || user_id != LLDB_INVALID_UID)
   to_context_md->setOrigin(to, origin);
 
-ImporterDelegateSP direct_completer =
-m_main.GetDelegate(&to->getASTContext(), origin.ctx);
-
-if (direct_completer.get() != this)
-  direct_completer->ASTImporter::Imported(origin.decl, to);
-
 LLDB_LOG(log,
  "[ClangASTImporter] Propagated origin "
  "(Decl*){0}/(ASTContext*){1} from (ASTContext*){2} to "


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -902,7 +902,6 @@
   // We want that 'to' is actually complete after this function so let's
   // tell the ASTImporter that 'to' was imported from 'from'.
   MapImported(from, to);
-  ASTImporter::Imported(from, to);
 
   Log *log = GetLog(LLDBLog::Expressions);
 
@@ -1028,7 +1027,7 @@
   // Some decls shouldn't be tracked here because they were not created by
   // copying 'from' to 'to'. Just exit early for those.
   if (m_decls_to_ignore.count(to))
-return clang::ASTImporter::Imported(from, to);
+return;
 
   // Transfer module ownership information.
   auto *from_source = llvm::dyn_cast_or_null(
@@ -1081,12 +1080,6 @@
 if (!to_context_md->hasOrigin(to) || user_id != LLDB_INVALID_UID)
   to_context_md->setOrigin(to, origin);
 
-ImporterDelegateSP direct_completer =
-m_main.GetDelegate(&to->getASTContext(), origin.ctx);
-
-if (direct_completer.get() != this)
-  direct_completer->ASTImporter::Imported(origin.decl, to);
-
 LLDB_LOG(log,
  "[ClangASTImporter] Propagated origin "
  "(Decl*){0}/(ASTContext*){1} from (ASTContext*){2} to "
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 4139461 - [lldb] Fix REQUIRES for DWO relative path test

2023-08-17 Thread David Spickett via lldb-commits 

Author: David Spickett
Date: 2023-08-17T14:19:27Z
New Revision: 4139461d4e7788128c2da5f77712b8d39898f189

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

LOG: [lldb] Fix REQUIRES for DWO relative path test

This was "x86-registered-target" which seems to be false in this test
suite despite me having the x86 backend enabled. The other tests use just "x86"
and with that the test passes on my AArch64 machine fine.

Added: 


Modified: 
lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s 
b/lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s
index 58c61a4b577a60..f3d44576532bf0 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s
+++ b/lldb/test/Shell/SymbolFile/DWARF/dwo-relative-path.s
@@ -1,7 +1,7 @@
 # Test to verify LLDB searches for dwos with relative paths relative to the
 # binary location, not relative to LLDB's launch location.
 
-# REQUIRES: x86-registered-target
+# REQUIRES: x86
 
 # RUN: llvm-mc --filetype=obj --triple x86_64-pc-linux %s -o %t.o
 # RUN: llvm-objcopy --split-dwo=%T/dwo-relative-path.dwo %t.o



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

[Lldb-commits] [PATCH] D158182: [lldb] Try to find relative DWO files by file name only, as a last resort

2023-08-17 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.

@clayborg This is I think what you meant. I'm not 100% sure it's safe because I 
keep wondering if we could find the wrong files this way, but with 1 DWO file 
per program it seems unlikely.

I've reduced the nesting in the new tests compared to the previous patch. Also 
this version builds all the possible paths first, then tries them all. I can 
try to rework that so we do it as we go, but tell me what you think of the 
functionality first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158182

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

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

2023-08-17 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett marked an inline comment as done.
DavidSpickett added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1737
+  if (!dwo_file.IsRelative()) {
+found = FileSystem::Instance().Exists(dwo_file);
+  } else {

DavidSpickett wrote:
> clayborg wrote:
> > Maybe we can also check the debug info search paths to see if they contain 
> > just the basename in case someone sends you a binary and any needed .dwo 
> > files?
> > 
> > So if we have a DWO path of "/users/someone/build/foo.dwo", we would check 
> > each of the debug info search paths for:
> > ```
> > path + dwo_file.GetFilename()
> > ```
> > This would allow someone to send a binary to someone else and allow them to 
> > load it up.
> > 
> > But any such search might belong down below this entire if/else statement 
> > when the file isn't found:
> > 
> > ```
> > if (!found) {
> >   // Search for debug info path + basename...
> > }
> > ```
> Good idea, I'll write a test for that scenario and see if it makes sense here 
> or in a follow up.
https://reviews.llvm.org/D158182


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157609

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

[Lldb-commits] [PATCH] D158182: [lldb] Try to find relative DWO files by file name only, as a last resort

2023-08-17 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett created this revision.
Herald added a project: All.
DavidSpickett requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: lldb-commits, jplehr, sstefan1.
Herald added a project: LLDB.

Relative DWO files are normally found at some/path/to/file.dwo.
Currently lldb will find the file by adding that path to the
binary's location and to any debug search paths that are set.

This patch adds a fallback on top of that where lldb will look
for just the filename (file.dwo) in those same places.

So if you had flattened a build directory before distribution,
you could still get symbols. Or perhaps you send out a DWO file
on demand, and don't want people to have to recreate the directory
layout.

For example if you were built:
program
obj/program.dwo

And changed to:
program
program.dwo

/obj/program.dwo is not found.
/program.dwo is.

If the layout was changed to:
program
mydebuginfo/progam.dwo

And we had a debug search path of /mydebuginfo:

- /mydebuginfo/obj/program.dwo is not found.
- /mydebuginfo/program.dwo is found.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158182

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/SymbolFile/DWARF/dwo-debug-file-search-paths-filename-only.c
  lldb/test/Shell/SymbolFile/DWARF/dwo-relative-filename-only-binary-dir.c


Index: lldb/test/Shell/SymbolFile/DWARF/dwo-relative-filename-only-binary-dir.c
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/dwo-relative-filename-only-binary-dir.c
@@ -0,0 +1,22 @@
+/// Check that LLDB can find a relative DWO file next to a binary just using 
the
+/// filename of that DWO. For example "main.dwo" not "a/b/main.dwo".
+// RUN: rm -rf %t.compdir/
+// RUN: mkdir -p %t.compdir/a/b/
+// RUN: cp %s %t.compdir/a/b/main.c
+// RUN: cd %t.compdir/a/
+/// The produced DWO is named b/main-main.dwo, with a DW_AT_comp_dir of a/.
+// RUN: %clang_host -g -gsplit-dwarf -fdebug-prefix-map=%t.compdir=. b/main.c 
-o b/main
+// RUN: cd ../..
+/// Move binary and DWO out of expected locations.
+// RUN: mv %t.compdir/a/b/main %t.compdir/
+// RUN: mv %t.compdir/a/b/main-main.dwo %t.compdir/
+// RUN: %lldb --no-lldbinit %t.compdir/main \
+// RUN:   -o "b main.c:21" -o "run" -o "frame variable" --batch 2>&1 | 
FileCheck %s
+
+// CHECK-NOT: warning: {{.*}}main unable to locate separate debug file (dwo, 
dwp). Debugging will be degraded.
+// CHECK: (int) num = 5
+
+int main(void) {
+  int num = 5;
+  return 0;
+}
\ No newline at end of file
Index: 
lldb/test/Shell/SymbolFile/DWARF/dwo-debug-file-search-paths-filename-only.c
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/dwo-debug-file-search-paths-filename-only.c
@@ -0,0 +1,27 @@
+/// Check that when LLDB is looking for a relative DWO it uses the debug search
+/// paths setting. If it doesn't find it by adding the whole relative path to
+/// of DWO it should try adding just the filename (e.g. main.dwo) to each debug
+/// search path.
+// RUN: rm -rf %t.compdir/
+// RUN: mkdir -p %t.compdir/a/b/
+// RUN: cp %s %t.compdir/a/b/main.c
+// RUN: cd %t.compdir/a/
+/// The produced DWO is named /b//main-main.dwo, with a DW_AT_comp_dir of a/.
+// RUN: %clang_host -g -gsplit-dwarf -fdebug-prefix-map=%t.compdir=. b/main.c 
-o b/main
+// RUN: cd ../..
+/// Move the DWO file away from the expected location.
+// RUN: mv %t.compdir/a/b/main-main.dwo %t.compdir/
+/// LLDB won't find the DWO next to the binary or by adding the relative path
+/// to any of the search paths. So it should find the DWO file at
+/// %t.compdir/main-main.dwo.
+// RUN: %lldb --no-lldbinit %t.compdir/a/b/main \
+// RUN:   -O "settings append target.debug-file-search-paths %t.compdir" \
+// RUN:   -o "b main.c:26" -o "run" -o "frame variable" --batch 2>&1 | 
FileCheck %s
+
+// CHECK-NOT: warning: {{.*}}main unable to locate separate debug file (dwo, 
dwp). Debugging will be degraded.
+// CHECK: (int) num = 5
+
+int main(void) {
+  int num = 5;
+  return 0;
+}
\ No newline at end of file
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1781,6 +1781,27 @@
 dwo_paths.Append(dirspec);
   }
 
+  FileSpec spec_to_get_name(dwo_name);
+  llvm::StringRef filename_only = spec_to_get_name.GetFilename();
+
+  // Try binary dir plus DWO filename only.
+  FileSpec filename_next_to_binary(
+  m_objfile_sp->GetFileSpec().GetDirectory().GetStringRef());
+  FileSystem::Instance().Resolve(filename_next_to_binary);
+  filename_next_to_binary.AppendPathComponent(filename_only);
+  dwo_paths.Append(filename_next_to_binary);
+
+  // Try adding just the filename to each of the se

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

2023-08-17 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett updated this revision to Diff 551122.
DavidSpickett added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157609

Files:
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/SymbolFile/DWARF/dwo-debug-file-search-paths.c

Index: lldb/test/Shell/SymbolFile/DWARF/dwo-debug-file-search-paths.c
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/dwo-debug-file-search-paths.c
@@ -0,0 +1,28 @@
+/// Check that LLDB uses the paths in target.debug-file-search-paths to find
+/// split DWARF files with DW_AT_comp_dir set, when the program file has been
+/// moved and/or we're executing it from another directory.
+// RUN: rm -rf %t.compdir/ %t.e/
+// RUN: mkdir -p %t.compdir/a/b/c/d/
+// RUN: cp %s %t.compdir/a/b/c/d/main.c
+// RUN: cd %t.compdir/a/b/
+/// The produced DWO is named c/d/main-main.dwo, with a DW_AT_comp_dir of a/b.
+// RUN: %clang_host -g -gsplit-dwarf -fdebug-prefix-map=%t.compdir=. c/d/main.c -o c/d/main
+// RUN: cd ../../..
+/// Move only the program, leaving the DWO file in place.
+// RUN: mv %t.compdir/a/b/c/d/main %t.compdir/a/
+/// Debug it from yet another path.
+// RUN: mkdir -p %t.e/
+// RUN: cd %t.e
+/// LLDB won't find the DWO next to the binary or in the current dir, so it
+/// should find the DWO file by doing %t.compdir/ + a/b/ + c/d/main-main.dwo.
+// RUN: %lldb --no-lldbinit %t.compdir/a/main \
+// RUN:   -O "settings append target.debug-file-search-paths %t.compdir" \
+// RUN:   -o "b main.c:27" -o "run" -o "frame variable" --batch 2>&1 | FileCheck %s
+
+// CHECK-NOT: warning: {{.*}}main unable to locate separate debug file (dwo, dwp). Debugging will be degraded.
+// CHECK: (int) num = 5
+
+int main(void) {
+  int num = 5;
+  return 0;
+}
\ No newline at end of file
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1731,7 +1731,11 @@
   const char *comp_dir = nullptr;
   FileSpec dwo_file(dwo_name);
   FileSystem::Instance().Resolve(dwo_file);
-  if (dwo_file.IsRelative()) {
+  bool found = false;
+
+  if (!dwo_file.IsRelative()) {
+found = FileSystem::Instance().Exists(dwo_file);
+  } else {
 comp_dir = cu_die.GetAttributeValueAsString(dwarf_cu, DW_AT_comp_dir,
 nullptr);
 if (!comp_dir) {
@@ -1744,18 +1748,51 @@
 }
 
 dwo_file.SetFile(comp_dir, FileSpec::Style::native);
-if (dwo_file.IsRelative()) {
+
+if (!dwo_file.IsRelative()) {
+  FileSystem::Instance().Resolve(dwo_file);
+  dwo_file.AppendPathComponent(dwo_name);
+  found = FileSystem::Instance().Exists(dwo_file);
+} else {
+  FileSpecList dwo_paths;
+
   // if DW_AT_comp_dir is relative, it should be relative to the location
   // of the executable, not to the location from which the debugger was
   // launched.
-  dwo_file.PrependPathComponent(
+  FileSpec relative_to_binary = dwo_file;
+  relative_to_binary.PrependPathComponent(
   m_objfile_sp->GetFileSpec().GetDirectory().GetStringRef());
+  FileSystem::Instance().Resolve(relative_to_binary);
+  relative_to_binary.AppendPathComponent(dwo_name);
+  dwo_paths.Append(relative_to_binary);
+
+  // Or it's relative to one of the user specified debug directories.
+  const FileSpecList &debug_file_search_paths =
+  Target::GetDefaultDebugFileSearchPaths();
+  size_t num_directories = debug_file_search_paths.GetSize();
+  for (size_t idx = 0; idx < num_directories; ++idx) {
+FileSpec dirspec = debug_file_search_paths.GetFileSpecAtIndex(idx);
+dirspec.AppendPathComponent(comp_dir);
+FileSystem::Instance().Resolve(dirspec);
+if (!FileSystem::Instance().IsDirectory(dirspec))
+  continue;
+
+dirspec.AppendPathComponent(dwo_name);
+dwo_paths.Append(dirspec);
+  }
+
+  size_t num_possible = dwo_paths.GetSize();
+  for (size_t idx = 0; idx < num_possible && !found; ++idx) {
+FileSpec dwo_spec = dwo_paths.GetFileSpecAtIndex(idx);
+if (FileSystem::Instance().Exists(dwo_spec)) {
+  dwo_file = dwo_spec;
+  found = true;
+}
+  }
 }
-FileSystem::Instance().Resolve(dwo_file);
-dwo_file.AppendPathComponent(dwo_name);
   }
 
-  if (!FileSystem::Instance().Exists(dwo_file)) {
+  if (!found) {
 unit.SetDwoError(Status::createWithFormat(
 "unable to locate .dwo debug file \"{0}\" for skeleton DIE "
 "{1:x16}",
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D149213: [lldb] Add basic support for Rust enums in TypeSystemClang

2023-08-17 Thread Vladimir Makaev via Phabricator via lldb-commits 
VladimirMakaev added a comment.

Thanks @DavidSpickett for fixing this. I did not intend to use that type. Any 
number of that size would work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149213

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

[Lldb-commits] [PATCH] D158172: [lldb][ClangASTImporter][NFC] Remove redundant calls to ASTImporter::Imported

2023-08-17 Thread Michael Buch via Phabricator via lldb-commits 
Michael137 updated this revision to Diff 551086.
Michael137 added a comment.

- Reword commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158172

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -902,7 +902,6 @@
   // We want that 'to' is actually complete after this function so let's
   // tell the ASTImporter that 'to' was imported from 'from'.
   MapImported(from, to);
-  ASTImporter::Imported(from, to);
 
   Log *log = GetLog(LLDBLog::Expressions);
 
@@ -1028,7 +1027,7 @@
   // Some decls shouldn't be tracked here because they were not created by
   // copying 'from' to 'to'. Just exit early for those.
   if (m_decls_to_ignore.count(to))
-return clang::ASTImporter::Imported(from, to);
+return;
 
   // Transfer module ownership information.
   auto *from_source = llvm::dyn_cast_or_null(
@@ -1081,12 +1080,6 @@
 if (!to_context_md->hasOrigin(to) || user_id != LLDB_INVALID_UID)
   to_context_md->setOrigin(to, origin);
 
-ImporterDelegateSP direct_completer =
-m_main.GetDelegate(&to->getASTContext(), origin.ctx);
-
-if (direct_completer.get() != this)
-  direct_completer->ASTImporter::Imported(origin.decl, to);
-
 LLDB_LOG(log,
  "[ClangASTImporter] Propagated origin "
  "(Decl*){0}/(ASTContext*){1} from (ASTContext*){2} to "


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -902,7 +902,6 @@
   // We want that 'to' is actually complete after this function so let's
   // tell the ASTImporter that 'to' was imported from 'from'.
   MapImported(from, to);
-  ASTImporter::Imported(from, to);
 
   Log *log = GetLog(LLDBLog::Expressions);
 
@@ -1028,7 +1027,7 @@
   // Some decls shouldn't be tracked here because they were not created by
   // copying 'from' to 'to'. Just exit early for those.
   if (m_decls_to_ignore.count(to))
-return clang::ASTImporter::Imported(from, to);
+return;
 
   // Transfer module ownership information.
   auto *from_source = llvm::dyn_cast_or_null(
@@ -1081,12 +1080,6 @@
 if (!to_context_md->hasOrigin(to) || user_id != LLDB_INVALID_UID)
   to_context_md->setOrigin(to, origin);
 
-ImporterDelegateSP direct_completer =
-m_main.GetDelegate(&to->getASTContext(), origin.ctx);
-
-if (direct_completer.get() != this)
-  direct_completer->ASTImporter::Imported(origin.decl, to);
-
 LLDB_LOG(log,
  "[ClangASTImporter] Propagated origin "
  "(Decl*){0}/(ASTContext*){1} from (ASTContext*){2} to "
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D158172: [lldb][ClangASTImporter][NFC] Remove redundant calls to ASTImporter::Imported

2023-08-17 Thread Michael Buch via Phabricator via lldb-commits 
Michael137 created this revision.
Michael137 added a reviewer: aprantl.
Herald added a subscriber: martong.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The `ASTImporter::Imported` base method has been made a no-op in
`26f72a96559f2acd6799c363f1ca88ef3238c601`. So all calls to it from
a base-class are now redundant. The API is now only used to notify
subclasses that an import occurred and not for any other bookkeeping.
That is done in `MapImported` which we call properly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158172

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -902,7 +902,6 @@
   // We want that 'to' is actually complete after this function so let's
   // tell the ASTImporter that 'to' was imported from 'from'.
   MapImported(from, to);
-  ASTImporter::Imported(from, to);
 
   Log *log = GetLog(LLDBLog::Expressions);
 
@@ -1028,7 +1027,7 @@
   // Some decls shouldn't be tracked here because they were not created by
   // copying 'from' to 'to'. Just exit early for those.
   if (m_decls_to_ignore.count(to))
-return clang::ASTImporter::Imported(from, to);
+return;
 
   // Transfer module ownership information.
   auto *from_source = llvm::dyn_cast_or_null(
@@ -1081,12 +1080,6 @@
 if (!to_context_md->hasOrigin(to) || user_id != LLDB_INVALID_UID)
   to_context_md->setOrigin(to, origin);
 
-ImporterDelegateSP direct_completer =
-m_main.GetDelegate(&to->getASTContext(), origin.ctx);
-
-if (direct_completer.get() != this)
-  direct_completer->ASTImporter::Imported(origin.decl, to);
-
 LLDB_LOG(log,
  "[ClangASTImporter] Propagated origin "
  "(Decl*){0}/(ASTContext*){1} from (ASTContext*){2} to "


Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTImporter.cpp
@@ -902,7 +902,6 @@
   // We want that 'to' is actually complete after this function so let's
   // tell the ASTImporter that 'to' was imported from 'from'.
   MapImported(from, to);
-  ASTImporter::Imported(from, to);
 
   Log *log = GetLog(LLDBLog::Expressions);
 
@@ -1028,7 +1027,7 @@
   // Some decls shouldn't be tracked here because they were not created by
   // copying 'from' to 'to'. Just exit early for those.
   if (m_decls_to_ignore.count(to))
-return clang::ASTImporter::Imported(from, to);
+return;
 
   // Transfer module ownership information.
   auto *from_source = llvm::dyn_cast_or_null(
@@ -1081,12 +1080,6 @@
 if (!to_context_md->hasOrigin(to) || user_id != LLDB_INVALID_UID)
   to_context_md->setOrigin(to, origin);
 
-ImporterDelegateSP direct_completer =
-m_main.GetDelegate(&to->getASTContext(), origin.ctx);
-
-if (direct_completer.get() != this)
-  direct_completer->ASTImporter::Imported(origin.decl, to);
-
 LLDB_LOG(log,
  "[ClangASTImporter] Propagated origin "
  "(Decl*){0}/(ASTContext*){1} from (ASTContext*){2} to "
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D149213: [lldb] Add basic support for Rust enums in TypeSystemClang

2023-08-17 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.

I fixed a typo in this to get the Windows build going again: 
https://github.com/llvm/llvm-project/commit/f8d1209f966ccd1dd0a19f3acef0871bf8fc3c94

I assume it's a typo, not an attempt to use what seems to have been a NetBSD 
type at one point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149213

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

[Lldb-commits] [lldb] f8d1209 - [lldb] Fix Windows on Arm build

2023-08-17 Thread David Spickett via lldb-commits 

Author: David Spickett
Date: 2023-08-17T09:56:54Z
New Revision: f8d1209f966ccd1dd0a19f3acef0871bf8fc3c94

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

LOG: [lldb] Fix Windows on Arm build

Since 
https://github.com/llvm/llvm-project/commit/e84751a21561c5b1d5673cdff8e22ac4cf2f5dc2
our bot has been failing:
https://lab.llvm.org/buildbot/#/builders/182/builds/7193

I assume this is just a typo, all tests pass.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 4f891a5c28c464..37a1174ad62bd1 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2509,7 +2509,7 @@ struct VariantMember {
   explicit VariantMember(DWARFDIE &die, ModuleSP module_sp);
   bool IsDefault() const;
 
-  std::optional discr_value;
+  std::optional discr_value;
   DWARFFormValue type_ref;
   ConstString variant_name;
   uint32_t byte_offset;



___
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

2023-08-17 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.

I think `CMAKE_TOOLCHAIN_FILE` is the "proper" way to be passing 
`CMAKE_SYSTEM_NAME` etc. so let me look into that. It may be something we get 
away with with current CMake, but breaks later, much like 
`CMAKE_CROSS_COMPILING` seemed to be less effective as time went on.

On caches, let me check the rest of the doc because there are some caches in 
tree for Mac OS. If they're already mentioned I can expand that. Maybe I'll add 
a cache for the x86 -> aarch64 cross compile.


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] D158022: [lldb] Print an actionable error message when sphinx_automodapi is not installed

2023-08-17 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.

I was in this exact situation yesterday but luckily I had already used a 
virtualenv to fix some other Python problems so I knew how to fix it. Had it 
not been for that, seeing the exact Python path would have been very useful 
here, thanks for adding this message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158022

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