[Lldb-commits] [lldb] [lldb][test] Remove vendored packages `unittest2` and `progress` (PR #82670)
https://github.com/DavidSpickett approved this pull request. https://github.com/llvm/llvm-project/pull/82670 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 55bc048 - Improve and modernize logging for Process::CompleteAttach() (#82717)
Author: Adrian Prantl
Date: 2024-02-23T08:00:58-08:00
New Revision: 55bc0488af077acb47be70542718d1bc17f3de4f
URL:
https://github.com/llvm/llvm-project/commit/55bc0488af077acb47be70542718d1bc17f3de4f
DIFF:
https://github.com/llvm/llvm-project/commit/55bc0488af077acb47be70542718d1bc17f3de4f.diff
LOG: Improve and modernize logging for Process::CompleteAttach() (#82717)
Target::SetArchitecture() does not necessarily set the triple that is
being passed in, and will unconditionally log the real architecture to
the log channel. By flipping the order between the log outputs, the
resulting combined log makes a lot more sense to read.
Added:
Modified:
lldb/source/Target/Process.cpp
Removed:
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 23a8a66645c02d..137795cb8cec9e 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -2937,14 +2937,11 @@ void Process::CompleteAttach() {
DidAttach(process_arch);
if (process_arch.IsValid()) {
+LLDB_LOG(log,
+ "Process::{0} replacing process architecture with DidAttach() "
+ "architecture: \"{1}\"",
+ __FUNCTION__, process_arch.GetTriple().getTriple());
GetTarget().SetArchitecture(process_arch);
-if (log) {
- const char *triple_str = process_arch.GetTriple().getTriple().c_str();
- LLDB_LOGF(log,
-"Process::%s replacing process architecture with DidAttach() "
-"architecture: %s",
-__FUNCTION__, triple_str ? triple_str : "");
-}
}
// We just attached. If we have a platform, ask it for the process
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Improve and modernize logging for Process::CompleteAttach() (PR #82717)
https://github.com/adrian-prantl closed https://github.com/llvm/llvm-project/pull/82717 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Print a message when background tasks take a while to complete (PR #82799)
https://github.com/JDevlieghere created
https://github.com/llvm/llvm-project/pull/82799
When terminating the debugger, we wait for all background tasks to complete.
Given that there's no way to interrupt those treads, this can take a while.
When that happens, the debugger appears to hang at exit.
The above situation is unfortunately not uncommon when background downloading
of dSYMs is enabled (`symbols.auto-download background`). Even when calling
dsymForUUID with a reasonable timeout, it can take a while to complete.
This patch improves the user experience by registering a callback and printing
a message when we're waiting on the thread pool for more than a second. Both
the timeout and the callback is configurable.
Fixing the underlying issue is a much harder problem. Neither C++ threads no
pthread support interrupting threads, so it would have to be a cooperative
interruption mechanism, similar to how we deal with this for HostThreads. If we
go that route, we should do it at the thread pool level so that it applies to
all background tasks.
A cooperative interruption mechanism alone solve the dsymForUUID. It's a
blocking call to an external binary, so we'd have to monitor for interrupts and
kill the execution from yet another thread.
>From 8ff01f8e7060ba46540503a48c9ec8bbefd2076c Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Fri, 23 Feb 2024 09:13:06 -0800
Subject: [PATCH] [lldb] Print a message when background tasks take a while to
complete
When terminating the debugger, we wait for all background tasks to
complete. Given that there's no way to interrupt those treads, this can
take a while. When that happens, the debugger appears to hang at exit.
The above situation is unfortunately not uncommon when background
downloading of dSYMs is enabled (`symbols.auto-download background`).
Even when calling dsymForUUID with a reasonable timeout, it can take a
while to complete.
This patch improves the user experience by registering a callback and
printing a message when we're waiting on the thread pool for more than a
second. Both the timeout and the callback is configurable.
Fixing the underlying issue is a much harder problem. Neither C++
threads no pthread support interrupting threads, so it would have to be
a cooperative interruption mechanism, similar to how we deal with this
for HostThreads. If we go that route, we should do it at the thread pool
level so that it applies to all background tasks.
A cooperative interruption mechanism alone solve the dsymForUUID. It's a
blocking call to an external binary, so we'd have to monitor for
interrupts and kill the execution from yet another thread.
---
lldb/include/lldb/API/SBDebugger.h | 7 +++
lldb/include/lldb/API/SBDefines.h | 1 +
lldb/include/lldb/Core/Debugger.h | 5 +
lldb/include/lldb/lldb-types.h | 1 +
lldb/source/API/SBDebugger.cpp | 8
lldb/source/Core/Debugger.cpp | 31 --
lldb/tools/driver/Driver.cpp | 6 ++
7 files changed, 57 insertions(+), 2 deletions(-)
diff --git a/lldb/include/lldb/API/SBDebugger.h
b/lldb/include/lldb/API/SBDebugger.h
index 62b2f91f5076d5..c2b4e6adfa5d27 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -331,6 +331,13 @@ class LLDB_API SBDebugger {
void SetDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
void *baton);
+ /// Register a callback that is called when destroying the thread pool
exceeds
+ /// the given timeout.
+ static void
+ SetThreadPoolTimeoutCallback(uint64_t timeout_milliseconds,
+ lldb::SBDebuggerTimeoutCallback
timeout_callback,
+ void *baton);
+
#ifndef SWIG
LLDB_DEPRECATED_FIXME("Use DispatchInput(const void *, size_t)",
"DispatchInput(const void *, size_t)")
diff --git a/lldb/include/lldb/API/SBDefines.h
b/lldb/include/lldb/API/SBDefines.h
index 1181920677b46f..5df380a250278c 100644
--- a/lldb/include/lldb/API/SBDefines.h
+++ b/lldb/include/lldb/API/SBDefines.h
@@ -138,6 +138,7 @@ typedef bool (*SBBreakpointHitCallback)(void *baton,
SBProcess &process,
typedef void (*SBDebuggerDestroyCallback)(lldb::user_id_t debugger_id,
void *baton);
+typedef void (*SBDebuggerTimeoutCallback)(void *baton);
typedef SBError (*SBPlatformLocateModuleCallback)(
void *baton, const SBModuleSpec &module_spec, SBFileSpec &module_file_spec,
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 6ba90eb6ed8fdf..b16334d302a47c 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -250,6 +250,11 @@ class Debugger : public
std::enable_shared_from_this,
void SetLoggingCallback(lldb::LogOutputCallback log_callback, void *baton);
+ static void
+ SetThreadPoolTimeoutCallback(std::chrono::milliseconds timeout_mil
[Lldb-commits] [lldb] [lldb] Print a message when background tasks take a while to complete (PR #82799)
https://github.com/JDevlieghere edited https://github.com/llvm/llvm-project/pull/82799 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Print a message when background tasks take a while to complete (PR #82799)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Jonas Devlieghere (JDevlieghere)
Changes
When terminating the debugger, we wait for all background tasks to complete.
Given that there's no way to interrupt those treads, this can take a while.
When that happens, the debugger appears to hang at exit.
The above situation is unfortunately not uncommon when background downloading
of dSYMs is enabled (`symbols.auto-download background`). Even when calling
dsymForUUID with a reasonable timeout, it can take a while to complete.
This patch improves the user experience by registering a callback and printing
a message when we're waiting on the thread pool for more than a second. Both
the timeout and the callback is configurable.
Fixing the underlying issue is a much harder problem. Neither C++ threads no
pthread support interrupting threads, so it would have to be a cooperative
interruption mechanism, similar to how we deal with this for HostThreads. If we
go that route, we should do it at the thread pool level so that it applies to
all background tasks.
A cooperative interruption mechanism alone solve the dsymForUUID problem. It's
a blocking call to an external binary, so we'd have to monitor for interrupts
and kill the execution from yet another thread.
---
Full diff: https://github.com/llvm/llvm-project/pull/82799.diff
7 Files Affected:
- (modified) lldb/include/lldb/API/SBDebugger.h (+7)
- (modified) lldb/include/lldb/API/SBDefines.h (+1)
- (modified) lldb/include/lldb/Core/Debugger.h (+5)
- (modified) lldb/include/lldb/lldb-types.h (+1)
- (modified) lldb/source/API/SBDebugger.cpp (+8)
- (modified) lldb/source/Core/Debugger.cpp (+29-2)
- (modified) lldb/tools/driver/Driver.cpp (+6)
``diff
diff --git a/lldb/include/lldb/API/SBDebugger.h
b/lldb/include/lldb/API/SBDebugger.h
index 62b2f91f5076d5..c2b4e6adfa5d27 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -331,6 +331,13 @@ class LLDB_API SBDebugger {
void SetDestroyCallback(lldb::SBDebuggerDestroyCallback destroy_callback,
void *baton);
+ /// Register a callback that is called when destroying the thread pool
exceeds
+ /// the given timeout.
+ static void
+ SetThreadPoolTimeoutCallback(uint64_t timeout_milliseconds,
+ lldb::SBDebuggerTimeoutCallback
timeout_callback,
+ void *baton);
+
#ifndef SWIG
LLDB_DEPRECATED_FIXME("Use DispatchInput(const void *, size_t)",
"DispatchInput(const void *, size_t)")
diff --git a/lldb/include/lldb/API/SBDefines.h
b/lldb/include/lldb/API/SBDefines.h
index 1181920677b46f..5df380a250278c 100644
--- a/lldb/include/lldb/API/SBDefines.h
+++ b/lldb/include/lldb/API/SBDefines.h
@@ -138,6 +138,7 @@ typedef bool (*SBBreakpointHitCallback)(void *baton,
SBProcess &process,
typedef void (*SBDebuggerDestroyCallback)(lldb::user_id_t debugger_id,
void *baton);
+typedef void (*SBDebuggerTimeoutCallback)(void *baton);
typedef SBError (*SBPlatformLocateModuleCallback)(
void *baton, const SBModuleSpec &module_spec, SBFileSpec &module_file_spec,
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 6ba90eb6ed8fdf..b16334d302a47c 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -250,6 +250,11 @@ class Debugger : public
std::enable_shared_from_this,
void SetLoggingCallback(lldb::LogOutputCallback log_callback, void *baton);
+ static void
+ SetThreadPoolTimeoutCallback(std::chrono::milliseconds timeout_milliseconds,
+ lldb::TimeoutCallback timeout_callback,
+ void *baton);
+
// Properties Functions
enum StopDisassemblyType {
eStopDisassemblyTypeNever = 0,
diff --git a/lldb/include/lldb/lldb-types.h b/lldb/include/lldb/lldb-types.h
index d60686e33142ac..e1d58f02f60820 100644
--- a/lldb/include/lldb/lldb-types.h
+++ b/lldb/include/lldb/lldb-types.h
@@ -69,6 +69,7 @@ typedef int pipe_t; // Host pipe type
#define LLDB_INVALID_HOST_THREAD ((lldb::thread_t)NULL)
#define LLDB_INVALID_PIPE ((lldb::pipe_t)-1)
+typedef void (*TimeoutCallback)(void *baton);
typedef void (*LogOutputCallback)(const char *, void *baton);
typedef bool (*CommandOverrideCallback)(void *baton, const char **argv);
typedef bool (*ExpressionCancelCallback)(ExpressionEvaluationPhase phase,
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index fbcf30e67fc1cd..5c78b4fa05aaaf 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -1695,6 +1695,14 @@ void SBDebugger::SetDestroyCallback(
}
}
+void SBDebugger::SetThreadPoolTimeoutCallback(
+uint64_t timeout_milliseconds,
+lldb::SBDebuggerTimeoutCallback timeout_callback, void *baton) {
+ LLDB_INSTRUMENT_VA(timeout_milliseconds,
[Lldb-commits] [lldb] [lldb] Correctly annotate threads at a bp site as hitting it (PR #82709)
https://github.com/jimingham edited https://github.com/llvm/llvm-project/pull/82709 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Correctly annotate threads at a bp site as hitting it (PR #82709)
@@ -1722,6 +1742,8 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
} else {
bool handled = false;
bool did_exec = false;
+ if (reason == "none")
jimingham wrote:
This definitely needs a comment, or maybe somewhere else saying that none ==
empty, so we always convert none to empty.
https://github.com/llvm/llvm-project/pull/82709
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Correctly annotate threads at a bp site as hitting it (PR #82709)
https://github.com/jimingham approved this pull request. This seems safe to me, and it can't hurt to take care of this corner case here regardless of what the higher levels of lldb does. It bugs me a little that we're treating what seems like a general problem down in the GDBRemote process plugin. But anyway, if we are going to make this a process plugin policy: that the plugins are responsible for producing breakpoint stop reasons both for when the underlying system generates one and when we see "artificial" hits like this we should probably say so somewhere. With the couple doc comments aside, I'm fine with this. https://github.com/llvm/llvm-project/pull/82709 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/81564 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/81564 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)
@@ -0,0 +1,70 @@
+#include
+#include
+#include
+#include
+#include
+#include
+
+pid_t g_pid = 0;
+std::mutex g_child_pids_mutex;
+std::vector g_child_pids;
+
+int call_vfork(int index) {
+ pid_t child_pid = vfork();
+
+ if (child_pid == -1) {
+// Error handling
+perror("vfork");
+return 1;
+ } else if (child_pid == 0) {
+// This code is executed by the child process
+g_pid = getpid();
+printf("Child process: %d\n", g_pid);
clayborg wrote:
Add code to call `exec*()` here:
```
if (call_exec)
execl(exec_path, exec_path, NULL); // Call this program through exec() with
no args.
else
_exit(index + 10); // Exit the child process
```
https://github.com/llvm/llvm-project/pull/81564
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)
@@ -0,0 +1,70 @@
+#include
+#include
+#include
+#include
+#include
+#include
+
+pid_t g_pid = 0;
+std::mutex g_child_pids_mutex;
+std::vector g_child_pids;
+
+int call_vfork(int index) {
clayborg wrote:
We probably want to test with both `fork()` and `vfork()` here and we want to
know if we want to call `exec*()` or not. If we add a `const char *exec_path`
to the call and this is not NULL, then we call exec. So maybe change this to be:
```
int call_vfork(int index, bool use_vfork, const char *exec_path)
```
https://github.com/llvm/llvm-project/pull/81564
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)
@@ -5681,7 +5685,10 @@ void ProcessGDBRemote::DidVForkDone() {
void ProcessGDBRemote::DidExec() {
// If we are following children, vfork is finished by exec (rather than
// vforkdone that is submitted for parent).
- if (GetFollowForkMode() == eFollowChild)
-m_vfork_in_progress = false;
+ if (GetFollowForkMode() == eFollowChild) {
+assert(m_vfork_in_progress_count > 0);
+if (m_vfork_in_progress_count > 0)
+ --m_vfork_in_progress_count;
clayborg wrote:
We need to verify this is needed here. Usually someone does a `fork()` or
`vfork()` and then they do an `exec*()` call (there are many different flavors
of exec:
```
int execl(const char *path, const char *arg0, ..., /*, (char *)0, */);
int execle(const char *path, const char *arg0, ..., /* (char *)0 char *const
envp[] */);
int execlp(const char *file, const char *arg0, ..., /*, (char *)0, */);
int execv(const char *path, char *const argv[]);
int execvp(const char *file, char *const argv[]);
int execvP(const char *file, const char *search_path, char *const argv[]);
```
So we need to verify in our test case that if we do a `fork() + exec*()` or
`vfork() + exec*()` call that we don't run both
`ProcessGDBRemote::DidVForkDone()` _and_ `ProcessGDBRemote::DidExec()` because
if we do this assertion will fire.
https://github.com/llvm/llvm-project/pull/81564
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)
@@ -0,0 +1,70 @@
+#include
+#include
+#include
+#include
+#include
+#include
+
+pid_t g_pid = 0;
+std::mutex g_child_pids_mutex;
+std::vector g_child_pids;
+
+int call_vfork(int index) {
+ pid_t child_pid = vfork();
clayborg wrote:
Use either `fork()` of `vfork()` depending on the value of the `use_vfork`
argument:
```
pid_t child_pid = use_vfork ? vfork() : fork();
```
https://github.com/llvm/llvm-project/pull/81564
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)
@@ -0,0 +1,70 @@
+#include
+#include
+#include
+#include
+#include
+#include
+
+pid_t g_pid = 0;
+std::mutex g_child_pids_mutex;
+std::vector g_child_pids;
+
+int call_vfork(int index) {
+ pid_t child_pid = vfork();
+
+ if (child_pid == -1) {
+// Error handling
+perror("vfork");
+return 1;
+ } else if (child_pid == 0) {
+// This code is executed by the child process
+g_pid = getpid();
+printf("Child process: %d\n", g_pid);
+_exit(index + 10); // Exit the child process
+ } else {
+// This code is executed by the parent process
+printf("[Parent] Forked process id: %d\n", child_pid);
+ }
+ return 0;
+}
+
+void wait_all_children_to_exit() {
+ std::lock_guard Lock(g_child_pids_mutex);
+ for (pid_t child_pid : g_child_pids) {
+int child_status = 0;
+pid_t pid = waitpid(child_pid, &child_status, 0);
+if (child_status != 0) {
+ int exit_code = WEXITSTATUS(child_status);
+ if (exit_code > 15 || exit_code < 10) {
+printf("Error: child process exits with unexpected code %d\n",
+ exit_code);
+_exit(1); // This will let our program know that some child processes
+ // didn't exist with an expected exit status.
+ }
+}
+if (pid != child_pid)
+ _exit(2); // This will let our program know it didn't succeed
+ }
+}
+
+void create_threads(int num_threads) {
+ std::vector threads;
+ for (int i = 0; i < num_threads; ++i) {
+threads.emplace_back(std::thread(call_vfork, i));
+ }
+ printf("Created %d threads, joining...\n",
+ num_threads); // end_of_create_threads
+ for (auto &thread : threads) {
+thread.join();
+ }
+ wait_all_children_to_exit();
+}
+
+int main() {
+ g_pid = getpid();
+ printf("Entering main() pid: %d\n", g_pid);
+
clayborg wrote:
parse args manually to check if we should use `fork` or `vfork` and if we
should exec:
```
bool use_vfork = true;
bool call_exec = false;
for (int i=1; ihttps://github.com/llvm/llvm-project/pull/81564
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)
@@ -5670,8 +5673,9 @@ void ProcessGDBRemote::DidVFork(lldb::pid_t child_pid,
lldb::tid_t child_tid) {
}
void ProcessGDBRemote::DidVForkDone() {
- assert(m_vfork_in_progress);
- m_vfork_in_progress = false;
+ assert(m_vfork_in_progress_count > 0);
+ if (m_vfork_in_progress_count > 0)
+--m_vfork_in_progress_count;
clayborg wrote:
We decrement `m_vfork_in_progress_count` in two places, here and in
`ProcessGDBRemote::DidExec()`. Before this wouldn't affect anything because it
was a boolean, but I fear we will decrement this twice now. We probably need
to add an `exec()` into our test case for this to ensure this assertion doesn't
fire as I believe the assertion inside of `ProcessGDBRemote::DidExec()` will
assert and crash.
https://github.com/llvm/llvm-project/pull/81564
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)
@@ -5615,8 +5614,12 @@ void ProcessGDBRemote::DidFork(lldb::pid_t child_pid,
lldb::tid_t child_tid) {
void ProcessGDBRemote::DidVFork(lldb::pid_t child_pid, lldb::tid_t child_tid) {
Log *log = GetLog(GDBRLog::Process);
- assert(!m_vfork_in_progress);
- m_vfork_in_progress = true;
+ LLDB_LOG(
+ log,
+ "ProcessGDBRemote::DidFork() called for child_pid: {0}, child_tid {1}",
+ child_pid, child_tid);
+ assert(m_vfork_in_progress >= 0);
clayborg wrote:
remove this assert now that `m_vfork_in_progress` in unsigned, it doesn't do
anything.
https://github.com/llvm/llvm-project/pull/81564
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)
@@ -0,0 +1,49 @@
+"""
+Make sure that the concurrent vfork() from multiple threads works correctly.
+"""
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+
+
+class TestConcurrentVFork(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def get_pid_from_variable(self):
+target = self.dbg.GetTargetAtIndex(0)
+return target.FindFirstGlobalVariable("g_pid").GetValueAsUnsigned()
+
+@skipIfWindows
+def test_vfork_follow_parent(self):
+"""
+Make sure that debugging concurrent vfork() from multiple threads
won't crash lldb during follow-parent.
+And follow-parent successfully detach all child processes and exit
debugger.
+"""
+
+self.build()
clayborg wrote:
We need to add arguments to this to specify if we are calling `vfork` or `fork`
and if we are calling exec.
We should make a helper function that calls `lldbutil.run_to_source_breakpoint`
so it can be used correctly with the right arguemnts getting to the program we
launch:
```
def run_to_breakpoint(self, use_fork, call_exec):
args = [self.getBuildArtifact("a.out")]
if use_fork:
args.append("--fork");
if call_exec:
args.append("--exec");
launch_info = lldb.SBLaunchInfo(args)
launch_info.SetWorkingDirectory(self.getBuildDir())
lldbutil.run_to_source_breakpoint(
self, "// break here", lldb.SBFileSpec("main.cpp"),
launch_info=launch_info
)
```
Then we can call this function from each of the variations we need:
```
def test_vfork_follow_parent_no_exec(self):
def test_vfork_follow_parent_with_exec(self):
def test_vfork_follow_child_no_exec(self):
def test_vfork_follow_child_with_exec(self):
```
https://github.com/llvm/llvm-project/pull/81564
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)
@@ -0,0 +1,49 @@
+"""
+Make sure that the concurrent vfork() from multiple threads works correctly.
+"""
+
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+
+
+class TestConcurrentVFork(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+def get_pid_from_variable(self):
+target = self.dbg.GetTargetAtIndex(0)
+return target.FindFirstGlobalVariable("g_pid").GetValueAsUnsigned()
+
+@skipIfWindows
+def test_vfork_follow_parent(self):
+"""
+Make sure that debugging concurrent vfork() from multiple threads
won't crash lldb during follow-parent.
+And follow-parent successfully detach all child processes and exit
debugger.
+"""
+
+self.build()
+lldbutil.run_to_source_breakpoint(
+self, "// break here", lldb.SBFileSpec("main.cpp")
+)
clayborg wrote:
This will become:
```
use_fork = False # Call `vfork()`
call_exec = False # Don't call `exec()`
self.run_to_breakpoint(use_fork, call_exec);
```
And all of the other variations will need to set `use_fork` and `call_exec`
correctly.
https://github.com/llvm/llvm-project/pull/81564
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)
@@ -0,0 +1,70 @@
+#include
+#include
+#include
+#include
+#include
+#include
+
+pid_t g_pid = 0;
+std::mutex g_child_pids_mutex;
+std::vector g_child_pids;
+
+int call_vfork(int index) {
+ pid_t child_pid = vfork();
+
+ if (child_pid == -1) {
+// Error handling
+perror("vfork");
+return 1;
+ } else if (child_pid == 0) {
+// This code is executed by the child process
+g_pid = getpid();
+printf("Child process: %d\n", g_pid);
+_exit(index + 10); // Exit the child process
+ } else {
+// This code is executed by the parent process
+printf("[Parent] Forked process id: %d\n", child_pid);
+ }
+ return 0;
+}
+
+void wait_all_children_to_exit() {
+ std::lock_guard Lock(g_child_pids_mutex);
+ for (pid_t child_pid : g_child_pids) {
+int child_status = 0;
+pid_t pid = waitpid(child_pid, &child_status, 0);
+if (child_status != 0) {
+ int exit_code = WEXITSTATUS(child_status);
+ if (exit_code > 15 || exit_code < 10) {
+printf("Error: child process exits with unexpected code %d\n",
+ exit_code);
+_exit(1); // This will let our program know that some child processes
+ // didn't exist with an expected exit status.
+ }
+}
+if (pid != child_pid)
+ _exit(2); // This will let our program know it didn't succeed
+ }
+}
+
+void create_threads(int num_threads) {
+ std::vector threads;
+ for (int i = 0; i < num_threads; ++i) {
+threads.emplace_back(std::thread(call_vfork, i));
+ }
+ printf("Created %d threads, joining...\n",
+ num_threads); // end_of_create_threads
+ for (auto &thread : threads) {
+thread.join();
+ }
+ wait_all_children_to_exit();
+}
+
+int main() {
clayborg wrote:
Add `argc` and `argv` so we can parse args to see which fork we should use and
if we should call exec().
```
int main(int argc, const char **argv) {
```
https://github.com/llvm/llvm-project/pull/81564
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)
@@ -0,0 +1,70 @@
+#include
+#include
+#include
+#include
+#include
+#include
+
+pid_t g_pid = 0;
+std::mutex g_child_pids_mutex;
+std::vector g_child_pids;
+
+int call_vfork(int index) {
+ pid_t child_pid = vfork();
+
+ if (child_pid == -1) {
+// Error handling
+perror("vfork");
clayborg wrote:
```
perror(use_vfork ? "vfork" : "fork");
```
https://github.com/llvm/llvm-project/pull/81564
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Fix lldb crash while handling concurrent vfork() (PR #81564)
@@ -0,0 +1,70 @@
+#include
+#include
+#include
+#include
+#include
+#include
+
+pid_t g_pid = 0;
+std::mutex g_child_pids_mutex;
+std::vector g_child_pids;
+
+int call_vfork(int index) {
+ pid_t child_pid = vfork();
+
+ if (child_pid == -1) {
+// Error handling
+perror("vfork");
+return 1;
+ } else if (child_pid == 0) {
+// This code is executed by the child process
+g_pid = getpid();
+printf("Child process: %d\n", g_pid);
+_exit(index + 10); // Exit the child process
+ } else {
+// This code is executed by the parent process
+printf("[Parent] Forked process id: %d\n", child_pid);
+ }
+ return 0;
+}
+
+void wait_all_children_to_exit() {
+ std::lock_guard Lock(g_child_pids_mutex);
+ for (pid_t child_pid : g_child_pids) {
+int child_status = 0;
+pid_t pid = waitpid(child_pid, &child_status, 0);
+if (child_status != 0) {
+ int exit_code = WEXITSTATUS(child_status);
+ if (exit_code > 15 || exit_code < 10) {
+printf("Error: child process exits with unexpected code %d\n",
+ exit_code);
+_exit(1); // This will let our program know that some child processes
+ // didn't exist with an expected exit status.
+ }
+}
+if (pid != child_pid)
+ _exit(2); // This will let our program know it didn't succeed
+ }
+}
+
+void create_threads(int num_threads) {
+ std::vector threads;
+ for (int i = 0; i < num_threads; ++i) {
+threads.emplace_back(std::thread(call_vfork, i));
+ }
+ printf("Created %d threads, joining...\n",
+ num_threads); // end_of_create_threads
+ for (auto &thread : threads) {
+thread.join();
+ }
+ wait_all_children_to_exit();
+}
+
+int main() {
+ g_pid = getpid();
+ printf("Entering main() pid: %d\n", g_pid);
+
+ int num_threads = 5; // break here
+ create_threads(num_threads);
clayborg wrote:
pass `use_vfork` and `exec_path` into this function so we can pass it to `int
call_vfork(int index, bool use_vfork, const char *exec_path)`:
```
create_threads(num_threads, use_vfork, call_exec ? argv[0] : nullptr);
```
https://github.com/llvm/llvm-project/pull/81564
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Replace ArchSpec::PiecewiseCompare() with Triple::operator==() (PR #82804)
https://github.com/adrian-prantl created
https://github.com/llvm/llvm-project/pull/82804
Looking ast the definition of both functions this is *almost* an NFC change,
except that Triple also looks at the SubArch (important) and ObjectFormat (less
so).
This fixes a bug that only manifests with how Xcode uses the SBAPI to attach to
a process by name: it guesses the architecture based on the system. If the
system is arm64 and the Process is arm64e Target fails to update the triple
because it deemed the two to be equivalent.
rdar://123338218
>From f1541edda94503adfaa52ef75e1fb53fce5f608e Mon Sep 17 00:00:00 2001
From: Adrian Prantl
Date: Fri, 23 Feb 2024 09:58:17 -0800
Subject: [PATCH] Replace ArchSpec::PiecewiseCompare() with
Triple::operator==()
Looking ast the definition of both functions this is *almost* an NFC
change, except that Triple also looks at the SubArch (important) and
ObjectFormat (less so).
This fixes a bug that only manifests with how Xcode uses the SBAPI to
attach to a process by name: it guesses the architecture based on the
system. If the system is arm64 and the Process is arm64e Target fails
to update the triple because it deemed the two to be equivalent.
rdar://123338218
---
lldb/include/lldb/Utility/ArchSpec.h | 5
lldb/source/Target/Target.cpp | 8 +-
lldb/source/Utility/ArchSpec.cpp | 17 -
lldb/test/API/macosx/arm64e-attach/Makefile | 2 ++
.../macosx/arm64e-attach/TestArm64eAttach.py | 25 +++
lldb/test/API/macosx/arm64e-attach/main.c | 2 ++
6 files changed, 30 insertions(+), 29 deletions(-)
create mode 100644 lldb/test/API/macosx/arm64e-attach/Makefile
create mode 100644 lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
create mode 100644 lldb/test/API/macosx/arm64e-attach/main.c
diff --git a/lldb/include/lldb/Utility/ArchSpec.h
b/lldb/include/lldb/Utility/ArchSpec.h
index a226a3a5a9b71d..50830b889b9115 100644
--- a/lldb/include/lldb/Utility/ArchSpec.h
+++ b/lldb/include/lldb/Utility/ArchSpec.h
@@ -505,11 +505,6 @@ class ArchSpec {
bool IsFullySpecifiedTriple() const;
- void PiecewiseTripleCompare(const ArchSpec &other, bool &arch_different,
- bool &vendor_different, bool &os_different,
- bool &os_version_different,
- bool &env_different) const;
-
/// Detect whether this architecture uses thumb code exclusively
///
/// Some embedded ARM chips (e.g. the ARM Cortex M0-7 line) can only execute
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index e17bfcb5d5e2ad..e982a30a3ae4ff 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1568,14 +1568,8 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec,
bool set_platform,
if (m_arch.GetSpec().IsCompatibleMatch(other)) {
compatible_local_arch = true;
-bool arch_changed, vendor_changed, os_changed, os_ver_changed,
-env_changed;
-m_arch.GetSpec().PiecewiseTripleCompare(other, arch_changed,
-vendor_changed, os_changed,
-os_ver_changed, env_changed);
-
-if (!arch_changed && !vendor_changed && !os_changed && !env_changed)
+if (m_arch.GetSpec().GetTriple() == other.GetTriple())
replace_local_arch = false;
}
}
diff --git a/lldb/source/Utility/ArchSpec.cpp b/lldb/source/Utility/ArchSpec.cpp
index fb0e985a0d5657..07ef435ef451d2 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -1421,23 +1421,6 @@ bool ArchSpec::IsFullySpecifiedTriple() const {
return true;
}
-void ArchSpec::PiecewiseTripleCompare(
-const ArchSpec &other, bool &arch_different, bool &vendor_different,
-bool &os_different, bool &os_version_different, bool &env_different) const
{
- const llvm::Triple &me(GetTriple());
- const llvm::Triple &them(other.GetTriple());
-
- arch_different = (me.getArch() != them.getArch());
-
- vendor_different = (me.getVendor() != them.getVendor());
-
- os_different = (me.getOS() != them.getOS());
-
- os_version_different = (me.getOSMajorVersion() != them.getOSMajorVersion());
-
- env_different = (me.getEnvironment() != them.getEnvironment());
-}
-
bool ArchSpec::IsAlwaysThumbInstructions() const {
std::string Status;
if (GetTriple().getArch() == llvm::Triple::arm ||
diff --git a/lldb/test/API/macosx/arm64e-attach/Makefile
b/lldb/test/API/macosx/arm64e-attach/Makefile
new file mode 100644
index 00..c9319d6e6888a4
--- /dev/null
+++ b/lldb/test/API/macosx/arm64e-attach/Makefile
@@ -0,0 +1,2 @@
+C_SOURCES := main.c
+include Makefile.rules
diff --git a/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
new file mode 100644
index 00..90daa54baa9d36
--- /dev/nu
[Lldb-commits] [lldb] Replace ArchSpec::PiecewiseCompare() with Triple::operator==() (PR #82804)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Adrian Prantl (adrian-prantl)
Changes
Looking ast the definition of both functions this is *almost* an NFC change,
except that Triple also looks at the SubArch (important) and ObjectFormat (less
so).
This fixes a bug that only manifests with how Xcode uses the SBAPI to attach to
a process by name: it guesses the architecture based on the system. If the
system is arm64 and the Process is arm64e Target fails to update the triple
because it deemed the two to be equivalent.
rdar://123338218
---
Full diff: https://github.com/llvm/llvm-project/pull/82804.diff
6 Files Affected:
- (modified) lldb/include/lldb/Utility/ArchSpec.h (-5)
- (modified) lldb/source/Target/Target.cpp (+1-7)
- (modified) lldb/source/Utility/ArchSpec.cpp (-17)
- (added) lldb/test/API/macosx/arm64e-attach/Makefile (+2)
- (added) lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py (+25)
- (added) lldb/test/API/macosx/arm64e-attach/main.c (+2)
``diff
diff --git a/lldb/include/lldb/Utility/ArchSpec.h
b/lldb/include/lldb/Utility/ArchSpec.h
index a226a3a5a9b71d..50830b889b9115 100644
--- a/lldb/include/lldb/Utility/ArchSpec.h
+++ b/lldb/include/lldb/Utility/ArchSpec.h
@@ -505,11 +505,6 @@ class ArchSpec {
bool IsFullySpecifiedTriple() const;
- void PiecewiseTripleCompare(const ArchSpec &other, bool &arch_different,
- bool &vendor_different, bool &os_different,
- bool &os_version_different,
- bool &env_different) const;
-
/// Detect whether this architecture uses thumb code exclusively
///
/// Some embedded ARM chips (e.g. the ARM Cortex M0-7 line) can only execute
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index e17bfcb5d5e2ad..e982a30a3ae4ff 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1568,14 +1568,8 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec,
bool set_platform,
if (m_arch.GetSpec().IsCompatibleMatch(other)) {
compatible_local_arch = true;
-bool arch_changed, vendor_changed, os_changed, os_ver_changed,
-env_changed;
-m_arch.GetSpec().PiecewiseTripleCompare(other, arch_changed,
-vendor_changed, os_changed,
-os_ver_changed, env_changed);
-
-if (!arch_changed && !vendor_changed && !os_changed && !env_changed)
+if (m_arch.GetSpec().GetTriple() == other.GetTriple())
replace_local_arch = false;
}
}
diff --git a/lldb/source/Utility/ArchSpec.cpp b/lldb/source/Utility/ArchSpec.cpp
index fb0e985a0d5657..07ef435ef451d2 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -1421,23 +1421,6 @@ bool ArchSpec::IsFullySpecifiedTriple() const {
return true;
}
-void ArchSpec::PiecewiseTripleCompare(
-const ArchSpec &other, bool &arch_different, bool &vendor_different,
-bool &os_different, bool &os_version_different, bool &env_different) const
{
- const llvm::Triple &me(GetTriple());
- const llvm::Triple &them(other.GetTriple());
-
- arch_different = (me.getArch() != them.getArch());
-
- vendor_different = (me.getVendor() != them.getVendor());
-
- os_different = (me.getOS() != them.getOS());
-
- os_version_different = (me.getOSMajorVersion() != them.getOSMajorVersion());
-
- env_different = (me.getEnvironment() != them.getEnvironment());
-}
-
bool ArchSpec::IsAlwaysThumbInstructions() const {
std::string Status;
if (GetTriple().getArch() == llvm::Triple::arm ||
diff --git a/lldb/test/API/macosx/arm64e-attach/Makefile
b/lldb/test/API/macosx/arm64e-attach/Makefile
new file mode 100644
index 00..c9319d6e6888a4
--- /dev/null
+++ b/lldb/test/API/macosx/arm64e-attach/Makefile
@@ -0,0 +1,2 @@
+C_SOURCES := main.c
+include Makefile.rules
diff --git a/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
new file mode 100644
index 00..90daa54baa9d36
--- /dev/null
+++ b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
@@ -0,0 +1,25 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestArm64eAttach(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+# On Darwin systems, arch arm64e means ARMv8.3 with ptrauth ABI used.
+@skipIf(archs=no_match(["arm64e"]))
+def test(self):
+self.build()
+popen = self.spawnSubprocess(self.getBuildArtifact(), [])
+error = lldb.SBError()
+# This simulates how Xcode attaches to a process by pid/name.
+target = self.dbg.CreateTarget("", "arm64", "", True, error)
+listener = lldb.SBListener("my.attach.listener")
+process = target.AttachToProcessWithID(listener, popen.pid, error)
+se
[Lldb-commits] [lldb] Replace ArchSpec::PiecewiseCompare() with Triple::operator==() (PR #82804)
github-actions[bot] wrote:
:warning: Python code formatter, darker found issues in your code. :warning:
You can test this locally with the following command:
``bash
darker --check --diff -r
750981f1a2c6069cded709b75cc87d7abd05277a...f1541edda94503adfaa52ef75e1fb53fce5f608e
lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
``
View the diff from darker here.
``diff
--- TestArm64eAttach.py 2024-02-23 18:03:17.00 +
+++ TestArm64eAttach.py 2024-02-23 18:06:51.765685 +
@@ -17,9 +17,12 @@
target = self.dbg.CreateTarget("", "arm64", "", True, error)
listener = lldb.SBListener("my.attach.listener")
process = target.AttachToProcessWithID(listener, popen.pid, error)
self.assertSuccess(error)
self.assertTrue(process, PROCESS_IS_VALID)
-self.assertEqual(target.GetTriple().split('-')[0], "arm64e",
- "target triple is updated correctly")
+self.assertEqual(
+target.GetTriple().split("-")[0],
+"arm64e",
+"target triple is updated correctly",
+)
error = process.Kill()
self.assertSuccess(error)
``
https://github.com/llvm/llvm-project/pull/82804
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Replace ArchSpec::PiecewiseCompare() with Triple::operator==() (PR #82804)
@@ -0,0 +1,2 @@ +C_SOURCES := main.c +include Makefile.rules JDevlieghere wrote: Did you want to build this test for arm64**e**? https://github.com/llvm/llvm-project/pull/82804 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Replace ArchSpec::PiecewiseCompare() with Triple::operator==() (PR #82804)
@@ -0,0 +1,25 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestArm64eAttach(TestBase): +NO_DEBUG_INFO_TESTCASE = True + +# On Darwin systems, arch arm64e means ARMv8.3 with ptrauth ABI used. +@skipIf(archs=no_match(["arm64e"])) +def test(self): +self.build() +popen = self.spawnSubprocess(self.getBuildArtifact(), []) JDevlieghere wrote: To run arm64e code you need to set a `-arm64e_preview_abi` boot arg. I'd be surprised if the bots have that set. You'll probably want to skip this test when this fails to launch. https://github.com/llvm/llvm-project/pull/82804 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Print a message when background tasks take a while to complete (PR #82799)
https://github.com/clayborg commented: Can we not just use a `Progress` object instead of installing a callback? Seems like a very specific use case to install a callback for in our public API. Do we create `Progress` objects for each background download? If we don't, should we? Then this would show up in the command line like "Downloading symbols for ..." and it might communicate to the user what is happening better than "Waiting for background tasks to complete...". What would happen if we could add an internal API that would cancel all background downloads of symbols? I am not sure I like installing a specific callback for thread pool issues. What if a thread gets deadlocked waiting for data and it never completes the download? https://github.com/llvm/llvm-project/pull/82799 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Print a message when background tasks take a while to complete (PR #82799)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/82799 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Print a message when background tasks take a while to complete (PR #82799)
@@ -623,8 +630,20 @@ void Debugger::Terminate() {
}
if (g_thread_pool) {
-// The destructor will wait for all the threads to complete.
-delete g_thread_pool;
+// Delete the thread pool in a different thread so we can set a timeout.
clayborg wrote:
Can we create a Progress object here? Maybe something like:
```
Progress progress("Shutting down worker threads");
```
Or is it too late and no one would receive events?
Another option would be to have the thread pool detach from all threads so that
we don't need to wait until they complete and just go ahead and exit?
https://github.com/llvm/llvm-project/pull/82799
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Print a message when background tasks take a while to complete (PR #82799)
@@ -623,8 +630,20 @@ void Debugger::Terminate() {
}
if (g_thread_pool) {
-// The destructor will wait for all the threads to complete.
-delete g_thread_pool;
+// Delete the thread pool in a different thread so we can set a timeout.
JDevlieghere wrote:
> Or is it too late and no one would receive events?
Yup, that's exactly the problem. The debugger has already been destroyed at
this point, otherwise I could've written to the debugger's output stream. By
the time we terminate, there's pretty much nothing left at this point. Before
I had the callback I just wrote to stderr directly, but that's not a very nice
thing to do as a library.
> Another option would be to have the thread pool detach from all threads so
> that we don't need to wait until they complete and just go ahead and exit?
That's what we did in the past and that caused a bunch of crashes. If you want
to do that without crashing, you need to be very careful about what you do in a
task running on the thread pool, because the debugger might have been destroyed
and terminated while you were running.
https://github.com/llvm/llvm-project/pull/82799
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Print a message when background tasks take a while to complete (PR #82799)
JDevlieghere wrote: > Can we not just use a `Progress` object instead of installing a callback? > Seems like a very specific use case to install a callback for in our public > API. Do we create `Progress` objects for each background download? If we > don't, should we? Then this would show up in the command line like > "Downloading symbols for ..." and it might communicate to the user what is > happening better than "Waiting for background tasks to complete...". We do emit progress events for all the downloads, but there's two reasons this doesn't work as you expect: 1. Currently, the default event handler ignores events that are shadowed. So if I kick off 4 downloads at the same time, we'll show the progress event for the first one only. The reason for this how we use ANSI escape codes to redraw the current line. I actually have something in the works to change that, but even if I can fix that, there's a second issue: 2. The thread pool is global and by the time we terminate, the debugger has already been destroyed, which includes it event handler thread and even its output stream. > What would happen if we could add an internal API that would cancel all > background downloads of symbols? That's what I described at the bottom of the description. You could come up with an ad-hoc solution for symbol downloads, but if we want to lean on the thread pool more, we'll see the same issue in other places too. If we want to go that route I think we should do it at the ThreadPool level and make all tasks (cooperatively) interruptible. > I am not sure I like installing a specific callback for thread pool issues. > What if a thread gets deadlocked waiting for data and it never completes the > download? That's already a risk. The [ThreadPool](https://llvm.org/doxygen/classllvm_1_1ThreadPool.html) explicitly provides no guarantees around deadlocking. https://github.com/llvm/llvm-project/pull/82799 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Replace ArchSpec::PiecewiseCompare() with Triple::operator==() (PR #82804)
@@ -0,0 +1,25 @@ +import lldb +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil + + +class TestArm64eAttach(TestBase): +NO_DEBUG_INFO_TESTCASE = True + +# On Darwin systems, arch arm64e means ARMv8.3 with ptrauth ABI used. +@skipIf(archs=no_match(["arm64e"])) +def test(self): +self.build() +popen = self.spawnSubprocess(self.getBuildArtifact(), []) adrian-prantl wrote: The bots will already fail at the `archs=no_match(["arm64e"]` decorator. https://github.com/llvm/llvm-project/pull/82804 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Replace ArchSpec::PiecewiseCompare() with Triple::operator==() (PR #82804)
@@ -0,0 +1,2 @@ +C_SOURCES := main.c +include Makefile.rules adrian-prantl wrote: That is implicit. If you are running the tests with an arm64e ARCH, the test will run (with that triple). https://github.com/llvm/llvm-project/pull/82804 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][X86] Fix setting target features in ClangExpressionParser (PR #82364)
adrian-prantl wrote: Would this change be observable by a test? https://github.com/llvm/llvm-project/pull/82364 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Print a message when background tasks take a while to complete (PR #82799)
JDevlieghere wrote: Greg and I talked this over offline. We both feel that progress events are the best way to convey that work is going on in the background. To make that work for the driver, we'll check if the debugger being destroyed is the very last one and if it is, we'll delay its destruction until the thread pool as completed. That still leaves the first problem of not shadowing progress events, but that can be deal with separately. https://github.com/llvm/llvm-project/pull/82799 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Replace ArchSpec::PiecewiseCompare() with Triple::operator==() (PR #82804)
https://github.com/adrian-prantl updated
https://github.com/llvm/llvm-project/pull/82804
>From d8e201683c460af42b671ecb433ba84620ada1c2 Mon Sep 17 00:00:00 2001
From: Adrian Prantl
Date: Fri, 23 Feb 2024 09:58:17 -0800
Subject: [PATCH] Replace ArchSpec::PiecewiseCompare() with
Triple::operator==()
Looking ast the definition of both functions this is *almost* an NFC
change, except that Triple also looks at the SubArch (important) and
ObjectFormat (less so).
This fixes a bug that only manifests with how Xcode uses the SBAPI to
attach to a process by name: it guesses the architecture based on the
system. If the system is arm64 and the Process is arm64e Target fails
to update the triple because it deemed the two to be equivalent.
rdar://123338218
---
lldb/include/lldb/Utility/ArchSpec.h | 5
lldb/source/Target/Target.cpp | 8 +-
lldb/source/Utility/ArchSpec.cpp | 17 ---
lldb/test/API/macosx/arm64e-attach/Makefile | 2 ++
.../macosx/arm64e-attach/TestArm64eAttach.py | 28 +++
lldb/test/API/macosx/arm64e-attach/main.c | 2 ++
6 files changed, 33 insertions(+), 29 deletions(-)
create mode 100644 lldb/test/API/macosx/arm64e-attach/Makefile
create mode 100644 lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
create mode 100644 lldb/test/API/macosx/arm64e-attach/main.c
diff --git a/lldb/include/lldb/Utility/ArchSpec.h
b/lldb/include/lldb/Utility/ArchSpec.h
index a226a3a5a9b71d..50830b889b9115 100644
--- a/lldb/include/lldb/Utility/ArchSpec.h
+++ b/lldb/include/lldb/Utility/ArchSpec.h
@@ -505,11 +505,6 @@ class ArchSpec {
bool IsFullySpecifiedTriple() const;
- void PiecewiseTripleCompare(const ArchSpec &other, bool &arch_different,
- bool &vendor_different, bool &os_different,
- bool &os_version_different,
- bool &env_different) const;
-
/// Detect whether this architecture uses thumb code exclusively
///
/// Some embedded ARM chips (e.g. the ARM Cortex M0-7 line) can only execute
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index e17bfcb5d5e2ad..e982a30a3ae4ff 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1568,14 +1568,8 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec,
bool set_platform,
if (m_arch.GetSpec().IsCompatibleMatch(other)) {
compatible_local_arch = true;
-bool arch_changed, vendor_changed, os_changed, os_ver_changed,
-env_changed;
-m_arch.GetSpec().PiecewiseTripleCompare(other, arch_changed,
-vendor_changed, os_changed,
-os_ver_changed, env_changed);
-
-if (!arch_changed && !vendor_changed && !os_changed && !env_changed)
+if (m_arch.GetSpec().GetTriple() == other.GetTriple())
replace_local_arch = false;
}
}
diff --git a/lldb/source/Utility/ArchSpec.cpp b/lldb/source/Utility/ArchSpec.cpp
index fb0e985a0d5657..07ef435ef451d2 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -1421,23 +1421,6 @@ bool ArchSpec::IsFullySpecifiedTriple() const {
return true;
}
-void ArchSpec::PiecewiseTripleCompare(
-const ArchSpec &other, bool &arch_different, bool &vendor_different,
-bool &os_different, bool &os_version_different, bool &env_different) const
{
- const llvm::Triple &me(GetTriple());
- const llvm::Triple &them(other.GetTriple());
-
- arch_different = (me.getArch() != them.getArch());
-
- vendor_different = (me.getVendor() != them.getVendor());
-
- os_different = (me.getOS() != them.getOS());
-
- os_version_different = (me.getOSMajorVersion() != them.getOSMajorVersion());
-
- env_different = (me.getEnvironment() != them.getEnvironment());
-}
-
bool ArchSpec::IsAlwaysThumbInstructions() const {
std::string Status;
if (GetTriple().getArch() == llvm::Triple::arm ||
diff --git a/lldb/test/API/macosx/arm64e-attach/Makefile
b/lldb/test/API/macosx/arm64e-attach/Makefile
new file mode 100644
index 00..c9319d6e6888a4
--- /dev/null
+++ b/lldb/test/API/macosx/arm64e-attach/Makefile
@@ -0,0 +1,2 @@
+C_SOURCES := main.c
+include Makefile.rules
diff --git a/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
new file mode 100644
index 00..0dc8700ed02dd8
--- /dev/null
+++ b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
@@ -0,0 +1,28 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestArm64eAttach(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+# On Darwin systems, arch arm64e means ARMv8.3 with ptrauth ABI used.
+@skipIf(archs=no_match(["arm64e"]))
+def test(self):
+# Skip this test if not running
[Lldb-commits] [lldb] Replace ArchSpec::PiecewiseCompare() with Triple::operator==() (PR #82804)
https://github.com/JDevlieghere approved this pull request. https://github.com/llvm/llvm-project/pull/82804 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Correctly annotate threads at a bp site as hitting it (PR #82709)
https://github.com/jasonmolenda updated
https://github.com/llvm/llvm-project/pull/82709
>From 171613d26338919e40584656a7f88899ba6cc5ca Mon Sep 17 00:00:00 2001
From: Jason Molenda
Date: Thu, 22 Feb 2024 15:35:31 -0800
Subject: [PATCH 1/2] [lldb] Correctly annotate threads at a bp site as hitting
it
This is next in my series of "fix the racey tests that fail on
greendragon" addressing the failure of TestConcurrentManyBreakpoints.py
where we set a breakpoint in a function that 100 threads execute,
and we check that we hit the breakpoint 100 times. But sometimes
it is only hit 99 times, and the test fails.
When we hit a software breakpoint, the pc value for the thread is
the address of the breakpoint instruction - as if it had not been
hit yet. And because a user might ADD a breakpoint for the current
pc from the commandline, when we go to resume execution, any thread
that is sitting at a breakpoint site will be silently advanced past
the breakpoint instruction (disable bp, instruction step that thread,
re-enable bp) before resuming.
What this test is exposing is that there is another corner case, a
thread that is sitting at a breakpoint site but has not yet executed
the breakpoint instruction. The thread will have no stop reason,
no mach exception, so it will not be recorded as having hit the
breakpoint (because it hasn't yet). But when we resume execution,
because it is sitting at a breakpoint site, we advance past it and
miss the breakpoint hit.
In 2016 Abhishek Aggarwal handled a similar issue with a patch in
`ProcessGDBRemote::SetThreadStopInfo()`, adding a breakpoint StopInfo
for a thread sitting at a breakpoint site that has no stop reason.
debugserver's `jThreadsInfo` would not correctly execute Abhishek's
code though because it would respond with `"reason":"none"` for a
thread with no stop reason, and `SetThreadStopInfo()` expected an
empty reason here. The first part of my patch is to clear the
`reason` if it is `"none"` so we flow through the code correctly.
On Darwin, though, our stop reply packet (Txx...) includes the
`threads`, `thread-pcs`, and `jstopinfo` keys, which give us the
tids for all current threads, the pc values for those threads, and
`jstopinfo` has a JSON dictionary with the mach exceptions for all
threads that have a mach exception. In
`ProcessGDBRemote::CalculateThreadStopInfo()` we set the StopInfo
for each thread for a private stop and if we have `jstopinfo` it
is the source of all the StopInfos. I have to add the same logic
here, to give the thread a breakpoint StopInfo even though it hasn't
executed the breakpoint yet. In this case we are very early in
thread construction and I only have the information in the Txx stop
reply packet -- tids, pcs, and jstopinfo, so I can't use the normal
general mechanisms of going through the RegisterContext to get the
pc, it's a bit different.
If I hack debugserver to not issue `jstopinfo`, `CalculateThreadStopInfo`
will fall back to sending `qThreadStopInfo` for each thread and
going through `ProcessGDBRemote::SetThreadStopInfo()` to set the
stop infos (and with the `reason:none` fix, use Abhishek's code).
rdar://110549165
---
.../Process/gdb-remote/ProcessGDBRemote.cpp | 33 +++
.../Process/gdb-remote/ProcessGDBRemote.h | 2 +-
2 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 629b191f3117aa..eabbc8ad433212 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1600,6 +1600,26 @@ bool
ProcessGDBRemote::CalculateThreadStopInfo(ThreadGDBRemote *thread) {
// has no stop reason.
thread->GetRegisterContext()->InvalidateIfNeeded(true);
if (!GetThreadStopInfoFromJSON(thread, m_jstopinfo_sp)) {
+ // If a thread is stopped at a breakpoint site, set that as the stop
+ // reason even if it hasn't executed the breakpoint instruction yet.
+ // We will silently step over the breakpoint when we resume execution
+ // and miss the fact that this thread hit the breakpoint.
+ const size_t num_thread_ids = m_thread_ids.size();
+ for (size_t i = 0; i < num_thread_ids; i++) {
+if (m_thread_ids[i] == thread->GetID() && m_thread_pcs.size() > i) {
+ addr_t pc = m_thread_pcs[i];
+ lldb::BreakpointSiteSP bp_site_sp =
+ thread->GetProcess()->GetBreakpointSiteList().FindByAddress(pc);
+ if (bp_site_sp) {
+if (bp_site_sp->ValidForThisThread(*thread)) {
+ thread->SetStopInfo(
+ StopInfo::CreateStopReasonWithBreakpointSiteID(
+ *thread, bp_site_sp->GetID()));
+ return true;
+}
+ }
+}
+ }
thread->SetStopInfo(StopInfoSP());
}
return true;
@@ -1630,7 +1650,7 @@ void ProcessGDBRemote::Pars
[Lldb-commits] [lldb] [lldb] Correctly annotate threads at a bp site as hitting it (PR #82709)
@@ -1722,6 +1742,8 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
} else {
bool handled = false;
bool did_exec = false;
+ if (reason == "none")
jasonmolenda wrote:
I updated the patch where I handle this a little more cleanly, checking that
`reason` has a value, and that the value is not `"none"`, and added a short
comment.
https://github.com/llvm/llvm-project/pull/82709
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 2594095 - Replace ArchSpec::PiecewiseCompare() with Triple::operator==() (#82804)
Author: Adrian Prantl
Date: 2024-02-23T14:00:15-08:00
New Revision: 25940956e68ec82d841e5748565e7250580e1d36
URL:
https://github.com/llvm/llvm-project/commit/25940956e68ec82d841e5748565e7250580e1d36
DIFF:
https://github.com/llvm/llvm-project/commit/25940956e68ec82d841e5748565e7250580e1d36.diff
LOG: Replace ArchSpec::PiecewiseCompare() with Triple::operator==() (#82804)
Looking ast the definition of both functions this is *almost* an NFC
change, except that Triple also looks at the SubArch (important) and
ObjectFormat (less so).
This fixes a bug that only manifests with how Xcode uses the SBAPI to
attach to a process by name: it guesses the architecture based on the
system. If the system is arm64 and the Process is arm64e Target fails to
update the triple because it deemed the two to be equivalent.
rdar://123338218
Added:
lldb/test/API/macosx/arm64e-attach/Makefile
lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
lldb/test/API/macosx/arm64e-attach/main.c
Modified:
lldb/include/lldb/Utility/ArchSpec.h
lldb/source/Target/Target.cpp
lldb/source/Utility/ArchSpec.cpp
Removed:
diff --git a/lldb/include/lldb/Utility/ArchSpec.h
b/lldb/include/lldb/Utility/ArchSpec.h
index a226a3a5a9b71d..50830b889b9115 100644
--- a/lldb/include/lldb/Utility/ArchSpec.h
+++ b/lldb/include/lldb/Utility/ArchSpec.h
@@ -505,11 +505,6 @@ class ArchSpec {
bool IsFullySpecifiedTriple() const;
- void PiecewiseTripleCompare(const ArchSpec &other, bool &arch_
diff erent,
- bool &vendor_
diff erent, bool &os_
diff erent,
- bool &os_version_
diff erent,
- bool &env_
diff erent) const;
-
/// Detect whether this architecture uses thumb code exclusively
///
/// Some embedded ARM chips (e.g. the ARM Cortex M0-7 line) can only execute
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index e17bfcb5d5e2ad..e982a30a3ae4ff 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1568,14 +1568,8 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec,
bool set_platform,
if (m_arch.GetSpec().IsCompatibleMatch(other)) {
compatible_local_arch = true;
-bool arch_changed, vendor_changed, os_changed, os_ver_changed,
-env_changed;
-m_arch.GetSpec().PiecewiseTripleCompare(other, arch_changed,
-vendor_changed, os_changed,
-os_ver_changed, env_changed);
-
-if (!arch_changed && !vendor_changed && !os_changed && !env_changed)
+if (m_arch.GetSpec().GetTriple() == other.GetTriple())
replace_local_arch = false;
}
}
diff --git a/lldb/source/Utility/ArchSpec.cpp
b/lldb/source/Utility/ArchSpec.cpp
index fb0e985a0d5657..07ef435ef451d2 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -1421,23 +1421,6 @@ bool ArchSpec::IsFullySpecifiedTriple() const {
return true;
}
-void ArchSpec::PiecewiseTripleCompare(
-const ArchSpec &other, bool &arch_
diff erent, bool &vendor_
diff erent,
-bool &os_
diff erent, bool &os_version_
diff erent, bool &env_
diff erent) const {
- const llvm::Triple &me(GetTriple());
- const llvm::Triple &them(other.GetTriple());
-
- arch_
diff erent = (me.getArch() != them.getArch());
-
- vendor_
diff erent = (me.getVendor() != them.getVendor());
-
- os_
diff erent = (me.getOS() != them.getOS());
-
- os_version_
diff erent = (me.getOSMajorVersion() != them.getOSMajorVersion());
-
- env_
diff erent = (me.getEnvironment() != them.getEnvironment());
-}
-
bool ArchSpec::IsAlwaysThumbInstructions() const {
std::string Status;
if (GetTriple().getArch() == llvm::Triple::arm ||
diff --git a/lldb/test/API/macosx/arm64e-attach/Makefile
b/lldb/test/API/macosx/arm64e-attach/Makefile
new file mode 100644
index 00..c9319d6e6888a4
--- /dev/null
+++ b/lldb/test/API/macosx/arm64e-attach/Makefile
@@ -0,0 +1,2 @@
+C_SOURCES := main.c
+include Makefile.rules
diff --git a/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
new file mode 100644
index 00..0dc8700ed02dd8
--- /dev/null
+++ b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
@@ -0,0 +1,28 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestArm64eAttach(TestBase):
+NO_DEBUG_INFO_TESTCASE = True
+
+# On Darwin systems, arch arm64e means ARMv8.3 with ptrauth ABI used.
+@skipIf(archs=no_match(["arm64e"]))
+def test(self):
+# Skip this test if not running on AArch64 target that supports PAC
+if not self.isAArch64PAuth():
+self.skipTest("Target must support
[Lldb-commits] [lldb] Replace ArchSpec::PiecewiseCompare() with Triple::operator==() (PR #82804)
https://github.com/adrian-prantl closed https://github.com/llvm/llvm-project/pull/82804 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Correctly annotate threads at a bp site as hitting it (PR #82709)
jasonmolenda wrote: > This seems safe to me, and it can't hurt to take care of this corner case > here regardless of what the higher levels of lldb does. This all falls under `Thread::CalculateStopInfo()` - `ThreadGDBRemote::CalculateStopInfo` is calling into ProcessGDBRemote where we do this. The base class method is pure virtual, so I'm not sure there's a higher level place where this logic could be situated. I updated the PR with a comment on `Thread::CalculateStopInfo()` specifically detailing this issue, so people writing new subclasses can (hopefully) be aware of it. https://github.com/llvm/llvm-project/pull/82709 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [libclc] [libcxx] [lld] [lldb] [llvm] [NFC] Remove trailing whitespace across all non-test related files (PR #82838)
llvmbot wrote: @llvm/pr-subscribers-backend-arm @llvm/pr-subscribers-clang Author: AtariDreams (AtariDreams) Changes Every once in a while, some formatting scanner will cause the CI to stop because of whitespace issues, so it is is best to just remove them in one go, especially since it seems only non-test files tend to be under scrutiny by automatic formatting checkers. --- Patch is 2.04 MiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/82838.diff 599 Files Affected: - (modified) .github/workflows/scorecard.yml (+2-2) - (modified) clang-tools-extra/clang-tidy/abseil/DurationDivisionCheck.h (+1-1) - (modified) clang-tools-extra/clang-tidy/abseil/RedundantStrcatCallsCheck.cpp (+10-10) - (modified) clang-tools-extra/clang-tidy/abseil/RedundantStrcatCallsCheck.h (+2-2) - (modified) clang-tools-extra/clang-tidy/abseil/StrCatAppendCheck.cpp (+2-2) - (modified) clang-tools-extra/clang-tidy/abseil/StrCatAppendCheck.h (+1-1) - (modified) clang-tools-extra/clang-tidy/bugprone/SpuriouslyWakeUpFunctionsCheck.h (+3-3) - (modified) clang-tools-extra/clang-tidy/fuchsia/StaticallyConstructedObjectsCheck.h (+1-1) - (modified) clang-tools-extra/clang-tidy/fuchsia/TrailingReturnCheck.h (+2-2) - (modified) clang-tools-extra/clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp (+1-1) - (modified) clang-tools-extra/clang-tidy/misc/ConfusableTable/confusables.txt (+4701-4701) - (modified) clang-tools-extra/clangd/ClangdLSPServer.h (+1-1) - (modified) clang-tools-extra/clangd/CodeComplete.cpp (+2-2) - (modified) clang-tools-extra/clangd/IncludeCleaner.cpp (+3-3) - (modified) clang-tools-extra/clangd/Protocol.h (+2-2) - (modified) clang-tools-extra/clangd/quality/CompletionModel.cmake (+2-2) - (modified) clang-tools-extra/clangd/quality/README.md (+8-8) - (modified) clang-tools-extra/clangd/refactor/tweaks/PopulateSwitch.cpp (+1-1) - (modified) clang-tools-extra/clangd/test/Inputs/symbols.test.yaml (+4-4) - (modified) clang-tools-extra/clangd/tool/Check.cpp (+1-1) - (modified) clang-tools-extra/docs/clang-tidy/checks/bugprone/assignment-in-if-condition.rst (+1-1) - (modified) clang-tools-extra/docs/clang-tidy/checks/bugprone/signal-handler.rst (+1-1) - (modified) clang-tools-extra/docs/clang-tidy/checks/bugprone/suspicious-realloc-usage.rst (+1-1) - (modified) clang-tools-extra/docs/clang-tidy/checks/misc/include-cleaner.rst (+2-2) - (modified) clang-tools-extra/docs/clang-tidy/checks/modernize/type-traits.rst (+1-1) - (modified) clang-tools-extra/docs/clang-tidy/checks/objc/nsdate-formatter.rst (+26-26) - (modified) clang-tools-extra/docs/clang-tidy/checks/readability/redundant-inline-specifier.rst (+1-1) - (modified) clang-tools-extra/pseudo/Disambiguation.md (+3-3) - (modified) clang-tools-extra/pseudo/gen/Main.cpp (+1-1) - (modified) clang/docs/HLSL/ExpectedDifferences.rst (+2-2) - (modified) clang/examples/PrintFunctionNames/PrintFunctionNames.cpp (+1-1) - (modified) clang/tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs (+4-3) - (modified) clang/tools/clang-format-vs/ClangFormat/RunningDocTableEventsDispatcher.cs (+1-1) - (modified) clang/tools/clang-fuzzer/CMakeLists.txt (+1-1) - (modified) clang/tools/clang-fuzzer/fuzzer-initialize/fuzzer_initialize.cpp (+1-1) - (modified) clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp (+2-2) - (modified) clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td (+2-2) - (modified) clang/tools/clang-offload-packager/CMakeLists.txt (+1-1) - (modified) clang/tools/diagtool/DiagTool.cpp (+3-3) - (modified) clang/tools/diagtool/DiagTool.h (+7-7) - (modified) clang/tools/diagtool/diagtool_main.cpp (+1-1) - (modified) clang/tools/libclang/ARCMigrate.cpp (-1) - (modified) clang/tools/libclang/CIndexCXX.cpp (+13-13) - (modified) clang/tools/libclang/CIndexCodeCompletion.cpp (+42-48) - (modified) clang/tools/libclang/CIndexDiagnostic.cpp (+22-22) - (modified) clang/tools/libclang/CIndexDiagnostic.h (+13-13) - (modified) clang/tools/libclang/CIndexHigh.cpp (+4-4) - (modified) clang/tools/libclang/CIndexer.h (+1-1) - (modified) clang/tools/libclang/CXIndexDataConsumer.cpp (+16-16) - (modified) clang/tools/libclang/CXIndexDataConsumer.h (+4-4) - (modified) clang/tools/libclang/CXLoadedDiagnostic.cpp (+10-11) - (modified) clang/tools/libclang/CXLoadedDiagnostic.h (+3-3) - (modified) clang/tools/libclang/CXSourceLocation.cpp (+19-20) - (modified) clang/tools/libclang/CXSourceLocation.h (+4-4) - (modified) clang/tools/libclang/CXStoredDiagnostic.cpp (+6-7) - (modified) clang/tools/libclang/CXTranslationUnit.h (+1-1) - (modified) clang/tools/libclang/CXType.cpp (+10-10) - (modified) clang/tools/libclang/CXType.h (+1-2) - (modified) clang/tools/libclang/Index_Internal.h (+1-1) - (modified) clang/tools/libclang/Indexing.cpp (+7-7) - (modified) clang/tools/scan-build-py/README.md (+6-6) - (modified) clang/tools/scan-build/bin/scan-build (+1-1) - (modified) clang/too
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [libclc] [libcxx] [lld] [lldb] [llvm] [NFC] Remove trailing whitespace across all non-test related files (PR #82838)
https://github.com/AtariDreams closed https://github.com/llvm/llvm-project/pull/82838 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [libclc] [libcxx] [lld] [lldb] [llvm] [NFC] Remove trailing whitespace across all non-test related files (PR #82838)
AtariDreams wrote: I'm just gonna split this up. Sorry. https://github.com/llvm/llvm-project/pull/82838 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [libclc] [libcxx] [lld] [lldb] [llvm] [NFC] Remove trailing whitespace across all non-test related files (PR #82838)
github-actions[bot] wrote: ⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo. Please turn off [Keep my email addresses private](https://github.com/settings/emails) setting in your account. See [LLVM Discourse](https://discourse.llvm.org/t/hidden-emails-on-github-should-we-do-something-about-it) for more information. https://github.com/llvm/llvm-project/pull/82838 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 87fadb3 - [lldb] Correctly annotate threads at a bp site as hitting it (#82709)
Author: Jason Molenda Date: 2024-02-23T14:45:22-08:00 New Revision: 87fadb3929163752f650a1fc08d5fb13ee4c1a3f URL: https://github.com/llvm/llvm-project/commit/87fadb3929163752f650a1fc08d5fb13ee4c1a3f DIFF: https://github.com/llvm/llvm-project/commit/87fadb3929163752f650a1fc08d5fb13ee4c1a3f.diff LOG: [lldb] Correctly annotate threads at a bp site as hitting it (#82709) This is next in my series of "fix the racey tests that fail on greendragon" addressing the failure of TestConcurrentManyBreakpoints.py where we set a breakpoint in a function that 100 threads execute, and we check that we hit the breakpoint 100 times. But sometimes it is only hit 99 times, and the test fails. When we hit a software breakpoint, the pc value for the thread is the address of the breakpoint instruction - as if it had not been hit yet. And because a user might ADD a breakpoint for the current pc from the commandline, when we go to resume execution, any thread that is sitting at a breakpoint site will be silently advanced past the breakpoint instruction (disable bp, instruction step that thread, re-enable bp) before resuming -- whether that thread has hit its breakpoint or not. What this test is exposing is that there is another corner case, a thread that is sitting at a breakpoint site but has not yet executed the breakpoint instruction. The thread will have no stop reason, no mach exception, so it will not be recorded as having hit the breakpoint (because it hasn't yet). But when we resume execution, because it is sitting at a breakpoint site, we advance past it and miss the breakpoint hit. In 2016 Abhishek Aggarwal handled a similar issue with a patch in `ProcessGDBRemote::SetThreadStopInfo()`, adding a breakpoint StopInfo for a thread sitting at a breakpoint site that has no stop reason. debugserver's `jThreadsInfo` would not correctly execute Abhishek's code though because it would respond with `"reason":"none"` for a thread with no stop reason, and `SetThreadStopInfo()` expected an empty reason here. The first part of my patch is to clear the `reason` if it is `"none"` so we flow through the code correctly. On Darwin, though, our stop reply packet (Txx...) includes the `threads`, `thread-pcs`, and `jstopinfo` keys, which give us the tids for all current threads, the pc values for those threads, and `jstopinfo` has a JSON dictionary with the mach exceptions for all threads that have a mach exception. In `ProcessGDBRemote::CalculateThreadStopInfo()` we set the StopInfo for each thread for a private stop and if we have `jstopinfo` it is the source of all the StopInfos. I have to add the same logic here, to give the thread a breakpoint StopInfo even though it hasn't executed the breakpoint yet. In this case we are very early in thread construction and I only have the information in the Txx stop reply packet -- tids, pcs, and jstopinfo, so I can't use the normal general mechanisms of going through the RegisterContext to get the pc, it's a bit different. If I hack debugserver to not issue `jstopinfo`, `CalculateThreadStopInfo` will fall back to sending `qThreadStopInfo` for each thread and going through `ProcessGDBRemote::SetThreadStopInfo()` to set the stop infos (and with the `reason:none` fix, use Abhishek's code). rdar://110549165 Added: Modified: lldb/include/lldb/Target/Thread.h lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp Removed: diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h index 96ca95ad233ff7..1efef93b17ded8 100644 --- a/lldb/include/lldb/Target/Thread.h +++ b/lldb/include/lldb/Target/Thread.h @@ -1163,13 +1163,20 @@ class Thread : public std::enable_shared_from_this, void CalculatePublicStopInfo(); - // Ask the thread subclass to set its stop info. - // - // Thread subclasses should call Thread::SetStopInfo(...) with the reason the - // thread stopped. - // - // \return - // True if Thread::SetStopInfo(...) was called, false otherwise. + /// Ask the thread subclass to set its stop info. + /// + /// Thread subclasses should call Thread::SetStopInfo(...) with the reason the + /// thread stopped. + /// + /// A thread that is sitting at a breakpoint site, but has not yet executed + /// the breakpoint instruction, should have a breakpoint-hit StopInfo set. + /// When execution is resumed, any thread sitting at a breakpoint site will + /// instruction-step over the breakpoint instruction silently, and we will + /// never record this breakpoint as being hit, updating the hit count, + /// possibly executing breakpoint commands or conditions. + /// + /// \return + /// True if Thread::SetStopInfo(...) was called, false otherwise. virtual bool CalculateStopInfo() = 0; // Gets the temporary resume state for a thread. diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Pr
[Lldb-commits] [lldb] [lldb] Correctly annotate threads at a bp site as hitting it (PR #82709)
https://github.com/jasonmolenda closed https://github.com/llvm/llvm-project/pull/82709 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [libclc] [libcxx] [lld] [lldb] [llvm] [NFC] Remove trailing whitespace across all non-test related files (PR #82838)
joker-eph wrote: Fine with me if we want to land this as is, but per-top-level subproject may be a better granularity. https://github.com/llvm/llvm-project/pull/82838 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)
@@ -3944,15 +3955,22 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
// The order of these checks is important.
if (process_does_not_exist (pid_attaching_to)) {
DNBLogError("Tried to attach to pid that doesn't exist");
- std::string return_message = "E96;";
- return_message += cstring_to_asciihex_string("no such process.");
+ std::string return_message = "E96";
jasonmolenda wrote:
I'm thinking of having a SendErrorPacket() instead of SendPacket() with an
error string optional argument, and having all error packets route through
that, and having all SendPacket("Exx")'s go through that instead. @bulbazord
@clayborg what do you think? I'm having trouble deciding if I want to have a
method that constructs the error string and the caller passes it to SendPacket
or if I want to have a method to do both.
https://github.com/llvm/llvm-project/pull/82593
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [libclc] [libcxx] [lld] [lldb] [llvm] [NFC] Remove trailing whitespace across all non-test related files (PR #82838)
felipepiovezan wrote: Juuust to make sure, was this discussed before? This seems akin to running clang-format on the entire project, which last time we talked about still faced opposition: https://discourse.llvm.org/t/rfc-clang-format-all-the-things/76614/11 https://github.com/llvm/llvm-project/pull/82838 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 3f91bdf - Revert "Replace ArchSpec::PiecewiseCompare() with Triple::operator==()"
Author: Adrian Prantl
Date: 2024-02-23T15:26:14-08:00
New Revision: 3f91bdfdd50aa4eaf1d3e49cf797220cfeccaf16
URL:
https://github.com/llvm/llvm-project/commit/3f91bdfdd50aa4eaf1d3e49cf797220cfeccaf16
DIFF:
https://github.com/llvm/llvm-project/commit/3f91bdfdd50aa4eaf1d3e49cf797220cfeccaf16.diff
LOG: Revert "Replace ArchSpec::PiecewiseCompare() with Triple::operator==()"
This reverts commit 5e6bed8c0ea2f7fe380127763c8f753adae0fc1b while
investigating the bots.
Added:
Modified:
lldb/include/lldb/Utility/ArchSpec.h
lldb/source/Target/Target.cpp
lldb/source/Utility/ArchSpec.cpp
Removed:
lldb/test/API/macosx/arm64e-attach/Makefile
lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
lldb/test/API/macosx/arm64e-attach/main.c
diff --git a/lldb/include/lldb/Utility/ArchSpec.h
b/lldb/include/lldb/Utility/ArchSpec.h
index 50830b889b9115..a226a3a5a9b71d 100644
--- a/lldb/include/lldb/Utility/ArchSpec.h
+++ b/lldb/include/lldb/Utility/ArchSpec.h
@@ -505,6 +505,11 @@ class ArchSpec {
bool IsFullySpecifiedTriple() const;
+ void PiecewiseTripleCompare(const ArchSpec &other, bool &arch_
diff erent,
+ bool &vendor_
diff erent, bool &os_
diff erent,
+ bool &os_version_
diff erent,
+ bool &env_
diff erent) const;
+
/// Detect whether this architecture uses thumb code exclusively
///
/// Some embedded ARM chips (e.g. the ARM Cortex M0-7 line) can only execute
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index e982a30a3ae4ff..e17bfcb5d5e2ad 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1568,8 +1568,14 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec,
bool set_platform,
if (m_arch.GetSpec().IsCompatibleMatch(other)) {
compatible_local_arch = true;
+bool arch_changed, vendor_changed, os_changed, os_ver_changed,
+env_changed;
-if (m_arch.GetSpec().GetTriple() == other.GetTriple())
+m_arch.GetSpec().PiecewiseTripleCompare(other, arch_changed,
+vendor_changed, os_changed,
+os_ver_changed, env_changed);
+
+if (!arch_changed && !vendor_changed && !os_changed && !env_changed)
replace_local_arch = false;
}
}
diff --git a/lldb/source/Utility/ArchSpec.cpp
b/lldb/source/Utility/ArchSpec.cpp
index 07ef435ef451d2..fb0e985a0d5657 100644
--- a/lldb/source/Utility/ArchSpec.cpp
+++ b/lldb/source/Utility/ArchSpec.cpp
@@ -1421,6 +1421,23 @@ bool ArchSpec::IsFullySpecifiedTriple() const {
return true;
}
+void ArchSpec::PiecewiseTripleCompare(
+const ArchSpec &other, bool &arch_
diff erent, bool &vendor_
diff erent,
+bool &os_
diff erent, bool &os_version_
diff erent, bool &env_
diff erent) const {
+ const llvm::Triple &me(GetTriple());
+ const llvm::Triple &them(other.GetTriple());
+
+ arch_
diff erent = (me.getArch() != them.getArch());
+
+ vendor_
diff erent = (me.getVendor() != them.getVendor());
+
+ os_
diff erent = (me.getOS() != them.getOS());
+
+ os_version_
diff erent = (me.getOSMajorVersion() != them.getOSMajorVersion());
+
+ env_
diff erent = (me.getEnvironment() != them.getEnvironment());
+}
+
bool ArchSpec::IsAlwaysThumbInstructions() const {
std::string Status;
if (GetTriple().getArch() == llvm::Triple::arm ||
diff --git a/lldb/test/API/macosx/arm64e-attach/Makefile
b/lldb/test/API/macosx/arm64e-attach/Makefile
deleted file mode 100644
index c9319d6e6888a4..00
--- a/lldb/test/API/macosx/arm64e-attach/Makefile
+++ /dev/null
@@ -1,2 +0,0 @@
-C_SOURCES := main.c
-include Makefile.rules
diff --git a/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
b/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
deleted file mode 100644
index 0dc8700ed02dd8..00
--- a/lldb/test/API/macosx/arm64e-attach/TestArm64eAttach.py
+++ /dev/null
@@ -1,28 +0,0 @@
-import lldb
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
-
-
-class TestArm64eAttach(TestBase):
-NO_DEBUG_INFO_TESTCASE = True
-
-# On Darwin systems, arch arm64e means ARMv8.3 with ptrauth ABI used.
-@skipIf(archs=no_match(["arm64e"]))
-def test(self):
-# Skip this test if not running on AArch64 target that supports PAC
-if not self.isAArch64PAuth():
-self.skipTest("Target must support pointer authentication.")
-self.build()
-popen = self.spawnSubprocess(self.getBuildArtifact(), [])
-error = lldb.SBError()
-# This simulates how Xcode attaches to a process by pid/name.
-target = self.dbg.CreateTarget("", "arm64", "", True, error)
-listener = lldb.SBListener("my.attach.listener")
-
[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)
@@ -3944,15 +3955,22 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
// The order of these checks is important.
if (process_does_not_exist (pid_attaching_to)) {
DNBLogError("Tried to attach to pid that doesn't exist");
- std::string return_message = "E96;";
- return_message += cstring_to_asciihex_string("no such process.");
+ std::string return_message = "E96";
clayborg wrote:
> I'm thinking of having a SendErrorPacket() instead of SendPacket() with an
> error string optional argument, and having all error packets route through
> that, and having all SendPacket("Exx")'s go through that instead. @bulbazord
> @clayborg what do you think? I'm having trouble deciding if I want to have a
> method that constructs the error string and the caller passes it to
> SendPacket or if I want to have a method to do both.
I like that idea!
https://github.com/llvm/llvm-project/pull/82593
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [libclc] [libcxx] [lld] [lldb] [llvm] [NFC] Remove trailing whitespace across all non-test related files (PR #82838)
arichardson wrote: This will cause huge merge conflicts for all downstreams. While they are easy to resolve it can be quite annoying. I think we should just do this as part of the global clang-format. https://github.com/llvm/llvm-project/pull/82838 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [libclc] [libcxx] [lld] [lldb] [llvm] [NFC] Remove trailing whitespace across all non-test related files (PR #82838)
jrtc27 wrote: Also this is the kind of commit that should really be done by a core trusted member of the community. It's way too easy to hide something nefarious (not that I'm accusing you of that, just that we always need to be vigilant) in an 8k+ diff, and it's not much fun to review that code. Also it's best when people provide the scripts used to do these kinds of mass changes; that (a) lets people verify what you did by auditing the script and running it themselves (b) lets downstreams easily perform the same change on their forks. The fact that this is a +8347 -8382 diff also confuses me; I would expect the number of lines to remain constant, but maybe you're including trailing blank lines as trailing whitespace?.. https://github.com/llvm/llvm-project/pull/82838 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [libclc] [libcxx] [lld] [lldb] [llvm] [NFC] Remove trailing whitespace across all non-test related files (PR #82838)
dwblaikie wrote: The dev policy says "Avoid committing formatting- or whitespace-only changes outside of code you plan to make subsequent changes to." - I think it's reasonable to consider changing this, but probably under the "clang-format everything" or a similar discussion (broad discussion before we worry about making pull requests https://github.com/llvm/llvm-project/pull/82838 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] [debugserver] fix qLaunchSuccess error, add QErrorStringInPacketSupported (PR #82593)
@@ -3944,15 +3955,22 @@ rnb_err_t RNBRemote::HandlePacket_v(const char *p) {
// The order of these checks is important.
if (process_does_not_exist (pid_attaching_to)) {
DNBLogError("Tried to attach to pid that doesn't exist");
- std::string return_message = "E96;";
- return_message += cstring_to_asciihex_string("no such process.");
+ std::string return_message = "E96";
bulbazord wrote:
Sounds great to me 😄
https://github.com/llvm/llvm-project/pull/82593
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Print a message when background tasks take a while to complete (PR #82799)
https://github.com/jasonmolenda approved this pull request. I haven't used the std::async feature before, but it looks reasonable. This looks good to me. https://github.com/llvm/llvm-project/pull/82799 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
