[Lldb-commits] [PATCH] D128956: make debugserver able to inspect mach-o binaries present in memory, but not yet registered with dyld

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added inline comments.



Comment at: lldb/tools/debugserver/source/MacOSX/MachProcess.mm:1186-1191
+  // dyld doesn't think there is a binary at this address,
+  // but maybe there isn't a binary YET - let's look in memory
+  // for a proper mach-o header etc and return what we can.
+  // We will have an empty filename for the binary (because dyld
+  // doesn't know about it yet) but we can read all of the mach-o
+  // load commands from memory directly.

JDevlieghere wrote:
> What happens when it turns out that there's no Mach-O header at this load 
> address? I expect it doesn't end up in the `jGetLoadedDynamicLibrariesInfos` 
> response then? Where does this filtering take place?
Yeah, it's a good question.  `MachProcess::GetMachOInformationFromMemory` does 
the Mach-O parsing, and reviewing that code, I do some sanity checking but I 
don't check for the magic number in the header.  At the very least, I should 
add that check.   I should also check the return value from 
`GetMachOInformationFromMemory` and not add the entry to my list of binaries 
I'm going to respond to lldb with.  So, a bit more work here would be a good 
idea, agreed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128956/new/

https://reviews.llvm.org/D128956

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

[Lldb-commits] [PATCH] D128932: [lldb] [llgs] Improve stdio forwarding in multiprocess+nonstop

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 441624.
mgorny added a comment.

Use semaphores to sync instead of relying on ugly sleeps.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128932/new/

https://reviews.llvm.org/D128932

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
  lldb/test/API/tools/lldb-server/main.cpp

Index: lldb/test/API/tools/lldb-server/main.cpp
===
--- lldb/test/API/tools/lldb-server/main.cpp
+++ lldb/test/API/tools/lldb-server/main.cpp
@@ -10,8 +10,10 @@
 #include 
 #if !defined(_WIN32)
 #include 
+#include 
 #include 
 #include 
+#include 
 #endif
 #include "thread.h"
 #include 
@@ -26,6 +28,13 @@
 #include 
 #endif
 
+#if !defined(_WIN32)
+struct semaphores {
+  sem_t parent_ready;
+  sem_t child_ready;
+};
+#endif
+
 static const char *const PRINT_PID_COMMAND = "print-pid";
 
 static bool g_print_thread_ids = false;
@@ -224,6 +233,9 @@
   int return_value = 0;
 
 #if !defined(_WIN32)
+  semaphores *sem = nullptr;
+  bool is_child = false;
+
   // Set the signal handler.
   sig_t sig_result = signal(SIGALRM, signal_handler);
   if (sig_result == SIG_ERR) {
@@ -324,10 +336,29 @@
   func_p();
 #if !defined(_WIN32) && !defined(TARGET_OS_WATCH) && !defined(TARGET_OS_TV)
 } else if (arg == "fork") {
-  assert (fork() != -1);
+  // Prepare the semaphores for parent-child synchronization.
+  if (sem == nullptr) {
+sem = static_cast(mmap(nullptr, sizeof(*sem),
+ PROT_READ | PROT_WRITE,
+ MAP_ANON | MAP_SHARED, -1, 0));
+assert(sem);
+assert(sem_init(&sem->parent_ready, 1, 0) == 0);
+assert(sem_init(&sem->child_ready, 1, 0) == 0);
+  }
+
+  pid_t fork_pid = fork();
+  assert(fork_pid != -1);
+  is_child = fork_pid == 0;
 } else if (arg == "vfork") {
   if (vfork() == 0)
 _exit(0);
+} else if (arg == "process:sync") {
+  // this is only valid after fork
+  assert(sem);
+  sem_t *my_sem = is_child ? &sem->child_ready : &sem->parent_ready;
+  sem_t *other_sem = !is_child ? &sem->child_ready : &sem->parent_ready;
+  assert(sem_post(my_sem) == 0);
+  assert(sem_wait(other_sem) == 0);
 #endif
 } else if (consume_front(arg, "thread:new")) {
   std::promise promise;
@@ -368,5 +399,13 @@
it != threads.end(); ++it)
 it->join();
 
+  if (sem != nullptr) {
+if (!is_child) {
+  assert(sem_destroy(&sem->child_ready));
+  assert(sem_destroy(&sem->parent_ready));
+}
+assert(munmap(sem, sizeof(*sem)) == 0);
+  }
+
   return return_value;
 }
Index: lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
@@ -1,3 +1,5 @@
+import binascii
+
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
@@ -107,3 +109,31 @@
 def test_vCont_interspersed_nonstop(self):
 self.resume_one_test(run_order=["parent", "child", "parent", "child"],
  use_vCont=True, nonstop=True)
+
+@add_test_categories(["fork"])
+def test_c_both_nonstop(self):
+parent_pid, parent_tid, child_pid, child_tid = (
+self.start_fork_test(["fork", "process:sync", "print-pid",
+  "process:sync", "stop"],
+ nonstop=True))
+
+self.test_sequence.add_log_lines([
+"read packet: $Hcp{}.{}#00".format(parent_pid, parent_tid),
+"send packet: $OK#00",
+"read packet: $c#00",
+"send packet: $OK#00",
+"read packet: $Hcp{}.{}#00".format(child_pid, child_tid),
+"send packet: $OK#00",
+"read packet: $c#00",
+"send packet: $OK#00",
+{"direction": "send", "regex": "%Stop:T.*"},
+# see the comment in TestNonStop.py, test_stdio
+"read packet: $vStdio#00",
+"read packet: $vStdio#00",
+"send packet: $OK#00",
+], True)
+ret = self.expect_gdbremote_sequence()
+self.assertIn("PID: {}".format(int(parent_pid, 16)).encode(),
+  ret["O_content"])
+self.assertIn("PID: {}".format(int(child_pid, 16)).encode(),
+  ret["O_content"])
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1107,14 +1107,16 @@
 SendProcessOutput();
 // Then stop

[Lldb-commits] [PATCH] D128932: [lldb] [llgs] Improve stdio forwarding in multiprocess+nonstop

[Michał Górny via Phabricator via lldb-commits]

mgorny added inline comments.



Comment at: lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py:116
+parent_pid, parent_tid, child_pid, child_tid = (
+self.start_fork_test(["fork", "sleep:2", "print-pid", "sleep:2",
+  "stop"],

mgorny wrote:
> I really dislike these sleeps but I can't think of a better way of doing it 
> (short of using IPC for synchronization). The goal is to 1) ensure that both 
> processes start before they start outputting, and 2) ensure that both output 
> before the first stop reason comes.
Ok, semaphores are not scary after all, and I suppose we can expect them to 
work if we expect `fork()` to work.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128932/new/

https://reviews.llvm.org/D128932

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

[Lldb-commits] [lldb] 3d477bb - [LLDB] Xfail TestStepNoDebug.py AArch64/Windows

[Muhammad Omair Javaid via lldb-commits]

Author: Muhammad Omair Javaid
Date: 2022-07-01T12:25:43+04:00
New Revision: 3d477bbeee55ca965553778e62085a37d062323f

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

LOG: [LLDB] Xfail TestStepNoDebug.py AArch64/Windows

LLDB fails to step in/out/over code with missing debug information.
This is only reproducible on AArch64/Windows. I have reported a issue
upstream at llvm.org/pr56292

This patch Xfail TestStepNoDebug.py for AArch64/Windows.

Added: 


Modified: 
lldb/test/API/functionalities/step-avoids-no-debug/TestStepNoDebug.py

Removed: 




diff  --git 
a/lldb/test/API/functionalities/step-avoids-no-debug/TestStepNoDebug.py 
b/lldb/test/API/functionalities/step-avoids-no-debug/TestStepNoDebug.py
index 9c72b0f03f791..98b80051368a9 100644
--- a/lldb/test/API/functionalities/step-avoids-no-debug/TestStepNoDebug.py
+++ b/lldb/test/API/functionalities/step-avoids-no-debug/TestStepNoDebug.py
@@ -14,6 +14,7 @@
 class StepAvoidsNoDebugTestCase(TestBase):
 
 @add_test_categories(['pyapi'])
+@expectedFailureAll(archs=["aarch64"], oslist=["windows"], 
bugnumber="llvm.org/pr56292")
 def test_step_out_with_python(self):
 """Test stepping out using avoid-no-debug with dsyms."""
 self.build()
@@ -31,6 +32,7 @@ def test_step_out_with_python(self):
 archs=["i386"],
 oslist=no_match(["freebsd"]),
 bugnumber="llvm.org/pr28549")
+@expectedFailureAll(archs=["aarch64"], oslist=["windows"], 
bugnumber="llvm.org/pr56292")
 def test_step_over_with_python(self):
 """Test stepping over using avoid-no-debug with dwarf."""
 self.build()
@@ -48,6 +50,7 @@ def test_step_over_with_python(self):
 archs=["i386"],
 oslist=no_match(["freebsd"]),
 bugnumber="llvm.org/pr28549")
+@expectedFailureAll(archs=["aarch64"], oslist=["windows"], 
bugnumber="llvm.org/pr56292")
 def test_step_in_with_python(self):
 """Test stepping in using avoid-no-debug with dwarf."""
 self.build()



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

[Lldb-commits] [PATCH] D128932: [lldb] [llgs] Improve stdio forwarding in multiprocess+nonstop

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 441664.
mgorny added a comment.

Fix `sem_destroy()` assertions.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128932/new/

https://reviews.llvm.org/D128932

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
  lldb/test/API/tools/lldb-server/main.cpp

Index: lldb/test/API/tools/lldb-server/main.cpp
===
--- lldb/test/API/tools/lldb-server/main.cpp
+++ lldb/test/API/tools/lldb-server/main.cpp
@@ -10,8 +10,10 @@
 #include 
 #if !defined(_WIN32)
 #include 
+#include 
 #include 
 #include 
+#include 
 #endif
 #include "thread.h"
 #include 
@@ -26,6 +28,13 @@
 #include 
 #endif
 
+#if !defined(_WIN32)
+struct semaphores {
+  sem_t parent_ready;
+  sem_t child_ready;
+};
+#endif
+
 static const char *const PRINT_PID_COMMAND = "print-pid";
 
 static bool g_print_thread_ids = false;
@@ -224,6 +233,9 @@
   int return_value = 0;
 
 #if !defined(_WIN32)
+  semaphores *sem = nullptr;
+  bool is_child = false;
+
   // Set the signal handler.
   sig_t sig_result = signal(SIGALRM, signal_handler);
   if (sig_result == SIG_ERR) {
@@ -324,10 +336,29 @@
   func_p();
 #if !defined(_WIN32) && !defined(TARGET_OS_WATCH) && !defined(TARGET_OS_TV)
 } else if (arg == "fork") {
-  assert (fork() != -1);
+  // Prepare the semaphores for parent-child synchronization.
+  if (sem == nullptr) {
+sem = static_cast(mmap(nullptr, sizeof(*sem),
+ PROT_READ | PROT_WRITE,
+ MAP_ANON | MAP_SHARED, -1, 0));
+assert(sem);
+assert(sem_init(&sem->parent_ready, 1, 0) == 0);
+assert(sem_init(&sem->child_ready, 1, 0) == 0);
+  }
+
+  pid_t fork_pid = fork();
+  assert(fork_pid != -1);
+  is_child = fork_pid == 0;
 } else if (arg == "vfork") {
   if (vfork() == 0)
 _exit(0);
+} else if (arg == "process:sync") {
+  // this is only valid after fork
+  assert(sem);
+  sem_t *my_sem = is_child ? &sem->child_ready : &sem->parent_ready;
+  sem_t *other_sem = !is_child ? &sem->child_ready : &sem->parent_ready;
+  assert(sem_post(my_sem) == 0);
+  assert(sem_wait(other_sem) == 0);
 #endif
 } else if (consume_front(arg, "thread:new")) {
   std::promise promise;
@@ -368,5 +399,13 @@
it != threads.end(); ++it)
 it->join();
 
+  if (sem != nullptr) {
+if (!is_child) {
+  assert(sem_destroy(&sem->child_ready) == 0);
+  assert(sem_destroy(&sem->parent_ready) == 0);
+}
+assert(munmap(sem, sizeof(*sem)) == 0);
+  }
+
   return return_value;
 }
Index: lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
@@ -1,3 +1,5 @@
+import binascii
+
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
@@ -107,3 +109,31 @@
 def test_vCont_interspersed_nonstop(self):
 self.resume_one_test(run_order=["parent", "child", "parent", "child"],
  use_vCont=True, nonstop=True)
+
+@add_test_categories(["fork"])
+def test_c_both_nonstop(self):
+parent_pid, parent_tid, child_pid, child_tid = (
+self.start_fork_test(["fork", "process:sync", "print-pid",
+  "process:sync", "stop"],
+ nonstop=True))
+
+self.test_sequence.add_log_lines([
+"read packet: $Hcp{}.{}#00".format(parent_pid, parent_tid),
+"send packet: $OK#00",
+"read packet: $c#00",
+"send packet: $OK#00",
+"read packet: $Hcp{}.{}#00".format(child_pid, child_tid),
+"send packet: $OK#00",
+"read packet: $c#00",
+"send packet: $OK#00",
+{"direction": "send", "regex": "%Stop:T.*"},
+# see the comment in TestNonStop.py, test_stdio
+"read packet: $vStdio#00",
+"read packet: $vStdio#00",
+"send packet: $OK#00",
+], True)
+ret = self.expect_gdbremote_sequence()
+self.assertIn("PID: {}".format(int(parent_pid, 16)).encode(),
+  ret["O_content"])
+self.assertIn("PID: {}".format(int(child_pid, 16)).encode(),
+  ret["O_content"])
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1107,14 +1107,16 @@
 SendProcessOutput();
 // Then stop the forwarding,

[Lldb-commits] [lldb] b15b142 - [lldb/test] Don't use preexec_fn for launching inferiors

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2022-07-01T14:36:01+02:00
New Revision: b15b1421bc9a11b318b65b489e5fd58dd917db1f

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

LOG: [lldb/test] Don't use preexec_fn for launching inferiors

As the documentation states, using this is not safe in multithreaded
programs, and I have traced it to a rare deadlock in some of the tests.

The reason this was introduced was to be able to attach to a program
from the very first instruction, where our usual mechanism of
synchronization -- waiting for a file to appear -- does not work.

However, this is only needed for a single test
(TestGdbRemoteAttachWait) so instead of doing this everywhere, I create
a bespoke solution for that single test. The solution basically
consists of outsourcing the preexec_fn code to a separate (and
single-threaded) shim process, which enables attaching and then executes
the real program.

This pattern could be generalized in case we needed to use it for other
tests, but I suspect that we will not be having many tests like this.

This effectively reverts commit
a997a1d7fbe229433fb458bb0035b32424ecf3bd.

Added: 
lldb/test/API/tools/lldb-server/attach-wait/Makefile
lldb/test/API/tools/lldb-server/attach-wait/TestGdbRemoteAttachWait.py
lldb/test/API/tools/lldb-server/attach-wait/main.cpp
lldb/test/API/tools/lldb-server/attach-wait/shim.cpp

Modified: 
lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
lldb/packages/Python/lldbsuite/test/lldbtest.py

Removed: 
lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py



diff  --git a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py 
b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
index e14c4f8c0d38..719131c9248e 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbplatformutil.py
@@ -4,12 +4,11 @@
 from __future__ import absolute_import
 
 # System modules
-import ctypes
 import itertools
-import os
 import re
 import subprocess
 import sys
+import os
 
 # Third-party modules
 import six
@@ -192,14 +191,3 @@ def hasChattyStderr(test_case):
 if match_android_device(test_case.getArchitecture(), ['aarch64'], 
range(22, 25+1)):
 return True  # The dynamic linker on the device will complain about 
unknown DT entries
 return False
-
-if getHostPlatform() == "linux":
-def enable_attach():
-"""Enable attaching to _this_ process, if host requires such an action.
-Suitable for use as a preexec_fn in subprocess.Popen and similar."""
-c = ctypes.CDLL(None)
-PR_SET_PTRACER = ctypes.c_int(0x59616d61)
-PR_SET_PTRACER_ANY = ctypes.c_ulong(-1)
-c.prctl(PR_SET_PTRACER, PR_SET_PTRACER_ANY)
-else:
-enable_attach = None

diff  --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py 
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 22851730153e..d46e54f30bd5 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -393,7 +393,6 @@ def launch(self, executable, args, extra_env):
 stdout=open(
 os.devnull) if not self._trace_on else None,
 stdin=PIPE,
-preexec_fn=lldbplatformutil.enable_attach,
 env=env)
 
 def terminate(self):

diff  --git a/lldb/test/API/tools/lldb-server/attach-wait/Makefile 
b/lldb/test/API/tools/lldb-server/attach-wait/Makefile
new file mode 100644
index ..22f1051530f8
--- /dev/null
+++ b/lldb/test/API/tools/lldb-server/attach-wait/Makefile
@@ -0,0 +1 @@
+include Makefile.rules

diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py 
b/lldb/test/API/tools/lldb-server/attach-wait/TestGdbRemoteAttachWait.py
similarity index 80%
rename from lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
rename to lldb/test/API/tools/lldb-server/attach-wait/TestGdbRemoteAttachWait.py
index ed31b5645c33..abcc98d22090 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteAttachWait.py
+++ b/lldb/test/API/tools/lldb-server/attach-wait/TestGdbRemoteAttachWait.py
@@ -14,10 +14,12 @@ class 
TestGdbRemoteAttachWait(gdbremote_testcase.GdbRemoteTestCaseBase):
 @skipIfWindows # This test is flaky on Windows
 def test_attach_with_vAttachWait(self):
 exe = '%s_%d' % (self.testMethodName, os.getpid())
+exe_to_attach = exe
+args = []
 
 def launch_inferior():
 inferior = self.launch_process_for_attach(
-inferior_args=["sleep:60"],
+inferior_args=args,
 exe_path=self.getBuildArtifact(exe))
 self.assertIsNotNone(inferior)
 self.assertTrue(inferior.pid > 0)
@@ -26,7 +28,14 @@ def launch_inferior():
   

[Lldb-commits] [PATCH] D126983: [lldb] [llgs] Support "t" vCont action

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Some flakyness here: 
https://lab.llvm.org/buildbot/#/builders/68/builds/35111/steps/6/logs/stdio


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126983/new/

https://reviews.llvm.org/D126983

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

[Lldb-commits] [PATCH] D128989: [lldb] [llgs] Support resuming multiple processes via vCont w/ nonstop

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski, jingham.
Herald added a subscriber: arichardson.
Herald added a project: All.
mgorny requested review of this revision.

Support using the vCont packet to resume multiple processes
simultaneously when in non-stop mode.  The new logic now assumes that:

- actions without a thread-id or with process id of "p-1" apply to all debugged 
processes

- actions with a thread-id without process id apply to the current process 
(m_continue_process)

As with the other continue packets, it is only possible to resume
processes that are currently stopped (or stop these that are running).
It is unsupported to resume or stop individual threads of a running
process.

Sponsored by: The FreeBSD Foundation


https://reviews.llvm.org/D128989

Files:
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py

Index: lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
===
--- lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteForkNonStop.py
@@ -137,3 +137,55 @@
   ret["O_content"])
 self.assertIn("PID: {}".format(int(child_pid, 16)).encode(),
   ret["O_content"])
+
+@add_test_categories(["fork"])
+def test_vCont_both_nonstop(self):
+parent_pid, parent_tid, child_pid, child_tid = (
+self.start_fork_test(["fork", "process:sync", "print-pid",
+  "process:sync", "stop"],
+ nonstop=True))
+
+self.test_sequence.add_log_lines([
+"read packet: $vCont;c:p{}.{};c:p{}.{}#00".format(
+parent_pid, parent_tid, child_pid, child_tid),
+"send packet: $OK#00",
+{"direction": "send", "regex": "%Stop:T.*"},
+# see the comment in TestNonStop.py, test_stdio
+"read packet: $vStdio#00",
+"read packet: $vStdio#00",
+"send packet: $OK#00",
+], True)
+ret = self.expect_gdbremote_sequence()
+self.assertIn("PID: {}".format(int(parent_pid, 16)).encode(),
+  ret["O_content"])
+self.assertIn("PID: {}".format(int(child_pid, 16)).encode(),
+  ret["O_content"])
+
+def vCont_both_nonstop_test(self, vCont_packet):
+parent_pid, parent_tid, child_pid, child_tid = (
+self.start_fork_test(["fork", "process:sync", "print-pid",
+  "process:sync", "stop"],
+ nonstop=True))
+
+self.test_sequence.add_log_lines([
+"read packet: ${}#00".format(vCont_packet),
+"send packet: $OK#00",
+{"direction": "send", "regex": "%Stop:T.*"},
+# see the comment in TestNonStop.py, test_stdio
+"read packet: $vStdio#00",
+"read packet: $vStdio#00",
+"send packet: $OK#00",
+], True)
+ret = self.expect_gdbremote_sequence()
+self.assertIn("PID: {}".format(int(parent_pid, 16)).encode(),
+  ret["O_content"])
+self.assertIn("PID: {}".format(int(child_pid, 16)).encode(),
+  ret["O_content"])
+
+@add_test_categories(["fork"])
+def test_vCont_both_implicit_nonstop(self):
+self.vCont_both_nonstop_test("vCont;c")
+
+@add_test_categories(["fork"])
+def test_vCont_both_minus_one_nonstop(self):
+self.vCont_both_nonstop_test("vCont;c:p-1.-1")
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -1751,6 +1751,7 @@
   break;
 }
 
+// If there's no thread-id (e.g. "vCont;c"), it's "p-1.-1".
 lldb::pid_t pid = StringExtractorGDBRemote::AllProcesses;
 lldb::tid_t tid = StringExtractorGDBRemote::AllThreads;
 
@@ -1759,37 +1760,40 @@
   // Consume the separator.
   packet.GetChar();
 
-  auto pid_tid = packet.GetPidTid(StringExtractorGDBRemote::AllProcesses);
+  auto pid_tid = packet.GetPidTid(LLDB_INVALID_PROCESS_ID);
   if (!pid_tid)
 return SendIllFormedResponse(packet, "Malformed thread-id");
 
   pid = pid_tid->first;
   tid = pid_tid->second;
-}
 
-if (pid == StringExtractorGDBRemote::AllProcesses) {
-  if (m_debugged_processes.size() > 1)
-return SendIllFormedResponse(
-packet, "Resuming multiple processes not supported yet");
-  if (!m_continue_process) {
-LLDB_LOG(log, "no debugged process");
-return SendErrorResponse(0x36);
+  // If we get TID without P

[Lldb-commits] [PATCH] D126983: [lldb] [llgs] Support "t" vCont action

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D126983#3624800 , @labath wrote:

> Some flakyness here: 
> https://lab.llvm.org/buildbot/#/builders/68/builds/35111/steps/6/logs/stdio

That's so rare it's going to be hard figuring out if any changes actually fix 
it. My only idea is to either increase the sleep within 
`continue_and_get_threads_running()` or make output inside the process more 
frequent — though the latter probably won't help if some threads don't get run 
because of loaded system.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126983/new/

https://reviews.llvm.org/D126983

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

[Lldb-commits] [PATCH] D126983: [lldb] [llgs] Support "t" vCont action

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D126983#3624901 , @mgorny wrote:

> In D126983#3624800 , @labath wrote:
>
>> Some flakyness here: 
>> https://lab.llvm.org/buildbot/#/builders/68/builds/35111/steps/6/logs/stdio
>
> That's so rare it's going to be hard figuring out if any changes actually fix 
> it. My only idea is to either increase the sleep within 
> `continue_and_get_threads_running()` or make output inside the process more 
> frequent — though the latter probably won't help if some threads don't get 
> run because of loaded system.

Don't we have the ability to wait for a specific output from the process? Could 
we make use of that to await for the actual "thread is running" message? If 
needed, we can put some barriers between the print calls, and then we can even 
make sure each thread prints its message exactly once.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126983/new/

https://reviews.llvm.org/D126983

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

[Lldb-commits] [PATCH] D126983: [lldb] [llgs] Support "t" vCont action

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

I don't think we can reliably check whether additional threads aren't run then.

I have another idea that could work better if we could assume that the 
scheduler runs threads of the same process somehow evenly — or well, I suppose 
it'd be better in any case. Rather than stopping the process from LLDB, I could 
add a counter that issues SIGSTOP from inside the process after doing N (say, 
5) prints. I suppose that should give it enough time for all threads to output 
something.

Just need to figure out a simple synchronization method to ensure that SIGSTOP 
is emitted only once.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126983/new/

https://reviews.llvm.org/D126983

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

[Lldb-commits] [PATCH] D126983: [lldb] [llgs] Support "t" vCont action

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D126983#3624915 , @labath wrote:

> Don't we have the ability to wait for a specific output from the process?

Hmm, actually thinking about it, I suppose we could wait for m*n messages from 
m threads (say, n=5) and then we should have a pretty good chance that if an 
extra thread is running, it would fit somewhere between them.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126983/new/

https://reviews.llvm.org/D126983

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

[Lldb-commits] [PATCH] D126983: [lldb] [llgs] Support "t" vCont action

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

Unfortunately, switching to `output_match` makes the test even more flaky. I 
think it doesn't handle it well if two output lines get sent as a single packet.

That said, the other tests are probably flaky the other way — they rely on all 
signals being reported in a single output packet.

I'm going to continue trying to improve the test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126983/new/

https://reviews.llvm.org/D126983

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

[Lldb-commits] [PATCH] D50304: [lldb] Fix thread step until to not set breakpoint(s) on incorrect line numbers

[Venkata Ramanaiah Nalamothu via Phabricator via lldb-commits]

RamNalamothu updated this revision to Diff 441726.
RamNalamothu added a comment.

Make the test case insensitive to compiler optimizations.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D50304/new/

https://reviews.llvm.org/D50304

Files:
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
  lldb/test/API/functionalities/thread/step_until/main.c


Index: lldb/test/API/functionalities/thread/step_until/main.c
===
--- lldb/test/API/functionalities/thread/step_until/main.c
+++ lldb/test/API/functionalities/thread/step_until/main.c
@@ -1,20 +1,25 @@
 #include 
 
-void call_me(int argc)
-{
+/* The return statements are purposefully so simple, and
+ * unrelated to the program, just to achieve consistent
+ * debug line tables, across platforms, that are not
+ * dependent on compiler optimzations. */
+int call_me(int argc) {
   printf ("At the start, argc: %d.\n", argc);
 
   if (argc < 2)
-printf("Less than 2.\n");
+return 1; /* Less than 2. */
   else
-printf("Greater than or equal to 2.\n");
+return argc; /* Greater than or equal to 2. */
 }
 
 int
 main(int argc, char **argv)
 {
-  call_me(argc);
-  printf("Back out in main.\n");
+  int res = 0;
+  res = call_me(argc); /* Back out in main. */
+  if (res)
+printf("Result: %d. \n", res);
 
   return 0;
 }
Index: lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
===
--- lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
+++ lldb/test/API/functionalities/thread/step_until/TestStepUntil.py
@@ -19,7 +19,7 @@
 self.greater_than_two = line_number('main.c', 'Greater than or equal 
to 2.')
 self.back_out_in_main = line_number('main.c', 'Back out in main')
 
-def do_until (self, args, until_lines, expected_linenum):
+def common_setup (self, args):
 self.build()
 exe = self.getBuildArtifact("a.out")
 
@@ -48,7 +48,8 @@
 thread = threads[0]
 return thread
 
-thread = self.common_setup(None)
+def do_until (self, args, until_lines, expected_linenum):
+thread = self.common_setup(args)
 
 cmd_interp = self.dbg.GetCommandInterpreter()
 ret_obj = lldb.SBCommandReturnObject()
@@ -77,7 +78,7 @@
 self.do_until(None, [self.less_than_two, self.greater_than_two], 
self.less_than_two)
 
 def test_missing_one (self):
-"""Test thread step until - targeting one line and missing it."""
+"""Test thread step until - targeting one line and missing it by 
stepping out to call site"""
 self.do_until(["foo", "bar", "baz"], [self.less_than_two], 
self.back_out_in_main)
 
 
Index: lldb/source/Commands/CommandObjectThread.cpp
===
--- lldb/source/Commands/CommandObjectThread.cpp
+++ lldb/source/Commands/CommandObjectThread.cpp
@@ -1033,11 +1033,21 @@
 line_table->FindLineEntryByAddress(fun_end_addr, function_start,
&end_ptr);
 
+// Since not all source lines will contribute code, check if we are
+// setting the breakpoint on the exact line number or the nearest
+// subsequent line number and set breakpoints at all the line table
+// entries of the chosen line number (exact or nearest subsequent).
 for (uint32_t line_number : line_numbers) {
+  LineEntry line_entry;
+  bool exact = false;
   uint32_t start_idx_ptr = index_ptr;
+  start_idx_ptr = sc.comp_unit->FindLineEntry(
+  index_ptr, line_number, nullptr, exact, &line_entry);
+  if (start_idx_ptr != UINT32_MAX)
+line_number = line_entry.line;
+  exact = true;
+  start_idx_ptr = index_ptr;
   while (start_idx_ptr <= end_ptr) {
-LineEntry line_entry;
-const bool exact = false;
 start_idx_ptr = sc.comp_unit->FindLineEntry(
 start_idx_ptr, line_number, nullptr, exact, &line_entry);
 if (start_idx_ptr == UINT32_MAX)


Index: lldb/test/API/functionalities/thread/step_until/main.c
===
--- lldb/test/API/functionalities/thread/step_until/main.c
+++ lldb/test/API/functionalities/thread/step_until/main.c
@@ -1,20 +1,25 @@
 #include 
 
-void call_me(int argc)
-{
+/* The return statements are purposefully so simple, and
+ * unrelated to the program, just to achieve consistent
+ * debug line tables, across platforms, that are not
+ * dependent on compiler optimzations. */
+int call_me(int argc) {
   printf ("At the start, argc: %d.\n", argc);
 
   if (argc < 2)
-printf("Less than 2.\n");
+return 1; /* Less than 2. */
   else
-printf("Greater than or equal to 2.\n");
+retur

[Lldb-commits] [PATCH] D129012: [lldb] [test] Improve stability of llgs vCont-threads tests

[Michał Górny via Phabricator via lldb-commits]

mgorny created this revision.
mgorny added reviewers: labath, emaste, krytarowski, jingham.
Herald added a subscriber: arichardson.
Herald added a project: All.
mgorny requested review of this revision.

Perform a major refactoring of vCont-threads tests in order to attempt
to improve their stability and performance.

Split test_vCont_run_subset_of_threads() into smaller test cases,
and split the whole suite into two files: one for signal-related tests,
the running-subset-of tests.

Eliminate output_match checks entirely, as they are fragile to
fragmentation of output.  Instead, for the initial thread list capture
raise an explicit SIGSTOP from inside the test program, and for
the remaining output let the test program run until exit, and check all
the captured output afterwards.

For resume tests, capture the LLDB's thread view before and after
starting new threads in order to determine the IDs corresponding
to subthreads rather than relying on program output for that.

Use write(2) for output, as using std::printf() seems to be fragile
to the output being interspersed between threads mid-line.

Call std::this_thread::yield() to reduce the risk of one of the threads
not being run.

Incidentally, this seems to stop the tests from hanging on FreeBSD.
Hopefully, it will also fix all the flakiness.

Sponsored by: The FreeBSD Foundation


https://reviews.llvm.org/D129012

Files:
  lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
  lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py
  lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
  lldb/test/API/tools/lldb-server/vCont-threads/main.cpp

Index: lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
===
--- lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
+++ lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
@@ -6,11 +6,13 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 
 pseudo_barrier_t barrier;
+bool can_exit_now = false;
 
 static void sigusr1_handler(int signo) {
   char buf[100];
@@ -18,14 +20,26 @@
 "received SIGUSR1 on thread id: %" PRIx64 "\n",
 get_thread_id());
   write(STDOUT_FILENO, buf, strlen(buf));
+
+  can_exit_now = true;
 }
 
 static void thread_func() {
   pseudo_barrier_wait(barrier);
-  for (int i = 0; i < 300; ++i) {
-std::printf("thread %" PRIx64 " running\n", get_thread_id());
+  for (int i = 0; i < 5; ++i) {
+char buf[100];
+std::snprintf(buf, sizeof(buf), "thread %" PRIx64 " running\n",
+  get_thread_id());
+write(STDOUT_FILENO, buf, strlen(buf));
+
+std::this_thread::yield();
 std::this_thread::sleep_for(std::chrono::milliseconds(200));
+if (can_exit_now)
+  return;
   }
+
+  // if we didn't get signaled, terminate the program explicitly.
+  exit(0);
 }
 
 int main(int argc, char **argv) {
@@ -36,12 +50,12 @@
   signal(SIGUSR1, sigusr1_handler);
 
   std::vector threads;
-  for(int i = 0; i < num; ++i)
+  for (int i = 0; i < num; ++i)
 threads.emplace_back(thread_func);
 
   pseudo_barrier_wait(barrier);
 
-  std::puts("@started");
+  std::raise(SIGSTOP);
 
   for (std::thread &thread : threads)
 thread.join();
Index: lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
===
--- lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
+++ lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
@@ -1,23 +1,18 @@
-import json
 import re
-import time
 
 import gdbremote_testcase
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
-class TestGdbRemote_vContThreads(gdbremote_testcase.GdbRemoteTestCaseBase):
 
+class TestSignal(gdbremote_testcase.GdbRemoteTestCaseBase):
 def start_threads(self, num):
 procs = self.prep_debug_monitor_and_inferior(inferior_args=[str(num)])
-# start the process and wait for output
 self.test_sequence.add_log_lines([
 "read packet: $c#63",
-{"type": "output_match", "regex": r".*@started\r\n.*"},
+{"direction": "send", "regex": "[$]T.*;reason:signal.*"},
 ], True)
-# then interrupt it
-self.add_interrupt_packets()
 self.add_threadinfo_collection_packets()
 
 context = self.expect_gdbremote_sequence()
@@ -29,22 +24,21 @@
 self.reset_test_sequence()
 return threads
 
+SIGNAL_MATCH_RE = re.compile(r"received SIGUSR1 on thread id: ([0-9a-f]+)")
+
 def send_and_check_signal(self, vCont_data, threads):
 self.test_sequence.add_log_lines([
 "read packet: $vCont;{0}#00".format(vCont_data),
-{"type": "output_match",
- "regex": len(threads) *
-  r".*received SIGUSR1 on thread id: ([0-9a-f]+)\r\n.*",
- "capture": dict((i, "tid{0}".format

[Lldb-commits] [PATCH] D129012: [lldb] [test] Improve stability of llgs vCont-threads tests

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 441768.
mgorny added a comment.

Remove obsolete  include.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129012/new/

https://reviews.llvm.org/D129012

Files:
  lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
  lldb/test/API/tools/lldb-server/vCont-threads/TestPartialResume.py
  lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
  lldb/test/API/tools/lldb-server/vCont-threads/main.cpp

Index: lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
===
--- lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
+++ lldb/test/API/tools/lldb-server/vCont-threads/main.cpp
@@ -11,6 +11,7 @@
 #include 
 
 pseudo_barrier_t barrier;
+bool can_exit_now = false;
 
 static void sigusr1_handler(int signo) {
   char buf[100];
@@ -18,14 +19,26 @@
 "received SIGUSR1 on thread id: %" PRIx64 "\n",
 get_thread_id());
   write(STDOUT_FILENO, buf, strlen(buf));
+
+  can_exit_now = true;
 }
 
 static void thread_func() {
   pseudo_barrier_wait(barrier);
-  for (int i = 0; i < 300; ++i) {
-std::printf("thread %" PRIx64 " running\n", get_thread_id());
+  for (int i = 0; i < 5; ++i) {
+char buf[100];
+std::snprintf(buf, sizeof(buf), "thread %" PRIx64 " running\n",
+  get_thread_id());
+write(STDOUT_FILENO, buf, strlen(buf));
+
+std::this_thread::yield();
 std::this_thread::sleep_for(std::chrono::milliseconds(200));
+if (can_exit_now)
+  return;
   }
+
+  // if we didn't get signaled, terminate the program explicitly.
+  exit(0);
 }
 
 int main(int argc, char **argv) {
@@ -36,12 +49,12 @@
   signal(SIGUSR1, sigusr1_handler);
 
   std::vector threads;
-  for(int i = 0; i < num; ++i)
+  for (int i = 0; i < num; ++i)
 threads.emplace_back(thread_func);
 
   pseudo_barrier_wait(barrier);
 
-  std::puts("@started");
+  std::raise(SIGSTOP);
 
   for (std::thread &thread : threads)
 thread.join();
Index: lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
===
--- lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
+++ lldb/test/API/tools/lldb-server/vCont-threads/TestSignal.py
@@ -1,23 +1,18 @@
-import json
 import re
-import time
 
 import gdbremote_testcase
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
-class TestGdbRemote_vContThreads(gdbremote_testcase.GdbRemoteTestCaseBase):
 
+class TestSignal(gdbremote_testcase.GdbRemoteTestCaseBase):
 def start_threads(self, num):
 procs = self.prep_debug_monitor_and_inferior(inferior_args=[str(num)])
-# start the process and wait for output
 self.test_sequence.add_log_lines([
 "read packet: $c#63",
-{"type": "output_match", "regex": r".*@started\r\n.*"},
+{"direction": "send", "regex": "[$]T.*;reason:signal.*"},
 ], True)
-# then interrupt it
-self.add_interrupt_packets()
 self.add_threadinfo_collection_packets()
 
 context = self.expect_gdbremote_sequence()
@@ -29,22 +24,21 @@
 self.reset_test_sequence()
 return threads
 
+SIGNAL_MATCH_RE = re.compile(r"received SIGUSR1 on thread id: ([0-9a-f]+)")
+
 def send_and_check_signal(self, vCont_data, threads):
 self.test_sequence.add_log_lines([
 "read packet: $vCont;{0}#00".format(vCont_data),
-{"type": "output_match",
- "regex": len(threads) *
-  r".*received SIGUSR1 on thread id: ([0-9a-f]+)\r\n.*",
- "capture": dict((i, "tid{0}".format(i)) for i
- in range(1, len(threads)+1)),
- },
+"send packet: $W00#00",
 ], True)
-
-context = self.expect_gdbremote_sequence()
-self.assertIsNotNone(context)
-tids = sorted(int(context["tid{0}".format(x)], 16)
-  for x in range(1, len(threads)+1))
-self.assertEqual(tids, sorted(threads))
+exp = self.expect_gdbremote_sequence()
+self.reset_test_sequence()
+tids = []
+for line in exp["O_content"].decode().splitlines():
+m = self.SIGNAL_MATCH_RE.match(line)
+if m is not None:
+tids.append(int(m.group(1), 16))
+self.assertEqual(sorted(tids), sorted(threads))
 
 def get_pid(self):
 self.add_process_info_collection_packets()
@@ -242,72 +236,3 @@
 
 context = self.expect_gdbremote_sequence()
 self.assertIsNotNone(context)
-
-THREAD_MATCH_RE = re.compile(r"thread ([0-9a-f]+) running")
-
-def continue_and_get_threads_running(self, continue_packet):
-self.test_sequence.add_log_lines(
-["read packet: ${}#00".format(continue_packet),
- ], True)
-self.expect_gdbremote_sequence()
-