labath wrote:
That would be great, thanks.
BTW, I've checked the test and its form after
https://github.com/llvm/llvm-project/pull/140331 (where `self.launch` does not
send the configurationDone` event) does reproduce the problem I encountered.
https://github.com/llvm/llvm-project/pull/138219
ashgti wrote:
I'll take a look at the currently disabled DAP tests and see if we can enable
more of them.
https://github.com/llvm/llvm-project/pull/138219
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman
labath wrote:
FWIW, https://github.com/llvm/llvm-project/pull/140331 fixes this. I was
looking at the test coverage, and it seems that the only test making use of
this is TestDAP_setBreakpoints.py. It superficially looks like it should catch
this, but after trying it out, it seems that it does
labath wrote:
I think I've found a(nother) problem with this change. One of the arguments of
the launch request is `sourceMap`, which sets the `target.source-map` setting.
The setting is necessary to correctly resolve file+line breakpoints in case the
file's path on the host system does not ma
JDevlieghere wrote:
The following two tests are both setting breakpoints without stopping at entry,
which means that we could have run past the breakpoint by the time it's set:
```
lldb-api :: tools/lldb-dap/completions/TestDAP_completions.py
lldb-api :: tools/lldb-dap/startDebugging/TestDA
JDevlieghere wrote:
Looking at the logs, the common theme appears to be that all the test binaries
exit before they hit any breakpoints. My best guess as to why this is only
happening on Windows is the way the dynamic loader there works. If that's the
case, then it means that those tests are m
DavidSpickett wrote:
I've reverted this. Please take a look at the logs of
https://lab.llvm.org/buildbot/#/builders/141/builds/8500 and see if any of it
makes sense to you.
We didn't have failures on Linux (though a lot of tests are disabled, they
would be disabled everywhere) so my first ins
DavidSpickett wrote:
We have failing tests on Windows after this. Latest result at this time is:
```
Unresolved Tests (2):
lldb-api :: tools/lldb-dap/completions/TestDAP_completions.py
lldb-api :: tools/lldb-dap/startDebugging/TestDAP_startDebugging.py
***
JDevlieghere wrote:
I created #138778 to track extending the launch and attach helpers to take
breakpoints.
https://github.com/llvm/llvm-project/pull/138219
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mail
https://github.com/JDevlieghere closed
https://github.com/llvm/llvm-project/pull/138219
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/138219
Rate limit ยท GitHub
body {
background-color: #f6f8fa;
color: #24292e;
font-family: -apple-system,BlinkMacSystemFont,Segoe
UI,Helvetica,Arial,s
JDevlieghere wrote:
> Unblocked!
Thanks!
> > @kusmour Please take another look. In the current state all the test pass
> > (reliably) locally and the linux bot seems happy too. Not sure if we want
> > to keep the test disabled for now or turn them back on and see what the
> > reliability is
kusmour wrote:
Unblocked!
> @kusmour Please take another look. In the current state all the test pass
> (reliably) locally and the linux bot seems happy too. Not sure if we want to
> keep the test disabled for now or turn them back on and see what the
> reliability is like on the bots.
I don
https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/138219
>From b7391070942219a691a691822477e2a7616be1ab Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Fri, 2 May 2025 09:59:01 -0700
Subject: [PATCH] [lldb-dap] Change the launch sequence
This PR changes h
https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/138219
>From afd0475ea23ed79f074c3f4143b86e6efb557d37 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Fri, 2 May 2025 09:59:01 -0700
Subject: [PATCH] [lldb-dap] Change the launch sequence
This PR changes h
@@ -333,6 +333,7 @@ def attach(
exitCommands=None,
attachCommands=None,
coreFile=None,
+stopOnAttach=True,
ashgti wrote:
Sounds good, we can follow up with some other improvements to tests.
https://github.com/llvm/llvm-project/
https://github.com/ashgti approved this pull request.
Yesterday I enabled all the lldb-dap tests that support macOS and I noticed
#138197 where some tests `attach` failed due to attaching to a parallel test
process. I was consistently hitting it on my MBP but I think due to timing not
on my Ma
@@ -137,55 +137,69 @@ void AttachRequestHandler::operator()(const
llvm::json::Object &request) const {
dap.SendOutput(OutputType::Console,
llvm::StringRef(attach_msg, attach_msg_len));
}
- if (attachCommands.empty()) {
-// No "attachCommands", jus
@@ -591,6 +591,7 @@ def request_attach(
attachCommands=None,
terminateCommands=None,
coreFile=None,
+stopOnAttach=True,
JDevlieghere wrote:
See my reply to John:
https://github.com/llvm/llvm-project/pull/138219#discussion_r2072
@@ -137,55 +137,69 @@ void AttachRequestHandler::operator()(const
llvm::json::Object &request) const {
dap.SendOutput(OutputType::Console,
llvm::StringRef(attach_msg, attach_msg_len));
}
- if (attachCommands.empty()) {
-// No "attachCommands", jus
@@ -25,9 +25,10 @@ def spawn_and_wait(program, delay):
process.wait()
-@skipIf
class TestDAP_attach(lldbdap_testcase.DAPTestCaseBase):
def set_and_hit_breakpoint(self, continueToExit=True):
+self.dap_server.wait_for_stopped()
kusmour wrote:
@@ -427,6 +430,7 @@ def cleanup():
# Initialize and launch the program
self.dap_server.request_initialize(sourceInitFile)
+self.dap_server.request_configurationDone()
kusmour wrote:
ditto
https://github.com/llvm/llvm-project/pull/1382
@@ -235,7 +235,7 @@ def test_auto_completions(self):
"""
Tests completion requests in "repl-mode=auto"
"""
-self.setup_debugee()
+self.setup_debugee(stopOnEntry=True)
kusmour wrote:
Any reason why we are changing the tes
@@ -591,6 +591,7 @@ def request_attach(
attachCommands=None,
terminateCommands=None,
coreFile=None,
+stopOnAttach=True,
kusmour wrote:
Is there any reason we want to make this true by default?
1) Our current tests don't have sto
@@ -88,8 +88,8 @@ def test_stopOnEntry(self):
"""
program = self.getBuildArtifact("a.out")
self.build_and_launch(program, stopOnEntry=True)
-self.set_function_breakpoints(["main"])
-stopped_events = self.continue_to_next_stop()
+
@@ -357,6 +358,7 @@ def cleanup():
self.addTearDownHook(cleanup)
# Initialize and launch the program
self.dap_server.request_initialize(sourceInitFile)
+self.dap_server.request_configurationDone()
kusmour wrote:
Shouldn't we wai
https://github.com/kusmour requested changes to this pull request.
https://github.com/llvm/llvm-project/pull/138219
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
@@ -333,6 +333,7 @@ def attach(
exitCommands=None,
attachCommands=None,
coreFile=None,
+stopOnAttach=True,
JDevlieghere wrote:
The way the tests are currently written, they expect to be stopped after the
launch (presumably beca
@@ -591,6 +591,7 @@ def request_attach(
attachCommands=None,
terminateCommands=None,
coreFile=None,
+stopOnAttach=True,
ashgti wrote:
Should we default to `False` for attach requests?
https://github.com/llvm/llvm-project/pull/1
@@ -333,6 +333,7 @@ def attach(
exitCommands=None,
attachCommands=None,
coreFile=None,
+stopOnAttach=True,
ashgti wrote:
Should we default this to `False`?
Or should we have the `DAPTestCaseBase` case `attach` and `launch` help
JDevlieghere wrote:
> > @kusmour Can you please confirm whether or not there's a reason to run the
> > launch or attach commands in asynchronous mode? It looks like the
> > divergence was introduced when the feature was added
> > (https://reviews.llvm.org/D154028) and I'm not sure it's necessa
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Jonas Devlieghere (JDevlieghere)
Changes
This PR changes how we treat the launch sequence in lldb-dap.
- Send the initialized event after we finish handling the initialize
request, rather than after we finish attaching or launching.
-
https://github.com/JDevlieghere ready_for_review
https://github.com/llvm/llvm-project/pull/138219
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/138219
>From e7333489e50b1efe16203e8fd7b6dc7f2dcd4439 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Fri, 2 May 2025 09:59:01 -0700
Subject: [PATCH 1/3] [lldb-dap] Change the launch sequence
This PR chang
@@ -893,10 +893,23 @@ llvm::Error DAP::Loop() {
return errWrapper;
}
+ // The launch sequence is special and we need to carefully handle
+ // packets in the right order. The launch and attach requests cannot
+ // be answered unt
@@ -893,10 +893,23 @@ llvm::Error DAP::Loop() {
return errWrapper;
}
+ // The launch sequence is special and we need to carefully handle
+ // packets in the right order. The launch and attach requests cannot
+ // be answered unt
kusmour wrote:
> @kusmour Can you please confirm whether or not there's a reason to run the
> launch or attach commands in asynchronous mode? It looks like the divergence
> was introduced when the feature was added (https://reviews.llvm.org/D154028)
> and I'm not sure it's necessary. My local
@@ -893,10 +893,23 @@ llvm::Error DAP::Loop() {
return errWrapper;
}
+ // The launch sequence is special and we need to carefully handle
+ // packets in the right order. The launch and attach requests cannot
+ // be answered unt
@@ -893,10 +893,23 @@ llvm::Error DAP::Loop() {
return errWrapper;
}
+ // The launch sequence is special and we need to carefully handle
+ // packets in the right order. The launch and attach requests cannot
+ // be answered unt
@@ -893,10 +893,23 @@ llvm::Error DAP::Loop() {
return errWrapper;
}
+ // The launch sequence is special and we need to carefully handle
+ // packets in the right order. The launch and attach requests cannot
+ // be answered unt
@@ -893,10 +893,23 @@ llvm::Error DAP::Loop() {
return errWrapper;
}
+ // The launch sequence is special and we need to carefully handle
+ // packets in the right order. The launch and attach requests cannot
+ // be answered unt
@@ -938,15 +954,30 @@ llvm::Error DAP::Loop() {
StopEventHandlers();
});
- while (!disconnecting || !m_queue.empty()) {
+ while (true) {
std::unique_lock lock(m_queue_mutex);
-m_queue_cv.wait(lock, [&] { return disconnecting || !m_queue.empty(); });
+m_que
@@ -417,8 +424,11 @@ struct DAP {
lldb::SBMutex GetAPIMutex() const { return target.GetAPIMutex(); }
private:
- std::mutex m_queue_mutex;
+ /// Queue for all incoming messages.
std::deque m_queue;
+ /// Dedicated queue for launching and attaching.
+ std::deque m_launc
@@ -161,20 +161,22 @@ static void EventThreadFunction(DAP &dap) {
case lldb::eStateSuspended:
break;
case lldb::eStateStopped:
-// We launch and attach in synchronous mode then the first stop
-// event will not be delivere
JDevlieghere wrote:
@kusmour Can you please confirm whether or not there's a reason to run the
launch or attach commands in asynchronous mode? It looks like the divergence
was introduced when the feature was added (https://reviews.llvm.org/D154028)
and I'm not sure it's necessary. My local tes
https://github.com/JDevlieghere edited
https://github.com/llvm/llvm-project/pull/138219
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
@@ -417,8 +424,11 @@ struct DAP {
lldb::SBMutex GetAPIMutex() const { return target.GetAPIMutex(); }
private:
- std::mutex m_queue_mutex;
+ /// Queue for all incoming messages.
std::deque m_queue;
+ /// Dedicated queue for launching and attaching.
+ std::deque m_launc
@@ -161,20 +161,22 @@ static void EventThreadFunction(DAP &dap) {
case lldb::eStateSuspended:
break;
case lldb::eStateStopped:
-// We launch and attach in synchronous mode then the first stop
-// event will not be delivere
https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/138219
>From e7333489e50b1efe16203e8fd7b6dc7f2dcd4439 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Fri, 2 May 2025 09:59:01 -0700
Subject: [PATCH] [lldb-dap] Change the launch sequence
This PR changes h
@@ -161,20 +161,22 @@ static void EventThreadFunction(DAP &dap) {
case lldb::eStateSuspended:
break;
case lldb::eStateStopped:
-// We launch and attach in synchronous mode then the first stop
-// event will not be delivere
@@ -161,20 +161,22 @@ static void EventThreadFunction(DAP &dap) {
case lldb::eStateSuspended:
break;
case lldb::eStateStopped:
-// We launch and attach in synchronous mode then the first stop
-// event will not be delivere
@@ -417,8 +424,11 @@ struct DAP {
lldb::SBMutex GetAPIMutex() const { return target.GetAPIMutex(); }
private:
- std::mutex m_queue_mutex;
+ /// Queue for all incoming messages.
std::deque m_queue;
+ /// Dedicated queue for launching and attaching.
+ std::deque m_launc
@@ -161,20 +161,22 @@ static void EventThreadFunction(DAP &dap) {
case lldb::eStateSuspended:
break;
case lldb::eStateStopped:
ashgti wrote:
May be unrelated but in
[`lldb_private::StateIsRunningState`](https://github.com/llvm/
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 HEAD~1 HEAD --extensions h,cpp --
lldb/tools/lldb-dap/DAP.cpp lldb/tools/lldb-dap/DAP
https://github.com/JDevlieghere created
https://github.com/llvm/llvm-project/pull/138219
This PR changes how we treat the launch sequence in lldb-dap.
- Send the initialized event after we finish handling the initialize request,
rather than after we finish attaching or launching.
- Delay han
55 matches
Mail list logo
