[Lldb-commits] [lldb] a18f45f - [lldb] Fix string truncation method when substring is the prefix of string (NFC) (#94785)

[via lldb-commits]

Author: Shivam Gupta
Date: 2024-07-11T12:57:50+05:30
New Revision: a18f45f556c781d711f82043bf451fcce8324163

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

LOG: [lldb] Fix string truncation method when substring is the prefix of string 
(NFC) (#94785)

Correct the method used to truncate the source_file string when
substring is a prefix. The previous method used substr, which was
changed to resize for clarity and efficiency.

Caught by cppcheck - 
lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp:290:19:
performance: Ineffective call of function 'substr' because a prefix of
the string is assigned to itself. Use resize() or pop_back() instead.
[uselessCallsSubstr]

Source code - 
source_file = source_file.substr(0, pos);

Fix #91211

-

Co-authored-by: Shivam Gupta 

Added: 


Modified: 
lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp 
b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
index e177c134fea20..50b6a5c29a657 100644
--- a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
+++ b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp
@@ -287,7 +287,7 @@ Status PlatformAndroid::DownloadModuleSlice(const FileSpec 
&src_file_spec,
   static constexpr llvm::StringLiteral k_zip_separator("!/");
   size_t pos = source_file.find(k_zip_separator);
   if (pos != std::string::npos)
-source_file = source_file.substr(0, pos);
+source_file.resize(pos);
 
   Status error;
   AdbClientUP adb(GetAdbClient(error));



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

[Lldb-commits] [lldb] [lldb] Fix string truncation method when substring is the prefix of string (NFC) (PR #94785)

[Shivam Gupta via lldb-commits]

https://github.com/xgupta closed https://github.com/llvm/llvm-project/pull/94785
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Correct format specifier for sscanf to prevent buffer overflow (NFC) (PR #94783)

[Shivam Gupta via lldb-commits]

https://github.com/xgupta updated 
https://github.com/llvm/llvm-project/pull/94783

>From 17d39d89ee723881063ecbea19caaa6806e4e095 Mon Sep 17 00:00:00 2001
From: Shivam Gupta 
Date: Sat, 15 Jun 2024 23:57:03 +0530
Subject: [PATCH 1/2] Resolved merge conflict

---
 lldb/source/Host/linux/Host.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/source/Host/linux/Host.cpp b/lldb/source/Host/linux/Host.cpp
index 5545f9ef4d70e..8a38947d4b665 100644
--- a/lldb/source/Host/linux/Host.cpp
+++ b/lldb/source/Host/linux/Host.cpp
@@ -100,7 +100,7 @@ static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo 
&ProcessInfo,
   StatFields stat_fields;
   if (sscanf(
   Rest.data(),
-  "%d %s %c %d %d %d %d %d %u %lu %lu %lu %lu %lu %lu %ld %ld %ld %ld",
+  "%d %15s %c %d %d %d %d %d %u %lu %lu %lu %lu %lu %lu %ld %ld %ld 
%ld",
   &stat_fields.pid, stat_fields.comm, &stat_fields.state,
   &stat_fields.ppid, &stat_fields.pgrp, &stat_fields.session,
   &stat_fields.tty_nr, &stat_fields.tpgid, &stat_fields.flags,

>From d84106a138040dc4e680ec82306969f8ff7ec01d Mon Sep 17 00:00:00 2001
From: Shivam Gupta 
Date: Sat, 6 Jul 2024 10:57:08 +0200
Subject: [PATCH 2/2] run clang-format

---
 lldb/source/Host/linux/Host.cpp | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/lldb/source/Host/linux/Host.cpp b/lldb/source/Host/linux/Host.cpp
index 8a38947d4b665..b5fa266da1d3b 100644
--- a/lldb/source/Host/linux/Host.cpp
+++ b/lldb/source/Host/linux/Host.cpp
@@ -98,16 +98,16 @@ static bool GetStatusInfo(::pid_t Pid, ProcessInstanceInfo 
&ProcessInfo,
   if (Rest.empty())
 return false;
   StatFields stat_fields;
-  if (sscanf(
-  Rest.data(),
-  "%d %15s %c %d %d %d %d %d %u %lu %lu %lu %lu %lu %lu %ld %ld %ld 
%ld",
-  &stat_fields.pid, stat_fields.comm, &stat_fields.state,
-  &stat_fields.ppid, &stat_fields.pgrp, &stat_fields.session,
-  &stat_fields.tty_nr, &stat_fields.tpgid, &stat_fields.flags,
-  &stat_fields.minflt, &stat_fields.cminflt, &stat_fields.majflt,
-  &stat_fields.cmajflt, &stat_fields.utime, &stat_fields.stime,
-  &stat_fields.cutime, &stat_fields.cstime,
-  &stat_fields.realtime_priority, &stat_fields.priority) < 0) {
+  if (sscanf(Rest.data(),
+ "%d %15s %c %d %d %d %d %d %u %lu %lu %lu %lu %lu %lu %ld %ld %ld 
"
+ "%ld",
+ &stat_fields.pid, stat_fields.comm, &stat_fields.state,
+ &stat_fields.ppid, &stat_fields.pgrp, &stat_fields.session,
+ &stat_fields.tty_nr, &stat_fields.tpgid, &stat_fields.flags,
+ &stat_fields.minflt, &stat_fields.cminflt, &stat_fields.majflt,
+ &stat_fields.cmajflt, &stat_fields.utime, &stat_fields.stime,
+ &stat_fields.cutime, &stat_fields.cstime,
+ &stat_fields.realtime_priority, &stat_fields.priority) < 0) {
 return false;
   }
 

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

[Lldb-commits] [clang] [lldb] [llvm] [llvm][TargetParser] Return StringMap from getHostCPUFeatures (PR #97824)

[Tomas Matheson via lldb-commits]

https://github.com/tmatheson-arm approved this pull request.


https://github.com/llvm/llvm-project/pull/97824
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add support for displaying `__float128` variables (PR #98369)

[David Spickett via lldb-commits]

@@ -195,10 +199,10 @@ enum Format {
  ///< character arrays that can contain non printable
  ///< characters
   eFormatAddressInfo,///< Describe what an address points to (func + offset
-  ///< with file/line, symbol + offset, data, etc)
-  eFormatHexFloat,///< ISO C99 hex float string
-  eFormatInstruction, ///< Disassemble an opcode
-  eFormatVoid,///< Do not print this
+ ///< with file/line, symbol + offset, data, etc)
+  eFormatHexFloat,   ///< ISO C99 hex float string
+  eFormatInstruction,///< Disassemble an opcode
+  eFormatVoid,   ///< Do not print this

DavidSpickett wrote:

As the codebase isn't 100% formatted, it's fine to ignore the formatter 
sometimes. Generally I'd push a reformat NFC commit directly before or after 
the PR changes, but here I think it'd need to be after. This means folks can 
ignore the NFC change without skipping over functional changes like this.

https://github.com/llvm/llvm-project/pull/98369
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][man][nfc] Don't register a markdown parser when building man packages (PR #98420)

[David Spickett via lldb-commits]

@@ -89,9 +89,13 @@
 # The suffix of source filenames.
 source_suffix = {
 ".rst": "restructuredtext",
-".md": "markdown",
 }
 
+# Man pages do not use markdown pages, so we don't need to register a markdown
+# parser.
+if not building_man_page:
+source_suffix[".md"] = "markdown"
+

DavidSpickett wrote:

I'd move `source_suffix` up to before the previous `if not building_man_page:` 
so that you can add to it during that if instead.

https://github.com/llvm/llvm-project/pull/98420
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][man][nfc] Don't register a markdown parser when building man packages (PR #98420)

[David Spickett via lldb-commits]

https://github.com/DavidSpickett commented:

Looks fine to me just a minor comment on placement.

https://github.com/llvm/llvm-project/pull/98420
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [clang] [lldb] [llvm] [llvm][TargetParser] Return StringMap from getHostCPUFeatures (PR #97824)

[David Spickett via lldb-commits]

https://github.com/DavidSpickett updated 
https://github.com/llvm/llvm-project/pull/97824

>From 7ebe4e487b763ff26fbab6d75aa7c8694d63e8b1 Mon Sep 17 00:00:00 2001
From: David Spickett 
Date: Fri, 5 Jul 2024 08:42:22 +
Subject: [PATCH 1/9] [llvm][TargetParser] Return optional from
 getHostCPUFeatures

Previously this took a reference to a map and returned a bool
to say whether it succeeded. This is an optional but with more
steps.

The only reason to keep it that way was if someone was appending
to an existing map, but all callers made a new map before calling
it.
---
 clang/lib/Driver/ToolChains/Arch/ARM.cpp  |  6 ++--
 clang/lib/Driver/ToolChains/Arch/X86.cpp  |  6 ++--
 lldb/utils/lit-cpuid/lit-cpuid.cpp| 21 +++---
 llvm/include/llvm/TargetParser/Host.h | 14 -
 llvm/lib/CodeGen/CommandFlags.cpp | 16 --
 .../Orc/JITTargetMachineBuilder.cpp   |  8 ++---
 llvm/lib/Target/TargetMachineC.cpp|  5 ++--
 llvm/lib/TargetParser/Host.cpp| 29 ---
 8 files changed, 54 insertions(+), 51 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp 
b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index 8ae22cc37a136..77adbf3865ab1 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -591,9 +591,9 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver 
&D,
 
   // Add CPU features for generic CPUs
   if (CPUName == "native") {
-llvm::StringMap HostFeatures;
-if (llvm::sys::getHostCPUFeatures(HostFeatures))
-  for (auto &F : HostFeatures)
+if (std::optional> HostFeatures =
+llvm::sys::getHostCPUFeatures())
+  for (auto &F : *HostFeatures)
 Features.push_back(
 Args.MakeArgString((F.second ? "+" : "-") + F.first()));
   } else if (!CPUName.empty()) {
diff --git a/clang/lib/Driver/ToolChains/Arch/X86.cpp 
b/clang/lib/Driver/ToolChains/Arch/X86.cpp
index 92821b2a82dae..e4adfcac23ca0 100644
--- a/clang/lib/Driver/ToolChains/Arch/X86.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/X86.cpp
@@ -131,9 +131,9 @@ void x86::getX86TargetFeatures(const Driver &D, const 
llvm::Triple &Triple,
   // If -march=native, autodetect the feature list.
   if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) {
 if (StringRef(A->getValue()) == "native") {
-  llvm::StringMap HostFeatures;
-  if (llvm::sys::getHostCPUFeatures(HostFeatures))
-for (auto &F : HostFeatures)
+  if (std::optional> HostFeatures =
+  llvm::sys::getHostCPUFeatures())
+for (auto &F : *HostFeatures)
   Features.push_back(
   Args.MakeArgString((F.second ? "+" : "-") + F.first()));
 }
diff --git a/lldb/utils/lit-cpuid/lit-cpuid.cpp 
b/lldb/utils/lit-cpuid/lit-cpuid.cpp
index be322cb6aa42a..16743164e6a5d 100644
--- a/lldb/utils/lit-cpuid/lit-cpuid.cpp
+++ b/lldb/utils/lit-cpuid/lit-cpuid.cpp
@@ -15,22 +15,23 @@
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/TargetParser/Host.h"
 
+#include 
+
 using namespace llvm;
 
 int main(int argc, char **argv) {
 #if defined(__i386__) || defined(_M_IX86) || \
 defined(__x86_64__) || defined(_M_X64)
-  StringMap features;
-
-  if (!sys::getHostCPUFeatures(features))
+  if (std::optional> features =
+  sys::getHostCPUFeatures(features)) {
+if ((*features)["sse"])
+  outs() << "sse\n";
+if ((*features)["avx"])
+  outs() << "avx\n";
+if ((*features)["avx512f"])
+  outs() << "avx512f\n";
+  } else
 return 1;
-
-  if (features["sse"])
-outs() << "sse\n";
-  if (features["avx"])
-outs() << "avx\n";
-  if (features["avx512f"])
-outs() << "avx512f\n";
 #endif
 
   return 0;
diff --git a/llvm/include/llvm/TargetParser/Host.h 
b/llvm/include/llvm/TargetParser/Host.h
index af72045a8fe67..d68655835a323 100644
--- a/llvm/include/llvm/TargetParser/Host.h
+++ b/llvm/include/llvm/TargetParser/Host.h
@@ -13,6 +13,7 @@
 #ifndef LLVM_TARGETPARSER_HOST_H
 #define LLVM_TARGETPARSER_HOST_H
 
+#include 
 #include 
 
 namespace llvm {
@@ -47,13 +48,12 @@ namespace sys {
   /// The particular format of the names are target dependent, and suitable for
   /// passing as -mattr to the target which matches the host.
   ///
-  /// \param Features - A string mapping feature names to either
-  /// true (if enabled) or false (if disabled). This routine makes no 
guarantees
-  /// about exactly which features may appear in this map, except that they are
-  /// all valid LLVM feature names.
-  ///
-  /// \return - True on success.
-  bool getHostCPUFeatures(StringMap &Features);
+  /// \return - If feature detection succeeds, a string map mapping feature
+  /// names to either true (if enabled) or false (if disabled). This routine
+  /// makes no guarantees about exactly which features may appear in this map,
+  /// except that they are all valid LLVM feature names. If feature detection
+  /// fails, an 

[Lldb-commits] [clang] [lldb] [llvm] [llvm][TargetParser] Return StringMap from getHostCPUFeatures (PR #97824)

[David Spickett via lldb-commits]

@@ -20,16 +20,15 @@ using namespace llvm;
 int main(int argc, char **argv) {
 #if defined(__i386__) || defined(_M_IX86) || \
 defined(__x86_64__) || defined(_M_X64)
-  StringMap features;
-
-  if (!sys::getHostCPUFeatures(features))
+  const StringMap features = sys::getHostCPUFeatures(features);
+  if (features.empty())
 return 1;
 
-  if (features["sse"])
+  if (features->lookup("sse"))

DavidSpickett wrote:

It doesn't, I forgot to rebuild on x86. Fixed this and removed the parameter 
that's no longer needed.

https://github.com/llvm/llvm-project/pull/97824
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 18e70a4 - [llvm][TargetParser] Return StringMap from getHostCPUFeatures (#97824)

[via lldb-commits]

Author: David Spickett
Date: 2024-07-11T10:32:43+01:00
New Revision: 18e70a4d5042299054dae7d3995f6ccd8f4112b3

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

LOG: [llvm][TargetParser] Return StringMap from getHostCPUFeatures (#97824)

Previously this took a reference to a map and returned a bool to say
whether it succeeded. We can return a StringMap instead, as all callers
but 1 simply iterated the map if the bool was true, and passed in empty
maps as the starting point.

lldb's lit-cpuid did specifically check whether the call failed, but due
to the way the x86 routines work this works out the same as checking if
the returned map is empty.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Arch/ARM.cpp
clang/lib/Driver/ToolChains/Arch/X86.cpp
lldb/utils/lit-cpuid/lit-cpuid.cpp
llvm/include/llvm/TargetParser/Host.h
llvm/lib/CodeGen/CommandFlags.cpp
llvm/lib/ExecutionEngine/Orc/JITTargetMachineBuilder.cpp
llvm/lib/Target/TargetMachineC.cpp
llvm/lib/TargetParser/Host.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Arch/ARM.cpp 
b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
index 8ae22cc37a136..a6041b809b80b 100644
--- a/clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -591,11 +591,9 @@ llvm::ARM::FPUKind arm::getARMTargetFeatures(const Driver 
&D,
 
   // Add CPU features for generic CPUs
   if (CPUName == "native") {
-llvm::StringMap HostFeatures;
-if (llvm::sys::getHostCPUFeatures(HostFeatures))
-  for (auto &F : HostFeatures)
-Features.push_back(
-Args.MakeArgString((F.second ? "+" : "-") + F.first()));
+for (auto &F : llvm::sys::getHostCPUFeatures())
+  Features.push_back(
+  Args.MakeArgString((F.second ? "+" : "-") + F.first()));
   } else if (!CPUName.empty()) {
 // This sets the default features for the specified CPU. We certainly don't
 // want to override the features that have been explicitly specified on the

diff  --git a/clang/lib/Driver/ToolChains/Arch/X86.cpp 
b/clang/lib/Driver/ToolChains/Arch/X86.cpp
index 92821b2a82dae..9fca7864b2546 100644
--- a/clang/lib/Driver/ToolChains/Arch/X86.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/X86.cpp
@@ -131,11 +131,9 @@ void x86::getX86TargetFeatures(const Driver &D, const 
llvm::Triple &Triple,
   // If -march=native, autodetect the feature list.
   if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) {
 if (StringRef(A->getValue()) == "native") {
-  llvm::StringMap HostFeatures;
-  if (llvm::sys::getHostCPUFeatures(HostFeatures))
-for (auto &F : HostFeatures)
-  Features.push_back(
-  Args.MakeArgString((F.second ? "+" : "-") + F.first()));
+  for (auto &F : llvm::sys::getHostCPUFeatures())
+Features.push_back(
+Args.MakeArgString((F.second ? "+" : "-") + F.first()));
 }
   }
 

diff  --git a/lldb/utils/lit-cpuid/lit-cpuid.cpp 
b/lldb/utils/lit-cpuid/lit-cpuid.cpp
index be322cb6aa42a..55f6dfd8ea906 100644
--- a/lldb/utils/lit-cpuid/lit-cpuid.cpp
+++ b/lldb/utils/lit-cpuid/lit-cpuid.cpp
@@ -20,16 +20,15 @@ using namespace llvm;
 int main(int argc, char **argv) {
 #if defined(__i386__) || defined(_M_IX86) || \
 defined(__x86_64__) || defined(_M_X64)
-  StringMap features;
-
-  if (!sys::getHostCPUFeatures(features))
+  const StringMap features = sys::getHostCPUFeatures();
+  if (features.empty())
 return 1;
 
-  if (features["sse"])
+  if (features.lookup("sse"))
 outs() << "sse\n";
-  if (features["avx"])
+  if (features.lookup("avx"))
 outs() << "avx\n";
-  if (features["avx512f"])
+  if (features.lookup("avx512f"))
 outs() << "avx512f\n";
 #endif
 

diff  --git a/llvm/include/llvm/TargetParser/Host.h 
b/llvm/include/llvm/TargetParser/Host.h
index af72045a8fe67..a639b093f21bd 100644
--- a/llvm/include/llvm/TargetParser/Host.h
+++ b/llvm/include/llvm/TargetParser/Host.h
@@ -47,13 +47,12 @@ namespace sys {
   /// The particular format of the names are target dependent, and suitable for
   /// passing as -mattr to the target which matches the host.
   ///
-  /// \param Features - A string mapping feature names to either
-  /// true (if enabled) or false (if disabled). This routine makes no 
guarantees
-  /// about exactly which features may appear in this map, except that they are
-  /// all valid LLVM feature names.
-  ///
-  /// \return - True on success.
-  bool getHostCPUFeatures(StringMap &Features);
+  /// \return - A string map mapping feature names to either true (if enabled)
+  /// or false (if disabled). This routine makes no guarantees about exactly
+  /// which features may appear in this map, except that they are all valid 
LLVM
+  /// feature names. The map can be empty, fo

[Lldb-commits] [clang] [lldb] [llvm] [llvm][TargetParser] Return StringMap from getHostCPUFeatures (PR #97824)

[David Spickett via lldb-commits]

https://github.com/DavidSpickett closed 
https://github.com/llvm/llvm-project/pull/97824
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix ThreadList assignment race (PR #98293)

[Pavel Labath via lldb-commits]

@@ -736,11 +734,12 @@ void ThreadList::NotifySelectedThreadChanged(lldb::tid_t 
tid) {
 
 void ThreadList::Update(ThreadList &rhs) {
   if (this != &rhs) {
-// Lock both mutexes to make sure neither side changes anyone on us while
-// the assignment occurs
-std::scoped_lock 
guard(GetMutex(), rhs.GetMutex());
+// Same process implies same mutex...
+assert(&m_process == &rhs.m_process);
+assert(&GetMutex() == &GetMutex());

labath wrote:

yes 🤦 

https://github.com/llvm/llvm-project/pull/98293
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix ThreadList assignment race (PR #98293)

[Pavel Labath via lldb-commits]

@@ -27,12 +27,13 @@ class ThreadList : public ThreadCollection {
   friend class Process;
 
 public:
-  ThreadList(Process *process);
+  ThreadList(Process &process);
 
   ThreadList(const ThreadList &rhs);
 
   ~ThreadList() override;
 
+  // Precondition: both thread lists must be belong to the same process.

labath wrote:

Works for me

https://github.com/llvm/llvm-project/pull/98293
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix a bug for PT_TLS segments getting loaded when they shouldn't. (PR #98432)

[Pavel Labath via lldb-commits]

https://github.com/labath approved this pull request.

LGTM. Coincidentally, I was just working on a bug where this was (a part of) 
the problem. :)

https://github.com/llvm/llvm-project/pull/98432
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix a bug for PT_TLS segments getting loaded when they shouldn't. (PR #98432)

[David Spickett via lldb-commits]

@@ -1,8 +1,25 @@
 # Overlapping PT_LOAD and PT_TLS segments should be able to exist side by side.

DavidSpickett wrote:

Maybe clarify this by adding "in an object file." on the end. So it's clear 
that lldb is not making use of them, just able to ignore them as the comment 
below explains.

As it is it could be taken as "exist side by side in lldb", which is the 
opposite of what we're doing here.

https://github.com/llvm/llvm-project/pull/98432
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix a bug for PT_TLS segments getting loaded when they shouldn't. (PR #98432)

[David Spickett via lldb-commits]

@@ -1,8 +1,25 @@
 # Overlapping PT_LOAD and PT_TLS segments should be able to exist side by side.
 
+# When an ELF file contains both PT_LOAD and PT_TLS segmentsq where the PT_TLS
+# segment has the same p_vaddr and p_paddr as a PT_LOAD segment, this
+# was causing LLDB, when loading a ELF object file at an address, to overwrite
+# the section load address for a PT_LOAD that shares the same p_vaddr value in
+# the section load list's addr to section map for this code. This test ensures
+# that no PT_TLS segments get loaded and can't interfere with real segments we
+# need to resolved as all access to thread specific memory is only done via
+# DWARF location expressions. We also don't have any code that loads any thread
+# specific segments at a different address for different threads, so there is
+# no reason currently to try and load thread specific segments.
+
 # RUN: yaml2obj %s -o %t
 # RUN: lldb-test object-file %t | FileCheck %s
-# RUN: %lldb %t -o "image lookup -a 0x1000" -b | FileCheck 
--check-prefix=LOOKUP %s
+# RUN: %lldb %t -b \
+# RUN:   -o "image lookup -a 0x1000" \
+# RUN:   -o "target modules load --file %t --slide 0" \
+# RUN:   -o "image lookup -a 0x1000" \
+# RUN:   -o "target dump section-load-list" \
+# RUN:   | FileCheck --check-prefix=LOOKUP %s

DavidSpickett wrote:

Would be clearer if you put this command after the CHECK lines.

https://github.com/llvm/llvm-project/pull/98432
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix a bug for PT_TLS segments getting loaded when they shouldn't. (PR #98432)

[David Spickett via lldb-commits]

@@ -1,8 +1,25 @@
 # Overlapping PT_LOAD and PT_TLS segments should be able to exist side by side.
 
+# When an ELF file contains both PT_LOAD and PT_TLS segmentsq where the PT_TLS

DavidSpickett wrote:

"PT_TLS segments" remove the "q" typo.

https://github.com/llvm/llvm-project/pull/98432
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add support for displaying `__float128` variables (PR #98369)

[Pavel Labath via lldb-commits]

@@ -819,6 +823,7 @@ enum BasicType {
   eBasicTypeFloat,
   eBasicTypeDouble,
   eBasicTypeLongDouble,
+  eBasicTypeFloat128,

labath wrote:

The same goes here, although apparently https://reviews.llvm.org/D120690 has 
snuck through and nobody noticed.

https://github.com/llvm/llvm-project/pull/98369
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add support for displaying `__float128` variables (PR #98369)

[Pavel Labath via lldb-commits]

@@ -334,6 +334,8 @@ static const llvm::fltSemantics &GetFloatSemantics(const 
TargetSP &target_sp,
   return llvm::APFloat::IEEEsingle();
 case 8:
   return llvm::APFloat::IEEEdouble();
+case 16:
+  return llvm::APFloat::IEEEquad();

labath wrote:

This may not be a "reasonable" guess, given that sizeof(long double) is also 16 
(on x86)

https://github.com/llvm/llvm-project/pull/98369
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add support for displaying `__float128` variables (PR #98369)

[Pavel Labath via lldb-commits]

@@ -170,6 +170,10 @@ enum Format {
   eFormatHex,
   eFormatHexUppercase,
   eFormatFloat,
+  eFormatFloat128, //< Disambiguate between 128-bit `long double` (which uses

labath wrote:

This list should be appended to at the end to preserve the values of existing 
enumerators.

https://github.com/llvm/llvm-project/pull/98369
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [WIP][lldb][test] Add a layout simulator test for std::unique_ptr (PR #98330)

[Pavel Labath via lldb-commits]

https://github.com/labath commented:

I'm like this approach. It can be a lot of work, so we may not be able to get 
every contributor to do this, but I'd certainly encourage everyone to try.

https://github.com/llvm/llvm-project/pull/98330
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [WIP][lldb][test] Add a layout simulator test for std::unique_ptr (PR #98330)

[Pavel Labath via lldb-commits]

https://github.com/labath edited https://github.com/llvm/llvm-project/pull/98330
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [WIP][lldb][test] Add a layout simulator test for std::unique_ptr (PR #98330)

[Pavel Labath via lldb-commits]

@@ -0,0 +1,89 @@
+#ifndef STD_LLDB_COMPRESSED_PAIR_H
+#define STD_LLDB_COMPRESSED_PAIR_H
+
+#include <__memory/compressed_pair.h>
+#include 
+
+namespace std {
+namespace __lldb {
+
+#if COMPRESSED_PAIR_REV == 0 // Post-c88580c layout
+struct __value_init_tag {};
+struct __default_init_tag {};
+
+template ::value && !std::is_final<_Tp>::value>
+struct __compressed_pair_elem {
+  explicit __compressed_pair_elem(__default_init_tag) {}
+  explicit __compressed_pair_elem(__value_init_tag) : __value_() {}
+
+  explicit __compressed_pair_elem(_Tp __t) : __value_(__t) {}
+
+  _Tp &__get() { return __value_; }
+
+private:
+  _Tp __value_;
+};
+
+template 
+struct __compressed_pair_elem<_Tp, _Idx, true> : private _Tp {
+  explicit __compressed_pair_elem(_Tp __t) : _Tp(__t) {}
+  explicit __compressed_pair_elem(__default_init_tag) {}
+  explicit __compressed_pair_elem(__value_init_tag) : _Tp() {}
+
+  _Tp &__get() { return *this; }
+};
+
+template 
+class __compressed_pair : private __compressed_pair_elem<_T1, 0>,
+  private __compressed_pair_elem<_T2, 1> {
+public:
+  using _Base1 = __compressed_pair_elem<_T1, 0>;
+  using _Base2 = __compressed_pair_elem<_T2, 1>;
+
+  explicit __compressed_pair(_T1 __t1, _T2 __t2) : _Base1(__t1), _Base2(__t2) 
{}
+  explicit __compressed_pair()
+  : _Base1(__value_init_tag()), _Base2(__value_init_tag()) {}
+
+  template 
+  explicit __compressed_pair(_U1 &&__t1, _U2 &&__t2)
+  : _Base1(std::forward<_U1>(__t1)), _Base2(std::forward<_U2>(__t2)) {}
+
+  _T1 &first() { return static_cast<_Base1 &>(*this).__get(); }
+};
+#elif COMPRESSED_PAIR_REV == 1
+#define _LLDB_COMPRESSED_PAIR(T1, Initializer1, T2, Initializer2)  
\
+  [[__gnu__::__aligned__(alignof(T2))]] [[no_unique_address]] T1 Initializer1; 
\
+  [[no_unique_address]] __compressed_pair_padding _LIBCPP_CONCAT3( 
\

labath wrote:

I'm also not sure if we really need the _LIBCPP_CONCAT3 concat thing. Since its 
going to produce nondeterministic (well deterministic, but they will change 
very easily) values, we can't really rely on it anywhere, so maybe we could 
just hardcode something here?

https://github.com/llvm/llvm-project/pull/98330
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [WIP][lldb][test] Add a layout simulator test for std::unique_ptr (PR #98330)

[Pavel Labath via lldb-commits]

@@ -0,0 +1,43 @@
+#include "../compressed_pair.h"

labath wrote:

We have some common includes in `packages/Python/lldbsuite/test/make/`. I think 
it'd be better to put this there (probably in a subdirectory)

https://github.com/llvm/llvm-project/pull/98330
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [WIP][lldb][test] Add a layout simulator test for std::unique_ptr (PR #98330)

[Pavel Labath via lldb-commits]

@@ -0,0 +1,89 @@
+#ifndef STD_LLDB_COMPRESSED_PAIR_H
+#define STD_LLDB_COMPRESSED_PAIR_H
+
+#include <__memory/compressed_pair.h>
+#include 
+
+namespace std {
+namespace __lldb {
+
+#if COMPRESSED_PAIR_REV == 0 // Post-c88580c layout
+struct __value_init_tag {};
+struct __default_init_tag {};
+
+template ::value && !std::is_final<_Tp>::value>
+struct __compressed_pair_elem {
+  explicit __compressed_pair_elem(__default_init_tag) {}
+  explicit __compressed_pair_elem(__value_init_tag) : __value_() {}
+
+  explicit __compressed_pair_elem(_Tp __t) : __value_(__t) {}
+
+  _Tp &__get() { return __value_; }
+
+private:
+  _Tp __value_;
+};
+
+template 
+struct __compressed_pair_elem<_Tp, _Idx, true> : private _Tp {
+  explicit __compressed_pair_elem(_Tp __t) : _Tp(__t) {}
+  explicit __compressed_pair_elem(__default_init_tag) {}
+  explicit __compressed_pair_elem(__value_init_tag) : _Tp() {}
+
+  _Tp &__get() { return *this; }
+};
+
+template 
+class __compressed_pair : private __compressed_pair_elem<_T1, 0>,
+  private __compressed_pair_elem<_T2, 1> {
+public:
+  using _Base1 = __compressed_pair_elem<_T1, 0>;
+  using _Base2 = __compressed_pair_elem<_T2, 1>;
+
+  explicit __compressed_pair(_T1 __t1, _T2 __t2) : _Base1(__t1), _Base2(__t2) 
{}
+  explicit __compressed_pair()
+  : _Base1(__value_init_tag()), _Base2(__value_init_tag()) {}
+
+  template 
+  explicit __compressed_pair(_U1 &&__t1, _U2 &&__t2)
+  : _Base1(std::forward<_U1>(__t1)), _Base2(std::forward<_U2>(__t2)) {}
+
+  _T1 &first() { return static_cast<_Base1 &>(*this).__get(); }
+};
+#elif COMPRESSED_PAIR_REV == 1
+#define _LLDB_COMPRESSED_PAIR(T1, Initializer1, T2, Initializer2)  
\
+  [[__gnu__::__aligned__(alignof(T2))]] [[no_unique_address]] T1 Initializer1; 
\
+  [[no_unique_address]] __compressed_pair_padding _LIBCPP_CONCAT3( 
\

labath wrote:

Where are  __compressed_pair_padding and _LIBCPP_CONCAT3  defined? in libc++? I 
don't think we want to depend on it for several reasons:
- it makes the test non-hermetic
- it means the test will not run without libc++
- libc++ folks might now like us reaching into their private headers like that

https://github.com/llvm/llvm-project/pull/98330
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [WIP][lldb][test] Add a layout simulator test for std::unique_ptr (PR #98330)

[Pavel Labath via lldb-commits]

https://github.com/labath edited https://github.com/llvm/llvm-project/pull/98330
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [WIP][lldb][test] Add a layout simulator test for std::unique_ptr (PR #98330)

[Pavel Labath via lldb-commits]

https://github.com/labath edited https://github.com/llvm/llvm-project/pull/98330
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test] Fix ComputeClangResourceDirectory test when CLANG_RESOURCE_DIR is set (PR #98464)

[David Spickett via lldb-commits]

https://github.com/DavidSpickett created 
https://github.com/llvm/llvm-project/pull/98464

This was found during testing of llvm snapshots for Fedora.

This test was looking for an exact string match of the path calculated by 
starting with lib/liblldb and with bin/lldb. However when CLANG_RESOURCE_DIR is 
set to something e.g. "../lib/clang/19", the way the initial path is handled is 
different.

Instead of taking the parent of the parent of the binary, that is foo/bin/lldb/ 
-> foo/, it uses the parent of the binary and appends CLANG_RESOURCE_DIR to 
that. As CLANG_RESOURCE_DIR is defined as being a path relative to the parent 
dir's of the clang binary.

This means that if you start with foo/lib/lidblldb the resulting path is 
lib/../lib/clang/19, but if you start with bin/lldb the result is 
bin/../lib/clang/19.

I don't want to change the starting path of 
DefaultComputeClangResourceDirectory (which is bin/lldb) as I suspect that's 
chosen instead of liblldb for good reason.

So the way to make this test work is to check not for exact path matches but 
that the "real" (".." resolved) version of the paths are the same. That way 
foo/bin/../lib and foo/lib/../lib will be the same.

>From f74cddc8e1826791aeef86ac260274d5d7b73eec Mon Sep 17 00:00:00 2001
From: David Spickett 
Date: Thu, 11 Jul 2024 10:51:58 +
Subject: [PATCH] [lldb][test] Fix ComputeClangResourceDirectory test when
 CLANG_RESOURCE_DIR is set

This test was looking for an exact string match of the path calculated by
starting with lib/liblldb and with bin/lldb. However when CLANG_RESOURCE_DIR
is set to something e.g. "../lib/clang/19", the way the initial path is handled
is different.

Instead of taking the parent of the parent of the binary, that is
foo/bin/lldb/ -> foo/, it uses the parent of the binary and appends
CLANG_RESOURCE_DIR to that. As CLANG_RESOURCE_DIR is defined as being
a path relative to the parent dir's of the clang binary.

This means that if you start with foo/lib/lidblldb the resulting
path is lib/../lib/clang/19, but if you start with bin/lldb the
result is bin/../lib/clang/19.

I don't want to change the starting path of DefaultComputeClangResourceDirectory
(which is bin/lldb) as I suspect that's chosen instead of liblldb for
good reason.

So the way to make this test work is to check not for exact path
matches but that the "real" (".." resolved) version of the paths
are the same. That way foo/bin/../lib and foo/lib/../lib will be the
same.
---
 lldb/unittests/Expression/ClangParserTest.cpp | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/lldb/unittests/Expression/ClangParserTest.cpp 
b/lldb/unittests/Expression/ClangParserTest.cpp
index ed5ee323b7d20..6f682f6c97fdb 100644
--- a/lldb/unittests/Expression/ClangParserTest.cpp
+++ b/lldb/unittests/Expression/ClangParserTest.cpp
@@ -44,7 +44,17 @@ TEST_F(ClangHostTest, ComputeClangResourceDirectory) {
 #endif
   std::string path_to_clang_dir = clang::driver::Driver::GetResourcesPath(
   path_to_liblldb + "liblldb", CLANG_RESOURCE_DIR);
-  EXPECT_EQ(ComputeClangResourceDir(path_to_liblldb), path_to_clang_dir);
+  llvm::SmallString<256> path_to_clang_lib_dir_real;
+  llvm::sys::fs::real_path(path_to_clang_dir, path_to_clang_lib_dir_real);
+
+  std::string computed_path = ComputeClangResourceDir(path_to_liblldb);
+  llvm::SmallString<256> computed_path_real;
+  llvm::sys::fs::real_path(computed_path, computed_path_real);
+
+  // When CLANG_RESOURCE_DIR is set, both the functions we use here behave in
+  // such a way that leads to one path being lib/ and the other bin/. Check 
that
+  // they are equivalent after any ".." have been resolved.
+  EXPECT_EQ(path_to_clang_lib_dir_real, computed_path_real);
 
   // The path doesn't really exist, so setting verify to true should make
   // ComputeClangResourceDir not give you path_to_clang_dir.

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

[Lldb-commits] [lldb] [lldb][test] Fix ComputeClangResourceDirectory test when CLANG_RESOURCE_DIR is set (PR #98464)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: David Spickett (DavidSpickett)


Changes

This was found during testing of llvm snapshots for Fedora.

This test was looking for an exact string match of the path calculated by 
starting with lib/liblldb and with bin/lldb. However when CLANG_RESOURCE_DIR is 
set to something e.g. "../lib/clang/19", the way the initial path is handled is 
different.

Instead of taking the parent of the parent of the binary, that is foo/bin/lldb/ 
-> foo/, it uses the parent of the binary and appends CLANG_RESOURCE_DIR to 
that. As CLANG_RESOURCE_DIR is defined as being a path relative to the parent 
dir's of the clang binary.

This means that if you start with foo/lib/lidblldb the resulting path is 
lib/../lib/clang/19, but if you start with bin/lldb the result is 
bin/../lib/clang/19.

I don't want to change the starting path of 
DefaultComputeClangResourceDirectory (which is bin/lldb) as I suspect that's 
chosen instead of liblldb for good reason.

So the way to make this test work is to check not for exact path matches but 
that the "real" (".." resolved) version of the paths are the same. That way 
foo/bin/../lib and foo/lib/../lib will be the same.

---
Full diff: https://github.com/llvm/llvm-project/pull/98464.diff


1 Files Affected:

- (modified) lldb/unittests/Expression/ClangParserTest.cpp (+11-1) 


``diff
diff --git a/lldb/unittests/Expression/ClangParserTest.cpp 
b/lldb/unittests/Expression/ClangParserTest.cpp
index ed5ee323b7d20..6f682f6c97fdb 100644
--- a/lldb/unittests/Expression/ClangParserTest.cpp
+++ b/lldb/unittests/Expression/ClangParserTest.cpp
@@ -44,7 +44,17 @@ TEST_F(ClangHostTest, ComputeClangResourceDirectory) {
 #endif
   std::string path_to_clang_dir = clang::driver::Driver::GetResourcesPath(
   path_to_liblldb + "liblldb", CLANG_RESOURCE_DIR);
-  EXPECT_EQ(ComputeClangResourceDir(path_to_liblldb), path_to_clang_dir);
+  llvm::SmallString<256> path_to_clang_lib_dir_real;
+  llvm::sys::fs::real_path(path_to_clang_dir, path_to_clang_lib_dir_real);
+
+  std::string computed_path = ComputeClangResourceDir(path_to_liblldb);
+  llvm::SmallString<256> computed_path_real;
+  llvm::sys::fs::real_path(computed_path, computed_path_real);
+
+  // When CLANG_RESOURCE_DIR is set, both the functions we use here behave in
+  // such a way that leads to one path being lib/ and the other bin/. Check 
that
+  // they are equivalent after any ".." have been resolved.
+  EXPECT_EQ(path_to_clang_lib_dir_real, computed_path_real);
 
   // The path doesn't really exist, so setting verify to true should make
   // ComputeClangResourceDir not give you path_to_clang_dir.

``




https://github.com/llvm/llvm-project/pull/98464
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][riscv] Fix setting breakpoint for undecoded instruction (PR #90075)

[via lldb-commits]

ita-sc wrote:

Gentle ping

https://github.com/llvm/llvm-project/pull/90075
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Support new libc++ __compressed_pair layout (PR #96538)

[Pavel Labath via lldb-commits]

https://github.com/labath edited https://github.com/llvm/llvm-project/pull/96538
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Support new libc++ __compressed_pair layout (PR #96538)

[Pavel Labath via lldb-commits]

@@ -254,6 +256,29 @@ lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::
 Update();
 }
 
+llvm::Expected
+lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::
+CalculateNumChildrenForOldCompressedPairLayout() {
+  ValueObjectSP node_sp(m_tree->GetChildMemberWithName("__pair3_"));
+  if (!node_sp)
+return 0;
+
+  // TODO: or should this just be: assert
+  // (!isOldCompressedPairLayout(*node_sp));

labath wrote:

I don't think so, as this could also be triggered by a some change in libc++, 
missing/corrupted debug info, etc.

https://github.com/llvm/llvm-project/pull/96538
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Support new libc++ __compressed_pair layout (PR #96538)

[Pavel Labath via lldb-commits]

https://github.com/labath approved this pull request.


https://github.com/llvm/llvm-project/pull/96538
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix ThreadList assignment race (PR #98293)

[Pavel Labath via lldb-commits]

https://github.com/labath updated 
https://github.com/llvm/llvm-project/pull/98293

>From 0a594fea0a5db3ebc8de74edbcd051cbac911697 Mon Sep 17 00:00:00 2001
From: Pavel Labath 
Date: Wed, 10 Jul 2024 04:06:42 +0200
Subject: [PATCH 1/2] [lldb] Fix ThreadList assignment race

ThreadList uses the Process mutex to guard its state. This means its not
possible to safely modify its process member, as the member is required
to lock the mutex.

Fortunately for us, we never actually need to change the process member
(we always just juggle different kinds of thread lists belonging to the
same process).

This patch replaces the process member assignment (which is technically
a race even when it assigns the same value) with an assertion.

Since all this means that the class can never change its process member
value (and it also must be non-null at all times), I've also changed the
member type to a reference.
---
 lldb/include/lldb/Target/ThreadList.h |  6 +-
 .../Python/OperatingSystemPython.cpp  |  2 +-
 lldb/source/Target/Process.cpp| 14 ++---
 lldb/source/Target/ThreadList.cpp | 59 +--
 4 files changed, 40 insertions(+), 41 deletions(-)

diff --git a/lldb/include/lldb/Target/ThreadList.h 
b/lldb/include/lldb/Target/ThreadList.h
index 6af04f8ffc376..91c4317cb8571 100644
--- a/lldb/include/lldb/Target/ThreadList.h
+++ b/lldb/include/lldb/Target/ThreadList.h
@@ -27,12 +27,13 @@ class ThreadList : public ThreadCollection {
   friend class Process;
 
 public:
-  ThreadList(Process *process);
+  ThreadList(Process &process);
 
   ThreadList(const ThreadList &rhs);
 
   ~ThreadList() override;
 
+  // Precondition: both thread lists must be belong to the same process.
   const ThreadList &operator=(const ThreadList &rhs);
 
   uint32_t GetSize(bool can_update = true);
@@ -135,6 +136,7 @@ class ThreadList : public ThreadCollection {
 
   std::recursive_mutex &GetMutex() const override;
 
+  // Precondition: both thread lists must be belong to the same process.
   void Update(ThreadList &rhs);
 
 protected:
@@ -143,7 +145,7 @@ class ThreadList : public ThreadCollection {
   void NotifySelectedThreadChanged(lldb::tid_t tid);
 
   // Classes that inherit from Process can see and modify these
-  Process *m_process; ///< The process that manages this thread list.
+  Process &m_process; ///< The process that manages this thread list.
   uint32_t
   m_stop_id; ///< The process stop ID that this thread list is valid for.
   lldb::tid_t
diff --git 
a/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp 
b/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
index 81ee7e328b6c0..e026ffefd645e 100644
--- a/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
+++ b/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
@@ -372,7 +372,7 @@ lldb::ThreadSP 
OperatingSystemPython::CreateThread(lldb::tid_t tid,
 
 std::vector core_used_map;
 if (thread_info_dict) {
-  ThreadList core_threads(m_process);
+  ThreadList core_threads(*m_process);
   ThreadList &thread_list = m_process->GetThreadList();
   bool did_create = false;
   ThreadSP thread_sp(
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index dc7f6c9e86a47..b91446e1c0e49 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -460,12 +460,10 @@ Process::Process(lldb::TargetSP target_sp, ListenerSP 
listener_sp,
   m_private_state_listener_sp(
   Listener::MakeListener("lldb.process.internal_state_listener")),
   m_mod_id(), m_process_unique_id(0), m_thread_index_id(0),
-  m_thread_id_to_index_id_map(), m_exit_status(-1), m_exit_string(),
-  m_exit_status_mutex(), m_thread_mutex(), m_thread_list_real(this),
-  m_thread_list(this), m_thread_plans(*this), m_extended_thread_list(this),
-  m_extended_thread_stop_id(0), m_queue_list(this), 
m_queue_list_stop_id(0),
-  m_watchpoint_resource_list(), m_notifications(), m_image_tokens(),
-  m_breakpoint_site_list(), m_dynamic_checkers_up(),
+  m_thread_id_to_index_id_map(), m_exit_status(-1),
+  m_thread_list_real(*this), m_thread_list(*this), m_thread_plans(*this),
+  m_extended_thread_list(*this), m_extended_thread_stop_id(0),
+  m_queue_list(this), m_queue_list_stop_id(0),
   m_unix_signals_sp(unix_signals_sp), m_abi_sp(), m_process_input_reader(),
   m_stdio_communication("process.stdio"), m_stdio_communication_mutex(),
   m_stdin_forward(false), m_stdout_data(), m_stderr_data(),
@@ -1183,8 +1181,8 @@ void Process::UpdateThreadListIfNeeded() {
   // mutex between the call to UpdateThreadList(...) and the
   // os->UpdateThreadList(...) so it doesn't change on us
   ThreadList &old_thread_list = m_thread_list;
-  ThreadList real_thread_list(this);
-  ThreadList new_thread_list(this);
+  ThreadList real_thread_list(*this);
+  ThreadList n

[Lldb-commits] [lldb] [lldb] Fix ThreadList assignment race (PR #98293)

[Pavel Labath via lldb-commits]

labath wrote:

> LGTM.
> 
> As a nice to have, maybe add a short explanation to the first mutex 
> explaining that these operations are only allowed between thread lists 
> belonging to the same process. Your current comment makes it sound as if that 
> only applies _if_ the processes are the same, but per the assertion, the 
> requirement is stricter than that.
> 
> EDIT: Oh I see you had the comment in the header. That covers my concern, 
> though I'd still add the same thing to the assert to quickly diagnose the 
> issue if someone were to hit it.

Elaborated the assert comment as well.

https://github.com/llvm/llvm-project/pull/98293
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] e1bd337 - [lldb] Fix ThreadList assignment race (#98293)

[via lldb-commits]

Author: Pavel Labath
Date: 2024-07-11T14:04:19+02:00
New Revision: e1bd337865fca9f455225ba37b76595d37bad213

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

LOG: [lldb] Fix ThreadList assignment race (#98293)

ThreadList uses the Process mutex to guard its state. This means its not
possible to safely modify its process member, as the member is required
to lock the mutex.

Fortunately for us, we never actually need to change the process member
(we always just juggle different kinds of thread lists belonging to the
same process).

This patch replaces the process member assignment (which is technically
a race even when it assigns the same value) with an assertion.

Since all this means that the class can never change its process member
value (and it also must be non-null at all times), I've also changed the
member type to a reference.

Added: 


Modified: 
lldb/include/lldb/Target/ThreadList.h
lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
lldb/source/Target/Process.cpp
lldb/source/Target/ThreadList.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/ThreadList.h 
b/lldb/include/lldb/Target/ThreadList.h
index 6af04f8ffc376..f931bb83a8cea 100644
--- a/lldb/include/lldb/Target/ThreadList.h
+++ b/lldb/include/lldb/Target/ThreadList.h
@@ -27,12 +27,13 @@ class ThreadList : public ThreadCollection {
   friend class Process;
 
 public:
-  ThreadList(Process *process);
+  ThreadList(Process &process);
 
   ThreadList(const ThreadList &rhs);
 
   ~ThreadList() override;
 
+  /// Precondition: both thread lists must be belong to the same process.
   const ThreadList &operator=(const ThreadList &rhs);
 
   uint32_t GetSize(bool can_update = true);
@@ -135,6 +136,7 @@ class ThreadList : public ThreadCollection {
 
   std::recursive_mutex &GetMutex() const override;
 
+  /// Precondition: both thread lists must be belong to the same process.
   void Update(ThreadList &rhs);
 
 protected:
@@ -143,7 +145,7 @@ class ThreadList : public ThreadCollection {
   void NotifySelectedThreadChanged(lldb::tid_t tid);
 
   // Classes that inherit from Process can see and modify these
-  Process *m_process; ///< The process that manages this thread list.
+  Process &m_process; ///< The process that manages this thread list.
   uint32_t
   m_stop_id; ///< The process stop ID that this thread list is valid for.
   lldb::tid_t

diff  --git 
a/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp 
b/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
index 81ee7e328b6c0..e026ffefd645e 100644
--- a/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
+++ b/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
@@ -372,7 +372,7 @@ lldb::ThreadSP 
OperatingSystemPython::CreateThread(lldb::tid_t tid,
 
 std::vector core_used_map;
 if (thread_info_dict) {
-  ThreadList core_threads(m_process);
+  ThreadList core_threads(*m_process);
   ThreadList &thread_list = m_process->GetThreadList();
   bool did_create = false;
   ThreadSP thread_sp(

diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index dc7f6c9e86a47..b91446e1c0e49 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -460,12 +460,10 @@ Process::Process(lldb::TargetSP target_sp, ListenerSP 
listener_sp,
   m_private_state_listener_sp(
   Listener::MakeListener("lldb.process.internal_state_listener")),
   m_mod_id(), m_process_unique_id(0), m_thread_index_id(0),
-  m_thread_id_to_index_id_map(), m_exit_status(-1), m_exit_string(),
-  m_exit_status_mutex(), m_thread_mutex(), m_thread_list_real(this),
-  m_thread_list(this), m_thread_plans(*this), m_extended_thread_list(this),
-  m_extended_thread_stop_id(0), m_queue_list(this), 
m_queue_list_stop_id(0),
-  m_watchpoint_resource_list(), m_notifications(), m_image_tokens(),
-  m_breakpoint_site_list(), m_dynamic_checkers_up(),
+  m_thread_id_to_index_id_map(), m_exit_status(-1),
+  m_thread_list_real(*this), m_thread_list(*this), m_thread_plans(*this),
+  m_extended_thread_list(*this), m_extended_thread_stop_id(0),
+  m_queue_list(this), m_queue_list_stop_id(0),
   m_unix_signals_sp(unix_signals_sp), m_abi_sp(), m_process_input_reader(),
   m_stdio_communication("process.stdio"), m_stdio_communication_mutex(),
   m_stdin_forward(false), m_stdout_data(), m_stderr_data(),
@@ -1183,8 +1181,8 @@ void Process::UpdateThreadListIfNeeded() {
   // mutex between the call to UpdateThreadList(...) and the
   // os->UpdateThreadList(...) so it doesn't change on us
   ThreadList &old_thread_list = m_thread_list;
-  ThreadList real_thread_list(this);
-  

[Lldb-commits] [lldb] [lldb] Fix ThreadList assignment race (PR #98293)

[Pavel Labath via lldb-commits]

https://github.com/labath closed https://github.com/llvm/llvm-project/pull/98293
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [WIP][lldb][test] Add a layout simulator test for std::unique_ptr (PR #98330)

[Michael Buch via lldb-commits]

https://github.com/Michael137 updated 
https://github.com/llvm/llvm-project/pull/98330

>From a25b3c8a6a36326730d00d1060ff84181bece26e Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Wed, 10 Jul 2024 15:37:45 +0100
Subject: [PATCH 1/3] [WIP][lldb][test] Add a layout simulator test for
 std::unique_ptr

---
 .../libcxx-simulators/compressed_pair.h   | 89 +++
 .../libcxx-simulators/unique_ptr/Makefile |  3 +
 ...stDataFormatterLibcxxUniquePtrSimulator.py | 32 +++
 .../libcxx-simulators/unique_ptr/main.cpp | 43 +
 4 files changed, 167 insertions(+)
 create mode 100644 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/compressed_pair.h
 create mode 100644 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/unique_ptr/Makefile
 create mode 100644 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/unique_ptr/TestDataFormatterLibcxxUniquePtrSimulator.py
 create mode 100644 
lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/unique_ptr/main.cpp

diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/compressed_pair.h
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/compressed_pair.h
new file mode 100644
index 0..ec978b8053646
--- /dev/null
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/compressed_pair.h
@@ -0,0 +1,89 @@
+#ifndef STD_LLDB_COMPRESSED_PAIR_H
+#define STD_LLDB_COMPRESSED_PAIR_H
+
+#include <__memory/compressed_pair.h>
+#include 
+
+namespace std {
+namespace __lldb {
+
+#if COMPRESSED_PAIR_REV == 0 // Post-c88580c layout
+struct __value_init_tag {};
+struct __default_init_tag {};
+
+template ::value && !std::is_final<_Tp>::value>
+struct __compressed_pair_elem {
+  explicit __compressed_pair_elem(__default_init_tag) {}
+  explicit __compressed_pair_elem(__value_init_tag) : __value_() {}
+
+  explicit __compressed_pair_elem(_Tp __t) : __value_(__t) {}
+
+  _Tp &__get() { return __value_; }
+
+private:
+  _Tp __value_;
+};
+
+template 
+struct __compressed_pair_elem<_Tp, _Idx, true> : private _Tp {
+  explicit __compressed_pair_elem(_Tp __t) : _Tp(__t) {}
+  explicit __compressed_pair_elem(__default_init_tag) {}
+  explicit __compressed_pair_elem(__value_init_tag) : _Tp() {}
+
+  _Tp &__get() { return *this; }
+};
+
+template 
+class __compressed_pair : private __compressed_pair_elem<_T1, 0>,
+  private __compressed_pair_elem<_T2, 1> {
+public:
+  using _Base1 = __compressed_pair_elem<_T1, 0>;
+  using _Base2 = __compressed_pair_elem<_T2, 1>;
+
+  explicit __compressed_pair(_T1 __t1, _T2 __t2) : _Base1(__t1), _Base2(__t2) 
{}
+  explicit __compressed_pair()
+  : _Base1(__value_init_tag()), _Base2(__value_init_tag()) {}
+
+  template 
+  explicit __compressed_pair(_U1 &&__t1, _U2 &&__t2)
+  : _Base1(std::forward<_U1>(__t1)), _Base2(std::forward<_U2>(__t2)) {}
+
+  _T1 &first() { return static_cast<_Base1 &>(*this).__get(); }
+};
+#elif COMPRESSED_PAIR_REV == 1
+#define _LLDB_COMPRESSED_PAIR(T1, Initializer1, T2, Initializer2)  
\
+  [[__gnu__::__aligned__(alignof(T2))]] [[no_unique_address]] T1 Initializer1; 
\
+  [[no_unique_address]] __compressed_pair_padding _LIBCPP_CONCAT3( 
\
+  __padding1_, __LINE__, _);   
\
+  [[no_unique_address]] T2 Initializer2;   
\
+  [[no_unique_address]] __compressed_pair_padding _LIBCPP_CONCAT3( 
\
+  __padding2_, __LINE__, _)
+
+#define _LLDB_COMPRESSED_TRIPLE(T1, Initializer1, T2, Initializer2, T3,
\
+Initializer3)  
\
+  [[using __gnu__: __aligned__(alignof(T2)),   
\
+__aligned__(alignof(T3))]] [[no_unique_address]] T1 Initializer1;  
\
+  [[no_unique_address]] __compressed_pair_padding _LIBCPP_CONCAT3( 
\
+  __padding1_, __LINE__, _);   
\
+  [[no_unique_address]] T2 Initializer2;   
\
+  [[no_unique_address]] __compressed_pair_padding _LIBCPP_CONCAT3( 
\
+  __padding2_, __LINE__, _);   
\
+  [[no_unique_address]] T3 Initializer3;   
\
+  [[no_unique_address]] __compressed_pair_padding _LIBCPP_CONCAT3( 
\
+  __padding3_, __LINE__, _)
+#elif COMPRESSED_PAIR_REV == 2
+#define _LLDB_COMPRESSED_PAIR(T1, Name1, T2, Name2)
\
+  [[no_unique_address]] T1 Name1;  
\
+  [[no_unique_address]] T2 Name2
+
+#define _LLDB_COMPRESSED_TRIPLE(T1, Name1, T2, Name2, T3, Name3)   
\
+  [[no_unique_address]] T1 Name1; 

[Lldb-commits] [lldb] [WIP][lldb][test] Add a layout simulator test for std::unique_ptr (PR #98330)

[Michael Buch via lldb-commits]

@@ -0,0 +1,89 @@
+#ifndef STD_LLDB_COMPRESSED_PAIR_H
+#define STD_LLDB_COMPRESSED_PAIR_H
+
+#include 
+
+namespace std {
+namespace __lldb {
+
+#if COMPRESSED_PAIR_REV == 0 // Post-c88580c layout
+struct __value_init_tag {};
+struct __default_init_tag {};
+
+template ::value && !std::is_final<_Tp>::value>
+struct __compressed_pair_elem {
+  explicit __compressed_pair_elem(__default_init_tag) {}
+  explicit __compressed_pair_elem(__value_init_tag) : __value_() {}
+
+  explicit __compressed_pair_elem(_Tp __t) : __value_(__t) {}
+
+  _Tp &__get() { return __value_; }
+
+private:
+  _Tp __value_;
+};
+
+template 
+struct __compressed_pair_elem<_Tp, _Idx, true> : private _Tp {
+  explicit __compressed_pair_elem(_Tp __t) : _Tp(__t) {}
+  explicit __compressed_pair_elem(__default_init_tag) {}
+  explicit __compressed_pair_elem(__value_init_tag) : _Tp() {}
+
+  _Tp &__get() { return *this; }
+};
+
+template 
+class __compressed_pair : private __compressed_pair_elem<_T1, 0>,
+  private __compressed_pair_elem<_T2, 1> {
+public:
+  using _Base1 = __compressed_pair_elem<_T1, 0>;
+  using _Base2 = __compressed_pair_elem<_T2, 1>;
+
+  explicit __compressed_pair(_T1 __t1, _T2 __t2) : _Base1(__t1), _Base2(__t2) 
{}
+  explicit __compressed_pair()
+  : _Base1(__value_init_tag()), _Base2(__value_init_tag()) {}
+
+  template 
+  explicit __compressed_pair(_U1 &&__t1, _U2 &&__t2)
+  : _Base1(std::forward<_U1>(__t1)), _Base2(std::forward<_U2>(__t2)) {}
+
+  _T1 &first() { return static_cast<_Base1 &>(*this).__get(); }
+};
+#elif COMPRESSED_PAIR_REV == 1
+template  class __compressed_pair_padding {
+  char __padding_[(is_empty<_ToPad>::value && 
!__libcpp_is_final<_ToPad>::value)
+  ? 0
+  : sizeof(_ToPad) - __datasizeof(_ToPad)];
+};
+
+#define _LLDB_COMPRESSED_PAIR(T1, Initializer1, T2, Initializer2)  
\
+  [[__gnu__::__aligned__(alignof(T2))]] [[no_unique_address]] T1 Initializer1; 
\
+  [[no_unique_address]] __compressed_pair_padding __padding1_; 
\

Michael137 wrote:

For now this doesn't cause any clashes. I think I've seen one instance in 
libc++ where we would have two compressed pairs within the same type. Can 
probably tackle that once we get to that? Alternatively could add another 
`suffix` parameter to this macro for an easy way to avoid clashes

https://github.com/llvm/llvm-project/pull/98330
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Fix test assertions in TestDAP_stepInTargets.py (PR #96687)

[via lldb-commits]

@@ -55,14 +55,23 @@ def test_basic(self):
 self.assertEqual(len(step_in_targets), 3, "expect 3 step in targets")
 
 # Verify the target names are correct.
-self.assertEqual(step_in_targets[0]["label"], "bar()", "expect bar()")
-self.assertEqual(step_in_targets[1]["label"], "bar2()", "expect 
bar2()")
-self.assertEqual(
-step_in_targets[2]["label"], "foo(int, int)", "expect foo(int, 
int)"
-)
+# The order of funcA and funcB may change depending on the compiler 
ABI.
+funcA_target = None
+funcB_target = None
+for target in step_in_targets[0:2]:
+if "funcB" in target["label"]:
+funcB_target = target
+elif "funcA" in target["label"]:
+funcA_target = target
+else:
+self.fail(f"Unexpected step in target: {target}")
+
+self.assertIsNotNone(funcA_target, "expect funcA")
+self.assertIsNotNone(funcB_target, "expect funcB")
+self.assertIn("foo", step_in_targets[2]["label"], "expect foo")
 
-# Choose to step into second target and verify that we are in bar2()
-self.stepIn(threadId=tid, targetId=step_in_targets[1]["id"], 
waitForStop=True)
+# Choose to step into second target and verify that we are in funcB()

jeffreytan81 wrote:

I do not think anyone will have time to get a Windows machine, investigate and 
fix there. If there is indeed a bug on Windows, the easiest way is skip this 
test for Windows (with a comment mentioning it is not working there). 

https://github.com/llvm/llvm-project/pull/96687
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Fix test assertions in TestDAP_stepInTargets.py (PR #96687)

[via lldb-commits]

https://github.com/jeffreytan81 edited 
https://github.com/llvm/llvm-project/pull/96687
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix a bug for PT_TLS segments getting loaded when they shouldn't. (PR #98432)

[Greg Clayton via lldb-commits]

https://github.com/clayborg updated 
https://github.com/llvm/llvm-project/pull/98432

>From ede6fc3939cbdf3a5744ff5e6919551a4b5dd438 Mon Sep 17 00:00:00 2001
From: Greg Clayton 
Date: Wed, 10 Jul 2024 21:49:04 -0700
Subject: [PATCH 1/2] Fix a bug for PT_TLS segments getting loaded when they
 shouldn't.

PT_LOAD and PT_TLS segments are top level sections in the ObjectFileELF section 
list. The two segments can often have the same program header p_vaddr and 
p_paddr values and this can cause section load list issues in LLDB if we load 
the PT_TLS segments. What happens is the SectionLoadList::m_addr_to_sect, when 
a library is loaded, will first map one of the sections named "PT_LOAD[0]" with 
the load address that matches the p_vaddr entry from the program header. Then 
the "PT_TLS[0]" would come along and try to load this section at the same 
address. This would cause the "PT_LOAD[0]" section to be unloaded as the 
SectionLoadList::m_addr_to_sect would replace the value for the matching 
p_vaddr with the last section to be seen. The sizes of the PT_TLS and PT_LOAD 
that have the same p_vaddr value don't need to have the same byte size, so this 
could cause lookups to fail for an addresses in the "PT_LOAD[0]" section or any 
of its children if the offset is greater than the offset size of the PT_TLS 
segment. It could also cause us to incorrectly attribute addresses from the 
"PT_LOAD[0]" to the "PT_TLS[0]" segment when doing lookups for offset  that are 
less than the size of the PT_TLS segment.

This fix stops us from loading PT_TLS segments in the section load lists and 
will prevent the bugs that resulted from this. No addresses the the DWARF refer 
to TLS data with a "file address" in any way. They all have TLS DWARF location 
expressions to locate these variables. We also don't have any support for 
having actual thread specific sections and having those sections resolve to 
something different for each thread, so there currently is no point in loading 
thread specific sections. Both the ObjectFileMachO and ObjectFileCOFF both 
ignore thread specific sections at the moment, so this brings the ObjectFileELF 
to parity with those plug-ins.

I added a test into an existing test to verify that things work as expected.

Prior to this fix with a real binary, the output of "target dump 
section-load-list" would look like this for the old LLDB:
```
// (lldb) target dump section-load-list
// addr = 0x, section = 0x55d46ab8c510: 0xfffd 
container[0x-0x0628)  r--  0x 
0x0628 0x a.out.PT_LOAD[0]
// addr = 0x1000, section = 0x55d46ab8b0c0: 0xfffc 
container[0x1000-0x1185)  r-x  0x1000 
0x0185 0x a.out.PT_LOAD[1]
// addr = 0x2000, section = 0x55d46ac040f0: 0xfffb 
container[0x2000-0x20cc)  r--  0x2000 
0x00cc 0x a.out.PT_LOAD[2]
// addr = 0x3db0, section = 0x55d46ab7cef0: 0xfff6 
container[0x3db0-0x3db4)  r--  0x2db0 
0x 0x a.out.PT_TLS[0]
```
And this for the fixed LLDB:
```
// (lldb) target dump section-load-list
// addr = 0x, section = 0x105f0a9a8: 0xfffd 
container[0x-0x0628)  r--  0x 
0x0628 0x a.out.PT_LOAD[0]
// addr = 0x1000, section = 0x105f0adb8: 0xfffc 
container[0x1000-0x1185)  r-x  0x1000 
0x0185 0x a.out.PT_LOAD[1]
// addr = 0x2000, section = 0x105f0af48: 0xfffb 
container[0x2000-0x20cc)  r--  0x2000 
0x00cc 0x a.out.PT_LOAD[2]
// addr = 0x3db0, section = 0x105f0b078: 0xfffa 
container[0x3db0-0x4028)  rw-  0x2db0 
0x0274 0x a.out.PT_LOAD[3]
```
We can see that previously the "PT_LOAD[3]" segment would be removed from the 
section load list, and after the fix it remains and there is on PT_TLS in the 
loaded sections.
---
 .../Plugins/ObjectFile/ELF/ObjectFileELF.cpp  | 14 +++
 .../ELF/PT_TLS-overlap-PT_LOAD.yaml   | 25 ++-
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp 
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 5c6b475044be5..51bd34e95c77d 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -717,6 +717,20 @@ bool ObjectFileELF::SetLoadAddress(Target &target, 
lldb::addr_t value,
 // Iterate through the object file sections to find all of the sections
 // that have SHF_ALLOC in their flag bits.
 SectionSP section_sp(section_list->GetSectionAtIndex(sect_idx));
+
+// PT_TLS segments can 

[Lldb-commits] [lldb] 55b1410 - [lldb] Fix a bug for PT_TLS segments getting loaded when they shouldn't. (#98432)

[via lldb-commits]

Author: Greg Clayton
Date: 2024-07-11T09:18:43-07:00
New Revision: 55b1410895b69d8e4517f6a2a2978b414b4ecac3

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

LOG: [lldb] Fix a bug for PT_TLS segments getting loaded when they shouldn't. 
(#98432)

PT_LOAD and PT_TLS segments are top level sections in the ObjectFileELF
section list. The two segments can often have the same program header
p_vaddr and p_paddr values and this can cause section load list issues
in LLDB if we load the PT_TLS segments. What happens is the
SectionLoadList::m_addr_to_sect, when a library is loaded, will first
map one of the sections named "PT_LOAD[0]" with the load address that
matches the p_vaddr entry from the program header. Then the "PT_TLS[0]"
would come along and try to load this section at the same address. This
would cause the "PT_LOAD[0]" section to be unloaded as the
SectionLoadList::m_addr_to_sect would replace the value for the matching
p_vaddr with the last section to be seen. The sizes of the PT_TLS and
PT_LOAD that have the same p_vaddr value don't need to have the same
byte size, so this could cause lookups to fail for an addresses in the
"PT_LOAD[0]" section or any of its children if the offset is greater
than the offset size of the PT_TLS segment. It could also cause us to
incorrectly attribute addresses from the "PT_LOAD[0]" to the "PT_TLS[0]"
segment when doing lookups for offset that are less than the size of the
PT_TLS segment.

This fix stops us from loading PT_TLS segments in the section load lists
and will prevent the bugs that resulted from this. No addresses the the
DWARF refer to TLS data with a "file address" in any way. They all have
TLS DWARF location expressions to locate these variables. We also don't
have any support for having actual thread specific sections and having
those sections resolve to something different for each thread, so there
currently is no point in loading thread specific sections. Both the
ObjectFileMachO and ObjectFileCOFF both ignore thread specific sections
at the moment, so this brings the ObjectFileELF to parity with those
plug-ins.

I added a test into an existing test to verify that things work as
expected.

Prior to this fix with a real binary, the output of "target dump
section-load-list" would look like this for the old LLDB:
```
// (lldb) target dump section-load-list
// addr = 0x, section = 0x55d46ab8c510: 0xfffd 
container[0x-0x0628)  r--  0x 
0x0628 0x a.out.PT_LOAD[0]
// addr = 0x1000, section = 0x55d46ab8b0c0: 0xfffc 
container[0x1000-0x1185)  r-x  0x1000 
0x0185 0x a.out.PT_LOAD[1]
// addr = 0x2000, section = 0x55d46ac040f0: 0xfffb 
container[0x2000-0x20cc)  r--  0x2000 
0x00cc 0x a.out.PT_LOAD[2]
// addr = 0x3db0, section = 0x55d46ab7cef0: 0xfff6 
container[0x3db0-0x3db4)  r--  0x2db0 
0x 0x a.out.PT_TLS[0]
```
And this for the fixed LLDB:
```
// (lldb) target dump section-load-list
// addr = 0x, section = 0x105f0a9a8: 0xfffd 
container[0x-0x0628)  r--  0x 
0x0628 0x a.out.PT_LOAD[0]
// addr = 0x1000, section = 0x105f0adb8: 0xfffc 
container[0x1000-0x1185)  r-x  0x1000 
0x0185 0x a.out.PT_LOAD[1]
// addr = 0x2000, section = 0x105f0af48: 0xfffb 
container[0x2000-0x20cc)  r--  0x2000 
0x00cc 0x a.out.PT_LOAD[2]
// addr = 0x3db0, section = 0x105f0b078: 0xfffa 
container[0x3db0-0x4028)  rw-  0x2db0 
0x0274 0x a.out.PT_LOAD[3]
```
We can see that previously the "PT_LOAD[3]" segment would be removed
from the section load list, and after the fix it remains and there is on
PT_TLS in the loaded sections.

Added: 


Modified: 
lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
lldb/test/Shell/ObjectFile/ELF/PT_TLS-overlap-PT_LOAD.yaml

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp 
b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
index 5c6b475044be5..51bd34e95c77d 100644
--- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -717,6 +717,20 @@ bool ObjectFileELF::SetLoadAddress(Target &target, 
lldb::addr_t value,
 // Iterate through the object file sections to find all of the sections
 // that have SHF_ALLOC in their flag bits

[Lldb-commits] [lldb] [lldb] Fix a bug for PT_TLS segments getting loaded when they shouldn't. (PR #98432)

[Greg Clayton via lldb-commits]

https://github.com/clayborg closed 
https://github.com/llvm/llvm-project/pull/98432
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Fix test assertions in TestDAP_stepInTargets.py (PR #96687)

[Kendal Harland via lldb-commits]

@@ -55,14 +55,23 @@ def test_basic(self):
 self.assertEqual(len(step_in_targets), 3, "expect 3 step in targets")
 
 # Verify the target names are correct.
-self.assertEqual(step_in_targets[0]["label"], "bar()", "expect bar()")
-self.assertEqual(step_in_targets[1]["label"], "bar2()", "expect 
bar2()")
-self.assertEqual(
-step_in_targets[2]["label"], "foo(int, int)", "expect foo(int, 
int)"
-)
+# The order of funcA and funcB may change depending on the compiler 
ABI.
+funcA_target = None
+funcB_target = None
+for target in step_in_targets[0:2]:
+if "funcB" in target["label"]:
+funcB_target = target
+elif "funcA" in target["label"]:
+funcA_target = target
+else:
+self.fail(f"Unexpected step in target: {target}")
+
+self.assertIsNotNone(funcA_target, "expect funcA")
+self.assertIsNotNone(funcB_target, "expect funcB")
+self.assertIn("foo", step_in_targets[2]["label"], "expect foo")
 
-# Choose to step into second target and verify that we are in bar2()
-self.stepIn(threadId=tid, targetId=step_in_targets[1]["id"], 
waitForStop=True)
+# Choose to step into second target and verify that we are in funcB()

kendalharland wrote:

That's reasonable. I'll mark as disabled for now and file a bug to investigate. 
I can look into it once I get the time.

https://github.com/llvm/llvm-project/pull/96687
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Add AST node classes, functions, etc. for Data Inspection Lang… (PR #95738)

[via lldb-commits]

jimingham wrote:



> On Jul 10, 2024, at 9:22 PM, cmtice ***@***.***> wrote:
> 
> 
> @cmtice commented on this pull request.
> 
> In lldb/include/lldb/Core/DILAST.h 
> :
> 
> > +/// Checks to see if the CompilerType is a Smart Pointer (shared, unique, 
> > weak)
> +/// or not. Only applicable for C++, which is why this is here and not part 
> of
> +/// the CompilerType class.
> When we go to dereference a pointer we need to know whether or not it's a 
> smart pointer, because if it's a smart pointer we need to get at the pointer 
> inside before we can dereference it. So we use IsSmartPtrType to determine 
> whether or not to insert this layer of unwrapping (converting the smart ptr 
> into a normal ptr).
> 
I'm not sure it's the job of the DIL to do this sort of unwrapping.  Then it 
would have to support all the ways this might need to be done in every language 
we might end up supporting.  This seems more a job for the data formatters 
provided by the various languages.  That's why Pavel was asking about the 
special deference Synthetic Child Provider API, I think.  That provides a way 
for various languages to encode this sort of behavior w/o it having to be know 
in advance by lldb.

Jim



> —
> Reply to this email directly, view it on GitHub 
> , or 
> unsubscribe 
> .
> You are receiving this because you are on a team that was mentioned.
> 



https://github.com/llvm/llvm-project/pull/95738
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][man][nfc] Don't register a markdown parser when building man packages (PR #98420)

[Alan Zhao via lldb-commits]

https://github.com/alanzhao1 updated 
https://github.com/llvm/llvm-project/pull/98420

>From fab10fb2efe8265d1b403a650dbd2d3348f29b73 Mon Sep 17 00:00:00 2001
From: Alan Zhao 
Date: Wed, 10 Jul 2024 15:56:39 -0700
Subject: [PATCH 1/3] [lldb][man][nfc] Don't register a markdown parser when
 building man pages

This reduces Sphinx dependencies for building lldb man pages as lldb man
pages don't use markdown.
---
 lldb/docs/conf.py | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lldb/docs/conf.py b/lldb/docs/conf.py
index 27a1cd7c3c31a..805a6dc19ac81 100644
--- a/lldb/docs/conf.py
+++ b/lldb/docs/conf.py
@@ -89,9 +89,13 @@
 # The suffix of source filenames.
 source_suffix = {
 ".rst": "restructuredtext",
-".md": "markdown",
 }
 
+# Man pages do not use markdown pages, so we don't need to register a markdown
+# parser.
+if not building_man_page:
+  source_suffix[".md"] = "markdown"
+
 # The encoding of source files.
 # source_encoding = 'utf-8-sig'
 

>From f89c189303a4a22e9087b190612b7d483238b703 Mon Sep 17 00:00:00 2001
From: Alan Zhao 
Date: Wed, 10 Jul 2024 17:30:44 -0700
Subject: [PATCH 2/3] fix formatting

---
 lldb/docs/conf.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/docs/conf.py b/lldb/docs/conf.py
index 805a6dc19ac81..233cdf7501934 100644
--- a/lldb/docs/conf.py
+++ b/lldb/docs/conf.py
@@ -94,7 +94,7 @@
 # Man pages do not use markdown pages, so we don't need to register a markdown
 # parser.
 if not building_man_page:
-  source_suffix[".md"] = "markdown"
+source_suffix[".md"] = "markdown"
 
 # The encoding of source files.
 # source_encoding = 'utf-8-sig'

>From cdb7be745cd2f16989829cc1953c2066f37f098c Mon Sep 17 00:00:00 2001
From: Alan Zhao 
Date: Thu, 11 Jul 2024 10:23:06 -0700
Subject: [PATCH 3/3] move source_suffix

---
 lldb/docs/conf.py | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/lldb/docs/conf.py b/lldb/docs/conf.py
index 233cdf7501934..9b005c9537f23 100644
--- a/lldb/docs/conf.py
+++ b/lldb/docs/conf.py
@@ -64,6 +64,11 @@
 
 autodoc_default_options = {"special-members": True}
 
+# The suffix of source filenames.
+source_suffix = {
+".rst": "restructuredtext",
+}
+
 # Unless we only generate the basic manpage we need the plugin for generating
 # the Python API documentation.
 if not building_man_page:
@@ -83,19 +88,13 @@
 # a list of builtin themes.
 html_theme = "furo"
 
+# Since man pages do not use markdown, we do not need to register a 
markdown
+# parser.
+source_suffix[".md"] = "markdown"
+
 # Add any paths that contain templates here, relative to this directory.
 templates_path = ["_templates"]
 
-# The suffix of source filenames.
-source_suffix = {
-".rst": "restructuredtext",
-}
-
-# Man pages do not use markdown pages, so we don't need to register a markdown
-# parser.
-if not building_man_page:
-source_suffix[".md"] = "markdown"
-
 # The encoding of source files.
 # source_encoding = 'utf-8-sig'
 

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

[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #98361)

[Zequan Wu via lldb-commits]

@@ -1631,26 +1638,48 @@ bool SymbolFileDWARF::CompleteType(CompilerType 
&compiler_type) {
 return true;
   }
 
-  DWARFDIE dwarf_die = GetDIE(die_it->getSecond());
-  if (dwarf_die) {
-// Once we start resolving this type, remove it from the forward
-// declaration map in case anyone child members or other types require this
-// type to get resolved. The type will get resolved when all of the calls
-// to SymbolFileDWARF::ResolveClangOpaqueTypeDefinition are done.
-GetForwardDeclCompilerTypeToDIE().erase(die_it);
+  DWARFDIE decl_die = GetDIE(die_it->getSecond());
+  // Once we start resolving this type, remove it from the forward
+  // declaration map in case anyone's child members or other types require this
+  // type to get resolved.
+  GetForwardDeclCompilerTypeToDIE().erase(die_it);
+  DWARFDIE def_die = FindDefinitionDIE(decl_die);
+  if (!def_die) {
+SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
+if (debug_map_symfile) {
+  // We weren't able to find a full declaration in this DWARF, see
+  // if we have a declaration anywhere else...
+  def_die = debug_map_symfile->FindDefinitionDIE(decl_die);
+}
+  }
+  if (!def_die) {
+// No definition found. Proceed with the declaration die. We can use it to
+// create a forward-declared type.
+def_die = decl_die;
+  }
 
-Type *type = GetDIEToType().lookup(dwarf_die.GetDIE());
+  Type *type = ResolveType(def_die);

ZequanWu wrote:

`ResolveType(def_die)` just do [this 
part](https://github.com/llvm/llvm-project/pull/98361/files/37b6878b9125c314c75053f7d5b0ba520111e9a3#diff-5dd6736e4d6c38623630df16c4494c1a8b099716ee0f05c9af54b4bafb1d864eR1607-R1674)
 of work and then return the type, which was created from the declaration DIE. 
It doesn't create a loop or recursion. We need it to do the work when finding 
existing type in the UniqueDWARFASTTypeMap, which is basically this block: 
https://github.com/llvm/llvm-project/pull/98361/files/37b6878b9125c314c75053f7d5b0ba520111e9a3#diff-5dd6736e4d6c38623630df16c4494c1a8b099716ee0f05c9af54b4bafb1d864eR1648-R1674.
 Those code are important to keep some maps(`GetDIEToType()`, 
`LinkDeclContextToDIE`, `UniqueDWARFASTTypeMap`) updated.

https://github.com/llvm/llvm-project/pull/98361
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere commented:

This is looking good, but needs a (pexpect) test. I would also like to add a 
setting to make the prefix and suffix configurable, like we do for things like 
`show-progress-ansi-prefix` and `show-autosuggestion-ansi-prefix`. That way 
user can pick their own preferred color, or even just turn this one feature off 
without having to disable color if they want to. 

https://github.com/llvm/llvm-project/pull/91404
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Fix test assertions in TestDAP_stepInTargets.py (PR #96687)

[Kendal Harland via lldb-commits]

https://github.com/kendalharland updated 
https://github.com/llvm/llvm-project/pull/96687

>From 02f06f90a40cc7ed18a9744918acf4daf6212486 Mon Sep 17 00:00:00 2001
From: kendal 
Date: Mon, 24 Jun 2024 14:01:31 -0700
Subject: [PATCH 1/2] Fix test assertions in TestDAP_stepInTargets.py

---
 .../stepInTargets/TestDAP_stepInTargets.py| 24 +--
 .../API/tools/lldb-dap/stepInTargets/main.cpp |  6 ++---
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git 
a/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py 
b/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py
index 6296f6554d07e..6670989a58fe8 100644
--- a/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py
+++ b/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py
@@ -55,14 +55,24 @@ def test_basic(self):
 self.assertEqual(len(step_in_targets), 3, "expect 3 step in targets")
 
 # Verify the target names are correct.
-self.assertEqual(step_in_targets[0]["label"], "bar()", "expect bar()")
-self.assertEqual(step_in_targets[1]["label"], "bar2()", "expect 
bar2()")
-self.assertEqual(
-step_in_targets[2]["label"], "foo(int, int)", "expect foo(int, 
int)"
-)
+# The order of funcA and funcB may change depending on the compiler 
ABI.
+funcA_target = None
+funcB_target = None
+for target in step_in_targets[0:2]:
+if "funcB" in target["label"]:
+funcB_target = target
+elif "funcA" in target["label"]:
+funcA_target = target
+else:
+self.fail(f"Unexpected step in target: {target}")
+
+self.assertIsNotNone(funcA_target, "expect funcA")
+self.assertIsNotNone(funcB_target, "expect funcB")
+self.assertIn("foo", step_in_targets[2]["label"], "expect foo")
 
-# Choose to step into second target and verify that we are in bar2()
+# Choose to step into second target and verify that we are in the 
second target,
+# be it funcA or funcB.
 self.stepIn(threadId=tid, targetId=step_in_targets[1]["id"], 
waitForStop=True)
 leaf_frame = self.dap_server.get_stackFrame()
 self.assertIsNotNone(leaf_frame, "expect a leaf frame")
-self.assertEqual(leaf_frame["name"], "bar2()")
+self.assertEqual(step_in_targets[1]["label"], leaf_frame["name"])
diff --git a/lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp 
b/lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp
index d3c3dbcc139ef..a48b79af0c760 100644
--- a/lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp
+++ b/lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp
@@ -1,11 +1,11 @@
 
 int foo(int val, int extra) { return val + extra; }
 
-int bar() { return 22; }
+int funcA() { return 22; }
 
-int bar2() { return 54; }
+int funcB() { return 54; }
 
 int main(int argc, char const *argv[]) {
-  foo(bar(), bar2()); // set breakpoint here
+  foo(funcA(), funcB()); // set breakpoint here
   return 0;
 }

>From f993be5d9e1b883aba7c9e998195958039e6ab27 Mon Sep 17 00:00:00 2001
From: kendal 
Date: Thu, 11 Jul 2024 11:52:03 -0700
Subject: [PATCH 2/2] Disable this test on all platforms with issue ID

---
 .../tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py  | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git 
a/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py 
b/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py
index 6670989a58fe8..0dfc19055b2eb 100644
--- a/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py
+++ b/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py
@@ -10,9 +10,10 @@
 
 
 class TestDAP_stepInTargets(lldbdap_testcase.DAPTestCaseBase):
-@skipIf(
-archs=no_match(["x86_64"])
-)  # InstructionControlFlowKind for ARM is not supported yet.
+@expectedFailureAll
+# InstructionControlFlowKind for ARM is not supported yet.
+# On x86_64 lldb-dap seems to ignore targetId when stepping into functions.
+# For more context, see https://github.com/llvm/llvm-project/issues/98509.
 def test_basic(self):
 """
 Tests the basic stepping in targets with directly calls.

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

[Lldb-commits] [lldb] Fix test assertions in TestDAP_stepInTargets.py (PR #96687)

[Kendal Harland via lldb-commits]

@@ -55,14 +55,23 @@ def test_basic(self):
 self.assertEqual(len(step_in_targets), 3, "expect 3 step in targets")
 
 # Verify the target names are correct.
-self.assertEqual(step_in_targets[0]["label"], "bar()", "expect bar()")
-self.assertEqual(step_in_targets[1]["label"], "bar2()", "expect 
bar2()")
-self.assertEqual(
-step_in_targets[2]["label"], "foo(int, int)", "expect foo(int, 
int)"
-)
+# The order of funcA and funcB may change depending on the compiler 
ABI.
+funcA_target = None
+funcB_target = None
+for target in step_in_targets[0:2]:
+if "funcB" in target["label"]:
+funcB_target = target
+elif "funcA" in target["label"]:
+funcA_target = target
+else:
+self.fail(f"Unexpected step in target: {target}")
+
+self.assertIsNotNone(funcA_target, "expect funcA")
+self.assertIsNotNone(funcB_target, "expect funcB")
+self.assertIn("foo", step_in_targets[2]["label"], "expect foo")
 
-# Choose to step into second target and verify that we are in bar2()
-self.stepIn(threadId=tid, targetId=step_in_targets[1]["id"], 
waitForStop=True)
+# Choose to step into second target and verify that we are in funcB()

kendalharland wrote:

Updated in the latest commit.

https://github.com/llvm/llvm-project/pull/96687
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Fix section printing to always align. (PR #98521)

[Greg Clayton via lldb-commits]

https://github.com/clayborg created 
https://github.com/llvm/llvm-project/pull/98521

Section IDs are 64 bit and if a section ID was over 4GB, then the tabular 
output of the "target modules dump sections" command would not align to the 
column headers. Also if the section type's name was too long, the output 
wouldn't algin. This patch fixes this issue.

Old output looked like:
```
(lldb) image dump sections a.out
Sections for '/tmp/a.out' (arm):
  SectID Type File Address Perm 
File Off.  File Size  Flags  Section Name
  --  ---   
-- -- -- 
  0x container[0x1000-0x1010)  
rw-  0x0074 0x0010 0x a.out.PT_LOAD[0]
  0x0001 data [0x1000-0x1010)  rw-  
0x0074 0x0010 0x0003 a.out.PT_LOAD[0]..data
  0xfffe container[0x1000-0x1010)  
rw-  0x0084 0x 0x a.out.PT_TLS[0]
  0x0002 zero-fill[0x1000-0x1010)  rw-  
0x0084 0x 0x0403 a.out.PT_TLS[0]..tbss
  0x0003 regular   ---  
0x0084 0x0001 0x a.out..strtab
  0x0004 regular   ---  
0x0085 0x001f 0x a.out..shstrtab
```
New output looks like:
```
(lldb) image dump sections a.out
Sections for '/tmp/a.out' (arm):
  SectID Type   File Address
 Perm File Off.  File Size  Flags  Section Name
  -- -- 
---   -- -- -- 

  0x container  
[0x1000-0x1010)  rw-  0x0074 0x0010 0x 
a.out.PT_LOAD[0]
  0x0001 data   
[0x1000-0x1010)  rw-  0x0074 0x0010 0x0003 
a.out.PT_LOAD[0]..data
  0xfffe container  
[0x1000-0x1010)  rw-  0x0084 0x 0x 
a.out.PT_TLS[0]
  0x0002 zero-fill  
[0x1000-0x1010)  rw-  0x0084 0x 0x0403 
a.out.PT_TLS[0]..tbss
  0x0003 regular
 ---  0x0084 0x0001 0x a.out..strtab
  0x0004 regular
 ---  0x0085 0x001f 0x a.out..shstrtab
```

>From ea7e58cf0677e6828b59038a527a4b23c189e548 Mon Sep 17 00:00:00 2001
From: Greg Clayton 
Date: Thu, 11 Jul 2024 11:57:11 -0700
Subject: [PATCH] Fix section printing to always align.

Section IDs are 64 bit and if a section ID was over 4GB, then the tabular 
output of the "target modules dump sections" command would not align to the 
column headers. Also if the section type's name was too long, the output 
wouldn't algin. This patch fixes this issue.

Old output looked like:
```
(lldb) image dump sections a.out
Sections for '/tmp/a.out' (arm):
  SectID Type File Address Perm 
File Off.  File Size  Flags  Section Name
  --  ---   
-- -- -- 
  0x container[0x1000-0x1010)  
rw-  0x0074 0x0010 0x a.out.PT_LOAD[0]
  0x0001 data [0x1000-0x1010)  rw-  
0x0074 0x0010 0x0003 a.out.PT_LOAD[0]..data
  0xfffe container[0x1000-0x1010)  
rw-  0x0084 0x 0x a.out.PT_TLS[0]
  0x0002 zero-fill[0x1000-0x1010)  rw-  
0x0084 0x 0x0403 a.out.PT_TLS[0]..tbss
  0x0003 regular   ---  
0x0084 0x0001 0x a.out..strtab
  0x0004 regular   ---  
0x0085 0x001f 0x a.out..shstrtab
```
New output looks like:
```
(lldb) image dump sections a.out
Sections for '/tmp/a.out' (arm):
  SectID Type   File Address
 Perm File Off.  File Size  Flags  Section Name
  -- -- 
---   -- -- -- 

  0x container  
[0x1000-0x1010)  rw-  0x0074 0x0010 0x 
a.out.PT_LOAD[0]
  0x0001 data   
[0x1000-0x1010)  rw-  0x0074 0x0010 0x0003

[Lldb-commits] [lldb] [lldb] Fix section printing to always align. (PR #98521)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Greg Clayton (clayborg)


Changes

Section IDs are 64 bit and if a section ID was over 4GB, then the tabular 
output of the "target modules dump sections" command would not align to the 
column headers. Also if the section type's name was too long, the output 
wouldn't algin. This patch fixes this issue.

Old output looked like:
```
(lldb) image dump sections a.out
Sections for '/tmp/a.out' (arm):
  SectID Type File Address Perm 
File Off.  File Size  Flags  Section Name
  --  ---   
-- -- -- 
  0x container[0x1000-0x1010)  
rw-  0x0074 0x0010 0x a.out.PT_LOAD[0]
  0x0001 data [0x1000-0x1010)  rw-  
0x0074 0x0010 0x0003 a.out.PT_LOAD[0]..data
  0xfffe container[0x1000-0x1010)  
rw-  0x0084 0x 0x a.out.PT_TLS[0]
  0x0002 zero-fill[0x1000-0x1010)  rw-  
0x0084 0x 0x0403 a.out.PT_TLS[0]..tbss
  0x0003 regular   ---  
0x0084 0x0001 0x a.out..strtab
  0x0004 regular   ---  
0x0085 0x001f 0x a.out..shstrtab
```
New output looks like:
```
(lldb) image dump sections a.out
Sections for '/tmp/a.out' (arm):
  SectID Type   File Address
 Perm File Off.  File Size  Flags  Section Name
  -- -- 
---   -- -- -- 

  0x container  
[0x1000-0x1010)  rw-  0x0074 0x0010 0x 
a.out.PT_LOAD[0]
  0x0001 data   
[0x1000-0x1010)  rw-  0x0074 0x0010 0x0003 
a.out.PT_LOAD[0]..data
  0xfffe container  
[0x1000-0x1010)  rw-  0x0084 0x 0x 
a.out.PT_TLS[0]
  0x0002 zero-fill  
[0x1000-0x1010)  rw-  0x0084 0x 0x0403 
a.out.PT_TLS[0]..tbss
  0x0003 regular
 ---  0x0084 0x0001 0x a.out..strtab
  0x0004 regular
 ---  0x0085 0x001f 0x a.out..shstrtab
```

---
Full diff: https://github.com/llvm/llvm-project/pull/98521.diff


2 Files Affected:

- (modified) lldb/source/Core/Section.cpp (+5-7) 
- (modified) lldb/test/Shell/Commands/command-target-modules-dump-sections.yaml 
(+7-7) 


``diff
diff --git a/lldb/source/Core/Section.cpp b/lldb/source/Core/Section.cpp
index 9e98b59deb035..0763e88d4608f 100644
--- a/lldb/source/Core/Section.cpp
+++ b/lldb/source/Core/Section.cpp
@@ -275,7 +275,7 @@ bool Section::ContainsFileAddress(addr_t vm_addr) const {
 void Section::Dump(llvm::raw_ostream &s, unsigned indent, Target *target,
uint32_t depth) const {
   s.indent(indent);
-  s << llvm::format("0x%8.8" PRIx64 " %-16s ", GetID(), GetTypeAsCString());
+  s << llvm::format("0x%16.16" PRIx64 " %-22s ", GetID(), GetTypeAsCString());
   bool resolved = true;
   addr_t addr = LLDB_INVALID_ADDRESS;
 
@@ -642,14 +642,12 @@ void SectionList::Dump(llvm::raw_ostream &s, unsigned 
indent, Target *target,
   if (show_header && !m_sections.empty()) {
 s.indent(indent);
 s << llvm::formatv(
-"SectID Type {0} Address  "
-"   Perm File Off.  File Size  Flags "
-" Section Name\n",
+"SectID Type   {0} Address"
+" Perm File Off.  File Size  Flags  Section Name\n",
 target_has_loaded_sections ? "Load" : "File");
 s.indent(indent);
-s << "--  "
- "---   -- "
- "-- "
+s << "-- -- "
+ "---   -- -- "
  "-- \n";
   }
 
diff --git a/lldb/test/Shell/Commands/command-target-modules-dump-sections.yaml 
b/lldb/test/Shell/Commands/command-target-modules-dump-sections.yaml
index 1fb06f651df8e..d75a24c60123c 100644
--- a/lldb/test/Shell/Commands/command-target-modules-dump-sections.yaml
+++ b/lldb/test/Shell/Commands/command-target-modules-dump-sections.yaml
@@ -3,18 +3,18 @@
 # RUN:   | FileCheck --match-full-lines --strict-whitespace %s
 
 #  CHECK:Sections for 
'{{.

[Lldb-commits] [lldb] [lldb][breakpoint] Grey out disabled breakpoints (PR #91404)

[Chelsea Cassanova via lldb-commits]

chelcassanova wrote:

Just making sure, a setting for changing the prefix/suffix would be in 
`CoreProperties.td` right?

https://github.com/llvm/llvm-project/pull/91404
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 43024a4 - [lldb][man][nfc] Don't register a markdown parser when building man packages (#98420)

[via lldb-commits]

Author: Alan Zhao
Date: 2024-07-11T12:54:26-07:00
New Revision: 43024a465120315464490ca9c6074553cd968089

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

LOG: [lldb][man][nfc] Don't register a markdown parser when building man 
packages (#98420)

This reduces Sphinx dependencies for building lldb man pages as lldb man
pages don't use markdown.

Added: 


Modified: 
lldb/docs/conf.py

Removed: 




diff  --git a/lldb/docs/conf.py b/lldb/docs/conf.py
index 27a1cd7c3c31a..9b005c9537f23 100644
--- a/lldb/docs/conf.py
+++ b/lldb/docs/conf.py
@@ -64,6 +64,11 @@
 
 autodoc_default_options = {"special-members": True}
 
+# The suffix of source filenames.
+source_suffix = {
+".rst": "restructuredtext",
+}
+
 # Unless we only generate the basic manpage we need the plugin for generating
 # the Python API documentation.
 if not building_man_page:
@@ -83,15 +88,13 @@
 # a list of builtin themes.
 html_theme = "furo"
 
+# Since man pages do not use markdown, we do not need to register a 
markdown
+# parser.
+source_suffix[".md"] = "markdown"
+
 # Add any paths that contain templates here, relative to this directory.
 templates_path = ["_templates"]
 
-# The suffix of source filenames.
-source_suffix = {
-".rst": "restructuredtext",
-".md": "markdown",
-}
-
 # The encoding of source files.
 # source_encoding = 'utf-8-sig'
 



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

[Lldb-commits] [lldb] [lldb][man][nfc] Don't register a markdown parser when building man packages (PR #98420)

[Alan Zhao via lldb-commits]

https://github.com/alanzhao1 closed 
https://github.com/llvm/llvm-project/pull/98420
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [llvm] [DRAFT][llvm]Added lib/Telemetry (PR #98528)

[Nikita Popov via lldb-commits]

nikic wrote:

Why does any part of this need to be in llvm/ at all?

https://github.com/llvm/llvm-project/pull/98528
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] ea4ae25 - [lldb] Fix section printing to always align. (#98521)

[via lldb-commits]

Author: Greg Clayton
Date: 2024-07-11T12:58:24-07:00
New Revision: ea4ae2590dea6ab5acf790a6098863d4ba63300f

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

LOG: [lldb] Fix section printing to always align. (#98521)

Section IDs are 64 bit and if a section ID was over 4GB, then the
tabular output of the "target modules dump sections" command would not
align to the column headers. Also if the section type's name was too
long, the output wouldn't algin. This patch fixes this issue.

Old output looked like:
```
(lldb) image dump sections a.out
Sections for '/tmp/a.out' (arm):
  SectID Type File Address Perm 
File Off.  File Size  Flags  Section Name
  --  ---   
-- -- -- 
  0x container[0x1000-0x1010)  
rw-  0x0074 0x0010 0x a.out.PT_LOAD[0]
  0x0001 data [0x1000-0x1010)  rw-  
0x0074 0x0010 0x0003 a.out.PT_LOAD[0]..data
  0xfffe container[0x1000-0x1010)  
rw-  0x0084 0x 0x a.out.PT_TLS[0]
  0x0002 zero-fill[0x1000-0x1010)  rw-  
0x0084 0x 0x0403 a.out.PT_TLS[0]..tbss
  0x0003 regular   ---  
0x0084 0x0001 0x a.out..strtab
  0x0004 regular   ---  
0x0085 0x001f 0x a.out..shstrtab
```
New output looks like:
```
(lldb) image dump sections a.out
Sections for '/tmp/a.out' (arm):
  SectID Type   File Address
 Perm File Off.  File Size  Flags  Section Name
  -- -- 
---   -- -- -- 

  0x container  
[0x1000-0x1010)  rw-  0x0074 0x0010 0x 
a.out.PT_LOAD[0]
  0x0001 data   
[0x1000-0x1010)  rw-  0x0074 0x0010 0x0003 
a.out.PT_LOAD[0]..data
  0xfffe container  
[0x1000-0x1010)  rw-  0x0084 0x 0x 
a.out.PT_TLS[0]
  0x0002 zero-fill  
[0x1000-0x1010)  rw-  0x0084 0x 0x0403 
a.out.PT_TLS[0]..tbss
  0x0003 regular
 ---  0x0084 0x0001 0x a.out..strtab
  0x0004 regular
 ---  0x0085 0x001f 0x a.out..shstrtab
```

Added: 


Modified: 
lldb/source/Core/Section.cpp
lldb/test/Shell/Commands/command-target-modules-dump-sections.yaml

Removed: 




diff  --git a/lldb/source/Core/Section.cpp b/lldb/source/Core/Section.cpp
index 9e98b59deb035..0763e88d4608f 100644
--- a/lldb/source/Core/Section.cpp
+++ b/lldb/source/Core/Section.cpp
@@ -275,7 +275,7 @@ bool Section::ContainsFileAddress(addr_t vm_addr) const {
 void Section::Dump(llvm::raw_ostream &s, unsigned indent, Target *target,
uint32_t depth) const {
   s.indent(indent);
-  s << llvm::format("0x%8.8" PRIx64 " %-16s ", GetID(), GetTypeAsCString());
+  s << llvm::format("0x%16.16" PRIx64 " %-22s ", GetID(), GetTypeAsCString());
   bool resolved = true;
   addr_t addr = LLDB_INVALID_ADDRESS;
 
@@ -642,14 +642,12 @@ void SectionList::Dump(llvm::raw_ostream &s, unsigned 
indent, Target *target,
   if (show_header && !m_sections.empty()) {
 s.indent(indent);
 s << llvm::formatv(
-"SectID Type {0} Address  "
-"   Perm File Off.  File Size  Flags "
-" Section Name\n",
+"SectID Type   {0} Address"
+" Perm File Off.  File Size  Flags  Section Name\n",
 target_has_loaded_sections ? "Load" : "File");
 s.indent(indent);
-s << "--  "
- "---   -- "
- "-- "
+s << "-- -- "
+ "---   -- -- "
  "-- \n";
   }
 

diff  --git 
a/lldb/test/Shell/Commands/command-target-modules-dump-sections.yaml 
b/lldb/test/Shell/Commands/command-target-modules-dump-sections.yaml
index 1fb06f651df8e..d75a24c60123c 100644
--- a/lld

[Lldb-commits] [lldb] [lldb] Fix section printing to always align. (PR #98521)

[Greg Clayton via lldb-commits]

https://github.com/clayborg closed 
https://github.com/llvm/llvm-project/pull/98521
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #98361)

[Pavel Labath via lldb-commits]

@@ -1631,26 +1638,48 @@ bool SymbolFileDWARF::CompleteType(CompilerType 
&compiler_type) {
 return true;
   }
 
-  DWARFDIE dwarf_die = GetDIE(die_it->getSecond());
-  if (dwarf_die) {
-// Once we start resolving this type, remove it from the forward
-// declaration map in case anyone child members or other types require this
-// type to get resolved. The type will get resolved when all of the calls
-// to SymbolFileDWARF::ResolveClangOpaqueTypeDefinition are done.
-GetForwardDeclCompilerTypeToDIE().erase(die_it);
+  DWARFDIE decl_die = GetDIE(die_it->getSecond());
+  // Once we start resolving this type, remove it from the forward
+  // declaration map in case anyone's child members or other types require this
+  // type to get resolved.
+  GetForwardDeclCompilerTypeToDIE().erase(die_it);
+  DWARFDIE def_die = FindDefinitionDIE(decl_die);
+  if (!def_die) {
+SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
+if (debug_map_symfile) {
+  // We weren't able to find a full declaration in this DWARF, see
+  // if we have a declaration anywhere else...
+  def_die = debug_map_symfile->FindDefinitionDIE(decl_die);
+}
+  }
+  if (!def_die) {
+// No definition found. Proceed with the declaration die. We can use it to
+// create a forward-declared type.
+def_die = decl_die;
+  }
 
-Type *type = GetDIEToType().lookup(dwarf_die.GetDIE());
+  Type *type = ResolveType(def_die);

labath wrote:

> We need it to do the work when finding existing type in the 
> UniqueDWARFASTTypeMap,

Why do we need to find that type? We already know which type we're completing 
(we get it as an argument to `SymbolFileDWARF::CompleteType`)...

> It doesn't create a loop or recursion. We need it to do the work when finding 
> existing type in the UniqueDWARFASTTypeMap, which is basically this block: 
> https://github.com/llvm/llvm-project/pull/98361/files/37b6878b9125c314c75053f7d5b0ba520111e9a3#diff-5dd6736e4d6c38623630df16c4494c1a8b099716ee0f05c9af54b4bafb1d864eR1648-R1674.

That's only true if `UniqueDWARFASTTypeMap` functions perfectly and returns the 
same type we started out with. My point is we shouldn't/don't need to rely on 
that.

> Those code are important to keep some maps(`GetDIEToType()`, 
> `LinkDeclContextToDIE`, `UniqueDWARFASTTypeMap`) updated

Yes, we need to update those maps, but I think we can/should just update those 
directly, instead of relying on it to be populated as a side-effect of the 
second ParseTypeFromDWARF call.  Like, we already have the type and the 
definition die, so why don't we just do a `GetDIEToType()[defn_die] = type`? 
And for the UniqueDWARFASTTypeMap, we can use the original declaration die to 
perform the lookup. This will be both faster (we can add a specialized lookup 
function that assumes that the lookup die matches one of the `udt.m_die` 
members) and guaranteed to be correct.

https://github.com/llvm/llvm-project/pull/98361
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test] Fix ComputeClangResourceDirectory test when CLANG_RESOURCE_DIR is set (PR #98464)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere approved this pull request.

This is more robust and I like that it doesn't risk breaking the clang resource 
directory path. LGTM. 

https://github.com/llvm/llvm-project/pull/98464
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb][test] Fix ComputeClangResourceDirectory test when CLANG_RESOURCE_DIR is set (PR #98464)

[Alex Langford via lldb-commits]

https://github.com/bulbazord approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/98464
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add support for displaying `__float128` variables (PR #98369)

[via lldb-commits]

https://github.com/beetrees edited 
https://github.com/llvm/llvm-project/pull/98369
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #98361)

[Zequan Wu via lldb-commits]

https://github.com/ZequanWu updated 
https://github.com/llvm/llvm-project/pull/98361

>From 37b6878b9125c314c75053f7d5b0ba520111e9a3 Mon Sep 17 00:00:00 2001
From: Zequan Wu 
Date: Tue, 9 Jul 2024 15:28:19 -0700
Subject: [PATCH 1/2] Reapply [lldb][DWARF] Delay struct/class/union definition
 DIE searching when parsing declaration DIEs.

---
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp  | 279 --
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp  |  67 +++--
 .../SymbolFile/DWARF/SymbolFileDWARF.h|  15 +-
 .../DWARF/SymbolFileDWARFDebugMap.h   |   9 +
 .../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp   |   2 +-
 .../SymbolFile/DWARF/SymbolFileDWARFDwo.h |   3 +-
 .../SymbolFile/DWARF/UniqueDWARFASTType.cpp   | 117 
 .../SymbolFile/DWARF/UniqueDWARFASTType.h |  36 +--
 .../delayed-definition-die-searching.test |  36 +++
 .../x86/simple-template-names-context.cpp |   4 +-
 10 files changed, 301 insertions(+), 267 deletions(-)
 create mode 100644 
lldb/test/Shell/SymbolFile/DWARF/delayed-definition-die-searching.test

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 8e297141f4e13..7b93f6941ddda 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1603,41 +1603,74 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const 
DWARFDIE &die) {
 
 TypeSP
 DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
-   const DWARFDIE &decl_die,
+   const DWARFDIE &die,
ParsedDWARFTypeAttributes &attrs) {
   CompilerType clang_type;
-  const dw_tag_t tag = decl_die.Tag();
-  SymbolFileDWARF *dwarf = decl_die.GetDWARF();
-  LanguageType cu_language = SymbolFileDWARF::GetLanguage(*decl_die.GetCU());
+  const dw_tag_t tag = die.Tag();
+  SymbolFileDWARF *dwarf = die.GetDWARF();
+  LanguageType cu_language = SymbolFileDWARF::GetLanguage(*die.GetCU());
   Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
 
-  // UniqueDWARFASTType is large, so don't create a local variables on the
-  // stack, put it on the heap. This function is often called recursively and
-  // clang isn't good at sharing the stack space for variables in different
-  // blocks.
-  auto unique_ast_entry_up = std::make_unique();
-
   ConstString unique_typename(attrs.name);
   Declaration unique_decl(attrs.decl);
+  uint64_t byte_size = attrs.byte_size.value_or(0);
+  if (attrs.byte_size && *attrs.byte_size == 0 && attrs.name &&
+  !die.HasChildren() && cu_language == eLanguageTypeObjC) {
+// Work around an issue with clang at the moment where forward
+// declarations for objective C classes are emitted as:
+//  DW_TAG_structure_type [2]
+//  DW_AT_name( "ForwardObjcClass" )
+//  DW_AT_byte_size( 0x00 )
+//  DW_AT_decl_file( "..." )
+//  DW_AT_decl_line( 1 )
+//
+// Note that there is no DW_AT_declaration and there are no children,
+// and the byte size is zero.
+attrs.is_forward_declaration = true;
+  }
 
   if (attrs.name) {
 if (Language::LanguageIsCPlusPlus(cu_language)) {
   // For C++, we rely solely upon the one definition rule that says
   // only one thing can exist at a given decl context. We ignore the
   // file and line that things are declared on.
-  std::string qualified_name = GetCPlusPlusQualifiedName(decl_die);
+  std::string qualified_name = GetCPlusPlusQualifiedName(die);
   if (!qualified_name.empty())
 unique_typename = ConstString(qualified_name);
   unique_decl.Clear();
 }
 
-if (dwarf->GetUniqueDWARFASTTypeMap().Find(
-unique_typename, decl_die, unique_decl,
-attrs.byte_size.value_or(-1), *unique_ast_entry_up)) {
-  if (TypeSP type_sp = unique_ast_entry_up->m_type_sp) {
+if (UniqueDWARFASTType *unique_ast_entry_type =
+dwarf->GetUniqueDWARFASTTypeMap().Find(
+unique_typename, die, unique_decl, byte_size,
+attrs.is_forward_declaration)) {
+  if (TypeSP type_sp = unique_ast_entry_type->m_type_sp) {
+dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
 LinkDeclContextToDIE(
-GetCachedClangDeclContextForDIE(unique_ast_entry_up->m_die),
-decl_die);
+GetCachedClangDeclContextForDIE(unique_ast_entry_type->m_die), 
die);
+// If the DIE being parsed in this function is a definition and the
+// entry in the map is a declaration, then we need to update the entry
+// to point to the definition DIE.
+if (!attrs.is_forward_declaration &&
+unique_ast_entry_type->m_is_forward_declaration) {
+  unique_ast_entry_type->m_die = die;
+  unique_ast_entry_type->m_byte_size = byte_size;
+  un

[Lldb-commits] [lldb] 1988c27 - [lldb] [NFC] Update TestEarlyProcessLaunch to work on macOS 15

[Jason Molenda via lldb-commits]

Author: Jason Molenda
Date: 2024-07-11T16:51:28-07:00
New Revision: 1988c27e5f4dbcf42c9a80f44bdee7ccd208a0ac

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

LOG: [lldb] [NFC] Update TestEarlyProcessLaunch to work on macOS 15

This test sets a breakpoint on malloc, as a way to stop early in
dyld's setting up code, before the system libraries are initialized
so we can confirm that we don't fetch the Objective-C class table
before it's initialized.

In macOS 15 (macOS Sonoma), dyld doesn't call malloc any longer,
so this heuristic/trick isn't working.  It does call other things
called *alloc though, so I'm changing this to use a regex breakpoint
on that, to keep the test working.

Added: 


Modified: 
lldb/test/API/macosx/early-process-launch/TestEarlyProcessLaunch.py

Removed: 




diff  --git 
a/lldb/test/API/macosx/early-process-launch/TestEarlyProcessLaunch.py 
b/lldb/test/API/macosx/early-process-launch/TestEarlyProcessLaunch.py
index c15abbabc2374..ff704104f7892 100644
--- a/lldb/test/API/macosx/early-process-launch/TestEarlyProcessLaunch.py
+++ b/lldb/test/API/macosx/early-process-launch/TestEarlyProcessLaunch.py
@@ -1,6 +1,5 @@
 """Test that we don't read objc class tables early in process startup."""
 
-
 import time
 import lldb
 from lldbsuite.test.decorators import *
@@ -30,7 +29,14 @@ def test_early_process_launch(self):
 ###
 ### Use the types logging to detect the 
diff erence.
 
-target, process, _, bkpt = lldbutil.run_to_name_breakpoint(self, 
"malloc")
+exe = self.getBuildArtifact("a.out")
+target = self.dbg.CreateTarget(exe)
+self.assertTrue(target.IsValid())
+bkpt = target.BreakpointCreateByRegex("alloc", None)
+self.assertTrue(bkpt.IsValid())
+(target, process, thread, bkpt) = lldbutil.run_to_breakpoint_do_run(
+self, target, bkpt
+)
 
 target.DisableAllBreakpoints()
 target.BreakpointCreateByName("main")



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

[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #98361)

[Zequan Wu via lldb-commits]

@@ -1631,26 +1638,48 @@ bool SymbolFileDWARF::CompleteType(CompilerType 
&compiler_type) {
 return true;
   }
 
-  DWARFDIE dwarf_die = GetDIE(die_it->getSecond());
-  if (dwarf_die) {
-// Once we start resolving this type, remove it from the forward
-// declaration map in case anyone child members or other types require this
-// type to get resolved. The type will get resolved when all of the calls
-// to SymbolFileDWARF::ResolveClangOpaqueTypeDefinition are done.
-GetForwardDeclCompilerTypeToDIE().erase(die_it);
+  DWARFDIE decl_die = GetDIE(die_it->getSecond());
+  // Once we start resolving this type, remove it from the forward
+  // declaration map in case anyone's child members or other types require this
+  // type to get resolved.
+  GetForwardDeclCompilerTypeToDIE().erase(die_it);
+  DWARFDIE def_die = FindDefinitionDIE(decl_die);
+  if (!def_die) {
+SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
+if (debug_map_symfile) {
+  // We weren't able to find a full declaration in this DWARF, see
+  // if we have a declaration anywhere else...
+  def_die = debug_map_symfile->FindDefinitionDIE(decl_die);
+}
+  }
+  if (!def_die) {
+// No definition found. Proceed with the declaration die. We can use it to
+// create a forward-declared type.
+def_die = decl_die;
+  }
 
-Type *type = GetDIEToType().lookup(dwarf_die.GetDIE());
+  Type *type = ResolveType(def_die);

ZequanWu wrote:

That makes sense. I created another function to update those maps and use 
decl_die to lookup in UniqueDWARFASTTypeMap.

https://github.com/llvm/llvm-project/pull/98361
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #98361)

[via lldb-commits]

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 7021e44b2f0e11717c0d82456bad0fed4a0b48f9 
1e86ac615e751f0c98836690a1a8ca2b007be03b --extensions h,cpp -- 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp 
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h 
lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp 
lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.h 
lldb/test/Shell/SymbolFile/DWARF/x86/simple-template-names-context.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
index 5eb6da6bbe..9c19c72a0d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
@@ -109,9 +109,9 @@ public:
   std::string GetDIEClassTemplateParams(
   const lldb_private::plugin::dwarf::DWARFDIE &die) override;
 
-  void MappingDeclDIEToDefDIE(
-  const lldb_private::plugin::dwarf::DWARFDIE &decl_die,
-  const lldb_private::plugin::dwarf::DWARFDIE &def_die);
+  void
+  MappingDeclDIEToDefDIE(const lldb_private::plugin::dwarf::DWARFDIE &decl_die,
+ const lldb_private::plugin::dwarf::DWARFDIE &def_die);
 
 protected:
   /// Protected typedefs and members.

``




https://github.com/llvm/llvm-project/pull/98361
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Allow fetching of RA register when above fault handler (PR #98566)

[Jason Molenda via lldb-commits]

https://github.com/jasonmolenda created 
https://github.com/llvm/llvm-project/pull/98566

In RegisterContextUnwind::SavedLocationForRegister we have special logic for 
retrieving the Return Address register when it has the caller's return address 
in it. An example would be the lr register on AArch64.

This register is never retrieved from a newer stack frame because it is 
necessarly overwritten by a normal ABI function call.  We allow frame 0 to 
provide its lr value to get the caller's return address, if it has not been 
overwritten/saved to stack yet.

When a function is interrupted asynchronously by a POSIX signal (sigtramp), or 
a fault handler more generally, the sigtramp/fault handler has the entire 
register context available. In this situation, if the fault handler is frame 0, 
the function that was async interrupted is frame 1 and frame 2's return address 
may still be stored in lr.  We need to get the lr value for frame 1 from the 
fault handler in frame 0, to get the return address for frame 2.

Without this fix, a frameless function that faults in a firmware environment 
(that's where we've seen this issue most commonly) hasn't spilled lr to stack, 
so we need to retrieve it from the fault handler's full-register-context to 
find the caller of the frameless function that faulted.

It's an unsurprising fix, all of the work was finding exactly where in 
RegisterContextUnwind we were only allowing RA register use for frame 0, when 
it should have been frame 0 or above a fault handler function.

rdar://127518945

>From 48a06a9d85d2ca07e50bfda9002350d3ab157ed9 Mon Sep 17 00:00:00 2001
From: Jason Molenda 
Date: Thu, 11 Jul 2024 17:03:13 -0700
Subject: [PATCH] [lldb] Allow fetching of RA register when above fault handler

In RegisterContextUnwind::SavedLocationForRegister we have special
logic for retrieving the Return Address register when it has
the caller's return address in it. An example would be the lr
register on AArch64.

This register is never retrieved from a newer stack frame because
it is necessarly overwritten by a normal ABI function call.  We
allow frame 0 to provide its lr value to get the caller's return
address, if it has not been overwritten/saved to stack yet.

When a function is interrupted asynchronously by a POSIX signal
(sigtramp), or a fault handler more generally, the sigtramp/fault
handler has the entire register context available. In this situation,
if the fault handler is frame 0, the function that was async
interrupted is frame 1 and frame 2's return address may still be
stored in lr.  We need to get the lr value for frame 1 from the
fault handler in frame 0, to get the return address for frame 2.

Without this fix, a frameless function that faults in a firmware
environment (that's where we've seen this issue most commonly)
hasn't spilled lr to stack, so we need to retrieve it from the
fault handler's full-register-context to find the caller of the
frameless function that faulted.

It's an unsurprising fix, all of the work was finding exactly where
in RegisterContextUnwind we were only allowing RA register use for
frame 0, when it should have been frame 0 or above a fault handler
function.

rdar://127518945
---
 lldb/source/Target/RegisterContextUnwind.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/source/Target/RegisterContextUnwind.cpp 
b/lldb/source/Target/RegisterContextUnwind.cpp
index 95e8abd763d53..bc8081f4e3b31 100644
--- a/lldb/source/Target/RegisterContextUnwind.cpp
+++ b/lldb/source/Target/RegisterContextUnwind.cpp
@@ -1401,7 +1401,7 @@ RegisterContextUnwind::SavedLocationForRegister(
   // it's still live in the actual register. Handle this specially.
 
   if (!have_unwindplan_regloc && return_address_reg.IsValid() &&
-  IsFrameZero()) {
+  BehavesLikeZerothFrame()) {
 if (return_address_reg.GetAsKind(eRegisterKindLLDB) !=
 LLDB_INVALID_REGNUM) {
   lldb_private::UnwindLLDB::RegisterLocation new_regloc;

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

[Lldb-commits] [lldb] [lldb] Allow fetching of RA register when above fault handler (PR #98566)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jason Molenda (jasonmolenda)


Changes

In RegisterContextUnwind::SavedLocationForRegister we have special logic for 
retrieving the Return Address register when it has the caller's return address 
in it. An example would be the lr register on AArch64.

This register is never retrieved from a newer stack frame because it is 
necessarly overwritten by a normal ABI function call.  We allow frame 0 to 
provide its lr value to get the caller's return address, if it has not been 
overwritten/saved to stack yet.

When a function is interrupted asynchronously by a POSIX signal (sigtramp), or 
a fault handler more generally, the sigtramp/fault handler has the entire 
register context available. In this situation, if the fault handler is frame 0, 
the function that was async interrupted is frame 1 and frame 2's return address 
may still be stored in lr.  We need to get the lr value for frame 1 from the 
fault handler in frame 0, to get the return address for frame 2.

Without this fix, a frameless function that faults in a firmware environment 
(that's where we've seen this issue most commonly) hasn't spilled lr to stack, 
so we need to retrieve it from the fault handler's full-register-context to 
find the caller of the frameless function that faulted.

It's an unsurprising fix, all of the work was finding exactly where in 
RegisterContextUnwind we were only allowing RA register use for frame 0, when 
it should have been frame 0 or above a fault handler function.

rdar://127518945

---
Full diff: https://github.com/llvm/llvm-project/pull/98566.diff


1 Files Affected:

- (modified) lldb/source/Target/RegisterContextUnwind.cpp (+1-1) 


``diff
diff --git a/lldb/source/Target/RegisterContextUnwind.cpp 
b/lldb/source/Target/RegisterContextUnwind.cpp
index 95e8abd763d53..bc8081f4e3b31 100644
--- a/lldb/source/Target/RegisterContextUnwind.cpp
+++ b/lldb/source/Target/RegisterContextUnwind.cpp
@@ -1401,7 +1401,7 @@ RegisterContextUnwind::SavedLocationForRegister(
   // it's still live in the actual register. Handle this specially.
 
   if (!have_unwindplan_regloc && return_address_reg.IsValid() &&
-  IsFrameZero()) {
+  BehavesLikeZerothFrame()) {
 if (return_address_reg.GetAsKind(eRegisterKindLLDB) !=
 LLDB_INVALID_REGNUM) {
   lldb_private::UnwindLLDB::RegisterLocation new_regloc;

``




https://github.com/llvm/llvm-project/pull/98566
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Support single stopped event in lldb-dap (PR #98568)

[via lldb-commits]

https://github.com/jeffreytan81 created 
https://github.com/llvm/llvm-project/pull/98568

Some targets have non-trivial amount of threads, when stopping, multiple 
stopped events can be sent causing VSCode to show multiple yellow lines in code 
editor which can be confusing to end users. 

This PR introduces a new single stopped event mode in lldb-dap which ensures 
only one stopped event is sent to VSCode UI. The implementation is preferring 
the stop reason from original thread with `focus_tid`. 

Note: if `singleStoppedEvent` option is not specified, it functions the same as 
before.

New tests are added.

>From 13af0ff31688ca0a23f1fec65ca2d5797b65e31f Mon Sep 17 00:00:00 2001
From: jeffreytan81 
Date: Thu, 11 Jul 2024 17:24:41 -0700
Subject: [PATCH] Support single stopped event in lldb-dap

---
 .../test/tools/lldb-dap/dap_server.py | 21 -
 .../test/tools/lldb-dap/lldbdap_testcase.py   |  5 +-
 .../API/tools/lldb-dap/stop-events/Makefile   |  4 +
 .../stop-events/TestDAP_stopEvents.py | 92 +++
 .../API/tools/lldb-dap/stop-events/main.cpp   | 74 +++
 lldb/tools/lldb-dap/DAP.cpp   |  2 +-
 lldb/tools/lldb-dap/DAP.h |  3 +
 lldb/tools/lldb-dap/lldb-dap.cpp  | 47 +-
 8 files changed, 240 insertions(+), 8 deletions(-)
 create mode 100644 lldb/test/API/tools/lldb-dap/stop-events/Makefile
 create mode 100644 
lldb/test/API/tools/lldb-dap/stop-events/TestDAP_stopEvents.py
 create mode 100644 lldb/test/API/tools/lldb-dap/stop-events/main.cpp

diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index a324af57b61df..84e6fe82a06dc 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -125,6 +125,8 @@ def __init__(self, recv, send, init_commands, 
log_file=None):
 self.initialize_body = None
 self.thread_stop_reasons = {}
 self.breakpoint_events = []
+self.thread_events_body = []
+self.stopped_events = []
 self.progress_events = []
 self.reverse_requests = []
 self.sequence = 1
@@ -227,6 +229,8 @@ def handle_recv_packet(self, packet):
 # When a new process is attached or launched, remember the
 # details that are available in the body of the event
 self.process_event_body = body
+elif event == "continued":
+self.stopped_events = []
 elif event == "stopped":
 # Each thread that stops with a reason will send a
 # 'stopped' event. We need to remember the thread stop
@@ -234,6 +238,7 @@ def handle_recv_packet(self, packet):
 # that information.
 self._process_stopped()
 tid = body["threadId"]
+self.stopped_events.append(packet)
 self.thread_stop_reasons[tid] = body
 elif event == "breakpoint":
 # Breakpoint events come in when a breakpoint has locations
@@ -242,6 +247,10 @@ def handle_recv_packet(self, packet):
 self.breakpoint_events.append(packet)
 # no need to add 'breakpoint' event packets to our packets list
 return keepGoing
+elif event == "thread":
+self.thread_events_body.append(body)
+# no need to add 'thread' event packets to our packets list
+return keepGoing
 elif event.startswith("progress"):
 # Progress events come in as 'progressStart', 'progressUpdate',
 # and 'progressEnd' events. Keep these around in case test
@@ -418,6 +427,15 @@ def get_threads(self):
 self.request_threads()
 return self.threads
 
+def get_thread_events(self, reason=None):
+if reason == None:
+return self.thread_events_body
+else:
+return [body for body in self.thread_events_body if body["reason"] 
== reason]
+
+def get_stopped_events(self):
+return self.stopped_events
+
 def get_thread_id(self, threadIndex=0):
 """Utility function to get the first thread ID in the thread list.
 If the thread list is empty, then fetch the threads.
@@ -707,7 +725,7 @@ def request_evaluate(self, expression, frameIndex=0, 
threadId=None, context=None
 }
 return self.send_recv(command_dict)
 
-def request_initialize(self, sourceInitFile):
+def request_initialize(self, sourceInitFile, singleStoppedEvent=False):
 command_dict = {
 "command": "initialize",
 "type": "request",
@@ -723,6 +741,7 @@ def request_initialize(self, sourceInitFile):
 "supportsVariableType": True,
 "supportsStartDebuggingRequest": True,
 "sourceInitFil

[Lldb-commits] [lldb] Support single stopped event in lldb-dap (PR #98568)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: None (jeffreytan81)


Changes

Some targets have non-trivial amount of threads, when stopping, multiple 
stopped events can be sent causing VSCode to show multiple yellow lines in code 
editor which can be confusing to end users. 

This PR introduces a new single stopped event mode in lldb-dap which ensures 
only one stopped event is sent to VSCode UI. The implementation is preferring 
the stop reason from original thread with `focus_tid`. 

Note: if `singleStoppedEvent` option is not specified, it functions the same as 
before.

New tests are added.

---
Full diff: https://github.com/llvm/llvm-project/pull/98568.diff


8 Files Affected:

- (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 
(+20-1) 
- (modified) 
lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py (+4-1) 
- (added) lldb/test/API/tools/lldb-dap/stop-events/Makefile (+4) 
- (added) lldb/test/API/tools/lldb-dap/stop-events/TestDAP_stopEvents.py (+92) 
- (added) lldb/test/API/tools/lldb-dap/stop-events/main.cpp (+74) 
- (modified) lldb/tools/lldb-dap/DAP.cpp (+1-1) 
- (modified) lldb/tools/lldb-dap/DAP.h (+3) 
- (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+42-5) 


``diff
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index a324af57b61df..84e6fe82a06dc 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -125,6 +125,8 @@ def __init__(self, recv, send, init_commands, 
log_file=None):
 self.initialize_body = None
 self.thread_stop_reasons = {}
 self.breakpoint_events = []
+self.thread_events_body = []
+self.stopped_events = []
 self.progress_events = []
 self.reverse_requests = []
 self.sequence = 1
@@ -227,6 +229,8 @@ def handle_recv_packet(self, packet):
 # When a new process is attached or launched, remember the
 # details that are available in the body of the event
 self.process_event_body = body
+elif event == "continued":
+self.stopped_events = []
 elif event == "stopped":
 # Each thread that stops with a reason will send a
 # 'stopped' event. We need to remember the thread stop
@@ -234,6 +238,7 @@ def handle_recv_packet(self, packet):
 # that information.
 self._process_stopped()
 tid = body["threadId"]
+self.stopped_events.append(packet)
 self.thread_stop_reasons[tid] = body
 elif event == "breakpoint":
 # Breakpoint events come in when a breakpoint has locations
@@ -242,6 +247,10 @@ def handle_recv_packet(self, packet):
 self.breakpoint_events.append(packet)
 # no need to add 'breakpoint' event packets to our packets list
 return keepGoing
+elif event == "thread":
+self.thread_events_body.append(body)
+# no need to add 'thread' event packets to our packets list
+return keepGoing
 elif event.startswith("progress"):
 # Progress events come in as 'progressStart', 'progressUpdate',
 # and 'progressEnd' events. Keep these around in case test
@@ -418,6 +427,15 @@ def get_threads(self):
 self.request_threads()
 return self.threads
 
+def get_thread_events(self, reason=None):
+if reason == None:
+return self.thread_events_body
+else:
+return [body for body in self.thread_events_body if body["reason"] 
== reason]
+
+def get_stopped_events(self):
+return self.stopped_events
+
 def get_thread_id(self, threadIndex=0):
 """Utility function to get the first thread ID in the thread list.
 If the thread list is empty, then fetch the threads.
@@ -707,7 +725,7 @@ def request_evaluate(self, expression, frameIndex=0, 
threadId=None, context=None
 }
 return self.send_recv(command_dict)
 
-def request_initialize(self, sourceInitFile):
+def request_initialize(self, sourceInitFile, singleStoppedEvent=False):
 command_dict = {
 "command": "initialize",
 "type": "request",
@@ -723,6 +741,7 @@ def request_initialize(self, sourceInitFile):
 "supportsVariableType": True,
 "supportsStartDebuggingRequest": True,
 "sourceInitFile": sourceInitFile,
+"singleStoppedEvent": singleStoppedEvent,
 },
 }
 response = self.send_recv(command_dict)
diff --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_tes

[Lldb-commits] [lldb] Support single stopped event in lldb-dap (PR #98568)

[via lldb-commits]

github-actions[bot] wrote:




:warning: Python code formatter, darker found issues in your code. :warning:



You can test this locally with the following command:


``bash
darker --check --diff -r 
b81fcd01bde51eb8976b81a2c0c19fc0645cd2ff...13af0ff31688ca0a23f1fec65ca2d5797b65e31f
 lldb/test/API/tools/lldb-dap/stop-events/TestDAP_stopEvents.py 
lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 
lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
``





View the diff from darker here.


``diff
--- packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 2024-07-12 
00:24:41.00 +
+++ packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 2024-07-12 
00:34:31.518976 +
@@ -429,12 +429,14 @@
 
 def get_thread_events(self, reason=None):
 if reason == None:
 return self.thread_events_body
 else:
-return [body for body in self.thread_events_body if body["reason"] 
== reason]
-
+return [
+body for body in self.thread_events_body if body["reason"] == 
reason
+]
+
 def get_stopped_events(self):
 return self.stopped_events
 
 def get_thread_id(self, threadIndex=0):
 """Utility function to get the first thread ID in the thread list.
--- test/API/tools/lldb-dap/stop-events/TestDAP_stopEvents.py   2024-07-12 
00:24:41.00 +
+++ test/API/tools/lldb-dap/stop-events/TestDAP_stopEvents.py   2024-07-12 
00:34:31.738292 +
@@ -7,11 +7,10 @@
 import lldbdap_testcase
 from lldbsuite.test import lldbutil
 
 
 class TestDAP_stopEvents(lldbdap_testcase.DAPTestCaseBase):
-
 @skipIfWindows
 @skipIfRemote
 def test_single_stop_event(self):
 """
 Ensure single stopped event is sent during stop when singleStoppedEvent

``




https://github.com/llvm/llvm-project/pull/98568
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Support single stopped event in lldb-dap (PR #98568)

[via lldb-commits]

https://github.com/jeffreytan81 updated 
https://github.com/llvm/llvm-project/pull/98568

>From 13af0ff31688ca0a23f1fec65ca2d5797b65e31f Mon Sep 17 00:00:00 2001
From: jeffreytan81 
Date: Thu, 11 Jul 2024 17:24:41 -0700
Subject: [PATCH 1/2] Support single stopped event in lldb-dap

---
 .../test/tools/lldb-dap/dap_server.py | 21 -
 .../test/tools/lldb-dap/lldbdap_testcase.py   |  5 +-
 .../API/tools/lldb-dap/stop-events/Makefile   |  4 +
 .../stop-events/TestDAP_stopEvents.py | 92 +++
 .../API/tools/lldb-dap/stop-events/main.cpp   | 74 +++
 lldb/tools/lldb-dap/DAP.cpp   |  2 +-
 lldb/tools/lldb-dap/DAP.h |  3 +
 lldb/tools/lldb-dap/lldb-dap.cpp  | 47 +-
 8 files changed, 240 insertions(+), 8 deletions(-)
 create mode 100644 lldb/test/API/tools/lldb-dap/stop-events/Makefile
 create mode 100644 
lldb/test/API/tools/lldb-dap/stop-events/TestDAP_stopEvents.py
 create mode 100644 lldb/test/API/tools/lldb-dap/stop-events/main.cpp

diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index a324af57b61df..84e6fe82a06dc 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -125,6 +125,8 @@ def __init__(self, recv, send, init_commands, 
log_file=None):
 self.initialize_body = None
 self.thread_stop_reasons = {}
 self.breakpoint_events = []
+self.thread_events_body = []
+self.stopped_events = []
 self.progress_events = []
 self.reverse_requests = []
 self.sequence = 1
@@ -227,6 +229,8 @@ def handle_recv_packet(self, packet):
 # When a new process is attached or launched, remember the
 # details that are available in the body of the event
 self.process_event_body = body
+elif event == "continued":
+self.stopped_events = []
 elif event == "stopped":
 # Each thread that stops with a reason will send a
 # 'stopped' event. We need to remember the thread stop
@@ -234,6 +238,7 @@ def handle_recv_packet(self, packet):
 # that information.
 self._process_stopped()
 tid = body["threadId"]
+self.stopped_events.append(packet)
 self.thread_stop_reasons[tid] = body
 elif event == "breakpoint":
 # Breakpoint events come in when a breakpoint has locations
@@ -242,6 +247,10 @@ def handle_recv_packet(self, packet):
 self.breakpoint_events.append(packet)
 # no need to add 'breakpoint' event packets to our packets list
 return keepGoing
+elif event == "thread":
+self.thread_events_body.append(body)
+# no need to add 'thread' event packets to our packets list
+return keepGoing
 elif event.startswith("progress"):
 # Progress events come in as 'progressStart', 'progressUpdate',
 # and 'progressEnd' events. Keep these around in case test
@@ -418,6 +427,15 @@ def get_threads(self):
 self.request_threads()
 return self.threads
 
+def get_thread_events(self, reason=None):
+if reason == None:
+return self.thread_events_body
+else:
+return [body for body in self.thread_events_body if body["reason"] 
== reason]
+
+def get_stopped_events(self):
+return self.stopped_events
+
 def get_thread_id(self, threadIndex=0):
 """Utility function to get the first thread ID in the thread list.
 If the thread list is empty, then fetch the threads.
@@ -707,7 +725,7 @@ def request_evaluate(self, expression, frameIndex=0, 
threadId=None, context=None
 }
 return self.send_recv(command_dict)
 
-def request_initialize(self, sourceInitFile):
+def request_initialize(self, sourceInitFile, singleStoppedEvent=False):
 command_dict = {
 "command": "initialize",
 "type": "request",
@@ -723,6 +741,7 @@ def request_initialize(self, sourceInitFile):
 "supportsVariableType": True,
 "supportsStartDebuggingRequest": True,
 "sourceInitFile": sourceInitFile,
+"singleStoppedEvent": singleStoppedEvent,
 },
 }
 response = self.send_recv(command_dict)
diff --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
index a312a88ebd7e5..90ab2a8d4898a 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
@@ -350,6 +35

[Lldb-commits] [lldb] [lldb] Add support for displaying `__float128` variables (PR #98369)

[Jason Molenda via lldb-commits]

@@ -46,6 +46,7 @@ static constexpr FormatInfo g_format_infos[] = {
 {eFormatHex, 'x', "hex"},
 {eFormatHexUppercase, 'X', "uppercase hex"},
 {eFormatFloat, 'f', "float"},
+{eFormatFloat128, '\0', "float128"},

jasonmolenda wrote:

I'm not familiar with this part of lldb, but do we need an entry with a 
character code that can't be entered?  I don't think there are commands which 
take the "long name" of these entries, do they?  Vaguely relatedly, I wondered 
about `OptionGroupFormat::ParserGDBFormatLetter` which recognizes the gdb 
formatters (v. 
https://sourceware.org/gdb/current/onlinedocs/gdb.html/Output-Formats.html ) 
but I think just "f" for "float of undetermined size" is correct as-is?  This 
is seen in commands like `x/2gx $fp` etc, which command aliases rewrite to a 
longer `memory read`.

https://github.com/llvm/llvm-project/pull/98369
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add support for displaying `__float128` variables (PR #98369)

[Jason Molenda via lldb-commits]

https://github.com/jasonmolenda edited 
https://github.com/llvm/llvm-project/pull/98369
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Private process events were being delivered to the secondary listener (PR #98571)

[via lldb-commits]

https://github.com/jimingham created 
https://github.com/llvm/llvm-project/pull/98571

This fixes a bug where Process events were being delivered to secondary 
listeners when the Private state thread listener was processing the event.  
That meant the secondary listener could get an event before the Primary 
listener did.  That in turn meant the state when the secondary listener got the 
event wasn't right yet.  Plus it meant that the secondary listener saw more 
events than the primary (not all events get forwarded from the private to the 
public Process listener.)

This bug became much more evident when we had a stop hook that did some work, 
since that delays the Primary listener event delivery.  So I also added a 
stop-hook to the test, and put a little delay in as well.

>From fb563e516f3a73d508ea7b3a61df4f1bab2f33a6 Mon Sep 17 00:00:00 2001
From: Jim Ingham 
Date: Thu, 11 Jul 2024 17:50:08 -0700
Subject: [PATCH] Fix a bug where process events were being delivered to the
 secondary Listener when the Private event queue was processing the event. 
 That meant the secondary listener could get an event before the Primary
 listener did.  That in turn meant the state when the secondary listener got
 the event wasn't right yet.  Plus it meant that the secondary listener saw
 more events than the primary (not all events get forwarded from the private
 to the public Process listener.)

This bug became much more evident when we had a stop hook that did some work,
since that delays the Primary listener event delivery.  So I also added a 
stop-hook
to the test, and put a little delay in as well.
---
 lldb/include/lldb/Target/Process.h   |  3 ++
 lldb/include/lldb/Utility/Event.h| 13 +
 lldb/source/Target/Process.cpp   | 14 ++
 lldb/source/Utility/Event.cpp| 16 --
 lldb/test/API/python_api/event/TestEvents.py | 53 +---
 5 files changed, 86 insertions(+), 13 deletions(-)

diff --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index ceaf547ebddaf..37fc192b4469c 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -465,6 +465,9 @@ class Process : public 
std::enable_shared_from_this,
 static bool SetUpdateStateOnRemoval(Event *event_ptr);
 
   private:
+  
+bool ForwardEventToPendingListeners(Event *event_ptr) override;
+
 void SetUpdateStateOnRemoval() { m_update_state++; }
 
 void SetRestarted(bool new_value) { m_restarted = new_value; }
diff --git a/lldb/include/lldb/Utility/Event.h 
b/lldb/include/lldb/Utility/Event.h
index 461d711b8c3f2..5bea328d1faa9 100644
--- a/lldb/include/lldb/Utility/Event.h
+++ b/lldb/include/lldb/Utility/Event.h
@@ -48,6 +48,19 @@ class EventData {
   virtual void Dump(Stream *s) const;
 
 private:
+  /// This will be queried for a Broadcaster with a primary and some secondary
+  /// listeners after the primary listener pulled the event from the event 
queue
+  /// and ran its DoOnRemoval, right before the event is delivered.  
+  /// If it returns true, the event will also be forwarded to the secondary  
+  /// listeners, and if false, event propagation stops at the primary 
listener. 
+  /// Some broadcasters (particularly the Process broadcaster) fetch events on
+  /// a private Listener, and then forward the event to the Public Listeners
+  /// after some processing.  The Process broadcaster does not want to forward
+  /// to the secondary listeners at the private processing stage.
+  virtual bool ForwardEventToPendingListeners(Event *event_ptr) {
+return true;
+  }
+
   virtual void DoOnRemoval(Event *event_ptr) {}
 
   EventData(const EventData &) = delete;
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 6fac0df1d7a66..a43684c999641 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -4248,7 +4248,21 @@ bool Process::ProcessEventData::ShouldStop(Event 
*event_ptr,
   return still_should_stop;
 }
 
+bool Process::ProcessEventData::ForwardEventToPendingListeners(Event 
*event_ptr) {
+  // STDIO and the other async event notifications should always be forwarded.
+  if (event_ptr->GetType() != Process::eBroadcastBitStateChanged)
+return true;
+
+  // For state changed events, if the update state is one, we are handling 
+  // this on the private state thread.  We should wait for the public event.
+  return m_update_state == 1;
+}
+
 void Process::ProcessEventData::DoOnRemoval(Event *event_ptr) {
+  // We only have work to do for state changed events:
+  if (event_ptr->GetType() != Process::eBroadcastBitStateChanged)
+return;
+
   ProcessSP process_sp(m_process_wp.lock());
 
   if (!process_sp)
diff --git a/lldb/source/Utility/Event.cpp b/lldb/source/Utility/Event.cpp
index 863167e56bce6..5f431c0a6dd89 100644
--- a/lldb/source/Utility/Event.cpp
+++ b/lldb/source/Utility/Event.cpp
@@ -83,14 +83,20 @@ void Event::Dump(Stream *s) const {
 void Event::D

[Lldb-commits] [lldb] Private process events were being delivered to the secondary listener (PR #98571)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: None (jimingham)


Changes

This fixes a bug where Process events were being delivered to secondary 
listeners when the Private state thread listener was processing the event.  
That meant the secondary listener could get an event before the Primary 
listener did.  That in turn meant the state when the secondary listener got the 
event wasn't right yet.  Plus it meant that the secondary listener saw more 
events than the primary (not all events get forwarded from the private to the 
public Process listener.)

This bug became much more evident when we had a stop hook that did some work, 
since that delays the Primary listener event delivery.  So I also added a 
stop-hook to the test, and put a little delay in as well.

---
Full diff: https://github.com/llvm/llvm-project/pull/98571.diff


5 Files Affected:

- (modified) lldb/include/lldb/Target/Process.h (+3) 
- (modified) lldb/include/lldb/Utility/Event.h (+13) 
- (modified) lldb/source/Target/Process.cpp (+14) 
- (modified) lldb/source/Utility/Event.cpp (+11-5) 
- (modified) lldb/test/API/python_api/event/TestEvents.py (+45-8) 


``diff
diff --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index ceaf547ebddaf..37fc192b4469c 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -465,6 +465,9 @@ class Process : public 
std::enable_shared_from_this,
 static bool SetUpdateStateOnRemoval(Event *event_ptr);
 
   private:
+  
+bool ForwardEventToPendingListeners(Event *event_ptr) override;
+
 void SetUpdateStateOnRemoval() { m_update_state++; }
 
 void SetRestarted(bool new_value) { m_restarted = new_value; }
diff --git a/lldb/include/lldb/Utility/Event.h 
b/lldb/include/lldb/Utility/Event.h
index 461d711b8c3f2..5bea328d1faa9 100644
--- a/lldb/include/lldb/Utility/Event.h
+++ b/lldb/include/lldb/Utility/Event.h
@@ -48,6 +48,19 @@ class EventData {
   virtual void Dump(Stream *s) const;
 
 private:
+  /// This will be queried for a Broadcaster with a primary and some secondary
+  /// listeners after the primary listener pulled the event from the event 
queue
+  /// and ran its DoOnRemoval, right before the event is delivered.  
+  /// If it returns true, the event will also be forwarded to the secondary  
+  /// listeners, and if false, event propagation stops at the primary 
listener. 
+  /// Some broadcasters (particularly the Process broadcaster) fetch events on
+  /// a private Listener, and then forward the event to the Public Listeners
+  /// after some processing.  The Process broadcaster does not want to forward
+  /// to the secondary listeners at the private processing stage.
+  virtual bool ForwardEventToPendingListeners(Event *event_ptr) {
+return true;
+  }
+
   virtual void DoOnRemoval(Event *event_ptr) {}
 
   EventData(const EventData &) = delete;
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 6fac0df1d7a66..a43684c999641 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -4248,7 +4248,21 @@ bool Process::ProcessEventData::ShouldStop(Event 
*event_ptr,
   return still_should_stop;
 }
 
+bool Process::ProcessEventData::ForwardEventToPendingListeners(Event 
*event_ptr) {
+  // STDIO and the other async event notifications should always be forwarded.
+  if (event_ptr->GetType() != Process::eBroadcastBitStateChanged)
+return true;
+
+  // For state changed events, if the update state is one, we are handling 
+  // this on the private state thread.  We should wait for the public event.
+  return m_update_state == 1;
+}
+
 void Process::ProcessEventData::DoOnRemoval(Event *event_ptr) {
+  // We only have work to do for state changed events:
+  if (event_ptr->GetType() != Process::eBroadcastBitStateChanged)
+return;
+
   ProcessSP process_sp(m_process_wp.lock());
 
   if (!process_sp)
diff --git a/lldb/source/Utility/Event.cpp b/lldb/source/Utility/Event.cpp
index 863167e56bce6..5f431c0a6dd89 100644
--- a/lldb/source/Utility/Event.cpp
+++ b/lldb/source/Utility/Event.cpp
@@ -83,14 +83,20 @@ void Event::Dump(Stream *s) const {
 void Event::DoOnRemoval() {
   std::lock_guard guard(m_listeners_mutex);
 
-  if (m_data_sp)
-m_data_sp->DoOnRemoval(this);
+  if (!m_data_sp)
+return;
+
+  m_data_sp->DoOnRemoval(this);
+
   // Now that the event has been handled by the primary event Listener, forward
   // it to the other Listeners.
+
   EventSP me_sp = shared_from_this();
-  for (auto listener_sp : m_pending_listeners)
-listener_sp->AddEvent(me_sp);
-  m_pending_listeners.clear();
+  if (m_data_sp->ForwardEventToPendingListeners(this)) {
+for (auto listener_sp : m_pending_listeners)
+  listener_sp->AddEvent(me_sp);
+m_pending_listeners.clear();
+  }
 }
 
 #pragma mark -
diff --git a/lldb/test/API/python_api/event/TestEvents.py 
b/lldb/test/API/python_api/event/TestEvents.py
index d8d3dd2d2b01b..ba9de897cf562 100644

[Lldb-commits] [lldb] Private process events were being delivered to the secondary listener (PR #98571)

[via lldb-commits]

jimingham wrote:

This test was marked as flakey on some Linux systems, but so were other of the 
process event tests in this file, which should not have been affected by this 
patch.  So I don't think this is going to fix whatever was flakey in general, 
though it should help this test.

https://github.com/llvm/llvm-project/pull/98571
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Private process events were being delivered to the secondary listener (PR #98571)

[via lldb-commits]

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff 7b604cdf75fd1c741a15138684ea0e98dca5e46f 
fb563e516f3a73d508ea7b3a61df4f1bab2f33a6 --extensions cpp,h -- 
lldb/include/lldb/Target/Process.h lldb/include/lldb/Utility/Event.h 
lldb/source/Target/Process.cpp lldb/source/Utility/Event.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/include/lldb/Target/Process.h 
b/lldb/include/lldb/Target/Process.h
index 37fc192b44..c8475db8ae 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -465,7 +465,6 @@ public:
 static bool SetUpdateStateOnRemoval(Event *event_ptr);
 
   private:
-  
 bool ForwardEventToPendingListeners(Event *event_ptr) override;
 
 void SetUpdateStateOnRemoval() { m_update_state++; }
diff --git a/lldb/include/lldb/Utility/Event.h 
b/lldb/include/lldb/Utility/Event.h
index 5bea328d1f..4f58f257d4 100644
--- a/lldb/include/lldb/Utility/Event.h
+++ b/lldb/include/lldb/Utility/Event.h
@@ -50,16 +50,14 @@ public:
 private:
   /// This will be queried for a Broadcaster with a primary and some secondary
   /// listeners after the primary listener pulled the event from the event 
queue
-  /// and ran its DoOnRemoval, right before the event is delivered.  
-  /// If it returns true, the event will also be forwarded to the secondary  
-  /// listeners, and if false, event propagation stops at the primary 
listener. 
+  /// and ran its DoOnRemoval, right before the event is delivered.
+  /// If it returns true, the event will also be forwarded to the secondary
+  /// listeners, and if false, event propagation stops at the primary listener.
   /// Some broadcasters (particularly the Process broadcaster) fetch events on
   /// a private Listener, and then forward the event to the Public Listeners
   /// after some processing.  The Process broadcaster does not want to forward
   /// to the secondary listeners at the private processing stage.
-  virtual bool ForwardEventToPendingListeners(Event *event_ptr) {
-return true;
-  }
+  virtual bool ForwardEventToPendingListeners(Event *event_ptr) { return true; 
}
 
   virtual void DoOnRemoval(Event *event_ptr) {}
 
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index a43684c999..618a6cf2d1 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -4248,12 +4248,13 @@ bool Process::ProcessEventData::ShouldStop(Event 
*event_ptr,
   return still_should_stop;
 }
 
-bool Process::ProcessEventData::ForwardEventToPendingListeners(Event 
*event_ptr) {
+bool Process::ProcessEventData::ForwardEventToPendingListeners(
+Event *event_ptr) {
   // STDIO and the other async event notifications should always be forwarded.
   if (event_ptr->GetType() != Process::eBroadcastBitStateChanged)
 return true;
 
-  // For state changed events, if the update state is one, we are handling 
+  // For state changed events, if the update state is one, we are handling
   // this on the private state thread.  We should wait for the public event.
   return m_update_state == 1;
 }

``




https://github.com/llvm/llvm-project/pull/98571
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Private process events were being delivered to the secondary listener (PR #98571)

[via lldb-commits]

github-actions[bot] wrote:




:warning: Python code formatter, darker found issues in your code. :warning:



You can test this locally with the following command:


``bash
darker --check --diff -r 
7b604cdf75fd1c741a15138684ea0e98dca5e46f...fb563e516f3a73d508ea7b3a61df4f1bab2f33a6
 lldb/test/API/python_api/event/TestEvents.py
``





View the diff from darker here.


``diff
--- TestEvents.py   2024-07-12 00:50:08.00 +
+++ TestEvents.py   2024-07-12 01:00:49.476504 +
@@ -343,14 +343,13 @@
 if not restart:
 self.stop_counter += 1
 self.assertEqual(
 stop_hook.StopHook.counter[self.instance],
 self.stop_counter,
-"matching stop hook"
+"matching stop hook",
 )
-
-
+
 if expected_state is not None:
 self.assertEqual(
 state, expected_state, "Primary thread got the correct event"
 )
 
@@ -358,18 +357,14 @@
 # listener:
 success = self.shadow_listener.WaitForEvent(5, event)
 self.assertTrue(success, "Shadow listener got event too")
 shadow_event_type = event.GetType()
 self.assertEqual(
-primary_event_type,
-shadow_event_type,
-"It was the same event type"
-) 
-self.assertEqual(
-state,
-lldb.SBProcess.GetStateFromEvent(event),
-"It was the same state"
+primary_event_type, shadow_event_type, "It was the same event type"
+)
+self.assertEqual(
+state, lldb.SBProcess.GetStateFromEvent(event), "It was the same 
state"
 )
 self.assertEqual(
 restart,
 lldb.SBProcess.GetRestartedFromEvent(event),
 "It was the same restarted",
@@ -413,29 +408,32 @@
 # this instance of the stop hook:
 self.instance = f"Key{random.randint(0,1)}"
 stop_hook_path = os.path.join(self.getSourceDir(), "stop_hook.py")
 self.runCmd(f"command script import {stop_hook_path}")
 import stop_hook
-self.runCmd(f"target stop-hook add -P stop_hook.StopHook -k instance 
-v {self.instance}")
+
+self.runCmd(
+f"target stop-hook add -P stop_hook.StopHook -k instance -v 
{self.instance}"
+)
 self.stop_counter = 0
-
+
 self.process = target.Launch(launch_info, error)
 self.assertSuccess(error, "Process launched successfully")
 
 # Keep fetching events from the primary to trigger the do on removal 
and
 # then from the shadow listener, and make sure they match:
 
 # Events in the launch sequence might be platform dependent, so don't
 # expect any particular event till we get the stopped:
 state = lldb.eStateInvalid
-
+
 while state != lldb.eStateStopped:
 state, restart = self.wait_for_next_event(None, False)
-
+
 # Okay, we're now at a good stop, so try a next:
 self.cur_thread = self.process.threads[0]
-
+
 # Make sure we're at our expected breakpoint:
 self.assertTrue(self.cur_thread.IsValid(), "Got a zeroth thread")
 self.assertEqual(self.cur_thread.stop_reason, 
lldb.eStopReasonBreakpoint)
 self.assertEqual(
 self.cur_thread.GetStopReasonDataCount(), 2, "Only one 
breakpoint/loc here"

``




https://github.com/llvm/llvm-project/pull/98571
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add support for displaying `__float128` variables (PR #98369)

[via lldb-commits]

@@ -46,6 +46,7 @@ static constexpr FormatInfo g_format_infos[] = {
 {eFormatHex, 'x', "hex"},
 {eFormatHexUppercase, 'X', "uppercase hex"},
 {eFormatFloat, 'f', "float"},
+{eFormatFloat128, '\0', "float128"},

beetrees wrote:

There's a static assert just below the array which states `"All formats must 
have a corresponding info entry."`, and will cause compilation to fail 
otherwise. Commands such as `type format add -f  ` can take 
either the single letter code or the long name to identify the format (`'\0'` 
is used to represent the format not having a single letter code).

On platforms with a 16-byte `long double`, the `f` format code will continue to 
map to `eFormatFloat` and therefore use `long double` for 16-byte values, just 
like in GDB.

https://github.com/llvm/llvm-project/pull/98369
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Unify Platform::ResolveExecutable (PR #96256)

[Greg Clayton via lldb-commits]

clayborg wrote:

We think this broke Android debugging on our end. We are investigating.

https://github.com/llvm/llvm-project/pull/96256
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Unify Platform::ResolveExecutable (PR #96256)

[Greg Clayton via lldb-commits]

clayborg wrote:

This did indeed break Android debugging. We reverted this in our repo and 
functionality was restored

https://github.com/llvm/llvm-project/pull/96256
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [LLDB] Fix Android debugging (PR #98581)

[Kazuki Sakamoto via lldb-commits]

https://github.com/splhack created 
https://github.com/llvm/llvm-project/pull/98581

# Goal

Fix Android debugging

# What

This pull request reverts these two commits because it broke Android debugging 
as explained below.

- 
https://github.com/llvm/llvm-project/commit/bf3e3289d67cb0fe136b0660cac39c24c9f65069
- 
https://github.com/llvm/llvm-project/commit/c3fe1c4472e72a3832be911e8fa9098fa84762a0

This pull requests also reverts this commit to avoid conflicts on the revert.

- 
https://github.com/llvm/llvm-project/commit/ed7e46877dc7f09b5d194f87c87bfc2bebfcf27a

# Breakage state

LLDB fails to resolve executable on remote-android `platform process attach`.

```
platform select remote-android
platform connect unix-connect://localhost:/
platform process attach -p NNN
```

Using logging `lldb` `all`

```
log enable -f /tmp/lldb.log lldb all
```

Without this pull request,

```
intern-state DynamicLoaderPOSIXDYLD::ResolveExecutableModule - got 
executable by pid 576: /system/bin/foo
intern-state DynamicLoaderPOSIXDYLD::ResolveExecutableModule - failed to 
resolve executable with module spec "file = '/system/bin/foo', arch = 
x86_64-unknown-linux-android": '/system/bin/foo' does not exist
```

With this pull request,

```
intern-state DynamicLoaderPOSIXDYLD::ResolveExecutableModule - got 
executable by pid 576: /system/bin/foo
intern-state 0x55C038956E30 Communication::Write (src = 
0x151FEC010C80, src_len = 135) connection = 0x55C0384F3AC0
intern-state 0x55c0384f3ac0 ConnectionFileDescriptor::Write (src = 
0x151fec010c80, src_len = 135)
intern-state 0x55c038946b80 Socket::Write() (socket = 10, src = 
0x151fec010c80, src_len = 135, flags = 0) => 135 (error = (null))
intern-state 0x55c0384f3ac0 ConnectionFileDescriptor::Write(fd = 10, src = 
0x151fec010c80, src_len = 135) => 135 (error = (null))
intern-state this = 0x55C038956E30, dst = 0x151FFA7FB770, dst_len = 
8192, timeout = 500 us, connection = 0x55C0384F3AC0
intern-state this = 0x55C0384F3AC0, timeout = 500 us
intern-state 0x55c038946b80 Socket::Read() (socket = 10, src = 
0x151ffa7fb770, src_len = 253, flags = 0) => 253 (error = (null))
intern-state 0x55c0384f3ac0 ConnectionFileDescriptor::Read()  fd = 10, dst 
= 0x151ffa7fb770, dst_len = 8192) => 253, error = (null)
intern-state PlatformRemoteGDBServer::GetModuleSpec - got module info for 
(/system/bin/foo:x86_64-unknown-linux-android) : file = '/system/bin/foo', arch 
= x86_64-unknown-linux-android, uuid = NN----, object 
size = 
```

# Breakage detail

>From the logs,

```
intern-state DynamicLoaderPOSIXDYLD::ResolveExecutableModule - got 
executable by pid 576: /system/bin/foo
intern-state DynamicLoaderPOSIXDYLD::ResolveExecutableModule - failed to 
resolve executable with module spec "file = '/system/bin/foo', arch = 
x86_64-unknown-linux-android": '/system/bin/foo' does not exist
```

LLDB failed to resolve executable between this log

https://github.com/llvm/llvm-project/blob/90abdf83e273586a43e1270e5f0a11de5cc35383/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L839-L842

and this log

https://github.com/llvm/llvm-project/blob/90abdf83e273586a43e1270e5f0a11de5cc35383/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L857-L860

Therefore, the problem is in `platform_sp->ResolveExecutable`.

https://github.com/llvm/llvm-project/blob/90abdf83e273586a43e1270e5f0a11de5cc35383/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L850-L852

According to the error message `'/system/bin/foo' does not exist`,
most likely `platform_sp->ResolveExecutable` tried to access the path in the 
local file system in the host that runs LLDB. It is supposed to access the path 
in the target file system via lldb-server.

>From 8a4152b0dbfbe2733fa8b359fe991bf335f6f530 Mon Sep 17 00:00:00 2001
From: Kazuki Sakamoto 
Date: Thu, 11 Jul 2024 18:37:17 -0700
Subject: [PATCH 1/3] Revert "[lldb] Improve error message for unrecognized
 executables (#97490)"

This reverts commit ed7e46877dc7f09b5d194f87c87bfc2bebfcf27a.
---
 lldb/include/lldb/Symbol/ObjectFile.h |  1 -
 lldb/source/Symbol/ObjectFile.cpp |  9 --
 lldb/source/Target/Platform.cpp   | 87 +--
 .../PECOFF/invalid-export-table.yaml  |  2 +-
 4 files changed, 44 insertions(+), 55 deletions(-)

diff --git a/lldb/include/lldb/Symbol/ObjectFile.h 
b/lldb/include/lldb/Symbol/ObjectFile.h
index 8592323322e38..6348d8103f85d 100644
--- a/lldb/include/lldb/Symbol/ObjectFile.h
+++ b/lldb/include/lldb/Symbol/ObjectFile.h
@@ -178,7 +178,6 @@ class ObjectFile : public 
std::enable_shared_from_this,
 lldb::offset_t file_offset,
 lldb::offset_t file_size,
 lldb_private::ModuleSpecList &specs);
-  static bool IsObjectFile(lld

[Lldb-commits] [lldb] [LLDB] Fix Android debugging (PR #98581)

[via lldb-commits]

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Kazuki Sakamoto (splhack)


Changes

# Goal

Fix Android debugging

# What

This pull request reverts these two commits because it broke Android debugging 
as explained below.

- 
https://github.com/llvm/llvm-project/commit/bf3e3289d67cb0fe136b0660cac39c24c9f65069
- 
https://github.com/llvm/llvm-project/commit/c3fe1c4472e72a3832be911e8fa9098fa84762a0

This pull requests also reverts this commit to avoid conflicts on the revert.

- 
https://github.com/llvm/llvm-project/commit/ed7e46877dc7f09b5d194f87c87bfc2bebfcf27a

# Breakage state

LLDB fails to resolve executable on remote-android `platform process attach`.

```
platform select remote-android
platform connect unix-connect://localhost:/
platform process attach -p NNN
```

Using logging `lldb` `all`

```
log enable -f /tmp/lldb.log lldb all
```

Without this pull request,

```
intern-state DynamicLoaderPOSIXDYLD::ResolveExecutableModule - got 
executable by pid 576: /system/bin/foo
intern-state DynamicLoaderPOSIXDYLD::ResolveExecutableModule - failed to 
resolve executable with module spec "file = '/system/bin/foo', arch = 
x86_64-unknown-linux-android": '/system/bin/foo' does not exist
```

With this pull request,

```
intern-state DynamicLoaderPOSIXDYLD::ResolveExecutableModule - got 
executable by pid 576: /system/bin/foo
intern-state 0x55C038956E30 Communication::Write (src = 
0x151FEC010C80, src_len = 135) connection = 0x55C0384F3AC0
intern-state 0x55c0384f3ac0 ConnectionFileDescriptor::Write (src = 
0x151fec010c80, src_len = 135)
intern-state 0x55c038946b80 Socket::Write() (socket = 10, src = 
0x151fec010c80, src_len = 135, flags = 0) => 135 (error = (null))
intern-state 0x55c0384f3ac0 ConnectionFileDescriptor::Write(fd = 10, src = 
0x151fec010c80, src_len = 135) => 135 (error = (null))
intern-state this = 0x55C038956E30, dst = 0x151FFA7FB770, dst_len = 
8192, timeout = 500 us, connection = 0x55C0384F3AC0
intern-state this = 0x55C0384F3AC0, timeout = 500 us
intern-state 0x55c038946b80 Socket::Read() (socket = 10, src = 
0x151ffa7fb770, src_len = 253, flags = 0) => 253 (error = (null))
intern-state 0x55c0384f3ac0 ConnectionFileDescriptor::Read()  fd = 10, dst 
= 0x151ffa7fb770, dst_len = 8192) => 253, error = (null)
intern-state PlatformRemoteGDBServer::GetModuleSpec - got module info for 
(/system/bin/foo:x86_64-unknown-linux-android) : file = '/system/bin/foo', arch 
= x86_64-unknown-linux-android, uuid = NN----, object 
size = 
```

# Breakage detail

>From the logs,

```
intern-state DynamicLoaderPOSIXDYLD::ResolveExecutableModule - got 
executable by pid 576: /system/bin/foo
intern-state DynamicLoaderPOSIXDYLD::ResolveExecutableModule - failed to 
resolve executable with module spec "file = '/system/bin/foo', arch = 
x86_64-unknown-linux-android": '/system/bin/foo' does not exist
```

LLDB failed to resolve executable between this log

https://github.com/llvm/llvm-project/blob/90abdf83e273586a43e1270e5f0a11de5cc35383/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L839-L842

and this log

https://github.com/llvm/llvm-project/blob/90abdf83e273586a43e1270e5f0a11de5cc35383/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L857-L860

Therefore, the problem is in `platform_sp->ResolveExecutable`.

https://github.com/llvm/llvm-project/blob/90abdf83e273586a43e1270e5f0a11de5cc35383/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L850-L852

According to the error message `'/system/bin/foo' does not exist`,
most likely `platform_sp->ResolveExecutable` tried to access the path in the 
local file system in the host that runs LLDB. It is supposed to access the path 
in the target file system via lldb-server.

---

Patch is 29.43 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/98581.diff


14 Files Affected:

- (modified) lldb/include/lldb/Symbol/ObjectFile.h (-1) 
- (modified) lldb/include/lldb/Target/Platform.h (+6-1) 
- (modified) lldb/include/lldb/Target/RemoteAwarePlatform.h (+4-5) 
- (modified) lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp 
(+75) 
- (modified) lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h (+4) 
- (modified) lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp 
(+65) 
- (modified) lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h 
(+4) 
- (modified) lldb/source/Symbol/ObjectFile.cpp (-9) 
- (modified) lldb/source/Target/Platform.cpp (+82-45) 
- (modified) lldb/source/Target/RemoteAwarePlatform.cpp (+127-11) 
- (modified) lldb/test/API/assert_messages_test/TestAssertMessages.py (+1-1) 
- (modified) lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py (+1-1) 
- (modified) lldb/test/Shell/ObjectFile/PECOFF/invalid-export-table.yaml (+1-1) 
- (modified) lldb/unittests/Target/RemoteAwarePlatfor

[Lldb-commits] [lldb] [LLDB] Fix Android debugging (PR #98581)

[via lldb-commits]

github-actions[bot] wrote:




:warning: C/C++ code formatter, clang-format found issues in your code. 
:warning:



You can test this locally with the following command:


``bash
git-clang-format --diff a853fe25df1cda0117055c0d836e7b107d98c791 
8602d97a8a9b1f81b6776bede708eeb54bc3 --extensions h,cpp -- 
lldb/include/lldb/Symbol/ObjectFile.h lldb/include/lldb/Target/Platform.h 
lldb/include/lldb/Target/RemoteAwarePlatform.h 
lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp 
lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h 
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp 
lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h 
lldb/source/Symbol/ObjectFile.cpp lldb/source/Target/Platform.cpp 
lldb/source/Target/RemoteAwarePlatform.cpp 
lldb/unittests/Target/RemoteAwarePlatformTest.cpp
``





View the diff from clang-format here.


``diff
diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index c39cc3f120..31e1bda7bc 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -757,16 +757,16 @@ Platform::ResolveExecutable(const ModuleSpec &module_spec,
   }
 }
   } else {
-error.SetErrorStringWithFormat(
-"'%s' does not exist", module_spec.GetFileSpec().GetPath().c_str());
+error.SetErrorStringWithFormat("'%s' does not exist",
+   
module_spec.GetFileSpec().GetPath().c_str());
   }
   return error;
 }
 
 Status
 Platform::ResolveRemoteExecutable(const ModuleSpec &module_spec,
-lldb::ModuleSP &exe_module_sp,
-const FileSpecList *module_search_paths_ptr) {
+  lldb::ModuleSP &exe_module_sp,
+  const FileSpecList *module_search_paths_ptr) 
{
   Status error;
 
   // We may connect to a process and use the provided executable (Don't use
diff --git a/lldb/source/Target/RemoteAwarePlatform.cpp 
b/lldb/source/Target/RemoteAwarePlatform.cpp
index 9a41a423ca..5c1979c979 100644
--- a/lldb/source/Target/RemoteAwarePlatform.cpp
+++ b/lldb/source/Target/RemoteAwarePlatform.cpp
@@ -93,11 +93,12 @@ Status RemoteAwarePlatform::ResolveExecutable(
   if (error.Success()) {
 if (resolved_module_spec.GetArchitecture().IsValid()) {
   error = ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
-  module_search_paths_ptr, nullptr, 
nullptr);
+  module_search_paths_ptr, nullptr,
+  nullptr);
   if (error.Fail()) {
 // If we failed, it may be because the vendor and os aren't known. If
-   // that is the case, try setting them to the host architecture and give
-   // it another try.
+// that is the case, try setting them to the host architecture and give
+// it another try.
 llvm::Triple &module_triple =
 resolved_module_spec.GetArchitecture().GetTriple();
 bool is_vendor_specified =
@@ -113,8 +114,9 @@ Status RemoteAwarePlatform::ResolveExecutable(
   if (!is_os_specified)
 module_triple.setOSName(host_triple.getOSName());
 
-  error = ModuleList::GetSharedModule(resolved_module_spec,
-  exe_module_sp, 
module_search_paths_ptr, nullptr, nullptr);
+  error = ModuleList::GetSharedModule(
+  resolved_module_spec, exe_module_sp, module_search_paths_ptr,
+  nullptr, nullptr);
 }
   }
 
@@ -137,7 +139,8 @@ Status RemoteAwarePlatform::ResolveExecutable(
GetSupportedArchitectures(process_host_arch)) {
 resolved_module_spec.GetArchitecture() = arch;
 error = ModuleList::GetSharedModule(resolved_module_spec, 
exe_module_sp,
-module_search_paths_ptr, nullptr, 
nullptr);
+module_search_paths_ptr, nullptr,
+nullptr);
 // Did we find an executable using one of the
 if (error.Success()) {
   if (exe_module_sp && exe_module_sp->GetObjectFile())

``




https://github.com/llvm/llvm-project/pull/98581
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] [lldb] Add support for displaying `__float128` variables (PR #98369)

[via lldb-commits]

https://github.com/beetrees updated 
https://github.com/llvm/llvm-project/pull/98369

>From 5e29db435c239a4817256d308727d772dca1a7e8 Mon Sep 17 00:00:00 2001
From: beetrees 
Date: Thu, 11 Jul 2024 19:56:45 +0100
Subject: [PATCH 1/2] [lldb] Format `lldb-enumerations.h` (NFC)

---
 lldb/include/lldb/lldb-enumerations.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lldb/include/lldb/lldb-enumerations.h 
b/lldb/include/lldb/lldb-enumerations.h
index 74ff458bf87de..5c32d71ee5d84 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -195,10 +195,10 @@ enum Format {
  ///< character arrays that can contain non printable
  ///< characters
   eFormatAddressInfo,///< Describe what an address points to (func + offset
-  ///< with file/line, symbol + offset, data, etc)
-  eFormatHexFloat,///< ISO C99 hex float string
-  eFormatInstruction, ///< Disassemble an opcode
-  eFormatVoid,///< Do not print this
+ ///< with file/line, symbol + offset, data, etc)
+  eFormatHexFloat,   ///< ISO C99 hex float string
+  eFormatInstruction,///< Disassemble an opcode
+  eFormatVoid,   ///< Do not print this
   eFormatUnicode8,
   kNumFormats
 };

>From fdd2a20380ef599952084b77206a40a6754bc564 Mon Sep 17 00:00:00 2001
From: beetrees 
Date: Wed, 10 Jul 2024 18:49:45 +0100
Subject: [PATCH 2/2] [lldb] Add support for displaying `__float128` variables

---
 lldb/bindings/python/python-extensions.swig   |  1 +
 lldb/docs/python_api_enums.rst|  2 ++
 lldb/include/lldb/lldb-enumerations.h |  7 ++-
 lldb/source/Commands/CommandObjectMemory.cpp  |  2 ++
 lldb/source/Core/DumpDataExtractor.cpp|  5 -
 lldb/source/Core/ValueObject.cpp  |  5 +++--
 lldb/source/DataFormatters/FormatManager.cpp  |  1 +
 lldb/source/DataFormatters/VectorType.cpp |  2 ++
 .../TypeSystem/Clang/TypeSystemClang.cpp  | 21 +++
 lldb/unittests/Core/DumpDataExtractorTest.cpp |  6 ++
 lldb/unittests/Symbol/TestTypeSystemClang.cpp |  2 ++
 11 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/lldb/bindings/python/python-extensions.swig 
b/lldb/bindings/python/python-extensions.swig
index 4ba1607c70909..40fa76872ee96 100644
--- a/lldb/bindings/python/python-extensions.swig
+++ b/lldb/bindings/python/python-extensions.swig
@@ -594,6 +594,7 @@ def is_numeric_type(basic_type):
 if basic_type == eBasicTypeFloat: return (True,True)
 if basic_type == eBasicTypeDouble: return (True,True)
 if basic_type == eBasicTypeLongDouble: return (True,True)
+if basic_type == eBasicTypeFloat128: return (True,True)
 if basic_type == eBasicTypeFloatComplex: return (True,True)
 if basic_type == eBasicTypeDoubleComplex: return (True,True)
 if basic_type == eBasicTypeLongDoubleComplex: return (True,True)
diff --git a/lldb/docs/python_api_enums.rst b/lldb/docs/python_api_enums.rst
index b6a2497ea878e..a43a47b8d6985 100644
--- a/lldb/docs/python_api_enums.rst
+++ b/lldb/docs/python_api_enums.rst
@@ -321,6 +321,7 @@ Format
 .. py:data:: eFormatInstruction
 .. py:data:: eFormatVoid
 .. py:data:: eFormatUnicode8
+.. py:data:: eFormatFloat128
 
 
 .. _DescriptionLevel:
@@ -1045,6 +1046,7 @@ BasicType
 .. py:data:: eBasicTypeObjCSel
 .. py:data:: eBasicTypeNullPtr
 .. py:data:: eBasicTypeOther
+.. py:data:: eBasicTypeFloat128
 
 
 .. _TraceType:
diff --git a/lldb/include/lldb/lldb-enumerations.h 
b/lldb/include/lldb/lldb-enumerations.h
index 5c32d71ee5d84..5a62e4064a855 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -200,6 +200,10 @@ enum Format {
   eFormatInstruction,///< Disassemble an opcode
   eFormatVoid,   ///< Do not print this
   eFormatUnicode8,
+  eFormatFloat128, //< Disambiguate between 128-bit `long double` (which uses
+   //< `eFormatFloat`) and `__float128` (which uses
+   //< `eFormatFloat128`). If the value being formatted is not
+   //< 128 bits, then this is identical to `eFormatFloat`.
   kNumFormats
 };
 
@@ -826,7 +830,8 @@ enum BasicType {
   eBasicTypeObjCClass,
   eBasicTypeObjCSel,
   eBasicTypeNullPtr,
-  eBasicTypeOther
+  eBasicTypeOther,
+  eBasicTypeFloat128
 };
 
 /// Deprecated
diff --git a/lldb/source/Commands/CommandObjectMemory.cpp 
b/lldb/source/Commands/CommandObjectMemory.cpp
index 137b1ad981073..dde27d2df0fe5 100644
--- a/lldb/source/Commands/CommandObjectMemory.cpp
+++ b/lldb/source/Commands/CommandObjectMemory.cpp
@@ -156,6 +156,7 @@ class OptionGroupReadMemory : public OptionGroup {
 
 case eFormatBinary:
 case eFormatFloat:
+case eFormatFloat128:
 case eFormatOctal:
 case eFormatDecimal:
 case eFormatEnum:
@@ -1329,6 +1330,7 @@ class CommandObjectMemoryWrite : public 
CommandObjectParsed {
   switch (m_format_option

[Lldb-commits] [lldb] [lldb] Add support for displaying `__float128` variables (PR #98369)

[via lldb-commits]

beetrees wrote:

(I've fixed the CI failure.)

https://github.com/llvm/llvm-project/pull/98369
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #98361)

[Pavel Labath via lldb-commits]

https://github.com/labath edited https://github.com/llvm/llvm-project/pull/98361
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #98361)

[Pavel Labath via lldb-commits]

https://github.com/labath commented:

Thanks. I think this looks pretty good now, just a couple of details to sort 
out.

https://github.com/llvm/llvm-project/pull/98361
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #98361)

[Pavel Labath via lldb-commits]

@@ -824,6 +824,36 @@ DWARFASTParserClang::GetDIEClassTemplateParams(const 
DWARFDIE &die) {
   return {};
 }
 
+void DWARFASTParserClang::MappingDeclDIEToDefDIE(
+const lldb_private::plugin::dwarf::DWARFDIE &decl_die,
+const lldb_private::plugin::dwarf::DWARFDIE &def_die) {
+  LinkDeclContextToDIE(GetCachedClangDeclContextForDIE(decl_die), def_die);
+  SymbolFileDWARF *dwarf = def_die.GetDWARF();
+  ParsedDWARFTypeAttributes decl_attrs(decl_die);
+  ParsedDWARFTypeAttributes def_attrs(def_die);
+  ConstString unique_typename(decl_attrs.name);
+  Declaration decl_declaration(decl_attrs.decl);
+  if (Language::LanguageIsCPlusPlus(
+  SymbolFileDWARF::GetLanguage(*decl_die.GetCU( {
+std::string qualified_name = GetCPlusPlusQualifiedName(decl_die);
+if (!qualified_name.empty())
+  unique_typename = ConstString(qualified_name);
+decl_declaration.Clear();
+  }

labath wrote:

Could we move create a helper function to unify this bit with the equivalent 
code in ParseTypeFromDWARF?

https://github.com/llvm/llvm-project/pull/98361
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #98361)

[Pavel Labath via lldb-commits]

@@ -1631,27 +1638,49 @@ bool SymbolFileDWARF::CompleteType(CompilerType 
&compiler_type) {
 return true;
   }
 
-  DWARFDIE dwarf_die = GetDIE(die_it->getSecond());
-  if (dwarf_die) {
-// Once we start resolving this type, remove it from the forward
-// declaration map in case anyone child members or other types require this
-// type to get resolved. The type will get resolved when all of the calls
-// to SymbolFileDWARF::ResolveClangOpaqueTypeDefinition are done.
-GetForwardDeclCompilerTypeToDIE().erase(die_it);
-
-Type *type = GetDIEToType().lookup(dwarf_die.GetDIE());
+  DWARFDIE decl_die = GetDIE(die_it->getSecond());
+  // Once we start resolving this type, remove it from the forward
+  // declaration map in case anyone's child members or other types require this
+  // type to get resolved.
+  GetForwardDeclCompilerTypeToDIE().erase(die_it);
+  DWARFDIE def_die = FindDefinitionDIE(decl_die);
+  if (!def_die) {
+SymbolFileDWARFDebugMap *debug_map_symfile = GetDebugMapSymfile();
+if (debug_map_symfile) {
+  // We weren't able to find a full declaration in this DWARF, see
+  // if we have a declaration anywhere else...
+  def_die = debug_map_symfile->FindDefinitionDIE(decl_die);
+}
+  }
+  if (!def_die) {
+// If we don't have definition DIE, CompleteTypeFromDWARF will forcefully
+// complete this type.
+def_die = decl_die;
+  }
 
-Log *log = GetLog(DWARFLog::DebugInfo | DWARFLog::TypeCompletion);
-if (log)
-  GetObjectFile()->GetModule()->LogMessageVerboseBacktrace(
-  log, "{0:x8}: {1} ({2}) '{3}' resolving forward declaration...",
-  dwarf_die.GetID(), DW_TAG_value_to_name(dwarf_die.Tag()),
-  dwarf_die.Tag(), type->GetName().AsCString());
-assert(compiler_type);
-if (DWARFASTParser *dwarf_ast = GetDWARFParser(*dwarf_die.GetCU()))
-  return dwarf_ast->CompleteTypeFromDWARF(dwarf_die, type, compiler_type);
+  DWARFASTParser *dwarf_ast = GetDWARFParser(*def_die.GetCU());
+  if (!dwarf_ast)
+return false;
+  Type *type = GetDIEToType().lookup(decl_die.GetDIE());
+  if (decl_die != def_die) {
+GetDIEToType()[def_die.GetDIE()] = type;
+// Need to update Type ID to refer to the definition DIE. because
+// it's used in ParseCXXMethod to determine if we need to copy cxx
+// method types from a declaration DIE to this definition DIE.
+type->SetID(def_die.GetID());
+if (DWARFASTParserClang *ast_parser =
+static_cast(dwarf_ast))

labath wrote:

this check is superfluous. static_cast will never turn a non-null pointer into 
nullptr.

https://github.com/llvm/llvm-project/pull/98361
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #98361)

[Pavel Labath via lldb-commits]

@@ -824,6 +824,36 @@ DWARFASTParserClang::GetDIEClassTemplateParams(const 
DWARFDIE &die) {
   return {};
 }
 
+void DWARFASTParserClang::MappingDeclDIEToDefDIE(

labath wrote:

nit: Maybe you're reading this another way, but this name does not sound like a 
verb phrase to me (which I think it should be, as the function changes the 
state of the program). I'd just call it `MapDeclDIEToDefDIE`

https://github.com/llvm/llvm-project/pull/98361
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #98361)

[Pavel Labath via lldb-commits]

@@ -4,8 +4,8 @@
 
 // REQUIRES: lld
 
-// RUN: %clang --target=x86_64-pc-linux -c %s -o %t-a.o -g 
-gsimple-template-names -DFILE_A
-// RUN: %clang --target=x86_64-pc-linux -c %s -o %t-b.o -g 
-gsimple-template-names -DFILE_B

labath wrote:

We should keep the existing test, but you can add another set of RUN lines to 
it.
If you do that, it would also be good to rename the test to reflect its wider 
scope. Maybe to something about type definition searching, as that's the common 
theme in the two cases.

https://github.com/llvm/llvm-project/pull/98361
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #98361)

[Pavel Labath via lldb-commits]

@@ -824,6 +824,36 @@ DWARFASTParserClang::GetDIEClassTemplateParams(const 
DWARFDIE &die) {
   return {};
 }
 
+void DWARFASTParserClang::MappingDeclDIEToDefDIE(
+const lldb_private::plugin::dwarf::DWARFDIE &decl_die,
+const lldb_private::plugin::dwarf::DWARFDIE &def_die) {
+  LinkDeclContextToDIE(GetCachedClangDeclContextForDIE(decl_die), def_die);
+  SymbolFileDWARF *dwarf = def_die.GetDWARF();
+  ParsedDWARFTypeAttributes decl_attrs(decl_die);
+  ParsedDWARFTypeAttributes def_attrs(def_die);
+  ConstString unique_typename(decl_attrs.name);
+  Declaration decl_declaration(decl_attrs.decl);
+  if (Language::LanguageIsCPlusPlus(
+  SymbolFileDWARF::GetLanguage(*decl_die.GetCU( {
+std::string qualified_name = GetCPlusPlusQualifiedName(decl_die);
+if (!qualified_name.empty())
+  unique_typename = ConstString(qualified_name);
+decl_declaration.Clear();
+  }
+  if (UniqueDWARFASTType *unique_ast_entry_type =

labath wrote:

This should always be true, right (because we've already parsed the type and 
inserted the proper entry? Failing to find the entry here probably deserves at 
least a log message..

https://github.com/llvm/llvm-project/pull/98361
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Reapply [lldb][DWARF] Delay struct/class/union definition DIE searching when parsing declaration DIEs. (PR #98361)

[Pavel Labath via lldb-commits]

@@ -1603,41 +1633,76 @@ DWARFASTParserClang::GetCPlusPlusQualifiedName(const 
DWARFDIE &die) {
 
 TypeSP
 DWARFASTParserClang::ParseStructureLikeDIE(const SymbolContext &sc,
-   const DWARFDIE &decl_die,
+   const DWARFDIE &die,
ParsedDWARFTypeAttributes &attrs) {
   CompilerType clang_type;
-  const dw_tag_t tag = decl_die.Tag();
-  SymbolFileDWARF *dwarf = decl_die.GetDWARF();
-  LanguageType cu_language = SymbolFileDWARF::GetLanguage(*decl_die.GetCU());
+  const dw_tag_t tag = die.Tag();
+  SymbolFileDWARF *dwarf = die.GetDWARF();
+  LanguageType cu_language = SymbolFileDWARF::GetLanguage(*die.GetCU());
   Log *log = GetLog(DWARFLog::TypeCompletion | DWARFLog::Lookups);
 
-  // UniqueDWARFASTType is large, so don't create a local variables on the
-  // stack, put it on the heap. This function is often called recursively and
-  // clang isn't good at sharing the stack space for variables in different
-  // blocks.
-  auto unique_ast_entry_up = std::make_unique();
-
   ConstString unique_typename(attrs.name);
   Declaration unique_decl(attrs.decl);
+  uint64_t byte_size = attrs.byte_size.value_or(0);
+  if (attrs.byte_size && *attrs.byte_size == 0 && attrs.name &&
+  !die.HasChildren() && cu_language == eLanguageTypeObjC) {
+// Work around an issue with clang at the moment where forward
+// declarations for objective C classes are emitted as:
+//  DW_TAG_structure_type [2]
+//  DW_AT_name( "ForwardObjcClass" )
+//  DW_AT_byte_size( 0x00 )
+//  DW_AT_decl_file( "..." )
+//  DW_AT_decl_line( 1 )
+//
+// Note that there is no DW_AT_declaration and there are no children,
+// and the byte size is zero.
+attrs.is_forward_declaration = true;
+  }
 
   if (attrs.name) {
 if (Language::LanguageIsCPlusPlus(cu_language)) {
   // For C++, we rely solely upon the one definition rule that says
   // only one thing can exist at a given decl context. We ignore the
   // file and line that things are declared on.
-  std::string qualified_name = GetCPlusPlusQualifiedName(decl_die);
+  std::string qualified_name = GetCPlusPlusQualifiedName(die);
   if (!qualified_name.empty())
 unique_typename = ConstString(qualified_name);
   unique_decl.Clear();
 }
 
-if (dwarf->GetUniqueDWARFASTTypeMap().Find(
-unique_typename, decl_die, unique_decl,
-attrs.byte_size.value_or(-1), *unique_ast_entry_up)) {
-  if (TypeSP type_sp = unique_ast_entry_up->m_type_sp) {
+if (UniqueDWARFASTType *unique_ast_entry_type =
+dwarf->GetUniqueDWARFASTTypeMap().Find(
+unique_typename, die, unique_decl, byte_size,
+attrs.is_forward_declaration)) {
+  if (TypeSP type_sp = unique_ast_entry_type->m_type_sp) {
+dwarf->GetDIEToType()[die.GetDIE()] = type_sp.get();
 LinkDeclContextToDIE(
-GetCachedClangDeclContextForDIE(unique_ast_entry_up->m_die),
-decl_die);
+GetCachedClangDeclContextForDIE(unique_ast_entry_type->m_die), 
die);
+// If the DIE being parsed in this function is a definition and the
+// entry in the map is a declaration, then we need to update the entry
+// to point to the definition DIE.
+if (!attrs.is_forward_declaration &&
+unique_ast_entry_type->m_is_forward_declaration) {
+  unique_ast_entry_type->m_die = die;
+  if (byte_size)
+unique_ast_entry_type->m_byte_size = byte_size;
+  if (unique_decl.IsValid())
+unique_ast_entry_type->m_declaration = unique_decl;
+  unique_ast_entry_type->m_is_forward_declaration = false;
+  // Need to update Type ID to refer to the definition DIE. because
+  // it's used in ParseCXXMethod to determine if we need to copy cxx
+  // method types from a declaration DIE to this definition DIE.
+  type_sp->SetID(die.GetID());

labath wrote:

And this could be a helper function to deduplicate the second part of 
`MappingDeclDIEToDefDIE`

https://github.com/llvm/llvm-project/pull/98361
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits