[Lldb-commits] [lldb] [lldb] Introduce an always-on system log category/channel (PR #108495)

2024-09-13 Thread David Spickett via lldb-commits


@@ -31,6 +31,22 @@ class ProcessInstanceInfo;
 class ProcessInstanceInfoMatch;
 typedef std::vector ProcessInstanceInfoList;
 
+// Always on system log category and channel.

DavidSpickett wrote:

So is the log naming `system system` or for now is it just `system` and we 
might add sub-channels later?

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


[Lldb-commits] [lldb] [lldb] Introduce an always-on system log category/channel (PR #108495)

2024-09-13 Thread David Spickett via lldb-commits


@@ -0,0 +1,3 @@
+RUN: %lldb -o 'log list' -o 'log disable system' 2>&1 | FileCheck %s
+CHECK-NOT: Logging categories for 'system'
+CHECK: Cannot disable internal log channel 'system'.

DavidSpickett wrote:

This is bikeshedding but perhaps:
```
`system` is an internal log channel, internal channels can not be disabled.
```
Just to be extra clear that we're not just printing "internal log" for the sake 
of it, it's the reason why it cannot be disabled.

(...maybe I am too used to software lying to me and should relax :rofl: )

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


[Lldb-commits] [lldb] [lldb] Introduce an always-on system log category/channel (PR #108495)

2024-09-13 Thread David Spickett via lldb-commits

DavidSpickett wrote:

> Unlike other, existing log channels, it is not exposed to users.

This is presumably because:
1. It contains very low level messages they likely won't be interested in.
2. If it weren't hidden, it would be printing into their sessions all the time.

Users can still list it, I see that from the test, but that's fine we don't 
need to lie about its existence.

If someone suspects that a relevant message might be in this channel how do 
they look at it? Do they crash lldb / use a command to dump an archive of logs?

> The channel is meant to be used sparsely and deliberately for logging 
> high-value information to the system log.

What is an example of that? So that folks (aka me) have an idea what might go 
here for other OSs. Is it stuff like this process was killed for this reason, 
this system resource had to be cleared, that sort of thing?

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


[Lldb-commits] [lldb] [lldb] Introduce an always-on system log category/channel (PR #108495)

2024-09-13 Thread David Spickett via lldb-commits

DavidSpickett wrote:

>We have a similar concept in the downstream Swift fork and this has proven to 
>be extremely valuable. This is especially true on macOS where system log 
>messages are automatically captured as part of a sysdiagnose.

Does this mean someone has already been putting non-MacOS messages in there in 
the Swift fork? Would be interested to know what those are.

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


[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

2024-09-13 Thread Dmitry Vasilyev via lldb-commits

slydiman wrote:

@labath Let's decide how to finish this patch, please.
I would prefer to commit the new functionality. 
Any cosmetics and refactoring is possible at any time.

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


[Lldb-commits] [lldb] [LLDB] Fix operators <= and >= returning a wrong result when comparing to a floating point NaN (PR #108060)

2024-09-13 Thread Pavel Labath via lldb-commits

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

Thanks.

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


[Lldb-commits] [lldb] [llvm] [lldb] Change lldb's breakpoint detection behavior [WIP] (PR #105594)

2024-09-13 Thread Jason Molenda via lldb-commits

https://github.com/jasonmolenda updated 
https://github.com/llvm/llvm-project/pull/105594

>From 56ca564185bdceea25162a1ce3b1e8c8fa2641e2 Mon Sep 17 00:00:00 2001
From: Jason Molenda 
Date: Fri, 19 Jul 2024 17:26:13 -0700
Subject: [PATCH 1/6] [lldb] Change lldb's breakpoint handling behavior
 (#96260)

lldb today has two rules: When a thread stops at a BreakpointSite, we
set the thread's StopReason to be "breakpoint hit" (regardless if we've
actually hit the breakpoint, or if we've merely stopped *at* the
breakpoint instruction/point and haven't tripped it yet). And second,
when resuming a process, any thread sitting at a BreakpointSite is
silently stepped over the BreakpointSite -- because we've already
flagged the breakpoint hit when we stopped there originally.

In this patch, I change lldb to only set a thread's stop reason to
breakpoint-hit when we've actually executed the instruction/triggered
the breakpoint. When we resume, we only silently step past a
BreakpointSite that we've registered as hit. We preserve this state
across inferior function calls that the user may do while stopped, etc.

Also, when a user adds a new breakpoint at $pc while stopped, or changes
$pc to be the address of a BreakpointSite, we will silently step past
that breakpoint when the process resumes. This is purely a UX call, I
don't think there's any person who wants to set a breakpoint at $pc and
then hit it immediately on resuming.

One non-intuitive UX from this change, but I'm convinced it is
necessary: If you're stopped at a BreakpointSite that has not yet
executed, you `stepi`, you will hit the breakpoint and the pc will not
yet advance. This thread has not completed its stepi, and the thread
plan is still on the stack. If you then `continue` the thread, lldb will
now stop and say, "instruction step completed", one instruction past the
BreakpointSite. You can continue a second time to resume execution. I
discussed this with Jim, and trying to paper over this behavior will
lead to more complicated scenarios behaving non-intuitively. And mostly
it's the testsuite that was trying to instruction step past a breakpoint
and getting thrown off -- and I changed those tests to expect the new
behavior.

The bugs driving this change are all from lldb dropping the real stop
reason for a thread and setting it to breakpoint-hit when that was not
the case. Jim hit one where we have an aarch64 watchpoint that triggers
one instruction before a BreakpointSite. On this arch we are notified of
the watchpoint hit after the instruction has been unrolled -- we disable
the watchpoint, instruction step, re-enable the watchpoint and collect
the new value. But now we're on a BreakpointSite so the watchpoint-hit
stop reason is lost.

Another was reported by ZequanWu in
https://discourse.llvm.org/t/lldb-unable-to-break-at-start/78282 we
attach to/launch a process with the pc at a BreakpointSite and
misbehave. Caroline Tice mentioned it is also a problem they've had with
putting a breakpoint on _dl_debug_state.

The change to each Process plugin that does execution control is that

1. If we've stopped at a BreakpointSite that has not been executed yet,
we will call Thread::SetThreadStoppedAtUnexecutedBP(pc) to record
that.  When the thread resumes, if the pc is still at the same site, we
will continue, hit the breakpoint, and stop again.

2. When we've actually hit a breakpoint (enabled for this thread or not),
the Process plugin should call Thread::SetThreadHitBreakpointSite().
When we go to resume the thread, we will push a step-over-breakpoint
ThreadPlan before resuming.

The biggest set of changes is to StopInfoMachException where we
translate a Mach Exception into a stop reason. The Mach exception codes
differ in a few places depending on the target (unambiguously), and I
didn't want to duplicate the new code for each target so I've tested
what mach exceptions we get for each action on each target, and
reorganized StopInfoMachException::CreateStopReasonWithMachException to
document these possible values, and handle them without specializing
based on the target arch.

rdar://123942164
---
 lldb/include/lldb/Target/Thread.h |  24 ++
 .../Process/Utility/StopInfoMachException.cpp | 322 --
 .../Process/Windows/Common/ProcessWindows.cpp |  32 +-
 .../Process/gdb-remote/ProcessGDBRemote.cpp   |  93 ++---
 .../Process/scripted/ScriptedThread.cpp   |  11 +
 lldb/source/Target/StopInfo.cpp   |   2 +
 lldb/source/Target/Thread.cpp |  15 +-
 .../TestConsecutiveBreakpoints.py |  26 +-
 .../TestStepOverBreakpoint.py |   6 +-
 9 files changed, 266 insertions(+), 265 deletions(-)

diff --git a/lldb/include/lldb/Target/Thread.h 
b/lldb/include/lldb/Target/Thread.h
index 38b65b2bc58490..22d452c7b4b8a3 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -131,6 +131,7 @@ class Thread : public std::enable_shared_from_this,
 register

[Lldb-commits] [lldb] [lldb] Set the stop reason when receiving swbreak/hwbreak (PR #108518)

2024-09-13 Thread Jason Molenda via lldb-commits

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

xusheng added support for swbreak/hwbreak a month ago, and no special support 
was needed in ProcessGDBRemote when they're received because lldb already marks 
a thread as having hit a breakpoint when it stops at a breakpoint site.  
However, with changes I am working on, we need to know the real stop reason a 
thread stopped or the breakpoint hit will not be recognized.

This is similar to how lldb processes the "watch/rwatch/awatch" keys in a 
thread stop packet -- we set the `reason` to `watchpoint`, and these set it to 
`breakpoint` so we set the stop reason correctly later in these methods.

>From 2a35c25449b6f5b9a0e0821f3f51042688670acf Mon Sep 17 00:00:00 2001
From: Jason Molenda 
Date: Fri, 13 Sep 2024 01:52:00 -0700
Subject: [PATCH] [lldb] Set the stop reason when receiving swbreak/hwbreak

xusheng added support for swbreak/hwbreak a month ago, and no special
support was needed in ProcessGDBRemote when they're received because
lldb already marks a thread as having hit a breakpoint when it stops
at a breakpoint site.  However, with changes I am working on,
we need to know the real stop reason a thread stopped or the breakpoint
hit will not be recognized.

This is similar to how lldb processes the "watch/rwatch/awatch" keys in
a thread stop packet -- we set the `reason` to `watchpoint`, and these
set it to `breakpoint` so we set the stop reason correctly later in
these methods.
---
 lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 5eaf9ce2a302aa..271ff61a7188a6 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2317,6 +2317,8 @@ StateType 
ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
 StreamString ostr;
 ostr.Printf("%" PRIu64, wp_addr);
 description = std::string(ostr.GetString());
+  } else if (key.compare("swbreak") == 0 || key.compare("hwbreak") == 0) {
+reason = "breakpoint";
   } else if (key.compare("library") == 0) {
 auto error = LoadModules();
 if (error) {

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


[Lldb-commits] [lldb] [lldb] Set the stop reason when receiving swbreak/hwbreak (PR #108518)

2024-09-13 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jason Molenda (jasonmolenda)


Changes

xusheng added support for swbreak/hwbreak a month ago, and no special support 
was needed in ProcessGDBRemote when they're received because lldb already marks 
a thread as having hit a breakpoint when it stops at a breakpoint site.  
However, with changes I am working on, we need to know the real stop reason a 
thread stopped or the breakpoint hit will not be recognized.

This is similar to how lldb processes the "watch/rwatch/awatch" keys in a 
thread stop packet -- we set the `reason` to `watchpoint`, and these set it to 
`breakpoint` so we set the stop reason correctly later in these methods.

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


1 Files Affected:

- (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+2) 


``diff
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 5eaf9ce2a302aa..271ff61a7188a6 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2317,6 +2317,8 @@ StateType 
ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
 StreamString ostr;
 ostr.Printf("%" PRIu64, wp_addr);
 description = std::string(ostr.GetString());
+  } else if (key.compare("swbreak") == 0 || key.compare("hwbreak") == 0) {
+reason = "breakpoint";
   } else if (key.compare("library") == 0) {
 auto error = LoadModules();
 if (error) {

``




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


[Lldb-commits] [lldb] [lldb] Set the stop reason when receiving swbreak/hwbreak (PR #108518)

2024-09-13 Thread Jason Molenda via lldb-commits

jasonmolenda wrote:

This is the follow up patch I alluded to in 
https://github.com/llvm/llvm-project/pull/102873 needed for my work to re-land 
my change to how breakpoints are handled in lldb (work in progress at 
https://github.com/llvm/llvm-project/pull/108504 ).  It's the same thing we do 
for breakpoints just above this point, setting the `reason` variable to the 
actual stop reason so we set the thread's status correctly.  There's no 
difference in behavior right now, but it will be important when I land my 
changes, if someone communicates with a stub that sends swbreak/hwbreak.

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


[Lldb-commits] [lldb] [llvm] [lldb] Change lldb's breakpoint detection behavior [WIP] (PR #105594)

2024-09-13 Thread Jason Molenda via lldb-commits

https://github.com/jasonmolenda updated 
https://github.com/llvm/llvm-project/pull/105594

>From 56ca564185bdceea25162a1ce3b1e8c8fa2641e2 Mon Sep 17 00:00:00 2001
From: Jason Molenda 
Date: Fri, 19 Jul 2024 17:26:13 -0700
Subject: [PATCH 1/7] [lldb] Change lldb's breakpoint handling behavior
 (#96260)

lldb today has two rules: When a thread stops at a BreakpointSite, we
set the thread's StopReason to be "breakpoint hit" (regardless if we've
actually hit the breakpoint, or if we've merely stopped *at* the
breakpoint instruction/point and haven't tripped it yet). And second,
when resuming a process, any thread sitting at a BreakpointSite is
silently stepped over the BreakpointSite -- because we've already
flagged the breakpoint hit when we stopped there originally.

In this patch, I change lldb to only set a thread's stop reason to
breakpoint-hit when we've actually executed the instruction/triggered
the breakpoint. When we resume, we only silently step past a
BreakpointSite that we've registered as hit. We preserve this state
across inferior function calls that the user may do while stopped, etc.

Also, when a user adds a new breakpoint at $pc while stopped, or changes
$pc to be the address of a BreakpointSite, we will silently step past
that breakpoint when the process resumes. This is purely a UX call, I
don't think there's any person who wants to set a breakpoint at $pc and
then hit it immediately on resuming.

One non-intuitive UX from this change, but I'm convinced it is
necessary: If you're stopped at a BreakpointSite that has not yet
executed, you `stepi`, you will hit the breakpoint and the pc will not
yet advance. This thread has not completed its stepi, and the thread
plan is still on the stack. If you then `continue` the thread, lldb will
now stop and say, "instruction step completed", one instruction past the
BreakpointSite. You can continue a second time to resume execution. I
discussed this with Jim, and trying to paper over this behavior will
lead to more complicated scenarios behaving non-intuitively. And mostly
it's the testsuite that was trying to instruction step past a breakpoint
and getting thrown off -- and I changed those tests to expect the new
behavior.

The bugs driving this change are all from lldb dropping the real stop
reason for a thread and setting it to breakpoint-hit when that was not
the case. Jim hit one where we have an aarch64 watchpoint that triggers
one instruction before a BreakpointSite. On this arch we are notified of
the watchpoint hit after the instruction has been unrolled -- we disable
the watchpoint, instruction step, re-enable the watchpoint and collect
the new value. But now we're on a BreakpointSite so the watchpoint-hit
stop reason is lost.

Another was reported by ZequanWu in
https://discourse.llvm.org/t/lldb-unable-to-break-at-start/78282 we
attach to/launch a process with the pc at a BreakpointSite and
misbehave. Caroline Tice mentioned it is also a problem they've had with
putting a breakpoint on _dl_debug_state.

The change to each Process plugin that does execution control is that

1. If we've stopped at a BreakpointSite that has not been executed yet,
we will call Thread::SetThreadStoppedAtUnexecutedBP(pc) to record
that.  When the thread resumes, if the pc is still at the same site, we
will continue, hit the breakpoint, and stop again.

2. When we've actually hit a breakpoint (enabled for this thread or not),
the Process plugin should call Thread::SetThreadHitBreakpointSite().
When we go to resume the thread, we will push a step-over-breakpoint
ThreadPlan before resuming.

The biggest set of changes is to StopInfoMachException where we
translate a Mach Exception into a stop reason. The Mach exception codes
differ in a few places depending on the target (unambiguously), and I
didn't want to duplicate the new code for each target so I've tested
what mach exceptions we get for each action on each target, and
reorganized StopInfoMachException::CreateStopReasonWithMachException to
document these possible values, and handle them without specializing
based on the target arch.

rdar://123942164
---
 lldb/include/lldb/Target/Thread.h |  24 ++
 .../Process/Utility/StopInfoMachException.cpp | 322 --
 .../Process/Windows/Common/ProcessWindows.cpp |  32 +-
 .../Process/gdb-remote/ProcessGDBRemote.cpp   |  93 ++---
 .../Process/scripted/ScriptedThread.cpp   |  11 +
 lldb/source/Target/StopInfo.cpp   |   2 +
 lldb/source/Target/Thread.cpp |  15 +-
 .../TestConsecutiveBreakpoints.py |  26 +-
 .../TestStepOverBreakpoint.py |   6 +-
 9 files changed, 266 insertions(+), 265 deletions(-)

diff --git a/lldb/include/lldb/Target/Thread.h 
b/lldb/include/lldb/Target/Thread.h
index 38b65b2bc58490..22d452c7b4b8a3 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -131,6 +131,7 @@ class Thread : public std::enable_shared_from_this,
 register

[Lldb-commits] [lldb] [lldb] Make sure TestDAP_subtleFrames actually uses libc++ (PR #108227)

2024-09-13 Thread Pavel Labath via lldb-commits

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


[Lldb-commits] [lldb] 069b841 - [lldb] Make sure TestDAP_subtleFrames actually uses libc++ (#108227)

2024-09-13 Thread via lldb-commits

Author: Pavel Labath
Date: 2024-09-13T11:18:05+02:00
New Revision: 069b841c2ecde39c65ca74660b681d4d25a4bbb2

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

LOG: [lldb] Make sure TestDAP_subtleFrames actually uses libc++ (#108227)

Without this, the binary would still be built with libstdc++, if that's
the system/compiler default.

Added: 


Modified: 
lldb/test/API/tools/lldb-dap/stackTrace/subtleFrames/Makefile

Removed: 




diff  --git a/lldb/test/API/tools/lldb-dap/stackTrace/subtleFrames/Makefile 
b/lldb/test/API/tools/lldb-dap/stackTrace/subtleFrames/Makefile
index 8b20bcb050..ab034edd121f9f 100644
--- a/lldb/test/API/tools/lldb-dap/stackTrace/subtleFrames/Makefile
+++ b/lldb/test/API/tools/lldb-dap/stackTrace/subtleFrames/Makefile
@@ -1,3 +1,4 @@
 CXX_SOURCES := main.cpp
+USE_LIBCPP := 1
 
 include Makefile.rules



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


[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)

2024-09-13 Thread Pavel Labath via lldb-commits

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


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


[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)

2024-09-13 Thread Pavel Labath via lldb-commits


@@ -94,25 +126,36 @@ Status Status::FromErrorStringWithFormat(const char 
*format, ...) {
   return Status(string);
 }
 
-Status Status::FromError(llvm::Error error) { return Status(std::move(error)); 
}
+Status Status::FromExpressionError(lldb::ExpressionResults result,
+   std::string msg) {
+  return Status(llvm::make_error(
+  std::error_code(result, expression_category()), msg));
+}
 
-llvm::Error Status::ToError() const {
-  if (Success())
+/// Creates a deep copy of all known errors and converts all other
+/// errors to a new llvm::StringError.
+static llvm::Error CloneError(const llvm::Error &error) {
+  std::vector> info;
+  llvm::visitErrors(error, [&](const llvm::ErrorInfoBase &error) {
+if (error.isA())
+  info.push_back(static_cast(&error)->Clone());

labath wrote:

btw `static_cast` is also a thing.

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


[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)

2024-09-13 Thread Pavel Labath via lldb-commits


@@ -94,26 +123,49 @@ Status Status::FromErrorStringWithFormat(const char 
*format, ...) {
   return Status(string);
 }
 
-Status Status::FromError(llvm::Error error) { return Status(std::move(error)); 
}
+Status Status::FromExpressionError(lldb::ExpressionResults result,
+   std::string msg) {
+  return Status(llvm::make_error(
+  std::error_code(result, expression_category()), msg));
+}
 
-llvm::Error Status::ToError() const {
-  if (Success())
+/// Creates a deep copy of all known errors and converts all other
+/// errors to a new llvm::StringError.
+static llvm::Error CloneError(llvm::Error &error) {
+  std::vector> info;

labath wrote:

How about this:
```
  Error result = ErrorSuccess();
  auto clone = [](const ErrorInfoBase &e) {
if (e.isA())
  return Error(static_cast(e)->Clone());
return make_error(e.message(), e.convertToErrorCode(), true);
  };
  visitErrors(r, [&](const ErrorInfoBase &e) {
result = joinErrors(std::move(result), clone(e));
  });
  return result;
```

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


[Lldb-commits] [lldb] [lldb] Deflake TestDAP_attach (PR #108226)

2024-09-13 Thread Pavel Labath via lldb-commits

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


[Lldb-commits] [lldb] e054712 - [lldb] Deflake TestDAP_attach (#108226)

2024-09-13 Thread via lldb-commits

Author: Pavel Labath
Date: 2024-09-13T11:21:50+02:00
New Revision: e054712a85f924e0afe7f180fd960be7a8214d64

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

LOG: [lldb] Deflake TestDAP_attach (#108226)

The test failed in
 due to frame
variable not being in stop commands, even though the DAP log shows the
command being present there. I'm pretty sure this is a race in the test
the collection of the test log. I fix that by making sure we wait for
the expected output, and also by increasing the timeout (1s is cutting
it very close).

The arm failure link is no longer functional, but I'm fairly certain
that this was the cause of those flakes as well.

Added: 


Modified: 
lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py

Removed: 




diff  --git a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py 
b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py
index 2df191cf0ae715..e143c2798b209a 100644
--- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py
+++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py
@@ -116,9 +116,6 @@ def test_by_name_waitFor(self):
 
 @skipIfDarwin
 @skipIfNetBSD  # Hangs on NetBSD as well
-@skipIf(
-archs=["arm", "aarch64"]
-)  # Example of a flaky run 
http://lab.llvm.org:8011/builders/lldb-aarch64-ubuntu/builds/5527/steps/test/logs/stdio
 def test_commands(self):
 """
 Tests the "initCommands", "preRunCommands", "stopCommands",
@@ -152,7 +149,7 @@ def test_commands(self):
 initCommands = ["target list", "platform list"]
 preRunCommands = ["image list a.out", "image dump sections a.out"]
 postRunCommands = ["help trace", "help process trace"]
-stopCommands = ["frame variable", "bt"]
+stopCommands = ["frame variable", "thread backtrace"]
 exitCommands = ["expr 2+3", "expr 3+4"]
 terminateCommands = ["expr 4+2"]
 self.attach(
@@ -179,7 +176,7 @@ def test_commands(self):
 breakpoint_ids = self.set_function_breakpoints(functions)
 self.assertEqual(len(breakpoint_ids), len(functions), "expect one 
breakpoint")
 self.continue_to_breakpoints(breakpoint_ids)
-output = self.get_console(timeout=1.0)
+output = self.collect_console(timeout_secs=10, 
pattern=stopCommands[-1])
 self.verify_commands("stopCommands", output, stopCommands)
 
 # Continue after launch and hit the "pause()" call and stop the target.
@@ -189,7 +186,7 @@ def test_commands(self):
 time.sleep(0.5)
 self.dap_server.request_pause()
 self.dap_server.wait_for_stopped()
-output = self.get_console(timeout=1.0)
+output = self.collect_console(timeout_secs=10, 
pattern=stopCommands[-1])
 self.verify_commands("stopCommands", output, stopCommands)
 
 # Continue until the program exits
@@ -198,7 +195,7 @@ def test_commands(self):
 # "exitCommands" that were run after the second breakpoint was hit
 # and the "terminateCommands" due to the debugging session ending
 output = self.collect_console(
-timeout_secs=1.0,
+timeout_secs=10.0,
 pattern=terminateCommands[0],
 )
 self.verify_commands("exitCommands", output, exitCommands)



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


[Lldb-commits] [lldb] [lldb][test] TestDataFormatterLibcxxStringSimulator.py: fix padding for current layout (PR #108362)

2024-09-13 Thread Pavel Labath via lldb-commits

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


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


[Lldb-commits] [lldb] 4ca8fb1 - [lldb][test] TestDataFormatterLibcxxStringSimulator.py: fix padding for current layout (#108362)

2024-09-13 Thread via lldb-commits

Author: Michael Buch
Date: 2024-09-13T11:01:25+01:00
New Revision: 4ca8fb18129e6465c3594a8681f1cca0e2aff724

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

LOG: [lldb][test] TestDataFormatterLibcxxStringSimulator.py: fix padding for 
current layout (#108362)

IIUC, the history of `std::string`'s `__short` structure in the
alternate ABI layout (as recorded by the simulator test) looks as
follows:
* First layout ( `SUBCLASS_PADDING` is defined):
```
 struct __short  
 {   
 value_type __data_[__min_cap];  
struct  
: __padding 
{   
unsigned char __size_;  
};
 };  
```
* Then:
```
 struct __short 
 {  
value_type __data_[__min_cap];  
 
unsigned char __padding[sizeof(value_type) - 1];   
unsigned char __size_; 
 };
```
* Then, post-`BITMASKS`:
```
 struct __short 
 {  
value_type __data_[__min_cap];  
 
unsigned char __padding[sizeof(value_type) - 1];   
unsigned char __size_ : 7;
unsigned char __is_long_ : 1; 
 };
```

Which is the one that's [on
top-of-tree](https://github.com/llvm/llvm-project/blob/89c10e27d8b4d5f44998aad9abd2590d9f96c5df/libcxx/include/string#L854-L859).

But for `REVISION > 1`, `BITMASKS` is never set, so for those tests we
lose the `__padding` member.

This patch fixes this by splitting out the `SUBCLASS_PADDING` out of the
ifdef.

Drive-by:
* Also run expression evaluator on the string to provide is with some
extra coverage.

Added: 


Modified: 

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp

Removed: 




diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py
index 3e5c493692c4f0..98d2c7320003e4 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py
@@ -22,6 +22,9 @@ def _run_test(self, defines):
 self.expect_var_path("shortstring", summary='"short"')
 self.expect_var_path("longstring", summary='"I am a very long string"')
 
+self.expect_expr("shortstring", result_summary='"short"')
+self.expect_expr("longstring", result_summary='"I am a very long 
string"')
+
 
 for v in [None, "ALTERNATE_LAYOUT"]:
 for r in range(5):

diff  --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp
index 7beeb9c39de49e..b010dc25f8f804 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp
@@ -71,19 +71,20 @@ template  
class basic_string {
 
   struct __short {
 value_type __data_[__min_cap];
-#ifdef BITMASKS
 #ifdef SUBCLASS_PADDING
 struct : __padding {
   unsigned char __size_;
 };
-#else
+#else // !SUBCLASS_PADDING
+
 unsigned char __padding[sizeof(value_type) - 1];
+#ifdef BITMASKS
 unsigned char __size_;
-#endif
 #else // !BITMASKS
 unsigned char __size_ : 7;
 unsigned char __is_long_ : 1;
-#endif
+#endif // BITMASKS
+#endif // SUBCLASS_PADDING
   };
 
 #ifdef BITMASKS



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


[Lldb-commits] [lldb] [lldb][test] TestDataFormatterLibcxxStringSimulator.py: fix padding for current layout (PR #108362)

2024-09-13 Thread Michael Buch via lldb-commits

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


[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)

2024-09-13 Thread Pavel Labath via lldb-commits


@@ -1376,6 +1382,16 @@ void request_evaluate(const llvm::json::Object &request) 
{
 EmplaceSafeString(body, "result", result);
 body.try_emplace("variablesReference", (int64_t)0);
   } else {
+if (context != "hover") {

labath wrote:

Should the `!=hover` be `==repl` instead? I don't think we want to repeat the 
expression s from the watch window..

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


[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)

2024-09-13 Thread Pavel Labath via lldb-commits


@@ -45,7 +45,8 @@ bool RunLLDBCommands(llvm::StringRef prefix,
   // RunTerminateCommands.
   static std::mutex handle_command_mutex;
   std::lock_guard locker(handle_command_mutex);
-  interp.HandleCommand(command.str().c_str(), result);
+  interp.HandleCommand(command.str().c_str(), result,
+   /* add_to_history */ true);

labath wrote:

The recommended (but not very strictly enforced) formatting is 
`/*add_to_history=*/` (equal sign, no spaces).

That said, how does this relate to the rest of the patch?

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


[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)

2024-09-13 Thread Pavel Labath via lldb-commits


@@ -1364,8 +1364,14 @@ void request_evaluate(const llvm::json::Object &request) 
{
   std::string expression = GetString(arguments, "expression").str();
   llvm::StringRef context = GetString(arguments, "context");
 
-  if (context == "repl" && g_dap.DetectExpressionContext(frame, expression) ==
-   ExpressionContext::Command) {
+  if (context == "repl" &&
+  ((!expression.empty() &&
+   g_dap.DetectExpressionContext(frame, expression) ==
+   ExpressionContext::Command) ||
+   (expression.empty() && g_dap.last_nonempty_var_expression.empty( {

labath wrote:

```suggestion
  (expression.empty() ? g_dap.last_nonempty_var_expression.empty() :
g_dap.DetectExpressionContext(frame, expression) ==
   ExpressionContext::Command)) {
```

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


[Lldb-commits] [lldb] [lldb][test] TestDataFormatterLibcxxStringSimulator.py: add new padding layout (PR #108375)

2024-09-13 Thread Michael Buch via lldb-commits


@@ -77,7 +89,12 @@ template  
class basic_string {
 };
 #else // !SUBCLASS_PADDING
 
+#ifdef UB_PADDING
 unsigned char __padding[sizeof(value_type) - 1];

Michael137 wrote:

Good point! I'll rename it to `NON_STANDARD_PADDING`

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


[Lldb-commits] [lldb] [lldb][test] TestDataFormatterLibcxxStringSimulator.py: add new padding layout (PR #108375)

2024-09-13 Thread Michael Buch via lldb-commits

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

>From 7932edadf5726b2c73911d1316112af36ab7f044 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Thu, 12 Sep 2024 12:55:17 +0100
Subject: [PATCH 1/2] [lldb][test] TestDataFormatterLibcxxStringSimulator.py:
 add new padding layout

Depends on https://github.com/llvm/llvm-project/pull/108362.

Adds new layout for https://github.com/llvm/llvm-project/pull/105865.
---
 .../TestDataFormatterLibcxxStringSimulator.py |  2 +-
 .../libcxx-simulators/string/main.cpp | 34 +++
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py
index 98d2c7320003e4..dd9f5de22c9057 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/TestDataFormatterLibcxxStringSimulator.py
@@ -27,7 +27,7 @@ def _run_test(self, defines):
 
 
 for v in [None, "ALTERNATE_LAYOUT"]:
-for r in range(5):
+for r in range(6):
 name = "test_r%d" % r
 defines = ["REVISION=%d" % r]
 if v:
diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp
index b010dc25f8f804..5cf3a85007fb64 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp
@@ -20,7 +20,11 @@
 // Pre-D128285 layout.
 #define PACKED_ANON_STRUCT
 #endif
-// REVISION == 4: current layout
+#if REVISION <= 4
+// Pre-TODO layout.
+#define UB_PADDING
+#endif
+// REVISION == 5: current layout
 
 #ifdef PACKED_ANON_STRUCT
 #define BEGIN_PACKED_ANON_STRUCT struct __attribute__((packed)) {
@@ -34,6 +38,7 @@
 namespace std {
 namespace __lldb {
 
+#ifdef UB_PADDING
 #if defined(ALTERNATE_LAYOUT) && defined(SUBCLASS_PADDING)
 template  struct __padding {
   unsigned char __xx[sizeof(_CharT) - 1];
@@ -41,6 +46,13 @@ template  struct 
__padding {
 
 template  struct __padding<_CharT, 1> {};
 #endif
+#else // !UB_PADDING
+template  struct __padding {
+  char __padding_[_PaddingSize];
+};
+
+template <> struct __padding<0> {};
+#endif
 
 template  class basic_string {
 public:
@@ -77,7 +89,12 @@ template  
class basic_string {
 };
 #else // !SUBCLASS_PADDING
 
+#ifdef UB_PADDING
 unsigned char __padding[sizeof(value_type) - 1];
+#else
+[[no_unique_address]] __padding __padding_;
+#endif
+
 #ifdef BITMASKS
 unsigned char __size_;
 #else // !BITMASKS
@@ -129,21 +146,26 @@ template  
class basic_string {
 union {
 #ifdef BITMASKS
   unsigned char __size_;
-#else
+#else  // !BITMASKS
   struct {
 unsigned char __is_long_ : 1;
 unsigned char __size_ : 7;
   };
-#endif
+#endif // BITMASKS
   value_type __lx;
 };
-#else
+#else  // !SHORT_UNION
 BEGIN_PACKED_ANON_STRUCT
 unsigned char __is_long_ : 1;
 unsigned char __size_ : 7;
 END_PACKED_ANON_STRUCT
-char __padding_[sizeof(value_type) - 1];
-#endif
+#ifdef UB_PADDING
+unsigned char __padding[sizeof(value_type) - 1];
+#else  // !UB_PADDING
+[[no_unique_address]] __padding __padding_;
+#endif // UB_PADDING
+
+#endif // SHORT_UNION
 value_type __data_[__min_cap];
   };
 

>From d3d78e4a2582fe8f572969b694675f90184894fa Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 13 Sep 2024 11:16:52 +0100
Subject: [PATCH 2/2] fixup! rename ifdef

---
 .../libcxx-simulators/string/main.cpp  | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp
 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp
index 5cf3a85007fb64..944d90ab187ba4 100644
--- 
a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp
+++ 
b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/string/main.cpp
@@ -22,7 +22,7 @@
 #endif
 #if REVISION <= 4
 // Pre-TODO layout.
-#define UB_PADDING
+#define NON_STANDARD_PADDING
 #endif
 // REVISION == 5: current layout
 
@@ -38,7 +38,7 @@
 namespace std {
 namespace __lldb {
 
-#ifdef UB_PADDING
+#ifdef NON_STANDARD_PADDING
 #if defined(ALTERNATE_LAYOUT) && defined(SUBCLASS_PADDING)
 template  struct __padding {
   unsigned char __xx[sizeof(_CharT) - 1];
@@ -46,7 +46,7 @@ template  struct 
__padding {
 
 template  s

[Lldb-commits] [lldb] [lldb] Add pc check for thread-step-by-bp algorithms (PR #108504)

2024-09-13 Thread Pavel Labath via lldb-commits

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


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


[Lldb-commits] [lldb] [lldb] Introduce an always-on system log category/channel (PR #108495)

2024-09-13 Thread Pavel Labath via lldb-commits

labath wrote:

My first thought when seeing (before opening the phabricator patch) this was 
"wouldn't this be better off as a separate API". After opening the link, I saw 
that this was the last comment on it as well. So, what has changed since then?

My think is that, our logging API is a fairly complex piece of machinery, and I 
think this is largely due to the fact that we're allowing the user to customize 
it (change the target, verbosity, etc.). For the this log channel, you 
basically take away all of that, so what's the point in going through all of 
that? Why not just have a single `SystemLog` function that does what you want?

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


[Lldb-commits] [lldb] [lldb] Add pc check for thread-step-by-bp algorithms (PR #108504)

2024-09-13 Thread David Spickett via lldb-commits

DavidSpickett wrote:

> "if a thread stops at a breakpoint site, we set the thread's stop reason to 
> breakpoint-hit, even if the breakpoint hasn't been executed"

What does it look like for a thread to be stopped at a *breakpoint site* but 
not to have executed the breakpoint?

Is this just a timing thing, if one thread executes the breakpoint and that 
causes all others to stop, and one of those others happens to be about to 
execute the same breakpoint?

```
T0:
  nop
  brk << $pc, hit the breakpoint and generated the debug exception

T1:
  nop
  brk << also $pc but didn't execute it yet
```


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


[Lldb-commits] [lldb] [lldb] Set the stop reason when receiving swbreak/hwbreak (PR #108518)

2024-09-13 Thread David Spickett via lldb-commits

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

Still curious exactly how we end up with threads on a breakpoint that didn't 
execute it - 
https://github.com/llvm/llvm-project/pull/108504#issuecomment-2348626494

But aside from that this LGTM.

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


[Lldb-commits] [lldb] [lldb] Add a MainLoop version of DomainSocket::Accept (PR #108188)

2024-09-13 Thread Pavel Labath via lldb-commits


@@ -85,6 +85,43 @@ TEST_P(SocketTest, DomainListenConnectAccept) {
   std::unique_ptr socket_b_up;
   CreateDomainConnectedSockets(Path, &socket_a_up, &socket_b_up);
 }
+
+TEST_P(SocketTest, DomainMainLoopAccept) {

labath wrote:

It's already in a ifdef block, next to the other domain socket test. That's why 
you don't see an ifdef in the default view.

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


[Lldb-commits] [lldb] ebbc9ed - [lldb] Add a MainLoop version of DomainSocket::Accept (#108188)

2024-09-13 Thread via lldb-commits

Author: Pavel Labath
Date: 2024-09-13T12:56:52+02:00
New Revision: ebbc9ed2d60cacffc87232dc32374a2b38b92175

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

LOG: [lldb] Add a MainLoop version of DomainSocket::Accept (#108188)

To go along with the existing TCPSocket implementation.

Added: 


Modified: 
lldb/include/lldb/Host/Socket.h
lldb/include/lldb/Host/common/TCPSocket.h
lldb/include/lldb/Host/common/UDPSocket.h
lldb/include/lldb/Host/posix/DomainSocket.h
lldb/source/Host/common/Socket.cpp
lldb/source/Host/common/TCPSocket.cpp
lldb/source/Host/common/UDPSocket.cpp
lldb/source/Host/posix/DomainSocket.cpp
lldb/unittests/Host/SocketTest.cpp

Removed: 




diff  --git a/lldb/include/lldb/Host/Socket.h b/lldb/include/lldb/Host/Socket.h
index 764a048976eb41..14468c98ac5a3a 100644
--- a/lldb/include/lldb/Host/Socket.h
+++ b/lldb/include/lldb/Host/Socket.h
@@ -12,6 +12,7 @@
 #include 
 #include 
 
+#include "lldb/Host/MainLoopBase.h"
 #include "lldb/lldb-private.h"
 
 #include "lldb/Host/SocketAddress.h"
@@ -97,7 +98,17 @@ class Socket : public IOObject {
 
   virtual Status Connect(llvm::StringRef name) = 0;
   virtual Status Listen(llvm::StringRef name, int backlog) = 0;
-  virtual Status Accept(Socket *&socket) = 0;
+
+  // Use the provided main loop instance to accept new connections. The 
callback
+  // will be called (from MainLoop::Run) for each new connection. This function
+  // does not block.
+  virtual llvm::Expected>
+  Accept(MainLoopBase &loop,
+ std::function socket)> sock_cb) = 0;
+
+  // Accept a single connection and "return" it in the pointer argument. This
+  // function blocks until the connection arrives.
+  virtual Status Accept(Socket *&socket);
 
   // Initialize a Tcp Socket object in listening mode.  listen and accept are
   // implemented separately because the caller may wish to manipulate or query

diff  --git a/lldb/include/lldb/Host/common/TCPSocket.h 
b/lldb/include/lldb/Host/common/TCPSocket.h
index 78e80568e39967..eefe0240fe4a95 100644
--- a/lldb/include/lldb/Host/common/TCPSocket.h
+++ b/lldb/include/lldb/Host/common/TCPSocket.h
@@ -42,16 +42,10 @@ class TCPSocket : public Socket {
   Status Connect(llvm::StringRef name) override;
   Status Listen(llvm::StringRef name, int backlog) override;
 
-  // Use the provided main loop instance to accept new connections. The 
callback
-  // will be called (from MainLoop::Run) for each new connection. This function
-  // does not block.
+  using Socket::Accept;
   llvm::Expected>
   Accept(MainLoopBase &loop,
- std::function socket)> sock_cb);
-
-  // Accept a single connection and "return" it in the pointer argument. This
-  // function blocks until the connection arrives.
-  Status Accept(Socket *&conn_socket) override;
+ std::function socket)> sock_cb) override;
 
   Status CreateSocket(int domain);
 

diff  --git a/lldb/include/lldb/Host/common/UDPSocket.h 
b/lldb/include/lldb/Host/common/UDPSocket.h
index bae707e345d87c..7348010d02ada2 100644
--- a/lldb/include/lldb/Host/common/UDPSocket.h
+++ b/lldb/include/lldb/Host/common/UDPSocket.h
@@ -27,7 +27,13 @@ class UDPSocket : public Socket {
   size_t Send(const void *buf, const size_t num_bytes) override;
   Status Connect(llvm::StringRef name) override;
   Status Listen(llvm::StringRef name, int backlog) override;
-  Status Accept(Socket *&socket) override;
+
+  llvm::Expected>
+  Accept(MainLoopBase &loop,
+ std::function socket)> sock_cb) override 
{
+return llvm::errorCodeToError(
+std::make_error_code(std::errc::operation_not_supported));
+  }
 
   SocketAddress m_sockaddr;
 };

diff  --git a/lldb/include/lldb/Host/posix/DomainSocket.h 
b/lldb/include/lldb/Host/posix/DomainSocket.h
index 35c33811f60de6..983f43bd633719 100644
--- a/lldb/include/lldb/Host/posix/DomainSocket.h
+++ b/lldb/include/lldb/Host/posix/DomainSocket.h
@@ -14,11 +14,17 @@
 namespace lldb_private {
 class DomainSocket : public Socket {
 public:
+  DomainSocket(NativeSocket socket, bool should_close,
+   bool child_processes_inherit);
   DomainSocket(bool should_close, bool child_processes_inherit);
 
   Status Connect(llvm::StringRef name) override;
   Status Listen(llvm::StringRef name, int backlog) override;
-  Status Accept(Socket *&socket) override;
+
+  using Socket::Accept;
+  llvm::Expected>
+  Accept(MainLoopBase &loop,
+ std::function socket)> sock_cb) override;
 
   std::string GetRemoteConnectionURI() const override;
 

diff  --git a/lldb/source/Host/common/Socket.cpp 
b/lldb/source/Host/common/Socket.cpp
index 1a63571b94c6f1..d69eb608204033 100644
--- a/lldb/source/Host/common/Socket.cpp
+++ b/lldb/source/Host/common/Socket.cpp
@@ -10,6 +10,7 @@
 
 #include "lldb/Host/Co

[Lldb-commits] [lldb] [lldb] Add a MainLoop version of DomainSocket::Accept (PR #108188)

2024-09-13 Thread Pavel Labath via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Add a MainLoop version of DomainSocket::Accept (PR #108188)

2024-09-13 Thread LLVM Continuous Integration via lldb-commits

llvm-ci wrote:

LLVM Buildbot has detected a new failure on builder `lldb-x86_64-debian` 
running on `lldb-x86_64-debian` while building `lldb` at step 6 "test".

Full details are available at: 
https://lab.llvm.org/buildbot/#/builders/162/builds/6215


Here is the relevant piece of the build log for the reference

```
Step 6 (test) failure: build (failure)
...
PASS: lldb-api :: 
functionalities/breakpoint/breakpoint_callback_command_source/TestBreakpointCallbackCommandSource.py
 (21 of 2682)
PASS: lldb-api :: iohandler/stdio/TestIOHandlerProcessSTDIO.py (22 of 2682)
PASS: lldb-api :: iohandler/completion/TestIOHandlerCompletion.py (23 of 2682)
PASS: lldb-api :: tools/lldb-server/TestGdbRemoteThreadsInStopReply.py (24 of 
2682)
PASS: lldb-api :: iohandler/sigint/TestProcessIOHandlerInterrupt.py (25 of 2682)
PASS: lldb-api :: 
commands/expression/multiline-completion/TestMultilineCompletion.py (26 of 2682)
PASS: lldb-api :: tools/lldb-server/TestGdbRemoteExpeditedRegisters.py (27 of 
2682)
PASS: lldb-api :: tools/lldb-server/vCont-threads/TestSignal.py (28 of 2682)
PASS: lldb-api :: tools/lldb-dap/variables/TestDAP_variables.py (29 of 2682)
PASS: lldb-api :: functionalities/load_unload/TestLoadUnload.py (30 of 2682)
FAIL: lldb-api :: tools/lldb-dap/launch/TestDAP_launch.py (31 of 2682)
 TEST 'lldb-api :: tools/lldb-dap/launch/TestDAP_launch.py' 
FAILED 
Script:
--
/usr/bin/python3 
/home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/dotest.py -u 
CXXFLAGS -u CFLAGS --env ARCHIVER=/usr/bin/ar --env OBJCOPY=/usr/bin/objcopy 
--env LLVM_LIBS_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/./lib --env 
LLVM_INCLUDE_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/include --env 
LLVM_TOOLS_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/./bin --arch x86_64 
--build-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex 
--lldb-module-cache-dir 
/home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-lldb/lldb-api
 --clang-module-cache-dir 
/home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-clang/lldb-api
 --executable /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/lldb --compiler 
/home/worker/2.0.1/lldb-x86_64-debian/build/./bin/clang --dsymutil 
/home/worker/2.0.1/lldb-x86_64-debian/build/./bin/dsymutil --llvm-tools-dir 
/home/worker/2.0.1/lldb-x86_64-debian/build/./bin --lldb-obj-root 
/home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb --lldb-libs-dir 
/home/worker/2.0.1/lldb-x86_64-debian/build/./lib -t 
/home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/tools/lldb-dap/launch
 -p TestDAP_launch.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 20.0.0git (https://github.com/llvm/llvm-project.git revision 
ebbc9ed2d60cacffc87232dc32374a2b38b92175)
  clang revision ebbc9ed2d60cacffc87232dc32374a2b38b92175
  llvm revision ebbc9ed2d60cacffc87232dc32374a2b38b92175
Skipping the following test categories: ['libc++', 'dsym', 'gmodules', 
'debugserver', 'objc']
= DEBUG ADAPTER PROTOCOL LOGS =
1726225186.522618532 --> 
Content-Length: 344

{
  "arguments": {
"adapterID": "lldb-native",
"clientID": "vscode",
"columnsStartAt1": true,
"linesStartAt1": true,
"locale": "en-us",
"pathFormat": "path",
"sourceInitFile": false,
"supportsRunInTerminalRequest": true,
"supportsStartDebuggingRequest": true,
"supportsVariablePaging": true,
"supportsVariableType": true
  },
  "command": "initialize",
  "seq": 1,
  "type": "request"
}
1726225186.524738789 <-- 
Content-Length: 1556


```



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


[Lldb-commits] [lldb] [lldb] Add pc check for thread-step-by-bp algorithms (PR #108504)

2024-09-13 Thread Pavel Labath via lldb-commits

labath wrote:

It can happen through timing, but also in single-threaded code as well. A 
thread could reach-but-not execute a breakpoint insn due to some activity on 
the previous insn. E.g. it could be single-stepping over it (so the stop reason 
is "i've completed a step", not "I've hit a breakpoint"), or (on x86, where 
watchpoint hits are reported after the fact), the previous insn could have 
triggered a watchpoint.

None of this really matters for this patch though. The problem here was that 
the lldb-server implementation has assuming that the stop reason of a 
software-single-stepping thread *must* be "i've completed a step" , but that 
won't be the case if the instruction you are stepping over is actually a 
breakpoint insn -- in that case, the reason will be "i've hit a breakpoint 
(other than the temporary bkpt i've set to single-step)". It just so happens 
that without that Jason's patch, lldb would never actually try to step over a 
breakpoint insn it knew of, so we'd never hit this problem.

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


[Lldb-commits] [lldb] [llvm] [lldb][RISCV] function calls support in lldb expressions (PR #99336)

2024-09-13 Thread via lldb-commits


@@ -471,6 +498,18 @@ static void SetupTargetOpts(CompilerInstance &compiler,
   // Set the target ABI
   if (std::string abi = GetClangTargetABI(target_arch); !abi.empty())
 compiler.getTargetOpts().ABI = std::move(abi);
+
+  if ((target_machine == llvm::Triple::riscv64 &&
+   compiler.getTargetOpts().ABI == "lp64f") ||
+  (target_machine == llvm::Triple::riscv32 &&
+   compiler.getTargetOpts().ABI == "ilp32f"))
+compiler->getTargetOpts().FeaturesAsWritten.emplace_back("+f");
+
+  if ((target_machine == llvm::Triple::riscv64 &&
+   compiler.getTargetOpts().ABI == "lp64d") ||
+  (target_machine == llvm::Triple::riscv32 &&
+   compiler.getTargetOpts().ABI == "ilp32d"))
+compiler->getTargetOpts().FeaturesAsWritten.emplace_back("+d");

dlav-sc wrote:

> What happens if we just unconditionally add +d and +f? My thinking is that we 
> could maybe just enable these features for all RISCV targets (like we do with 
> x86 sse?) 

To be honest I don't know exactly, but I think it is a bad idea, because some 
RISCV targets may not support float/double instructions (F/D extension), while 
MCJIT will create hard-float instructions.

> I'm not super familiar with RISCV but are ilp32d/ilp32f/ilp64f/ilp32f all the 
> supported ABI strings?

Nope, there is ilp32/lp64 for example, that don't support doubles instructions, 
so the first problem was that MCJIT used ilp32/lp64 ABIs by default, so there 
weren't any float or double instructions in the resulting assembly. Now I set 
ABI according to the one in the debuggee binary.

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


[Lldb-commits] [lldb] [llvm] [lldb][RISCV] function calls support in lldb expressions (PR #99336)

2024-09-13 Thread via lldb-commits

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


[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)

2024-09-13 Thread Walter Erquinigo via lldb-commits


@@ -1376,6 +1382,16 @@ void request_evaluate(const llvm::json::Object &request) 
{
 EmplaceSafeString(body, "result", result);
 body.try_emplace("variablesReference", (int64_t)0);
   } else {
+if (context != "hover") {

walter-erquinigo wrote:

yeah, this feature should be limited to the repl Debug Console

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


[Lldb-commits] [lldb] Make env and source map dictionaries #95137 (PR #106919)

2024-09-13 Thread Walter Erquinigo via lldb-commits

https://github.com/walter-erquinigo approved this pull request.


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


[Lldb-commits] [lldb] [llvm] [lldb][RISCV] function calls support in lldb expressions (PR #99336)

2024-09-13 Thread Michael Buch via lldb-commits


@@ -471,6 +498,18 @@ static void SetupTargetOpts(CompilerInstance &compiler,
   // Set the target ABI
   if (std::string abi = GetClangTargetABI(target_arch); !abi.empty())
 compiler.getTargetOpts().ABI = std::move(abi);
+
+  if ((target_machine == llvm::Triple::riscv64 &&
+   compiler.getTargetOpts().ABI == "lp64f") ||
+  (target_machine == llvm::Triple::riscv32 &&
+   compiler.getTargetOpts().ABI == "ilp32f"))
+compiler->getTargetOpts().FeaturesAsWritten.emplace_back("+f");
+
+  if ((target_machine == llvm::Triple::riscv64 &&
+   compiler.getTargetOpts().ABI == "lp64d") ||
+  (target_machine == llvm::Triple::riscv32 &&
+   compiler.getTargetOpts().ABI == "ilp32d"))
+compiler->getTargetOpts().FeaturesAsWritten.emplace_back("+d");

Michael137 wrote:

Makes sense, thanks!

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


[Lldb-commits] [lldb] [llvm] [lldb][RISCV] function calls support in lldb expressions (PR #99336)

2024-09-13 Thread Michael Buch via lldb-commits

https://github.com/Michael137 commented:

Expression parser side of things LGTM (apart from the redundant tests and the 
RuntimeDyld changes being in this patch). I'll let the others comment on the 
RISCV plugin changes.

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


[Lldb-commits] [lldb] Make env and source map dictionaries #95137 (PR #106919)

2024-09-13 Thread via lldb-commits

Da-Viper wrote:

> Nice improvement. Don't forget to write a test

Will do this evening 

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


[Lldb-commits] [lldb] [lldb] Add pc check for thread-step-by-bp algorithms (PR #108504)

2024-09-13 Thread David Spickett via lldb-commits

DavidSpickett wrote:

Thanks I understand now.

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


[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

2024-09-13 Thread Pavel Labath via lldb-commits

labath wrote:

>  If you want to execute such child process for unix socket too, it is 
> necessary to transfer the protocol somehow too.

I do want it. I don't think it's necessary to transfer the type of the socket, 
as that can be determined by inspecting the socket FD 
(`getsockopt(SO_DOMAIN/SO_PROTOCOL)`). I'd probably do it by creating a new 
`Socket::Create` static function which takes the FD, and instantiates the 
appropriate type based on what this function returns.

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


[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)

2024-09-13 Thread via lldb-commits


@@ -45,7 +45,8 @@ bool RunLLDBCommands(llvm::StringRef prefix,
   // RunTerminateCommands.
   static std::mutex handle_command_mutex;
   std::lock_guard locker(handle_command_mutex);
-  interp.HandleCommand(command.str().c_str(), result);
+  interp.HandleCommand(command.str().c_str(), result,
+   /* add_to_history */ true);

cmtice wrote:

add_to_history defaults to false. If the commands are not added to the history, 
then the CommandInterpreter can't process an empty expression because there's 
no record of what the previous non-empty expression was.

I will update the comment as you request.

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


[Lldb-commits] [lldb] [lldb] Add pc check for thread-step-by-bp algorithms (PR #108504)

2024-09-13 Thread Jason Molenda via lldb-commits

jasonmolenda wrote:

Yeah, the problem with our current algorithm is that you lose the actual stop 
reason, and sometimes that's important.  For instance, if you have a store 
instruction that modifies a watchpointed memory, and you have a user breakpoint 
on the next instruction, on AArch64, the store triggers the watchpoint.  lldb 
reads the old value, instruction steps, reads the new value and then tells the 
user.  But now it's sitting on a breakpoint site so that overwrites the 
watchpoint stop reason and we never tell the user about that - we tell it we've 
hit the breakpoint.  gdb behaves the same way.

The user-visible down side to this new approach was seen with Dexter debuginfo 
engine, where it sets a breakpoint on every source line in a program, and then 
does step-in across the source lines.  In this case, the step-in executes to 
the first instruction of the next source line (where there is a breakpoint).  
So we have a "step-in completed" stop reason.  Then Dexter step-in's again, and 
now it executes the breakpoint instruction and stops, without having advanced 
the pc, with a "breakpoint hit" stop reason.  *then* doing a step-in will 
advance to the next source line.  

Users will surely experience this (with only one breakpoint in code they're 
stepping over), and I'm sure they will scratch their heads about why they 
needed to Step twice to get past a breakpoint (if they notice at all), but 
that's the only bad fallout from this change to the algorithm.

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


[Lldb-commits] [lldb] 213c59d - [lldb] Add pc check for thread-step-by-bp algorithms (#108504)

2024-09-13 Thread via lldb-commits

Author: Jason Molenda
Date: 2024-09-13T09:02:31-07:00
New Revision: 213c59ddd2a702ddd3d849cea250440b1ed718e0

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

LOG: [lldb] Add pc check for thread-step-by-bp algorithms (#108504)

lldb-server built with NativeProcessLinux.cpp and
NativeProcessFreeBSD.cpp can use breakpoints to implement instruction
stepping on cores where there is no native instruction-step primitive.
Currently these set a breakpoint, continue, and if we hit the breakpoint
with the original thread, set the stop reason to be "trace".

I am wrapping up a change to lldb's breakpoint algorithm where I change
its current behavior of

"if a thread stops at a breakpoint site, we set
the thread's stop reason to breakpoint-hit, even if the breakpoint
hasn't been executed" +
"when resuming any thread at a breakpoint site, instruction-step past
the breakpoint before resuming"

to a behavior of

"when a thread executes a breakpoint, set the stop reason to
breakpoint-hit" +
"when a thread has hit a breakpoint, when the thread resumes, we
silently step past the breakpoint and then resume the thread".

For these lldb-server targets doing breakpoint stepping, this means that
if we are sitting on a breakpoint that has not yet executed, and
instruction-step the thread, we will execute the breakpoint instruction
at $pc (instead of $next-pc where it meant to go), and stop again -- at
the same pc value. Then we will rewrite the stop reason to 'trace'. The
higher level logic will see that we haven't hit the breakpoint
instruction again, so it will try to instruction step again, hitting the
breakpoint again forever.

To fix this, I'm checking that the thread matches the one we are
instruction-stepping-by-breakpoint AND that we've stopped at the
breakpoint address we are stepping to. Only in that case will the stop
reason be rewritten to "trace" hiding the implementation detail that the
step was done by breakpoints.

Added: 


Modified: 
lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp

Removed: 




diff  --git a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp 
b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
index 97fff4b9f65a82..80b27571f43d55 100644
--- a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -319,9 +319,12 @@ void NativeProcessFreeBSD::MonitorSIGTRAP(lldb::pid_t pid) 
{
info.pl_siginfo.si_addr);
 
   if (thread) {
+auto ®ctx = static_cast(
+thread->GetRegisterContext());
 auto thread_info =
 m_threads_stepping_with_breakpoint.find(thread->GetID());
-if (thread_info != m_threads_stepping_with_breakpoint.end()) {
+if (thread_info != m_threads_stepping_with_breakpoint.end() &&
+threads_info->second == regctx.GetPC()) {
   thread->SetStoppedByTrace();
   Status brkpt_error = RemoveBreakpoint(thread_info->second);
   if (brkpt_error.Fail())

diff  --git a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp 
b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
index 5c262db8db7fde..38b7092682873b 100644
--- a/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -829,8 +829,11 @@ void 
NativeProcessLinux::MonitorBreakpoint(NativeThreadLinux &thread) {
   thread.SetStoppedByBreakpoint();
   FixupBreakpointPCAsNeeded(thread);
 
-  if (m_threads_stepping_with_breakpoint.find(thread.GetID()) !=
-  m_threads_stepping_with_breakpoint.end())
+  NativeRegisterContextLinux ®_ctx = thread.GetRegisterContext();
+  auto stepping_with_bp_it =
+  m_threads_stepping_with_breakpoint.find(thread.GetID());
+  if (stepping_with_bp_it != m_threads_stepping_with_breakpoint.end() &&
+  stepping_with_bp_it->second == reg_ctx.GetPC())
 thread.SetStoppedByTrace();
 
   StopRunningThreads(thread.GetID());



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


[Lldb-commits] [lldb] [lldb] Add pc check for thread-step-by-bp algorithms (PR #108504)

2024-09-13 Thread Jason Molenda via lldb-commits

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


[Lldb-commits] [lldb] 65a4d11 - [lldb] Set the stop reason when receiving swbreak/hwbreak (#108518)

2024-09-13 Thread via lldb-commits

Author: Jason Molenda
Date: 2024-09-13T09:04:28-07:00
New Revision: 65a4d11b1e67429d53df1fcee0f93492aa95c448

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

LOG: [lldb] Set the stop reason when receiving swbreak/hwbreak (#108518)

xusheng added support for swbreak/hwbreak a month ago, and no special
support was needed in ProcessGDBRemote when they're received because
lldb already marks a thread as having hit a breakpoint when it stops at
a breakpoint site. However, with changes I am working on, we need to
know the real stop reason a thread stopped or the breakpoint hit will
not be recognized.

This is similar to how lldb processes the "watch/rwatch/awatch" keys in
a thread stop packet -- we set the `reason` to `watchpoint`, and these
set it to `breakpoint` so we set the stop reason correctly later in
these methods.

Added: 


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

Removed: 




diff  --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp 
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 5eaf9ce2a302aa..271ff61a7188a6 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2317,6 +2317,8 @@ StateType 
ProcessGDBRemote::SetThreadStopInfo(StringExtractor &stop_packet) {
 StreamString ostr;
 ostr.Printf("%" PRIu64, wp_addr);
 description = std::string(ostr.GetString());
+  } else if (key.compare("swbreak") == 0 || key.compare("hwbreak") == 0) {
+reason = "breakpoint";
   } else if (key.compare("library") == 0) {
 auto error = LoadModules();
 if (error) {



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


[Lldb-commits] [lldb] [lldb] Set the stop reason when receiving swbreak/hwbreak (PR #108518)

2024-09-13 Thread Jason Molenda via lldb-commits

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


[Lldb-commits] [lldb] [llvm] [lldb] Change lldb's breakpoint detection behavior [WIP] (PR #105594)

2024-09-13 Thread Jason Molenda via lldb-commits

https://github.com/jasonmolenda updated 
https://github.com/llvm/llvm-project/pull/105594

>From 56ca564185bdceea25162a1ce3b1e8c8fa2641e2 Mon Sep 17 00:00:00 2001
From: Jason Molenda 
Date: Fri, 19 Jul 2024 17:26:13 -0700
Subject: [PATCH 1/7] [lldb] Change lldb's breakpoint handling behavior
 (#96260)

lldb today has two rules: When a thread stops at a BreakpointSite, we
set the thread's StopReason to be "breakpoint hit" (regardless if we've
actually hit the breakpoint, or if we've merely stopped *at* the
breakpoint instruction/point and haven't tripped it yet). And second,
when resuming a process, any thread sitting at a BreakpointSite is
silently stepped over the BreakpointSite -- because we've already
flagged the breakpoint hit when we stopped there originally.

In this patch, I change lldb to only set a thread's stop reason to
breakpoint-hit when we've actually executed the instruction/triggered
the breakpoint. When we resume, we only silently step past a
BreakpointSite that we've registered as hit. We preserve this state
across inferior function calls that the user may do while stopped, etc.

Also, when a user adds a new breakpoint at $pc while stopped, or changes
$pc to be the address of a BreakpointSite, we will silently step past
that breakpoint when the process resumes. This is purely a UX call, I
don't think there's any person who wants to set a breakpoint at $pc and
then hit it immediately on resuming.

One non-intuitive UX from this change, but I'm convinced it is
necessary: If you're stopped at a BreakpointSite that has not yet
executed, you `stepi`, you will hit the breakpoint and the pc will not
yet advance. This thread has not completed its stepi, and the thread
plan is still on the stack. If you then `continue` the thread, lldb will
now stop and say, "instruction step completed", one instruction past the
BreakpointSite. You can continue a second time to resume execution. I
discussed this with Jim, and trying to paper over this behavior will
lead to more complicated scenarios behaving non-intuitively. And mostly
it's the testsuite that was trying to instruction step past a breakpoint
and getting thrown off -- and I changed those tests to expect the new
behavior.

The bugs driving this change are all from lldb dropping the real stop
reason for a thread and setting it to breakpoint-hit when that was not
the case. Jim hit one where we have an aarch64 watchpoint that triggers
one instruction before a BreakpointSite. On this arch we are notified of
the watchpoint hit after the instruction has been unrolled -- we disable
the watchpoint, instruction step, re-enable the watchpoint and collect
the new value. But now we're on a BreakpointSite so the watchpoint-hit
stop reason is lost.

Another was reported by ZequanWu in
https://discourse.llvm.org/t/lldb-unable-to-break-at-start/78282 we
attach to/launch a process with the pc at a BreakpointSite and
misbehave. Caroline Tice mentioned it is also a problem they've had with
putting a breakpoint on _dl_debug_state.

The change to each Process plugin that does execution control is that

1. If we've stopped at a BreakpointSite that has not been executed yet,
we will call Thread::SetThreadStoppedAtUnexecutedBP(pc) to record
that.  When the thread resumes, if the pc is still at the same site, we
will continue, hit the breakpoint, and stop again.

2. When we've actually hit a breakpoint (enabled for this thread or not),
the Process plugin should call Thread::SetThreadHitBreakpointSite().
When we go to resume the thread, we will push a step-over-breakpoint
ThreadPlan before resuming.

The biggest set of changes is to StopInfoMachException where we
translate a Mach Exception into a stop reason. The Mach exception codes
differ in a few places depending on the target (unambiguously), and I
didn't want to duplicate the new code for each target so I've tested
what mach exceptions we get for each action on each target, and
reorganized StopInfoMachException::CreateStopReasonWithMachException to
document these possible values, and handle them without specializing
based on the target arch.

rdar://123942164
---
 lldb/include/lldb/Target/Thread.h |  24 ++
 .../Process/Utility/StopInfoMachException.cpp | 322 --
 .../Process/Windows/Common/ProcessWindows.cpp |  32 +-
 .../Process/gdb-remote/ProcessGDBRemote.cpp   |  93 ++---
 .../Process/scripted/ScriptedThread.cpp   |  11 +
 lldb/source/Target/StopInfo.cpp   |   2 +
 lldb/source/Target/Thread.cpp |  15 +-
 .../TestConsecutiveBreakpoints.py |  26 +-
 .../TestStepOverBreakpoint.py |   6 +-
 9 files changed, 266 insertions(+), 265 deletions(-)

diff --git a/lldb/include/lldb/Target/Thread.h 
b/lldb/include/lldb/Target/Thread.h
index 38b65b2bc58490..22d452c7b4b8a3 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -131,6 +131,7 @@ class Thread : public std::enable_shared_from_this,
 register

[Lldb-commits] [lldb] 661382f - [LLDB][Minidump] Minidump erase file on failure (#108259)

2024-09-13 Thread via lldb-commits

Author: Jacob Lalonde
Date: 2024-09-13T09:17:06-07:00
New Revision: 661382f2c07ba464caa0ad0fb8c64c1c3b20e9a4

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

LOG: [LLDB][Minidump] Minidump erase file on failure (#108259)

In #95312 Minidump file creation was moved from being created at the
end, to the file being emitted in chunks. This causes some undesirable
behavior where the file can still be present after an error has
occurred. To resolve this we will now delete the file upon an error.

Added: 


Modified: 
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp

lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp 
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index edc568a6b47e00..ca22dacb2ba6c9 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -1218,3 +1218,15 @@ Status MinidumpFileBuilder::DumpFile() {
 
   return error;
 }
+
+void MinidumpFileBuilder::DeleteFile() noexcept {
+  Log *log = GetLog(LLDBLog::Object);
+
+  if (m_core_file) {
+Status error = m_core_file->Close();
+if (error.Fail())
+  LLDB_LOGF(log, "Failed to close minidump file: %s", error.AsCString());
+
+m_core_file.reset();
+  }
+}

diff  --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h 
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
index 71001e26c00e91..72e5658718b3c4 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.h
@@ -115,6 +115,9 @@ class MinidumpFileBuilder {
   // Run cleanup and write all remaining bytes to file
   lldb_private::Status DumpFile();
 
+  // Delete the file if it exists
+  void DeleteFile() noexcept;
+
 private:
   // Add data to the end of the buffer, if the buffer exceeds the flush level,
   // trigger a flush.

diff  --git a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp 
b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
index 5da69dd4f2ce79..be47991bb09fcc 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/ObjectFileMinidump.cpp
@@ -55,6 +55,21 @@ size_t ObjectFileMinidump::GetModuleSpecifications(
   return 0;
 }
 
+struct DumpFailRemoveHolder {
+  DumpFailRemoveHolder(MinidumpFileBuilder &builder) : m_builder(builder) {}
+
+  ~DumpFailRemoveHolder() {
+if (!m_success)
+  m_builder.DeleteFile();
+  }
+
+  void SetSuccess() { m_success = true; }
+
+private:
+  MinidumpFileBuilder &m_builder;
+  bool m_success = false;
+};
+
 bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP &process_sp,
   lldb_private::SaveCoreOptions &options,
   lldb_private::Status &error) {
@@ -75,6 +90,7 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP 
&process_sp,
   }
   MinidumpFileBuilder builder(std::move(maybe_core_file.get()), process_sp,
   options);
+  DumpFailRemoveHolder request(builder);
 
   Log *log = GetLog(LLDBLog::Object);
   error = builder.AddHeaderAndCalculateDirectories();
@@ -133,5 +149,7 @@ bool ObjectFileMinidump::SaveCore(const lldb::ProcessSP 
&process_sp,
 return false;
   }
 
+  request.SetSuccess();
+
   return true;
 }

diff  --git 
a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
 
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
index 2cbe20ee10b1af..ccdb6653cf16f8 100644
--- 
a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ 
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -493,3 +493,32 @@ def 
test_save_minidump_custom_save_style_duplicated_regions(self):
 
 finally:
 self.assertTrue(self.dbg.DeleteTarget(target))
+
+@skipUnlessPlatform(["linux"])
+def minidump_deleted_on_save_failure(self):
+"""Test that verifies the minidump file is deleted after an error"""
+
+self.build()
+exe = self.getBuildArtifact("a.out")
+try:
+target = self.dbg.CreateTarget(exe)
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory()
+)
+self.assertState(process.GetState(), lldb.eStateStopped)
+
+custom_file = 
self.g

[Lldb-commits] [lldb] [LLDB][Minidump] Minidump erase file on failure (PR #108259)

2024-09-13 Thread Jacob Lalonde via lldb-commits

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


[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)

2024-09-13 Thread via lldb-commits

https://github.com/cmtice updated 
https://github.com/llvm/llvm-project/pull/107485

>From 15541f354decf80586d590db9f9cb353be04b122 Mon Sep 17 00:00:00 2001
From: Caroline Tice 
Date: Thu, 5 Sep 2024 15:51:35 -0700
Subject: [PATCH 1/6] [lldb-dap] Add feature to remember last non-empty
 expression.

Update lldb-dap so if the user just presses return, which sends an
empty expression, it re-evaluates the most recent non-empty
expression/command. Also udpated test to test this case.
---
 lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py | 3 +++
 lldb/tools/lldb-dap/lldb-dap.cpp  | 8 
 2 files changed, 11 insertions(+)

diff --git a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py 
b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
index 29548a835c6919..9ed0fc564268a7 100644
--- a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
+++ b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py
@@ -60,7 +60,10 @@ def run_test_evaluate_expressions(
 
 # Expressions at breakpoint 1, which is in main
 self.assertEvaluate("var1", "20")
+# Empty expression should equate to the previous expression.
+self.assertEvaluate("", "20")
 self.assertEvaluate("var2", "21")
+self.assertEvaluate("", "21")
 self.assertEvaluate("static_int", "42")
 self.assertEvaluate("non_static_int", "43")
 self.assertEvaluate("struct1.foo", "15")
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index c5c4b09f15622b..a6a701dc2219fa 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1363,6 +1363,14 @@ void request_evaluate(const llvm::json::Object &request) 
{
   lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments);
   std::string expression = GetString(arguments, "expression").str();
   llvm::StringRef context = GetString(arguments, "context");
+  static std::string last_nonempty_expression;
+
+  // Remember the last non-empty expression from the user, and use that if
+  // the current expression is empty (i.e. the user hit plain 'return').
+  if (!expression.empty())
+last_nonempty_expression = expression;
+  else
+expression = last_nonempty_expression;
 
   if (context == "repl" && g_dap.DetectExpressionContext(frame, expression) ==
ExpressionContext::Command) {

>From e35928e08f792163dd4886e797bc6de3d16ea6e6 Mon Sep 17 00:00:00 2001
From: Caroline Tice 
Date: Fri, 6 Sep 2024 10:02:18 -0700
Subject: [PATCH 2/6] [lldb] Add feature to remember last non-empty expression

Make last_nonempty_spression part of DAP struct rather than a
static variable.
---
 lldb/tools/lldb-dap/DAP.h| 1 +
 lldb/tools/lldb-dap/lldb-dap.cpp | 5 ++---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index f4fdec6e895ad1..4220c15d3ae70d 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -205,6 +205,7 @@ struct DAP {
   std::string command_escape_prefix = "`";
   lldb::SBFormat frame_format;
   lldb::SBFormat thread_format;
+  std::string last_nonempty_expression;
 
   DAP();
   ~DAP();
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index a6a701dc2219fa..d3728df9183aa1 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1363,14 +1363,13 @@ void request_evaluate(const llvm::json::Object 
&request) {
   lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments);
   std::string expression = GetString(arguments, "expression").str();
   llvm::StringRef context = GetString(arguments, "context");
-  static std::string last_nonempty_expression;
 
   // Remember the last non-empty expression from the user, and use that if
   // the current expression is empty (i.e. the user hit plain 'return').
   if (!expression.empty())
-last_nonempty_expression = expression;
+g_dap.last_nonempty_expression = expression;
   else
-expression = last_nonempty_expression;
+expression = g_dap.last_nonempty_expression;
 
   if (context == "repl" && g_dap.DetectExpressionContext(frame, expression) ==
ExpressionContext::Command) {

>From 616017152f3f0611462e9863273754036b52f7eb Mon Sep 17 00:00:00 2001
From: Caroline Tice 
Date: Thu, 12 Sep 2024 10:52:32 -0700
Subject: [PATCH 3/6] [lldb-dap] Add feature to remember last non-empty
 expression

Update to handle commands & variables separately: empty command
expressions are passed to the CommandIntepreter to handle as it
normally does; empty variable expressions are updated to use the last
non-empty variable expression, if the last expression was a variable (not
a command).  Also updated the test case to test these cases properly, and
added a 'memory read' followed by an empty expression, to make sure it
handles that sequence correctly.
---
 .../lldb-dap/evaluate/TestDAP_evaluate.py | 16 +++--
 ...

[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)

2024-09-13 Thread via lldb-commits


@@ -45,7 +45,8 @@ bool RunLLDBCommands(llvm::StringRef prefix,
   // RunTerminateCommands.
   static std::mutex handle_command_mutex;
   std::lock_guard locker(handle_command_mutex);
-  interp.HandleCommand(command.str().c_str(), result);
+  interp.HandleCommand(command.str().c_str(), result,
+   /* add_to_history */ true);

cmtice wrote:

Done.

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


[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)

2024-09-13 Thread via lldb-commits


@@ -1364,8 +1364,14 @@ void request_evaluate(const llvm::json::Object &request) 
{
   std::string expression = GetString(arguments, "expression").str();
   llvm::StringRef context = GetString(arguments, "context");
 
-  if (context == "repl" && g_dap.DetectExpressionContext(frame, expression) ==
-   ExpressionContext::Command) {
+  if (context == "repl" &&
+  ((!expression.empty() &&
+   g_dap.DetectExpressionContext(frame, expression) ==
+   ExpressionContext::Command) ||
+   (expression.empty() && g_dap.last_nonempty_var_expression.empty( {

cmtice wrote:

Thank you for the suggestion! Done.

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


[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)

2024-09-13 Thread via lldb-commits


@@ -1376,6 +1382,16 @@ void request_evaluate(const llvm::json::Object &request) 
{
 EmplaceSafeString(body, "result", result);
 body.try_emplace("variablesReference", (int64_t)0);
   } else {
+if (context != "hover") {

cmtice wrote:

Done.

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


[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)

2024-09-13 Thread via lldb-commits

cmtice wrote:

All reviewer comments have been addressed. Please take another look. Thank you!

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


[Lldb-commits] [lldb] [LLDB][Minidump] Add Multiplatform test to ensure determinism (PR #108602)

2024-09-13 Thread Jacob Lalonde via lldb-commits

https://github.com/Jlalond created 
https://github.com/llvm/llvm-project/pull/108602

Adds a test that ensures two minidumps produced from the same target are the 
same bytes. Covers the three primary core flavors.

>From 60b48f98ffa70dbf633f1b9bf935479dbfce59d4 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde 
Date: Fri, 13 Sep 2024 09:29:46 -0700
Subject: [PATCH] Add Add multiplatform minidump determinism test

---
 .../TestProcessSaveCoreMinidump.py| 39 +++
 1 file changed, 39 insertions(+)

diff --git 
a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
 
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
index ccdb6653cf16f8..d4eaa40b13f7ee 100644
--- 
a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ 
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -522,3 +522,42 @@ def minidump_deleted_on_save_failure(self):
 
 finally:
 self.assertTrue(self.dbg.DeleteTarget(target))
+
+def minidump_deterministic_difference(self):
+"""Test that verifies that two minidumps produced are identical."""
+
+self.build()
+exe = self.getBuildArtifact("a.out")
+try:
+target = self.dbg.CreateTarget(exe)
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory()
+)
+self.assertState(process.GetState(), lldb.eStateStopped)
+
+core_styles = [lldb.eSaveCoreStackOnly, lldb.eSaveCoreDirtyOnly, 
lldb.eSaveCoreFull]
+for style in core_styles:
+spec_one = 
lldb.SBFileSpec(self.getBuildArtifact("core.one.dmp"))
+spec_two = 
lldb.SBFileSpec(self.getBuildArtifact("core.two.dmp"))
+options = lldb.SBSaveCoreOptions()
+options.SetOutputFile(spec_one)
+options.SetPluginName("minidump")
+options.SetStyle(style)
+error = process.SaveCore(options)
+self.assertTrue(error.Success())
+options.SetOutputFile(spec_two)
+error = process.SaveCore(options)
+self.assertTrue(error.Success())
+
+file_one = None
+file_two = None
+with open(spec_one.GetFileName(), mode='rb') as file: # b is 
important -> binary
+file_one = file.read()
+with open(spec_two.GetFileName(), mode='rb') as file:
+file_two = file.read()
+self.assertEqual(file_one, file_two)
+self.assertTrue(os.unlink(spec_one.GetFileName()))
+self.assertTrue(os.unlink(spec_two.GetFileName()))
+
+finally:
+self.assertTrue(self.dbg.DeleteTarget(target))

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


[Lldb-commits] [lldb] [LLDB][Minidump] Add Multiplatform test to ensure determinism (PR #108602)

2024-09-13 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Jacob Lalonde (Jlalond)


Changes

Adds a test that ensures two minidumps produced from the same target are the 
same bytes. Covers the three primary core flavors.

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


1 Files Affected:

- (modified) 
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
 (+39) 


``diff
diff --git 
a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
 
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
index ccdb6653cf16f8..d4eaa40b13f7ee 100644
--- 
a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ 
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -522,3 +522,42 @@ def minidump_deleted_on_save_failure(self):
 
 finally:
 self.assertTrue(self.dbg.DeleteTarget(target))
+
+def minidump_deterministic_difference(self):
+"""Test that verifies that two minidumps produced are identical."""
+
+self.build()
+exe = self.getBuildArtifact("a.out")
+try:
+target = self.dbg.CreateTarget(exe)
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory()
+)
+self.assertState(process.GetState(), lldb.eStateStopped)
+
+core_styles = [lldb.eSaveCoreStackOnly, lldb.eSaveCoreDirtyOnly, 
lldb.eSaveCoreFull]
+for style in core_styles:
+spec_one = 
lldb.SBFileSpec(self.getBuildArtifact("core.one.dmp"))
+spec_two = 
lldb.SBFileSpec(self.getBuildArtifact("core.two.dmp"))
+options = lldb.SBSaveCoreOptions()
+options.SetOutputFile(spec_one)
+options.SetPluginName("minidump")
+options.SetStyle(style)
+error = process.SaveCore(options)
+self.assertTrue(error.Success())
+options.SetOutputFile(spec_two)
+error = process.SaveCore(options)
+self.assertTrue(error.Success())
+
+file_one = None
+file_two = None
+with open(spec_one.GetFileName(), mode='rb') as file: # b is 
important -> binary
+file_one = file.read()
+with open(spec_two.GetFileName(), mode='rb') as file:
+file_two = file.read()
+self.assertEqual(file_one, file_two)
+self.assertTrue(os.unlink(spec_one.GetFileName()))
+self.assertTrue(os.unlink(spec_two.GetFileName()))
+
+finally:
+self.assertTrue(self.dbg.DeleteTarget(target))

``




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


[Lldb-commits] [lldb] [LLDB][Minidump] Add Multiplatform test to ensure determinism (PR #108602)

2024-09-13 Thread via lldb-commits

github-actions[bot] wrote:




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



You can test this locally with the following command:


``bash
darker --check --diff -r 
661382f2c07ba464caa0ad0fb8c64c1c3b20e9a4...60b48f98ffa70dbf633f1b9bf935479dbfce59d4
 
lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
``





View the diff from darker here.


``diff
--- TestProcessSaveCoreMinidump.py  2024-09-13 16:29:46.00 +
+++ TestProcessSaveCoreMinidump.py  2024-09-13 16:37:56.432599 +
@@ -533,11 +533,15 @@
 process = target.LaunchSimple(
 None, None, self.get_process_working_directory()
 )
 self.assertState(process.GetState(), lldb.eStateStopped)
 
-core_styles = [lldb.eSaveCoreStackOnly, lldb.eSaveCoreDirtyOnly, 
lldb.eSaveCoreFull]
+core_styles = [
+lldb.eSaveCoreStackOnly,
+lldb.eSaveCoreDirtyOnly,
+lldb.eSaveCoreFull,
+]
 for style in core_styles:
 spec_one = 
lldb.SBFileSpec(self.getBuildArtifact("core.one.dmp"))
 spec_two = 
lldb.SBFileSpec(self.getBuildArtifact("core.two.dmp"))
 options = lldb.SBSaveCoreOptions()
 options.SetOutputFile(spec_one)
@@ -549,13 +553,15 @@
 error = process.SaveCore(options)
 self.assertTrue(error.Success())
 
 file_one = None
 file_two = None
-with open(spec_one.GetFileName(), mode='rb') as file: # b is 
important -> binary
+with open(
+spec_one.GetFileName(), mode="rb"
+) as file:  # b is important -> binary
 file_one = file.read()
-with open(spec_two.GetFileName(), mode='rb') as file:
+with open(spec_two.GetFileName(), mode="rb") as file:
 file_two = file.read()
 self.assertEqual(file_one, file_two)
 self.assertTrue(os.unlink(spec_one.GetFileName()))
 self.assertTrue(os.unlink(spec_two.GetFileName()))
 

``




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


[Lldb-commits] [lldb] [lldb] Emit signpost intervals for progress events (NFC) (PR #108498)

2024-09-13 Thread Med Ismail Bennani via lldb-commits

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

LGTM

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


[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)

2024-09-13 Thread Adrian Prantl via lldb-commits


@@ -94,25 +126,36 @@ Status Status::FromErrorStringWithFormat(const char 
*format, ...) {
   return Status(string);
 }
 
-Status Status::FromError(llvm::Error error) { return Status(std::move(error)); 
}
+Status Status::FromExpressionError(lldb::ExpressionResults result,
+   std::string msg) {
+  return Status(llvm::make_error(
+  std::error_code(result, expression_category()), msg));
+}
 
-llvm::Error Status::ToError() const {
-  if (Success())
+/// Creates a deep copy of all known errors and converts all other
+/// errors to a new llvm::StringError.
+static llvm::Error CloneError(const llvm::Error &error) {
+  std::vector> info;
+  llvm::visitErrors(error, [&](const llvm::ErrorInfoBase &error) {
+if (error.isA())
+  info.push_back(static_cast(&error)->Clone());

adrian-prantl wrote:

Oh nice. I actually hadn't realized this before.

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


[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)

2024-09-13 Thread Adrian Prantl via lldb-commits


@@ -94,25 +126,36 @@ Status Status::FromErrorStringWithFormat(const char 
*format, ...) {
   return Status(string);
 }
 
-Status Status::FromError(llvm::Error error) { return Status(std::move(error)); 
}
+Status Status::FromExpressionError(lldb::ExpressionResults result,
+   std::string msg) {
+  return Status(llvm::make_error(
+  std::error_code(result, expression_category()), msg));
+}
 
-llvm::Error Status::ToError() const {
-  if (Success())
+/// Creates a deep copy of all known errors and converts all other
+/// errors to a new llvm::StringError.
+static llvm::Error CloneError(const llvm::Error &error) {
+  std::vector> info;
+  llvm::visitErrors(error, [&](const llvm::ErrorInfoBase &error) {
+if (error.isA())
+  info.push_back(static_cast(&error)->Clone());

adrian-prantl wrote:

Ah, I didn't realize that `joinErrors(success(), e) == e`

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


[Lldb-commits] [lldb] [LLDB][Minidump] Add Multiplatform test to ensure determinism (PR #108602)

2024-09-13 Thread Jacob Lalonde via lldb-commits

https://github.com/Jlalond updated 
https://github.com/llvm/llvm-project/pull/108602

>From 41bb7f8a13483e55f677d51e049e3d79d763088b Mon Sep 17 00:00:00 2001
From: Jacob Lalonde 
Date: Fri, 13 Sep 2024 09:29:46 -0700
Subject: [PATCH 1/2] Add multiplatform minidump determinism test

---
 .../TestProcessSaveCoreMinidump.py| 39 +++
 1 file changed, 39 insertions(+)

diff --git 
a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
 
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
index ccdb6653cf16f8..d4eaa40b13f7ee 100644
--- 
a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ 
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -522,3 +522,42 @@ def minidump_deleted_on_save_failure(self):
 
 finally:
 self.assertTrue(self.dbg.DeleteTarget(target))
+
+def minidump_deterministic_difference(self):
+"""Test that verifies that two minidumps produced are identical."""
+
+self.build()
+exe = self.getBuildArtifact("a.out")
+try:
+target = self.dbg.CreateTarget(exe)
+process = target.LaunchSimple(
+None, None, self.get_process_working_directory()
+)
+self.assertState(process.GetState(), lldb.eStateStopped)
+
+core_styles = [lldb.eSaveCoreStackOnly, lldb.eSaveCoreDirtyOnly, 
lldb.eSaveCoreFull]
+for style in core_styles:
+spec_one = 
lldb.SBFileSpec(self.getBuildArtifact("core.one.dmp"))
+spec_two = 
lldb.SBFileSpec(self.getBuildArtifact("core.two.dmp"))
+options = lldb.SBSaveCoreOptions()
+options.SetOutputFile(spec_one)
+options.SetPluginName("minidump")
+options.SetStyle(style)
+error = process.SaveCore(options)
+self.assertTrue(error.Success())
+options.SetOutputFile(spec_two)
+error = process.SaveCore(options)
+self.assertTrue(error.Success())
+
+file_one = None
+file_two = None
+with open(spec_one.GetFileName(), mode='rb') as file: # b is 
important -> binary
+file_one = file.read()
+with open(spec_two.GetFileName(), mode='rb') as file:
+file_two = file.read()
+self.assertEqual(file_one, file_two)
+self.assertTrue(os.unlink(spec_one.GetFileName()))
+self.assertTrue(os.unlink(spec_two.GetFileName()))
+
+finally:
+self.assertTrue(self.dbg.DeleteTarget(target))

>From b52d9e4b80a35c3f1e58d53b7134b01e5b44bf30 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde 
Date: Fri, 13 Sep 2024 10:08:51 -0700
Subject: [PATCH 2/2] Comment cleanup

---
 .../TestProcessSaveCoreMinidump.py   | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git 
a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
 
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
index d4eaa40b13f7ee..e994dd75645400 100644
--- 
a/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
+++ 
b/lldb/test/API/functionalities/process_save_core_minidump/TestProcessSaveCoreMinidump.py
@@ -535,7 +535,11 @@ def minidump_deterministic_difference(self):
 )
 self.assertState(process.GetState(), lldb.eStateStopped)
 
-core_styles = [lldb.eSaveCoreStackOnly, lldb.eSaveCoreDirtyOnly, 
lldb.eSaveCoreFull]
+core_styles = [
+lldb.eSaveCoreStackOnly,
+lldb.eSaveCoreDirtyOnly,
+lldb.eSaveCoreFull,
+]
 for style in core_styles:
 spec_one = 
lldb.SBFileSpec(self.getBuildArtifact("core.one.dmp"))
 spec_two = 
lldb.SBFileSpec(self.getBuildArtifact("core.two.dmp"))
@@ -551,9 +555,11 @@ def minidump_deterministic_difference(self):
 
 file_one = None
 file_two = None
-with open(spec_one.GetFileName(), mode='rb') as file: # b is 
important -> binary
+with open(
+spec_one.GetFileName(), mode="rb"
+) as file:
 file_one = file.read()
-with open(spec_two.GetFileName(), mode='rb') as file:
+with open(spec_two.GetFileName(), mode="rb") as file:
 file_two = file.read()
 self.assertEqual(file_one, file_two)
 self.assertTrue(os.unlink(spec_one.GetFileName()))

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


[Lldb-commits] [lldb] Avoid expression evaluation in libStdC++ std::vector synthetic children provider (PR #108414)

2024-09-13 Thread via lldb-commits

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

LGTM

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


[Lldb-commits] [lldb] Avoid expression evaluation in libStdC++ std::vector synthetic children provider (PR #108414)

2024-09-13 Thread via lldb-commits


@@ -645,6 +645,23 @@ lldb::SBValue SBValue::CreateValueFromData(const char 
*name, SBData data,
   return sb_value;
 }
 
+lldb::SBValue SBValue::CreateBoolValue(const char *name, bool value) {
+  LLDB_INSTRUMENT_VA(this, name);
+
+  lldb::SBValue sb_value;
+  lldb::ValueObjectSP new_value_sp;
+  ValueLocker locker;
+  lldb::ValueObjectSP value_sp(GetSP(locker));
+  lldb::TargetSP target_sp = m_opaque_sp->GetTargetSP();
+  if (value_sp && target_sp) {
+new_value_sp =
+ValueObject::CreateValueObjectFromBool(target_sp, value, name);
+new_value_sp->SetAddressTypeOfChildren(eAddressTypeLoad);

jimingham wrote:

The problem comes with ValueObjects that live in lldb host memory for instance 
ValueObjectConstResult "history" entities or ValueObject made from 
DataExtractors.  If you have such a result value that includes pointers that 
you copied up from the target memory, even though the ValueObject itself lives 
in lldb host memory, the pointer children still point into the target (are load 
addresses).  This API states that that is true for this ValueObject.

It's not relevant for ValueObjects that can't have pointer children.

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


[Lldb-commits] [lldb] 02d8813 - Add a comment in the SB API doc about keeping the SB API's lightweight. (#108462)

2024-09-13 Thread via lldb-commits

Author: jimingham
Date: 2024-09-13T10:18:03-07:00
New Revision: 02d8813820b1ebf3fae6993e677db269f0077272

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

LOG: Add a comment in the SB API doc about keeping the SB API's lightweight. 
(#108462)

Added: 


Modified: 
lldb/docs/resources/sbapi.rst

Removed: 




diff  --git a/lldb/docs/resources/sbapi.rst b/lldb/docs/resources/sbapi.rst
index cf32cc6c815581..4ca3909e0f2919 100644
--- a/lldb/docs/resources/sbapi.rst
+++ b/lldb/docs/resources/sbapi.rst
@@ -72,6 +72,17 @@ building the LLDB framework for macOS, the headers are 
processed with
 ``unifdef`` prior to being copied into the framework bundle to remove macros
 involving SWIG.
 
+Another good principle when adding SB API methods is: if you find yourself
+implementing a significant algorithm in the SB API method, you should not do
+that, but instead look for and then add it - if not found - as a method in the
+underlying lldb_private class, and then call that from your SB API method.
+If it was a useful algorithm, it's very likely it already exists
+because the lldb_private code also needed to do it.  And if it doesn't at
+present, if it was a useful thing to do, it's likely someone will later need
+it in lldb_private and then we end up with two implementations of the same
+algorithm.  If we keep the SB API code to just what's needed to manage the SB
+objects and requests, we won't get into this situation.
+
 Lifetime
 
 Many SB API methods will return strings in the form of ``const char *`` values.



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


[Lldb-commits] [lldb] Add a comment in the SB API doc about keeping the SB API's lightweight. (PR #108462)

2024-09-13 Thread via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Emit signpost intervals for progress events (NFC) (PR #108498)

2024-09-13 Thread Alex Langford via lldb-commits

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


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


[Lldb-commits] [lldb] b6bf27e - Avoid expression evaluation in libStdC++ std::vector synthetic children provider (#108414)

2024-09-13 Thread via lldb-commits

Author: jeffreytan81
Date: 2024-09-13T10:26:01-07:00
New Revision: b6bf27ef3c179eefd805f39aa681705fc980ceed

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

LOG: Avoid expression evaluation in libStdC++ std::vector synthetic 
children provider  (#108414)

Our customers is reporting a serious performance issue (expanding a this
pointer takes 70 seconds in VSCode) in a specific execution context.

Profiling shows the hot path is triggered by an expression evaluation
from libStdC++ synthetic children provider for `std::vector` since
it uses `CreateValueFromExpression()`.

This PR added a new `SBValue::CreateBoolValue()` API and switch
`std::vector` synthetic children provider to use the new API
without performing expression evaluation.

Note: there might be other cases of `CreateValueFromExpression()` in our
summary/synthetic children providers which I will sweep through in later
PRs.

With this PR, the customer's scenario reduces from 70 seconds => 50
seconds. I will add other PRs to further optimize the remaining 50
seconds (mostly from type/namespace lookup).

Testing:

`test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/vbool/TestDataFormatterStdVBool.py`
passes with the PR

-

Co-authored-by: jeffreytan81 

Added: 


Modified: 
lldb/examples/synthetic/gnu_libstdcpp.py
lldb/include/lldb/API/SBValue.h
lldb/source/API/SBValue.cpp

Removed: 




diff  --git a/lldb/examples/synthetic/gnu_libstdcpp.py 
b/lldb/examples/synthetic/gnu_libstdcpp.py
index d98495b8a9df38..a6605a7a7eb5b3 100644
--- a/lldb/examples/synthetic/gnu_libstdcpp.py
+++ b/lldb/examples/synthetic/gnu_libstdcpp.py
@@ -473,11 +473,7 @@ def get_child_at_index(self, index):
 "[" + str(index) + "]", element_offset, element_type
 )
 bit = element.GetValueAsUnsigned(0) & (1 << bit_offset)
-if bit != 0:
-value_expr = "(bool)true"
-else:
-value_expr = "(bool)false"
-return self.valobj.CreateValueFromExpression("[%d]" % index, 
value_expr)
+return self.valobj.CreateBoolValue("[%d]" % index, bool(bit))
 
 def update(self):
 try:

diff  --git a/lldb/include/lldb/API/SBValue.h b/lldb/include/lldb/API/SBValue.h
index bec816fb451844..9090cece80f7ce 100644
--- a/lldb/include/lldb/API/SBValue.h
+++ b/lldb/include/lldb/API/SBValue.h
@@ -145,6 +145,8 @@ class LLDB_API SBValue {
   // AddressOf() on the return of this call all return invalid
   lldb::SBValue CreateValueFromData(const char *name, lldb::SBData data,
 lldb::SBType type);
+  // Returned value has no address.
+  lldb::SBValue CreateBoolValue(const char *name, bool value);
 
   /// Get a child value by index from a value.
   ///

diff  --git a/lldb/source/API/SBValue.cpp b/lldb/source/API/SBValue.cpp
index 273aac5ad47989..e1a31708d46ffb 100644
--- a/lldb/source/API/SBValue.cpp
+++ b/lldb/source/API/SBValue.cpp
@@ -645,6 +645,22 @@ lldb::SBValue SBValue::CreateValueFromData(const char 
*name, SBData data,
   return sb_value;
 }
 
+lldb::SBValue SBValue::CreateBoolValue(const char *name, bool value) {
+  LLDB_INSTRUMENT_VA(this, name);
+
+  lldb::SBValue sb_value;
+  lldb::ValueObjectSP new_value_sp;
+  ValueLocker locker;
+  lldb::ValueObjectSP value_sp(GetSP(locker));
+  lldb::TargetSP target_sp = m_opaque_sp->GetTargetSP();
+  if (value_sp && target_sp) {
+new_value_sp =
+ValueObject::CreateValueObjectFromBool(target_sp, value, name);
+  }
+  sb_value.SetSP(new_value_sp);
+  return sb_value;
+}
+
 SBValue SBValue::GetChildAtIndex(uint32_t idx) {
   LLDB_INSTRUMENT_VA(this, idx);
 



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


[Lldb-commits] [lldb] Avoid expression evaluation in libStdC++ std::vector synthetic children provider (PR #108414)

2024-09-13 Thread via lldb-commits

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


[Lldb-commits] [lldb] 0351dc5 - [lldb] Do not use LC_FUNCTION_STARTS data to determine symbol size as symbols are created (#106791)

2024-09-13 Thread via lldb-commits

Author: Alex Langford
Date: 2024-09-13T10:33:43-07:00
New Revision: 0351dc522a25df0473a63b414a5bfde5814d3dc3

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

LOG: [lldb] Do not use LC_FUNCTION_STARTS data to determine symbol size as 
symbols are created (#106791)

Summary:
This improves the performance of ObjectFileMacho::ParseSymtab by
removing eager and expensive work in favor of doing it later in a
less-expensive fashion.

Experiment:
My goal was to understand LLDB's startup time.
First, I produced a Debug build of LLDB (no dSYM) and a
Release+NoAsserts build of LLDB. The Release build debugged the Debug
build as it debugged a small C++ program. I found that
ObjectFileMachO::ParseSymtab accounted for somewhere between 1.2 and 1.3
seconds consistently. After applying this change, I consistently
measured a reduction of approximately 100ms, putting the time closer to
1.1s and 1.2s on average.

Background:
ObjectFileMachO::ParseSymtab will incrementally create symbols by
parsing nlist entries from the symtab section of a MachO binary. As it
does this, it eagerly tries to determine the size of symbols (e.g. how
long a function is) using LC_FUNCTION_STARTS data (or eh_frame if
LC_FUNCTION_STARTS is unavailable). Concretely, this is done by
performing a binary search on the function starts array and calculating
the distance to the next function or the end of the section (whichever
is smaller).

However, this work is unnecessary for 2 reasons:
1. If you have debug symbol entries (i.e. STABs), the size of a function
is usually stored right after the function's entry. Performing this work
right before parsing the next entry is unnecessary work.
2. Calculating symbol sizes for symbols of size 0 is already performed
in `Symtab::InitAddressIndexes` after all the symbols are added to the
Symtab. It also does this more efficiently by walking over a list of
symbols sorted by address, so the work to calculate the size per symbol
is constant instead of O(log n).

Added: 


Modified: 
lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp 
b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
index 06da83e26a26a5..c36748963db37b 100644
--- a/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
+++ b/lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
@@ -3768,7 +3768,6 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
 
   SymbolType type = eSymbolTypeInvalid;
   SectionSP symbol_section;
-  lldb::addr_t symbol_byte_size = 0;
   bool add_nlist = true;
   bool is_gsym = false;
   bool demangled_is_synthesized = false;
@@ -4354,47 +4353,6 @@ void ObjectFileMachO::ParseSymtab(Symtab &symtab) {
 
   if (symbol_section) {
 const addr_t section_file_addr = symbol_section->GetFileAddress();
-if (symbol_byte_size == 0 && function_starts_count > 0) {
-  addr_t symbol_lookup_file_addr = nlist.n_value;
-  // Do an exact address match for non-ARM addresses, else get the
-  // closest since the symbol might be a thumb symbol which has an
-  // address with bit zero set.
-  FunctionStarts::Entry *func_start_entry =
-  function_starts.FindEntry(symbol_lookup_file_addr, !is_arm);
-  if (is_arm && func_start_entry) {
-// Verify that the function start address is the symbol address
-// (ARM) or the symbol address + 1 (thumb).
-if (func_start_entry->addr != symbol_lookup_file_addr &&
-func_start_entry->addr != (symbol_lookup_file_addr + 1)) {
-  // Not the right entry, NULL it out...
-  func_start_entry = nullptr;
-}
-  }
-  if (func_start_entry) {
-func_start_entry->data = true;
-
-addr_t symbol_file_addr = func_start_entry->addr;
-if (is_arm)
-  symbol_file_addr &= THUMB_ADDRESS_BIT_MASK;
-
-const FunctionStarts::Entry *next_func_start_entry =
-function_starts.FindNextEntry(func_start_entry);
-const addr_t section_end_file_addr =
-section_file_addr + symbol_section->GetByteSize();
-if (next_func_start_entry) {
-  addr_t next_symbol_file_addr = next_func_start_entry->addr;
-  // Be sure the clear the Thumb address bit when we calculate the
-  // size from the current and next address
-  if (is_arm)
-next_symbol_file_addr &= THUMB_ADDRESS_BIT_MASK;
-  symbol_byte_size = std::min(
-  next_symbol_file_addr - symbol_file_addr,
-  section_end_file_addr - symbol_file_addr);
-   

[Lldb-commits] [lldb] [lldb] Do not use LC_FUNCTION_STARTS data to determine symbol size as symbols are created (PR #106791)

2024-09-13 Thread Alex Langford via lldb-commits

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


[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

2024-09-13 Thread Dmitry Vasilyev via lldb-commits

slydiman wrote:

> I do want it. I don't think it's necessary to transfer the type of the 
> socket, as that can be determined by inspecting the socket FD 
> (getsockopt(SO_DOMAIN/SO_PROTOCOL)).

Oh, I expected this. We can determine non-tcp by missing or 0 gdb port. But it 
is impossible to separare ProtocolUnixDomain and ProtocolUnixAbstract. The 
difference is only the implementation of Socket::GetNameOffset().
getsockopt(SO_DOMAIN/SO_PROTOCOL) will not help.
We can provide `--listen same_host_port ` to the child process additionally to 
`--child-platform-fd X`  and move the parsing of the listen parameter to the 
beginning to get the correct protocol.
I will make the second commit with the version where I will mux functions 
platform_tcp() and platform_named() back together, complicated and hard as 
possible.

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


[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

2024-09-13 Thread Dmitry Vasilyev via lldb-commits

https://github.com/slydiman updated 
https://github.com/llvm/llvm-project/pull/104238

>From 792cb17f05ded47b152408ac7f4d1de6c986013f Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev 
Date: Fri, 6 Sep 2024 16:12:50 +0400
Subject: [PATCH] [lldb] Removed gdbserver ports map from lldb-server

Listen to gdbserver-port, accept the connection and run lldb-server gdbserver 
--fd on all platforms.
Parameters --min-gdbserver-port and --max-gdbserver-port are deprecated now.

This is the part 2 of #101283.

Fixes #97537, fixes #101475.
---
 lldb/docs/man/lldb-server.rst |  11 +-
 lldb/docs/resources/qemu-testing.rst  |  19 +-
 .../GDBRemoteCommunicationServerPlatform.cpp  | 287 +
 .../GDBRemoteCommunicationServerPlatform.h|  82 +---
 .../TestPlatformLaunchGDBServer.py|  12 +-
 .../Shell/lldb-server/TestGdbserverPort.test  |   4 -
 lldb/tools/lldb-server/Acceptor.cpp   |  98 -
 lldb/tools/lldb-server/Acceptor.h |  60 ---
 lldb/tools/lldb-server/CMakeLists.txt |   1 -
 lldb/tools/lldb-server/lldb-gdbserver.cpp |  17 +-
 lldb/tools/lldb-server/lldb-platform.cpp  | 391 --
 .../Process/gdb-remote/CMakeLists.txt |   1 -
 .../Process/gdb-remote/PortMapTest.cpp| 115 --
 13 files changed, 391 insertions(+), 707 deletions(-)
 delete mode 100644 lldb/test/Shell/lldb-server/TestGdbserverPort.test
 delete mode 100644 lldb/tools/lldb-server/Acceptor.cpp
 delete mode 100644 lldb/tools/lldb-server/Acceptor.h
 delete mode 100644 lldb/unittests/Process/gdb-remote/PortMapTest.cpp

diff --git a/lldb/docs/man/lldb-server.rst b/lldb/docs/man/lldb-server.rst
index a67c00b305f6d2..31f5360df5e23e 100644
--- a/lldb/docs/man/lldb-server.rst
+++ b/lldb/docs/man/lldb-server.rst
@@ -147,15 +147,8 @@ GDB-SERVER CONNECTIONS
 
 .. option:: --gdbserver-port 
 
- Define a port to be used for gdb-server connections. Can be specified multiple
- times to allow multiple ports. Has no effect if --min-gdbserver-port
- and --max-gdbserver-port are specified.
-
-.. option:: --min-gdbserver-port 
-.. option:: --max-gdbserver-port 
-
- Specify the range of ports that can be used for gdb-server connections. Both
- options need to be specified simultaneously. Overrides --gdbserver-port.
+ Define a port to be used for gdb-server connections. This port is used for
+ multiple connections.
 
 .. option:: --port-offset 
 
diff --git a/lldb/docs/resources/qemu-testing.rst 
b/lldb/docs/resources/qemu-testing.rst
index 51a30b11717a87..e102f84a1d31f4 100644
--- a/lldb/docs/resources/qemu-testing.rst
+++ b/lldb/docs/resources/qemu-testing.rst
@@ -149,7 +149,6 @@ to the host (refer to QEMU's manuals for the specific 
options).
 * At least one to connect to the intial ``lldb-server``.
 * One more if you want to use ``lldb-server`` in ``platform mode``, and have it
   start a ``gdbserver`` instance for you.
-* A bunch more if you want to run tests against the ``lldb-server`` platform.
 
 If you are doing either of the latter 2 you should also restrict what ports
 ``lldb-server tries`` to use, otherwise it will randomly pick one that is 
almost
@@ -157,22 +156,14 @@ certainly not forwarded. An example of this is shown 
below.
 
 ::
 
-  $ lldb-server plaform --server --listen 0.0.0.0:54321 \
---min-gdbserver-port 49140 --max-gdbserver-port 49150
+  $ lldb-server plaform --server --listen 0.0.0.0:54321 --gdbserver-port 49140
 
 The result of this is that:
 
 * ``lldb-server`` platform mode listens externally on port ``54321``.
 
-* When it is asked to start a new gdbserver mode instance, it will use a port
-  in the range ``49140`` to ``49150``.
+* When it is asked to start a new gdbserver mode instance, it will use the port
+  ``49140``.
 
-Your VM configuration should have ports ``54321``, and ``49140`` to ``49150``
-forwarded for this to work.
-
-.. note::
-  These options are used to create a "port map" within ``lldb-server``.
-  Unfortunately this map is not cleaned up on Windows on connection close,
-  and across a few uses you may run out of valid ports. To work around this,
-  restart the platform every so often, especially after running a set of tests.
-  This is tracked here: https://github.com/llvm/llvm-project/issues/90923
+Your VM configuration should have ports ``54321`` and ``49140`` forwarded for
+this to work.
diff --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
index c60a83d351ba09..647130b2984aac 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -44,79 +44,14 @@ using namespace lldb;
 using namespace lldb_private::process_gdb_remote;
 using namespace lldb_private;
 
-GDBRemoteCommunicationServerPlatform::PortMap::PortMap(uint16_t min_port,
-   

[Lldb-commits] [lldb] Make env and source map dictionaries #95137 (PR #106919)

2024-09-13 Thread via lldb-commits

https://github.com/Da-Viper updated 
https://github.com/llvm/llvm-project/pull/106919

>From d2bddca1753b4c960895f51d7eb80b6efa7dc986 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka 
Date: Sun, 1 Sep 2024 17:26:11 +0100
Subject: [PATCH 1/9] [lldb-dap] Make environment option an object

---
 .../tools/lldb-dap/launch/TestDAP_launch.py   |  4 +-
 lldb/tools/lldb-dap/JSONUtils.cpp | 40 ---
 lldb/tools/lldb-dap/JSONUtils.h   | 22 ++
 lldb/tools/lldb-dap/README.md |  5 ++-
 lldb/tools/lldb-dap/lldb-dap.cpp  |  8 +++-
 lldb/tools/lldb-dap/package.json  | 11 +++--
 6 files changed, 77 insertions(+), 13 deletions(-)

diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py 
b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
index a16f2da3c4df71..6b9993a2548b8d 100644
--- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
+++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
@@ -229,7 +229,7 @@ def test_environment(self):
 Tests launch of a simple program with environment variables
 """
 program = self.getBuildArtifact("a.out")
-env = ["NO_VALUE", "WITH_VALUE=BAR", "EMPTY_VALUE=", "SPACE=Hello 
World"]
+env = {"NO_VALUE": "", "WITH_VALUE":"BAR", "EMPTY_VALUE": "", "SPACE": 
"Hello World"}
 self.build_and_launch(program, env=env)
 self.continue_to_exit()
 
@@ -242,7 +242,7 @@ def test_environment(self):
 lines.pop(0)
 # Make sure each environment variable in "env" is actually set in the
 # program environment that was printed to STDOUT
-for var in env:
+for var in env.keys():
 found = False
 for program_var in lines:
 if var in program_var:
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp 
b/lldb/tools/lldb-dap/JSONUtils.cpp
index 7338e7cf41eb03..29b3ad490af0b6 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -136,6 +136,31 @@ std::vector GetStrings(const 
llvm::json::Object *obj,
   return strs;
 }
 
+std::unordered_map
+GetStringMap(const llvm::json::Object &obj, llvm::StringRef key) {
+  std::unordered_map strs;
+  const auto *const json_object = obj.getObject(key);
+  if (!json_object)
+return strs;
+
+  for (const auto &[key, value] : *json_object) {
+switch (value.kind()) {
+case llvm::json::Value::String:
+  strs.emplace(key.str(), value.getAsString()->str());
+  break;
+case llvm::json::Value::Number:
+case llvm::json::Value::Boolean:
+  strs.emplace(key.str(), llvm::to_string(value));
+  break;
+case llvm::json::Value::Null:
+case llvm::json::Value::Object:
+case llvm::json::Value::Array:
+  break;
+}
+  }
+  return strs;
+}
+
 static bool IsClassStructOrUnionType(lldb::SBType t) {
   return (t.GetTypeClass() & (lldb::eTypeClassUnion | lldb::eTypeClassStruct |
   lldb::eTypeClassArray)) != 0;
@@ -1370,13 +1395,16 @@ CreateRunInTerminalReverseRequest(const 
llvm::json::Object &launch_request,
   if (!cwd.empty())
 run_in_terminal_args.try_emplace("cwd", cwd);
 
-  // We need to convert the input list of environments variables into a
-  // dictionary
-  std::vector envs = GetStrings(launch_request_arguments, "env");
+  std::unordered_map envMap =
+  GetStringMap(*launch_request_arguments, "env");
   llvm::json::Object environment;
-  for (const std::string &env : envs) {
-size_t index = env.find('=');
-environment.try_emplace(env.substr(0, index), env.substr(index + 1));
+  for (const auto &[key, value] : envMap) {
+if (key.empty())
+  g_dap.SendOutput(OutputType::Stderr,
+   "empty environment variable for value: \"" + value +
+   '\"');
+else
+  environment.try_emplace(key, value);
   }
   run_in_terminal_args.try_emplace("env",
llvm::json::Value(std::move(environment)));
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index b6356630b72682..60d5db06560657 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -16,6 +16,7 @@
 #include "llvm/Support/JSON.h"
 #include 
 #include 
+#include 
 
 namespace lldb_dap {
 
@@ -152,6 +153,27 @@ bool ObjectContainsKey(const llvm::json::Object &obj, 
llvm::StringRef key);
 std::vector GetStrings(const llvm::json::Object *obj,
 llvm::StringRef key);
 
+/// Extract an object of key value strings for the specified key from an 
object.
+///
+/// String values in the object will be extracted without any quotes
+/// around them. Numbers and Booleans will be converted into
+/// strings. Any NULL, array or objects values in the array will be
+/// ignored.
+///
+/// \param[in] obj
+/// A JSON object that we will attempt to extract the array from
+///
+/// \param[in] key
+/// The key to use when extr

[Lldb-commits] [lldb] [lldb] Emit signpost intervals for progress events (NFC) (PR #108498)

2024-09-13 Thread Jonas Devlieghere via lldb-commits

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


[Lldb-commits] [lldb] [lldb][test] Remove benchmark API tests (PR #108629)

2024-09-13 Thread Michael Buch via lldb-commits

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

These benchmarks don't get run as part of the regular API test-suite. And I'm 
not aware of any CI running this. Also, I haven't quite managed to actually run 
them locally using the `bench.py` script. It looks like these are obsolete, so 
I'm proposing to remove the infrastructure around it entirely.

If anyone does know of a use for these do let me know.

>From c58fd8c9a3e7f568cc9fb451297a840b52ccb2b4 Mon Sep 17 00:00:00 2001
From: Michael Buch 
Date: Fri, 13 Sep 2024 20:07:16 +0100
Subject: [PATCH] [lldb][test] Remove benchmark API tests

These benchmarks don't get run as part of the regular
API test-suite. And I'm not aware of any CI running this.
Also, I haven't quite managed to actually run them locally
using the `bench.py` script. It looks like these are obsolete,
so I'm proposing to remove the infrastructure around it entirely.

If anyone does know of a use for these do let me know.
---
 lldb/packages/Python/lldbsuite/test/bench.py  |  77 --
 .../Python/lldbsuite/test/decorators.py   |  12 --
 lldb/test/API/benchmarks/continue/Makefile|   3 -
 .../continue/TestBenchmarkContinue.py |  65 -
 lldb/test/API/benchmarks/continue/main.cpp|  36 -
 lldb/test/API/benchmarks/expression/Makefile  |   3 -
 .../expression/TestExpressionCmd.py   |  74 --
 .../expression/TestRepeatedExprs.py   | 131 --
 lldb/test/API/benchmarks/expression/main.cpp  |  43 --
 .../TestFrameVariableResponse.py  |  68 -
 lldb/test/API/benchmarks/libcxxlist/Makefile  |   3 -
 .../libcxxlist/TestBenchmarkLibcxxList.py |  58 
 lldb/test/API/benchmarks/libcxxlist/main.cpp  |  11 --
 lldb/test/API/benchmarks/libcxxmap/Makefile   |   3 -
 .../libcxxmap/TestBenchmarkLibcxxMap.py   |  58 
 lldb/test/API/benchmarks/libcxxmap/main.cpp   |  11 --
 .../benchmarks/startup/TestStartupDelays.py   |  78 ---
 .../benchmarks/stepping/TestSteppingSpeed.py  |  69 -
 .../TestCompileRunToBreakpointTurnaround.py   | 122 
 19 files changed, 925 deletions(-)
 delete mode 100644 lldb/packages/Python/lldbsuite/test/bench.py
 delete mode 100644 lldb/test/API/benchmarks/continue/Makefile
 delete mode 100644 lldb/test/API/benchmarks/continue/TestBenchmarkContinue.py
 delete mode 100644 lldb/test/API/benchmarks/continue/main.cpp
 delete mode 100644 lldb/test/API/benchmarks/expression/Makefile
 delete mode 100644 lldb/test/API/benchmarks/expression/TestExpressionCmd.py
 delete mode 100644 lldb/test/API/benchmarks/expression/TestRepeatedExprs.py
 delete mode 100644 lldb/test/API/benchmarks/expression/main.cpp
 delete mode 100644 
lldb/test/API/benchmarks/frame_variable/TestFrameVariableResponse.py
 delete mode 100644 lldb/test/API/benchmarks/libcxxlist/Makefile
 delete mode 100644 
lldb/test/API/benchmarks/libcxxlist/TestBenchmarkLibcxxList.py
 delete mode 100644 lldb/test/API/benchmarks/libcxxlist/main.cpp
 delete mode 100644 lldb/test/API/benchmarks/libcxxmap/Makefile
 delete mode 100644 lldb/test/API/benchmarks/libcxxmap/TestBenchmarkLibcxxMap.py
 delete mode 100644 lldb/test/API/benchmarks/libcxxmap/main.cpp
 delete mode 100644 lldb/test/API/benchmarks/startup/TestStartupDelays.py
 delete mode 100644 lldb/test/API/benchmarks/stepping/TestSteppingSpeed.py
 delete mode 100644 
lldb/test/API/benchmarks/turnaround/TestCompileRunToBreakpointTurnaround.py

diff --git a/lldb/packages/Python/lldbsuite/test/bench.py 
b/lldb/packages/Python/lldbsuite/test/bench.py
deleted file mode 100644
index 1a11b3ef4f0e64..00
--- a/lldb/packages/Python/lldbsuite/test/bench.py
+++ /dev/null
@@ -1,77 +0,0 @@
-#!/usr/bin/env python
-
-"""
-A simple bench runner which delegates to the ./dotest.py test driver to run the
-benchmarks defined in the list named 'benches'.
-
-You need to hand edit 'benches' to modify/change the command lines passed to 
the
-test driver.
-
-Use the following to get only the benchmark results in your terminal output:
-
-./bench.py -e /Volumes/data/lldb/svn/regression/build/Debug/lldb -x '-F 
Driver::MainLoop()' 2>&1 | grep -P '^lldb.*benchmark:'
-"""
-
-import os
-from optparse import OptionParser
-
-# dotest.py invocation with no '-e exe-path' uses lldb as the inferior program,
-# unless there is a mentioning of custom executable program.
-benches = [
-# Measure startup delays creating a target, setting a breakpoint, and run
-# to breakpoint stop.
-"./dotest.py -v +b %E %X -n -p TestStartupDelays.py",
-# Measure 'frame variable' response after stopping at a breakpoint.
-"./dotest.py -v +b %E %X -n -p TestFrameVariableResponse.py",
-# Measure stepping speed after stopping at a breakpoint.
-"./dotest.py -v +b %E %X -n -p TestSteppingSpeed.py",
-# Measure expression cmd response with a simple custom executable program.
-"./dotest.py +b -n -p TestExpressionCmd.py",
-

[Lldb-commits] [lldb] [lldb][test] Remove benchmark API tests (PR #108629)

2024-09-13 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)


Changes

These benchmarks don't get run as part of the regular API test-suite. And I'm 
not aware of any CI running this. Also, I haven't quite managed to actually run 
them locally using the `bench.py` script. It looks like these are obsolete, so 
I'm proposing to remove the infrastructure around it entirely.

If anyone does know of a use for these do let me know.

---

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


19 Files Affected:

- (removed) lldb/packages/Python/lldbsuite/test/bench.py (-77) 
- (modified) lldb/packages/Python/lldbsuite/test/decorators.py (-12) 
- (removed) lldb/test/API/benchmarks/continue/Makefile (-3) 
- (removed) lldb/test/API/benchmarks/continue/TestBenchmarkContinue.py (-65) 
- (removed) lldb/test/API/benchmarks/continue/main.cpp (-36) 
- (removed) lldb/test/API/benchmarks/expression/Makefile (-3) 
- (removed) lldb/test/API/benchmarks/expression/TestExpressionCmd.py (-74) 
- (removed) lldb/test/API/benchmarks/expression/TestRepeatedExprs.py (-131) 
- (removed) lldb/test/API/benchmarks/expression/main.cpp (-43) 
- (removed) 
lldb/test/API/benchmarks/frame_variable/TestFrameVariableResponse.py (-68) 
- (removed) lldb/test/API/benchmarks/libcxxlist/Makefile (-3) 
- (removed) lldb/test/API/benchmarks/libcxxlist/TestBenchmarkLibcxxList.py 
(-58) 
- (removed) lldb/test/API/benchmarks/libcxxlist/main.cpp (-11) 
- (removed) lldb/test/API/benchmarks/libcxxmap/Makefile (-3) 
- (removed) lldb/test/API/benchmarks/libcxxmap/TestBenchmarkLibcxxMap.py (-58) 
- (removed) lldb/test/API/benchmarks/libcxxmap/main.cpp (-11) 
- (removed) lldb/test/API/benchmarks/startup/TestStartupDelays.py (-78) 
- (removed) lldb/test/API/benchmarks/stepping/TestSteppingSpeed.py (-69) 
- (removed) 
lldb/test/API/benchmarks/turnaround/TestCompileRunToBreakpointTurnaround.py 
(-122) 


``diff
diff --git a/lldb/packages/Python/lldbsuite/test/bench.py 
b/lldb/packages/Python/lldbsuite/test/bench.py
deleted file mode 100644
index 1a11b3ef4f0e64..00
--- a/lldb/packages/Python/lldbsuite/test/bench.py
+++ /dev/null
@@ -1,77 +0,0 @@
-#!/usr/bin/env python
-
-"""
-A simple bench runner which delegates to the ./dotest.py test driver to run the
-benchmarks defined in the list named 'benches'.
-
-You need to hand edit 'benches' to modify/change the command lines passed to 
the
-test driver.
-
-Use the following to get only the benchmark results in your terminal output:
-
-./bench.py -e /Volumes/data/lldb/svn/regression/build/Debug/lldb -x '-F 
Driver::MainLoop()' 2>&1 | grep -P '^lldb.*benchmark:'
-"""
-
-import os
-from optparse import OptionParser
-
-# dotest.py invocation with no '-e exe-path' uses lldb as the inferior program,
-# unless there is a mentioning of custom executable program.
-benches = [
-# Measure startup delays creating a target, setting a breakpoint, and run
-# to breakpoint stop.
-"./dotest.py -v +b %E %X -n -p TestStartupDelays.py",
-# Measure 'frame variable' response after stopping at a breakpoint.
-"./dotest.py -v +b %E %X -n -p TestFrameVariableResponse.py",
-# Measure stepping speed after stopping at a breakpoint.
-"./dotest.py -v +b %E %X -n -p TestSteppingSpeed.py",
-# Measure expression cmd response with a simple custom executable program.
-"./dotest.py +b -n -p TestExpressionCmd.py",
-# Attach to a spawned process then run disassembly benchmarks.
-"./dotest.py -v +b -n %E -p TestDoAttachThenDisassembly.py",
-]
-
-
-def main():
-"""Read the items from 'benches' and run the command line one by one."""
-parser = OptionParser(
-usage="""\
-%prog [options]
-Run the standard benchmarks defined in the list named 'benches'.\
-"""
-)
-parser.add_option(
-"-e",
-"--executable",
-type="string",
-action="store",
-dest="exe",
-help="The target program launched by lldb.",
-)
-parser.add_option(
-"-x",
-"--breakpoint-spec",
-type="string",
-action="store",
-dest="break_spec",
-help="The lldb breakpoint spec for the target program.",
-)
-
-# Parses the options, if any.
-opts, args = parser.parse_args()
-
-print("Starting bench runner")
-
-for item in benches:
-command = item.replace("%E", '-e "%s"' % opts.exe if opts.exe else "")
-command = command.replace(
-"%X", '-x "%s"' % opts.break_spec if opts.break_spec else ""
-)
-print("Running %s" % (command))
-os.system(command)
-
-print("Bench runner done.")
-
-
-if __name__ == "__main__":
-main()
diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py 
b/lldb/packages/Python/lldbsuite/test/decorators.py
index 834f01aaa61e6b..34319e203a3177 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/ll

[Lldb-commits] [lldb] [lldb][test] Remove benchmark API tests (PR #108629)

2024-09-13 Thread Adrian Prantl via lldb-commits

https://github.com/adrian-prantl approved this pull request.

Looks like a nice thing to have conceptually, but if it's not maintained and 
not functional ...

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


[Lldb-commits] [lldb] [lldb][test] Remove benchmark API tests (PR #108629)

2024-09-13 Thread Jonas Devlieghere via lldb-commits

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

Unless someone steps up to maintain these tests I'm fine with removing them.

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


[Lldb-commits] [lldb] [lldb] Introduce an always-on system log category/channel (PR #108495)

2024-09-13 Thread Adrian Prantl via lldb-commits


@@ -31,6 +31,22 @@ class ProcessInstanceInfo;
 class ProcessInstanceInfoMatch;
 typedef std::vector ProcessInstanceInfoList;
 
+// Always on system log category and channel.

adrian-prantl wrote:

I think we should have a comment that explains what this channel is supposed to 
be used for?

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


[Lldb-commits] [lldb] [lldb] Removed gdbserver ports map from lldb-server (PR #104238)

2024-09-13 Thread Dmitry Vasilyev via lldb-commits

https://github.com/slydiman updated 
https://github.com/llvm/llvm-project/pull/104238

>From 792cb17f05ded47b152408ac7f4d1de6c986013f Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev 
Date: Fri, 6 Sep 2024 16:12:50 +0400
Subject: [PATCH 1/2] [lldb] Removed gdbserver ports map from lldb-server

Listen to gdbserver-port, accept the connection and run lldb-server gdbserver 
--fd on all platforms.
Parameters --min-gdbserver-port and --max-gdbserver-port are deprecated now.

This is the part 2 of #101283.

Fixes #97537, fixes #101475.
---
 lldb/docs/man/lldb-server.rst |  11 +-
 lldb/docs/resources/qemu-testing.rst  |  19 +-
 .../GDBRemoteCommunicationServerPlatform.cpp  | 287 +
 .../GDBRemoteCommunicationServerPlatform.h|  82 +---
 .../TestPlatformLaunchGDBServer.py|  12 +-
 .../Shell/lldb-server/TestGdbserverPort.test  |   4 -
 lldb/tools/lldb-server/Acceptor.cpp   |  98 -
 lldb/tools/lldb-server/Acceptor.h |  60 ---
 lldb/tools/lldb-server/CMakeLists.txt |   1 -
 lldb/tools/lldb-server/lldb-gdbserver.cpp |  17 +-
 lldb/tools/lldb-server/lldb-platform.cpp  | 391 --
 .../Process/gdb-remote/CMakeLists.txt |   1 -
 .../Process/gdb-remote/PortMapTest.cpp| 115 --
 13 files changed, 391 insertions(+), 707 deletions(-)
 delete mode 100644 lldb/test/Shell/lldb-server/TestGdbserverPort.test
 delete mode 100644 lldb/tools/lldb-server/Acceptor.cpp
 delete mode 100644 lldb/tools/lldb-server/Acceptor.h
 delete mode 100644 lldb/unittests/Process/gdb-remote/PortMapTest.cpp

diff --git a/lldb/docs/man/lldb-server.rst b/lldb/docs/man/lldb-server.rst
index a67c00b305f6d2..31f5360df5e23e 100644
--- a/lldb/docs/man/lldb-server.rst
+++ b/lldb/docs/man/lldb-server.rst
@@ -147,15 +147,8 @@ GDB-SERVER CONNECTIONS
 
 .. option:: --gdbserver-port 
 
- Define a port to be used for gdb-server connections. Can be specified multiple
- times to allow multiple ports. Has no effect if --min-gdbserver-port
- and --max-gdbserver-port are specified.
-
-.. option:: --min-gdbserver-port 
-.. option:: --max-gdbserver-port 
-
- Specify the range of ports that can be used for gdb-server connections. Both
- options need to be specified simultaneously. Overrides --gdbserver-port.
+ Define a port to be used for gdb-server connections. This port is used for
+ multiple connections.
 
 .. option:: --port-offset 
 
diff --git a/lldb/docs/resources/qemu-testing.rst 
b/lldb/docs/resources/qemu-testing.rst
index 51a30b11717a87..e102f84a1d31f4 100644
--- a/lldb/docs/resources/qemu-testing.rst
+++ b/lldb/docs/resources/qemu-testing.rst
@@ -149,7 +149,6 @@ to the host (refer to QEMU's manuals for the specific 
options).
 * At least one to connect to the intial ``lldb-server``.
 * One more if you want to use ``lldb-server`` in ``platform mode``, and have it
   start a ``gdbserver`` instance for you.
-* A bunch more if you want to run tests against the ``lldb-server`` platform.
 
 If you are doing either of the latter 2 you should also restrict what ports
 ``lldb-server tries`` to use, otherwise it will randomly pick one that is 
almost
@@ -157,22 +156,14 @@ certainly not forwarded. An example of this is shown 
below.
 
 ::
 
-  $ lldb-server plaform --server --listen 0.0.0.0:54321 \
---min-gdbserver-port 49140 --max-gdbserver-port 49150
+  $ lldb-server plaform --server --listen 0.0.0.0:54321 --gdbserver-port 49140
 
 The result of this is that:
 
 * ``lldb-server`` platform mode listens externally on port ``54321``.
 
-* When it is asked to start a new gdbserver mode instance, it will use a port
-  in the range ``49140`` to ``49150``.
+* When it is asked to start a new gdbserver mode instance, it will use the port
+  ``49140``.
 
-Your VM configuration should have ports ``54321``, and ``49140`` to ``49150``
-forwarded for this to work.
-
-.. note::
-  These options are used to create a "port map" within ``lldb-server``.
-  Unfortunately this map is not cleaned up on Windows on connection close,
-  and across a few uses you may run out of valid ports. To work around this,
-  restart the platform every so often, especially after running a set of tests.
-  This is tracked here: https://github.com/llvm/llvm-project/issues/90923
+Your VM configuration should have ports ``54321`` and ``49140`` forwarded for
+this to work.
diff --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
index c60a83d351ba09..647130b2984aac 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -44,79 +44,14 @@ using namespace lldb;
 using namespace lldb_private::process_gdb_remote;
 using namespace lldb_private;
 
-GDBRemoteCommunicationServerPlatform::PortMap::PortMap(uint16_t min_port,
-   

[Lldb-commits] [lldb] [llvm] [lldb] Change lldb's breakpoint detection behavior [WIP] (PR #105594)

2024-09-13 Thread Jason Molenda via lldb-commits

jasonmolenda wrote:

(1) the armv7/aarch32 step-by-breakpoint fix for lldb-server is merged on 
github main.  
(2) the change to ProcessGDBRemote to recognize "swbreak" and "hwbreak" as a 
breakpoint is merged on github main.
(3) lldb with this patch works fine with the existing debugserver on armv7k 
where we get the breakpoint hit mach exception when we instruction step 
(StopInfoMachException handles it)
(4) dexter has been updated on github main to work with this patch or with old 
lldb stepping

which leaves

(5) Martin's mingw finish bug report
(6) Review Omair's Windows aarch64 hardware breakpoint/watchpoint PR for any 
changes necessary to work with this PR.

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


[Lldb-commits] [lldb] [lldb] [debugserver] Use "full" x86_64 GPR state when available. (PR #108663)

2024-09-13 Thread Brendan Shanks via lldb-commits

https://github.com/mrpippy created 
https://github.com/llvm/llvm-project/pull/108663

macOS 10.15 added a "full" x86_64 GPR thread state flavor, equivalent to the 
normal one but with DS, ES, SS, and GSbase added. This flavor can only be used 
with processes that install a custom LDT (functionality that was also added in 
10.15 and is used by apps like Wine to execute 32-bit code).

Along with allowing DS, ES, SS, and GSbase to be viewed/modified, using the 
full flavor is necessary when debugging a thread executing 32-bit code.
If thread_set_state() is used with the regular thread state flavor, the kernel 
resets CS to the 64-bit code segment (see 
[set_thread_state64()](https://github.com/apple-oss-distributions/xnu/blob/94d3b452840153a99b38a3a9659680b2a006908e/osfmk/i386/pcb.c#L723),
 which makes debugging impossible.

There's no way to detect whether the full flavor is available, try to use it 
and fall back to the regular one if it's not available.

A downside is that this patch exposes the DS, ES, SS, and GSbase registers for 
all x86_64 processes, even though they are not populated unless the full thread 
state is available.
I'm not sure if there's a way to tell LLDB that a register is unavailable. The 
classic GDB `g` command [allows returning 
`x`](https://sourceware.org/gdb/current/onlinedocs/gdb.html/Packets.html#Packets)
 to denote unavailable registers, but it seems like the debug server uses newer 
commands like `jThreadsInfo` and I'm not sure if those have the same support.

Fixes #57591
(also filed as Apple FB11464104)

@jasonmolenda 

>From 4453801c7d8abf7a6adfb7fae57ad9fa9d52a0c4 Mon Sep 17 00:00:00 2001
From: Brendan Shanks 
Date: Thu, 12 Sep 2024 16:01:30 -0700
Subject: [PATCH] [lldb] [debugserver] Use "full" x86_64 GPR state when
 available.

macOS 10.15 added a "full" x86_64 GPR thread state flavor, equivalent to
the normal one but with DS, ES, SS, and GSbase added.
This flavor can only be used with processes that install a custom LDT
(functionality that was also added in 10.15 and is used by apps like
Wine to execute 32-bit code).

Along with allowing DS, ES, SS, and GSbase to be viewed/modified, using
the full flavor is necessary when debugging a thread executing 32-bit
code.
If thread_set_state() is used with the regular thread state flavor, the
kernel resets CS to the 64-bit code segment, which makes debugging
impossible.

There's no way to detect whether the full flavor is available, try to
use it and fall back to the regular one if it's not available.

A downside is that this patch exposes the DS, ES, SS, and GSbase
registers for all x86_64 processes, even though they are not populated
unless the full thread state is available.

Fixes #57591
---
 .../MacOSX/x86_64/DNBArchImplX86_64.cpp   | 52 +++
 .../source/MacOSX/x86_64/DNBArchImplX86_64.h  |  4 +-
 .../MacOSX/x86_64/MachRegisterStatesX86_64.h  |  5 ++
 3 files changed, 51 insertions(+), 10 deletions(-)

diff --git a/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp 
b/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
index 5a62e2a8d12e2c..286fd72267b349 100644
--- a/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
@@ -182,22 +182,39 @@ kern_return_t DNBArchImplX86_64::GetGPRState(bool force) {
 m_state.context.gpr.__gs = ('g' << 8) + 's';
 m_state.SetError(e_regSetGPR, Read, 0);
 #else
-mach_msg_type_number_t count = e_regSetWordSizeGPR;
+mach_msg_type_number_t count = e_regSetWordSizeGPRFull;
+int flavor = __x86_64_THREAD_FULL_STATE;
 m_state.SetError(
 e_regSetGPR, Read,
-::thread_get_state(m_thread->MachPortNumber(), __x86_64_THREAD_STATE,
+::thread_get_state(m_thread->MachPortNumber(), flavor,
(thread_state_t)&m_state.context.gpr, &count));
+
+if (!m_state.GetError(e_regSetGPR, Read)) {
+  m_state.hasFullGPRState = true;
+} else {
+  m_state.hasFullGPRState = false;
+  count = e_regSetWordSizeGPR;
+  flavor = __x86_64_THREAD_STATE;
+  m_state.SetError(
+  e_regSetGPR, Read,
+  ::thread_get_state(m_thread->MachPortNumber(), flavor,
+ (thread_state_t)&m_state.context.gpr, &count));
+}
 DNBLogThreadedIf(
 LOG_THREAD,
-"::thread_get_state (0x%4.4x, %u, &gpr, %u) => 0x%8.8x"
+"::thread_get_state (0x%4.4x, %u (%s), &gpr, %u) => 0x%8.8x"
 "\n\trax = %16.16llx rbx = %16.16llx rcx = %16.16llx rdx = %16.16llx"
 "\n\trdi = %16.16llx rsi = %16.16llx rbp = %16.16llx rsp = %16.16llx"
 "\n\t r8 = %16.16llx  r9 = %16.16llx r10 = %16.16llx r11 = %16.16llx"
 "\n\tr12 = %16.16llx r13 = %16.16llx r14 = %16.16llx r15 = %16.16llx"
 "\n\trip = %16.16llx"
-"\n\tflg = %16.16llx  cs = %16.16llx  fs = %16.16llx  gs = %16.16llx",
-m_thread->MachPortNumber(), x86_THREAD_STATE64,

[Lldb-commits] [lldb] [lldb] [debugserver] Use "full" x86_64 GPR state when available. (PR #108663)

2024-09-13 Thread via lldb-commits

llvmbot wrote:




@llvm/pr-subscribers-lldb

Author: Brendan Shanks (mrpippy)


Changes

macOS 10.15 added a "full" x86_64 GPR thread state flavor, equivalent to the 
normal one but with DS, ES, SS, and GSbase added. This flavor can only be used 
with processes that install a custom LDT (functionality that was also added in 
10.15 and is used by apps like Wine to execute 32-bit code).

Along with allowing DS, ES, SS, and GSbase to be viewed/modified, using the 
full flavor is necessary when debugging a thread executing 32-bit code.
If thread_set_state() is used with the regular thread state flavor, the kernel 
resets CS to the 64-bit code segment (see 
[set_thread_state64()](https://github.com/apple-oss-distributions/xnu/blob/94d3b452840153a99b38a3a9659680b2a006908e/osfmk/i386/pcb.c#L723),
 which makes debugging impossible.

There's no way to detect whether the full flavor is available, try to use it 
and fall back to the regular one if it's not available.

A downside is that this patch exposes the DS, ES, SS, and GSbase registers for 
all x86_64 processes, even though they are not populated unless the full thread 
state is available.
I'm not sure if there's a way to tell LLDB that a register is unavailable. The 
classic GDB `g` command [allows returning 
`x`](https://sourceware.org/gdb/current/onlinedocs/gdb.html/Packets.html#Packets)
 to denote unavailable registers, but it seems like the debug server uses newer 
commands like `jThreadsInfo` and I'm not sure if those have the same support.

Fixes #57591
(also filed as Apple FB11464104)

@jasonmolenda 

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


3 Files Affected:

- (modified) lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp 
(+43-9) 
- (modified) lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.h 
(+3-1) 
- (modified) 
lldb/tools/debugserver/source/MacOSX/x86_64/MachRegisterStatesX86_64.h (+5) 


``diff
diff --git a/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp 
b/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
index 5a62e2a8d12e2c..286fd72267b349 100644
--- a/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
@@ -182,22 +182,39 @@ kern_return_t DNBArchImplX86_64::GetGPRState(bool force) {
 m_state.context.gpr.__gs = ('g' << 8) + 's';
 m_state.SetError(e_regSetGPR, Read, 0);
 #else
-mach_msg_type_number_t count = e_regSetWordSizeGPR;
+mach_msg_type_number_t count = e_regSetWordSizeGPRFull;
+int flavor = __x86_64_THREAD_FULL_STATE;
 m_state.SetError(
 e_regSetGPR, Read,
-::thread_get_state(m_thread->MachPortNumber(), __x86_64_THREAD_STATE,
+::thread_get_state(m_thread->MachPortNumber(), flavor,
(thread_state_t)&m_state.context.gpr, &count));
+
+if (!m_state.GetError(e_regSetGPR, Read)) {
+  m_state.hasFullGPRState = true;
+} else {
+  m_state.hasFullGPRState = false;
+  count = e_regSetWordSizeGPR;
+  flavor = __x86_64_THREAD_STATE;
+  m_state.SetError(
+  e_regSetGPR, Read,
+  ::thread_get_state(m_thread->MachPortNumber(), flavor,
+ (thread_state_t)&m_state.context.gpr, &count));
+}
 DNBLogThreadedIf(
 LOG_THREAD,
-"::thread_get_state (0x%4.4x, %u, &gpr, %u) => 0x%8.8x"
+"::thread_get_state (0x%4.4x, %u (%s), &gpr, %u) => 0x%8.8x"
 "\n\trax = %16.16llx rbx = %16.16llx rcx = %16.16llx rdx = %16.16llx"
 "\n\trdi = %16.16llx rsi = %16.16llx rbp = %16.16llx rsp = %16.16llx"
 "\n\t r8 = %16.16llx  r9 = %16.16llx r10 = %16.16llx r11 = %16.16llx"
 "\n\tr12 = %16.16llx r13 = %16.16llx r14 = %16.16llx r15 = %16.16llx"
 "\n\trip = %16.16llx"
-"\n\tflg = %16.16llx  cs = %16.16llx  fs = %16.16llx  gs = %16.16llx",
-m_thread->MachPortNumber(), x86_THREAD_STATE64,
-x86_THREAD_STATE64_COUNT, m_state.GetError(e_regSetGPR, Read),
+"\n\tflg = %16.16llx  cs = %16.16llx  fs = %16.16llx  gs = %16.16llx"
+"\n\t ds = %16.16llx  es = %16.16llx  ss = %16.16llx gsB = %16.16llx",
+m_thread->MachPortNumber(), flavor,
+m_state.hasFullGPRState ? "full" : "non-full",
+m_state.hasFullGPRState ? __x86_64_THREAD_FULL_STATE
+: __x86_64_THREAD_STATE,
+m_state.GetError(e_regSetGPR, Read),
 m_state.context.gpr.__rax, m_state.context.gpr.__rbx,
 m_state.context.gpr.__rcx, m_state.context.gpr.__rdx,
 m_state.context.gpr.__rdi, m_state.context.gpr.__rsi,
@@ -208,7 +225,9 @@ kern_return_t DNBArchImplX86_64::GetGPRState(bool force) {
 m_state.context.gpr.__r14, m_state.context.gpr.__r15,
 m_state.context.gpr.__rip, m_state.context.gpr.__rflags,
 m_state.context.gpr.__cs, m_state.context.gpr.__fs,
-m_state.context.gpr.__gs);
+m

[Lldb-commits] [lldb] [lldb] [debugserver] Use "full" x86_64 GPR state when available. (PR #108663)

2024-09-13 Thread via lldb-commits

github-actions[bot] wrote:



Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this 
page.

If this is not working for you, it is probably because you do not have write 
permissions for the repository. In which case you can instead tag reviewers by 
name in a comment by using `@` followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a 
review by "ping"ing the PR by adding a comment “Ping”. The common courtesy 
"ping" rate is once a week. Please remember that you are asking for valuable 
time from other developers.

If you have further questions, they may be answered by the [LLVM GitHub User 
Guide](https://llvm.org/docs/GitHub.html).

You can also ask questions in a comment on this PR, on the [LLVM 
Discord](https://discord.com/invite/xS7Z362) or on the 
[forums](https://discourse.llvm.org/).

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


[Lldb-commits] [lldb] [lldb] [debugserver] Use "full" x86_64 GPR state when available. (PR #108663)

2024-09-13 Thread via lldb-commits

github-actions[bot] wrote:




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



You can test this locally with the following command:


``bash
git-clang-format --diff b74e7792194d9a8a9ef32c7dc1ffcd205b299336 
4453801c7d8abf7a6adfb7fae57ad9fa9d52a0c4 --extensions cpp,h -- 
lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp 
lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.h 
lldb/tools/debugserver/source/MacOSX/x86_64/MachRegisterStatesX86_64.h
``





View the diff from clang-format here.


``diff
diff --git a/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp 
b/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
index 286fd72267..6bc1055278 100644
--- a/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/x86_64/DNBArchImplX86_64.cpp
@@ -184,10 +184,10 @@ kern_return_t DNBArchImplX86_64::GetGPRState(bool force) {
 #else
 mach_msg_type_number_t count = e_regSetWordSizeGPRFull;
 int flavor = __x86_64_THREAD_FULL_STATE;
-m_state.SetError(
-e_regSetGPR, Read,
-::thread_get_state(m_thread->MachPortNumber(), flavor,
-   (thread_state_t)&m_state.context.gpr, &count));
+m_state.SetError(e_regSetGPR, Read,
+ ::thread_get_state(m_thread->MachPortNumber(), flavor,
+(thread_state_t)&m_state.context.gpr,
+&count));
 
 if (!m_state.GetError(e_regSetGPR, Read)) {
   m_state.hasFullGPRState = true;
@@ -195,10 +195,10 @@ kern_return_t DNBArchImplX86_64::GetGPRState(bool force) {
   m_state.hasFullGPRState = false;
   count = e_regSetWordSizeGPR;
   flavor = __x86_64_THREAD_STATE;
-  m_state.SetError(
-  e_regSetGPR, Read,
-  ::thread_get_state(m_thread->MachPortNumber(), flavor,
- (thread_state_t)&m_state.context.gpr, &count));
+  m_state.SetError(e_regSetGPR, Read,
+   ::thread_get_state(m_thread->MachPortNumber(), flavor,
+  (thread_state_t)&m_state.context.gpr,
+  &count));
 }
 DNBLogThreadedIf(
 LOG_THREAD,
@@ -214,20 +214,19 @@ kern_return_t DNBArchImplX86_64::GetGPRState(bool force) {
 m_state.hasFullGPRState ? "full" : "non-full",
 m_state.hasFullGPRState ? __x86_64_THREAD_FULL_STATE
 : __x86_64_THREAD_STATE,
-m_state.GetError(e_regSetGPR, Read),
-m_state.context.gpr.__rax, m_state.context.gpr.__rbx,
-m_state.context.gpr.__rcx, m_state.context.gpr.__rdx,
-m_state.context.gpr.__rdi, m_state.context.gpr.__rsi,
-m_state.context.gpr.__rbp, m_state.context.gpr.__rsp,
-m_state.context.gpr.__r8, m_state.context.gpr.__r9,
-m_state.context.gpr.__r10, m_state.context.gpr.__r11,
-m_state.context.gpr.__r12, m_state.context.gpr.__r13,
-m_state.context.gpr.__r14, m_state.context.gpr.__r15,
-m_state.context.gpr.__rip, m_state.context.gpr.__rflags,
-m_state.context.gpr.__cs, m_state.context.gpr.__fs,
-m_state.context.gpr.__gs, m_state.context.gpr.__ds,
-m_state.context.gpr.__es, m_state.context.gpr.__ss,
-m_state.context.gpr.__gsbase );
+m_state.GetError(e_regSetGPR, Read), m_state.context.gpr.__rax,
+m_state.context.gpr.__rbx, m_state.context.gpr.__rcx,
+m_state.context.gpr.__rdx, m_state.context.gpr.__rdi,
+m_state.context.gpr.__rsi, m_state.context.gpr.__rbp,
+m_state.context.gpr.__rsp, m_state.context.gpr.__r8,
+m_state.context.gpr.__r9, m_state.context.gpr.__r10,
+m_state.context.gpr.__r11, m_state.context.gpr.__r12,
+m_state.context.gpr.__r13, m_state.context.gpr.__r14,
+m_state.context.gpr.__r15, m_state.context.gpr.__rip,
+m_state.context.gpr.__rflags, m_state.context.gpr.__cs,
+m_state.context.gpr.__fs, m_state.context.gpr.__gs,
+m_state.context.gpr.__ds, m_state.context.gpr.__es,
+m_state.context.gpr.__ss, m_state.context.gpr.__gsbase);
 
 //  DNBLogThreadedIf (LOG_THREAD, "thread_get_state(0x%4.4x, %u, &gpr, %u)
 //  => 0x%8.8x"
@@ -478,13 +477,14 @@ kern_return_t DNBArchImplX86_64::SetGPRState() {
   "(SetGPRState() for stop_count = %u)",
   m_thread->MachPortNumber(), kret, m_thread->Process()->StopCount());
 
-  m_state.SetError(e_regSetGPR, Write,
-   ::thread_set_state(m_thread->MachPortNumber(),
-  m_state.hasFullGPRState ? 
__x86_64_THREAD_FULL_STATE
-  : 
__x86_64_THREAD_STATE,
-  (thread_state_t)&m_state.context.gpr,
-  m_state.hasFul