[Lldb-commits] [lldb] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
https://github.com/joker-eph updated https://github.com/llvm/llvm-project/pull/82094 >From f9f2e9380b1333b3b2503aebb6ee234b84b8c035 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 - 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 +- 15 files changed, 174 insertions(+), 114 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 &getThreadPool(); +ThreadPoolInterface &getThreadPool(); /// 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 &GetThreadPool(); + /// Shared thread pool. Use only with ThreadPoolTaskGroup. + static llvm::ThreadPoolInterface &GetThreadPool(); /// 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 &Log; - ThreadPool &Pool; + ThreadPoolInterface &Pool; Timer UpdateTimer; sys::Mutex UpdateMutex; @@ -145,7 +145,7 @@ class DebuginfodCollection { public: DebuginfodCollection(ArrayRef Paths, DebuginfodLog &Log, - ThreadPool &Pool, double MinInterval); + ThreadPoolInterface &Pool, 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 &TheThreadPool; +ThreadPoolInterface &TheThreadPool; std::mutex mtx; std::condition_variable cv; /// The number of threads tha
[Lldb-commits] [lldb] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
@@ -92,30 +104,32 @@ class ThreadPool { &Group); } - /// Blocking wait for all the threads to complete and the queue to be empty. - /// It is an error to try to add new tasks while blocking on this call. - /// Calling wait() from a task would deadlock waiting for itself. - void wait(); +private: + /// Asynchronous submission of a task to the pool. The returned future can be + /// used to wait for the task to finish and is *non-blocking* on destruction. + template + std::shared_future asyncImpl(std::function Task, + ThreadPoolTaskGroup *Group) { - /// Blocking wait for only all the threads in the given group to complete. - /// It is possible to wait even inside a task, but waiting (directly or - /// indirectly) on itself will deadlock. If called from a task running on a - /// worker thread, the call may process pending tasks while waiting in order - /// not to waste the thread. - void wait(ThreadPoolTaskGroup &Group); +#if LLVM_ENABLE_THREADS +/// Wrap the Task in a std::function that sets the result of the +/// corresponding future. +auto R = createTaskAndFuture(Task); - // Returns the maximum number of worker threads in the pool, not the current - // number of threads! - unsigned getMaxConcurrency() const { return MaxThreadCount; } +asyncEnqueue(std::move(R.first), Group); +return R.second.share(); - // TODO: misleading legacy name warning! - LLVM_DEPRECATED("Use getMaxConcurrency instead", "getMaxConcurrency") - unsigned getThreadCount() const { return MaxThreadCount; } +#else // LLVM_ENABLE_THREADS Disabled joker-eph wrote: I just split the implementations, but still using a static `using ThreadPool = ` declaration, how does this 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
@@ -92,30 +104,32 @@ class ThreadPool { &Group); } - /// Blocking wait for all the threads to complete and the queue to be empty. - /// It is an error to try to add new tasks while blocking on this call. - /// Calling wait() from a task would deadlock waiting for itself. - void wait(); +private: + /// Asynchronous submission of a task to the pool. The returned future can be + /// used to wait for the task to finish and is *non-blocking* on destruction. + template + std::shared_future asyncImpl(std::function Task, + ThreadPoolTaskGroup *Group) { - /// Blocking wait for only all the threads in the given group to complete. - /// It is possible to wait even inside a task, but waiting (directly or - /// indirectly) on itself will deadlock. If called from a task running on a - /// worker thread, the call may process pending tasks while waiting in order - /// not to waste the thread. - void wait(ThreadPoolTaskGroup &Group); +#if LLVM_ENABLE_THREADS +/// Wrap the Task in a std::function that sets the result of the +/// corresponding future. +auto R = createTaskAndFuture(Task); - // Returns the maximum number of worker threads in the pool, not the current - // number of threads! - unsigned getMaxConcurrency() const { return MaxThreadCount; } +asyncEnqueue(std::move(R.first), Group); +return R.second.share(); - // TODO: misleading legacy name warning! - LLVM_DEPRECATED("Use getMaxConcurrency instead", "getMaxConcurrency") - unsigned getThreadCount() const { return MaxThreadCount; } +#else // LLVM_ENABLE_THREADS Disabled joker-eph wrote: And here: for the followup renaming: https://github.com/llvm/llvm-project/commit/12073a1f3de2552a0b5f48930a755d5119fae9e6 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] a1bbba0 - [lldb][test][Windows] Fix NonStop tests on case insensitive file systems
Author: David Spickett Date: 2024-02-27T09:34:42Z New Revision: a1bbba06eabc5ddf533e611d15fddcfd0a8e79db URL: https://github.com/llvm/llvm-project/commit/a1bbba06eabc5ddf533e611d15fddcfd0a8e79db DIFF: https://github.com/llvm/llvm-project/commit/a1bbba06eabc5ddf533e611d15fddcfd0a8e79db.diff LOG: [lldb][test][Windows] Fix NonStop tests on case insensitive file systems On a case insensitive file sytem, the build dir for `test_multiple_c` and `test_multiple_C` are the same and therefore the log files are in the same place. This means one tries to clear the log file for the other. To fix this, make the names unique by adding the meaning of each protocol packet. https://sourceware.org/gdb/current/onlinedocs/gdb.html/Packets.html#Packets Added: Modified: lldb/test/API/tools/lldb-server/TestNonStop.py Removed: diff --git a/lldb/test/API/tools/lldb-server/TestNonStop.py b/lldb/test/API/tools/lldb-server/TestNonStop.py index b2d13614772217..62bda48ee049be 100644 --- a/lldb/test/API/tools/lldb-server/TestNonStop.py +++ b/lldb/test/API/tools/lldb-server/TestNonStop.py @@ -220,15 +220,15 @@ def multiple_resume_test(self, second_command): self.expect_gdbremote_sequence() @add_test_categories(["llgs"]) -def test_multiple_C(self): +def test_multiple_C_continue_with_signal(self): self.multiple_resume_test("C05") @add_test_categories(["llgs"]) -def test_multiple_c(self): +def test_multiple_c_continue_with_addr(self): self.multiple_resume_test("c") @add_test_categories(["llgs"]) -def test_multiple_s(self): +def test_multiple_s_single_step_with_addr(self): self.multiple_resume_test("s") @skipIfWindows ___ 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)
@@ -407,6 +407,129 @@ class LLDB_API SBProcess { /// the process isn't loaded from a core file. lldb::SBFileSpec GetCoreFile(); + /// Get the current address mask that can be applied to addresses + /// before reading from memory. + /// + /// \param[in] type + /// lldb may have different address masks for code and data + /// addresses. Either can be requested, or most commonly, + /// eAddressMaskTypeAny can be requested and the least specific + /// mask will be fetched. e.g. on a target where instructions + /// are word aligned, the Code mask might clear the low 2 bits. + /// + /// \param[in] addr_range + /// Specify whether the address mask for high or low address spaces + /// is requested. + /// It is highly unusual to have different address masks in high + /// or low memory, and by default the eAddressMaskRangeLow is the + /// only one used for both types of addresses, the default value for + /// this argument is the correct one. + /// + /// On some architectures like AArch64, it is possible to have + /// different page table setups for low and high memory, so different + /// numbers of bits relevant to addressing, and it is possible to have + /// a program running in one half of memory and accessing the other + /// as heap, etc. In that case the eAddressMaskRangeLow and + /// eAddressMaskRangeHigh will have different masks that must be handled. + /// + /// \return + /// The address mask currently in use. Bits which are not used + /// for addressing will be set to 1 in the mask. + lldb::addr_t GetAddressMask( + lldb::AddressMaskType type, + lldb::AddressMaskRange addr_range = lldb::eAddressMaskRangeLow); + + /// Set the current address mask that can be applied to addresses + /// before reading from memory. + /// + /// \param[in] type + /// lldb may have different address masks for code and data + /// addresses. Either can be set, or most commonly, + /// eAddressMaskTypeAll can be set for both types of addresses. + /// An example where they could be different is a target where + /// instructions are word aligned, so the low 2 bits are always + /// zero. + /// + /// \param[in] mask + /// The address mask to set. Bits which are not used for addressing + /// should be set to 1 in the mask. + /// + /// \param[in] addr_range + /// Specify whether the address mask for high or low address spaces + /// is being set. + /// It is highly unusual to have different address masks in high + /// or low memory, and by default the eAddressMaskRangeLow is the + /// only one used for both types of addresses, the default value for + /// this argument is the correct one. + /// + /// On some architectures like AArch64, it is possible to have + /// different page table setups for low and high memory, so different + /// numbers of bits relevant to addressing, and it is possible to have + /// a program running in one half of memory and accessing the other + /// as heap, etc. In that case the eAddressMaskRangeLow and + /// eAddressMaskRangeHigh will have different masks that must be + /// specified. + void SetAddressMask( + lldb::AddressMaskType type, lldb::addr_t mask, + lldb::AddressMaskRange addr_range = lldb::eAddressMaskRangeLow); + + /// Set the number of bits used for addressing in this Process. + /// + /// In some environments, the number of bits that are used for addressing + /// is the natural representation insted of a mask; this method calculates DavidSpickett wrote: "this method uses that number to calculate the addressing mask that lldb uses internally." So that "that number" unambiguously refers to the number of addressable bits. 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] [lldb] Add SBProcess methods for get/set/use address masks (PR #83095)
@@ -407,6 +407,129 @@ class LLDB_API SBProcess { /// the process isn't loaded from a core file. lldb::SBFileSpec GetCoreFile(); + /// Get the current address mask that can be applied to addresses + /// before reading from memory. + /// + /// \param[in] type + /// lldb may have different address masks for code and data + /// addresses. Either can be requested, or most commonly, + /// eAddressMaskTypeAny can be requested and the least specific + /// mask will be fetched. e.g. on a target where instructions + /// are word aligned, the Code mask might clear the low 2 bits. + /// + /// \param[in] addr_range + /// Specify whether the address mask for high or low address spaces + /// is requested. + /// It is highly unusual to have different address masks in high + /// or low memory, and by default the eAddressMaskRangeLow is the + /// only one used for both types of addresses, the default value for + /// this argument is the correct one. + /// + /// On some architectures like AArch64, it is possible to have + /// different page table setups for low and high memory, so different + /// numbers of bits relevant to addressing, and it is possible to have + /// a program running in one half of memory and accessing the other + /// as heap, etc. In that case the eAddressMaskRangeLow and + /// eAddressMaskRangeHigh will have different masks that must be handled. + /// + /// \return + /// The address mask currently in use. Bits which are not used + /// for addressing will be set to 1 in the mask. + lldb::addr_t GetAddressMask( + lldb::AddressMaskType type, + lldb::AddressMaskRange addr_range = lldb::eAddressMaskRangeLow); + + /// Set the current address mask that can be applied to addresses + /// before reading from memory. + /// + /// \param[in] type + /// lldb may have different address masks for code and data + /// addresses. Either can be set, or most commonly, + /// eAddressMaskTypeAll can be set for both types of addresses. + /// An example where they could be different is a target where + /// instructions are word aligned, so the low 2 bits are always + /// zero. + /// + /// \param[in] mask + /// The address mask to set. Bits which are not used for addressing + /// should be set to 1 in the mask. + /// + /// \param[in] addr_range + /// Specify whether the address mask for high or low address spaces + /// is being set. + /// It is highly unusual to have different address masks in high + /// or low memory, and by default the eAddressMaskRangeLow is the + /// only one used for both types of addresses, the default value for + /// this argument is the correct one. + /// + /// On some architectures like AArch64, it is possible to have + /// different page table setups for low and high memory, so different + /// numbers of bits relevant to addressing, and it is possible to have + /// a program running in one half of memory and accessing the other + /// as heap, etc. In that case the eAddressMaskRangeLow and + /// eAddressMaskRangeHigh will have different masks that must be + /// specified. + void SetAddressMask( + lldb::AddressMaskType type, lldb::addr_t mask, + lldb::AddressMaskRange addr_range = lldb::eAddressMaskRangeLow); + + /// Set the number of bits used for addressing in this Process. + /// + /// In some environments, the number of bits that are used for addressing + /// is the natural representation insted of a mask; this method calculates + /// the addressing mask that lldb uses internally from that number. + /// + /// \param[in] type + /// lldb may have different address masks for code and data + /// addresses. Either can be set, or most commonly, + /// eAddressMaskTypeAll can be set for both types of addresses. + /// An example where they could be different is a target where + /// instructions are word aligned, so the low 2 bits are always + /// zero. DavidSpickett wrote: "so the low 2 bits of the PC are always zero" Otherwise it's a bit of a "so what" statement. Mention the PC so it's more obvious that the code mask would be different in this case. 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] [lldb] Add SBProcess methods for get/set/use address masks (PR #83095)
@@ -407,6 +407,129 @@ class LLDB_API SBProcess { /// the process isn't loaded from a core file. lldb::SBFileSpec GetCoreFile(); + /// Get the current address mask that can be applied to addresses + /// before reading from memory. + /// + /// \param[in] type + /// lldb may have different address masks for code and data + /// addresses. Either can be requested, or most commonly, + /// eAddressMaskTypeAny can be requested and the least specific + /// mask will be fetched. e.g. on a target where instructions + /// are word aligned, the Code mask might clear the low 2 bits. + /// + /// \param[in] addr_range + /// Specify whether the address mask for high or low address spaces + /// is requested. + /// It is highly unusual to have different address masks in high + /// or low memory, and by default the eAddressMaskRangeLow is the + /// only one used for both types of addresses, the default value for + /// this argument is the correct one. + /// + /// On some architectures like AArch64, it is possible to have + /// different page table setups for low and high memory, so different + /// numbers of bits relevant to addressing, and it is possible to have + /// a program running in one half of memory and accessing the other + /// as heap, etc. In that case the eAddressMaskRangeLow and + /// eAddressMaskRangeHigh will have different masks that must be handled. + /// + /// \return + /// The address mask currently in use. Bits which are not used + /// for addressing will be set to 1 in the mask. + lldb::addr_t GetAddressMask( + lldb::AddressMaskType type, + lldb::AddressMaskRange addr_range = lldb::eAddressMaskRangeLow); + + /// Set the current address mask that can be applied to addresses + /// before reading from memory. + /// + /// \param[in] type + /// lldb may have different address masks for code and data + /// addresses. Either can be set, or most commonly, + /// eAddressMaskTypeAll can be set for both types of addresses. + /// An example where they could be different is a target where + /// instructions are word aligned, so the low 2 bits are always + /// zero. + /// + /// \param[in] mask + /// The address mask to set. Bits which are not used for addressing + /// should be set to 1 in the mask. + /// + /// \param[in] addr_range + /// Specify whether the address mask for high or low address spaces + /// is being set. + /// It is highly unusual to have different address masks in high + /// or low memory, and by default the eAddressMaskRangeLow is the + /// only one used for both types of addresses, the default value for + /// this argument is the correct one. + /// + /// On some architectures like AArch64, it is possible to have + /// different page table setups for low and high memory, so different + /// numbers of bits relevant to addressing, and it is possible to have + /// a program running in one half of memory and accessing the other + /// as heap, etc. In that case the eAddressMaskRangeLow and + /// eAddressMaskRangeHigh will have different masks that must be + /// specified. + void SetAddressMask( + lldb::AddressMaskType type, lldb::addr_t mask, + lldb::AddressMaskRange addr_range = lldb::eAddressMaskRangeLow); + + /// Set the number of bits used for addressing in this Process. + /// + /// In some environments, the number of bits that are used for addressing + /// is the natural representation insted of a mask; this method calculates + /// the addressing mask that lldb uses internally from that number. + /// + /// \param[in] type + /// lldb may have different address masks for code and data + /// addresses. Either can be set, or most commonly, + /// eAddressMaskTypeAll can be set for both types of addresses. + /// An example where they could be different is a target where + /// instructions are word aligned, so the low 2 bits are always + /// zero. + /// + /// \param[in] num_bits + /// Number of bits that are used for addressing. e.g. the low 42 DavidSpickett wrote: Maybe phrase this like: e.g. 42 means that the least significant 42 bits are used for The important thing here is that this is not total bits used for addressing, this is a range from bit 0 to some N. 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] [lldb] Add SBProcess methods for get/set/use address masks (PR #83095)
@@ -407,6 +407,129 @@ class LLDB_API SBProcess { /// the process isn't loaded from a core file. lldb::SBFileSpec GetCoreFile(); + /// Get the current address mask that can be applied to addresses + /// before reading from memory. + /// + /// \param[in] type + /// lldb may have different address masks for code and data + /// addresses. Either can be requested, or most commonly, + /// eAddressMaskTypeAny can be requested and the least specific + /// mask will be fetched. e.g. on a target where instructions + /// are word aligned, the Code mask might clear the low 2 bits. + /// + /// \param[in] addr_range + /// Specify whether the address mask for high or low address spaces + /// is requested. + /// It is highly unusual to have different address masks in high + /// or low memory, and by default the eAddressMaskRangeLow is the + /// only one used for both types of addresses, the default value for + /// this argument is the correct one. + /// + /// On some architectures like AArch64, it is possible to have + /// different page table setups for low and high memory, so different + /// numbers of bits relevant to addressing, and it is possible to have + /// a program running in one half of memory and accessing the other + /// as heap, etc. In that case the eAddressMaskRangeLow and + /// eAddressMaskRangeHigh will have different masks that must be handled. + /// + /// \return + /// The address mask currently in use. Bits which are not used + /// for addressing will be set to 1 in the mask. + lldb::addr_t GetAddressMask( + lldb::AddressMaskType type, + lldb::AddressMaskRange addr_range = lldb::eAddressMaskRangeLow); + + /// Set the current address mask that can be applied to addresses + /// before reading from memory. + /// + /// \param[in] type + /// lldb may have different address masks for code and data + /// addresses. Either can be set, or most commonly, + /// eAddressMaskTypeAll can be set for both types of addresses. + /// An example where they could be different is a target where + /// instructions are word aligned, so the low 2 bits are always + /// zero. + /// + /// \param[in] mask + /// The address mask to set. Bits which are not used for addressing + /// should be set to 1 in the mask. + /// + /// \param[in] addr_range + /// Specify whether the address mask for high or low address spaces + /// is being set. + /// It is highly unusual to have different address masks in high + /// or low memory, and by default the eAddressMaskRangeLow is the + /// only one used for both types of addresses, the default value for + /// this argument is the correct one. + /// + /// On some architectures like AArch64, it is possible to have + /// different page table setups for low and high memory, so different + /// numbers of bits relevant to addressing, and it is possible to have + /// a program running in one half of memory and accessing the other + /// as heap, etc. In that case the eAddressMaskRangeLow and + /// eAddressMaskRangeHigh will have different masks that must be + /// specified. + void SetAddressMask( + lldb::AddressMaskType type, lldb::addr_t mask, + lldb::AddressMaskRange addr_range = lldb::eAddressMaskRangeLow); + + /// Set the number of bits used for addressing in this Process. + /// + /// In some environments, the number of bits that are used for addressing + /// is the natural representation insted of a mask; this method calculates + /// the addressing mask that lldb uses internally from that number. + /// + /// \param[in] type + /// lldb may have different address masks for code and data + /// addresses. Either can be set, or most commonly, + /// eAddressMaskTypeAll can be set for both types of addresses. + /// An example where they could be different is a target where + /// instructions are word aligned, so the low 2 bits are always + /// zero. + /// + /// \param[in] num_bits + /// Number of bits that are used for addressing. e.g. the low 42 + /// bits may be the only ones used for addressing, and high bits may + /// store metadata and should be ignored by lldb. + /// + /// \param[in] addr_range + /// Specify whether the address mask for high or low address spaces + /// is being set. + /// It is highly unusual to have different address masks in high + /// or low memory, and by default the eAddressMaskRangeLow is the + /// only one used for both types of addresses, the default value for + /// this argument is the correct one. + /// + /// On some architectures like AArch64, it is possible to have + /// different page table setups for low and high memory, so different + /// numbers of bits relevant to addressing, and it is possible
[Lldb-commits] [lldb] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
@@ -227,7 +265,7 @@ class ThreadPool { class ThreadPoolTaskGroup { aganea wrote: Wouldn't the implemention for `ThreadPoolTaskGroup` come in hand with the one for `ThreadPool`? I feel if someone implements the `ThreadPoolInterface` they would want something similar for the groups? The use case is like I said: the new implementation would want to store something in the `ThreadPoolTaskGroup`. 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] Aim debugserver workaround more precisely. (PR #83099)
https://github.com/adrian-prantl closed https://github.com/llvm/llvm-project/pull/83099 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 8a87f76 - Aim debugserver workaround more precisely. (#83099)
Author: Adrian Prantl Date: 2024-02-27T08:14:46-08:00 New Revision: 8a87f763a6841832e71bcd24dea45eac8d2dbee1 URL: https://github.com/llvm/llvm-project/commit/8a87f763a6841832e71bcd24dea45eac8d2dbee1 DIFF: https://github.com/llvm/llvm-project/commit/8a87f763a6841832e71bcd24dea45eac8d2dbee1.diff LOG: Aim debugserver workaround more precisely. (#83099) Added: Modified: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp lldb/source/Target/Target.cpp Removed: diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp index 6f8aa262289946..1f6116967a4f64 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -17,6 +17,7 @@ #include "lldb/Core/ModuleSpec.h" #include "lldb/Host/HostInfo.h" +#include "lldb/Host/SafeMachO.h" #include "lldb/Host/XML.h" #include "lldb/Symbol/Symbol.h" #include "lldb/Target/MemoryRegionInfo.h" @@ -2147,8 +2148,20 @@ bool GDBRemoteCommunicationClient::GetCurrentProcessInfo(bool allow_lazy) { if (!value.getAsInteger(16, cpu)) ++num_keys_decoded; } else if (name.equals("cpusubtype")) { - if (!value.getAsInteger(16, sub)) + if (!value.getAsInteger(16, sub)) { ++num_keys_decoded; +// Workaround for pre-2024 Apple debugserver, which always +// returns arm64e on arm64e-capable hardware regardless of +// what the process is. This can be deleted at some point +// in the future. +if (cpu == llvm::MachO::CPU_TYPE_ARM64 && +sub == llvm::MachO::CPU_SUBTYPE_ARM64E) { + if (GetGDBServerVersion()) +if (m_gdb_server_version >= 1000 && +m_gdb_server_version <= 1504) + sub = 0; +} + } } else if (name.equals("triple")) { StringExtractor extractor(value); extractor.GetHexByteString(triple); diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp index 089915cab4915a..e982a30a3ae4ff 100644 --- a/lldb/source/Target/Target.cpp +++ b/lldb/source/Target/Target.cpp @@ -33,7 +33,6 @@ #include "lldb/Expression/UtilityFunction.h" #include "lldb/Host/Host.h" #include "lldb/Host/PosixApi.h" -#include "lldb/Host/SafeMachO.h" #include "lldb/Host/StreamFile.h" #include "lldb/Interpreter/CommandInterpreter.h" #include "lldb/Interpreter/CommandReturnObject.h" @@ -1572,13 +1571,6 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec, bool set_platform, if (m_arch.GetSpec().GetTriple() == other.GetTriple()) replace_local_arch = false; -// Workaround for for pre-2024 debugserver, which always -// returns arm64e on arm64e-capable hardware regardless of -// what the process is. This can be deleted at some point in -// the future. -if (!m_arch.GetSpec().GetMachOCPUSubType() && -other.GetMachOCPUSubType() == llvm::MachO::CPU_SUBTYPE_ARM64E) - replace_local_arch = true; } } } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
https://github.com/GeorgeARM edited 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
https://github.com/GeorgeARM commented: Sorry to pitch in; hope my comments help. Overall, looks like a nice starting drop-in interface injection on top of the what we have. Thanks @joker-eph 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
@@ -92,30 +104,20 @@ class ThreadPool { &Group); } - /// Blocking wait for all the threads to complete and the queue to be empty. - /// It is an error to try to add new tasks while blocking on this call. - /// Calling wait() from a task would deadlock waiting for itself. - void wait(); - - /// Blocking wait for only all the threads in the given group to complete. - /// It is possible to wait even inside a task, but waiting (directly or - /// indirectly) on itself will deadlock. If called from a task running on a - /// worker thread, the call may process pending tasks while waiting in order - /// not to waste the thread. - void wait(ThreadPoolTaskGroup &Group); - - // Returns the maximum number of worker threads in the pool, not the current - // number of threads! - unsigned getMaxConcurrency() const { return MaxThreadCount; } - - // TODO: misleading legacy name warning! - LLVM_DEPRECATED("Use getMaxConcurrency instead", "getMaxConcurrency") - unsigned getThreadCount() const { return MaxThreadCount; } +private: + /// Asynchronous submission of a task to the pool. The returned future can be + /// used to wait for the task to finish and is *non-blocking* on destruction. + template + std::shared_future asyncImpl(std::function Task, + ThreadPoolTaskGroup *Group) { GeorgeARM wrote: Is there a reason that this is part of the interface itself? Unless I am misreading something? If is for reusability across interface does it make sense to be a free function (if possible)? 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
@@ -227,7 +265,7 @@ class ThreadPool { class ThreadPoolTaskGroup { GeorgeARM wrote: Get the point of @aganea; the `ThreadPoolTaskGroup` is a bit tricky to reason when it comes to customized extensibility as the concrete implementation is injected to the interface itself. IMHO I think that this construct can sit on top of the `ThreadPoolInterface` itself (as it kind of does now) without semantically injecting itself in the pool. Someone can implement this through some API that returns a trackable construct; e.g. a `TaskID` or some kind of `Future` interface from the `asyncEnqueue`. Not sure if that's worth the effort though. 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
https://github.com/GeorgeARM edited 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] Start to clean up the process of defining command arguments. (PR #83097)
@@ -694,27 +712,32 @@ void CommandObject::GenerateHelpText(Stream &output_strm) { } } -void CommandObject::AddIDsArgumentData(CommandArgumentEntry &arg, - CommandArgumentType ID, - CommandArgumentType IDRange) { +void CommandObject::AddIDsArgumentData(CommandObject::IDType type) { + CommandArgumentEntry arg; CommandArgumentData id_arg; CommandArgumentData id_range_arg; // Create the first variant for the first (and only) argument for this // command. - id_arg.arg_type = ID; + switch (type) { +case eBreakpointArgs: + id_arg.arg_type = eArgTypeBreakpointID; + id_range_arg.arg_type = eArgTypeBreakpointIDRange; + break; +case eWatchpointArgs: + id_arg.arg_type = eArgTypeWatchpointID; + id_range_arg.arg_type = eArgTypeWatchpointIDRange; + break; + } jimingham wrote: This bit of code gets run once per SBDebugger startup per command that has breakpoint or watchpoint ID/IDRange argument types, about 10 times per SBDebugger::Create. I don't think we need to overthink this one. https://github.com/llvm/llvm-project/pull/83097 ___ 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)
https://github.com/hawkinsw commented: I really appreciate the thorough documentation you wrote for these new functions. Because there is so much overlap in the documentation between the functions, could we refactor it somehow (not sure how?) so that any future change could be more easily tracked? Just a question. Again, I appreciate your thorough documentation! Will 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] [lldb] Add SBProcess methods for get/set/use address masks (PR #83095)
@@ -407,6 +407,129 @@ class LLDB_API SBProcess { /// the process isn't loaded from a core file. lldb::SBFileSpec GetCoreFile(); + /// Get the current address mask that can be applied to addresses + /// before reading from memory. + /// + /// \param[in] type + /// lldb may have different address masks for code and data + /// addresses. Either can be requested, or most commonly, + /// eAddressMaskTypeAny can be requested and the least specific + /// mask will be fetched. e.g. on a target where instructions + /// are word aligned, the Code mask might clear the low 2 bits. hawkinsw wrote: ```suggestion /// are word aligned, the code mask might clear the low 2 bits. ``` 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] [lldb] Add SBProcess methods for get/set/use address masks (PR #83095)
https://github.com/hawkinsw edited 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] [lldb] Add SBProcess methods for get/set/use address masks (PR #83095)
@@ -407,6 +407,129 @@ class LLDB_API SBProcess { /// the process isn't loaded from a core file. lldb::SBFileSpec GetCoreFile(); + /// Get the current address mask that can be applied to addresses + /// before reading from memory. + /// + /// \param[in] type + /// lldb may have different address masks for code and data + /// addresses. Either can be requested, or most commonly, + /// eAddressMaskTypeAny can be requested and the least specific + /// mask will be fetched. e.g. on a target where instructions + /// are word aligned, the Code mask might clear the low 2 bits. + /// + /// \param[in] addr_range + /// Specify whether the address mask for high or low address spaces + /// is requested. + /// It is highly unusual to have different address masks in high + /// or low memory, and by default the eAddressMaskRangeLow is the + /// only one used for both types of addresses, the default value for + /// this argument is the correct one. + /// + /// On some architectures like AArch64, it is possible to have + /// different page table setups for low and high memory, so different + /// numbers of bits relevant to addressing, and it is possible to have + /// a program running in one half of memory and accessing the other + /// as heap, etc. In that case the eAddressMaskRangeLow and hawkinsw wrote: ```suggestion /// as heap, etc. In that case, the eAddressMaskRangeLow and ``` 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] [lldb][dap] Avoid concurrent `HandleCommand` calls (PR #83162)
https://github.com/rupprecht created https://github.com/llvm/llvm-project/pull/83162 The `EventThreadFunction` can end up calling `HandleCommand` concurrently with the main request processing thread. The underlying API does not appear to be thread safe, so add a narrowly scoped mutex lock to prevent calling it in this place from more than one thread. Fixes #81686. Prior to this, TestDAP_launch.py is 4% flaky. After, it passes in 1000 runs. >From 10104e6fcb0a279da202c02e242d69d92655128a Mon Sep 17 00:00:00 2001 From: Jordan Rupprecht Date: Tue, 27 Feb 2024 09:50:57 -0800 Subject: [PATCH] [lldb][dap] Avoid concurrent `HandleCommand` calls The `EventThreadFunction` can end up calling `HandleCommand` concurrently with the main request processing thread. The underlying API does not appear to be thread safe, so add a narrowly scoped mutex lock to prevent calling it in this place from more than one thread. Prior to this, TestDAP_launch.py is 4% flaky. After, it passes in 1000 runs. --- .../API/tools/lldb-dap/commands/TestDAP_commands.py | 2 -- lldb/tools/lldb-dap/LLDBUtils.cpp| 12 +++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py b/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py index 8dcda75d0a4520..226b9385fe719a 100644 --- a/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py +++ b/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py @@ -1,5 +1,4 @@ import os -import unittest import dap_server import lldbdap_testcase @@ -7,7 +6,6 @@ from lldbsuite.test.decorators import * -@unittest.skip("https://llvm.org/PR81686";) class TestDAP_commands(lldbdap_testcase.DAPTestCaseBase): def test_command_directive_quiet_on_success(self): program = self.getBuildArtifact("a.out") diff --git a/lldb/tools/lldb-dap/LLDBUtils.cpp b/lldb/tools/lldb-dap/LLDBUtils.cpp index 35b7a986a8964b..a91cc6718f4dfc 100644 --- a/lldb/tools/lldb-dap/LLDBUtils.cpp +++ b/lldb/tools/lldb-dap/LLDBUtils.cpp @@ -9,6 +9,8 @@ #include "LLDBUtils.h" #include "DAP.h" +#include + namespace lldb_dap { bool RunLLDBCommands(llvm::StringRef prefix, @@ -37,7 +39,15 @@ bool RunLLDBCommands(llvm::StringRef prefix, } } -interp.HandleCommand(command.str().c_str(), result); +{ + // Prevent simultaneous calls to HandleCommand, e.g. EventThreadFunction + // may asynchronously call RunExitCommands when we are already calling + // RunTerminateCommands. + static std::mutex handle_command_mutex; + std::lock_guard locker(handle_command_mutex); + interp.HandleCommand(command.str().c_str(), result); +} + const bool got_error = !result.Succeeded(); // The if statement below is assuming we always print out `!` prefixed // lines. The only time we don't print is when we have `quiet_on_success == ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][dap] Avoid concurrent `HandleCommand` calls (PR #83162)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Jordan Rupprecht (rupprecht) Changes The `EventThreadFunction` can end up calling `HandleCommand` concurrently with the main request processing thread. The underlying API does not appear to be thread safe, so add a narrowly scoped mutex lock to prevent calling it in this place from more than one thread. Fixes #81686. Prior to this, TestDAP_launch.py is 4% flaky. After, it passes in 1000 runs. --- Full diff: https://github.com/llvm/llvm-project/pull/83162.diff 2 Files Affected: - (modified) lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py (-2) - (modified) lldb/tools/lldb-dap/LLDBUtils.cpp (+11-1) ``diff diff --git a/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py b/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py index 8dcda75d0a4520..226b9385fe719a 100644 --- a/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py +++ b/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py @@ -1,5 +1,4 @@ import os -import unittest import dap_server import lldbdap_testcase @@ -7,7 +6,6 @@ from lldbsuite.test.decorators import * -@unittest.skip("https://llvm.org/PR81686";) class TestDAP_commands(lldbdap_testcase.DAPTestCaseBase): def test_command_directive_quiet_on_success(self): program = self.getBuildArtifact("a.out") diff --git a/lldb/tools/lldb-dap/LLDBUtils.cpp b/lldb/tools/lldb-dap/LLDBUtils.cpp index 35b7a986a8964b..a91cc6718f4dfc 100644 --- a/lldb/tools/lldb-dap/LLDBUtils.cpp +++ b/lldb/tools/lldb-dap/LLDBUtils.cpp @@ -9,6 +9,8 @@ #include "LLDBUtils.h" #include "DAP.h" +#include + namespace lldb_dap { bool RunLLDBCommands(llvm::StringRef prefix, @@ -37,7 +39,15 @@ bool RunLLDBCommands(llvm::StringRef prefix, } } -interp.HandleCommand(command.str().c_str(), result); +{ + // Prevent simultaneous calls to HandleCommand, e.g. EventThreadFunction + // may asynchronously call RunExitCommands when we are already calling + // RunTerminateCommands. + static std::mutex handle_command_mutex; + std::lock_guard locker(handle_command_mutex); + interp.HandleCommand(command.str().c_str(), result); +} + const bool got_error = !result.Succeeded(); // The if statement below is assuming we always print out `!` prefixed // lines. The only time we don't print is when we have `quiet_on_success == `` https://github.com/llvm/llvm-project/pull/83162 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][dap] Avoid concurrent `HandleCommand` calls (PR #83162)
https://github.com/walter-erquinigo approved this pull request. This is amazing! That finally explains some flakes I've seen https://github.com/llvm/llvm-project/pull/83162 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Start to clean up the process of defining command arguments. (PR #83097)
https://github.com/jimingham updated https://github.com/llvm/llvm-project/pull/83097 >From 2ed71038d9d16a09b3a04cf667dd272c911fef23 Mon Sep 17 00:00:00 2001 From: Jim Ingham Date: Mon, 26 Feb 2024 18:09:18 -0800 Subject: [PATCH 1/2] Start to clean up the process of defining command arguments. Partly, there's just a lot of unnecessary boiler plate. It's also possible to define combinations of arguments that make no sense (e.g. eArgRepeatPlus followed by eArgRepeatPlain...) but these are never checked since we just push_back directly into the argument definitions. This commit is step 1. In it, all the simple homogenous argument lists are set with a common function. This is an NFC change, it just centralizes boiler plate. There's no checking yet because you can't get a single argument wrong. --- lldb/include/lldb/Interpreter/CommandObject.h | 21 +++- lldb/source/Commands/CommandObjectApropos.cpp | 14 +-- .../Commands/CommandObjectBreakpoint.cpp | 72 ++- .../CommandObjectBreakpointCommand.cpp| 42 +-- .../source/Commands/CommandObjectCommands.cpp | 119 ++ .../Commands/CommandObjectDWIMPrint.cpp | 3 +- .../Commands/CommandObjectExpression.cpp | 14 +-- lldb/source/Commands/CommandObjectFrame.cpp | 59 + lldb/source/Commands/CommandObjectHelp.cpp| 13 +- lldb/source/Commands/CommandObjectLog.cpp | 56 + .../source/Commands/CommandObjectPlatform.cpp | 80 ++-- lldb/source/Commands/CommandObjectPlugin.cpp | 14 +-- lldb/source/Commands/CommandObjectProcess.cpp | 50 ++-- lldb/source/Commands/CommandObjectQuit.cpp| 3 +- .../source/Commands/CommandObjectRegister.cpp | 22 +--- lldb/source/Commands/CommandObjectSession.cpp | 4 +- .../source/Commands/CommandObjectSettings.cpp | 42 +-- lldb/source/Commands/CommandObjectTarget.cpp | 107 +++- lldb/source/Commands/CommandObjectThread.cpp | 90 ++--- .../Commands/CommandObjectThreadUtil.cpp | 6 +- lldb/source/Commands/CommandObjectTrace.cpp | 9 +- lldb/source/Commands/CommandObjectType.cpp| 113 ++--- .../Commands/CommandObjectWatchpoint.cpp | 68 ++ .../CommandObjectWatchpointCommand.cpp| 42 +-- lldb/source/Interpreter/CommandObject.cpp | 39 -- .../ItaniumABI/ItaniumABILanguageRuntime.cpp | 14 +-- .../AppleObjCRuntime/AppleObjCRuntimeV2.cpp | 28 + .../Process/gdb-remote/ProcessGDBRemote.cpp | 6 +- 28 files changed, 164 insertions(+), 986 deletions(-) diff --git a/lldb/include/lldb/Interpreter/CommandObject.h b/lldb/include/lldb/Interpreter/CommandObject.h index a326c6dc38a37a..49cf4a21e25d73 100644 --- a/lldb/include/lldb/Interpreter/CommandObject.h +++ b/lldb/include/lldb/Interpreter/CommandObject.h @@ -206,6 +206,22 @@ class CommandObject : public std::enable_shared_from_this { static const ArgumentTableEntry * FindArgumentDataByType(lldb::CommandArgumentType arg_type); + + // Sets the argument list for this command to one homogenous argument type, + // with the repeat specified. + void AddSimpleArgumentList(lldb::CommandArgumentType arg_type, + ArgumentRepetitionType repetition_type = eArgRepeatPlain); + + // Helper function to set BP IDs or ID ranges as the command argument data + // for this command. + // This used to just populate an entry you could add to, but that was never + // used. If we ever need that we can take optional extra args here. + // Use this to define a simple argument list: + enum IDType { +eBreakpointArgs = 0, +eWatchpointArgs = 1 + }; + void AddIDsArgumentData(IDType type); int GetNumArgumentEntries(); @@ -392,11 +408,6 @@ class CommandObject : public std::enable_shared_from_this { void *m_command_override_baton; bool m_is_user_command = false; - // Helper function to populate IDs or ID ranges as the command argument data - // to the specified command argument entry. - static void AddIDsArgumentData(CommandArgumentEntry &arg, - lldb::CommandArgumentType ID, - lldb::CommandArgumentType IDRange); }; class CommandObjectParsed : public CommandObject { diff --git a/lldb/source/Commands/CommandObjectApropos.cpp b/lldb/source/Commands/CommandObjectApropos.cpp index 88c214d4fc56ab..d663f2bd923fe2 100644 --- a/lldb/source/Commands/CommandObjectApropos.cpp +++ b/lldb/source/Commands/CommandObjectApropos.cpp @@ -21,19 +21,7 @@ CommandObjectApropos::CommandObjectApropos(CommandInterpreter &interpreter) : CommandObjectParsed( interpreter, "apropos", "List debugger commands related to a word or subject.", nullptr) { - CommandArgumentEntry arg; - CommandArgumentData search_word_arg; - - // Define the first (and only) variant of this arg. - search_word_arg.arg_type = eArgTypeSearchWord; - search_word_arg.arg_repetition = eArgRepeatPlain; - - // There is only one v
[Lldb-commits] [lldb] 0ef66fc - [lldb] Use CreateOptionParsingError in CommandObjectBreakpoint (#83086)
Author: Alex Langford Date: 2024-02-27T10:25:56-08:00 New Revision: 0ef66fcc858cc8abb978d83d48b3e7a8b23742c9 URL: https://github.com/llvm/llvm-project/commit/0ef66fcc858cc8abb978d83d48b3e7a8b23742c9 DIFF: https://github.com/llvm/llvm-project/commit/0ef66fcc858cc8abb978d83d48b3e7a8b23742c9.diff LOG: [lldb] Use CreateOptionParsingError in CommandObjectBreakpoint (#83086) This updates the remaining SetOptionValue methods in CommandObjectBreakpoint to use CreateOptionParsingError. I found a few minor bugs that were fixed during this refactor (e.g. using the wrong flag in an error message). That is one of the benefits of centralizing error message creation. I also found some option parsing code that is written incorrectly. I do not make an attempt to update those here because this PR is primarily about changing existing error handling code, not adding new error handling code. Added: Modified: lldb/include/lldb/Interpreter/Options.h lldb/source/Commands/CommandObjectBreakpoint.cpp Removed: diff --git a/lldb/include/lldb/Interpreter/Options.h b/lldb/include/lldb/Interpreter/Options.h index 18a87e49deee5c..9a6a17c2793fa4 100644 --- a/lldb/include/lldb/Interpreter/Options.h +++ b/lldb/include/lldb/Interpreter/Options.h @@ -368,6 +368,8 @@ static constexpr llvm::StringLiteral g_bool_parsing_error_message = "Failed to parse as boolean"; static constexpr llvm::StringLiteral g_int_parsing_error_message = "Failed to parse as integer"; +static constexpr llvm::StringLiteral g_language_parsing_error_message = +"Unknown language"; } // namespace lldb_private diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp b/lldb/source/Commands/CommandObjectBreakpoint.cpp index fc2217608a0bb9..cfb0b87a59e640 100644 --- a/lldb/source/Commands/CommandObjectBreakpoint.cpp +++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp @@ -266,6 +266,8 @@ class CommandObjectBreakpointSet : public CommandObjectParsed { Status error; const int short_option = g_breakpoint_set_options[option_idx].short_option; + const char *long_option = + g_breakpoint_set_options[option_idx].long_option; switch (short_option) { case 'a': { @@ -284,13 +286,15 @@ class CommandObjectBreakpointSet : public CommandObjectParsed { case 'u': if (option_arg.getAsInteger(0, m_column)) - error.SetErrorStringWithFormat("invalid column number: %s", - option_arg.str().c_str()); + error = + CreateOptionParsingError(option_arg, short_option, long_option, + g_int_parsing_error_message); break; case 'E': { LanguageType language = Language::GetLanguageTypeFromString(option_arg); +llvm::StringRef error_context; switch (language) { case eLanguageTypeC89: case eLanguageTypeC: @@ -308,19 +312,18 @@ class CommandObjectBreakpointSet : public CommandObjectParsed { m_exception_language = eLanguageTypeObjC; break; case eLanguageTypeObjC_plus_plus: - error.SetErrorStringWithFormat( - "Set exception breakpoints separately for c++ and objective-c"); + error_context = + "Set exception breakpoints separately for c++ and objective-c"; break; case eLanguageTypeUnknown: - error.SetErrorStringWithFormat( - "Unknown language type: '%s' for exception breakpoint", - option_arg.str().c_str()); + error_context = "Unknown language type for exception breakpoint"; break; default: - error.SetErrorStringWithFormat( - "Unsupported language type: '%s' for exception breakpoint", - option_arg.str().c_str()); + error_context = "Unsupported language type for exception breakpoint"; } +if (!error_context.empty()) + error = CreateOptionParsingError(option_arg, short_option, + long_option, error_context); } break; case 'f': @@ -336,9 +339,9 @@ class CommandObjectBreakpointSet : public CommandObjectParsed { bool success; m_catch_bp = OptionArgParser::ToBoolean(option_arg, true, &success); if (!success) - error.SetErrorStringWithFormat( - "Invalid boolean value for on-catch option: '%s'", - option_arg.str().c_str()); + error = + CreateOptionParsingError(option_arg, short_option, long_option, + g_bool_parsing_error_message); } break; case 'H': @@ -355,23 +358,24 @@ class CommandObjectBreakpointSet : public CommandObjectParsed { m_skip_prologue = eLazyBoolNo; if (!success) - error.SetEr
[Lldb-commits] [lldb] [lldb] Use CreateOptionParsingError in CommandObjectBreakpoint (PR #83086)
https://github.com/bulbazord closed https://github.com/llvm/llvm-project/pull/83086 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][dap] Avoid concurrent `HandleCommand` calls (PR #83162)
https://github.com/ZequanWu approved this pull request. https://github.com/llvm/llvm-project/pull/83162 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
https://github.com/chelcassanova updated https://github.com/llvm/llvm-project/pull/83069 >From d2854ecb51d5996cd5cabf4e1c1ac9dc6e01240b Mon Sep 17 00:00:00 2001 From: Chelsea Cassanova Date: Tue, 20 Feb 2024 13:56:53 -0800 Subject: [PATCH] [lldb][progress] Hook up new broadcast bit and progress manager 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. --- lldb/include/lldb/Core/Debugger.h | 10 ++-- lldb/include/lldb/Core/Progress.h | 43 +++- lldb/source/Core/Debugger.cpp | 25 +++--- lldb/source/Core/Progress.cpp | 42 lldb/unittests/Core/ProgressReportTest.cpp | 57 ++ 5 files changed, 146 insertions(+), 31 deletions(-) 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 eb4d9f9d7af08e..7554d4fe2b77e9 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 @@ -97,27 +98,44 @@ class Progress { /// Used to indicate a non-deterministic progress report static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX; + /// Use a struct to send data from a Progress object to + /// the progress manager. + struct ProgressData { +/// The title of the progress activity, also used as a category to group +/// reports. +std::string title; +/// More specific information about the current file being displayed in the +/// report. +std::string details; +/// A unique integer identifier for progress reporting. +uint64_t progress_id; +/// How much work ([0...m_total]) that has been completed. +uint64_t completed; +/// Total amount of work, use a std::nullopt in the constructor for non +/// deterministic progress. +uint64_t total; +/// 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: + friend class Debugger; void ReportProgress(); static std::atomic g_id; - /// The title of the progress activity. std::string m_title; 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; /// 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 deb
[Lldb-commits] [lldb] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
https://github.com/chelcassanova updated https://github.com/llvm/llvm-project/pull/83069 >From 2cef4a29f0105847fa11b7f6a6ee063184fb838a Mon Sep 17 00:00:00 2001 From: Chelsea Cassanova Date: Tue, 20 Feb 2024 13:56:53 -0800 Subject: [PATCH] [lldb][progress] Hook up new broadcast bit and progress manager 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. --- lldb/include/lldb/Core/Debugger.h | 10 ++-- lldb/include/lldb/Core/Progress.h | 43 +++- lldb/source/Core/Debugger.cpp | 25 +++--- lldb/source/Core/Progress.cpp | 40 --- lldb/unittests/Core/ProgressReportTest.cpp | 57 ++ 5 files changed, 145 insertions(+), 30 deletions(-) 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 eb4d9f9d7af08e..7554d4fe2b77e9 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 @@ -97,27 +98,44 @@ class Progress { /// Used to indicate a non-deterministic progress report static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX; + /// Use a struct to send data from a Progress object to + /// the progress manager. + struct ProgressData { +/// The title of the progress activity, also used as a category to group +/// reports. +std::string title; +/// More specific information about the current file being displayed in the +/// report. +std::string details; +/// A unique integer identifier for progress reporting. +uint64_t progress_id; +/// How much work ([0...m_total]) that has been completed. +uint64_t completed; +/// Total amount of work, use a std::nullopt in the constructor for non +/// deterministic progress. +uint64_t total; +/// 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: + friend class Debugger; void ReportProgress(); static std::atomic g_id; - /// The title of the progress activity. std::string m_title; 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; /// 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 debu
[Lldb-commits] [lldb] Start to clean up the process of defining command arguments. (PR #83097)
https://github.com/jimingham closed https://github.com/llvm/llvm-project/pull/83097 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 2d704f4 - Start to clean up the process of defining command arguments. (#83097)
Author: jimingham Date: 2024-02-27T10:34:01-08:00 New Revision: 2d704f4bf2edb0f9343dac818ab4d29442be9968 URL: https://github.com/llvm/llvm-project/commit/2d704f4bf2edb0f9343dac818ab4d29442be9968 DIFF: https://github.com/llvm/llvm-project/commit/2d704f4bf2edb0f9343dac818ab4d29442be9968.diff LOG: Start to clean up the process of defining command arguments. (#83097) Partly, there's just a lot of unnecessary boiler plate. It's also possible to define combinations of arguments that make no sense (e.g. eArgRepeatPlus followed by eArgRepeatPlain...) but these are never checked since we just push_back directly into the argument definitions. This commit is step 1 of this cleanup - do the obvious stuff. In it, all the simple homogenous argument lists and the breakpoint/watchpoint ID/Range types, are set with common functions. This is an NFC change, it just centralizes boiler plate. There's no checking yet because you can't get a single argument wrong. The end goal is that all argument definition goes through functions and m_arguments is hidden so that you can't define inconsistent argument sets. Added: Modified: lldb/include/lldb/Interpreter/CommandObject.h lldb/source/Commands/CommandObjectApropos.cpp lldb/source/Commands/CommandObjectBreakpoint.cpp lldb/source/Commands/CommandObjectBreakpointCommand.cpp lldb/source/Commands/CommandObjectCommands.cpp lldb/source/Commands/CommandObjectDWIMPrint.cpp lldb/source/Commands/CommandObjectExpression.cpp lldb/source/Commands/CommandObjectFrame.cpp lldb/source/Commands/CommandObjectHelp.cpp lldb/source/Commands/CommandObjectLog.cpp lldb/source/Commands/CommandObjectPlatform.cpp lldb/source/Commands/CommandObjectPlugin.cpp lldb/source/Commands/CommandObjectProcess.cpp lldb/source/Commands/CommandObjectQuit.cpp lldb/source/Commands/CommandObjectRegister.cpp lldb/source/Commands/CommandObjectSession.cpp lldb/source/Commands/CommandObjectSettings.cpp lldb/source/Commands/CommandObjectTarget.cpp lldb/source/Commands/CommandObjectThread.cpp lldb/source/Commands/CommandObjectThreadUtil.cpp lldb/source/Commands/CommandObjectTrace.cpp lldb/source/Commands/CommandObjectType.cpp lldb/source/Commands/CommandObjectWatchpoint.cpp lldb/source/Commands/CommandObjectWatchpointCommand.cpp lldb/source/Interpreter/CommandObject.cpp lldb/source/Plugins/LanguageRuntime/CPlusPlus/ItaniumABI/ItaniumABILanguageRuntime.cpp lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Removed: diff --git a/lldb/include/lldb/Interpreter/CommandObject.h b/lldb/include/lldb/Interpreter/CommandObject.h index a326c6dc38a37a..a641a468b49d20 100644 --- a/lldb/include/lldb/Interpreter/CommandObject.h +++ b/lldb/include/lldb/Interpreter/CommandObject.h @@ -207,6 +207,20 @@ class CommandObject : public std::enable_shared_from_this { static const ArgumentTableEntry * FindArgumentDataByType(lldb::CommandArgumentType arg_type); + // Sets the argument list for this command to one homogenous argument type, + // with the repeat specified. + void AddSimpleArgumentList( + lldb::CommandArgumentType arg_type, + ArgumentRepetitionType repetition_type = eArgRepeatPlain); + + // Helper function to set BP IDs or ID ranges as the command argument data + // for this command. + // This used to just populate an entry you could add to, but that was never + // used. If we ever need that we can take optional extra args here. + // Use this to define a simple argument list: + enum IDType { eBreakpointArgs = 0, eWatchpointArgs = 1 }; + void AddIDsArgumentData(IDType type); + int GetNumArgumentEntries(); CommandArgumentEntry *GetArgumentEntryAtIndex(int idx); @@ -391,12 +405,6 @@ class CommandObject : public std::enable_shared_from_this { lldb_private::CommandOverrideCallbackWithResult m_command_override_callback; void *m_command_override_baton; bool m_is_user_command = false; - - // Helper function to populate IDs or ID ranges as the command argument data - // to the specified command argument entry. - static void AddIDsArgumentData(CommandArgumentEntry &arg, - lldb::CommandArgumentType ID, - lldb::CommandArgumentType IDRange); }; class CommandObjectParsed : public CommandObject { diff --git a/lldb/source/Commands/CommandObjectApropos.cpp b/lldb/source/Commands/CommandObjectApropos.cpp index 88c214d4fc56ab..d663f2bd923fe2 100644 --- a/lldb/source/Commands/CommandObjectApropos.cpp +++ b/lldb/source/Commands/CommandObjectApropos.cpp @@ -21,19 +21,7 @@ CommandObjectApropos::CommandObjectApropos(CommandInterpreter &interpreter) : CommandObjectParsed( interpreter, "apropos", "List debugger comman
[Lldb-commits] [lldb] e427e93 - [lldb][dap] Avoid concurrent `HandleCommand` calls (#83162)
Author: Jordan Rupprecht Date: 2024-02-27T12:43:05-06:00 New Revision: e427e934f677567f8184ff900cb4cbdb8cf21a21 URL: https://github.com/llvm/llvm-project/commit/e427e934f677567f8184ff900cb4cbdb8cf21a21 DIFF: https://github.com/llvm/llvm-project/commit/e427e934f677567f8184ff900cb4cbdb8cf21a21.diff LOG: [lldb][dap] Avoid concurrent `HandleCommand` calls (#83162) The `EventThreadFunction` can end up calling `HandleCommand` concurrently with the main request processing thread. The underlying API does not appear to be thread safe, so add a narrowly scoped mutex lock to prevent calling it in this place from more than one thread. Fixes #81686. Prior to this, TestDAP_launch.py is 4% flaky. After, it passes in 1000 runs. Added: Modified: lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py lldb/tools/lldb-dap/LLDBUtils.cpp Removed: diff --git a/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py b/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py index 8dcda75d0a4520..226b9385fe719a 100644 --- a/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py +++ b/lldb/test/API/tools/lldb-dap/commands/TestDAP_commands.py @@ -1,5 +1,4 @@ import os -import unittest import dap_server import lldbdap_testcase @@ -7,7 +6,6 @@ from lldbsuite.test.decorators import * -@unittest.skip("https://llvm.org/PR81686";) class TestDAP_commands(lldbdap_testcase.DAPTestCaseBase): def test_command_directive_quiet_on_success(self): program = self.getBuildArtifact("a.out") diff --git a/lldb/tools/lldb-dap/LLDBUtils.cpp b/lldb/tools/lldb-dap/LLDBUtils.cpp index 35b7a986a8964b..a91cc6718f4dfc 100644 --- a/lldb/tools/lldb-dap/LLDBUtils.cpp +++ b/lldb/tools/lldb-dap/LLDBUtils.cpp @@ -9,6 +9,8 @@ #include "LLDBUtils.h" #include "DAP.h" +#include + namespace lldb_dap { bool RunLLDBCommands(llvm::StringRef prefix, @@ -37,7 +39,15 @@ bool RunLLDBCommands(llvm::StringRef prefix, } } -interp.HandleCommand(command.str().c_str(), result); +{ + // Prevent simultaneous calls to HandleCommand, e.g. EventThreadFunction + // may asynchronously call RunExitCommands when we are already calling + // RunTerminateCommands. + static std::mutex handle_command_mutex; + std::lock_guard locker(handle_command_mutex); + interp.HandleCommand(command.str().c_str(), result); +} + const bool got_error = !result.Succeeded(); // The if statement below is assuming we always print out `!` prefixed // lines. The only time we don't print is when we have `quiet_on_success == ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][dap] Avoid concurrent `HandleCommand` calls (PR #83162)
https://github.com/rupprecht closed https://github.com/llvm/llvm-project/pull/83162 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
@@ -227,7 +265,7 @@ class ThreadPool { class ThreadPoolTaskGroup { joker-eph wrote: > Wouldn't the implemention for ThreadPoolTaskGroup come in hand with the one > for ThreadPool? Right now the only thing the TaskGroup provide is a unique ID in the form of its own address. I don't quite get what I would change in the implementation? Would you want the group to customize its own `wait()`? Right now it dispatches to the threadpool implementation: `wait(ThreadPoolTaskGroup &Group)`. Customizing the task group would require to change the pattern at every use site as well: https://github.com/llvm/llvm-project/blob/0e0bee26e7f33c065eebef9a674b2f19bb156414/mlir/include/mlir/IR/Threading.h#L69-L70 Instead of: ``` llvm::ThreadPool &threadPool = context->getThreadPool(); llvm::ThreadPoolTaskGroup tasksGroup(threadPool); ``` we would need to use the pool as a factory somehow: ``` llvm::ThreadPool &threadPool = context->getThreadPool(); std::unique_ptr tasksGroup = threadPool.createTaskGroup(); ``` I'm not opposed to this, but that still seems like a separate change, and one that would better be done when there is somehow with the need for this. 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
https://github.com/joker-eph edited 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
@@ -92,30 +104,32 @@ class ThreadPool { &Group); } - /// Blocking wait for all the threads to complete and the queue to be empty. - /// It is an error to try to add new tasks while blocking on this call. - /// Calling wait() from a task would deadlock waiting for itself. - void wait(); +private: + /// Asynchronous submission of a task to the pool. The returned future can be + /// used to wait for the task to finish and is *non-blocking* on destruction. + template + std::shared_future asyncImpl(std::function Task, + ThreadPoolTaskGroup *Group) { - /// Blocking wait for only all the threads in the given group to complete. - /// It is possible to wait even inside a task, but waiting (directly or - /// indirectly) on itself will deadlock. If called from a task running on a - /// worker thread, the call may process pending tasks while waiting in order - /// not to waste the thread. - void wait(ThreadPoolTaskGroup &Group); +#if LLVM_ENABLE_THREADS +/// Wrap the Task in a std::function that sets the result of the +/// corresponding future. +auto R = createTaskAndFuture(Task); - // Returns the maximum number of worker threads in the pool, not the current - // number of threads! - unsigned getMaxConcurrency() const { return MaxThreadCount; } +asyncEnqueue(std::move(R.first), Group); +return R.second.share(); - // TODO: misleading legacy name warning! - LLVM_DEPRECATED("Use getMaxConcurrency instead", "getMaxConcurrency") - unsigned getThreadCount() const { return MaxThreadCount; } +#else // LLVM_ENABLE_THREADS Disabled joker-eph wrote: I can also squash the renaming here @dwblaikie if you feel it's better to not do this separately by the way 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][test] Exclude third_party packages by default (PR #83191)
https://github.com/rupprecht created https://github.com/llvm/llvm-project/pull/83191 The goal here is to remove the third_party/Python/module tree, which LLDB tests only use to `import pexpect`. This package is available on `pip`, and I believe should not be hard to obtain. However, in case it isn't easily available, deleting the tree right now could cause disruption. This introduces a `LLDB_TEST_USE_VENDOR_PACKAGES` cmake param that can be enabled, and the tests will continue loading that tree. By default, it is disabled, and an eager cmake check runs that makes sure `pexpect` is available before waiting for the test to fail in an obscure way. Later, this option will go away, and when it does, we can delete the tree too. Ideally this is not disruptive, and we can remove it in a week or two. >From d21ac756226a364603acd0180f9a06c23600acb1 Mon Sep 17 00:00:00 2001 From: Jordan Rupprecht Date: Tue, 27 Feb 2024 13:25:12 -0800 Subject: [PATCH] [lldb][test] Exclude third_party packages by default The goal here is to remove the third_party/Python/module tree, which LLDB tests only use to `import pexpect`. This package is available on `pip`, and I believe should not be hard to obtain. However, in case it isn't easily available, deleting the tree right now could cause disruption. This introduces a `LLDB_TEST_USE_VENDOR_PACKAGES` cmake param that can be enabled, and the tests will continue loading that tree. By default, it is disabled, and an eager cmake check runs that makes sure `pexpect` is available before waiting for the test to fail in an obscure way. Later, this option will go away, and when it does, we can delete the tree too. Ideally this is not disruptive, and we can remove it in a week or two. --- lldb/cmake/modules/LLDBConfig.cmake | 2 ++ lldb/test/API/lit.cfg.py | 3 +++ lldb/test/API/lit.site.cfg.py.in | 1 + lldb/test/CMakeLists.txt | 17 - lldb/use_lldb_suite_root.py | 4 +++- lldb/utils/lldb-dotest/CMakeLists.txt | 1 + lldb/utils/lldb-dotest/lldb-dotest.in | 5 + 7 files changed, 31 insertions(+), 2 deletions(-) diff --git a/lldb/cmake/modules/LLDBConfig.cmake b/lldb/cmake/modules/LLDBConfig.cmake index a758261073ac57..5d62213c3f5838 100644 --- a/lldb/cmake/modules/LLDBConfig.cmake +++ b/lldb/cmake/modules/LLDBConfig.cmake @@ -67,6 +67,8 @@ option(LLDB_SKIP_STRIP "Whether to skip stripping of binaries when installing ll option(LLDB_SKIP_DSYM "Whether to skip generating a dSYM when installing lldb." OFF) option(LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS "Fail to configure if certain requirements are not met for testing." OFF) +option(LLDB_TEST_USE_VENDOR_PACKAGES + "Use packages from lldb/third_party/Python/module instead of system deps." OFF) set(LLDB_GLOBAL_INIT_DIRECTORY "" CACHE STRING "Path to the global lldbinit directory. Relative paths are resolved relative to the diff --git a/lldb/test/API/lit.cfg.py b/lldb/test/API/lit.cfg.py index 12675edc0fd3b9..f9497b632fc504 100644 --- a/lldb/test/API/lit.cfg.py +++ b/lldb/test/API/lit.cfg.py @@ -309,3 +309,6 @@ def delete_module_cache(path): # Propagate XDG_CACHE_HOME if "XDG_CACHE_HOME" in os.environ: config.environment["XDG_CACHE_HOME"] = os.environ["XDG_CACHE_HOME"] + +if is_configured("use_vendor_packages"): +config.environment["LLDB_TEST_USE_VENDOR_PACKAGES"] = "1" diff --git a/lldb/test/API/lit.site.cfg.py.in b/lldb/test/API/lit.site.cfg.py.in index 053331dc4881f7..c2602acd2ef85a 100644 --- a/lldb/test/API/lit.site.cfg.py.in +++ b/lldb/test/API/lit.site.cfg.py.in @@ -38,6 +38,7 @@ config.libcxx_include_target_dir = "@LIBCXX_GENERATED_INCLUDE_TARGET_DIR@" # The API tests use their own module caches. config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-api") config.clang_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_CLANG@", "lldb-api") +config.use_vendor_packages = @LLDB_TEST_USE_VENDOR_PACKAGES@ # Plugins lldb_build_intel_pt = '@LLDB_BUILD_INTEL_PT@' diff --git a/lldb/test/CMakeLists.txt b/lldb/test/CMakeLists.txt index 1aa8843b6a2e78..d8cbb24b6c9b81 100644 --- a/lldb/test/CMakeLists.txt +++ b/lldb/test/CMakeLists.txt @@ -26,6 +26,20 @@ if(LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS) endforeach() endif() +# The "pexpect" package should come from the system environment, not from the +# LLDB tree. However, we delay the deletion of it from the tree in case +# users/buildbots don't have the package yet and need some time to install it. +if (NOT LLDB_TEST_USE_VENDOR_PACKAGES) + lldb_find_python_module(pexpect) + if (NOT PY_pexpect_FOUND) +message(FATAL_ERROR + "Python module 'pexpect' not found. Please install it via pip or via " + "your operating system's package manager. For a temporary workaround, " + "use a version from the LLDB tree with " + "`LLDB_TEST_USE_VENDOR_PACKAGES=ON`") + endif() +endif() + if(LLDB_BUILT_STANDALONE) # In order to run check-lldb-*
[Lldb-commits] [lldb] [lldb][test] Exclude third_party packages by default (PR #83191)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Jordan Rupprecht (rupprecht) Changes The goal here is to remove the third_party/Python/module tree, which LLDB tests only use to `import pexpect`. This package is available on `pip`, and I believe should not be hard to obtain. However, in case it isn't easily available, deleting the tree right now could cause disruption. This introduces a `LLDB_TEST_USE_VENDOR_PACKAGES` cmake param that can be enabled, and the tests will continue loading that tree. By default, it is disabled, and an eager cmake check runs that makes sure `pexpect` is available before waiting for the test to fail in an obscure way. Later, this option will go away, and when it does, we can delete the tree too. Ideally this is not disruptive, and we can remove it in a week or two. --- Full diff: https://github.com/llvm/llvm-project/pull/83191.diff 7 Files Affected: - (modified) lldb/cmake/modules/LLDBConfig.cmake (+2) - (modified) lldb/test/API/lit.cfg.py (+3) - (modified) lldb/test/API/lit.site.cfg.py.in (+1) - (modified) lldb/test/CMakeLists.txt (+16-1) - (modified) lldb/use_lldb_suite_root.py (+3-1) - (modified) lldb/utils/lldb-dotest/CMakeLists.txt (+1) - (modified) lldb/utils/lldb-dotest/lldb-dotest.in (+5) ``diff diff --git a/lldb/cmake/modules/LLDBConfig.cmake b/lldb/cmake/modules/LLDBConfig.cmake index a758261073ac57..5d62213c3f5838 100644 --- a/lldb/cmake/modules/LLDBConfig.cmake +++ b/lldb/cmake/modules/LLDBConfig.cmake @@ -67,6 +67,8 @@ option(LLDB_SKIP_STRIP "Whether to skip stripping of binaries when installing ll option(LLDB_SKIP_DSYM "Whether to skip generating a dSYM when installing lldb." OFF) option(LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS "Fail to configure if certain requirements are not met for testing." OFF) +option(LLDB_TEST_USE_VENDOR_PACKAGES + "Use packages from lldb/third_party/Python/module instead of system deps." OFF) set(LLDB_GLOBAL_INIT_DIRECTORY "" CACHE STRING "Path to the global lldbinit directory. Relative paths are resolved relative to the diff --git a/lldb/test/API/lit.cfg.py b/lldb/test/API/lit.cfg.py index 12675edc0fd3b9..f9497b632fc504 100644 --- a/lldb/test/API/lit.cfg.py +++ b/lldb/test/API/lit.cfg.py @@ -309,3 +309,6 @@ def delete_module_cache(path): # Propagate XDG_CACHE_HOME if "XDG_CACHE_HOME" in os.environ: config.environment["XDG_CACHE_HOME"] = os.environ["XDG_CACHE_HOME"] + +if is_configured("use_vendor_packages"): +config.environment["LLDB_TEST_USE_VENDOR_PACKAGES"] = "1" diff --git a/lldb/test/API/lit.site.cfg.py.in b/lldb/test/API/lit.site.cfg.py.in index 053331dc4881f7..c2602acd2ef85a 100644 --- a/lldb/test/API/lit.site.cfg.py.in +++ b/lldb/test/API/lit.site.cfg.py.in @@ -38,6 +38,7 @@ config.libcxx_include_target_dir = "@LIBCXX_GENERATED_INCLUDE_TARGET_DIR@" # The API tests use their own module caches. config.lldb_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_LLDB@", "lldb-api") config.clang_module_cache = os.path.join("@LLDB_TEST_MODULE_CACHE_CLANG@", "lldb-api") +config.use_vendor_packages = @LLDB_TEST_USE_VENDOR_PACKAGES@ # Plugins lldb_build_intel_pt = '@LLDB_BUILD_INTEL_PT@' diff --git a/lldb/test/CMakeLists.txt b/lldb/test/CMakeLists.txt index 1aa8843b6a2e78..d8cbb24b6c9b81 100644 --- a/lldb/test/CMakeLists.txt +++ b/lldb/test/CMakeLists.txt @@ -26,6 +26,20 @@ if(LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS) endforeach() endif() +# The "pexpect" package should come from the system environment, not from the +# LLDB tree. However, we delay the deletion of it from the tree in case +# users/buildbots don't have the package yet and need some time to install it. +if (NOT LLDB_TEST_USE_VENDOR_PACKAGES) + lldb_find_python_module(pexpect) + if (NOT PY_pexpect_FOUND) +message(FATAL_ERROR + "Python module 'pexpect' not found. Please install it via pip or via " + "your operating system's package manager. For a temporary workaround, " + "use a version from the LLDB tree with " + "`LLDB_TEST_USE_VENDOR_PACKAGES=ON`") + endif() +endif() + if(LLDB_BUILT_STANDALONE) # In order to run check-lldb-* we need the correct map_config directives in # llvm-lit. Because this is a standalone build, LLVM doesn't know about LLDB, @@ -240,7 +254,8 @@ llvm_canonicalize_cmake_booleans( LLDB_HAS_LIBCXX LLDB_TOOL_LLDB_SERVER_BUILD LLDB_USE_SYSTEM_DEBUGSERVER - LLDB_IS_64_BITS) + LLDB_IS_64_BITS + LLDB_TEST_USE_VENDOR_PACKAGES) # Configure the individual test suites. add_subdirectory(API) diff --git a/lldb/use_lldb_suite_root.py b/lldb/use_lldb_suite_root.py index fd42f63a3c7f30..b8f8acf5dd94a4 100644 --- a/lldb/use_lldb_suite_root.py +++ b/lldb/use_lldb_suite_root.py @@ -21,5 +21,7 @@ def add_lldbsuite_packages_dir(lldb_root): lldb_root = os.path.dirname(inspect.getfile(inspect.currentframe())) -add_third_party_module_dirs(lldb_root) +# Use environment variables to avoid plumbing flags, lit configs, etc. +if os.getenv("LLDB_TEST_USE_VEN
[Lldb-commits] [lldb] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
@@ -92,30 +104,20 @@ class ThreadPool { &Group); } - /// Blocking wait for all the threads to complete and the queue to be empty. - /// It is an error to try to add new tasks while blocking on this call. - /// Calling wait() from a task would deadlock waiting for itself. - void wait(); - - /// Blocking wait for only all the threads in the given group to complete. - /// It is possible to wait even inside a task, but waiting (directly or - /// indirectly) on itself will deadlock. If called from a task running on a - /// worker thread, the call may process pending tasks while waiting in order - /// not to waste the thread. - void wait(ThreadPoolTaskGroup &Group); - - // Returns the maximum number of worker threads in the pool, not the current - // number of threads! - unsigned getMaxConcurrency() const { return MaxThreadCount; } - - // TODO: misleading legacy name warning! - LLVM_DEPRECATED("Use getMaxConcurrency instead", "getMaxConcurrency") - unsigned getThreadCount() const { return MaxThreadCount; } +private: + /// Asynchronous submission of a task to the pool. The returned future can be + /// used to wait for the task to finish and is *non-blocking* on destruction. + template + std::shared_future asyncImpl(std::function Task, + ThreadPoolTaskGroup *Group) { GeorgeARM wrote: Ignore I just saw the code above. 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
@@ -92,30 +104,32 @@ class ThreadPool { &Group); } - /// Blocking wait for all the threads to complete and the queue to be empty. - /// It is an error to try to add new tasks while blocking on this call. - /// Calling wait() from a task would deadlock waiting for itself. - void wait(); +private: + /// Asynchronous submission of a task to the pool. The returned future can be + /// used to wait for the task to finish and is *non-blocking* on destruction. + template + std::shared_future asyncImpl(std::function Task, + ThreadPoolTaskGroup *Group) { - /// Blocking wait for only all the threads in the given group to complete. - /// It is possible to wait even inside a task, but waiting (directly or - /// indirectly) on itself will deadlock. If called from a task running on a - /// worker thread, the call may process pending tasks while waiting in order - /// not to waste the thread. - void wait(ThreadPoolTaskGroup &Group); +#if LLVM_ENABLE_THREADS +/// Wrap the Task in a std::function that sets the result of the +/// corresponding future. +auto R = createTaskAndFuture(Task); - // Returns the maximum number of worker threads in the pool, not the current - // number of threads! - unsigned getMaxConcurrency() const { return MaxThreadCount; } +asyncEnqueue(std::move(R.first), Group); +return R.second.share(); - // TODO: misleading legacy name warning! - LLVM_DEPRECATED("Use getMaxConcurrency instead", "getMaxConcurrency") - unsigned getThreadCount() const { return MaxThreadCount; } +#else // LLVM_ENABLE_THREADS Disabled - /// Returns true if the current thread is a worker thread of this thread pool. - bool isWorkerThread() const; +// Get a Future with launch::deferred execution using std::async joker-eph wrote: I removed this now 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
https://github.com/joker-eph updated https://github.com/llvm/llvm-project/pull/82094 >From f13befdfdb8715c034eed6dd4c04f712d30d043a 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 - 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 +- 15 files changed, 174 insertions(+), 114 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 &getThreadPool(); +ThreadPoolInterface &getThreadPool(); /// 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 &GetThreadPool(); + /// Shared thread pool. Use only with ThreadPoolTaskGroup. + static llvm::ThreadPoolInterface &GetThreadPool(); /// 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 &Log; - ThreadPool &Pool; + ThreadPoolInterface &Pool; Timer UpdateTimer; sys::Mutex UpdateMutex; @@ -145,7 +145,7 @@ class DebuginfodCollection { public: DebuginfodCollection(ArrayRef Paths, DebuginfodLog &Log, - ThreadPool &Pool, double MinInterval); + ThreadPoolInterface &Pool, 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 &TheThreadPool; +ThreadPoolInterface &TheThreadPool; std::mutex mtx; std::condition_variable cv; /// The number of threads tha
[Lldb-commits] [lldb] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
@@ -92,30 +104,32 @@ class ThreadPool { &Group); } - /// Blocking wait for all the threads to complete and the queue to be empty. - /// It is an error to try to add new tasks while blocking on this call. - /// Calling wait() from a task would deadlock waiting for itself. - void wait(); +private: + /// Asynchronous submission of a task to the pool. The returned future can be + /// used to wait for the task to finish and is *non-blocking* on destruction. + template + std::shared_future asyncImpl(std::function Task, + ThreadPoolTaskGroup *Group) { - /// Blocking wait for only all the threads in the given group to complete. - /// It is possible to wait even inside a task, but waiting (directly or - /// indirectly) on itself will deadlock. If called from a task running on a - /// worker thread, the call may process pending tasks while waiting in order - /// not to waste the thread. - void wait(ThreadPoolTaskGroup &Group); +#if LLVM_ENABLE_THREADS +/// Wrap the Task in a std::function that sets the result of the joker-eph wrote: removed 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-dap] Deduplicate watchpoints starting at the same address on SetDataBreakpointsRequest. (PR #83192)
https://github.com/ZequanWu created https://github.com/llvm/llvm-project/pull/83192 If a SetDataBreakpointsRequest contains a list data breakpoints which have duplicate starting addresses, the current behaviour is returning `{verified: true}` to both watchpoints with duplicated starting addresses. This confuses the client and what actually happens in lldb is the second one overwrite the first one. This fixes it by letting the last watchpoint at given address have `{verified: true}` and all previous watchpoints at the same address should have `{verfied: false}` at response. >From 563a4e5c9306fb5f06bdcc4a1b71dc92efbc86f8 Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Tue, 27 Feb 2024 16:40:38 -0500 Subject: [PATCH] [lldb-dap] Deduplicate watchpoints starting at the same address on SetDataBreakpointsRequest. --- .../TestDAP_setDataBreakpoints.py | 45 +++ lldb/tools/lldb-dap/Watchpoint.cpp| 23 +- lldb/tools/lldb-dap/Watchpoint.h | 5 +++ lldb/tools/lldb-dap/lldb-dap.cpp | 16 ++- 4 files changed, 78 insertions(+), 11 deletions(-) diff --git a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py index 17cdad89aa6d10..52c0bbfb33dad8 100644 --- a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py +++ b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py @@ -12,6 +12,51 @@ def setUp(self): lldbdap_testcase.DAPTestCaseBase.setUp(self) self.accessTypes = ["read", "write", "readWrite"] +@skipIfWindows +@skipIfRemote +def test_duplicate_start_addresses(self): +"""Test setDataBreakpoints with multiple watchpoints starting at the same addresses.""" +program = self.getBuildArtifact("a.out") +self.build_and_launch(program) +source = "main.cpp" +first_loop_break_line = line_number(source, "// first loop breakpoint") +self.set_source_breakpoints(source, [first_loop_break_line]) +self.continue_to_next_stop() +self.dap_server.get_stackFrame() +# Test setting write watchpoint using expressions: &x, arr+2 +response_x = self.dap_server.request_dataBreakpointInfo(0, "&x") +response_arr_2 = self.dap_server.request_dataBreakpointInfo(0, "arr+2") +# Test response from dataBreakpointInfo request. +self.assertEquals(response_x["body"]["dataId"].split("/")[1], "4") +self.assertEquals(response_x["body"]["accessTypes"], self.accessTypes) +self.assertEquals(response_arr_2["body"]["dataId"].split("/")[1], "4") +self.assertEquals(response_arr_2["body"]["accessTypes"], self.accessTypes) +# The first one should be overwritten by the third one as they start at +# the same address. This is indicated by returning {verified: False} for +# the first one. +dataBreakpoints = [ +{"dataId": response_x["body"]["dataId"], "accessType": "read"}, +{"dataId": response_arr_2["body"]["dataId"], "accessType": "write"}, +{"dataId": response_x["body"]["dataId"], "accessType": "write"}, +] +set_response = self.dap_server.request_setDataBreakpoint(dataBreakpoints) +self.assertEquals( +set_response["body"]["breakpoints"], +[{"verified": False}, {"verified": True}, {"verified": True}], +) + +self.continue_to_next_stop() +x_val = self.dap_server.get_local_variable_value("x") +i_val = self.dap_server.get_local_variable_value("i") +self.assertEquals(x_val, "2") +self.assertEquals(i_val, "1") + +self.continue_to_next_stop() +arr_2 = self.dap_server.get_local_variable_child("arr", "[2]") +i_val = self.dap_server.get_local_variable_value("i") +self.assertEquals(arr_2["value"], "42") +self.assertEquals(i_val, "2") + @skipIfWindows @skipIfRemote def test_expression(self): diff --git a/lldb/tools/lldb-dap/Watchpoint.cpp b/lldb/tools/lldb-dap/Watchpoint.cpp index 2f176e0da84f15..21765509449140 100644 --- a/lldb/tools/lldb-dap/Watchpoint.cpp +++ b/lldb/tools/lldb-dap/Watchpoint.cpp @@ -16,17 +16,11 @@ Watchpoint::Watchpoint(const llvm::json::Object &obj) : BreakpointBase(obj) { llvm::StringRef dataId = GetString(obj, "dataId"); std::string accessType = GetString(obj, "accessType").str(); auto [addr_str, size_str] = dataId.split('/'); - lldb::addr_t addr; - size_t size; llvm::to_integer(addr_str, addr, 16); llvm::to_integer(size_str, size); - lldb::SBWatchpointOptions options; options.SetWatchpointTypeRead(accessType != "write"); if (accessType != "read") options.SetWatchpointTypeWrite(lldb::eWatchpointWriteTypeOnModify); - wp = g_dap.target.WatchpointCreateByAddress(addr, size, options, error); - SetCondition(); - SetHitConditi
[Lldb-commits] [lldb] [lldb-dap] Deduplicate watchpoints starting at the same address on SetDataBreakpointsRequest. (PR #83192)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Zequan Wu (ZequanWu) Changes If a SetDataBreakpointsRequest contains a list data breakpoints which have duplicate starting addresses, the current behaviour is returning `{verified: true}` to both watchpoints with duplicated starting addresses. This confuses the client and what actually happens in lldb is the second one overwrite the first one. This fixes it by letting the last watchpoint at given address have `{verified: true}` and all previous watchpoints at the same address should have `{verfied: false}` at response. --- Full diff: https://github.com/llvm/llvm-project/pull/83192.diff 4 Files Affected: - (modified) lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py (+45) - (modified) lldb/tools/lldb-dap/Watchpoint.cpp (+13-10) - (modified) lldb/tools/lldb-dap/Watchpoint.h (+5) - (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+15-1) ``diff diff --git a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py index 17cdad89aa6d10..52c0bbfb33dad8 100644 --- a/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py +++ b/lldb/test/API/tools/lldb-dap/databreakpoint/TestDAP_setDataBreakpoints.py @@ -12,6 +12,51 @@ def setUp(self): lldbdap_testcase.DAPTestCaseBase.setUp(self) self.accessTypes = ["read", "write", "readWrite"] +@skipIfWindows +@skipIfRemote +def test_duplicate_start_addresses(self): +"""Test setDataBreakpoints with multiple watchpoints starting at the same addresses.""" +program = self.getBuildArtifact("a.out") +self.build_and_launch(program) +source = "main.cpp" +first_loop_break_line = line_number(source, "// first loop breakpoint") +self.set_source_breakpoints(source, [first_loop_break_line]) +self.continue_to_next_stop() +self.dap_server.get_stackFrame() +# Test setting write watchpoint using expressions: &x, arr+2 +response_x = self.dap_server.request_dataBreakpointInfo(0, "&x") +response_arr_2 = self.dap_server.request_dataBreakpointInfo(0, "arr+2") +# Test response from dataBreakpointInfo request. +self.assertEquals(response_x["body"]["dataId"].split("/")[1], "4") +self.assertEquals(response_x["body"]["accessTypes"], self.accessTypes) +self.assertEquals(response_arr_2["body"]["dataId"].split("/")[1], "4") +self.assertEquals(response_arr_2["body"]["accessTypes"], self.accessTypes) +# The first one should be overwritten by the third one as they start at +# the same address. This is indicated by returning {verified: False} for +# the first one. +dataBreakpoints = [ +{"dataId": response_x["body"]["dataId"], "accessType": "read"}, +{"dataId": response_arr_2["body"]["dataId"], "accessType": "write"}, +{"dataId": response_x["body"]["dataId"], "accessType": "write"}, +] +set_response = self.dap_server.request_setDataBreakpoint(dataBreakpoints) +self.assertEquals( +set_response["body"]["breakpoints"], +[{"verified": False}, {"verified": True}, {"verified": True}], +) + +self.continue_to_next_stop() +x_val = self.dap_server.get_local_variable_value("x") +i_val = self.dap_server.get_local_variable_value("i") +self.assertEquals(x_val, "2") +self.assertEquals(i_val, "1") + +self.continue_to_next_stop() +arr_2 = self.dap_server.get_local_variable_child("arr", "[2]") +i_val = self.dap_server.get_local_variable_value("i") +self.assertEquals(arr_2["value"], "42") +self.assertEquals(i_val, "2") + @skipIfWindows @skipIfRemote def test_expression(self): diff --git a/lldb/tools/lldb-dap/Watchpoint.cpp b/lldb/tools/lldb-dap/Watchpoint.cpp index 2f176e0da84f15..21765509449140 100644 --- a/lldb/tools/lldb-dap/Watchpoint.cpp +++ b/lldb/tools/lldb-dap/Watchpoint.cpp @@ -16,17 +16,11 @@ Watchpoint::Watchpoint(const llvm::json::Object &obj) : BreakpointBase(obj) { llvm::StringRef dataId = GetString(obj, "dataId"); std::string accessType = GetString(obj, "accessType").str(); auto [addr_str, size_str] = dataId.split('/'); - lldb::addr_t addr; - size_t size; llvm::to_integer(addr_str, addr, 16); llvm::to_integer(size_str, size); - lldb::SBWatchpointOptions options; options.SetWatchpointTypeRead(accessType != "write"); if (accessType != "read") options.SetWatchpointTypeWrite(lldb::eWatchpointWriteTypeOnModify); - wp = g_dap.target.WatchpointCreateByAddress(addr, size, options, error); - SetCondition(); - SetHitCondition(); } void Watchpoint::SetCondition() { wp.SetCondition(condition.c_str()); } @@ -38,11 +32,20 @@ void Watchpoint::SetHitCondition() { } void Watchpoint::CreateJsonObject(l
[Lldb-commits] [lldb] [lldb][test] Exclude third_party packages by default (PR #83191)
https://github.com/bulbazord approved this pull request. LGTM @JDevlieghere @adrian-prantl we should consider adding this to the list of required python modules on our bots too and enforce it with `LLDB_ENFORCE_STRICT_TEST_REQUIREMENTS`? Probably not in this PR, but after we can get our bots configured correctly. 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] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -1433,13 +1434,17 @@ void Debugger::SetDestroyCallback( static void PrivateReportProgress(Debugger &debugger, uint64_t progress_id, std::string title, std::string details, uint64_t completed, uint64_t total, - bool is_debugger_specific) { + bool is_debugger_specific, + uint32_t progress_broadcast_bit) { // Only deliver progress events if we have any progress listeners. const uint32_t event_type = Debugger::eBroadcastBitProgress; - if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type)) + const uint32_t category_event_type = Debugger::eBroadcastBitProgressCategory; + if (!debugger.GetBroadcaster().EventTypeHasListeners(event_type | + category_event_type)) JDevlieghere wrote: You only have to check the `progress_broadcast_bit`. 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] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -1929,6 +1938,8 @@ lldb::thread_result_t Debugger::DefaultEventHandler() { } else if (broadcaster == &m_broadcaster) { if (event_type & Debugger::eBroadcastBitProgress) HandleProgressEvent(event_sp); +else if (event_type & Debugger::eBroadcastBitProgressCategory) + HandleProgressEvent(event_sp); JDevlieghere wrote: Now we're going to handle the same event twice, once when broadcast as `eBroadcastBitProgress` and once when broadcast as `eBroadcastBitProgressCategory`. In the default event handler we want to see all the individual updates and as that's a superset, we should not handle `eBroadcastBitProgressCategory`. 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] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -29,8 +30,12 @@ Progress::Progress(std::string title, std::string details, if (debugger) m_debugger_id = debugger->GetID(); + + m_progress_data = {m_title, m_details, m_id, + m_completed, m_total, m_debugger_id}; JDevlieghere wrote: Did `clang-format` format this like this? Interesting... Anyway, you should initialize this in the constructors initializer list (and get rid of `m_title` etc) as per my previous comment. 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] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -1875,7 +1883,8 @@ lldb::thread_result_t Debugger::DefaultEventHandler() { listener_sp->StartListeningForEvents( &m_broadcaster, eBroadcastBitProgress | eBroadcastBitWarning | - eBroadcastBitError | eBroadcastSymbolChange); + eBroadcastBitError | eBroadcastSymbolChange | JDevlieghere wrote: See my comment below why we shouldn't listen for `eBroadcastBitProgressCategory` in the default event handler. 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] [lldb][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -97,27 +98,44 @@ class Progress { /// Used to indicate a non-deterministic progress report static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX; + /// Use a struct to send data from a Progress object to + /// the progress manager. + struct ProgressData { +/// The title of the progress activity, also used as a category to group +/// reports. +std::string title; +/// More specific information about the current file being displayed in the +/// report. +std::string details; +/// A unique integer identifier for progress reporting. +uint64_t progress_id; +/// How much work ([0...m_total]) that has been completed. +uint64_t completed; +/// Total amount of work, use a std::nullopt in the constructor for non +/// deterministic progress. +uint64_t total; +/// 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: + friend class Debugger; void ReportProgress(); static std::atomic g_id; - /// The title of the progress activity. std::string m_title; std::string m_details; JDevlieghere wrote: You're still storing the same fields as in `ProgressData`. You should only have the latter and initialize the progress data in the constructor. 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] [LLDB] Fix completion of space-only lines in the REPL on Linux (PR #83203)
https://github.com/walter-erquinigo created https://github.com/llvm/llvm-project/pull/83203 https://github.com/modularml/mojo/issues/1796 discovered that if you try to complete a space-only line in the REPL on Linux, LLDB crashes. I suspect that editline doesn't behave the same way on linux and on darwin, because I can't replicate this on darwin. Adding a boundary check in the completion code prevents the crash from happening. >From 8dcc86a33bf10547f4a1c4175830b50a0508efe0 Mon Sep 17 00:00:00 2001 From: walter erquinigo Date: Tue, 27 Feb 2024 17:59:20 -0500 Subject: [PATCH] [LLDB] Fix completion of space-only lines in the REPL on Linux https://github.com/modularml/mojo/issues/1796 discovered that if you try to complete a space-only line in the REPL on Linux, LLDB crashes. I suspect that editline doesn't behave the same way on linux and on darwin, because I can't replicate this on darwin. Adding a boundary check in the completion code prevents the crash from happening. --- lldb/source/Host/common/Editline.cpp | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lldb/source/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp index ce707e530d008b..e66271e8a6ee99 100644 --- a/lldb/source/Host/common/Editline.cpp +++ b/lldb/source/Host/common/Editline.cpp @@ -1029,8 +1029,11 @@ unsigned char Editline::TabCommand(int ch) { case CompletionMode::Normal: { std::string to_add = completion.GetCompletion(); // Terminate the current argument with a quote if it started with a quote. - if (!request.GetParsedLine().empty() && request.GetParsedArg().IsQuoted()) + Args &parsedLine = request.GetParsedLine(); + if (!parsedLine.empty() && request.GetCursorIndex() < parsedLine.size() && + request.GetParsedArg().IsQuoted()) { to_add.push_back(request.GetParsedArg().GetQuoteChar()); + } to_add.push_back(' '); el_deletestr(m_editline, request.GetCursorArgumentPrefix().size()); el_insertstr(m_editline, to_add.c_str()); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Fix completion of space-only lines in the REPL on Linux (PR #83203)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Walter Erquinigo (walter-erquinigo) Changes https://github.com/modularml/mojo/issues/1796 discovered that if you try to complete a space-only line in the REPL on Linux, LLDB crashes. I suspect that editline doesn't behave the same way on linux and on darwin, because I can't replicate this on darwin. Adding a boundary check in the completion code prevents the crash from happening. --- Full diff: https://github.com/llvm/llvm-project/pull/83203.diff 1 Files Affected: - (modified) lldb/source/Host/common/Editline.cpp (+4-1) ``diff diff --git a/lldb/source/Host/common/Editline.cpp b/lldb/source/Host/common/Editline.cpp index ce707e530d008b..e66271e8a6ee99 100644 --- a/lldb/source/Host/common/Editline.cpp +++ b/lldb/source/Host/common/Editline.cpp @@ -1029,8 +1029,11 @@ unsigned char Editline::TabCommand(int ch) { case CompletionMode::Normal: { std::string to_add = completion.GetCompletion(); // Terminate the current argument with a quote if it started with a quote. - if (!request.GetParsedLine().empty() && request.GetParsedArg().IsQuoted()) + Args &parsedLine = request.GetParsedLine(); + if (!parsedLine.empty() && request.GetCursorIndex() < parsedLine.size() && + request.GetParsedArg().IsQuoted()) { to_add.push_back(request.GetParsedArg().GetQuoteChar()); + } to_add.push_back(' '); el_deletestr(m_editline, request.GetCursorArgumentPrefix().size()); el_insertstr(m_editline, to_add.c_str()); `` https://github.com/llvm/llvm-project/pull/83203 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
@@ -227,7 +265,7 @@ class ThreadPool { class ThreadPoolTaskGroup { aganea wrote: That point was raised by the OP in the Discourse thread (allow for custom implementations of `ThreadPoolTaskGroup`) and I think it deserved a clear answer, and I do agree now to your point. Until there's no clear use-case, the behavior of `ThreadPoolTaskGroup` will remain as is, deferred to the `ThreadPool` implementation. 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][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -29,8 +30,12 @@ Progress::Progress(std::string title, std::string details, if (debugger) m_debugger_id = debugger->GetID(); + + m_progress_data = {m_title, m_details, m_id, + m_completed, m_total, m_debugger_id}; chelcassanova wrote: I'm removing this anyways but yeah not sure what `clang-format` was thinking here. 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] [lldb] Add SBProcess methods for get/set/use address masks (PR #83095)
https://github.com/jasonmolenda updated https://github.com/llvm/llvm-project/pull/83095 >From dae16776e8c97158e8965e4d0e950cd2ce836f75 Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Mon, 26 Feb 2024 18:05:27 -0800 Subject: [PATCH 1/2] [lldb] Add SBProcess methods for get/set/use address masks 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 | 123 ++ lldb/include/lldb/Utility/AddressableBits.h | 3 + lldb/include/lldb/lldb-enumerations.h | 14 ++ lldb/source/API/SBProcess.cpp | 89 + lldb/source/Utility/AddressableBits.cpp | 12 +- .../python_api/process/address-masks/Makefile | 3 + .../process/address-masks/TestAddressMasks.py | 64 + .../python_api/process/address-masks/main.c | 5 + 8 files changed, 311 insertions(+), 2 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..7e9ad7d9a274f2 100644 --- a/lldb/include/lldb/API/SBProcess.h +++ b/lldb/include/lldb/API/SBProcess.h @@ -407,6 +407,129 @@ class LLDB_API SBProcess { /// the process isn't loaded from a core file. lldb::SBFileSpec GetCoreFile(); + /// Get the current address mask that can be applied to addresses + /// before reading from memory. + /// + /// \param[in] type + /// lldb may have different address masks for code and data + /// addresses. Either can be requested, or most commonly, + /// eAddressMaskTypeAny can be requested and the least specific + /// mask will be fetched. e.g. on a target where instructions + /// are word aligned, the Code mask might clear the low 2 bits. + /// + /// \param[in] addr_range + /// Specify whether the address mask for high or low address spaces + /// is requested. + /// It is highly unusual to have different address masks in high + /// or low memory, and by default the eAddressMaskRangeLow is the + /// only one used for both types of addresses, the default value for + /// this argument is the correct one. + /// + /// On some architectures like AArch64, it is possible to have + /// different page table setups for low and high memory, so different + /// numbers of bits relevant to addressing, and it is possible to have + /// a program running in one half of memory and accessing the other + /// as heap, etc. In that case the eAddressMaskRangeLow and + /// eAddressMaskRangeHigh will have different masks that must be handled. + /// + /// \return + /// The address mask currently in use. Bits which are not used + /// for addressing will be set to 1 in the mask. + lldb::addr_t GetAddressMask( + lldb::AddressMaskType type, + lldb::AddressMaskRange addr_range = lldb::eAddressMaskRangeLow); + + /// Set the current address mask that can be applied to addresses + /// before reading from memory. + /// + /// \param[in] type + /// lldb may have different address masks for code and data + /// addresses. Either can be set, or most commonly, + /// eAddressMaskTypeAll can be set for both types of addresses. + /// An example where they could be different is a target where + /// instructions are word aligned, so the low 2 bits are always + /// zero. + /// + /// \param[in] mask + /// The address mask to set. Bits which are not used for addressing + /// should be set to 1 in the mask. + /// + /// \param[in] addr_range + /// Specify whether the address mask for high or low address spaces + /// is being set. + /// It is highly unusual to have different address masks in high + /// or low memory, and by default the eAddressMaskRangeLow is the + /// only one used for both types of addresses, the default value for + /// this argument is the correct one. + /// + /// On some architectures like AArch64, it is possible to have + /// different page table setups for low and high memory, so different + /// numbers
[Lldb-commits] [lldb] [lldb] Add SBProcess methods for get/set/use address masks (PR #83095)
jasonmolenda wrote: Thanks so much for reading through these @DavidSpickett and @hawkinsw ! @adrian-prantl and @JDevlieghere suggested using a doxygen group for this set of methods and having the long definitions of `type` and `addr_range` a single time, referring back to them from the individual methods, I wasn't thrilled with all that duplicated text either. I think I did this well enough? 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
@@ -227,7 +265,7 @@ class ThreadPool { class ThreadPoolTaskGroup { GeorgeARM wrote: Btw, I still think that passing a `ThreadPoolTaskGroup` for the sole purpose of a uid is a bit brittle as you bind the interface with this construct but internally to the `ThreadPool` no member function of it is called?! We could change for example this with a `std::string` with probably minimal changes to any call site. Apologies if that is not right but I am not familiar with the code. 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
https://github.com/GeorgeARM edited 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
@@ -227,7 +265,7 @@ class ThreadPool { class ThreadPoolTaskGroup { joker-eph wrote: > We could change for example this with a std::string or uint64_t with probably > minimal changes to any call site. How would you get a unique id at runtime if not for the address of the object? 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
@@ -227,7 +265,7 @@ class ThreadPool { class ThreadPoolTaskGroup { GeorgeARM wrote: Sure you can convert the address of the object to a string or int that is fine; it is not the best but not worse than what is there. You can do this inside the `ThreadPoolTaskGroup` which is essentially a RAII wrapper. What you get is probably a cleaner API IMHO and don't impose to the implementations to have to consume a struct that by definition loops around the interface. 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
https://github.com/GeorgeARM edited 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
@@ -227,7 +265,7 @@ class ThreadPool { class ThreadPoolTaskGroup { joker-eph wrote: > What you get is probably a cleaner API IMHO I don't quite get why it would cleaner actually? This makes is for a weaker interface that does not guarantee any uniqueness and you lose type safety as well. I must missing your point here. 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
https://github.com/joker-eph edited 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
@@ -227,7 +265,7 @@ class ThreadPool { class ThreadPoolTaskGroup { GeorgeARM wrote: So the way I see this is: I have a public API that looks like: ``` virtual void asyncEnqueue(std::function Task, ThreadPoolTaskGroup *Group) = 0; // or similar ``` My thoughts: * `ThreadPoolTaskGroup` implementation is actually internal to the `ThreadPool` * `ThreadPoolTaskGroup` being passed to the API but only used internally as just a UID. No member functions being called internally. Moreover, the object definition itself is a RAII wrapper on top of the ThreadPool API implementation so I find that this is an unnecessary circular dependency and coupling. * Uniqueness is `guaranteed` through memory addressing. Someone can `break` uniqueness when going through the public `ThreadPool` API. * Type safety; sure you don't have implicit conversions going on. What `guarantees` to you uniqueness is essentially when you use RAII wrapper itself and the fact that calls at termination: ` ~ThreadPoolTaskGroup() { wait(); }` which will go through; process the tasks and erase the group from the `ActiveGroups`. So what I think is that passing instead of passing `*this` you can convert the address to a string/int and pass this instead. Eitherway, please ignore my comment if it doesn't make sense; as said not familiar with the codebase. I just find strange a bit this aspect of the API. 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
https://github.com/GeorgeARM edited 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
https://github.com/GeorgeARM edited 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
https://github.com/GeorgeARM edited 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
https://github.com/GeorgeARM edited 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
https://github.com/GeorgeARM edited 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
@@ -227,7 +265,7 @@ class ThreadPool { class ThreadPoolTaskGroup { joker-eph wrote: > Uniqueness is "guaranteed" through memory addressing. Someone can break > uniqueness when going through the public ThreadPool API. How so? > Type safety; sure you don't have implicit conversions going on. There is much more than: the API is constrained to say: "it operates on an instance of a TaskGroup object". This is a very important API: it is explicit in terms of the scope on which it operates, and the fact that it care about the **actual instance** of the TaskGroup object. > So what I think is that instead of passing *this you can convert the address > to a string/int and pass this instead; hence you don't lose any guarantees. I > understand your arguments but I find this a bit more decoupled. As I said above, I would have strong concerns with this: it is a weak API (it is open to any integer and does not carry the semantics I described above) and "unsafe", just like using `void*`. There is not way for a user to know what integer to pass here for example. As a user such API is highly confusing and really I'm not sure what the pattern would be other than always casting the address of the task group to an int. 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