[Lldb-commits] [lldb] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)

2024-03-01 Thread Mehdi Amini via lldb-commits 


@@ -209,25 +231,66 @@ class ThreadPool {
   /// Number of threads active for tasks in the given group (only non-zero).
   DenseMap ActiveGroups;
 
-#if LLVM_ENABLE_THREADS // avoids warning for unused variable
   /// Signal for the destruction of the pool, asking thread to exit.
   bool EnableFlag = true;
-#endif
 
   const ThreadPoolStrategy Strategy;
 
   /// Maximum number of threads to potentially grow this pool to.
   const unsigned MaxThreadCount;
 };
 
+/// A non-threaded implementation.
+class SingleThreadExecutor : public ThreadPoolInterface {

joker-eph wrote:

I just implemented it: now the unit-tests are ran for both the single thread 
and std::thread implementation (except when LLVM_ENABLE_THREADS=OFF, then only 
the single-thread implementation is tested).

Can you take a look?

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

[Lldb-commits] [lldb] [lldb] Add SBProcess methods for get/set/use address masks (PR #83095)

2024-03-01 Thread Jason Molenda via lldb-commits 

jasonmolenda wrote:

After debugging, updating, and testing on aarch64-unbuntu, x86_64-macos, and 
arm64-macos, I have created a new PR with this commit plus an additional commit 
to fix the issues I found on the different platforms. 
https://github.com/llvm/llvm-project/pull/83663

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

[Lldb-commits] [lldb] Address mask sbprocess apis and new mask invalid const (PR #83663)

2024-03-01 Thread Jason Molenda via lldb-commits 

jasonmolenda wrote:

@DavidSpickett I spent a little time debugging this on aarch64-ubuntu and also 
testing on x86_64-macos (which has no address masks at all).  This PR has the 
original commit from https://github.com/llvm/llvm-project/pull/83095 and then a 
second commit with the changes I needed to make as I debugged this in the full 
environment, so you only need to look at the second commit for the new changes.

The main question I have (beyond feedback on how I describe things in comments 
of course ;) ) is about how TestAddressMasks.py assumes all Fix*Address 
implementations will handle a low and high memory address mask.  The test 
currently assumes they do, but that means all people writing these methods need 
to get the highmem mask if it is a high-mem addr_t and apply that mask if it is 
set.  I think it's the right choice, but I'm not wedded to it; maybe the right 
choice is narrowing that part of TestAddressMasks.py to only run on darwin 
systems.

I also added a base class ABI::Fix*Address so these tests can be used on 
targets with no address masks at all, if someone wants to manipulate them via 
SB API for some reason (of course, the real reason is to make the test 
runnable).  Maybe the test should be limited to only running on aarch64-linux 
and arm64-apple, I also am not convinced my choice is best here.

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

[Lldb-commits] [lldb] Address mask sbprocess apis and new mask invalid const (PR #83663)

2024-03-01 Thread Jason Molenda via lldb-commits 

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

[Lldb-commits] [lldb] Address mask sbprocess apis and new mask invalid const (PR #83663)

2024-03-01 Thread via lldb-commits 

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jason Molenda (jasonmolenda)


Changes

[lldb] Add SBProcess methods for get/set/use address masks (#83095)

I'm reviving a patch from phabracator, https://reviews.llvm.org/D155905
which was approved but I wasn't thrilled with all the API I was adding
to SBProcess for all of the address mask types / memory regions. In this
update, I added enums to control type address mask type (code, data,
any) and address space specifiers (low, high, all) with defaulted
arguments for the most common case.

This patch is also fixing a bug in the "addressable bits to address
mask" calculation I added in AddressableBits::SetProcessMasks. If lldb
were told that 64 bits are valid for addressing, this method would
overflow the calculation and set an invalid mask. Added tests to check
this specific bug while I was adding these APIs.

This patch changes the value of "no mask set" from 0 to
LLDB_INVALID_ADDRESS_MASK, which is UINT64_MAX. A mask of all 1's
means "no bits are used for addressing" which is an impossible mask,
whereas a mask of 0 means "all bits are used for addressing" which
is possible.

I added a base class implementation of ABI::FixCodeAddress and
ABI::FixDataAddress that will apply the Process mask values if they
are set to a value other than LLDB_INVALID_ADDRESS_MASK.

I updated all the callers/users of the Mask methods which were
handling a value of 0 to mean invalid mask to use
LLDB_INVALID_ADDRESS_MASK.

I added code to the ABISysV_arm64 Fix override methods to apply the
Highmem masks if they have been set.  These will not be set on a
Linux environment, but I have tests in TestAddressMasks.py which
set the highmem masks and I need all Fix*Mask implementations to
check  use them for the tests to pass.

rdar://123530562


---

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


18 Files Affected:

- (modified) lldb/include/lldb/API/SBProcess.h (+114) 
- (modified) lldb/include/lldb/Target/ABI.h (+2-6) 
- (modified) lldb/include/lldb/Target/Process.h (+27-9) 
- (modified) lldb/include/lldb/Utility/AddressableBits.h (+3) 
- (modified) lldb/include/lldb/lldb-defines.h (+5) 
- (modified) lldb/include/lldb/lldb-enumerations.h (+16) 
- (modified) lldb/source/API/SBProcess.cpp (+92) 
- (modified) lldb/source/Commands/CommandObjectProcess.cpp (+1-1) 
- (modified) lldb/source/Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp (+4-4) 
- (modified) lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp (+30-10) 
- (modified) 
lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp 
(+2-1) 
- (modified) lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp (+2-2) 
- (modified) lldb/source/Target/ABI.cpp (+26) 
- (modified) lldb/source/Target/Process.cpp (+8-6) 
- (modified) lldb/source/Utility/AddressableBits.cpp (+10-2) 
- (added) lldb/test/API/python_api/process/address-masks/Makefile (+3) 
- (added) lldb/test/API/python_api/process/address-masks/TestAddressMasks.py 
(+74) 
- (added) lldb/test/API/python_api/process/address-masks/main.c (+5) 


``diff
diff --git a/lldb/include/lldb/API/SBProcess.h 
b/lldb/include/lldb/API/SBProcess.h
index 4f92a41f3028a2..7da3335a7234b7 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/lldb/include/lldb/API/SBProcess.h
@@ -407,6 +407,120 @@ class LLDB_API SBProcess {
   /// the process isn't loaded from a core file.
   lldb::SBFileSpec GetCoreFile();
 
+  /// \{
+  /// \group Mask Address Methods
+  ///
+  /// \a type
+  /// All of the methods in this group take \a type argument
+  /// which is an AddressMaskType enum value.
+  /// There can be different address masks for code addresses and
+  /// data addresses, this argument can select which to get/set,
+  /// or to use when clearing non-addressable bits from an address.
+  /// This choice of mask can be important for example on AArch32
+  /// systems. Where instructions where instructions start on even addresses,
+  /// the 0th bit may be used to indicate that a function is thumb code.  On
+  /// such a target, the eAddressMaskTypeCode may clear the 0th bit from an
+  /// address to get the actual address Whereas eAddressMaskTypeData would not.
+  ///
+  /// \a addr_range
+  /// Many of the methods in this group take an \a addr_range argument
+  /// which is an AddressMaskRange enum value.
+  /// Needing to specify the address range is highly unusual, and the
+  /// default argument can be used in nearly all circumstances.
+  /// On some architectures (e.g., AArch64), it is possible to have
+  /// different page table setups for low and high memory, so different
+  /// numbers of bits relevant to addressing. It is possible to have
+  /// a program running in one half of memory and accessing the other
+  /// as heap, so we need to maintain two different sets of address masks
+  /// to debug this correctly.
+
+  /// Get the current address mask that will be applied to addresses
+  ///

[Lldb-commits] [lldb] Address mask sbprocess apis and new mask invalid const (PR #83663)

2024-03-01 Thread Jason Molenda via lldb-commits 

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

[lldb] Add SBProcess methods for get/set/use address masks (#83095)

I'm reviving a patch from phabracator, https://reviews.llvm.org/D155905
which was approved but I wasn't thrilled with all the API I was adding
to SBProcess for all of the address mask types / memory regions. In this
update, I added enums to control type address mask type (code, data,
any) and address space specifiers (low, high, all) with defaulted
arguments for the most common case.

This patch is also fixing a bug in the "addressable bits to address
mask" calculation I added in AddressableBits::SetProcessMasks. If lldb
were told that 64 bits are valid for addressing, this method would
overflow the calculation and set an invalid mask. Added tests to check
this specific bug while I was adding these APIs.

This patch changes the value of "no mask set" from 0 to
LLDB_INVALID_ADDRESS_MASK, which is UINT64_MAX. A mask of all 1's
means "no bits are used for addressing" which is an impossible mask,
whereas a mask of 0 means "all bits are used for addressing" which
is possible.

I added a base class implementation of ABI::FixCodeAddress and
ABI::FixDataAddress that will apply the Process mask values if they
are set to a value other than LLDB_INVALID_ADDRESS_MASK.

I updated all the callers/users of the Mask methods which were
handling a value of 0 to mean invalid mask to use
LLDB_INVALID_ADDRESS_MASK.

I added code to the ABISysV_arm64 Fix override methods to apply the
Highmem masks if they have been set.  These will not be set on a
Linux environment, but I have tests in TestAddressMasks.py which
set the highmem masks and I need all Fix*Mask implementations to
check & use them for the tests to pass.

rdar://123530562


>From c993c7cc7c1669ca7d06e52f1a1ff8dbefe9ebc9 Mon Sep 17 00:00:00 2001
From: Jason Molenda 
Date: Thu, 29 Feb 2024 17:02:42 -0800
Subject: [PATCH 1/2] [lldb] Add SBProcess methods for get/set/use address
 masks (#83095)

I'm reviving a patch from phabracator, https://reviews.llvm.org/D155905
which was approved but I wasn't thrilled with all the API I was adding
to SBProcess for all of the address mask types / memory regions. In this
update, I added enums to control type address mask type (code, data,
any) and address space specifiers (low, high, all) with defaulted
arguments for the most common case.

This patch is also fixing a bug in the "addressable bits to address
mask" calculation I added in AddressableBits::SetProcessMasks. If lldb
were told that 64 bits are valid for addressing, this method would
overflow the calculation and set an invalid mask. Added tests to check
this specific bug while I was adding these APIs.

rdar://123530562
---
 lldb/include/lldb/API/SBProcess.h | 114 ++
 lldb/include/lldb/Utility/AddressableBits.h   |   3 +
 lldb/include/lldb/lldb-defines.h  |   5 +
 lldb/include/lldb/lldb-enumerations.h |  16 +++
 lldb/source/API/SBProcess.cpp |  92 ++
 lldb/source/Target/Process.cpp|  10 +-
 lldb/source/Utility/AddressableBits.cpp   |  12 +-
 .../python_api/process/address-masks/Makefile |   3 +
 .../process/address-masks/TestAddressMasks.py |  74 
 .../python_api/process/address-masks/main.c   |   5 +
 10 files changed, 328 insertions(+), 6 deletions(-)
 create mode 100644 lldb/test/API/python_api/process/address-masks/Makefile
 create mode 100644 
lldb/test/API/python_api/process/address-masks/TestAddressMasks.py
 create mode 100644 lldb/test/API/python_api/process/address-masks/main.c

diff --git a/lldb/include/lldb/API/SBProcess.h 
b/lldb/include/lldb/API/SBProcess.h
index 4f92a41f3028a2..7da3335a7234b7 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/lldb/include/lldb/API/SBProcess.h
@@ -407,6 +407,120 @@ class LLDB_API SBProcess {
   /// the process isn't loaded from a core file.
   lldb::SBFileSpec GetCoreFile();
 
+  /// \{
+  /// \group Mask Address Methods
+  ///
+  /// \a type
+  /// All of the methods in this group take \a type argument
+  /// which is an AddressMaskType enum value.
+  /// There can be different address masks for code addresses and
+  /// data addresses, this argument can select which to get/set,
+  /// or to use when clearing non-addressable bits from an address.
+  /// This choice of mask can be important for example on AArch32
+  /// systems. Where instructions where instructions start on even addresses,
+  /// the 0th bit may be used to indicate that a function is thumb code.  On
+  /// such a target, the eAddressMaskTypeCode may clear the 0th bit from an
+  /// address to get the actual address Whereas eAddressMaskTypeData would not.
+  ///
+  /// \a addr_range
+  /// Many of the methods in this group take an \a addr_range argument
+  /// which is an AddressMaskRange enum value.
+  /// Needing to specify the address range is highly unusual, and the
+  /// default argument

[Lldb-commits] [lldb] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)

2024-03-01 Thread Mehdi Amini via lldb-commits 

https://github.com/joker-eph updated 
https://github.com/llvm/llvm-project/pull/82094

>From fa249b44a8bbcbc7b0da148c51acd5792f393869 Mon Sep 17 00:00:00 2001
From: Mehdi Amini 
Date: Fri, 16 Feb 2024 21:55:57 -0800
Subject: [PATCH] Split the llvm::ThreadPool into an abstract base class and an
 implementation

This decouples the public API used to enqueue tasks and wait for completion
from the actual implementation, and opens up the possibility for clients to
set their own thread pool implementation for the pool.

https://discourse.llvm.org/t/construct-threadpool-from-vector-of-existing-threads/76883
---
 bolt/include/bolt/Core/ParallelUtilities.h|   3 +-
 lldb/include/lldb/Core/Debugger.h |   6 +-
 llvm/include/llvm/Debuginfod/Debuginfod.h |   6 +-
 .../llvm/Support/BalancedPartitioning.h   |   7 +-
 llvm/include/llvm/Support/ThreadPool.h| 181 +++---
 llvm/lib/Debuginfod/Debuginfod.cpp|   3 +-
 llvm/lib/Support/ThreadPool.cpp   |  41 ++--
 llvm/tools/llvm-cov/CoverageReport.h  |   4 +-
 llvm/tools/llvm-cov/SourceCoverageViewHTML.h  |   2 -
 llvm/unittests/Support/ThreadPool.cpp | 107 +++
 mlir/include/mlir/CAPI/Support.h  |   4 +-
 mlir/include/mlir/IR/MLIRContext.h|   6 +-
 mlir/include/mlir/IR/Threading.h  |   2 +-
 mlir/lib/CAPI/IR/IR.cpp   |   1 +
 mlir/lib/IR/MLIRContext.cpp   |   8 +-
 mlir/lib/Tools/mlir-opt/MlirOptMain.cpp   |   4 +-
 16 files changed, 220 insertions(+), 165 deletions(-)

diff --git a/bolt/include/bolt/Core/ParallelUtilities.h 
b/bolt/include/bolt/Core/ParallelUtilities.h
index 7d3af47757bce6..8a95a486113321 100644
--- a/bolt/include/bolt/Core/ParallelUtilities.h
+++ b/bolt/include/bolt/Core/ParallelUtilities.h
@@ -28,7 +28,6 @@ extern cl::opt TaskCount;
 } // namespace opts
 
 namespace llvm {
-class ThreadPool;
 
 namespace bolt {
 class BinaryContext;
@@ -51,7 +50,7 @@ enum SchedulingPolicy {
 };
 
 /// Return the managed thread pool and initialize it if not initiliazed.
-ThreadPool ();
+ThreadPoolInterface ();
 
 /// Perform the work on each BinaryFunction except those that are accepted
 /// by SkipPredicate, scheduling heuristic is based on SchedPolicy.
diff --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index 6ba90eb6ed8fdf..66b1e28a9cc618 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -52,7 +52,7 @@
 
 namespace llvm {
 class raw_ostream;
-class ThreadPool;
+class ThreadPoolInterface;
 } // namespace llvm
 
 namespace lldb_private {
@@ -499,8 +499,8 @@ class Debugger : public 
std::enable_shared_from_this,
 return m_broadcaster_manager_sp;
   }
 
-  /// Shared thread poll. Use only with ThreadPoolTaskGroup.
-  static llvm::ThreadPool ();
+  /// Shared thread pool. Use only with ThreadPoolTaskGroup.
+  static llvm::ThreadPoolInterface ();
 
   /// Report warning events.
   ///
diff --git a/llvm/include/llvm/Debuginfod/Debuginfod.h 
b/llvm/include/llvm/Debuginfod/Debuginfod.h
index ef03948a706c06..99fe15ad859794 100644
--- a/llvm/include/llvm/Debuginfod/Debuginfod.h
+++ b/llvm/include/llvm/Debuginfod/Debuginfod.h
@@ -97,7 +97,7 @@ Expected getCachedOrDownloadArtifact(
 StringRef UniqueKey, StringRef UrlPath, StringRef CacheDirectoryPath,
 ArrayRef DebuginfodUrls, std::chrono::milliseconds Timeout);
 
-class ThreadPool;
+class ThreadPoolInterface;
 
 struct DebuginfodLogEntry {
   std::string Message;
@@ -135,7 +135,7 @@ class DebuginfodCollection {
   // error.
   Expected updateIfStale();
   DebuginfodLog 
-  ThreadPool 
+  ThreadPoolInterface 
   Timer UpdateTimer;
   sys::Mutex UpdateMutex;
 
@@ -145,7 +145,7 @@ class DebuginfodCollection {
 
 public:
   DebuginfodCollection(ArrayRef Paths, DebuginfodLog ,
-   ThreadPool , double MinInterval);
+   ThreadPoolInterface , double MinInterval);
   Error update();
   Error updateForever(std::chrono::milliseconds Interval);
   Expected findDebugBinaryPath(object::BuildIDRef);
diff --git a/llvm/include/llvm/Support/BalancedPartitioning.h 
b/llvm/include/llvm/Support/BalancedPartitioning.h
index a8464ac0fe60e5..9738e742f7f1e9 100644
--- a/llvm/include/llvm/Support/BalancedPartitioning.h
+++ b/llvm/include/llvm/Support/BalancedPartitioning.h
@@ -50,7 +50,7 @@
 
 namespace llvm {
 
-class ThreadPool;
+class ThreadPoolInterface;
 /// A function with a set of utility nodes where it is beneficial to order two
 /// functions close together if they have similar utility nodes
 class BPFunctionNode {
@@ -115,7 +115,7 @@ class BalancedPartitioning {
   /// threads, so we need to track how many active threads that could spawn 
more
   /// threads.
   struct BPThreadPool {
-ThreadPool 
+ThreadPoolInterface 
 std::mutex mtx;
 std::condition_variable cv;
 /// The number of threads that could spawn more threads
@@ -128,7 +128,8 @@

[Lldb-commits] [lldb] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)

2024-03-01 Thread Mehdi Amini via lldb-commits 

https://github.com/joker-eph updated 
https://github.com/llvm/llvm-project/pull/82094

>From e2a6d2860c21489445a87bfd4ced85108462f601 Mon Sep 17 00:00:00 2001
From: Mehdi Amini 
Date: Fri, 16 Feb 2024 21:55:57 -0800
Subject: [PATCH] Split the llvm::ThreadPool into an abstract base class and an
 implementation

This decouples the public API used to enqueue tasks and wait for completion
from the actual implementation, and opens up the possibility for clients to
set their own thread pool implementation for the pool.

https://discourse.llvm.org/t/construct-threadpool-from-vector-of-existing-threads/76883
---
 bolt/include/bolt/Core/ParallelUtilities.h|   3 +-
 lldb/include/lldb/Core/Debugger.h |   6 +-
 llvm/include/llvm/Debuginfod/Debuginfod.h |   6 +-
 .../llvm/Support/BalancedPartitioning.h   |   7 +-
 llvm/include/llvm/Support/ThreadPool.h| 191 --
 llvm/lib/Debuginfod/Debuginfod.cpp|   3 +-
 llvm/lib/Support/ThreadPool.cpp   |  41 ++--
 llvm/tools/llvm-cov/CoverageReport.h  |   4 +-
 llvm/tools/llvm-cov/SourceCoverageViewHTML.h  |   2 -
 llvm/unittests/Support/ThreadPool.cpp | 103 ++
 mlir/include/mlir/CAPI/Support.h  |   4 +-
 mlir/include/mlir/IR/MLIRContext.h|   6 +-
 mlir/include/mlir/IR/Threading.h  |   2 +-
 mlir/lib/CAPI/IR/IR.cpp   |   1 +
 mlir/lib/IR/MLIRContext.cpp   |   8 +-
 mlir/lib/Tools/mlir-opt/MlirOptMain.cpp   |   4 +-
 16 files changed, 235 insertions(+), 156 deletions(-)

diff --git a/bolt/include/bolt/Core/ParallelUtilities.h 
b/bolt/include/bolt/Core/ParallelUtilities.h
index 7d3af47757bce6..8a95a486113321 100644
--- a/bolt/include/bolt/Core/ParallelUtilities.h
+++ b/bolt/include/bolt/Core/ParallelUtilities.h
@@ -28,7 +28,6 @@ extern cl::opt TaskCount;
 } // namespace opts
 
 namespace llvm {
-class ThreadPool;
 
 namespace bolt {
 class BinaryContext;
@@ -51,7 +50,7 @@ enum SchedulingPolicy {
 };
 
 /// Return the managed thread pool and initialize it if not initiliazed.
-ThreadPool ();
+ThreadPoolInterface ();
 
 /// Perform the work on each BinaryFunction except those that are accepted
 /// by SkipPredicate, scheduling heuristic is based on SchedPolicy.
diff --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index 6ba90eb6ed8fdf..66b1e28a9cc618 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -52,7 +52,7 @@
 
 namespace llvm {
 class raw_ostream;
-class ThreadPool;
+class ThreadPoolInterface;
 } // namespace llvm
 
 namespace lldb_private {
@@ -499,8 +499,8 @@ class Debugger : public 
std::enable_shared_from_this,
 return m_broadcaster_manager_sp;
   }
 
-  /// Shared thread poll. Use only with ThreadPoolTaskGroup.
-  static llvm::ThreadPool ();
+  /// Shared thread pool. Use only with ThreadPoolTaskGroup.
+  static llvm::ThreadPoolInterface ();
 
   /// Report warning events.
   ///
diff --git a/llvm/include/llvm/Debuginfod/Debuginfod.h 
b/llvm/include/llvm/Debuginfod/Debuginfod.h
index ef03948a706c06..99fe15ad859794 100644
--- a/llvm/include/llvm/Debuginfod/Debuginfod.h
+++ b/llvm/include/llvm/Debuginfod/Debuginfod.h
@@ -97,7 +97,7 @@ Expected getCachedOrDownloadArtifact(
 StringRef UniqueKey, StringRef UrlPath, StringRef CacheDirectoryPath,
 ArrayRef DebuginfodUrls, std::chrono::milliseconds Timeout);
 
-class ThreadPool;
+class ThreadPoolInterface;
 
 struct DebuginfodLogEntry {
   std::string Message;
@@ -135,7 +135,7 @@ class DebuginfodCollection {
   // error.
   Expected updateIfStale();
   DebuginfodLog 
-  ThreadPool 
+  ThreadPoolInterface 
   Timer UpdateTimer;
   sys::Mutex UpdateMutex;
 
@@ -145,7 +145,7 @@ class DebuginfodCollection {
 
 public:
   DebuginfodCollection(ArrayRef Paths, DebuginfodLog ,
-   ThreadPool , double MinInterval);
+   ThreadPoolInterface , double MinInterval);
   Error update();
   Error updateForever(std::chrono::milliseconds Interval);
   Expected findDebugBinaryPath(object::BuildIDRef);
diff --git a/llvm/include/llvm/Support/BalancedPartitioning.h 
b/llvm/include/llvm/Support/BalancedPartitioning.h
index a8464ac0fe60e5..9738e742f7f1e9 100644
--- a/llvm/include/llvm/Support/BalancedPartitioning.h
+++ b/llvm/include/llvm/Support/BalancedPartitioning.h
@@ -50,7 +50,7 @@
 
 namespace llvm {
 
-class ThreadPool;
+class ThreadPoolInterface;
 /// A function with a set of utility nodes where it is beneficial to order two
 /// functions close together if they have similar utility nodes
 class BPFunctionNode {
@@ -115,7 +115,7 @@ class BalancedPartitioning {
   /// threads, so we need to track how many active threads that could spawn 
more
   /// threads.
   struct BPThreadPool {
-ThreadPool 
+ThreadPoolInterface 
 std::mutex mtx;
 std::condition_variable cv;
 /// The number of threads that could spawn more threads
@@ -128,7 +128,8 @@ class

[Lldb-commits] [lldb] Change the return type of CalculateNumChildren to uint32_t. (PR #83501)

2024-03-01 Thread Jonas Devlieghere via lldb-commits 

JDevlieghere wrote:

> > Why not increase TypeSystem::GetNumChildren to return a size_t instead?
> 
> We could, but even 2^32 seems like an outrageous amount of children to me. 
> What would be a use-case for this? A ValueObject that has the entire address 
> space as synthetic children?
> 
> There is other code that assumes that we can store the result of 
> GetNumChildren() in an int64 and use negative numbers as out-of-band 
> signaling. We could replace that with std::optional of course.

But what's there to gain by limiting it to 32 bits instead of 64? Saying it's 
unlikely or overkill isn't a very strong argument. I think a possibly better 
answer to Alex' question is probably: because `SBValue::GetNumChildren` returns 
a `uint32_t` and we cannot change that, so we'd have to check for overflow at 
SB API level. I don't feel strongly either way (though if it were me, I'd make 
everything a `size_t`.) 

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

[Lldb-commits] [lldb] Change the return type of CalculateNumChildren to uint32_t. (PR #83501)

2024-03-01 Thread via lldb-commits 

jimingham wrote:

I got one bug report - which I can't currently find - from someone who had 
managed to make a thing this big and we wouldn't print elements out of it 
correctly.  We didn't do anything about it at the time, it did seem silly.

But it WAS actually silly, at least once.

Jim


> On Mar 1, 2024, at 4:36 PM, Adrian Prantl ***@***.***> wrote:
> 
> 
> Why not increase TypeSystem::GetNumChildren to return a size_t instead?
> 
> We could, but even 2^32 seems like an outrageous amount of children to me. 
> What would be a use-case for this? A ValueObject that has the entire address 
> space as synthetic children?
> 
> There is other code that assumes that we can store the result of 
> GetNumChildren() in an int64 and use negative numbers as out-of-band 
> signaling. We could replace that with std::optional of course.
> 
> —
> 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/83501
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] Change the return type of CalculateNumChildren to uint32_t. (PR #83501)

2024-03-01 Thread Adrian Prantl via lldb-commits 

adrian-prantl wrote:

> Why not increase TypeSystem::GetNumChildren to return a size_t instead?

We could, but even 2^32 seems like an outrageous amount of children to me. What 
would be a use-case for this? A ValueObject that has the entire address space 
as synthetic children?

There is other code that assumes that we can store the result of 
GetNumChildren() in an int64 and use negative numbers as out-of-band signaling. 
We could replace that with std::optional of course.

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

[Lldb-commits] [lldb] ebaf26d - [lldb] Fix -Wformat after #83602

2024-03-01 Thread Fangrui Song via lldb-commits 

Author: Fangrui Song
Date: 2024-03-01T12:09:32-08:00
New Revision: ebaf26dabec00c32177cd4fa47f3bf5902b194b7

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

LOG: [lldb] Fix -Wformat after #83602

Added: 


Modified: 
lldb/source/Commands/CommandObjectThread.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectThread.cpp 
b/lldb/source/Commands/CommandObjectThread.cpp
index 6d84315a471d95..cf4f8ccaa0c4aa 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -280,7 +280,7 @@ class ThreadStepScopeOptionGroup : public OptionGroup {
   if (!success)
 error.SetErrorStringWithFormat(
 "invalid boolean value for option '%c': %s", short_option,
-option_arg);
+option_arg.data());
   else {
 m_step_in_avoid_no_debug = avoid_no_debug ? eLazyBoolYes : eLazyBoolNo;
   }
@@ -293,7 +293,7 @@ class ThreadStepScopeOptionGroup : public OptionGroup {
   if (!success)
 error.SetErrorStringWithFormat(
 "invalid boolean value for option '%c': %s", short_option,
-option_arg);
+option_arg.data());
   else {
 m_step_out_avoid_no_debug = avoid_no_debug ? eLazyBoolYes : 
eLazyBoolNo;
   }



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

[Lldb-commits] [lldb] [lldb] Fix `thread backtrace --count` (PR #83602)

2024-03-01 Thread Jonas Devlieghere via lldb-commits 

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

[Lldb-commits] [lldb] af00945 - [lldb] Fix `thread backtrace --count` (#83602)

2024-03-01 Thread via lldb-commits 

Author: Jonas Devlieghere
Date: 2024-03-01T11:06:58-08:00
New Revision: af009451ec439593554f03bc714e46ad2cd41738

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

LOG: [lldb] Fix `thread backtrace --count` (#83602)

The help output for `thread backtrace` specifies that you can pass -1 to
`--count` to display all the frames.

```
-c  ( --count  )
How many frames to display (-1 for all)
```

However, that doesn't work:

```
(lldb) thread backtrace --count -1
error: invalid integer value for option 'c'
```

The problem is that we store the option value as an unsigned and the
code to parse the string correctly rejects it. There's two ways to fix
this:

1. Make `m_count` a signed value so that it accepts negative values and
appease the parser. The function that prints the frames takes an
unsigned so a negative value will just become a really large positive
value, which is what the current implementation relies on.
2. Keep `m_count` unsigned and instead use 0 the magic value to show all
frames. I don't really see a point in not showing any frames at all,
plus that's already broken (`error: error displaying backtrace for
thread: "0x0001"`).

This patch implements (2) and at the same time improve the error
reporting so that we print the invalid value when we cannot parse it.

rdar://123881767

Added: 
lldb/test/Shell/Commands/command-thread-backtrace.test

Modified: 
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Commands/Options.td

Removed: 




diff  --git a/lldb/source/Commands/CommandObjectThread.cpp 
b/lldb/source/Commands/CommandObjectThread.cpp
index 9cfff059d6bfa4..6d84315a471d95 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -67,13 +67,18 @@ class CommandObjectThreadBacktrace : public 
CommandObjectIterateOverThreads {
 if (option_arg.getAsInteger(0, m_count)) {
   m_count = UINT32_MAX;
   error.SetErrorStringWithFormat(
-  "invalid integer value for option '%c'", short_option);
+  "invalid integer value for option '%c': %s", short_option,
+  option_arg.data());
 }
+// A count of 0 means all frames.
+if (m_count == 0)
+  m_count = UINT32_MAX;
 break;
   case 's':
 if (option_arg.getAsInteger(0, m_start))
   error.SetErrorStringWithFormat(
-  "invalid integer value for option '%c'", short_option);
+  "invalid integer value for option '%c': %s", short_option,
+  option_arg.data());
 break;
   case 'e': {
 bool success;
@@ -81,7 +86,8 @@ class CommandObjectThreadBacktrace : public 
CommandObjectIterateOverThreads {
 OptionArgParser::ToBoolean(option_arg, false, );
 if (!success)
   error.SetErrorStringWithFormat(
-  "invalid boolean value for option '%c'", short_option);
+  "invalid boolean value for option '%c': %s", short_option,
+  option_arg.data());
   } break;
   default:
 llvm_unreachable("Unimplemented option");
@@ -228,9 +234,9 @@ class CommandObjectThreadBacktrace : public 
CommandObjectIterateOverThreads {
   thread->GetIndexID());
   return false;
 }
-if (m_options.m_extended_backtrace) { 
-  if (!INTERRUPT_REQUESTED(GetDebugger(), 
-  "Interrupt skipped extended backtrace")) {
+if (m_options.m_extended_backtrace) {
+  if (!INTERRUPT_REQUESTED(GetDebugger(),
+   "Interrupt skipped extended backtrace")) {
 DoExtendedBacktrace(thread, result);
   }
 }
@@ -272,8 +278,9 @@ class ThreadStepScopeOptionGroup : public OptionGroup {
   bool avoid_no_debug =
   OptionArgParser::ToBoolean(option_arg, true, );
   if (!success)
-error.SetErrorStringWithFormat("invalid boolean value for option '%c'",
-   short_option);
+error.SetErrorStringWithFormat(
+"invalid boolean value for option '%c': %s", short_option,
+option_arg);
   else {
 m_step_in_avoid_no_debug = avoid_no_debug ? eLazyBoolYes : eLazyBoolNo;
   }
@@ -284,8 +291,9 @@ class ThreadStepScopeOptionGroup : public OptionGroup {
   bool avoid_no_debug =
   OptionArgParser::ToBoolean(option_arg, true, );
   if (!success)
-error.SetErrorStringWithFormat("invalid boolean value for option '%c'",
-   short_option);
+error.SetErrorStringWithFormat(
+"invalid boolean value for option '%c': %s", short_option,
+option_arg);
   else {
 m_step_out_avoid_no_debug = avoid_no_debug ?

[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)

2024-03-01 Thread Chelsea Cassanova via lldb-commits 

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

[Lldb-commits] [lldb] 137ed17 - [lldb][progress] Hook up new broadcast bit and progress manager (#83069)

2024-03-01 Thread via lldb-commits 

Author: Chelsea Cassanova
Date: 2024-03-01T10:56:45-08:00
New Revision: 137ed17016757a6caea9db70ee6408dc3e2f6232

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

LOG: [lldb][progress] Hook up new broadcast bit and progress manager (#83069)

This commit adds the functionality to broadcast events using the
`Debugger::eBroadcastProgressCategory`
bit (https://github.com/llvm/llvm-project/pull/81169) by keeping track
of these reports with the `ProgressManager`
class (https://github.com/llvm/llvm-project/pull/81319). The new bit is
used in such a way that it will only broadcast the initial and final
progress reports for specific progress categories that are managed by
the progress manager.

This commit also adds a new test to the progress report unit test that
checks that only the initial/final reports are broadcasted when using
the new bit.

Added: 


Modified: 
lldb/include/lldb/Core/Debugger.h
lldb/include/lldb/Core/Progress.h
lldb/source/Core/Debugger.cpp
lldb/source/Core/Progress.cpp
lldb/unittests/Core/ProgressReportTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Core/Debugger.h 
b/lldb/include/lldb/Core/Debugger.h
index 6ba90eb6ed8fdf..b65ec1029ab24b 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -593,6 +593,7 @@ class Debugger : public 
std::enable_shared_from_this,
   friend class CommandInterpreter;
   friend class REPL;
   friend class Progress;
+  friend class ProgressManager;
 
   /// Report progress events.
   ///
@@ -623,10 +624,11 @@ class Debugger : public 
std::enable_shared_from_this,
   ///   debugger identifier that this progress should be delivered to. If this
   ///   optional parameter does not have a value, the progress will be
   ///   delivered to all debuggers.
-  static void ReportProgress(uint64_t progress_id, std::string title,
- std::string details, uint64_t completed,
- uint64_t total,
- std::optional debugger_id);
+  static void
+  ReportProgress(uint64_t progress_id, std::string title, std::string details,
+ uint64_t completed, uint64_t total,
+ std::optional debugger_id,
+ uint32_t progress_category_bit = eBroadcastBitProgress);
 
   static void ReportDiagnosticImpl(DiagnosticEventData::Type type,
std::string message,

diff  --git a/lldb/include/lldb/Core/Progress.h 
b/lldb/include/lldb/Core/Progress.h
index 9549e3b5aea950..c6fc861fb71d86 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -9,10 +9,11 @@
 #ifndef LLDB_CORE_PROGRESS_H
 #define LLDB_CORE_PROGRESS_H
 
-#include "lldb/Utility/ConstString.h"
+#include "lldb/lldb-forward.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/StringMap.h"
 #include 
+#include 
 #include 
 #include 
 
@@ -100,27 +101,36 @@ class Progress {
   /// Used to indicate a non-deterministic progress report
   static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
 
+  /// Data belonging to this Progress event that is used for bookkeeping by
+  /// ProgressManager.
+  struct ProgressData {
+/// The title of the progress activity, also used as a category.
+std::string title;
+/// A unique integer identifier for progress reporting.
+uint64_t progress_id;
+/// The optional debugger ID to report progress to. If this has no value
+/// then all debuggers will receive this event.
+std::optional debugger_id;
+  };
+
 private:
   void ReportProgress();
   static std::atomic g_id;
-  /// The title of the progress activity.
-  std::string m_title;
+  /// More specific information about the current file being displayed in the
+  /// report.
   std::string m_details;
-  std::mutex m_mutex;
-  /// A unique integer identifier for progress reporting.
-  const uint64_t m_id;
   /// How much work ([0...m_total]) that has been completed.
   uint64_t m_completed;
   /// Total amount of work, use a std::nullopt in the constructor for non
   /// deterministic progress.
   uint64_t m_total;
-  /// The optional debugger ID to report progress to. If this has no value then
-  /// all debuggers will receive this event.
-  std::optional m_debugger_id;
+  std::mutex m_mutex;
   /// Set to true when progress has been reported where m_completed == m_total
   /// to ensure that we don't send progress updates after progress has
   /// completed.
   bool m_complete = false;
+  /// Data needed by the debugger to broadcast a progress event.
+  ProgressData m_progress_data;
 };
 
 /// A class used to group progress reports by category. This is done by using a
@@ -133,13 +143,16 @@ class ProgressManager {
   ~ProgressManager();
 
   /// Control the

[Lldb-commits] [lldb] [lldb][X86] Fix setting target features in ClangExpressionParser (PR #82364)

2024-03-01 Thread Adrian Prantl via lldb-commits 

adrian-prantl wrote:

I'll defer to @Michael137 but if he's okay with this, I'm fine with it.

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

[Lldb-commits] [lldb] [lldb] Fix `thread backtrace --count` (PR #83602)

2024-03-01 Thread via lldb-commits 

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

LGTM.  We really need to get option values to parse somewhere more generically 
so we don't have to change so many places just to get a good error for value 
conversion, but that's not this PR's fault...

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

[Lldb-commits] [lldb] [lldb] Log to system log instead of stderr from Host::SystemLog (PR #83366)

2024-03-01 Thread Jonas Devlieghere via lldb-commits 

JDevlieghere wrote:

@rupprecht @oontvoo How do you feel about the current approach? The other 
alternative is to make this function a NOOP on all platforms but Darwin (where 
I want to actively use it). It's not currently load bearing (except the log, 
which is opt-in).

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

[Lldb-commits] [lldb] [lldb] Fix `thread backtrace --count` (PR #83602)

2024-03-01 Thread via lldb-commits 

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)


Changes

The help output for `thread backtrace` specifies that you can pass -1 to 
`--count` to display all the frames.

```
-c count ( --count count )
How many frames to display (-1 for all)
```

However, that doesn't work:

```
(lldb) thread backtrace --count -1
error: invalid integer value for option 'c'
```

The problem is that we store the option value as an unsigned and the code to 
parse the string correctly rejects it. There's two ways to fix this:

 1. Make `m_count` a signed value so that it accepts negative values and 
appease the parser. The function that prints the frames takes an unsigned so a 
negative value will just become a really large positive value, which is what 
the current implementation relies on.
 2. Keep `m_count` unsigned and instead use 0 the magic value to show all 
frames. I don't really see a point in not showing any frames at all, plus 
that's already broken (`error: error displaying backtrace for thread: 
"0x0001"`).

This patch implements (2) and at the same time improve the error reporting so 
that we print the invalid value when we cannot parse it.

rdar://123881767

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


3 Files Affected:

- (modified) lldb/source/Commands/CommandObjectThread.cpp (+21-12) 
- (modified) lldb/source/Commands/Options.td (+3-3) 
- (added) lldb/test/Shell/Commands/command-thread-backtrace.test (+14) 


``diff
diff --git a/lldb/source/Commands/CommandObjectThread.cpp 
b/lldb/source/Commands/CommandObjectThread.cpp
index 9cfff059d6bfa4..6d84315a471d95 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -67,13 +67,18 @@ class CommandObjectThreadBacktrace : public 
CommandObjectIterateOverThreads {
 if (option_arg.getAsInteger(0, m_count)) {
   m_count = UINT32_MAX;
   error.SetErrorStringWithFormat(
-  "invalid integer value for option '%c'", short_option);
+  "invalid integer value for option '%c': %s", short_option,
+  option_arg.data());
 }
+// A count of 0 means all frames.
+if (m_count == 0)
+  m_count = UINT32_MAX;
 break;
   case 's':
 if (option_arg.getAsInteger(0, m_start))
   error.SetErrorStringWithFormat(
-  "invalid integer value for option '%c'", short_option);
+  "invalid integer value for option '%c': %s", short_option,
+  option_arg.data());
 break;
   case 'e': {
 bool success;
@@ -81,7 +86,8 @@ class CommandObjectThreadBacktrace : public 
CommandObjectIterateOverThreads {
 OptionArgParser::ToBoolean(option_arg, false, );
 if (!success)
   error.SetErrorStringWithFormat(
-  "invalid boolean value for option '%c'", short_option);
+  "invalid boolean value for option '%c': %s", short_option,
+  option_arg.data());
   } break;
   default:
 llvm_unreachable("Unimplemented option");
@@ -228,9 +234,9 @@ class CommandObjectThreadBacktrace : public 
CommandObjectIterateOverThreads {
   thread->GetIndexID());
   return false;
 }
-if (m_options.m_extended_backtrace) { 
-  if (!INTERRUPT_REQUESTED(GetDebugger(), 
-  "Interrupt skipped extended backtrace")) {
+if (m_options.m_extended_backtrace) {
+  if (!INTERRUPT_REQUESTED(GetDebugger(),
+   "Interrupt skipped extended backtrace")) {
 DoExtendedBacktrace(thread, result);
   }
 }
@@ -272,8 +278,9 @@ class ThreadStepScopeOptionGroup : public OptionGroup {
   bool avoid_no_debug =
   OptionArgParser::ToBoolean(option_arg, true, );
   if (!success)
-error.SetErrorStringWithFormat("invalid boolean value for option '%c'",
-   short_option);
+error.SetErrorStringWithFormat(
+"invalid boolean value for option '%c': %s", short_option,
+option_arg);
   else {
 m_step_in_avoid_no_debug = avoid_no_debug ? eLazyBoolYes : eLazyBoolNo;
   }
@@ -284,8 +291,9 @@ class ThreadStepScopeOptionGroup : public OptionGroup {
   bool avoid_no_debug =
   OptionArgParser::ToBoolean(option_arg, true, );
   if (!success)
-error.SetErrorStringWithFormat("invalid boolean value for option '%c'",
-   short_option);
+error.SetErrorStringWithFormat(
+"invalid boolean value for option '%c': %s", short_option,
+option_arg);
   else {
 m_step_out_avoid_no_debug = avoid_no_debug ? eLazyBoolYes : 
eLazyBoolNo;
   }
@@ -293,8 +301,9 @@ class ThreadStepScopeOptionGroup : public OptionGroup {
 
 case 'c':
   if (option_arg.getAsInteger(0, m_step_count))
-

[Lldb-commits] [lldb] [lldb] Fix `thread backtrace --count` (PR #83602)

2024-03-01 Thread Jonas Devlieghere via lldb-commits 

https://github.com/JDevlieghere created 
https://github.com/llvm/llvm-project/pull/83602

The help output for `thread backtrace` specifies that you can pass -1 to 
`--count` to display all the frames.

```
-c  ( --count  )
How many frames to display (-1 for all)
```

However, that doesn't work:

```
(lldb) thread backtrace --count -1
error: invalid integer value for option 'c'
```

The problem is that we store the option value as an unsigned and the code to 
parse the string correctly rejects it. There's two ways to fix this:

 1. Make `m_count` a signed value so that it accepts negative values and 
appease the parser. The function that prints the frames takes an unsigned so a 
negative value will just become a really large positive value, which is what 
the current implementation relies on.
 2. Keep `m_count` unsigned and instead use 0 the magic value to show all 
frames. I don't really see a point in not showing any frames at all, plus 
that's already broken (`error: error displaying backtrace for thread: 
"0x0001"`).

This patch implements (2) and at the same time improve the error reporting so 
that we print the invalid value when we cannot parse it.

rdar://123881767

>From 223a8373dd5a5bf5c96cf3311ece39c97dd7b615 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere 
Date: Fri, 1 Mar 2024 10:02:25 -0800
Subject: [PATCH] [lldb] Fix `thread backtrace --count`

The help output for `thread backtrace` specifies that you can pass -1 to
`--count` to display all the frames.

```
-c  ( --count  )
How many frames to display (-1 for all)
```

However, that doesn't work:

```
(lldb) thread backtrace --count -1
error: invalid integer value for option 'c'
```

The problem is that we store the option value as an unsigned and the
code to parse the string correctly rejects it. There's two ways to fix
this:

 1. Make `m_count` a signed value so that it accepts negative values and
appease the parser. The function that prints the frames takes an
unsigned so a negative value will just become a really large
positive value, which is what the current implementation relies on.
 2. Keep `m_count` unsigned and instead use 0 the magic value to show
all frames. I don't really see a point in not showing any frames at
all, plus that's already broken (`error: error displaying backtrace
for thread: "0x0001"`).

This patch implements (2) and at the same time improve the error
reporting so that we print the invalid value when we cannot parse it.

rdar://123881767
---
 lldb/source/Commands/CommandObjectThread.cpp  | 33 ---
 lldb/source/Commands/Options.td   |  6 ++--
 .../Commands/command-thread-backtrace.test| 14 
 3 files changed, 38 insertions(+), 15 deletions(-)
 create mode 100644 lldb/test/Shell/Commands/command-thread-backtrace.test

diff --git a/lldb/source/Commands/CommandObjectThread.cpp 
b/lldb/source/Commands/CommandObjectThread.cpp
index 9cfff059d6bfa4..6d84315a471d95 100644
--- a/lldb/source/Commands/CommandObjectThread.cpp
+++ b/lldb/source/Commands/CommandObjectThread.cpp
@@ -67,13 +67,18 @@ class CommandObjectThreadBacktrace : public 
CommandObjectIterateOverThreads {
 if (option_arg.getAsInteger(0, m_count)) {
   m_count = UINT32_MAX;
   error.SetErrorStringWithFormat(
-  "invalid integer value for option '%c'", short_option);
+  "invalid integer value for option '%c': %s", short_option,
+  option_arg.data());
 }
+// A count of 0 means all frames.
+if (m_count == 0)
+  m_count = UINT32_MAX;
 break;
   case 's':
 if (option_arg.getAsInteger(0, m_start))
   error.SetErrorStringWithFormat(
-  "invalid integer value for option '%c'", short_option);
+  "invalid integer value for option '%c': %s", short_option,
+  option_arg.data());
 break;
   case 'e': {
 bool success;
@@ -81,7 +86,8 @@ class CommandObjectThreadBacktrace : public 
CommandObjectIterateOverThreads {
 OptionArgParser::ToBoolean(option_arg, false, );
 if (!success)
   error.SetErrorStringWithFormat(
-  "invalid boolean value for option '%c'", short_option);
+  "invalid boolean value for option '%c': %s", short_option,
+  option_arg.data());
   } break;
   default:
 llvm_unreachable("Unimplemented option");
@@ -228,9 +234,9 @@ class CommandObjectThreadBacktrace : public 
CommandObjectIterateOverThreads {
   thread->GetIndexID());
   return false;
 }
-if (m_options.m_extended_backtrace) { 
-  if (!INTERRUPT_REQUESTED(GetDebugger(), 
-  "Interrupt skipped extended backtrace")) {
+if (m_options.m_extended_backtrace) {
+  if (!INTERRUPT_REQUESTED(GetDebugger(),
+   "Interrupt skipped extended backtrace")) {
 DoExtendedBacktrace(thread,

[Lldb-commits] [lldb] [lldb] Add support for sorting by size to `target module dump symtab` (PR #83527)

2024-03-01 Thread Jonas Devlieghere via lldb-commits 

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

[Lldb-commits] [lldb] 8fa3301 - [lldb] Add support for sorting by size to `target module dump symtab` (#83527)

2024-03-01 Thread via lldb-commits 

Author: Jonas Devlieghere
Date: 2024-03-01T08:34:36-08:00
New Revision: 8fa33013de5d2c47da93083642cf9f655a3d9722

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

LOG: [lldb] Add support for sorting by size to `target module dump symtab` 
(#83527)

This patch adds support to sort the symbol table by size. The command
already supports sorting and it already reports sizes. Sorting by size
helps diagnosing size issues.

rdar://123788375

Added: 
lldb/test/Shell/SymbolFile/Breakpad/symtab-sorted-by-size.test

Modified: 
lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h
lldb/include/lldb/lldb-private-enumerations.h
lldb/source/Symbol/Symtab.cpp

Removed: 




diff  --git a/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h 
b/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h
index 9248e2ac814461..b5e989633ea3fc 100644
--- a/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h
+++ b/lldb/include/lldb/Interpreter/CommandOptionArgumentTable.h
@@ -50,6 +50,11 @@ static constexpr OptionEnumValueElement 
g_sort_option_enumeration[] = {
 "name",
 "Sort output by symbol name.",
 },
+{
+eSortOrderBySize,
+"size",
+"Sort output by symbol byte size.",
+},
 };
 
 // Note that the negation in the argument name causes a slightly confusing

diff  --git a/lldb/include/lldb/lldb-private-enumerations.h 
b/lldb/include/lldb/lldb-private-enumerations.h
index 9e8ab56305bef3..b8f504529683ad 100644
--- a/lldb/include/lldb/lldb-private-enumerations.h
+++ b/lldb/include/lldb/lldb-private-enumerations.h
@@ -108,7 +108,12 @@ enum ArgumentRepetitionType {
   // optional
 };
 
-enum SortOrder { eSortOrderNone, eSortOrderByAddress, eSortOrderByName };
+enum SortOrder {
+  eSortOrderNone,
+  eSortOrderByAddress,
+  eSortOrderByName,
+  eSortOrderBySize
+};
 
 // LazyBool is for boolean values that need to be calculated lazily. Values
 // start off set to eLazyBoolCalculate, and then they can be calculated once

diff  --git a/lldb/source/Symbol/Symtab.cpp b/lldb/source/Symbol/Symtab.cpp
index 564a3a94cfa202..b7837892d7e26d 100644
--- a/lldb/source/Symbol/Symtab.cpp
+++ b/lldb/source/Symbol/Symtab.cpp
@@ -124,12 +124,8 @@ void Symtab::Dump(Stream *s, Target *target, SortOrder 
sort_order,
   DumpSymbolHeader(s);
 
   std::multimap name_map;
-  for (const_iterator pos = m_symbols.begin(), end = m_symbols.end();
-   pos != end; ++pos) {
-const char *name = pos->GetName().AsCString();
-if (name && name[0])
-  name_map.insert(std::make_pair(name, &(*pos)));
-  }
+  for (const Symbol  : m_symbols)
+name_map.emplace(llvm::StringRef(symbol.GetName()), );
 
   for (const auto _to_symbol : name_map) {
 const Symbol *symbol = name_to_symbol.second;
@@ -138,6 +134,21 @@ void Symtab::Dump(Stream *s, Target *target, SortOrder 
sort_order,
   }
 } break;
 
+case eSortOrderBySize: {
+  s->PutCString(" (sorted by size):\n");
+  DumpSymbolHeader(s);
+
+  std::multimap> size_map;
+  for (const Symbol  : m_symbols)
+size_map.emplace(symbol.GetByteSize(), );
+
+  for (const auto _to_symbol : size_map) {
+const Symbol *symbol = size_to_symbol.second;
+s->Indent();
+symbol->Dump(s, target, symbol - _symbols[0], name_preference);
+  }
+} break;
+
 case eSortOrderByAddress:
   s->PutCString(" (sorted by address):\n");
   DumpSymbolHeader(s);

diff  --git a/lldb/test/Shell/SymbolFile/Breakpad/symtab-sorted-by-size.test 
b/lldb/test/Shell/SymbolFile/Breakpad/symtab-sorted-by-size.test
new file mode 100644
index 00..a9b6c0b1ef09b0
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/Breakpad/symtab-sorted-by-size.test
@@ -0,0 +1,11 @@
+# RUN: yaml2obj %S/Inputs/basic-elf.yaml -o %T/symtab.out
+# RUN: %lldb %T/symtab.out -o "target symbols add -s symtab.out 
%S/Inputs/symtab.syms" \
+# RUN:   -s %s | FileCheck %s
+
+# CHECK: num_symbols = 4 (sorted by size):
+# CHECK: [0]  0  SX Code0x0040 
   0x00b0 0x ___lldb_unnamed_symbol0
+# CHECK: [3]  0   X Code0x004000d0 
   0x0022 0x _start
+# CHECK: [1]  0   X Code0x004000b0 
   0x0010 0x f1
+# CHECK: [2]  0   X Code0x004000c0 
   0x0010 0x f2
+
+image dump symtab -s size symtab.out



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

[Lldb-commits] [lldb] [lldb][test] Add test for chained PCH debugging (PR #83582)

2024-03-01 Thread Michael Buch via lldb-commits 


@@ -0,0 +1,73 @@
+"""
+Tests that we correctly track AST layout info
+(specifically alignment) when moving AST nodes
+between several ClangASTImporter instances
+(in this case, from a pch chain to executable
+to expression AST).
+"""
+
+import lldb
+import os
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestPchChain(TestBase):
+@add_test_categories(["gmodules"])
+@expectedFailureAll("Chained pch debugging currently not fully supported")
+def test_expr(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+lldbutil.run_break_set_by_file_and_line(
+self, "main.cpp", 9, num_expected_locations=1
+)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect(
+"frame variable data",
+substrs=["row = 1", "col = 2", "row = 3", "col = 4", "stride = 5"],
+)
+
+@add_test_categories(["gmodules"])
+@expectedFailureAll("Chained pch debugging currently not fully supported")
+def test_frame_var(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+lldbutil.run_break_set_by_file_and_line(
+self, "main.cpp", 9, num_expected_locations=1
+)

Michael137 wrote:

For some reason regex breakpoint didn't resolve...probably related to the 
Makefile shenanigans

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

[Lldb-commits] [lldb] [lldb][test] Add test for chained PCH debugging (PR #83582)

2024-03-01 Thread via lldb-commits 

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)


Changes

Adds a test-case for debugging a program with a
pch chain, that is, the main executable depends
on a pch that itself included another pch.

Currently clang doesn't emit the sekeleton CUs
required for LLDB to track all types on the pch chain. Thus this test is 
XFAILed for now.

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


5 Files Affected:

- (added) lldb/test/API/lang/cpp/gmodules/pch-chain/Makefile (+10) 
- (added) lldb/test/API/lang/cpp/gmodules/pch-chain/TestPchChain.py (+73) 
- (added) lldb/test/API/lang/cpp/gmodules/pch-chain/base-pch.h (+9) 
- (added) lldb/test/API/lang/cpp/gmodules/pch-chain/main.cpp (+10) 
- (added) lldb/test/API/lang/cpp/gmodules/pch-chain/pch.h (+16) 


``diff
diff --git a/lldb/test/API/lang/cpp/gmodules/pch-chain/Makefile 
b/lldb/test/API/lang/cpp/gmodules/pch-chain/Makefile
new file mode 100644
index 00..6477c25dedf7e1
--- /dev/null
+++ b/lldb/test/API/lang/cpp/gmodules/pch-chain/Makefile
@@ -0,0 +1,10 @@
+include Makefile.rules
+
+OBJECTS += main.o
+
+$(EXE): $(BUILDDIR)/main.o
+
+$(BUILDDIR)/main.o: main.cpp
+   $(CC) -cc1 -emit-pch -x c++-header -fmodule-format=obj -fmodules -O0 
-dwarf-ext-refs -debug-info-kind=standalone $(SRCDIR)/base-pch.h -o 
base-pch.h.gch
+   $(CC) -cc1 -emit-pch -x c++-header -fmodule-format=obj -fmodules -O0 
-dwarf-ext-refs -debug-info-kind=standalone -include-pch base-pch.h.gch 
$(SRCDIR)/pch.h -o pch.h.gch
+   $(CC) -cc1 -emit-obj -x c++ -fmodules -O0 -dwarf-ext-refs 
-debug-info-kind=standalone -include-pch pch.h.gch $(SRCDIR)/main.cpp -o 
$(BUILDDIR)/main.o
diff --git a/lldb/test/API/lang/cpp/gmodules/pch-chain/TestPchChain.py 
b/lldb/test/API/lang/cpp/gmodules/pch-chain/TestPchChain.py
new file mode 100644
index 00..b08c6caa713be0
--- /dev/null
+++ b/lldb/test/API/lang/cpp/gmodules/pch-chain/TestPchChain.py
@@ -0,0 +1,73 @@
+"""
+Tests that we correctly track AST layout info
+(specifically alignment) when moving AST nodes
+between several ClangASTImporter instances
+(in this case, from a pch chain to executable
+to expression AST).
+"""
+
+import lldb
+import os
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestPchChain(TestBase):
+@add_test_categories(["gmodules"])
+@expectedFailureAll("Chained pch debugging currently not fully supported")
+def test_expr(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+lldbutil.run_break_set_by_file_and_line(
+self, "main.cpp", 9, num_expected_locations=1
+)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect(
+"frame variable data",
+substrs=["row = 1", "col = 2", "row = 3", "col = 4", "stride = 5"],
+)
+
+@add_test_categories(["gmodules"])
+@expectedFailureAll("Chained pch debugging currently not fully supported")
+def test_frame_var(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+lldbutil.run_break_set_by_file_and_line(
+self, "main.cpp", 9, num_expected_locations=1
+)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect_expr(
+"data",
+result_type="MatrixData",
+result_children=[
+ValueCheck(
+name="section",
+children=[
+ValueCheck(
+name="origin",
+children=[
+ValueCheck(name="row", value="1"),
+ValueCheck(name="col", value="2"),
+],
+),
+ValueCheck(
+name="size",
+children=[
+ValueCheck(name="row", value="3"),
+ValueCheck(name="col", value="4"),
+],
+),
+],
+),
+ValueCheck(name="stride", value="5"),
+],
+)
diff --git a/lldb/test/API/lang/cpp/gmodules/pch-chain/base-pch.h 
b/lldb/test/API/lang/cpp/gmodules/pch-chain/base-pch.h
new file mode 100644
index 00..53dacd796f2c91
--- /dev/null
+++ b/lldb/test/API/lang/cpp/gmodules/pch-chain/base-pch.h
@@ -0,0 +1,9 @@
+#ifndef BASE_PCH_H_IN
+#define BASE_PCH_H_IN
+
+struct [[gnu::aligned(128)]] RowCol {
+  unsigned row;
+  unsigned col;
+};
+
+#endif // _H_IN
diff --git a/lldb/test/API/lang/cpp/gmodules/pch-chain/main.cpp

[Lldb-commits] [lldb] [lldb][test] Add test for chained PCH debugging (PR #83582)

2024-03-01 Thread Michael Buch via lldb-commits 


@@ -0,0 +1,10 @@
+include Makefile.rules
+
+OBJECTS += main.o
+
+$(EXE): $(BUILDDIR)/main.o
+
+$(BUILDDIR)/main.o: main.cpp
+   $(CC) -cc1 -emit-pch -x c++-header -fmodule-format=obj -fmodules -O0 
-dwarf-ext-refs -debug-info-kind=standalone $(SRCDIR)/base-pch.h -o 
base-pch.h.gch
+   $(CC) -cc1 -emit-pch -x c++-header -fmodule-format=obj -fmodules -O0 
-dwarf-ext-refs -debug-info-kind=standalone -include-pch base-pch.h.gch 
$(SRCDIR)/pch.h -o pch.h.gch
+   $(CC) -cc1 -emit-obj -x c++ -fmodules -O0 -dwarf-ext-refs 
-debug-info-kind=standalone -include-pch pch.h.gch $(SRCDIR)/main.cpp -o 
$(BUILDDIR)/main.o

Michael137 wrote:

I would love to find a better way of doing this

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

[Lldb-commits] [lldb] [lldb][test] Add test for chained PCH debugging (PR #83582)

2024-03-01 Thread Michael Buch via lldb-commits 

https://github.com/Michael137 created 
https://github.com/llvm/llvm-project/pull/83582

Adds a test-case for debugging a program with a
pch chain, that is, the main executable depends
on a pch that itself included another pch.

Currently clang doesn't emit the sekeleton CUs
required for LLDB to track all types on the pch chain. Thus this test is 
XFAILed for now.

>From 04114ab99a4689d038535c072a9902a6bce5b955 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 1 Mar 2024 15:14:48 +
Subject: [PATCH] [lldb][test] Add test for chained PCH debugging

Adds a test-case for debugging a program with a
pch chain, that is, the main executable depends
on a pch that itself included another pch.

Currently clang doesn't emit the sekeleton CUs
required for LLDB to track all types on the pch chain.
Thus this test is XFAILed for now.
---
 .../API/lang/cpp/gmodules/pch-chain/Makefile  | 10 +++
 .../cpp/gmodules/pch-chain/TestPchChain.py| 73 +++
 .../lang/cpp/gmodules/pch-chain/base-pch.h|  9 +++
 .../API/lang/cpp/gmodules/pch-chain/main.cpp  | 10 +++
 .../API/lang/cpp/gmodules/pch-chain/pch.h | 16 
 5 files changed, 118 insertions(+)
 create mode 100644 lldb/test/API/lang/cpp/gmodules/pch-chain/Makefile
 create mode 100644 lldb/test/API/lang/cpp/gmodules/pch-chain/TestPchChain.py
 create mode 100644 lldb/test/API/lang/cpp/gmodules/pch-chain/base-pch.h
 create mode 100644 lldb/test/API/lang/cpp/gmodules/pch-chain/main.cpp
 create mode 100644 lldb/test/API/lang/cpp/gmodules/pch-chain/pch.h

diff --git a/lldb/test/API/lang/cpp/gmodules/pch-chain/Makefile 
b/lldb/test/API/lang/cpp/gmodules/pch-chain/Makefile
new file mode 100644
index 00..6477c25dedf7e1
--- /dev/null
+++ b/lldb/test/API/lang/cpp/gmodules/pch-chain/Makefile
@@ -0,0 +1,10 @@
+include Makefile.rules
+
+OBJECTS += main.o
+
+$(EXE): $(BUILDDIR)/main.o
+
+$(BUILDDIR)/main.o: main.cpp
+   $(CC) -cc1 -emit-pch -x c++-header -fmodule-format=obj -fmodules -O0 
-dwarf-ext-refs -debug-info-kind=standalone $(SRCDIR)/base-pch.h -o 
base-pch.h.gch
+   $(CC) -cc1 -emit-pch -x c++-header -fmodule-format=obj -fmodules -O0 
-dwarf-ext-refs -debug-info-kind=standalone -include-pch base-pch.h.gch 
$(SRCDIR)/pch.h -o pch.h.gch
+   $(CC) -cc1 -emit-obj -x c++ -fmodules -O0 -dwarf-ext-refs 
-debug-info-kind=standalone -include-pch pch.h.gch $(SRCDIR)/main.cpp -o 
$(BUILDDIR)/main.o
diff --git a/lldb/test/API/lang/cpp/gmodules/pch-chain/TestPchChain.py 
b/lldb/test/API/lang/cpp/gmodules/pch-chain/TestPchChain.py
new file mode 100644
index 00..b08c6caa713be0
--- /dev/null
+++ b/lldb/test/API/lang/cpp/gmodules/pch-chain/TestPchChain.py
@@ -0,0 +1,73 @@
+"""
+Tests that we correctly track AST layout info
+(specifically alignment) when moving AST nodes
+between several ClangASTImporter instances
+(in this case, from a pch chain to executable
+to expression AST).
+"""
+
+import lldb
+import os
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestPchChain(TestBase):
+@add_test_categories(["gmodules"])
+@expectedFailureAll("Chained pch debugging currently not fully supported")
+def test_expr(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+lldbutil.run_break_set_by_file_and_line(
+self, "main.cpp", 9, num_expected_locations=1
+)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect(
+"frame variable data",
+substrs=["row = 1", "col = 2", "row = 3", "col = 4", "stride = 5"],
+)
+
+@add_test_categories(["gmodules"])
+@expectedFailureAll("Chained pch debugging currently not fully supported")
+def test_frame_var(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.target = self.dbg.CreateTarget(exe)
+self.assertTrue(self.target, VALID_TARGET)
+lldbutil.run_break_set_by_file_and_line(
+self, "main.cpp", 9, num_expected_locations=1
+)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+self.expect_expr(
+"data",
+result_type="MatrixData",
+result_children=[
+ValueCheck(
+name="section",
+children=[
+ValueCheck(
+name="origin",
+children=[
+ValueCheck(name="row", value="1"),
+ValueCheck(name="col", value="2"),
+],
+),
+ValueCheck(
+name="size",
+children=[
+ValueCheck(name="row", value="3"),
+ValueCheck(name="col",

[Lldb-commits] [lldb] [lldb][test][NFC] Add option to exclude third_party packages (PR #83191)

2024-03-01 Thread David Spickett via lldb-commits 

DavidSpickett wrote:

Done: 
https://github.com/llvm/llvm-project/commit/ec8df555702d85511290742388d28016b69468de
 / 
https://github.com/llvm/llvm-zorg/commit/e08c692f75a922ee820ac8722f5033255913c195

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

[Lldb-commits] [lldb] ec8df55 - [lldb][test][Windows] Don't check for pexpect with LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS

2024-03-01 Thread David Spickett via lldb-commits 

Author: David Spickett
Date: 2024-03-01T09:47:17Z
New Revision: ec8df555702d85511290742388d28016b69468de

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

LOG: [lldb][test][Windows] Don't check for pexpect with 
LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS

See https://github.com/llvm/llvm-project/issues/22648 for why we don't use it on
Windows. Any pexpect tests are skipped there.

Added: 


Modified: 
lldb/test/CMakeLists.txt

Removed: 




diff  --git a/lldb/test/CMakeLists.txt b/lldb/test/CMakeLists.txt
index 2a9877c721e3b4..950643a5b8cc8e 100644
--- a/lldb/test/CMakeLists.txt
+++ b/lldb/test/CMakeLists.txt
@@ -11,10 +11,14 @@ endif()
 
 if(LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS)
   message(STATUS "Enforcing strict test requirements for LLDB")
-  set(useful_python_modules
-psutil  # Lit uses psutil to do per-test timeouts.
-pexpect # We no longer vendor pexpect.
-  )
+  # Lit uses psutil to do per-test timeouts.
+  set(useful_python_modules psutil)
+
+  if(NOT WIN32)
+# We no longer vendor pexpect and it is not used on Windows.
+list(APPEND pexpect)
+  endif()
+
   foreach(module ${useful_python_modules})
 lldb_find_python_module(${module})
 if (NOT PY_${module}_FOUND)



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