[Lldb-commits] [PATCH] D125575: [lldb] [llgs] Implement non-stop style stop notification packets
mgorny updated this revision to Diff 434006.
mgorny added a comment.
Send `OK` before the asynchronous stop reason to `k` packet. Support `vCont;t`
(as a special case) in non-stop mode.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125575/new/
https://reviews.llvm.org/D125575
Files:
lldb/include/lldb/Utility/StringExtractorGDBRemote.h
lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/source/Utility/StringExtractorGDBRemote.cpp
lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -1410,3 +1410,128 @@
self.assertEqual(decoded[errno_idx], 0) # si_errno
self.assertEqual(decoded[code_idx], SEGV_MAPERR) # si_code
self.assertEqual(decoded[addr_idx], 0) # si_addr
+
+def test_QNonStop(self):
+self.build()
+self.set_inferior_startup_launch()
+thread_num = 3
+procs = self.prep_debug_monitor_and_inferior(
+inferior_args=["thread:segfault"] + thread_num * ["thread:new"])
+self.test_sequence.add_log_lines(
+["read packet: $QNonStop:1#00",
+ "send packet: $OK#00",
+ "read packet: $c#63",
+ "send packet: $OK#00",
+ ], True)
+self.expect_gdbremote_sequence()
+
+segv_signo = lldbutil.get_signal_number('SIGSEGV')
+all_threads = set()
+all_segv_threads = []
+
+# we should get segfaults from all the threads
+for segv_no in range(thread_num):
+# first wait for the notification event
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+[{"direction": "send",
+ "regex": r"^%Stop:(T([0-9a-fA-F]{2})thread:([0-9a-fA-F]+);)",
+ "capture": {1: "packet", 2: "signo", 3: "thread_id"},
+ },
+ ], True)
+m = self.expect_gdbremote_sequence()
+del m["O_content"]
+threads = [m]
+
+# then we may get events for the remaining threads
+# (but note that not all threads may have been started yet)
+while True:
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+["read packet: $vStopped#00",
+ {"direction": "send",
+ "regex": r"^\$(OK|T([0-9a-fA-F]{2})thread:([0-9a-fA-F]+);)",
+ "capture": {1: "packet", 2: "signo", 3: "thread_id"},
+ },
+ ], True)
+m = self.expect_gdbremote_sequence()
+if m["packet"] == "OK":
+break
+del m["O_content"]
+threads.append(m)
+
+segv_threads = []
+other_threads = []
+for t in threads:
+signo = int(t["signo"], 16)
+if signo == segv_signo:
+segv_threads.append(t["thread_id"])
+else:
+self.assertEqual(signo, 0)
+other_threads.append(t["thread_id"])
+
+# verify that exactly one thread segfaulted
+self.assertEqual(len(segv_threads), 1)
+# we should get only one segv from every thread
+self.assertNotIn(segv_threads[0], all_segv_threads)
+all_segv_threads.extend(segv_threads)
+# segv_threads + other_threads should always be a superset
+# of all_threads, i.e. we should get states for all threads
+# already started
+self.assertFalse(
+all_threads.difference(other_threads + segv_threads))
+all_threads.update(other_threads + segv_threads)
+
+# verify that `?` returns the same result
+self.reset_test_sequence()
+self.test_sequence.add_log_lines(
+["read packet: $?#00",
+ ], True)
+threads_verify = []
+while True:
+self.test_sequence.add_log_lines(
+[{"direction": "send",
+ "regex": r"^\$(OK|T([0-9a-fA-F]{2})thread:([0-9a-fA-F]+);)",
+ "capture": {1: "packet", 2: "signo", 3: "thread_id"},
+ },
+ ], True)
+m = self.expect_gdbremote_sequence()
+
[Lldb-commits] [PATCH] D126614: [lldb] [gdb-remote] Client support for using the non-stop protocol
mgorny updated this revision to Diff 434007.
mgorny added a comment.
Use `vCont;t` instead of `vCtrlC` to stop other threads for better gdbserver
compatibility. Defer enabling non-stop until after the `?` status query, to
workaround a bug in gdbserver.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126614/new/
https://reviews.llvm.org/D126614
Files:
lldb/packages/Python/lldbsuite/test/gdbclientutils.py
lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
lldb/source/Target/Process.cpp
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemoteClient.py
@@ -585,3 +585,135 @@
}
self.do_siginfo_test("remote-freebsd", "basic_eh_frame.yaml",
data, expected)
+
+def test_QNonStop_run(self):
+class MyResponder(MockGDBServerResponder):
+vStopped_counter = 0
+
+def qSupported(self, client_supported):
+return "QNonStop+;" + super().qSupported(client_supported)
+
+def QNonStop(self, val):
+assert val == 1
+return "OK"
+
+def qfThreadInfo(self):
+return "m10,12"
+
+def qsThreadInfo(self):
+return "l"
+
+def vStopped(self):
+self.vStopped_counter += 1
+return ("OK" if self.vStopped_counter > 1
+else "T00;thread:10")
+
+def cont(self):
+self.vStopped_counter = 0
+return ["OK", "%Stop:T02;thread:12"]
+
+def vCtrlC(self):
+return "OK"
+
+self.dbg.HandleCommand(
+"settings set plugin.process.gdb-remote.use-non-stop-protocol true")
+self.addTearDownHook(lambda:
+self.runCmd(
+"settings set plugin.process.gdb-remote.use-non-stop-protocol "
+"false"))
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget("")
+process = self.connect(target)
+self.assertPacketLogContains(["QNonStop:1"])
+
+process.Continue()
+self.assertPacketLogContains(["vStopped", "vCont;t"])
+self.assertEqual(process.GetSelectedThread().GetStopReason(),
+ lldb.eStopReasonSignal)
+self.assertEqual(process.GetSelectedThread().GetStopDescription(100),
+ "signal SIGINT")
+
+def test_QNonStop_run_non_stop_server(self):
+class MyResponder(MockGDBServerResponder):
+def qSupported(self, client_supported):
+return "QNonStop+;" + super().qSupported(client_supported)
+
+def QNonStop(self, val):
+assert val == 1
+return "OK"
+
+def qfThreadInfo(self):
+return "m10,12"
+
+def qsThreadInfo(self):
+return "l"
+
+def vStopped(self):
+return "OK"
+
+def cont(self):
+return ["OK", "%Stop:T02;thread:12"]
+
+def vCtrlC(self):
+return ["OK", "%Stop:T00;thread:10"]
+
+self.dbg.HandleCommand(
+"settings set plugin.process.gdb-remote.use-non-stop-protocol true")
+self.addTearDownHook(lambda:
+self.runCmd(
+"settings set plugin.process.gdb-remote.use-non-stop-protocol "
+"false"))
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget("")
+process = self.connect(target)
+self.assertPacketLogContains(["QNonStop:1"])
+
+process.Continue()
+self.assertPacketLogContains(["vStopped", "vCont;t"])
+self.assertEqual(process.GetSelectedThread().GetStopReason(),
+ lldb.eStopReasonSignal)
+self.assertEqual(process.GetSelectedThread().GetStopDescription(100),
+ "signal SIGINT")
+
+def test_QNonStop_vCtrlC(self):
+class MyResponder(MockGDBServerResponder):
+vStopped_counter = 0
+
+def qSupported(self, client_supported):
+return "QNonStop+;" + super().qSupported(client_supported)
+
+def QNonStop(self, val):
+assert val == 1
+return "OK"
[Lldb-commits] [PATCH] D126615: [lldb] Add an integration test for non-stop protocol
mgorny updated this revision to Diff 434008.
mgorny added a comment.
Update the test following changes in the dependent patches.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126615/new/
https://reviews.llvm.org/D126615
Files:
lldb/test/Shell/Process/Inputs/signal.c
lldb/test/Shell/Process/TestNonStop.test
Index: lldb/test/Shell/Process/TestNonStop.test
===
--- /dev/null
+++ lldb/test/Shell/Process/TestNonStop.test
@@ -0,0 +1,30 @@
+# UNSUPPORTED: lldb-repro
+# REQUIRES: native
+# RUN: %clang_host %p/Inputs/signal.c -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+settings set plugin.process.gdb-remote.use-non-stop-protocol true
+log enable gdb-remote packets
+process launch
+
+# CHECK: lldb < 14> send packet: $QNonStop:1#8d
+# CHECK-NEXT: lldb < 6> read packet: $OK#9a
+
+# CHECK: b-remote.async> < 5> send packet: $c#63
+# CHECK-NEXT: b-remote.async> < 6> read packet: $OK#9a
+# CHECK-NEXT: b-remote.async> <{{[ 0-9]+}}> read packet: %Stop:T{{.*}}
+# CHECK-NEXT: b-remote.async> < 12> send packet: $vStopped#55
+# CHECK-NEXT: b-remote.async> < 6> read packet: $OK#9a
+# CHECK-NEXT: b-remote.async> < 11> send packet: $vCont;t#b9
+# CHECK-NEXT: b-remote.async> < 6> read packet: $OK#9a
+
+# CHECK: * thread #1, name = '{{.*}}', stop reason = signal SIGINT
+
+process continue
+
+# CHECK: b-remote.async> < 5> send packet: $c#63
+# CHECK-NEXT: b-remote.async> < 6> read packet: $OK#9a
+# CHECK-NEXT: b-remote.async> < 12> read packet: %Stop:W2a#ea
+# CHECK-NEXT: b-remote.async> < 12> send packet: $vStopped#55
+# CHECK-NEXT: b-remote.async> < 6> read packet: $OK#9a
+
+# CHECK: Process {{[0-9]+}} exited with status = 42 (0x002a)
Index: lldb/test/Shell/Process/Inputs/signal.c
===
--- /dev/null
+++ lldb/test/Shell/Process/Inputs/signal.c
@@ -0,0 +1,13 @@
+#include
+#include
+#include
+
+void handler() {
+ printf("got SIGINT\n");
+}
+
+int main() {
+ assert(signal(SIGINT, handler) != SIG_ERR);
+ raise(SIGINT);
+ return 42;
+}
Index: lldb/test/Shell/Process/TestNonStop.test
===
--- /dev/null
+++ lldb/test/Shell/Process/TestNonStop.test
@@ -0,0 +1,30 @@
+# UNSUPPORTED: lldb-repro
+# REQUIRES: native
+# RUN: %clang_host %p/Inputs/signal.c -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+settings set plugin.process.gdb-remote.use-non-stop-protocol true
+log enable gdb-remote packets
+process launch
+
+# CHECK: lldb < 14> send packet: $QNonStop:1#8d
+# CHECK-NEXT: lldb < 6> read packet: $OK#9a
+
+# CHECK: b-remote.async> < 5> send packet: $c#63
+# CHECK-NEXT: b-remote.async> < 6> read packet: $OK#9a
+# CHECK-NEXT: b-remote.async> <{{[ 0-9]+}}> read packet: %Stop:T{{.*}}
+# CHECK-NEXT: b-remote.async> < 12> send packet: $vStopped#55
+# CHECK-NEXT: b-remote.async> < 6> read packet: $OK#9a
+# CHECK-NEXT: b-remote.async> < 11> send packet: $vCont;t#b9
+# CHECK-NEXT: b-remote.async> < 6> read packet: $OK#9a
+
+# CHECK: * thread #1, name = '{{.*}}', stop reason = signal SIGINT
+
+process continue
+
+# CHECK: b-remote.async> < 5> send packet: $c#63
+# CHECK-NEXT: b-remote.async> < 6> read packet: $OK#9a
+# CHECK-NEXT: b-remote.async> < 12> read packet: %Stop:W2a#ea
+# CHECK-NEXT: b-remote.async> < 12> send packet: $vStopped#55
+# CHECK-NEXT: b-remote.async> < 6> read packet: $OK#9a
+
+# CHECK: Process {{[0-9]+}} exited with status = 42 (0x002a)
Index: lldb/test/Shell/Process/Inputs/signal.c
===
--- /dev/null
+++ lldb/test/Shell/Process/Inputs/signal.c
@@ -0,0 +1,13 @@
+#include
+#include
+#include
+
+void handler() {
+ printf("got SIGINT\n");
+}
+
+int main() {
+ assert(signal(SIGINT, handler) != SIG_ERR);
+ raise(SIGINT);
+ return 42;
+}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] aed179f - [lldb] [Process/FreeBSD] Do not send SIGSTOP to stopped process
Author: Michał Górny
Date: 2022-06-03T15:19:09+02:00
New Revision: aed179f5f557664d6deb26ef6fdc6aa944af41af
URL:
https://github.com/llvm/llvm-project/commit/aed179f5f557664d6deb26ef6fdc6aa944af41af
DIFF:
https://github.com/llvm/llvm-project/commit/aed179f5f557664d6deb26ef6fdc6aa944af41af.diff
LOG: [lldb] [Process/FreeBSD] Do not send SIGSTOP to stopped process
Do not send SIGSTOP when requested to halt a process that's already
stopped. This results in the signal being queued for delivery once
the process is resumed, and unexpectedly stopping it again.
This is necessary for non-stop protocol patches to land.
Sponsored by: The FreeBSD Foundation
Differential Revision: https://reviews.llvm.org/D126770
Added:
Modified:
lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
Removed:
diff --git a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
index 21c9ead0eca41..5d73ec3457e37 100644
--- a/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -497,6 +497,10 @@ Status NativeProcessFreeBSD::Resume(const ResumeActionList
&resume_actions) {
Status NativeProcessFreeBSD::Halt() {
Status error;
+ // Do not try to stop a process that's already stopped, this may cause
+ // the SIGSTOP to get queued and stop the process again once resumed.
+ if (StateIsStoppedState(m_state, false))
+return error;
if (kill(GetID(), SIGSTOP) != 0)
error.SetErrorToErrno();
return error;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D126770: [lldb] [Process/FreeBSD] Do not send SIGSTOP to stopped process
This revision was automatically updated to reflect the committed changes.
Closed by commit rGaed179f5f557: [lldb] [Process/FreeBSD] Do not send SIGSTOP
to stopped process (authored by mgorny).
Herald added a project: LLDB.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126770/new/
https://reviews.llvm.org/D126770
Files:
lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
Index: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -497,6 +497,10 @@
Status NativeProcessFreeBSD::Halt() {
Status error;
+ // Do not try to stop a process that's already stopped, this may cause
+ // the SIGSTOP to get queued and stop the process again once resumed.
+ if (StateIsStoppedState(m_state, false))
+return error;
if (kill(GetID(), SIGSTOP) != 0)
error.SetErrorToErrno();
return error;
Index: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -497,6 +497,10 @@
Status NativeProcessFreeBSD::Halt() {
Status error;
+ // Do not try to stop a process that's already stopped, this may cause
+ // the SIGSTOP to get queued and stop the process again once resumed.
+ if (StateIsStoppedState(m_state, false))
+return error;
if (kill(GetID(), SIGSTOP) != 0)
error.SetErrorToErrno();
return error;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D126982: [lldb] [test] Implement getting thread ID on FreeBSD
mgorny created this revision. mgorny added reviewers: labath, krytarowski, emaste. Herald added a subscriber: arichardson. Herald added a project: All. mgorny requested review of this revision. Sponsored by: The FreeBSD Foundation https://reviews.llvm.org/D126982 Files: lldb/packages/Python/lldbsuite/test/make/thread.h Index: lldb/packages/Python/lldbsuite/test/make/thread.h === --- lldb/packages/Python/lldbsuite/test/make/thread.h +++ lldb/packages/Python/lldbsuite/test/make/thread.h @@ -9,6 +9,8 @@ #elif defined(__linux__) #include #include +#elif defined(__FreeBSD__) +#include #elif defined(__NetBSD__) #include #elif defined(_WIN32) @@ -22,6 +24,8 @@ return tid; #elif defined(__linux__) return syscall(__NR_gettid); +#elif defined(__FreeBSD__) + return static_cast(pthread_getthreadid_np()); #elif defined(__NetBSD__) // Technically lwpid_t is 32-bit signed integer return static_cast(_lwp_self()); Index: lldb/packages/Python/lldbsuite/test/make/thread.h === --- lldb/packages/Python/lldbsuite/test/make/thread.h +++ lldb/packages/Python/lldbsuite/test/make/thread.h @@ -9,6 +9,8 @@ #elif defined(__linux__) #include #include +#elif defined(__FreeBSD__) +#include #elif defined(__NetBSD__) #include #elif defined(_WIN32) @@ -22,6 +24,8 @@ return tid; #elif defined(__linux__) return syscall(__NR_gettid); +#elif defined(__FreeBSD__) + return static_cast(pthread_getthreadid_np()); #elif defined(__NetBSD__) // Technically lwpid_t is 32-bit signed integer return static_cast(_lwp_self()); ___ 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
mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste.
Herald added a subscriber: arichardson.
Herald added a project: All.
mgorny requested review of this revision.
Implement support for the "t" action that is used to stop a thread.
Normally this action is used only in non-stop mode. However, there's
no technical reason why it couldn't be also used in all-stop mode,
e.g. to express "resume all threads except ..." (`t:...;c`).
While at it, add a more complete test for vCont correctly resuming
a subset of program's threads.
Sponsored by: The FreeBSD Foundation
https://reviews.llvm.org/D126983
Files:
lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/test/API/tools/lldb-server/TestGdbRemote_vCont.py
lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.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
@@ -22,7 +22,10 @@
static void thread_func() {
pseudo_barrier_wait(barrier);
- std::this_thread::sleep_for(std::chrono::minutes(1));
+ for (int i = 0; i < 300; ++i) {
+std::printf("thread %" PRIx64 " running\n", get_thread_id());
+std::this_thread::sleep_for(std::chrono::milliseconds(200));
+ }
}
int main(int argc, char **argv) {
Index: lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
===
--- lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
+++ lldb/test/API/tools/lldb-server/vCont-threads/TestGdbRemote_vContThreads.py
@@ -1,5 +1,6 @@
import json
import re
+import time
import gdbremote_testcase
from lldbsuite.test.decorators import *
@@ -14,8 +15,7 @@
# start the process and wait for output
self.test_sequence.add_log_lines([
"read packet: $c#63",
-{"type": "output_match", "regex": self.maybe_strict_output_regex(
-r"@started\r\n")},
+{"type": "output_match", "regex": r".*@started\r\n.*"},
], True)
# then interrupt it
self.add_interrupt_packets()
@@ -34,9 +34,8 @@
self.test_sequence.add_log_lines([
"read packet: $vCont;{0}#00".format(vCont_data),
{"type": "output_match",
- "regex": self.maybe_strict_output_regex(
- len(threads) *
- r"received SIGUSR1 on thread id: ([0-9a-f]+)\r\n"),
+ "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)),
},
@@ -228,3 +227,71 @@
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()
+self.reset_test_sequence()
+time.sleep(1)
+self.add_interrupt_packets()
+exp = self.expect_gdbremote_sequence()
+found = set()
+for line in exp["O_content"].decode().splitlines():
+m = self.THREAD_MATCH_RE.match(line)
+if m is not None:
+found.add(int(m.group(1), 16))
+return found
+
+@add_test_categories(["llgs"])
+def test_vCont_run_subset_of_threads(self):
+self.build()
+self.set_inferior_startup_launch()
+
+threads = set(self.start_threads(3))
+all_subthreads = self.continue_and_get_threads_running("c")
+all_subthreads_list = list(all_subthreads)
+self.assertEqual(len(all_subthreads), 3)
+self.assertEqual(threads & all_subthreads, all_subthreads)
+
+# resume two threads explicitly, stop the third one implicitly
+self.assertEqual(
+self.continue_and_get_threads_running(
+"vCont;c:{:x};c:{:x}".format(*all_subthreads_list[:2])),
+set(all_subthreads_list[:2]))
+
+# resume two threads explicitly, stop others explicitly
+self.assertEqual(
+self.continue_and_get_threads_running(
+"vCont;c:{:x};c:{:x};t".format(*all_subthreads_list[:2])),
+set(all_subthreads_list[:2]))
+
+# stop one thread explicitly, resume others
+self.assertEqual(
+self.continue_and_get_threads_running(
+"vCont;t:{:
[Lldb-commits] [PATCH] D126789: Stop regex commands from double entry into the history
jingham added inline comments. Comment at: lldb/source/Commands/CommandObjectRegexCommand.cpp:74-76 // Pass in true for "no context switching". The command that called us // should have set up the context appropriately, we shouldn't have to // redo that. clayborg wrote: > Is this comment out of date now? It mentions passing in "true"? No, that comment has to do with the Interpreter's ExecutionContext, not the history state. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126789/new/ https://reviews.llvm.org/D126789 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D126789: Stop regex commands from double entry into the history
jingham added inline comments. Comment at: lldb/source/Commands/CommandObjectRegexCommand.cpp:74-76 // Pass in true for "no context switching". The command that called us // should have set up the context appropriately, we shouldn't have to // redo that. jingham wrote: > clayborg wrote: > > Is this comment out of date now? It mentions passing in "true"? > No, that comment has to do with the Interpreter's ExecutionContext, not the > history state. Or rather, I think it was obsoleted a while ago. What it's really explaining is why you don't need to call the version of HandleCommand that takes an override_context. I think that was controlled by a bool at some point in the past. I'll update the comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126789/new/ https://reviews.llvm.org/D126789 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D126394: [trace][intelpt] Support system-wide tracing [14] - Decode per cpu
wallace updated this revision to Diff 434081.
wallace edited the summary of this revision.
wallace added a comment.
Herald added subscribers: mgrang, mgorny.
finish diff
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126394/new/
https://reviews.llvm.org/D126394
Files:
lldb/include/lldb/Target/Trace.h
lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
lldb/source/Plugins/Trace/intel-pt/PerfContextSwitchDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/PerfContextSwitchDecoder.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.h
lldb/source/Target/Trace.cpp
lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
lldb/test/API/commands/trace/TestTraceLoad.py
lldb/test/API/commands/trace/intelpt-multi-core-trace/cores/45.intelpt_trace
lldb/test/API/commands/trace/intelpt-multi-core-trace/cores/45.perf_context_switch_trace
lldb/test/API/commands/trace/intelpt-multi-core-trace/cores/51.intelpt_trace
lldb/test/API/commands/trace/intelpt-multi-core-trace/cores/51.perf_context_switch_trace
lldb/test/API/commands/trace/intelpt-multi-core-trace/modules/home/wallace/tests/m.out
lldb/test/API/commands/trace/intelpt-multi-core-trace/modules/lib64/libc.so.6
lldb/test/API/commands/trace/intelpt-multi-core-trace/modules/lib64/libgcc_s.so.1
lldb/test/API/commands/trace/intelpt-multi-core-trace/modules/lib64/libm.so.6
lldb/test/API/commands/trace/intelpt-multi-core-trace/modules/lib64/libpthread.so.0
lldb/test/API/commands/trace/intelpt-multi-core-trace/modules/lib64/libstdc++.so.6
lldb/test/API/commands/trace/intelpt-multi-core-trace/modules/usr/lib64/ld-2.28.so
lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
lldb/unittests/Process/Linux/PerfTests.cpp
Index: lldb/unittests/Process/Linux/PerfTests.cpp
===
--- lldb/unittests/Process/Linux/PerfTests.cpp
+++ lldb/unittests/Process/Linux/PerfTests.cpp
@@ -74,12 +74,12 @@
if (!tsc_after_sleep)
GTEST_SKIP() << toString(tsc_after_sleep.takeError());
- std::chrono::nanoseconds converted_tsc_diff =
+ uint64_t converted_tsc_diff =
params->ToNanos(*tsc_after_sleep) - params->ToNanos(*tsc_before_sleep);
std::chrono::microseconds acceptable_overhead(500);
- ASSERT_GE(converted_tsc_diff.count(), SLEEP_NANOS.count());
- ASSERT_LT(converted_tsc_diff.count(),
-(SLEEP_NANOS + acceptable_overhead).count());
+ ASSERT_GE(converted_tsc_diff, static_cast(SLEEP_NANOS.count()));
+ ASSERT_LT(converted_tsc_diff,
+static_cast((SLEEP_NANOS + acceptable_overhead).count()));
}
Index: lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
===
--- lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
+++ lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
@@ -233,6 +233,6 @@
# We must have captured the context switch of when the target resumed
self.assertTrue(found_non_empty_context_switch)
-self.expect("thread trace dump instructions", substrs=['unimplemented'])
+self.expect("thread trace dump instructions")
self.traceStopProcess()
Index: lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
@@ -0,0 +1,51 @@
+{
+ "cores": [
+{
+ "contextSwitchTrace": "/tmp/trace8/cores/45.perf_context_switch_trace",
+ "coreId": 45,
+ "traceBuffer": "/tmp/trace8/cores/45.intelpt_trace"
+},
+{
+ "contextSwitchTrace": "/tmp/trace8/cores/51.perf_context_switch_trace",
+ "coreId": 51,
+ "traceBuffer": "/tmp/trace8/cores/51.intelpt_trace"
+}
+ ],
+ "cpuInfo": {
+"family": 6,
+"model": 85,
+"stepping": 4,
+"vendor": "GenuineIntel"
+ },
+ "processes": [
+{
+ "modules": [
+{
+ "file": "/tmp/trace8/modules/home/wallace/tests/m.out",
+ "loadAddress": 4194304,
+ "systemPath": "/home/wallace/tests/m.out",
+ "uuid": "AEFB0D59-233F-80FF-6D3C-4DED498534CF-11017B3B"
+}
+ ],
+ "pid": 3497234,
+ "threads": [
+{
+ "tid": 3497234
+},
+{
+ "tid": 3497496
+},
+{
+ "tid": 3497497
+}
+ ],
+ "tripl
[Lldb-commits] [PATCH] D126789: Stop regex commands from double entry into the history
jingham updated this revision to Diff 434082.
jingham added a comment.
Fix a comment that hadn't been correct for a while...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126789/new/
https://reviews.llvm.org/D126789
Files:
lldb/source/Commands/CommandObjectRegexCommand.cpp
lldb/test/API/functionalities/history/TestHistoryRecall.py
Index: lldb/test/API/functionalities/history/TestHistoryRecall.py
===
--- lldb/test/API/functionalities/history/TestHistoryRecall.py
+++ lldb/test/API/functionalities/history/TestHistoryRecall.py
@@ -1,5 +1,5 @@
"""
-Make sure the !N and !-N commands work properly.
+Test some features of "session history" and history recall.
"""
@@ -20,9 +20,25 @@
def test_history_recall(self):
"""Test the !N and !-N functionality of the command interpreter."""
-self.sample_test()
+self.do_bang_N_test()
-def sample_test(self):
+def test_regex_history(self):
+"""Test the regex commands don't add two elements to the history"""
+self.do_regex_history_test()
+
+def do_regex_history_test(self):
+interp = self.dbg.GetCommandInterpreter()
+result = lldb.SBCommandReturnObject()
+command = "_regexp-break foo.c:12"
+self.runCmd(command, msg="Run the regex break command", inHistory =
True)
+interp.HandleCommand("session history", result, True)
+self.assertTrue(result.Succeeded(), "session history ran successfully")
+results = result.GetOutput()
+self.assertIn(command, results, "Recorded the actual command")
+self.assertNotIn("breakpoint set --file 'foo.c' --line 12", results,
+ "Didn't record the resolved command")
+
+def do_bang_N_test(self):
interp = self.dbg.GetCommandInterpreter()
result = lldb.SBCommandReturnObject()
interp.HandleCommand("session history", result, True)
Index: lldb/source/Commands/CommandObjectRegexCommand.cpp
===
--- lldb/source/Commands/CommandObjectRegexCommand.cpp
+++ lldb/source/Commands/CommandObjectRegexCommand.cpp
@@ -71,11 +71,10 @@
// Interpret the new command and return this as the result!
if (m_interpreter.GetExpandRegexAliases())
result.GetOutputStream().Printf("%s\n", new_command->c_str());
- // Pass in true for "no context switching". The command that called us
- // should have set up the context appropriately, we shouldn't have to
- // redo that.
+ // We don't have to pass an override_context here, as the command that
+ // called us should have set up the context appropriately.
return m_interpreter.HandleCommand(new_command->c_str(),
- eLazyBoolCalculate, result);
+ eLazyBoolNo, result);
}
}
result.SetStatus(eReturnStatusFailed);
Index: lldb/test/API/functionalities/history/TestHistoryRecall.py
===
--- lldb/test/API/functionalities/history/TestHistoryRecall.py
+++ lldb/test/API/functionalities/history/TestHistoryRecall.py
@@ -1,5 +1,5 @@
"""
-Make sure the !N and !-N commands work properly.
+Test some features of "session history" and history recall.
"""
@@ -20,9 +20,25 @@
def test_history_recall(self):
"""Test the !N and !-N functionality of the command interpreter."""
-self.sample_test()
+self.do_bang_N_test()
-def sample_test(self):
+def test_regex_history(self):
+"""Test the regex commands don't add two elements to the history"""
+self.do_regex_history_test()
+
+def do_regex_history_test(self):
+interp = self.dbg.GetCommandInterpreter()
+result = lldb.SBCommandReturnObject()
+command = "_regexp-break foo.c:12"
+self.runCmd(command, msg="Run the regex break command", inHistory = True)
+interp.HandleCommand("session history", result, True)
+self.assertTrue(result.Succeeded(), "session history ran successfully")
+results = result.GetOutput()
+self.assertIn(command, results, "Recorded the actual command")
+self.assertNotIn("breakpoint set --file 'foo.c' --line 12", results,
+ "Didn't record the resolved command")
+
+def do_bang_N_test(self):
interp = self.dbg.GetCommandInterpreter()
result = lldb.SBCommandReturnObject()
interp.HandleCommand("session history", result, True)
Index: lldb/source/Commands/CommandObjectRegexCommand.cpp
===
--- lldb/source/Commands/CommandObjectRegexCommand.cpp
+++ lldb/source/Commands/CommandObjectRegexCommand.cpp
@@ -71,11 +71,10 @@
// Interpret the new command and return this as
[Lldb-commits] [lldb] 8cc8b36 - CommandObjectRegexCommand shouldn't put two commands on the history stack.
Author: Jim Ingham
Date: 2022-06-03T11:34:53-07:00
New Revision: 8cc8b36f24d6f3133c44e238a657309620eedc19
URL:
https://github.com/llvm/llvm-project/commit/8cc8b36f24d6f3133c44e238a657309620eedc19
DIFF:
https://github.com/llvm/llvm-project/commit/8cc8b36f24d6f3133c44e238a657309620eedc19.diff
LOG: CommandObjectRegexCommand shouldn't put two commands on the history stack.
It was putting the command the user typed, and then the resolved command in the
command history. That caused up-arrow not to work correctly when the regex
command
was invoked from a Python-command. Plus it's just weird.
Differential Revision: https://reviews.llvm.org/D126789
Added:
Modified:
lldb/source/Commands/CommandObjectRegexCommand.cpp
lldb/test/API/functionalities/history/TestHistoryRecall.py
Removed:
diff --git a/lldb/source/Commands/CommandObjectRegexCommand.cpp
b/lldb/source/Commands/CommandObjectRegexCommand.cpp
index a99a9e760cb26..857193036e393 100644
--- a/lldb/source/Commands/CommandObjectRegexCommand.cpp
+++ b/lldb/source/Commands/CommandObjectRegexCommand.cpp
@@ -71,11 +71,10 @@ bool CommandObjectRegexCommand::DoExecute(llvm::StringRef
command,
// Interpret the new command and return this as the result!
if (m_interpreter.GetExpandRegexAliases())
result.GetOutputStream().Printf("%s\n", new_command->c_str());
- // Pass in true for "no context switching". The command that called us
- // should have set up the context appropriately, we shouldn't have to
- // redo that.
+ // We don't have to pass an override_context here, as the command that
+ // called us should have set up the context appropriately.
return m_interpreter.HandleCommand(new_command->c_str(),
- eLazyBoolCalculate, result);
+ eLazyBoolNo, result);
}
}
result.SetStatus(eReturnStatusFailed);
diff --git a/lldb/test/API/functionalities/history/TestHistoryRecall.py
b/lldb/test/API/functionalities/history/TestHistoryRecall.py
index b3a012dd7bbae..2c17fd3e77446 100644
--- a/lldb/test/API/functionalities/history/TestHistoryRecall.py
+++ b/lldb/test/API/functionalities/history/TestHistoryRecall.py
@@ -1,5 +1,5 @@
"""
-Make sure the !N and !-N commands work properly.
+Test some features of "session history" and history recall.
"""
@@ -20,9 +20,25 @@ class TestHistoryRecall(TestBase):
def test_history_recall(self):
"""Test the !N and !-N functionality of the command interpreter."""
-self.sample_test()
+self.do_bang_N_test()
-def sample_test(self):
+def test_regex_history(self):
+"""Test the regex commands don't add two elements to the history"""
+self.do_regex_history_test()
+
+def do_regex_history_test(self):
+interp = self.dbg.GetCommandInterpreter()
+result = lldb.SBCommandReturnObject()
+command = "_regexp-break foo.c:12"
+self.runCmd(command, msg="Run the regex break command", inHistory =
True)
+interp.HandleCommand("session history", result, True)
+self.assertTrue(result.Succeeded(), "session history ran successfully")
+results = result.GetOutput()
+self.assertIn(command, results, "Recorded the actual command")
+self.assertNotIn("breakpoint set --file 'foo.c' --line 12", results,
+ "Didn't record the resolved command")
+
+def do_bang_N_test(self):
interp = self.dbg.GetCommandInterpreter()
result = lldb.SBCommandReturnObject()
interp.HandleCommand("session history", result, True)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D126789: Stop regex commands from double entry into the history
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8cc8b36f24d6: CommandObjectRegexCommand shouldn't put
two commands on the history stack. (authored by jingham).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126789/new/
https://reviews.llvm.org/D126789
Files:
lldb/source/Commands/CommandObjectRegexCommand.cpp
lldb/test/API/functionalities/history/TestHistoryRecall.py
Index: lldb/test/API/functionalities/history/TestHistoryRecall.py
===
--- lldb/test/API/functionalities/history/TestHistoryRecall.py
+++ lldb/test/API/functionalities/history/TestHistoryRecall.py
@@ -1,5 +1,5 @@
"""
-Make sure the !N and !-N commands work properly.
+Test some features of "session history" and history recall.
"""
@@ -20,9 +20,25 @@
def test_history_recall(self):
"""Test the !N and !-N functionality of the command interpreter."""
-self.sample_test()
+self.do_bang_N_test()
-def sample_test(self):
+def test_regex_history(self):
+"""Test the regex commands don't add two elements to the history"""
+self.do_regex_history_test()
+
+def do_regex_history_test(self):
+interp = self.dbg.GetCommandInterpreter()
+result = lldb.SBCommandReturnObject()
+command = "_regexp-break foo.c:12"
+self.runCmd(command, msg="Run the regex break command", inHistory =
True)
+interp.HandleCommand("session history", result, True)
+self.assertTrue(result.Succeeded(), "session history ran successfully")
+results = result.GetOutput()
+self.assertIn(command, results, "Recorded the actual command")
+self.assertNotIn("breakpoint set --file 'foo.c' --line 12", results,
+ "Didn't record the resolved command")
+
+def do_bang_N_test(self):
interp = self.dbg.GetCommandInterpreter()
result = lldb.SBCommandReturnObject()
interp.HandleCommand("session history", result, True)
Index: lldb/source/Commands/CommandObjectRegexCommand.cpp
===
--- lldb/source/Commands/CommandObjectRegexCommand.cpp
+++ lldb/source/Commands/CommandObjectRegexCommand.cpp
@@ -71,11 +71,10 @@
// Interpret the new command and return this as the result!
if (m_interpreter.GetExpandRegexAliases())
result.GetOutputStream().Printf("%s\n", new_command->c_str());
- // Pass in true for "no context switching". The command that called us
- // should have set up the context appropriately, we shouldn't have to
- // redo that.
+ // We don't have to pass an override_context here, as the command that
+ // called us should have set up the context appropriately.
return m_interpreter.HandleCommand(new_command->c_str(),
- eLazyBoolCalculate, result);
+ eLazyBoolNo, result);
}
}
result.SetStatus(eReturnStatusFailed);
Index: lldb/test/API/functionalities/history/TestHistoryRecall.py
===
--- lldb/test/API/functionalities/history/TestHistoryRecall.py
+++ lldb/test/API/functionalities/history/TestHistoryRecall.py
@@ -1,5 +1,5 @@
"""
-Make sure the !N and !-N commands work properly.
+Test some features of "session history" and history recall.
"""
@@ -20,9 +20,25 @@
def test_history_recall(self):
"""Test the !N and !-N functionality of the command interpreter."""
-self.sample_test()
+self.do_bang_N_test()
-def sample_test(self):
+def test_regex_history(self):
+"""Test the regex commands don't add two elements to the history"""
+self.do_regex_history_test()
+
+def do_regex_history_test(self):
+interp = self.dbg.GetCommandInterpreter()
+result = lldb.SBCommandReturnObject()
+command = "_regexp-break foo.c:12"
+self.runCmd(command, msg="Run the regex break command", inHistory = True)
+interp.HandleCommand("session history", result, True)
+self.assertTrue(result.Succeeded(), "session history ran successfully")
+results = result.GetOutput()
+self.assertIn(command, results, "Recorded the actual command")
+self.assertNotIn("breakpoint set --file 'foo.c' --line 12", results,
+ "Didn't record the resolved command")
+
+def do_bang_N_test(self):
interp = self.dbg.GetCommandInterpreter()
result = lldb.SBCommandReturnObject()
interp.HandleCommand("session history", result, True)
Index: lldb/source/Commands/CommandObjectRegexCommand.cpp
===
--- lldb/source/Commands/CommandObjectRegexCommand.cpp
+++ lldb/source/Commands/CommandObjectRegexCommand.
[Lldb-commits] [PATCH] D126394: [trace][intelpt] Support system-wide tracing [14] - Decode per cpu
wallace updated this revision to Diff 434093.
wallace added a comment.
update test files
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126394/new/
https://reviews.llvm.org/D126394
Files:
lldb/include/lldb/Target/Trace.h
lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
lldb/source/Plugins/Trace/intel-pt/CMakeLists.txt
lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.h
lldb/source/Plugins/Trace/intel-pt/PerfContextSwitchDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/PerfContextSwitchDecoder.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCoreDecoder.h
lldb/source/Target/Trace.cpp
lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
lldb/test/API/commands/trace/TestTraceLoad.py
lldb/test/API/commands/trace/intelpt-multi-core-trace/cores/45.intelpt_trace
lldb/test/API/commands/trace/intelpt-multi-core-trace/cores/45.perf_context_switch_trace
lldb/test/API/commands/trace/intelpt-multi-core-trace/cores/51.intelpt_trace
lldb/test/API/commands/trace/intelpt-multi-core-trace/cores/51.perf_context_switch_trace
lldb/test/API/commands/trace/intelpt-multi-core-trace/modules/m.out
lldb/test/API/commands/trace/intelpt-multi-core-trace/multi_thread.cpp
lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
lldb/unittests/Process/Linux/PerfTests.cpp
Index: lldb/unittests/Process/Linux/PerfTests.cpp
===
--- lldb/unittests/Process/Linux/PerfTests.cpp
+++ lldb/unittests/Process/Linux/PerfTests.cpp
@@ -74,12 +74,12 @@
if (!tsc_after_sleep)
GTEST_SKIP() << toString(tsc_after_sleep.takeError());
- std::chrono::nanoseconds converted_tsc_diff =
+ uint64_t converted_tsc_diff =
params->ToNanos(*tsc_after_sleep) - params->ToNanos(*tsc_before_sleep);
std::chrono::microseconds acceptable_overhead(500);
- ASSERT_GE(converted_tsc_diff.count(), SLEEP_NANOS.count());
- ASSERT_LT(converted_tsc_diff.count(),
-(SLEEP_NANOS + acceptable_overhead).count());
+ ASSERT_GE(converted_tsc_diff, static_cast(SLEEP_NANOS.count()));
+ ASSERT_LT(converted_tsc_diff,
+static_cast((SLEEP_NANOS + acceptable_overhead).count()));
}
Index: lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
===
--- lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
+++ lldb/test/API/commands/trace/multiple-threads/TestTraceStartStopMultipleThreads.py
@@ -233,6 +233,6 @@
# We must have captured the context switch of when the target resumed
self.assertTrue(found_non_empty_context_switch)
-self.expect("thread trace dump instructions", substrs=['unimplemented'])
+self.expect("thread trace dump instructions")
self.traceStopProcess()
Index: lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
@@ -0,0 +1,51 @@
+{
+ "cores": [
+{
+ "contextSwitchTrace": "/tmp/trace8/cores/45.perf_context_switch_trace",
+ "coreId": 45,
+ "traceBuffer": "/tmp/trace8/cores/45.intelpt_trace"
+},
+{
+ "contextSwitchTrace": "/tmp/trace8/cores/51.perf_context_switch_trace",
+ "coreId": 51,
+ "traceBuffer": "/tmp/trace8/cores/51.intelpt_trace"
+}
+ ],
+ "cpuInfo": {
+"family": 6,
+"model": 85,
+"stepping": 4,
+"vendor": "GenuineIntel"
+ },
+ "processes": [
+{
+ "modules": [
+{
+ "file": "modules/m.out",
+ "systemPath": "/tmp/m.out",
+ "loadAddress": 4194304,
+ "uuid": "AEFB0D59-233F-80FF-6D3C-4DED498534CF-11017B3B"
+}
+ ],
+ "pid": 3497234,
+ "threads": [
+{
+ "tid": 3497234
+},
+{
+ "tid": 3497496
+},
+{
+ "tid": 3497497
+}
+ ],
+ "triple": "x86_64-unknown-linux-gnu"
+}
+ ],
+ "tscPerfZeroConversion": {
+"timeMult": 1076264588,
+"timeShift": 31,
+"timeZero": 18433473881008870804
+ },
+ "type": "intel-pt"
+}
Index: lldb/test/API/commands/trace/intelpt-multi-core-trace/multi_thread.cpp
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-multi-core-trace/multi_thread.cpp
@@ -0,0 +1,34 @@
+#include
+#include
+#include
+using namespace std;
+
+bool done = false;
+void foo() {
+ int x = 0;
+ for (int i = 0; i < 1000
[Lldb-commits] [PATCH] D126990: [trace][intelpt] Support system-wide tracing [15] - Make triple optional
wallace created this revision.
wallace added a reviewer: jj10306.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
The process triple should only be needed when LLDB can't identify the correct
triple on its own. Examples could be universal mach-o binaries. But in any case,
at least for most of ELF files, LLDB should be able to do the job without having
the user specify the triple manually.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D126990
Files:
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
lldb/test/API/commands/trace/TestTraceLoad.py
lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
Index: lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
===
--- lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
+++ lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
@@ -2,14 +2,13 @@
"type": "intel-pt",
"cpuInfo": {
"vendor": "GenuineIntel",
-"family": 6,
+"family": "123",
"model": 79,
"stepping": 1
},
"processes": [
{
"pid": 1234,
- "triple": "x86_64-*-linux",
"threads": [
{
"tid": 5678,
Index: lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
===
--- lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
+++ lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
@@ -38,8 +38,7 @@
{
"tid": 3497497
}
- ],
- "triple": "x86_64-unknown-linux-gnu"
+ ]
}
],
"tscPerfZeroConversion": {
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -94,9 +94,9 @@
"stepping": integer
},'''])
-# Now we test a missing field in the global session file
+# Now we test a wrong cpu family field in the global session file
self.expect("trace load -v " + os.path.join(src_dir, "intelpt-trace",
"trace_bad2.json"), error=True,
-substrs=['error: missing value at
traceSession.processes[1].triple', "Context", "Schema"])
+substrs=['error: expected uint64_t at
traceSession.cpuInfo.family', "Context", "Schema"])
# Now we test a missing field in the intel-pt settings
self.expect("trace load -v " + os.path.join(src_dir, "intelpt-trace",
"trace_bad4.json"), error=True,
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
@@ -88,7 +88,7 @@
TraceIntelPTSessionFileParser::ParseProcess(const JSONProcess &process) {
TargetSP target_sp;
Status error = m_debugger.GetTargetList().CreateTarget(
- m_debugger, /*user_exe_path*/ StringRef(), process.triple,
+ m_debugger, /*user_exe_path*/ StringRef(), process.triple.getValueOr(""),
eLoadDependentsNo,
/*platform_options*/ nullptr, target_sp);
@@ -161,8 +161,8 @@
"processes": [
{
"pid": integer,
- "triple": string,
- // clang/llvm target triple.
+ "triple"?: string,
+ // Optional clang/llvm target triple.
"threads": [
{
"tid": integer,
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h
@@ -36,7 +36,7 @@
struct JSONProcess {
int64_t pid;
- std::string triple;
+ llvm::Optional triple;
std::vector threads;
std::vector modules;
};
Index: lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
===
--- lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
+++ lldb/test/API/commands/trace/intelpt-trace/trace_bad2.json
@@ -2,14 +2,13 @@
"type": "intel-pt",
"cpuInfo": {
"vendor": "GenuineIntel",
-"family": 6,
+"family": "123",
"model": 79,
"stepping": 1
},
"processes": [
{
"pid": 1234,
- "triple": "x86_64-*-linux",
"threads": [
{
"tid": 5678,
Index: lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
===
--- lldb/test/API/commands/trace/intelpt-multi-core-trace/trace.json
+++ lldb/test/API/commands/
[Lldb-commits] [PATCH] D126240: [lldb] Tighten the scope of a couple of locks in DataFormatters.
hawkinsw added inline comments.
Comment at: lldb/source/DataFormatters/FormatManager.cpp:595
+ {
+std::lock_guard guard(m_language_categories_mutex);
+m_language_categories_map[lang_type] =
Forgive me if I am speaking out of turn, but do we need to check again here for
whether `lang_type` is in the map? In other words, it seems possible that (in
some zany situation) while we are doing the memory allocation (line 593)
someone else has come along and added that category since we released the lock
on it in 592.
I hope that this is helpful. Please (again) forgive me if I am speaking out of
turn.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126240/new/
https://reviews.llvm.org/D126240
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D126015: [trace][intelpt] Support system-wide tracing [12] - Support multi-core trace load and save
jj10306 added a comment.
feedback-v2
Comment at: lldb/source/Plugins/Process/Linux/Perf.cpp:183
+Expected>
+PerfEvent::ReadFlushedOutDataCyclicBuffer(size_t offset, size_t size) {
+ CollectionState previous_state = m_collection_state;
Do we need the offset/size? These arguments make the reading logic much more
involved versus reading the whole buffer.
I understand that this is a more "general" method, but iiuc currently we only
read the entire buffer. I guess in the future somebody could want the
flexibility of providing an offset/size, but I don't see the need currently.
wdyt?
Same comment applies to the AUX buffer read method
Comment at: lldb/source/Plugins/Process/Linux/Perf.cpp:206-212
+for (uint64_t i = actual_data_head + offset;
+ i < data_size && output.size() < size; i++)
+ output.push_back(data[i]);
+
+// We need to find the starting position for the left part if the offset
was
+// too big
+uint64_t left_part_start = actual_data_head + offset >= data_size
What happens if an offset larger then the size of the buffer is provided?
should that offset be wrapped or not?
What happens if a size larger than the size of the buffer is provided? should
we only copy up to buffer size bytes or should we do multiple copies of the
buffer to satisfy the request?
As I mentioned above, imo the ambiguity/complexity these arguments introduce is
not worth the value add (which currently is none since we are always copying
the whole buffer).
If we do stick with these args, we should make it very clear what happens in
the cases I mentioned above and/or put restrictions on the ranges of these
values (ie they must always be less than the buffer size)
Comment at: lldb/source/Plugins/Trace/common/ThreadPostMortemTrace.h:47
/// The trace file of this thread.
- const FileSpec &GetTraceFile() const;
+ const llvm::Optional &GetTraceFile() const;
why is this now optional?
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h:169
+ ///
+ /// \param[in] traces_proceses
+ /// The processes traced in the live session.
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h:176-177
+ TraceIntelPT(JSONTraceSession &session,
+ llvm::ArrayRef traced_processes,
+ llvm::ArrayRef traced_threads);
do we need the threads explicitly or does `traced_processes` implicitly contain
this info?
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp:53
+ return json::Object{{"tid", thread.tid},
+ {"traceBuffer", thread.trace_buffer}};
+}
if this is none, does this still create a key value pair?
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp:88-90
+ if (!o || !o.map("coreId", core_id) ||
+ !o.map("traceBuffer", core.trace_buffer) ||
+ !o.map("contextSwitchTrace", core.context_switch_trace))
nit: use what De Morgan taught us (;
Having to add a `!` for each new addition to the predicate is very error prone
and would be a pain to debug if you accidentally forgot to add the negation.
Comment at:
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp:109-110
+ uint64_t family, model, stepping;
+ if (!o || !o.map("vendor", vendor) || !o.map("family", family) ||
+ !o.map("model", model) || !o.map("stepping", stepping))
+return false;
same comment as above related to De Morgan's law
Comment at:
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.cpp:131-133
+ if (!o || !o.map("processes", session.processes) ||
+ !o.map("type", session.type) || !o.map("cores", session.cores) ||
+ !o.map("tscPerfZeroConversion", session.tsc_perf_zero_conversion))
same comment as above related to De Morgan's law
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTJSONStructs.h:32-40
+struct JSONThread {
+ int64_t tid;
+ llvm::Optional trace_buffer;
};
-struct JSONTraceIntelPTTrace {
- std::string type;
- JSONTraceIntelPTCPUInfo cpuInfo;
+struct JSONProcess {
+ int64_t pid;
If the intention here is for system wide tracing to not provide threads while
single thread tracing does then should we change this like so?
(i know you're currently working on the thread detection so potentially this
will be addressed then?)
Comment at:
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp:31
+
+Error TraceIntelPTSessionFileParser::ParseModule(lldb::TargetSP &target_sp,
+ const JSONModule &module) {
take a reference to the underlying object directly?
Comment at:
lldb/source/Plugin
[Lldb-commits] [PATCH] D127001: [trace][intelpt] Support system-wide tracing [16] - Create threads automatically from context switch data in the post-mortem case
wallace created this revision.
wallace added a reviewer: jj10306.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
For some context, The context switch data contains information of which threads
were
executed by each traced process, therefore it's not necessary to specify
them in the trace file.
So this diffs adds support for that automatic feature. Eventually we
could include it to live processes as well.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D127001
Files:
lldb/include/lldb/Target/Trace.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
lldb/source/Target/Trace.cpp
lldb/test/API/commands/trace/TestTraceLoad.py
lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_missing_threads.json
Index: lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_missing_threads.json
===
--- /dev/null
+++ lldb/test/API/commands/trace/intelpt-multi-core-trace/trace_missing_threads.json
@@ -0,0 +1,44 @@
+{
+ "cores": [
+{
+ "contextSwitchTrace": "/tmp/trace8/cores/45.perf_context_switch_trace",
+ "coreId": 45,
+ "traceBuffer": "/tmp/trace8/cores/45.intelpt_trace"
+},
+{
+ "contextSwitchTrace": "/tmp/trace8/cores/51.perf_context_switch_trace",
+ "coreId": 51,
+ "traceBuffer": "/tmp/trace8/cores/51.intelpt_trace"
+}
+ ],
+ "cpuInfo": {
+"family": 6,
+"model": 85,
+"stepping": 4,
+"vendor": "GenuineIntel"
+ },
+ "processes": [
+{
+ "modules": [
+{
+ "file": "modules/m.out",
+ "systemPath": "/tmp/m.out",
+ "loadAddress": 4194304,
+ "uuid": "AEFB0D59-233F-80FF-6D3C-4DED498534CF-11017B3B"
+}
+ ],
+ "pid": 3497234,
+ "threads": [
+{
+ "tid": 3497234
+}
+ ]
+}
+ ],
+ "tscPerfZeroConversion": {
+"timeMult": 1076264588,
+"timeShift": 31,
+"timeZero": 18433473881008870804
+ },
+ "type": "intel-pt"
+}
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -21,6 +21,18 @@
substrs=["67910: [tsc=0x008fb5211bfdf270] 0x00400bd7addl $0x1, -0x4(%rbp)",
"m.out`bar() + 26 at multi_thread.cpp:20:6"])
+def testLoadMultiCoreTraceWithMissingThreads(self):
+src_dir = self.getSourceDir()
+trace_definition_file = os.path.join(src_dir, "intelpt-multi-core-trace", "trace_missing_threads.json")
+self.expect("trace load -v " + trace_definition_file, substrs=["intel-pt"])
+self.expect("thread trace dump instructions 3 -t",
+ substrs=["19521: [tsc=0x008fb5211c143fd8] error: expected tracing enabled event",
+ "m.out`foo() + 65 at multi_thread.cpp:12:21",
+ "19520: [tsc=0x008fb5211bfbc69e] 0x00400ba7jg 0x400bb3"])
+self.expect("thread trace dump instructions 2 -t",
+ substrs=["67910: [tsc=0x008fb5211bfdf270] 0x00400bd7addl $0x1, -0x4(%rbp)",
+ "m.out`bar() + 26 at multi_thread.cpp:20:6"])
+
def testLoadTrace(self):
src_dir = self.getSourceDir()
trace_definition_file = os.path.join(src_dir, "intelpt-trace", "trace.json")
Index: lldb/source/Target/Trace.cpp
===
--- lldb/source/Target/Trace.cpp
+++ lldb/source/Target/Trace.cpp
@@ -449,3 +449,14 @@
return *m_cores;
return {};
}
+
+std::vector Trace::GetTracedProcesses() const {
+ std::vector processes;
+
+ for (Process *proc : m_postmortem_processes)
+processes.push_back(proc);
+
+ if (m_live_process)
+processes.push_back(m_live_process);
+ return processes;
+}
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPTSessionFileParser.h
@@ -52,7 +52,7 @@
/// errors, return a null pointer.
llvm::Expected Parse();
- lldb::TraceSP
+ llvm::Expected
CreateTraceIntelPTInstance(JSONTraceSession &session,
std::vector &parsed_processes);
@@ -64,6 +64,11 @@
lldb::ThreadPostMortemTraceSP ParseThread(lldb::ProcessSP &process_sp,
const JSONThread &thread);
+ /// Create the corresponding Threads and Process objects given the JSON
+ /// process definition.
+ ///
+ /// \param[i
[Lldb-commits] [PATCH] D127016: [lldb] Prevent crash due to reading memory from page zero.
cassanova created this revision. cassanova added reviewers: JDevlieghere, mib. cassanova added a project: LLDB. Herald added a project: All. cassanova requested review of this revision. Herald added a subscriber: lldb-commits. Adds a check to ensure that a process exists before attempting to get its ABI to prevent lldb from crash due to trying to read from page zero. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D127016 Files: lldb/source/Commands/CommandObjectMemory.cpp lldb/test/Shell/Driver/TestPageZeroRead.test Index: lldb/test/Shell/Driver/TestPageZeroRead.test === --- /dev/null +++ lldb/test/Shell/Driver/TestPageZeroRead.test @@ -0,0 +1,3 @@ +# Ensure that the read from memory command doesn't try and read from page zero. +# RUN: %clang_host %p/Inputs/hello.c -g -o a.out +# RUN: %lldb -b a.out -o 'x 0' Index: lldb/source/Commands/CommandObjectMemory.cpp === --- lldb/source/Commands/CommandObjectMemory.cpp +++ lldb/source/Commands/CommandObjectMemory.cpp @@ -593,7 +593,10 @@ return false; } -ABISP abi = m_exe_ctx.GetProcessPtr()->GetABI(); +ABISP abi; +if (Process * proc = m_exe_ctx.GetProcessPtr()) + abi = proc->GetABI(); + if (abi) addr = abi->FixDataAddress(addr); Index: lldb/test/Shell/Driver/TestPageZeroRead.test === --- /dev/null +++ lldb/test/Shell/Driver/TestPageZeroRead.test @@ -0,0 +1,3 @@ +# Ensure that the read from memory command doesn't try and read from page zero. +# RUN: %clang_host %p/Inputs/hello.c -g -o a.out +# RUN: %lldb -b a.out -o 'x 0' Index: lldb/source/Commands/CommandObjectMemory.cpp === --- lldb/source/Commands/CommandObjectMemory.cpp +++ lldb/source/Commands/CommandObjectMemory.cpp @@ -593,7 +593,10 @@ return false; } -ABISP abi = m_exe_ctx.GetProcessPtr()->GetABI(); +ABISP abi; +if (Process * proc = m_exe_ctx.GetProcessPtr()) + abi = proc->GetABI(); + if (abi) addr = abi->FixDataAddress(addr); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D127016: [lldb] Prevent crash due to reading memory from page zero.
kastiglione added inline comments. Comment at: lldb/source/Commands/CommandObjectMemory.cpp:597-598 +ABISP abi; +if (Process * proc = m_exe_ctx.GetProcessPtr()) + abi = proc->GetABI(); + Should `memory read` emit an error if there's no process (and no core file or any other memory to read from)? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127016/new/ https://reviews.llvm.org/D127016 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D127016: [lldb] Prevent crash due to reading memory from page zero.
mib accepted this revision. mib added a comment. This revision is now accepted and ready to land. LGTM with some minor feedbacks :) ! Comment at: lldb/source/Commands/CommandObjectMemory.cpp:597 +ABISP abi; +if (Process * proc = m_exe_ctx.GetProcessPtr()) + abi = proc->GetABI(); kastiglione wrote: > Should `memory read` emit an error if there's no process (and no core file or > any other memory to read from)? Nit: I don't think this is formatted properly Comment at: lldb/source/Commands/CommandObjectMemory.cpp:597-598 +ABISP abi; +if (Process * proc = m_exe_ctx.GetProcessPtr()) + abi = proc->GetABI(); + mib wrote: > kastiglione wrote: > > Should `memory read` emit an error if there's no process (and no core file > > or any other memory to read from)? > Nit: I don't think this is formatted properly @kastiglione On Apple's lldb, we get this: ``` (lldb) x 0 error: error reading data from section __PAGEZERO (lldb) target delete 0 1 targets deleted. (lldb) x 0 error: invalid target, create a target using the 'target create' command (lldb) ``` Might be an oversight. Comment at: lldb/test/Shell/Driver/TestPageZeroRead.test:3 +# RUN: %clang_host %p/Inputs/hello.c -g -o a.out +# RUN: %lldb -b a.out -o 'x 0' You probably want to check the error string Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127016/new/ https://reviews.llvm.org/D127016 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D127016: [lldb] Prevent crash due to reading memory from page zero.
cassanova added inline comments. Comment at: lldb/source/Commands/CommandObjectMemory.cpp:597-598 +ABISP abi; +if (Process * proc = m_exe_ctx.GetProcessPtr()) + abi = proc->GetABI(); + kastiglione wrote: > Should `memory read` emit an error if there's no process (and no core file or > any other memory to read from)? Right now running `memory read` without a target indicates that there's no target. The input that can cause lldb to crash is `x 0` (such as `./bin/lldb -o 'x 0' ./bin/count`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127016/new/ https://reviews.llvm.org/D127016 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D127038: Add some documentation for the "breakpoint name" feature
jingham created this revision. jingham added reviewers: kastiglione, mib, JDevlieghere, clayborg. Herald added a project: All. jingham requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. breakpoint names have a bunch of uses, some obvious and some not so (to anyone who was not me...) so it seemed worthwhile to add some documentation. I hung it off the "breakpoint name" command - that's the natural place to look when you are browsing the command set. This seems clear to me, but it would be useful to know if it is so to others, or how to make it so... Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D127038 Files: lldb/source/Commands/CommandObjectBreakpoint.cpp Index: lldb/source/Commands/CommandObjectBreakpoint.cpp === --- lldb/source/Commands/CommandObjectBreakpoint.cpp +++ lldb/source/Commands/CommandObjectBreakpoint.cpp @@ -2018,7 +2018,93 @@ public: CommandObjectBreakpointName(CommandInterpreter &interpreter) : CommandObjectMultiword( -interpreter, "name", "Commands to manage name tags for breakpoints", +interpreter, "name", +R"( +Commands to manage "breakpoint names". + +Breakpoint names provide a general tagging mechanism for breakpoints. Each +breakpoint name can be added to any number of breakpoints, and each breakpoint +can have any number of breakpoint names attached to it. For instance: + +(lldb) break name add -N MyName 1-10 + +adds the name MyName to breakpoints 1-10, and: + +(lldb) break set -n myFunc -N Name1 -N Name2 + +adds two names to the breakpoint set at myFunc. + +They have a number of interrelated uses: + +1) They provide a stable way to refer to a breakpoint (e.g. in another +breakpoint's action). Using the breakpoint ID for this purpose is fragile, since +it depends on the order of breakpoint creation. Giving a name to the breakpoint +you want to act on, and then referring to it by name, is more robust: + +(lldb) break set -n myFunc -N BKPT1 +(lldb) break set -n myOtherFunc -C "break disable BKPT1" + +2) This is actually just a specific use of a more general feature of breakpoint +names. The argument type used to specify one or more +breakpoints in most of the commands that deal with breakpoints also accepts +breakpoint names. That allows you to refer to one breakpoint in a stable +manner, but also makes them a convenient grouping mechanism, allowing you to +easily act on a group of breakpoints by using their name, for instance disabling +them all in one action: + +(lldb) break set -n myFunc -N Group1 +(lldb) break set -n myOtherFunc -N Group1 +(lldb) break disable Group1 + +3) But breakpoint names are also entities in their own right, and can be +configured with all the modifiable attributes of a breakpoint. Then when you +add a breakpoint name to a breakpoint, the breakpoint will be configured to +match the state of the breakpoint name. The link between the name and the +breakpoints sharing it remains live, so if you change the configuration on the +name, it will also change the configurations on the breakpoints: + +(lldb) break name configure -i 10 IgnoreSome +(lldb) break set -n myFunc -N IgnoreSome +(lldb) break list IgnoreSome +2: name = 'myFunc', locations = 0 (pending) Options: ignore: 10 enabled + Names: +IgnoreSome +(lldb) break name configure -i 5 IgnoreSome +(lldb) break list IgnoreSome +2: name = 'myFunc', locations = 0 (pending) Options: ignore: 5 enabled + Names: +IgnoreSome + +Options that are not configured on a breakpoint name don't affect the value of +those options on the breakpoints they are added to. So for instance, if Name1 +has the -i option configured and Name2 the -c option, adding both names to a +breakpoint will set the -i option from Name1 and the -c option from Name2, and +the other options will be unaltered. + +If you add multiple names to a breakpoint which have configured values for +the same option, the last name added's value wins. + +The "liveness" of these settings is one way, from name to breakpoint. +If you use break modify to change an option that is also configured on a name +which that breakpoint has, the "break modify" command will override the setting +for that breakpoint, but won't change the value configured in the name or on the +other breakpoints sharing that name. + +4) Breakpoint names are also a convenient way to copy option sets from one +breakpoint to another. Using the -B option to "breakpoint name configure" makes +a name configured with all the options of the original breakpoint. Then +adding that name to another breakpoint copies over all the values from the +original breakpoint to the new one. + +5) You can also use breakpoint names to hide breakpoints from normal breakpoint +operations (list, delete and d
[Lldb-commits] [PATCH] D127016: [lldb] Prevent crash due to reading memory from page zero.
jingham added a comment. More generally, you can tell which elements of the exe_ctx you need to check in any given command by looking at requirements for the command which are passed into the constructor for the command. Any entity that's required there, you don't need to check for, but otherwise, you do have to check. "CommandObjectMemoryRead" has: eCommandRequiresTarget | eCommandProcessMustBePaused so you should not need to check the target, but any references to the process, thread or frame in the exe_ctx must be checked. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127016/new/ https://reviews.llvm.org/D127016 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D127038: Add some documentation for the "breakpoint name" feature
mib added a comment.
Thanks for writing that! I'm sure many users will find it very useful.
Comment at: lldb/source/Commands/CommandObjectBreakpoint.cpp:2088
+The "liveness" of these settings is one way, from name to breakpoint.
+If you use break modify to change an option that is also configured on a name
+which that breakpoint has, the "break modify" command will override the
setting
Can we add some kind of quoting (backticks maybe) to this ?
Comment at: lldb/source/Commands/CommandObjectBreakpoint.cpp:2099-2102
+5) You can also use breakpoint names to hide breakpoints from normal breakpoint
+operations (list, delete and disable) by running "break name configure -N
+" and specifying the --allow-{list,delete,disable} options and then
adding
+that name to a breakpoint. This won't keep the breakpoint from being deleted
Does it mean that giving a breakpoint a name will hide it by default to the
`list,delete,disable` commands ?
Comment at: lldb/source/Commands/CommandObjectBreakpoint.cpp:2103-2106
+or disabled if you refer to it specifically by ID, but "break delete" and
+"break disable" to delete or disable ALL breakpoints won't delete the ones
+sharing the name, and "break list" won't show them.
+
This sentences is already very long. I'd either surround `to delete or disable
ALL breakpoints` between commas or rephrase it like this.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127038/new/
https://reviews.llvm.org/D127038
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D127038: Add some documentation for the "breakpoint name" feature
jingham updated this revision to Diff 434222.
jingham added a comment.
Address review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127038/new/
https://reviews.llvm.org/D127038
Files:
lldb/source/Commands/CommandObjectBreakpoint.cpp
Index: lldb/source/Commands/CommandObjectBreakpoint.cpp
===
--- lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -2018,7 +2018,98 @@
public:
CommandObjectBreakpointName(CommandInterpreter &interpreter)
: CommandObjectMultiword(
-interpreter, "name", "Commands to manage name tags for breakpoints",
+interpreter, "name",
+R"(
+Commands to manage "breakpoint names".
+
+Breakpoint names provide a general tagging mechanism for breakpoints. Each
+breakpoint name can be added to any number of breakpoints, and each breakpoint
+can have any number of breakpoint names attached to it. For instance:
+
+(lldb) break name add -N MyName 1-10
+
+adds the name MyName to breakpoints 1-10, and:
+
+(lldb) break set -n myFunc -N Name1 -N Name2
+
+adds two names to the breakpoint set at myFunc.
+
+They have a number of interrelated uses:
+
+1) They provide a stable way to refer to a breakpoint (e.g. in another
+breakpoint's action). Using the breakpoint ID for this purpose is fragile, since
+it depends on the order of breakpoint creation. Giving a name to the breakpoint
+you want to act on, and then referring to it by name, is more robust:
+
+(lldb) break set -n myFunc -N BKPT1
+(lldb) break set -n myOtherFunc -C "break disable BKPT1"
+
+2) This is actually just a specific use of a more general feature of breakpoint
+names. The argument type used to specify one or more
+breakpoints in most of the commands that deal with breakpoints also accepts
+breakpoint names. That allows you to refer to one breakpoint in a stable
+manner, but also makes them a convenient grouping mechanism, allowing you to
+easily act on a group of breakpoints by using their name, for instance disabling
+them all in one action:
+
+(lldb) break set -n myFunc -N Group1
+(lldb) break set -n myOtherFunc -N Group1
+(lldb) break disable Group1
+
+3) But breakpoint names are also entities in their own right, and can be
+configured with all the modifiable attributes of a breakpoint. Then when you
+add a breakpoint name to a breakpoint, the breakpoint will be configured to
+match the state of the breakpoint name. The link between the name and the
+breakpoints sharing it remains live, so if you change the configuration on the
+name, it will also change the configurations on the breakpoints:
+
+(lldb) break name configure -i 10 IgnoreSome
+(lldb) break set -n myFunc -N IgnoreSome
+(lldb) break list IgnoreSome
+2: name = 'myFunc', locations = 0 (pending) Options: ignore: 10 enabled
+ Names:
+IgnoreSome
+(lldb) break name configure -i 5 IgnoreSome
+(lldb) break list IgnoreSome
+2: name = 'myFunc', locations = 0 (pending) Options: ignore: 5 enabled
+ Names:
+IgnoreSome
+
+Options that are not configured on a breakpoint name don't affect the value of
+those options on the breakpoints they are added to. So for instance, if Name1
+has the -i option configured and Name2 the -c option, adding both names to a
+breakpoint will set the -i option from Name1 and the -c option from Name2, and
+the other options will be unaltered.
+
+If you add multiple names to a breakpoint which have configured values for
+the same option, the last name added's value wins.
+
+The "liveness" of these settings is one way, from name to breakpoint.
+If you use "break modify" to change an option that is also configured on a name
+which that breakpoint has, the "break modify" command will override the setting
+for that breakpoint, but won't change the value configured in the name or on the
+other breakpoints sharing that name.
+
+4) Breakpoint names are also a convenient way to copy option sets from one
+breakpoint to another. Using the -B option to "breakpoint name configure" makes
+a name configured with all the options of the original breakpoint. Then
+adding that name to another breakpoint copies over all the values from the
+original breakpoint to the new one.
+
+5) You can also use breakpoint names to hide breakpoints from the breakpoint
+operations that act on all breakpoints: "break delete", "break disable" and
+"break list". You do that by specifying a "false" value for the
+--allow-{list,delete,disable} options to "breakpoint name configure" and then
+adding that name to a breakpoint.
+
+This won't keep the breakpoint from being deleted or disabled if you refer to it
+specifically by ID. The point of the feature is to make sure users don't
+inadvertently delete or disable useful breakpoints (e.g. ones an IDE is
[Lldb-commits] [PATCH] D127038: Add some documentation for the "breakpoint name" feature
jingham added a comment.
Mostly reworking the paragraph on breakpoint hiding.
Comment at: lldb/source/Commands/CommandObjectBreakpoint.cpp:2088
+The "liveness" of these settings is one way, from name to breakpoint.
+If you use break modify to change an option that is also configured on a name
+which that breakpoint has, the "break modify" command will override the
setting
mib wrote:
> Can we add some kind of quoting (backticks maybe) to this ?
I don't think we use backticks anywhere in the help, but that should have
quotes.
Comment at: lldb/source/Commands/CommandObjectBreakpoint.cpp:2099-2102
+5) You can also use breakpoint names to hide breakpoints from normal breakpoint
+operations (list, delete and disable) by running "break name configure -N
+" and specifying the --allow-{list,delete,disable} options and then
adding
+that name to a breakpoint. This won't keep the breakpoint from being deleted
mib wrote:
> Does it mean that giving a breakpoint a name will hide it by default to the
> `list,delete,disable` commands ?
Not sure what you are asking. These options are all off by default, so if you
add a name that doesn't have the --allow-list, --allow-delete or
--allow-disable explicitly configured, it won't affect how the breakpoint shows
up. But if you do configure these to true, the indeed, the breakpoint won't
show up.
IDE's often want to use some breakpoints for their own purposes, and it's
annoying when a user does "break delete" and then messes up their IDE because
they deleted breakpoints it was relying on. This is a mechanism to protect
those breakpoints, but it is purely opt in.
Comment at: lldb/source/Commands/CommandObjectBreakpoint.cpp:2103-2106
+or disabled if you refer to it specifically by ID, but "break delete" and
+"break disable" to delete or disable ALL breakpoints won't delete the ones
+sharing the name, and "break list" won't show them.
+
mib wrote:
> This sentences is already very long. I'd either surround `to delete or
> disable ALL breakpoints` between commas or rephrase it like this.
That was a little contorted. I rewrote it to be clearer, see what you think.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127038/new/
https://reviews.llvm.org/D127038
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D126240: [lldb] Tighten the scope of a couple of locks in DataFormatters.
jgorbe added inline comments.
Comment at: lldb/source/DataFormatters/FormatManager.cpp:595
+ {
+std::lock_guard guard(m_language_categories_mutex);
+m_language_categories_map[lang_type] =
hawkinsw wrote:
> Forgive me if I am speaking out of turn, but do we need to check again here
> for whether `lang_type` is in the map? In other words, it seems possible that
> (in some zany situation) while we are doing the memory allocation (line 593)
> someone else has come along and added that category since we released the
> lock on it in 592.
>
> I hope that this is helpful. Please (again) forgive me if I am speaking out
> of turn.
Not at all! Thank you for your comment! :)
You are absolutely right, I had overlooked this case. Another annoying
consequence is that in this case the racing threads would create multiple
`LanguageCategory` objects. The `LanguageCategory` constructor calls `Enable`
which in turn ends up calling the `Changed` method of the listener, so even if
we check the map before insertion I think we'd still see the listener receiving
multiple notifications that the category was enabled, which is not ideal.
I don't know how to properly fix this. I'll give it some more thought.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126240/new/
https://reviews.llvm.org/D126240
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
