[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
[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 {
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/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 {
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/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 {
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)
@@ -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)
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 {
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] [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] [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
+ ///
[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] [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] 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 = 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] [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 = 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][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(
_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][progress] Hook up new broadcast bit and progress manager (PR #83069)
@@ -1433,13 +1434,17 @@ void Debugger::SetDestroyCallback(
static void PrivateReportProgress(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 == _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][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-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: , arr+2
+response_x = self.dap_server.request_dataBreakpointInfo(0, "")
+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 ) :
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
[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: , arr+2
+response_x = self.dap_server.request_dataBreakpointInfo(0, "")
+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 ) :
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();
[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 {
);
}
- /// 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 );
+#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] [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 ();
+ThreadPoolInterface ();
/// Perform the work on each BinaryFunction except those that are accepted
/// by SkipPredicate, scheduling heuristic is based on SchedPolicy.
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 6ba90eb6ed8fdf..66b1e28a9cc618 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -52,7 +52,7 @@
namespace llvm {
class raw_ostream;
-class ThreadPool;
+class ThreadPoolInterface;
} // namespace llvm
namespace lldb_private {
@@ -499,8 +499,8 @@ class Debugger : public
std::enable_shared_from_this,
return m_broadcaster_manager_sp;
}
- /// Shared thread poll. Use only with ThreadPoolTaskGroup.
- static llvm::ThreadPool ();
+ /// Shared thread pool. Use only with ThreadPoolTaskGroup.
+ static llvm::ThreadPoolInterface ();
/// Report warning events.
///
diff --git a/llvm/include/llvm/Debuginfod/Debuginfod.h
b/llvm/include/llvm/Debuginfod/Debuginfod.h
index ef03948a706c06..99fe15ad859794 100644
--- a/llvm/include/llvm/Debuginfod/Debuginfod.h
+++ b/llvm/include/llvm/Debuginfod/Debuginfod.h
@@ -97,7 +97,7 @@ Expected getCachedOrDownloadArtifact(
StringRef UniqueKey, StringRef UrlPath, StringRef CacheDirectoryPath,
ArrayRef DebuginfodUrls, std::chrono::milliseconds Timeout);
-class ThreadPool;
+class ThreadPoolInterface;
struct DebuginfodLogEntry {
std::string Message;
@@ -135,7 +135,7 @@ class DebuginfodCollection {
// error.
Expected updateIfStale();
DebuginfodLog
- ThreadPool
+ ThreadPoolInterface
Timer UpdateTimer;
sys::Mutex UpdateMutex;
@@ -145,7 +145,7 @@ class DebuginfodCollection {
public:
DebuginfodCollection(ArrayRef Paths, DebuginfodLog ,
- ThreadPool , double MinInterval);
+ ThreadPoolInterface , double MinInterval);
Error update();
Error updateForever(std::chrono::milliseconds Interval);
Expected findDebugBinaryPath(object::BuildIDRef);
diff --git a/llvm/include/llvm/Support/BalancedPartitioning.h
b/llvm/include/llvm/Support/BalancedPartitioning.h
index a8464ac0fe60e5..9738e742f7f1e9 100644
--- a/llvm/include/llvm/Support/BalancedPartitioning.h
+++ b/llvm/include/llvm/Support/BalancedPartitioning.h
@@ -50,7 +50,7 @@
namespace llvm {
-class ThreadPool;
+class ThreadPoolInterface;
/// A function with a set of utility nodes where it is beneficial to order two
/// functions close together if they have similar utility nodes
class BPFunctionNode {
@@ -115,7 +115,7 @@ class BalancedPartitioning {
/// threads, so we need to track how many active threads that could spawn
more
/// threads.
struct BPThreadPool {
-ThreadPool
+ThreadPoolInterface
std::mutex mtx;
std::condition_variable cv;
/// The number of threads that could spawn more threads
@@ -128,7 +128,8 @@ class BalancedPartitioning {
/// acceptable for other threads to
[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 {
);
}
- /// 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 );
+#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)
@@ -92,30 +104,20 @@ class ThreadPool {
);
}
- /// 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 );
-
- // 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] [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
[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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
@@ -92,30 +104,32 @@ class ThreadPool {
);
}
- /// 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 );
+#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] [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 {
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 )`.
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 = context->getThreadPool();
llvm::ThreadPoolTaskGroup tasksGroup(threadPool);
```
we would need to use the pool as a factory somehow:
```
llvm::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] [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] 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] 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 ,
- 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 )
: CommandObjectParsed(
interpreter, "apropos",
"List debugger commands related to a
[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] [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
[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
[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] 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] 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, );
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)
-
[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 ,
- 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 )
: 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 variant this
[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] [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/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] 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] 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.
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 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] Start to clean up the process of defining command arguments. (PR #83097)
@@ -694,27 +712,32 @@ void CommandObject::GenerateHelpText(Stream _strm)
{
}
}
-void CommandObject::AddIDsArgumentData(CommandArgumentEntry ,
- 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] [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 {
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)
@@ -92,30 +104,20 @@ class ThreadPool {
);
}
- /// 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 );
-
- // 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)
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)
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] 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 _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] 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] [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] [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] [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.
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
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] 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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
@@ -92,30 +104,32 @@ class ThreadPool {
);
}
- /// 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 );
+#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] [llvm] [mlir] Split the llvm::ThreadPool into an abstract base class and an implementation (PR #82094)
@@ -92,30 +104,32 @@ class ThreadPool {
);
}
- /// 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 );
+#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)
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 ();
+ThreadPoolInterface ();
/// Perform the work on each BinaryFunction except those that are accepted
/// by SkipPredicate, scheduling heuristic is based on SchedPolicy.
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 6ba90eb6ed8fdf..66b1e28a9cc618 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -52,7 +52,7 @@
namespace llvm {
class raw_ostream;
-class ThreadPool;
+class ThreadPoolInterface;
} // namespace llvm
namespace lldb_private {
@@ -499,8 +499,8 @@ class Debugger : public
std::enable_shared_from_this,
return m_broadcaster_manager_sp;
}
- /// Shared thread poll. Use only with ThreadPoolTaskGroup.
- static llvm::ThreadPool ();
+ /// Shared thread pool. Use only with ThreadPoolTaskGroup.
+ static llvm::ThreadPoolInterface ();
/// Report warning events.
///
diff --git a/llvm/include/llvm/Debuginfod/Debuginfod.h
b/llvm/include/llvm/Debuginfod/Debuginfod.h
index ef03948a706c06..99fe15ad859794 100644
--- a/llvm/include/llvm/Debuginfod/Debuginfod.h
+++ b/llvm/include/llvm/Debuginfod/Debuginfod.h
@@ -97,7 +97,7 @@ Expected getCachedOrDownloadArtifact(
StringRef UniqueKey, StringRef UrlPath, StringRef CacheDirectoryPath,
ArrayRef DebuginfodUrls, std::chrono::milliseconds Timeout);
-class ThreadPool;
+class ThreadPoolInterface;
struct DebuginfodLogEntry {
std::string Message;
@@ -135,7 +135,7 @@ class DebuginfodCollection {
// error.
Expected updateIfStale();
DebuginfodLog
- ThreadPool
+ ThreadPoolInterface
Timer UpdateTimer;
sys::Mutex UpdateMutex;
@@ -145,7 +145,7 @@ class DebuginfodCollection {
public:
DebuginfodCollection(ArrayRef Paths, DebuginfodLog ,
- ThreadPool , double MinInterval);
+ ThreadPoolInterface , double MinInterval);
Error update();
Error updateForever(std::chrono::milliseconds Interval);
Expected findDebugBinaryPath(object::BuildIDRef);
diff --git a/llvm/include/llvm/Support/BalancedPartitioning.h
b/llvm/include/llvm/Support/BalancedPartitioning.h
index a8464ac0fe60e5..9738e742f7f1e9 100644
--- a/llvm/include/llvm/Support/BalancedPartitioning.h
+++ b/llvm/include/llvm/Support/BalancedPartitioning.h
@@ -50,7 +50,7 @@
namespace llvm {
-class ThreadPool;
+class ThreadPoolInterface;
/// A function with a set of utility nodes where it is beneficial to order two
/// functions close together if they have similar utility nodes
class BPFunctionNode {
@@ -115,7 +115,7 @@ class BalancedPartitioning {
/// threads, so we need to track how many active threads that could spawn
more
/// threads.
struct BPThreadPool {
-ThreadPool
+ThreadPoolInterface
std::mutex mtx;
std::condition_variable cv;
/// The number of threads that could spawn more threads
@@ -128,7 +128,8 @@ class BalancedPartitioning {
/// acceptable for other threads to
