[Lldb-commits] [lldb] [lldb][test] Fix TestMultipleDebuggers test on non-x86, other small issues (PR #101169)
https://github.com/DavidSpickett closed https://github.com/llvm/llvm-project/pull/101169 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] aa07282 - [lldb][test] Fix TestMultipleDebuggers test on non-x86, other small issues (#101169)
Author: David Spickett
Date: 2024-07-31T08:37:42+01:00
New Revision: aa07282a25c3d6df04af9a4d34874cc433c9c68a
URL:
https://github.com/llvm/llvm-project/commit/aa07282a25c3d6df04af9a4d34874cc433c9c68a
DIFF:
https://github.com/llvm/llvm-project/commit/aa07282a25c3d6df04af9a4d34874cc433c9c68a.diff
LOG: [lldb][test] Fix TestMultipleDebuggers test on non-x86, other small issues
(#101169)
This test has been flaky lately
(https://github.com/llvm/llvm-project/issues/101162) and I disabled it
everywhere initially.
I found that it always uses "x86_64" for the program architecture so the
test was "passing" elsewhere but I don't think it was meant to. So I
have added a define to pass on the host's architecture when compiling.
This makes it work on AArch64 as well.
While I'm here I've fixed the uint64_t formatting warnings by using the
defined formats that'll work everywhere.
In addition, I found that the function names include "()" on Linux, so
now we check for "foo" or "foo()".
The test cpp file has never been, or was only partially formatted so
I've not formatted the changes, just kept to the local style.
I've removed the Linux skip to see if any of this helps the timeouts,
and to verify the build command changes. If the timeouts come back I'll
disable it again.
Added:
Modified:
lldb/packages/Python/lldbsuite/test/lldbtest.py
lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py
lldb/test/API/api/multiple-debuggers/multi-process-driver.cpp
Removed:
diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py
b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index f97c41d867e78..b57c3bdd87c83 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -1514,19 +1514,23 @@ def getstdFlag(self):
stdflag = "-std=c++11"
return stdflag
-def buildDriver(self, sources, exe_name):
+def buildDriver(self, sources, exe_name, defines=None):
"""Platform-specific way to build a program that links with LLDB (via
the liblldb.so
or LLDB.framework).
"""
+if defines is None:
+defines = []
+
stdflag = self.getstdFlag()
stdlibflag = self.getstdlibFlag()
+defines = " ".join(["-D{}={}".format(name, value) for name, value in
defines])
lib_dir = configuration.lldb_libs_dir
if self.hasDarwinFramework():
d = {
"CXX_SOURCES": sources,
"EXE": exe_name,
-"CFLAGS_EXTRAS": "%s %s" % (stdflag, stdlibflag),
+"CFLAGS_EXTRAS": "%s %s %s" % (stdflag, stdlibflag, defines),
"FRAMEWORK_INCLUDES": "-F%s" % self.framework_dir,
"LD_EXTRAS": "%s -Wl,-rpath,%s" % (self.lib_lldb,
self.framework_dir),
}
@@ -1534,12 +1538,13 @@ def buildDriver(self, sources, exe_name):
d = {
"CXX_SOURCES": sources,
"EXE": exe_name,
-"CFLAGS_EXTRAS": "%s %s -I%s -I%s"
+"CFLAGS_EXTRAS": "%s %s -I%s -I%s %s"
% (
stdflag,
stdlibflag,
os.path.join(os.environ["LLDB_SRC"], "include"),
os.path.join(configuration.lldb_obj_root, "include"),
+defines,
),
"LD_EXTRAS": "-L%s -lliblldb" % lib_dir,
}
@@ -1547,12 +1552,13 @@ def buildDriver(self, sources, exe_name):
d = {
"CXX_SOURCES": sources,
"EXE": exe_name,
-"CFLAGS_EXTRAS": "%s %s -I%s -I%s"
+"CFLAGS_EXTRAS": "%s %s -I%s -I%s %s"
% (
stdflag,
stdlibflag,
os.path.join(os.environ["LLDB_SRC"], "include"),
os.path.join(configuration.lldb_obj_root, "include"),
+defines,
),
"LD_EXTRAS": "-L%s -llldb -Wl,-rpath,%s" % (lib_dir, lib_dir),
}
diff --git a/lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py
b/lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py
index fc9ddac9c1d59..bee6e66aa7219 100644
--- a/lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py
+++ b/lldb/test/API/api/multiple-debuggers/TestMultipleDebuggers.py
@@ -12,15 +12,17 @@
class TestMultipleSimultaneousDebuggers(TestBase):
NO_DEBUG_INFO_TESTCASE = True
-# This test has been flaky lately on Linux buildbots and Github/Buildkite
CI
-# runs.
-@skipIfLinux
+# Sometimes times out on Linux, see
https://github.com/llvm/llvm-project/issues/101162.
@skipIfNoSBHeaders
@skipIfWindows
@skipIfHostIncompatibleWithTarget
def test_multiple_debuggers(self):
self.driver_exe = self.getBuildArt
[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)
https://github.com/DavidSpickett commented: I'm not sure whether we need `LLDB_SHELL_TESTS_DISABLE_REMOTE`, can you explain how that would be used? Would I set up a build with all the other flags, do my remote testing, then set `LLDB_SHELL_TESTS_DISABLE_REMOTE=OFF` and run the shell tests locally, using that same build? I guess I'm asking whether a given build should be all remote or all local or a mix. https://github.com/llvm/llvm-project/pull/95986 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/95986 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)
@@ -22,6 +24,55 @@ def _disallow(config, execName):
config.substitutions.append((" {0} ".format(execName),
warning.format(execName)))
+def get_lldb_args(config, suffix=None):
+lldb_args = []
+if "remote-linux" in config.available_features:
+lldb_args += [
+"-O",
+'"platform select remote-linux"',
+"-O",
+f'"platform connect {config.lldb_platform_url}"',
+]
+if config.lldb_platform_working_dir:
+dir = f"{config.lldb_platform_working_dir}/shell"
+if suffix:
+dir += f"/{suffix}"
DavidSpickett wrote:
Though I would default suffix to `""` instead of `None` just to be clear that
it's default is no suffix but if you do want a suffix, it should be a single
string.
https://github.com/llvm/llvm-project/pull/95986
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)
@@ -22,6 +24,55 @@ def _disallow(config, execName):
config.substitutions.append((" {0} ".format(execName),
warning.format(execName)))
+def get_lldb_args(config, suffix=None):
+lldb_args = []
+if "remote-linux" in config.available_features:
+lldb_args += [
+"-O",
+'"platform select remote-linux"',
+"-O",
+f'"platform connect {config.lldb_platform_url}"',
+]
+if config.lldb_platform_working_dir:
+dir = f"{config.lldb_platform_working_dir}/shell"
+if suffix:
+dir += f"/{suffix}"
DavidSpickett wrote:
I was going to be pedantic and ask for os.path.join here but of course that
would make paths in the host format not the remote's, correct?
https://github.com/llvm/llvm-project/pull/95986
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)
@@ -22,6 +24,55 @@ def _disallow(config, execName):
config.substitutions.append((" {0} ".format(execName),
warning.format(execName)))
+def get_lldb_args(config, suffix=None):
+lldb_args = []
+if "remote-linux" in config.available_features:
+lldb_args += [
+"-O",
+'"platform select remote-linux"',
+"-O",
+f'"platform connect {config.lldb_platform_url}"',
+]
+if config.lldb_platform_working_dir:
+dir = f"{config.lldb_platform_working_dir}/shell"
+if suffix:
+dir += f"/{suffix}"
+lldb_args += [
+"-O",
+f'"platform shell mkdir -p {dir}"',
+"-O",
+f'"platform settings -w {dir}"',
+]
+lldb_args += ["--no-lldbinit", "-S", _get_lldb_init_path(config)]
+return lldb_args
+
+
+class ShTestLldb(ShTest):
+def __init__(
+self, execute_external=False, extra_substitutions=[],
preamble_commands=[]
+):
+super().__init__(execute_external, extra_substitutions,
preamble_commands)
+
+def execute(self, test, litConfig):
DavidSpickett wrote:
This needs some comments. Which need to include justifying the catch all
`except`.
https://github.com/llvm/llvm-project/pull/95986
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)
@@ -4,16 +4,16 @@ target create -l "ls" /bin/ls target list -# CHECK: * target #0 (ls): /bin/ls +# CHECK: * target #0 (ls): [[LS_PATH:.*]] DavidSpickett wrote: Could make this supported on Windows too if we happen to have some basic C file hanging around in this dir and use that to make the test binaries instead, but out of scope for this PR. https://github.com/llvm/llvm-project/pull/95986 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)
@@ -4,16 +4,16 @@ target create -l "ls" /bin/ls target list -# CHECK: * target #0 (ls): /bin/ls +# CHECK: * target #0 (ls): [[LS_PATH:.*]] DavidSpickett wrote: Though `/bin/ls` surely isn't going to be compatible with the remote a large percentage of the time. I assume it's never actually run. https://github.com/llvm/llvm-project/pull/95986 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)
@@ -138,7 +191,7 @@ def use_support_substitutions(config): # Set up substitutions for support tools. These tools can be overridden at the CMake # level (by specifying -DLLDB_LIT_TOOLS_DIR), installed, or as a last resort, we can use # the just-built version. -host_flags = ["--target=" + config.host_triple] +host_flags = ["--target=" + config.target_triple] DavidSpickett wrote: Is it worth changing this variable name? Naming is hard, but "host" sounds like the thing hosting the test runner, where now it refers to the thing that the tests run on. `target_flags` maybe? https://github.com/llvm/llvm-project/pull/95986 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)
@@ -4,16 +4,16 @@ target create -l "ls" /bin/ls target list -# CHECK: * target #0 (ls): /bin/ls +# CHECK: * target #0 (ls): [[LS_PATH:.*]] DavidSpickett wrote: Do I understand correctly here that the path `/bin/ls` is given to target create, that program is sent to the remote and this LS_PATH will be `/ls`? So that's why the substitution is needed. https://github.com/llvm/llvm-project/pull/95986 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/DavidSpickett commented: What, if any, are the changes to how you would run lldb-server on Windows? I see a new argument here but not sure if that's only meant for lldb-server to pass to itself when it starts a child process. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -159,6 +159,40 @@
GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform(
GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() =
default;
+void GDBRemoteCommunicationServerPlatform::Proc(
+const lldb_private::Args &args) {
+ if (!IsConnected())
+return;
+
+ if (args.GetArgumentCount() > 0) {
+lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
+std::optional port;
+std::string socket_name;
+Status error = LaunchGDBServer(args,
+ "", // hostname
+ pid, port, socket_name);
+if (error.Success())
+ SetPendingGdbServer(pid, *port, socket_name);
+ }
+
+ bool interrupt = false;
+ bool done = false;
+ Status error;
+ while (!interrupt && !done) {
+if (GetPacketAndSendResponse(std::nullopt, error, interrupt, done) !=
+GDBRemoteCommunication::PacketResult::Success)
+ break;
+ }
+
+ if (error.Fail()) {
+Log *log = GetLog(LLDBLog::Platform);
+LLDB_LOGF(log,
+ "GDBRemoteCommunicationServerPlatform::%s() "
+ "GetPacketAndSendResponse: %s",
+ __FUNCTION__, error.AsCString());
DavidSpickett wrote:
I would've thought that `__FUNCTION__` gave you the function name here but
maybe not. Can you just try making it `if (true)` and confirm what it prints?
https://github.com/llvm/llvm-project/pull/101283
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -283,54 +293,94 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t
size,
DWORD sys_bytes_read = size;
BOOL result = ::ReadFile(m_read, buf, sys_bytes_read, &sys_bytes_read,
&m_read_overlapped);
- if (!result && GetLastError() != ERROR_IO_PENDING)
-return Status(::GetLastError(), eErrorTypeWin32);
-
- DWORD timeout = (duration == std::chrono::microseconds::zero())
- ? INFINITE
- : duration.count() * 1000;
- DWORD wait_result = ::WaitForSingleObject(m_read_overlapped.hEvent, timeout);
- if (wait_result != WAIT_OBJECT_0) {
-// The operation probably failed. However, if it timed out, we need to
-// cancel the I/O. Between the time we returned from WaitForSingleObject
-// and the time we call CancelIoEx, the operation may complete. If that
-// hapens, CancelIoEx will fail and return ERROR_NOT_FOUND. If that
-// happens, the original operation should be considered to have been
-// successful.
-bool failed = true;
-DWORD failure_error = ::GetLastError();
-if (wait_result == WAIT_TIMEOUT) {
- BOOL cancel_result = CancelIoEx(m_read, &m_read_overlapped);
- if (!cancel_result && GetLastError() == ERROR_NOT_FOUND)
-failed = false;
+ if (!result) {
+if (GetLastError() != ERROR_IO_PENDING)
+ return Status(::GetLastError(), eErrorTypeWin32);
+else {
DavidSpickett wrote:
Don't need the else given that the line above is a return.
https://github.com/llvm/llvm-project/pull/101283
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -159,6 +159,40 @@
GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform(
GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() =
default;
+void GDBRemoteCommunicationServerPlatform::Proc(
+const lldb_private::Args &args) {
+ if (!IsConnected())
+return;
+
+ if (args.GetArgumentCount() > 0) {
+lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
+std::optional port;
+std::string socket_name;
+Status error = LaunchGDBServer(args,
+ "", // hostname
+ pid, port, socket_name);
+if (error.Success())
+ SetPendingGdbServer(pid, *port, socket_name);
DavidSpickett wrote:
What happens with the implicit `else` part of this, does
`GetPacketAndSendResponse` somehow fail immediately?
https://github.com/llvm/llvm-project/pull/101283
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -27,11 +27,13 @@ class TestLogHandler : public LogHandler {
: m_stream_sp(stream_sp) {}
void Emit(llvm::StringRef message) override {
+std::lock_guard guard(m_mutex);
DavidSpickett wrote:
A mutex is now required because of the sharing of the socket I assume?
Could you add a comment on the member variable to say that as well.
https://github.com/llvm/llvm-project/pull/101283
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -283,54 +293,94 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t
size,
DWORD sys_bytes_read = size;
BOOL result = ::ReadFile(m_read, buf, sys_bytes_read, &sys_bytes_read,
&m_read_overlapped);
- if (!result && GetLastError() != ERROR_IO_PENDING)
-return Status(::GetLastError(), eErrorTypeWin32);
-
- DWORD timeout = (duration == std::chrono::microseconds::zero())
- ? INFINITE
- : duration.count() * 1000;
- DWORD wait_result = ::WaitForSingleObject(m_read_overlapped.hEvent, timeout);
- if (wait_result != WAIT_OBJECT_0) {
-// The operation probably failed. However, if it timed out, we need to
-// cancel the I/O. Between the time we returned from WaitForSingleObject
-// and the time we call CancelIoEx, the operation may complete. If that
-// hapens, CancelIoEx will fail and return ERROR_NOT_FOUND. If that
-// happens, the original operation should be considered to have been
-// successful.
-bool failed = true;
-DWORD failure_error = ::GetLastError();
-if (wait_result == WAIT_TIMEOUT) {
- BOOL cancel_result = CancelIoEx(m_read, &m_read_overlapped);
- if (!cancel_result && GetLastError() == ERROR_NOT_FOUND)
-failed = false;
+ if (!result) {
DavidSpickett wrote:
Could do an early return here `if (result) {`. You'll have 2 copies of:
```
bytes_read = sys_bytes_read;
return Status();
```
But it could be easier to read.
https://github.com/llvm/llvm-project/pull/101283
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -114,6 +120,218 @@ static Status save_socket_id_to_file(const std::string
&socket_id,
return status;
}
+static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap;
+static std::mutex gdbserver_portmap_mutex;
+
+#if defined(_WIN32)
+static void SpawnProcessReaped(lldb::pid_t pid, int signal, int status) {
+ std::lock_guard guard(gdbserver_portmap_mutex);
+ gdbserver_portmap.FreePortForProcess(pid);
+}
+
+static bool SpawnProcessParent(const char *progname, Connection *conn,
+ uint16_t gdb_port, uint16_t port_offset,
+ const lldb_private::Args &args,
+ const std::string &log_file,
+ const StringRef log_channels) {
+ Log *log = GetLog(LLDBLog::Platform);
+ Pipe socket_pipe;
+ Status error = socket_pipe.CreateNew(true);
+ if (error.Fail()) {
+LLDB_LOGF(log,
+ "lldb-platform parent: "
+ "cannot create pipe: %s",
+ error.AsCString());
+return false;
+ }
+
+ ProcessLaunchInfo launch_info;
+ FileSpec self_spec(progname, FileSpec::Style::native);
+ launch_info.SetExecutableFile(self_spec, true);
+ Args &self_args = launch_info.GetArguments();
+ self_args.AppendArgument(llvm::StringRef("platform"));
+ self_args.AppendArgument(llvm::StringRef("--accept"));
+ self_args.AppendArgument(llvm::to_string(socket_pipe.GetReadPipe()));
+ if (gdb_port) {
+self_args.AppendArgument(llvm::StringRef("--gdbserver-port"));
+self_args.AppendArgument(llvm::to_string(gdb_port));
+ }
+ if (port_offset > 0) {
+self_args.AppendArgument(llvm::StringRef("--port-offset"));
+self_args.AppendArgument(llvm::to_string(port_offset));
+ }
+ if (!log_file.empty()) {
+self_args.AppendArgument(llvm::StringRef("--log-file"));
+self_args.AppendArgument(log_file);
+ }
+ if (!log_channels.empty()) {
+self_args.AppendArgument(llvm::StringRef("--log-channels"));
+self_args.AppendArgument(log_channels);
+ }
+ if (args.GetArgumentCount() > 0) {
+self_args.AppendArgument("--");
+self_args.AppendArguments(args);
+ }
+
+ launch_info.SetLaunchInSeparateProcessGroup(false);
+ launch_info.SetMonitorProcessCallback(&SpawnProcessReaped);
+
+ // Copy the current environment.
+ // WSASocket(FROM_PROTOCOL_INFO) will fail in the child process
+ // with the error WSAEPROVIDERFAILEDINIT if the SystemRoot is missing
+ // in the environment.
+ launch_info.GetEnvironment() = Host::GetEnvironment();
+
+ launch_info.GetFlags().Set(eLaunchFlagDisableSTDIO);
+
+ launch_info.AppendCloseFileAction(socket_pipe.GetWriteFileDescriptor());
+
+ // Close STDIN, STDOUT and STDERR.
+ launch_info.AppendCloseFileAction(STDIN_FILENO);
+ launch_info.AppendCloseFileAction(STDOUT_FILENO);
+ launch_info.AppendCloseFileAction(STDERR_FILENO);
+
+ // Redirect STDIN, STDOUT and STDERR to "/dev/null".
+ launch_info.AppendSuppressFileAction(STDIN_FILENO, true, false);
+ launch_info.AppendSuppressFileAction(STDOUT_FILENO, false, true);
+ launch_info.AppendSuppressFileAction(STDERR_FILENO, false, true);
+
+ std::string cmd;
+ self_args.GetCommandString(cmd);
+
+ error = Host::LaunchProcess(launch_info);
+ if (error.Fail()) {
+LLDB_LOGF(log,
+ "lldb-platform parent: "
+ "cannot launch child process for connection: %s",
+ error.AsCString());
+return false;
+ }
+
+ lldb::pid_t childPid = launch_info.GetProcessID();
+ if (childPid == LLDB_INVALID_PROCESS_ID) {
+LLDB_LOGF(log, "lldb-platform parent: "
+ "cannot launch child process for connection: invalid pid");
+return false;
+ }
+ LLDB_LOGF(log,
+"lldb-platform parent: "
+"launched '%s', pid=0x%x",
+cmd.c_str(), childPid);
+
+ {
+std::lock_guard guard(gdbserver_portmap_mutex);
+gdbserver_portmap.AssociatePortWithProcess(gdb_port, childPid);
+ }
+
+ if (socket_pipe.CanRead())
+socket_pipe.CloseReadFileDescriptor();
+ if (!socket_pipe.CanWrite()) {
+LLDB_LOGF(log, "lldb-platform parent: "
+ "cannot write to socket_pipe");
+Host::Kill(childPid, SIGTERM);
+return false;
+ }
+
+ const TCPSocket &socket =
+ static_cast(*conn->GetReadObject());
+ NativeSocket nativeSocket = socket.GetNativeSocket();
+
+ WSAPROTOCOL_INFO protocol_info;
+ if (::WSADuplicateSocket(nativeSocket, childPid, &protocol_info) ==
+ SOCKET_ERROR) {
+LLDB_LOGF(log,
+ "lldb-platform parent: "
+ "WSADuplicateSocket() failed, error: %d",
+ ::WSAGetLastError());
+Host::Kill(childPid, SIGTERM);
+return false;
+ }
+
+ size_t num_bytes;
+ error = socket_pipe.WriteWithTimeout(&protocol_info, sizeof(protocol_info),
+ std::chrono::seconds(2), num_bytes);
+ if (error.Fail()) {
+LLDB_LOGF(log,
+ "lldb-platform parent: "
+ "s
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -159,6 +159,40 @@ GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform( GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() = default; +void GDBRemoteCommunicationServerPlatform::Proc( DavidSpickett wrote: If we're changing the name could we have a better name than `Proc`, or if `Proc` is just a term I am unfamiliar with, a comment in the header to explain what this function does? https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
DavidSpickett wrote: What exactly does `lldb-server --server` mean? Because at least on Linux I see: ``` $ ./bin/lldb-server Usage: ./bin/lldb-server v[ersion] ./bin/lldb-server g[dbserver] [options] ./bin/lldb-server p[latform] [options] Invoke subcommand for additional help ``` Maybe you just have a typo or `--server` is a shortcut for one of those. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -114,6 +120,218 @@ static Status save_socket_id_to_file(const std::string &socket_id, return status; } +static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap; +static std::mutex gdbserver_portmap_mutex; + +#if defined(_WIN32) DavidSpickett wrote: Do you think we will always have these large __WIN32 blocks, or if we go forward with your plan, will some of that become generic code once again? https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -336,3 +336,52 @@ TEST(MinidumpYAML, ExceptionStream_ExtraParameter) {
0xab, 0xad, 0xca, 0xfe}),
*ExpectedContext);
}
+
+TEST(MinidumpYAML, MemoryRegion_64bit) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Memory64List
+Memory Ranges:
+ - Start of Memory Range: 0x7FCF0818283
+Content: '68656c6c6f'
+ - Start of Memory Range: 0x7FFF0818283
+Content: '776f726c64'
+)");
+
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile &File = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected> ExpectedMemoryList =
+ File.getMemory64List();
+
+ ASSERT_THAT_EXPECTED(ExpectedMemoryList, Succeeded());
+
+ ArrayRef MemoryList = *ExpectedMemoryList;
+ ASSERT_EQ(2u, MemoryList.size());
+
+ const minidump::MemoryDescriptor_64 &DescOne = MemoryList[0];
+ ASSERT_EQ(0x7FCF0818283u, DescOne.StartOfMemoryRange);
+ ASSERT_EQ(5u, DescOne.DataSize);
+
+ const minidump::MemoryDescriptor_64 &DescTwo = MemoryList[1];
+ ASSERT_EQ(0x7FFF0818283u, DescTwo.StartOfMemoryRange);
+ ASSERT_EQ(5u, DescTwo.DataSize);
+
+ const std::optional> ExpectedContent =
+ File.getRawStream(StreamType::Memory64List);
+ ASSERT_TRUE(ExpectedContent);
+ const size_t ExpectedStreamSize = sizeof(Memory64ListHeader) +
(sizeof(MemoryDescriptor_64) * 2);
+ ASSERT_EQ(ExpectedStreamSize, ExpectedContent->size());
+
+ Expected> DescOneExpectedContentSlice =
File.getRawData(DescOne);
+ ASSERT_THAT_EXPECTED(DescOneExpectedContentSlice, Succeeded());
+ ASSERT_EQ("hello", reinterpret_cast(DescOneExpectedContentSlice->data()));
labath wrote:
`ASSERT_THAT_EXPECTED(File.getRawData(DescOne),
HasValue(arrayRefFromStringRef("hello")));`
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -336,3 +336,52 @@ TEST(MinidumpYAML, ExceptionStream_ExtraParameter) {
0xab, 0xad, 0xca, 0xfe}),
*ExpectedContext);
}
+
+TEST(MinidumpYAML, MemoryRegion_64bit) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Memory64List
+Memory Ranges:
+ - Start of Memory Range: 0x7FCF0818283
+Content: '68656c6c6f'
+ - Start of Memory Range: 0x7FFF0818283
+Content: '776f726c64'
+)");
+
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile &File = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected> ExpectedMemoryList =
+ File.getMemory64List();
+
+ ASSERT_THAT_EXPECTED(ExpectedMemoryList, Succeeded());
+
+ ArrayRef MemoryList = *ExpectedMemoryList;
+ ASSERT_EQ(2u, MemoryList.size());
+
+ const minidump::MemoryDescriptor_64 &DescOne = MemoryList[0];
+ ASSERT_EQ(0x7FCF0818283u, DescOne.StartOfMemoryRange);
+ ASSERT_EQ(5u, DescOne.DataSize);
+
+ const minidump::MemoryDescriptor_64 &DescTwo = MemoryList[1];
+ ASSERT_EQ(0x7FFF0818283u, DescTwo.StartOfMemoryRange);
+ ASSERT_EQ(5u, DescTwo.DataSize);
labath wrote:
I'd try something like
`ASSERT_THAT_EXPECTED(ExpectedMemoryList,
HasValue(testing::ElementsAre(MemoryDescriptor_64{foo, bar},
MemoryDescriptor_64{baz, bazz})))`
I'm not sure it will work with all of the c++ magic around these classes, but
it'd look much nicer if it did.
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -1,7 +1,7 @@ --- !minidump -Streams: +Streams: labath wrote: Please either revert these changes or put them in as a separate patch (you don't need to wait for a review on that). https://github.com/llvm/llvm-project/pull/101272 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -373,7 +373,6 @@ void yaml::MappingContextTraits::mapping(
void yaml::MappingContextTraits::mapping(
IO &IO, MemoryDescriptor_64 &Memory, BinaryRef &Content) {
mapRequiredHex(IO, "Start of Memory Range", Memory.StartOfMemoryRange);
- mapRequiredHex(IO, "Data Size", Memory.DataSize);
labath wrote:
I actually think we should provide a way to override the DataSize field, as the
main purpose of the yaml is to test the parser with possibly invalid input. It
doesn't have to be present by default though, which I think you can achieve by
using `IO.mapOptional("Data Size", Memory.DataSize, Content.binary_size());` --
and putting it after the "Content" line.
(I'm not using hex, because the RawContentStream is also printed in decimal,
but I could be convinced to change both).
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -550,7 +588,7 @@ Stream::create(const Directory &StreamDesc, const
object::MinidumpFile &File) {
llvm_unreachable("Unhandled stream kind!");
}
-Expected Object::create(const object::MinidumpFile &File) {
+Expected Object::create(object::MinidumpFile &File) {
labath wrote:
Yeah, this doesn't seem completely ideal. Looking at the code, it looks like
the only place where you're accessing these, is when you're iterating over them
anyway, so maybe some sort of an iterator would be in order?
It could store the current offset in its state without modifying the underlying
container. And perhaps its value_type could be `std::pair>` ? It might also need to be an `llvm::fallible_iterator` so
it can report errors it encounters mid-flight.
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/labath commented: I think we should slow down a bit. For consistency, and to minimize the number of ifdefs, I believe windows and non-windows paths should use as similar approaches as possible. That means I think the end state should be that the posix path also uses a fork+exec approach. For me, it also seems natural that this should be the first step. While doing that, we can make sure to create abstractions that can be used to implement the windows behavior as well. I'm imagining some sort of a function or a class, which takes the target binary, its arguments, and a socket, and launches that binary with that socket. (we'll probably also need a similar function/class on the other end to receive that socket). The function doesn't have to be terribly generic, but it ideally should encompass all of platform specific behavior, so that the surrounding code does not need to use ifdefs. (But, ideally, it should at least be slightly generic, so that one can write a unit test for this functionality). (BTW, I'm OOO for the rest of the week, so I won't be able to review this in detail. Upon cursory inspection, the general idea makes sense, but we really need to figure out how to encapsulate this. I'm not sure if we really need the WriteWithTimeout functionality. The reason we don't have it so far is that pipes have a buffer, and unless we're writing a huge chunk of data, the write will never block anyway. There's nothing wrong with it in principle though, and if you do want to have it, it'd be best if it comes with in its own patch and with a test.) https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
slydiman wrote: > What exactly does `lldb-server --server` mean? Sorry. I have updated the description with `lldb-server platform --server`. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -114,6 +120,218 @@ static Status save_socket_id_to_file(const std::string &socket_id, return status; } +static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap; +static std::mutex gdbserver_portmap_mutex; + +#if defined(_WIN32) slydiman wrote: We can move this code to some windows specific directory. But it is windows specific anyway. Note it contains a lot of `lldb-server platform` specific parameters. We can just pass the socket FD via the command line on non-Windows OS. `lldb-server gdbserver` already supports it with the parameter `--fd`. But it does not work on Windows. acceprt_fd here is a pipe FD which is used to transfer the structure WSAPROTOCOL_INFO. We need to know the child pid to initialize WSAPROTOCOL_INFO. Finally the child process is able to initialize the accepted socket FROM_PROTOCOL_INFO. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -27,11 +27,13 @@ class TestLogHandler : public LogHandler {
: m_stream_sp(stream_sp) {}
void Emit(llvm::StringRef message) override {
+std::lock_guard guard(m_mutex);
slydiman wrote:
Probably I can separate this change to another patch. It is not related to the
socket sharing. It is the fix of an old bug.
Host::LaunchProcess() requires to SetMonitorProcessCallback. This callback is
called from the child process monitor thread. We cannot control this thread.
lldb-server may crash if there is a logging around this callback because
TestLogHandler is not thread safe.
I've kept this code here for now because I faced this issue only debugging this
patch with 100 simultaneous child processes. It is hard to reproduce it other
way. Note also [this
comment](https://github.com/llvm/llvm-project/pull/101283#issuecomment-2259628620).
https://github.com/llvm/llvm-project/pull/101283
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -159,6 +159,40 @@ GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform( GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() = default; +void GDBRemoteCommunicationServerPlatform::Proc( slydiman wrote: Note `void SetInferiorArguments(const lldb_private::Args &args);` is a missing function. It is a bug. I have just removed it and added Proc(). There is no any connection between these functions. I have just moved a common code from lldb-platform to GDBRemoteCommunicationServerPlatform::Proc(). We can rename it to ClientHandle() or such and even kept inside lldb-platform.cpp https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -159,6 +159,40 @@
GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform(
GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() =
default;
+void GDBRemoteCommunicationServerPlatform::Proc(
+const lldb_private::Args &args) {
+ if (!IsConnected())
+return;
+
+ if (args.GetArgumentCount() > 0) {
+lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
+std::optional port;
+std::string socket_name;
+Status error = LaunchGDBServer(args,
+ "", // hostname
+ pid, port, socket_name);
+if (error.Success())
+ SetPendingGdbServer(pid, *port, socket_name);
+ }
+
+ bool interrupt = false;
+ bool done = false;
+ Status error;
+ while (!interrupt && !done) {
+if (GetPacketAndSendResponse(std::nullopt, error, interrupt, done) !=
+GDBRemoteCommunication::PacketResult::Success)
+ break;
+ }
+
+ if (error.Fail()) {
+Log *log = GetLog(LLDBLog::Platform);
+LLDB_LOGF(log,
+ "GDBRemoteCommunicationServerPlatform::%s() "
+ "GetPacketAndSendResponse: %s",
+ __FUNCTION__, error.AsCString());
slydiman wrote:
Look at other LLDB_LOGF() usage in GDBRemoteCommunicationServerPlatform.cpp I
just used the similar code and format. This will print
`GDBRemoteCommunicationServerPlatform::Proc() GetPacketAndSendResponse: lost
connection` to the log on Linux and
`GDBRemoteCommunicationServerPlatform::lldb_private::process_gdb_remote::GDBRemoteCommunicationServerPlatform::Proc()
GetPacketAndSendResponse: lost connection` on Windows.
https://github.com/llvm/llvm-project/pull/101283
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -159,6 +159,40 @@
GDBRemoteCommunicationServerPlatform::GDBRemoteCommunicationServerPlatform(
GDBRemoteCommunicationServerPlatform::~GDBRemoteCommunicationServerPlatform() =
default;
+void GDBRemoteCommunicationServerPlatform::Proc(
+const lldb_private::Args &args) {
+ if (!IsConnected())
+return;
+
+ if (args.GetArgumentCount() > 0) {
+lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
+std::optional port;
+std::string socket_name;
+Status error = LaunchGDBServer(args,
+ "", // hostname
+ pid, port, socket_name);
+if (error.Success())
+ SetPendingGdbServer(pid, *port, socket_name);
slydiman wrote:
This code is just moved from lldb-platform.cpp
GetPacketAndSendResponse() ignores `error` value. GetPacketAndSendResponse()
must set the flag `quit` or `interrupt` and update `error`.
https://github.com/llvm/llvm-project/pull/101283
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman updated
https://github.com/llvm/llvm-project/pull/101283
>From 526708c8ac09275049c9afc8e7673fc5f3d889ed Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev
Date: Wed, 31 Jul 2024 05:41:21 +0400
Subject: [PATCH] [lldb] Updated lldb-server to spawn the child process and
share socket on Windows
`lldb-server platform --server` works on Windows now w/o multithreading. The
rest functionality remains unchanged.
Added also PipeWindows::WriteWithTimeout(), fixed
PipeWindows::ReadWithTimeout() and missing initialization of
m_read_overlapped.hEvent in the constructor PipeWindows(lldb::pipe_t read,
lldb::pipe_t write).
Fixes #90923, fixes #56346.
This is the part 1 of the replacement of #100670.
In the part 2 I plan to switch `lldb-server gdbserver` to use `--fd` and listen
a common gdb port for all gdbserver connections. Then we can remove gdb port
mapping to fix #97537.
---
lldb/include/lldb/Host/windows/PipeWindows.h | 3 +
lldb/source/Host/windows/PipeWindows.cpp | 69 +++-
.../GDBRemoteCommunicationServerPlatform.cpp | 34 ++
.../GDBRemoteCommunicationServerPlatform.h| 2 +-
.../tools/lldb-server/LLDBServerUtilities.cpp | 2 +
lldb/tools/lldb-server/lldb-platform.cpp | 315 +++---
6 files changed, 375 insertions(+), 50 deletions(-)
diff --git a/lldb/include/lldb/Host/windows/PipeWindows.h
b/lldb/include/lldb/Host/windows/PipeWindows.h
index 4b5be28d7ae6c..c99cf52be46c1 100644
--- a/lldb/include/lldb/Host/windows/PipeWindows.h
+++ b/lldb/include/lldb/Host/windows/PipeWindows.h
@@ -61,6 +61,9 @@ class PipeWindows : public PipeBase {
Status Delete(llvm::StringRef name) override;
Status Write(const void *buf, size_t size, size_t &bytes_written) override;
+ Status WriteWithTimeout(const void *buf, size_t size,
+ const std::chrono::microseconds &timeout,
+ size_t &bytes_written);
Status ReadWithTimeout(void *buf, size_t size,
const std::chrono::microseconds &timeout,
size_t &bytes_read) override;
diff --git a/lldb/source/Host/windows/PipeWindows.cpp
b/lldb/source/Host/windows/PipeWindows.cpp
index c82c919607b5b..8f86c7e28046e 100644
--- a/lldb/source/Host/windows/PipeWindows.cpp
+++ b/lldb/source/Host/windows/PipeWindows.cpp
@@ -58,7 +58,10 @@ PipeWindows::PipeWindows(pipe_t read, pipe_t write)
}
ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped));
+ m_read_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr);
+
ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
+ m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr);
}
PipeWindows::~PipeWindows() { Close(); }
@@ -77,6 +80,7 @@ Status PipeWindows::CreateNew(bool child_process_inherit) {
m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY);
ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
+ m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr);
return Status();
}
@@ -202,6 +206,7 @@ Status PipeWindows::OpenNamedPipe(llvm::StringRef name,
m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY);
ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
+m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr);
}
return Status();
@@ -228,6 +233,8 @@ int PipeWindows::ReleaseWriteFileDescriptor() {
return PipeWindows::kInvalidDescriptor;
int result = m_write_fd;
m_write_fd = PipeWindows::kInvalidDescriptor;
+ if (m_write_overlapped.hEvent)
+::CloseHandle(m_write_overlapped.hEvent);
m_write = INVALID_HANDLE_VALUE;
ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
return result;
@@ -250,6 +257,9 @@ void PipeWindows::CloseWriteFileDescriptor() {
if (!CanWrite())
return;
+ if (m_write_overlapped.hEvent)
+::CloseHandle(m_write_overlapped.hEvent);
+
_close(m_write_fd);
m_write = INVALID_HANDLE_VALUE;
m_write_fd = PipeWindows::kInvalidDescriptor;
@@ -283,7 +293,12 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t size,
DWORD sys_bytes_read = size;
BOOL result = ::ReadFile(m_read, buf, sys_bytes_read, &sys_bytes_read,
&m_read_overlapped);
- if (!result && GetLastError() != ERROR_IO_PENDING)
+ if (result) {
+bytes_read = sys_bytes_read;
+return Status();
+ }
+
+ if (GetLastError() != ERROR_IO_PENDING)
return Status(::GetLastError(), eErrorTypeWin32);
DWORD timeout = (duration == std::chrono::microseconds::zero())
@@ -319,18 +334,56 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t
size,
Status PipeWindows::Write(const void *buf, size_t num_bytes,
size_t &bytes_written) {
+ return WriteWithTimeout(buf, num_bytes, std::chrono::microseconds::zero(),
+ bytes_written);
+}
+
+Status PipeWindows::WriteWithTimeout(const void *buf, size_t size,
+
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -283,54 +293,94 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t
size,
DWORD sys_bytes_read = size;
BOOL result = ::ReadFile(m_read, buf, sys_bytes_read, &sys_bytes_read,
&m_read_overlapped);
- if (!result && GetLastError() != ERROR_IO_PENDING)
-return Status(::GetLastError(), eErrorTypeWin32);
-
- DWORD timeout = (duration == std::chrono::microseconds::zero())
- ? INFINITE
- : duration.count() * 1000;
- DWORD wait_result = ::WaitForSingleObject(m_read_overlapped.hEvent, timeout);
- if (wait_result != WAIT_OBJECT_0) {
-// The operation probably failed. However, if it timed out, we need to
-// cancel the I/O. Between the time we returned from WaitForSingleObject
-// and the time we call CancelIoEx, the operation may complete. If that
-// hapens, CancelIoEx will fail and return ERROR_NOT_FOUND. If that
-// happens, the original operation should be considered to have been
-// successful.
-bool failed = true;
-DWORD failure_error = ::GetLastError();
-if (wait_result == WAIT_TIMEOUT) {
- BOOL cancel_result = CancelIoEx(m_read, &m_read_overlapped);
- if (!cancel_result && GetLastError() == ERROR_NOT_FOUND)
-failed = false;
+ if (!result) {
+if (GetLastError() != ERROR_IO_PENDING)
+ return Status(::GetLastError(), eErrorTypeWin32);
+else {
slydiman wrote:
Thanks. Updated.
https://github.com/llvm/llvm-project/pull/101283
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -283,54 +293,94 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t
size,
DWORD sys_bytes_read = size;
BOOL result = ::ReadFile(m_read, buf, sys_bytes_read, &sys_bytes_read,
&m_read_overlapped);
- if (!result && GetLastError() != ERROR_IO_PENDING)
-return Status(::GetLastError(), eErrorTypeWin32);
-
- DWORD timeout = (duration == std::chrono::microseconds::zero())
- ? INFINITE
- : duration.count() * 1000;
- DWORD wait_result = ::WaitForSingleObject(m_read_overlapped.hEvent, timeout);
- if (wait_result != WAIT_OBJECT_0) {
-// The operation probably failed. However, if it timed out, we need to
-// cancel the I/O. Between the time we returned from WaitForSingleObject
-// and the time we call CancelIoEx, the operation may complete. If that
-// hapens, CancelIoEx will fail and return ERROR_NOT_FOUND. If that
-// happens, the original operation should be considered to have been
-// successful.
-bool failed = true;
-DWORD failure_error = ::GetLastError();
-if (wait_result == WAIT_TIMEOUT) {
- BOOL cancel_result = CancelIoEx(m_read, &m_read_overlapped);
- if (!cancel_result && GetLastError() == ERROR_NOT_FOUND)
-failed = false;
+ if (!result) {
slydiman wrote:
Thanks. Updated.
https://github.com/llvm/llvm-project/pull/101283
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
slydiman wrote: > What, if any, are the changes to how you would run lldb-server on Windows? I > see a new argument here but not sure if that's only meant for lldb-server to > pass to itself when it starts a child process. The explanation [is here](https://github.com/llvm/llvm-project/pull/101283#discussion_r1698289698). This new argument is the fork() workaround for Windows. We can keep this new argument for non-Windows OS, but it is redundant because fork() is enough. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)
@@ -873,33 +873,29 @@ Address ObjectFileELF::GetImageInfoAddress(Target
*target) {
if (!section_list)
return Address();
- // Find the SHT_DYNAMIC (.dynamic) section.
- SectionSP dynsym_section_sp(
- section_list->FindSectionByType(eSectionTypeELFDynamicLinkInfo, true));
- if (!dynsym_section_sp)
-return Address();
- assert(dynsym_section_sp->GetObjectFile() == this);
-
- user_id_t dynsym_id = dynsym_section_sp->GetID();
- const ELFSectionHeaderInfo *dynsym_hdr = GetSectionHeaderByIndex(dynsym_id);
- if (!dynsym_hdr)
-return Address();
-
for (size_t i = 0; i < m_dynamic_symbols.size(); ++i) {
-ELFDynamic &symbol = m_dynamic_symbols[i];
+const ELFDynamic &symbol = m_dynamic_symbols[i].symbol;
if (symbol.d_tag == DT_DEBUG) {
// Compute the offset as the number of previous entries plus the size of
// d_tag.
- addr_t offset = i * dynsym_hdr->sh_entsize + GetAddressByteSize();
- return Address(dynsym_section_sp, offset);
+ addr_t offset = (i * 2 + 1) * GetAddressByteSize();
+ addr_t file_addr = m_dynamic_base_addr + offset;
+ Address addr;
labath wrote:
Could this be moved outside the if block ?
https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)
@@ -2472,48 +2429,47 @@ size_t ObjectFileELF::ParseDynamicSymbols() {
if (m_dynamic_symbols.size())
return m_dynamic_symbols.size();
- SectionList *section_list = GetSectionList();
- if (!section_list)
+ std::optional dynamic_data = GetDynamicData();
+ if (!dynamic_data)
return 0;
- // Find the SHT_DYNAMIC section.
- Section *dynsym =
- section_list->FindSectionByType(eSectionTypeELFDynamicLinkInfo, true)
- .get();
- if (!dynsym)
-return 0;
- assert(dynsym->GetObjectFile() == this);
-
- ELFDynamic symbol;
- DataExtractor dynsym_data;
- if (ReadSectionData(dynsym, dynsym_data)) {
-const lldb::offset_t section_size = dynsym_data.GetByteSize();
-lldb::offset_t cursor = 0;
-
-while (cursor < section_size) {
- if (!symbol.Parse(dynsym_data, &cursor))
+ ELFDynamicWithName e;
+ lldb::offset_t cursor = 0;
+ while (e.symbol.Parse(*dynamic_data, &cursor)) {
+m_dynamic_symbols.push_back(e);
+if (e.symbol.d_tag == DT_NULL)
+ break;
+ }
+ if (std::optional dynstr_data = GetDynstrData()) {
+for (ELFDynamicWithName &entry : m_dynamic_symbols) {
+ switch (entry.symbol.d_tag) {
+ case DT_NEEDED:
+ case DT_SONAME:
+ case DT_RPATH:
+ case DT_RUNPATH:
+ case DT_AUXILIARY:
+ case DT_FILTER: {
+lldb::offset_t cursor = entry.symbol.d_val;
+const char *name = dynstr_data->GetCStr(&cursor);
+if (name)
+ entry.name = name;
labath wrote:
```suggestion
if (const char *name = dynstr_data->GetCStr(&cursor))
entry.name = std::string(name);
```
(using the std::string as a subtle cue to the reader that the lhs is a string.)
https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)
@@ -873,33 +873,29 @@ Address ObjectFileELF::GetImageInfoAddress(Target
*target) {
if (!section_list)
return Address();
- // Find the SHT_DYNAMIC (.dynamic) section.
- SectionSP dynsym_section_sp(
- section_list->FindSectionByType(eSectionTypeELFDynamicLinkInfo, true));
- if (!dynsym_section_sp)
-return Address();
- assert(dynsym_section_sp->GetObjectFile() == this);
-
- user_id_t dynsym_id = dynsym_section_sp->GetID();
- const ELFSectionHeaderInfo *dynsym_hdr = GetSectionHeaderByIndex(dynsym_id);
- if (!dynsym_hdr)
-return Address();
-
for (size_t i = 0; i < m_dynamic_symbols.size(); ++i) {
-ELFDynamic &symbol = m_dynamic_symbols[i];
+const ELFDynamic &symbol = m_dynamic_symbols[i].symbol;
if (symbol.d_tag == DT_DEBUG) {
// Compute the offset as the number of previous entries plus the size of
// d_tag.
- addr_t offset = i * dynsym_hdr->sh_entsize + GetAddressByteSize();
- return Address(dynsym_section_sp, offset);
+ addr_t offset = (i * 2 + 1) * GetAddressByteSize();
+ addr_t file_addr = m_dynamic_base_addr + offset;
+ Address addr;
+ if (addr.ResolveAddressUsingFileSections(file_addr, GetSectionList()))
+return addr;
}
// MIPS executables uses DT_MIPS_RLD_MAP_REL to support PIE.
DT_MIPS_RLD_MAP
// exists in non-PIE.
else if ((symbol.d_tag == DT_MIPS_RLD_MAP ||
symbol.d_tag == DT_MIPS_RLD_MAP_REL) &&
target) {
- addr_t offset = i * dynsym_hdr->sh_entsize + GetAddressByteSize();
+ SectionSP dynsym_section_sp(section_list->FindSectionByType(
+ eSectionTypeELFDynamicLinkInfo, true));
+ if (!dynsym_section_sp)
+return Address();
+
+ addr_t offset = (i * 2 + 1) * GetAddressByteSize();
addr_t dyn_base = dynsym_section_sp->GetLoadBaseAddress(target);
if (dyn_base == LLDB_INVALID_ADDRESS)
return Address();
labath wrote:
This is just the old way of getting `m_dynamic_base_addr` is it not?
https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)
@@ -0,0 +1,58 @@ +// REQUIRES: system-linux, native labath wrote: I would like to see a test which uses an on-disk section-header-free object file. (Maybe you could base it off of llvm/test/tools/llvm-readobj/ELF/dynamic-reloc-no-section-headers.test). The in-memory test has a lot of moving parts, it seems pretty fragile(*), and can only run on linux (or at least an elf system). OTOH, an on-disk test would be very self contained and can run anywhere. If this breaks (which I'm sure it eventually will), I'd much rather be debugging the second one. I know that in-memory object files are your main use case, and we should have a test for that, but I think it should be like a cherry on top rather than the first&only line of defense. (*) It relies on certain entries being produces by the linker. Most of these are pretty much guaranteed to be there, but for example DT_NEEDED(libc) may not exist if the c library is linked statically. If we can test all of the small pieces with files, then I think it'd be best if the overall in memory test was a full end-to-end API test, which actually deletes the object file, and then verifies that we're still able to read certain things from the object. https://github.com/llvm/llvm-project/pull/101237 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)
@@ -402,6 +413,29 @@ class ObjectFileELF : public lldb_private::ObjectFile {
/// .gnu_debugdata section or \c nullptr if an error occured or if there's no
/// section with that name.
std::shared_ptr GetGnuDebugDataObjectFile();
+
+ /// Get the bytes that represent the .dynamic section.
+ ///
+ /// This function will fetch the data for the .dynamic section in an ELF
file.
+ /// If the ELF file is loaded from a file on disk, it will use the PT_DYNAMIC
+ /// program header to extract the data and fall back to using the section
+ /// headers. If the ELF file is loaded from memory, it will use the
PT_DYNAMIC
labath wrote:
I believe these functions shouldn't distinguish whether the file is in memory
or not, but rather whether it has section headers present.
https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)
@@ -3704,3 +3790,83 @@ ObjectFileELF::MapFileDataWritable(const FileSpec &file,
uint64_t Size,
return FileSystem::Instance().CreateWritableDataBuffer(file.GetPath(), Size,
Offset);
}
+
+std::optional ObjectFileELF::GetDynstrData() {
+
+ SectionList *section_list = GetSectionList();
+ if (section_list) {
+// Find the SHT_DYNAMIC section.
+Section *dynamic =
+section_list->FindSectionByType(eSectionTypeELFDynamicLinkInfo, true)
+.get();
+if (dynamic) {
+ assert(dynamic->GetObjectFile() == this);
+ const ELFSectionHeaderInfo *header =
+ GetSectionHeaderByIndex(dynamic->GetID());
+ if (header) {
+// sh_link: section header index of string table used by entries in
+// the section.
+Section *dynstr = section_list->FindSectionByID(header->sh_link).get();
+DataExtractor data;
+if (dynstr && ReadSectionData(dynstr, data))
+ return data;
+ }
+}
+ }
+
+ // Every ELF file which represents an executable or shared library has
+ // mandatory .dynamic entries. Two of these values are DT_STRTAB and DT_STRSZ
+ // and represent the dynamic symbol tables's string table. These are needed
+ // by the dynamic loader and we can read them from a process' address space.
+ //
+ // When loading and ELF file from memory, only the program headers end up
+ // being mapped into memory, and we can find these values in the PT_DYNAMIC
+ // segment.
+ if (!IsInMemory())
+return std::nullopt;
+ ProcessSP process_sp(m_process_wp.lock());
+ if (!process_sp)
+return std::nullopt;
+
+ const ELFDynamic *strtab = FindDynamicSymbol(DT_STRTAB);
+ const ELFDynamic *strsz = FindDynamicSymbol(DT_STRSZ);
+ if (strtab == nullptr || strsz == nullptr)
+return std::nullopt;
+
+ DataBufferSP data_sp = ReadMemory(process_sp, strtab->d_ptr, strsz->d_val);
labath wrote:
This is the tricky part. This line assumes that the DT_STRTAB entry has been
relocated by the dynamic linker. This will be true for an in-memory elf file
(*), but not for an on-disk file, where it should be an unrelocated file
(virtual) address (@MaskRay, is that guaranteed?) If we add a fallback for
treating this as a file address for on-disk files, then we should be able to
run this code on files as well -- which will make testing a lot easier.
(*) it's only true for if the dynamic linker has had a chance to do it's job
(i.e., we don't run this too early). This should be mostly fine because the way
we normally learn about shared libraries is through linker notifications, but I
could imagine some sort of a file-addr fallback for in-memory files as well.
https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)
@@ -3664,7 +3730,27 @@ llvm::ArrayRef
ObjectFileELF::ProgramHeaders() {
}
DataExtractor ObjectFileELF::GetSegmentData(const ELFProgramHeader &H) {
- return DataExtractor(m_data, H.p_offset, H.p_filesz);
+ // Try and read the program header from our cached m_data which can come from
+ // the file on disk being mmap'ed or from the initial part of the ELF file we
+ // read from memory and cached.
+ DataExtractor data = DataExtractor(m_data, H.p_offset, H.p_filesz);
+ if (data.GetByteSize() == H.p_filesz)
+return data;
+ if (IsInMemory()) {
+// We have a ELF file in process memory, read the program header data from
+// the process.
+ProcessSP process_sp(m_process_wp.lock());
+if (process_sp) {
+ const lldb::offset_t base_file_addr = GetBaseAddress().GetFileAddress();
+ // const addr_t data_addr = m_memory_addr + H.p_offset; // Not correct
for
labath wrote:
It might be less tempting to use p_offset if this were written as
```
addr_t load_bias = m_memory_addr - base_file_addr;
data_addr = h.p_vaddr + load_bias;
```
https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)
@@ -2550,6 +2550,21 @@ ModuleSP Process::ReadModuleFromMemory(const FileSpec
&file_spec,
}
ModuleSP module_sp(new Module(file_spec, ArchSpec()));
if (module_sp) {
+if (size_to_read == 0) {
+ // Default to 8192 in case we can't find a memory region.
+ size_to_read = 0x2000;
+ MemoryRegionInfo range_info;
+ Status error(GetMemoryRegionInfo(header_addr, range_info));
+ if (error.Success()) {
+// We found a memory region, set the range of bytes ro read to read to
+// the end of the memory region. This should be enough to contain the
+// file header and important bits.
+const auto &range = range_info.GetRange();
labath wrote:
Depending on how the file is linked (`-z noseparate-code`) the first region
could end up containing all of the code in the binary (and so be arbitrarily
big), it might not be a good idea to read all of it. What's the reason for this
change? Could that be a separate patch as well? (I think we can test DT_DYNAMIC
parsing without actually loading the file from memory)
For reference, this is what I get with a hello world binary built like this:
```
$ clang++ -o /tmp/a.out -x c++ - -static <<<"int main(){}"
-Wl,-z,noseparate-code && llvm-readelf -e /tmp/a.out | grep -e LOAD -B 1
Type Offset VirtAddr PhysAddr FileSiz MemSiz
Flg Align
LOAD 0x00 0x0040 0x0040 0x09caf6
0x09caf6 R E 0x1000
LOAD 0x09ceb0 0x0049deb0 0x0049deb0 0x005b58
0x00b2b8 RW 0x1000
```
https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Impove ObjectFileELF's .dynamic parsing and usage. (PR #101237)
@@ -402,6 +413,29 @@ class ObjectFileELF : public lldb_private::ObjectFile {
/// .gnu_debugdata section or \c nullptr if an error occured or if there's no
/// section with that name.
std::shared_ptr GetGnuDebugDataObjectFile();
+
+ /// Get the bytes that represent the .dynamic section.
+ ///
+ /// This function will fetch the data for the .dynamic section in an ELF
file.
+ /// If the ELF file is loaded from a file on disk, it will use the PT_DYNAMIC
+ /// program header to extract the data and fall back to using the section
+ /// headers. If the ELF file is loaded from memory, it will use the
PT_DYNAMIC
+ /// program header to get the information.
+ ///
+ /// \return The bytes that represent the string table data or \c std::nullopt
+ /// if an error occured.
+ std::optional GetDynamicData();
labath wrote:
Do we need to distinguish between an empty dynamic header, and one that was not
present (or there was some other error in retrieving it). I.e., what do you
think about returning an empty DataExtractor in case of an error?
https://github.com/llvm/llvm-project/pull/101237
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Unify the way we get the Target in CommandObject (PR #101208)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/101208 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
slydiman wrote: @labath > For consistency, and to minimize the number of ifdefs, I believe windows and > non-windows paths should use as similar approaches as possible. That means I > think the end state should be that the posix path also uses a fork+exec > approach. fork+exec on non-Windows OS is redundant. It simply does not make sence. We can use multithreading or CreateProcess on Windows. It seems [the multithreading solution](https://github.com/llvm/llvm-project/pull/100670) faced a lot of issues and system's bugs on Linux. So CreateProcess is the only way. But it is the fork() workaround for Windows. > For me, it also seems natural that this should be the first step. While doing > that, we can make sure to create abstractions that can be used to implement > the windows behavior as well. I'm imagining some sort of a function or a > class, which takes the target binary, its arguments, and a socket, and > launches that binary with that socket. (we'll probably also need a similar > function/class on the other end to receive that socket). Look at GDBRemoteCommunication::StartDebugserverProcess(). I planed to extend it to use `pass_comm_fd` of Windows in the part 2. It is hard to create abstractions because of a lot of parameters. We can move most of SpawnProcessChild() code to `ConnectionFileDescriptor::ConnectFD` in ConnectionFileDescriptorPosix.cpp BTW, I don't like #ifdef _WIN32 in *Posix.cpp files. But currently the common ConnectionFileDescriptor.h contains `#include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"` > I'm not sure if we really need the WriteWithTimeout functionality. The reason > we don't have it so far is that pipes have a buffer, and unless we're writing > a huge chunk of data, the write will never block anyway. There's nothing > wrong with it in principle though, and if you do want to have it, it'd be > best if it comes with in its own patch and with a test.) Before this patch the pipe read/write 1 byte in ConnectionFileDescriptor::CommandPipe. The maximal size was 6 bytes (the port value) in GDBRemoteCommunication::StartDebugserverProcess() and it is only the place where ReadWithTimeout() is used. But WSAPROTOCOL_INFO is big enough (628 bytes). Usually the pipe buffer is 4K. But we need an ability to avoid hangs anyway. I agree that all pipe related changes may be moved to another patch. And we need to add WriteWithTimeout() to PipePosix too. Note there is not any test for Pipe now. And I have no idea how to test it, especially async version. Running `lldb-server platform --server` on Windows is the best test. It must be tested with a high load with multiple connections. Otherwise any pipe tests are useles. https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed lldb-server crash (TestLogHandler was not thread safe) (PR #101326)
https://github.com/slydiman created
https://github.com/llvm/llvm-project/pull/101326
Host::LaunchProcess() requires to SetMonitorProcessCallback. This callback is
called from the child process monitor thread. We cannot control this thread
anyway. lldb-server may crash if there is a logging around this callback
because TestLogHandler is not thread safe. I faced this issue debugging 100
simultaneous child processes. Note StreamLogHandler::Emit() in
lldb/source/Utility/Log.cpp already contains the similar mutex.
>From 113340676cfd90867a272db0ba78f1385f00cb4c Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev
Date: Wed, 31 Jul 2024 16:33:13 +0400
Subject: [PATCH] [lldb] Fixed lldb-server crash (TestLogHandler was not thread
safe)
Host::LaunchProcess() requires to SetMonitorProcessCallback. This callback is
called from the child process monitor thread. We cannot control this thread
anyway. lldb-server may crash if there is a logging around this callback
because TestLogHandler is not thread safe. I faced this issue debugging 100
simultaneous child processes. Note StreamLogHandler::Emit() in
lldb/source/Utility/Log.cpp already contains the similar mutex.
---
lldb/tools/lldb-server/LLDBServerUtilities.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lldb/tools/lldb-server/LLDBServerUtilities.cpp
b/lldb/tools/lldb-server/LLDBServerUtilities.cpp
index c3a8df19e969e..5facfbf3105e9 100644
--- a/lldb/tools/lldb-server/LLDBServerUtilities.cpp
+++ b/lldb/tools/lldb-server/LLDBServerUtilities.cpp
@@ -27,11 +27,13 @@ class TestLogHandler : public LogHandler {
: m_stream_sp(stream_sp) {}
void Emit(llvm::StringRef message) override {
+std::lock_guard guard(m_mutex);
(*m_stream_sp) << message;
m_stream_sp->flush();
}
private:
+ std::mutex m_mutex;
std::shared_ptr m_stream_sp;
};
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed lldb-server crash (TestLogHandler was not thread safe) (PR #101326)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Dmitry Vasilyev (slydiman)
Changes
Host::LaunchProcess() requires to SetMonitorProcessCallback. This callback is
called from the child process monitor thread. We cannot control this thread
anyway. lldb-server may crash if there is a logging around this callback
because TestLogHandler is not thread safe. I faced this issue debugging 100
simultaneous child processes. Note StreamLogHandler::Emit() in
lldb/source/Utility/Log.cpp already contains the similar mutex.
---
Full diff: https://github.com/llvm/llvm-project/pull/101326.diff
1 Files Affected:
- (modified) lldb/tools/lldb-server/LLDBServerUtilities.cpp (+2)
``diff
diff --git a/lldb/tools/lldb-server/LLDBServerUtilities.cpp
b/lldb/tools/lldb-server/LLDBServerUtilities.cpp
index c3a8df19e969e..5facfbf3105e9 100644
--- a/lldb/tools/lldb-server/LLDBServerUtilities.cpp
+++ b/lldb/tools/lldb-server/LLDBServerUtilities.cpp
@@ -27,11 +27,13 @@ class TestLogHandler : public LogHandler {
: m_stream_sp(stream_sp) {}
void Emit(llvm::StringRef message) override {
+std::lock_guard guard(m_mutex);
(*m_stream_sp) << message;
m_stream_sp->flush();
}
private:
+ std::mutex m_mutex;
std::shared_ptr m_stream_sp;
};
``
https://github.com/llvm/llvm-project/pull/101326
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Create VLAs of explicitly 0-size as ConstantArrayType (PR #100710)
Michael137 wrote:
> @Michael137 This broke the windows bot:
> https://lab.llvm.org/buildbot/#/builders/141/builds/1214 Should we skip it on
> that platform or do you see an easy fix ?
Thanks for pinging. This is the failure:
```
| error: no variable named 'f' found in this frame
# `-
# executed command:
'c:\users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\filecheck.exe'
'C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\Shell\SymbolFile\DWARF\vla.cpp'
# .---command stderr
# |
C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\Shell\SymbolFile\DWARF\vla.cpp:26:17:
error: CHECK-NEXT: expected string not found in input
# | // CHECK-NEXT: (Foo) f = {
# | ^
# | :16:32: note: scanning from here
# | (lldb) frame var --show-types f
# |^
# |
# | Input file:
# | Check file:
C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\Shell\SymbolFile\DWARF\vla.cpp
# |
# | -dump-input=help explains the following input dump.
# |
# | Input was:
# | <<
# | .
# | .
# | .
# | 11: vla.cpp.tmp`main:
# | 12: -> 0x7ff66e41eb18 <+396>: ldur x8, [x29, #-0x28]
# | 13: 0x7ff66e41eb1c <+400>: mov sp, x8
# | 14: 0x7ff66e41eb20 <+404>: ldur w0, [x29, #-0x4]
# | 15: 0x7ff66e41eb24 <+408>: mov sp, x29
# | 16: (lldb) frame var --show-types f
# | next:26X error: no match found
# | >>
# `-
```
Looks strange. Not sure why it's not finding the symbols properly.
I'm tempted to skip this on Windows since I don't have an LLDB setup to test
this on. @DavidSpickett Am I remembering correctly that you might have an
AArch64 Windows setup? If so, does this reproduce for you? I'm confused as to
why the debug symbols don't seem to be found.
https://github.com/llvm/llvm-project/pull/100710
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman updated
https://github.com/llvm/llvm-project/pull/101283
>From c785238cf366746a383e74a89be7c7431cd41d3c Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev
Date: Wed, 31 Jul 2024 05:41:21 +0400
Subject: [PATCH] [lldb] Updated lldb-server to spawn the child process and
share socket on Windows
`lldb-server platform --server` works on Windows now w/o multithreading. The
rest functionality remains unchanged.
Added also PipeWindows::WriteWithTimeout(), fixed
PipeWindows::ReadWithTimeout() and missing initialization of
m_read_overlapped.hEvent in the constructor PipeWindows(lldb::pipe_t read,
lldb::pipe_t write).
Fixes #90923, fixes #56346.
Depends on #101326.
This is the part 1 of the replacement of #100670.
In the part 2 I plan to switch `lldb-server gdbserver` to use `--fd` and listen
a common gdb port for all gdbserver connections. Then we can remove gdb port
mapping to fix #97537.
---
lldb/include/lldb/Host/windows/PipeWindows.h | 3 +
lldb/source/Host/windows/PipeWindows.cpp | 69 +++-
lldb/tools/lldb-server/lldb-platform.cpp | 347 ---
3 files changed, 370 insertions(+), 49 deletions(-)
diff --git a/lldb/include/lldb/Host/windows/PipeWindows.h
b/lldb/include/lldb/Host/windows/PipeWindows.h
index 4b5be28d7ae6c..c99cf52be46c1 100644
--- a/lldb/include/lldb/Host/windows/PipeWindows.h
+++ b/lldb/include/lldb/Host/windows/PipeWindows.h
@@ -61,6 +61,9 @@ class PipeWindows : public PipeBase {
Status Delete(llvm::StringRef name) override;
Status Write(const void *buf, size_t size, size_t &bytes_written) override;
+ Status WriteWithTimeout(const void *buf, size_t size,
+ const std::chrono::microseconds &timeout,
+ size_t &bytes_written);
Status ReadWithTimeout(void *buf, size_t size,
const std::chrono::microseconds &timeout,
size_t &bytes_read) override;
diff --git a/lldb/source/Host/windows/PipeWindows.cpp
b/lldb/source/Host/windows/PipeWindows.cpp
index c82c919607b5b..8f86c7e28046e 100644
--- a/lldb/source/Host/windows/PipeWindows.cpp
+++ b/lldb/source/Host/windows/PipeWindows.cpp
@@ -58,7 +58,10 @@ PipeWindows::PipeWindows(pipe_t read, pipe_t write)
}
ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped));
+ m_read_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr);
+
ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
+ m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr);
}
PipeWindows::~PipeWindows() { Close(); }
@@ -77,6 +80,7 @@ Status PipeWindows::CreateNew(bool child_process_inherit) {
m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY);
ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
+ m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr);
return Status();
}
@@ -202,6 +206,7 @@ Status PipeWindows::OpenNamedPipe(llvm::StringRef name,
m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY);
ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
+m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr);
}
return Status();
@@ -228,6 +233,8 @@ int PipeWindows::ReleaseWriteFileDescriptor() {
return PipeWindows::kInvalidDescriptor;
int result = m_write_fd;
m_write_fd = PipeWindows::kInvalidDescriptor;
+ if (m_write_overlapped.hEvent)
+::CloseHandle(m_write_overlapped.hEvent);
m_write = INVALID_HANDLE_VALUE;
ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
return result;
@@ -250,6 +257,9 @@ void PipeWindows::CloseWriteFileDescriptor() {
if (!CanWrite())
return;
+ if (m_write_overlapped.hEvent)
+::CloseHandle(m_write_overlapped.hEvent);
+
_close(m_write_fd);
m_write = INVALID_HANDLE_VALUE;
m_write_fd = PipeWindows::kInvalidDescriptor;
@@ -283,7 +293,12 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t size,
DWORD sys_bytes_read = size;
BOOL result = ::ReadFile(m_read, buf, sys_bytes_read, &sys_bytes_read,
&m_read_overlapped);
- if (!result && GetLastError() != ERROR_IO_PENDING)
+ if (result) {
+bytes_read = sys_bytes_read;
+return Status();
+ }
+
+ if (GetLastError() != ERROR_IO_PENDING)
return Status(::GetLastError(), eErrorTypeWin32);
DWORD timeout = (duration == std::chrono::microseconds::zero())
@@ -319,18 +334,56 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t
size,
Status PipeWindows::Write(const void *buf, size_t num_bytes,
size_t &bytes_written) {
+ return WriteWithTimeout(buf, num_bytes, std::chrono::microseconds::zero(),
+ bytes_written);
+}
+
+Status PipeWindows::WriteWithTimeout(const void *buf, size_t size,
+ const std::chrono::microseconds &duration,
+ size_t &bytes_written) {
if (!CanWrit
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman updated
https://github.com/llvm/llvm-project/pull/101283
>From add315c8db91c4d51f9b298cc64478c0497857a5 Mon Sep 17 00:00:00 2001
From: Dmitry Vasilyev
Date: Wed, 31 Jul 2024 05:41:21 +0400
Subject: [PATCH] [lldb] Updated lldb-server to spawn the child process and
share socket on Windows
`lldb-server platform --server` works on Windows now w/o multithreading. The
rest functionality remains unchanged.
Added also PipeWindows::WriteWithTimeout(), fixed
PipeWindows::ReadWithTimeout() and missing initialization of
m_read_overlapped.hEvent in the constructor PipeWindows(lldb::pipe_t read,
lldb::pipe_t write).
Fixes #90923, fixes #56346.
Depends on #101326.
This is the part 1 of the replacement of #100670.
In the part 2 I plan to switch `lldb-server gdbserver` to use `--fd` and listen
a common gdb port for all gdbserver connections. Then we can remove gdb port
mapping to fix #97537.
---
lldb/include/lldb/Host/windows/PipeWindows.h | 3 +
lldb/source/Host/windows/PipeWindows.cpp | 69 +++-
lldb/tools/lldb-server/lldb-platform.cpp | 347 ---
3 files changed, 370 insertions(+), 49 deletions(-)
diff --git a/lldb/include/lldb/Host/windows/PipeWindows.h
b/lldb/include/lldb/Host/windows/PipeWindows.h
index 4b5be28d7ae6c..c99cf52be46c1 100644
--- a/lldb/include/lldb/Host/windows/PipeWindows.h
+++ b/lldb/include/lldb/Host/windows/PipeWindows.h
@@ -61,6 +61,9 @@ class PipeWindows : public PipeBase {
Status Delete(llvm::StringRef name) override;
Status Write(const void *buf, size_t size, size_t &bytes_written) override;
+ Status WriteWithTimeout(const void *buf, size_t size,
+ const std::chrono::microseconds &timeout,
+ size_t &bytes_written);
Status ReadWithTimeout(void *buf, size_t size,
const std::chrono::microseconds &timeout,
size_t &bytes_read) override;
diff --git a/lldb/source/Host/windows/PipeWindows.cpp
b/lldb/source/Host/windows/PipeWindows.cpp
index c82c919607b5b..8f86c7e28046e 100644
--- a/lldb/source/Host/windows/PipeWindows.cpp
+++ b/lldb/source/Host/windows/PipeWindows.cpp
@@ -58,7 +58,10 @@ PipeWindows::PipeWindows(pipe_t read, pipe_t write)
}
ZeroMemory(&m_read_overlapped, sizeof(m_read_overlapped));
+ m_read_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr);
+
ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
+ m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr);
}
PipeWindows::~PipeWindows() { Close(); }
@@ -77,6 +80,7 @@ Status PipeWindows::CreateNew(bool child_process_inherit) {
m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY);
ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
+ m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr);
return Status();
}
@@ -202,6 +206,7 @@ Status PipeWindows::OpenNamedPipe(llvm::StringRef name,
m_write_fd = _open_osfhandle((intptr_t)m_write, _O_WRONLY);
ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
+m_write_overlapped.hEvent = ::CreateEventA(nullptr, TRUE, FALSE, nullptr);
}
return Status();
@@ -228,6 +233,8 @@ int PipeWindows::ReleaseWriteFileDescriptor() {
return PipeWindows::kInvalidDescriptor;
int result = m_write_fd;
m_write_fd = PipeWindows::kInvalidDescriptor;
+ if (m_write_overlapped.hEvent)
+::CloseHandle(m_write_overlapped.hEvent);
m_write = INVALID_HANDLE_VALUE;
ZeroMemory(&m_write_overlapped, sizeof(m_write_overlapped));
return result;
@@ -250,6 +257,9 @@ void PipeWindows::CloseWriteFileDescriptor() {
if (!CanWrite())
return;
+ if (m_write_overlapped.hEvent)
+::CloseHandle(m_write_overlapped.hEvent);
+
_close(m_write_fd);
m_write = INVALID_HANDLE_VALUE;
m_write_fd = PipeWindows::kInvalidDescriptor;
@@ -283,7 +293,12 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t size,
DWORD sys_bytes_read = size;
BOOL result = ::ReadFile(m_read, buf, sys_bytes_read, &sys_bytes_read,
&m_read_overlapped);
- if (!result && GetLastError() != ERROR_IO_PENDING)
+ if (result) {
+bytes_read = sys_bytes_read;
+return Status();
+ }
+
+ if (GetLastError() != ERROR_IO_PENDING)
return Status(::GetLastError(), eErrorTypeWin32);
DWORD timeout = (duration == std::chrono::microseconds::zero())
@@ -319,18 +334,56 @@ Status PipeWindows::ReadWithTimeout(void *buf, size_t
size,
Status PipeWindows::Write(const void *buf, size_t num_bytes,
size_t &bytes_written) {
+ return WriteWithTimeout(buf, num_bytes, std::chrono::microseconds::zero(),
+ bytes_written);
+}
+
+Status PipeWindows::WriteWithTimeout(const void *buf, size_t size,
+ const std::chrono::microseconds &duration,
+ size_t &bytes_written) {
if (!CanWrit
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff c35c4c72e4977258fc1da940e0470e8d0671bf07
c785238cf366746a383e74a89be7c7431cd41d3c --extensions cpp,h --
lldb/include/lldb/Host/windows/PipeWindows.h
lldb/source/Host/windows/PipeWindows.cpp
lldb/tools/lldb-server/lldb-platform.cpp
``
View the diff from clang-format here.
``diff
diff --git a/lldb/tools/lldb-server/lldb-platform.cpp
b/lldb/tools/lldb-server/lldb-platform.cpp
index b0059055bd..e23237ef57 100644
--- a/lldb/tools/lldb-server/lldb-platform.cpp
+++ b/lldb/tools/lldb-server/lldb-platform.cpp
@@ -121,7 +121,7 @@ static Status save_socket_id_to_file(const std::string
&socket_id,
}
static void client_handle(GDBRemoteCommunicationServerPlatform &platform,
- const lldb_private::Args &args) {
+ const lldb_private::Args &args) {
if (!platform.IsConnected())
return;
``
https://github.com/llvm/llvm-project/pull/101283
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
labath wrote: > @labath > > > For consistency, and to minimize the number of ifdefs, I believe windows > > and non-windows paths should use as similar approaches as possible. That > > means I think the end state should be that the posix path also uses a > > fork+exec approach. > > fork+exec on non-Windows OS is redundant. It simply does not make sence. We > can use multithreading or CreateProcess on Windows. It seems [the > multithreading solution](https://github.com/llvm/llvm-project/pull/100670) > faced a lot of issues and system's bugs on Linux. So CreateProcess is the > only way. But it is the fork() workaround for Windows. This is where I'll have to disagree with you. The exec following the fork is not "redundant" on linux. A naked fork is an extremely dangerous thing in todays (multithreaded) applications. I'm absolutely certain that the improper use of fork with threads (and the FD leaks that go along with it) is the cause of at least one of the issues (ETXTBSY) you refer to as "system bugs", and fairly sure it's also the cause of the other one (the pipe workaround) as well. In a multithreaded application (which lldb-platform isn't right now, but more of an accident than design, and which you've tried to make extremely multithreaded) the only safe use of fork is if it is immediately followed by an exec, and this has to be done in an extremely careful way (we try to do that, and we still failed). But even if it weren't for all of this, I'd still think it makes sense to start a new process, just for symmetry with windows. That's the nature of portable programming. > > > For me, it also seems natural that this should be the first step. While > > doing that, we can make sure to create abstractions that can be used to > > implement the windows behavior as well. I'm imagining some sort of a > > function or a class, which takes the target binary, its arguments, and a > > socket, and launches that binary with that socket. (we'll probably also > > need a similar function/class on the other end to receive that socket). > > Look at GDBRemoteCommunication::StartDebugserverProcess(). I planed to extend > it to use `pass_comm_fd` of Windows in the part 2. It is hard to create > abstractions because of a lot of parameters. We can move most of > SpawnProcessChild() code to `ConnectionFileDescriptor::ConnectFD` in > ConnectionFileDescriptorPosix.cpp BTW, I don't like #ifdef _WIN32 in > *Posix.cpp files. But currently the common ConnectionFileDescriptor.h > contains just `#include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"` I didn't say it was going to be easy. :) I can look at this closer when I get back next week, but I definitely think we can and should come up with a solution involving less ifdefs. > > > I'm not sure if we really need the WriteWithTimeout functionality. The > > reason we don't have it so far is that pipes have a buffer, and unless > > we're writing a huge chunk of data, the write will never block anyway. > > There's nothing wrong with it in principle though, and if you do want to > > have it, it'd be best if it comes with in its own patch and with a test.) > > Before this patch the pipe read/write 1 byte in > ConnectionFileDescriptor::CommandPipe. The maximal size was 6 bytes (the port > value) in GDBRemoteCommunication::StartDebugserverProcess() and it is only > the place where ReadWithTimeout() is used. But WSAPROTOCOL_INFO is big enough > (628 bytes). Usually the pipe buffer is 4K. 628 is still less that 4k. Can the buffer be smaller than that? > But we need an ability to avoid hangs anyway. I agree that all pipe related > changes may be moved to another patch. And we need to add WriteWithTimeout() > to PipePosix too. Note there is no any test for Pipe now. And I have no idea > how to test it, especially async version. Running `lldb-server platform > --server` on Windows is the best test. It must be tested with a high load > with multiple connections. Otherwise any pipe tests are useles. We have one and a half pipe tests (one of them claims to be broken on windows). I wrote them when fixing some Pipe bugs. We should definitely have more. I wholeheartedly disagree with the assertion that the test is useless unless it uses multiple threads and high load. Even the most simple test serves as documents the correct/expected API usage and proves that the functionality works at least in the simple cases. For something like `WriteWithTimeout` I'd probably create single threaded test which does something like this: - write to the pipe until it is full - try to write with a larger timeout and check that it actually times out - read from the pipe and check that you got back what was written That will cover most of the codepaths and obvious ways it can go wrong (e.g. during refactoring). Then, if you wanted to be fancy, you could do things like start a write with a long timeout and then try to read from the pipe, and see that ca
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
@@ -114,6 +120,218 @@ static Status save_socket_id_to_file(const std::string
&socket_id,
return status;
}
+static GDBRemoteCommunicationServerPlatform::PortMap gdbserver_portmap;
+static std::mutex gdbserver_portmap_mutex;
+
+#if defined(_WIN32)
+static void SpawnProcessReaped(lldb::pid_t pid, int signal, int status) {
+ std::lock_guard guard(gdbserver_portmap_mutex);
+ gdbserver_portmap.FreePortForProcess(pid);
+}
+
+static bool SpawnProcessParent(const char *progname, Connection *conn,
+ uint16_t gdb_port, uint16_t port_offset,
+ const lldb_private::Args &args,
+ const std::string &log_file,
+ const StringRef log_channels) {
+ Log *log = GetLog(LLDBLog::Platform);
+ Pipe socket_pipe;
+ Status error = socket_pipe.CreateNew(true);
+ if (error.Fail()) {
+LLDB_LOGF(log,
+ "lldb-platform parent: "
+ "cannot create pipe: %s",
+ error.AsCString());
+return false;
+ }
+
+ ProcessLaunchInfo launch_info;
+ FileSpec self_spec(progname, FileSpec::Style::native);
+ launch_info.SetExecutableFile(self_spec, true);
+ Args &self_args = launch_info.GetArguments();
+ self_args.AppendArgument(llvm::StringRef("platform"));
+ self_args.AppendArgument(llvm::StringRef("--accept"));
+ self_args.AppendArgument(llvm::to_string(socket_pipe.GetReadPipe()));
+ if (gdb_port) {
+self_args.AppendArgument(llvm::StringRef("--gdbserver-port"));
+self_args.AppendArgument(llvm::to_string(gdb_port));
+ }
+ if (port_offset > 0) {
+self_args.AppendArgument(llvm::StringRef("--port-offset"));
+self_args.AppendArgument(llvm::to_string(port_offset));
+ }
+ if (!log_file.empty()) {
+self_args.AppendArgument(llvm::StringRef("--log-file"));
+self_args.AppendArgument(log_file);
+ }
+ if (!log_channels.empty()) {
+self_args.AppendArgument(llvm::StringRef("--log-channels"));
+self_args.AppendArgument(log_channels);
+ }
+ if (args.GetArgumentCount() > 0) {
+self_args.AppendArgument("--");
+self_args.AppendArguments(args);
+ }
+
+ launch_info.SetLaunchInSeparateProcessGroup(false);
+ launch_info.SetMonitorProcessCallback(&SpawnProcessReaped);
+
+ // Copy the current environment.
+ // WSASocket(FROM_PROTOCOL_INFO) will fail in the child process
+ // with the error WSAEPROVIDERFAILEDINIT if the SystemRoot is missing
+ // in the environment.
+ launch_info.GetEnvironment() = Host::GetEnvironment();
+
+ launch_info.GetFlags().Set(eLaunchFlagDisableSTDIO);
+
+ launch_info.AppendCloseFileAction(socket_pipe.GetWriteFileDescriptor());
+
+ // Close STDIN, STDOUT and STDERR.
+ launch_info.AppendCloseFileAction(STDIN_FILENO);
+ launch_info.AppendCloseFileAction(STDOUT_FILENO);
+ launch_info.AppendCloseFileAction(STDERR_FILENO);
+
+ // Redirect STDIN, STDOUT and STDERR to "/dev/null".
+ launch_info.AppendSuppressFileAction(STDIN_FILENO, true, false);
+ launch_info.AppendSuppressFileAction(STDOUT_FILENO, false, true);
+ launch_info.AppendSuppressFileAction(STDERR_FILENO, false, true);
+
+ std::string cmd;
+ self_args.GetCommandString(cmd);
+
+ error = Host::LaunchProcess(launch_info);
+ if (error.Fail()) {
+LLDB_LOGF(log,
+ "lldb-platform parent: "
+ "cannot launch child process for connection: %s",
+ error.AsCString());
+return false;
+ }
+
+ lldb::pid_t childPid = launch_info.GetProcessID();
+ if (childPid == LLDB_INVALID_PROCESS_ID) {
+LLDB_LOGF(log, "lldb-platform parent: "
+ "cannot launch child process for connection: invalid pid");
+return false;
+ }
+ LLDB_LOGF(log,
+"lldb-platform parent: "
+"launched '%s', pid=0x%x",
+cmd.c_str(), childPid);
+
+ {
+std::lock_guard guard(gdbserver_portmap_mutex);
+gdbserver_portmap.AssociatePortWithProcess(gdb_port, childPid);
+ }
+
+ if (socket_pipe.CanRead())
+socket_pipe.CloseReadFileDescriptor();
+ if (!socket_pipe.CanWrite()) {
+LLDB_LOGF(log, "lldb-platform parent: "
+ "cannot write to socket_pipe");
+Host::Kill(childPid, SIGTERM);
+return false;
+ }
+
+ const TCPSocket &socket =
+ static_cast(*conn->GetReadObject());
+ NativeSocket nativeSocket = socket.GetNativeSocket();
+
+ WSAPROTOCOL_INFO protocol_info;
+ if (::WSADuplicateSocket(nativeSocket, childPid, &protocol_info) ==
+ SOCKET_ERROR) {
+LLDB_LOGF(log,
+ "lldb-platform parent: "
+ "WSADuplicateSocket() failed, error: %d",
+ ::WSAGetLastError());
+Host::Kill(childPid, SIGTERM);
+return false;
+ }
+
+ size_t num_bytes;
+ error = socket_pipe.WriteWithTimeout(&protocol_info, sizeof(protocol_info),
+ std::chrono::seconds(2), num_bytes);
+ if (error.Fail()) {
+LLDB_LOGF(log,
+ "lldb-platform parent: "
+ "s
[Lldb-commits] [lldb] [lldb] Fixed lldb-server crash (TestLogHandler was not thread safe) (PR #101326)
https://github.com/labath approved this pull request. This makes sense, though it would probably be even better to just use the real StreamLogHandler class. I think all that's needed would be to add a new constructor to it. https://github.com/llvm/llvm-project/pull/101326 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Create VLAs of explicitly 0-size as ConstantArrayType (PR #100710)
DavidSpickett wrote: Linaro does yes, I think @omjavaid is currently dealing with that bot. https://github.com/llvm/llvm-project/pull/100710 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 93fecc2 - [lldb] Fixed lldb-server crash (TestLogHandler was not thread safe) (#101326)
Author: Dmitry Vasilyev
Date: 2024-07-31T17:51:06+04:00
New Revision: 93fecc2577ece0329f3bbe2719bbc5b4b9b30010
URL:
https://github.com/llvm/llvm-project/commit/93fecc2577ece0329f3bbe2719bbc5b4b9b30010
DIFF:
https://github.com/llvm/llvm-project/commit/93fecc2577ece0329f3bbe2719bbc5b4b9b30010.diff
LOG: [lldb] Fixed lldb-server crash (TestLogHandler was not thread safe)
(#101326)
Host::LaunchProcess() requires to SetMonitorProcessCallback. This
callback is called from the child process monitor thread. We cannot
control this thread anyway. lldb-server may crash if there is a logging
around this callback because TestLogHandler is not thread safe. I faced
this issue debugging 100 simultaneous child processes. Note
StreamLogHandler::Emit() in lldb/source/Utility/Log.cpp already contains
the similar mutex.
Added:
Modified:
lldb/tools/lldb-server/LLDBServerUtilities.cpp
Removed:
diff --git a/lldb/tools/lldb-server/LLDBServerUtilities.cpp
b/lldb/tools/lldb-server/LLDBServerUtilities.cpp
index c3a8df19e969e..5facfbf3105e9 100644
--- a/lldb/tools/lldb-server/LLDBServerUtilities.cpp
+++ b/lldb/tools/lldb-server/LLDBServerUtilities.cpp
@@ -27,11 +27,13 @@ class TestLogHandler : public LogHandler {
: m_stream_sp(stream_sp) {}
void Emit(llvm::StringRef message) override {
+std::lock_guard guard(m_mutex);
(*m_stream_sp) << message;
m_stream_sp->flush();
}
private:
+ std::mutex m_mutex;
std::shared_ptr m_stream_sp;
};
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fixed lldb-server crash (TestLogHandler was not thread safe) (PR #101326)
https://github.com/slydiman closed https://github.com/llvm/llvm-project/pull/101326 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Updated lldb-server to spawn the child process and share socket on Windows (PR #101283)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/101283 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Refactor TypeQuery::ContextMatches, take 2 (PR #101333)
https://github.com/labath created
https://github.com/llvm/llvm-project/pull/101333
This is an alternative, much simpler implementation of #99305. In this version
I replace the AnyModule wildcard match with a special TypeQuery flag which
achieves (mostly) the same thing.
It is a preparatory step for teaching ContextMatches about anonymous
namespaces. It started out as a way to remove the assumption that the pattern
and target contexts must be of the same length -- that's will not be correct
with anonymous namespaces, and probably isn't even correct right now for
AnyModule matches.
>From e87b2b24cd673584aabd00eaf6ad8fc4c0c52c98 Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Tue, 16 Jul 2024 14:18:27 +
Subject: [PATCH] [lldb] Refactor TypeQuery::ContextMatches, take 2
This is an alternative, much simpler implementation of #99305. In this
version I replace the AnyModule wildcard match with a special TypeQuery
flag which achieves (mostly) the same thing.
It is a preparatory step for teaching ContextMatches about anonymous
namespaces. It started out as a way to remove the assumption that the
pattern and target contexts must be of the same length -- that's will
not be correct with anonymous namespaces, and probably isn't even
correct right now for AnyModule matches.
---
lldb/include/lldb/Symbol/Type.h | 13 +--
lldb/include/lldb/lldb-private-enumerations.h | 2 -
lldb/source/Symbol/Type.cpp | 74 ++
.../DWARF/clang-gmodules-type-lookup.c| 5 +-
.../SymbolFile/DWARF/x86/compilercontext.ll | 7 +-
lldb/tools/lldb-test/lldb-test.cpp| 18 +++-
lldb/unittests/Symbol/TestType.cpp| 97 ---
7 files changed, 119 insertions(+), 97 deletions(-)
diff --git a/lldb/include/lldb/Symbol/Type.h b/lldb/include/lldb/Symbol/Type.h
index c6f30cde81867..a35f9974fa39a 100644
--- a/lldb/include/lldb/Symbol/Type.h
+++ b/lldb/include/lldb/Symbol/Type.h
@@ -65,11 +65,6 @@ struct CompilerContext {
llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
const CompilerContext &rhs);
-/// Match \p context_chain against \p pattern, which may contain "Any"
-/// kinds. The \p context_chain should *not* contain any "Any" kinds.
-bool contextMatches(llvm::ArrayRef context_chain,
-llvm::ArrayRef pattern);
-
FLAGS_ENUM(TypeQueryOptions){
e_none = 0u,
/// If set, TypeQuery::m_context contains an exact context that must match
@@ -79,10 +74,13 @@ FLAGS_ENUM(TypeQueryOptions){
/// If set, TypeQuery::m_context is a clang module compiler context. If not
/// set TypeQuery::m_context is normal type lookup context.
e_module_search = (1u << 1),
+/// If set, the query will ignore all Module entries in the type context,
+/// even for exact matches.
+e_ignore_modules = (1u << 2),
/// When true, the find types call should stop the query as soon as a
single
/// matching type is found. When false, the type query should find all
/// matching types.
-e_find_one = (1u << 2),
+e_find_one = (1u << 3),
};
LLDB_MARK_AS_BITMASK_ENUM(TypeQueryOptions)
@@ -264,6 +262,9 @@ class TypeQuery {
bool LanguageMatches(lldb::LanguageType language) const;
bool GetExactMatch() const { return (m_options & e_exact_match) != 0; }
+
+ bool GetIgnoreModules() const { return (m_options & e_ignore_modules) != 0; }
+
/// The \a m_context can be used in two ways: normal types searching with
/// the context containing a stanadard declaration context for a type, or
/// with the context being more complete for exact matches in clang modules.
diff --git a/lldb/include/lldb/lldb-private-enumerations.h
b/lldb/include/lldb/lldb-private-enumerations.h
index 9d18316dcea25..c24a3538f58da 100644
--- a/lldb/include/lldb/lldb-private-enumerations.h
+++ b/lldb/include/lldb/lldb-private-enumerations.h
@@ -207,8 +207,6 @@ enum class CompilerContextKind : uint16_t {
Builtin = 1 << 10,
Any = 1 << 15,
- /// Match 0..n nested modules.
- AnyModule = Any | Module,
/// Match any type.
AnyType = Any | ClassOrStruct | Union | Enum | Typedef | Builtin,
/// Math any declaration context.
diff --git a/lldb/source/Symbol/Type.cpp b/lldb/source/Symbol/Type.cpp
index e76574795733f..eb321407e3734 100644
--- a/lldb/source/Symbol/Type.cpp
+++ b/lldb/source/Symbol/Type.cpp
@@ -6,7 +6,9 @@
//
//===--===//
+#include
#include
+#include
#include
#include "lldb/Core/Module.h"
@@ -30,6 +32,7 @@
#include "lldb/Target/Process.h"
#include "lldb/Target/Target.h"
#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-private-enumerations.h"
#include "llvm/ADT/StringRef.h"
@@ -43,35 +46,6 @@ llvm::raw_ostream
&lldb_private::operator<<(llvm::raw_ostream &os,
return os << lldb_stream.GetString();
}
-bool lldb_private::contextMatches(llvm::ArrayRef
context_chain,
-
[Lldb-commits] [lldb] [lldb] Refactor TypeQuery::ContextMatches, take 2 (PR #101333)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Pavel Labath (labath)
Changes
This is an alternative, much simpler implementation of #99305. In this
version I replace the AnyModule wildcard match with a special TypeQuery flag
which achieves (mostly) the same thing.
It is a preparatory step for teaching ContextMatches about anonymous
namespaces. It started out as a way to remove the assumption that the pattern
and target contexts must be of the same length -- that's will not be correct
with anonymous namespaces, and probably isn't even correct right now for
AnyModule matches.
---
Full diff: https://github.com/llvm/llvm-project/pull/101333.diff
7 Files Affected:
- (modified) lldb/include/lldb/Symbol/Type.h (+7-6)
- (modified) lldb/include/lldb/lldb-private-enumerations.h (-2)
- (modified) lldb/source/Symbol/Type.cpp (+31-43)
- (modified) lldb/test/Shell/SymbolFile/DWARF/clang-gmodules-type-lookup.c
(+3-2)
- (modified) lldb/test/Shell/SymbolFile/DWARF/x86/compilercontext.ll (+2-5)
- (modified) lldb/tools/lldb-test/lldb-test.cpp (+14-4)
- (modified) lldb/unittests/Symbol/TestType.cpp (+62-35)
``diff
diff --git a/lldb/include/lldb/Symbol/Type.h b/lldb/include/lldb/Symbol/Type.h
index c6f30cde81867..a35f9974fa39a 100644
--- a/lldb/include/lldb/Symbol/Type.h
+++ b/lldb/include/lldb/Symbol/Type.h
@@ -65,11 +65,6 @@ struct CompilerContext {
llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
const CompilerContext &rhs);
-/// Match \p context_chain against \p pattern, which may contain "Any"
-/// kinds. The \p context_chain should *not* contain any "Any" kinds.
-bool contextMatches(llvm::ArrayRef context_chain,
-llvm::ArrayRef pattern);
-
FLAGS_ENUM(TypeQueryOptions){
e_none = 0u,
/// If set, TypeQuery::m_context contains an exact context that must match
@@ -79,10 +74,13 @@ FLAGS_ENUM(TypeQueryOptions){
/// If set, TypeQuery::m_context is a clang module compiler context. If not
/// set TypeQuery::m_context is normal type lookup context.
e_module_search = (1u << 1),
+/// If set, the query will ignore all Module entries in the type context,
+/// even for exact matches.
+e_ignore_modules = (1u << 2),
/// When true, the find types call should stop the query as soon as a
single
/// matching type is found. When false, the type query should find all
/// matching types.
-e_find_one = (1u << 2),
+e_find_one = (1u << 3),
};
LLDB_MARK_AS_BITMASK_ENUM(TypeQueryOptions)
@@ -264,6 +262,9 @@ class TypeQuery {
bool LanguageMatches(lldb::LanguageType language) const;
bool GetExactMatch() const { return (m_options & e_exact_match) != 0; }
+
+ bool GetIgnoreModules() const { return (m_options & e_ignore_modules) != 0; }
+
/// The \a m_context can be used in two ways: normal types searching with
/// the context containing a stanadard declaration context for a type, or
/// with the context being more complete for exact matches in clang modules.
diff --git a/lldb/include/lldb/lldb-private-enumerations.h
b/lldb/include/lldb/lldb-private-enumerations.h
index 9d18316dcea25..c24a3538f58da 100644
--- a/lldb/include/lldb/lldb-private-enumerations.h
+++ b/lldb/include/lldb/lldb-private-enumerations.h
@@ -207,8 +207,6 @@ enum class CompilerContextKind : uint16_t {
Builtin = 1 << 10,
Any = 1 << 15,
- /// Match 0..n nested modules.
- AnyModule = Any | Module,
/// Match any type.
AnyType = Any | ClassOrStruct | Union | Enum | Typedef | Builtin,
/// Math any declaration context.
diff --git a/lldb/source/Symbol/Type.cpp b/lldb/source/Symbol/Type.cpp
index e76574795733f..eb321407e3734 100644
--- a/lldb/source/Symbol/Type.cpp
+++ b/lldb/source/Symbol/Type.cpp
@@ -6,7 +6,9 @@
//
//===--===//
+#include
#include
+#include
#include
#include "lldb/Core/Module.h"
@@ -30,6 +32,7 @@
#include "lldb/Target/Process.h"
#include "lldb/Target/Target.h"
#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-private-enumerations.h"
#include "llvm/ADT/StringRef.h"
@@ -43,35 +46,6 @@ llvm::raw_ostream
&lldb_private::operator<<(llvm::raw_ostream &os,
return os << lldb_stream.GetString();
}
-bool lldb_private::contextMatches(llvm::ArrayRef
context_chain,
- llvm::ArrayRef pattern) {
- auto ctx = context_chain.begin();
- auto ctx_end = context_chain.end();
- for (const CompilerContext &pat : pattern) {
-// Early exit if the pattern is too long.
-if (ctx == ctx_end)
- return false;
-if (*ctx != pat) {
- // Skip any number of module matches.
- if (pat.kind == CompilerContextKind::AnyModule) {
-// Greedily match 0..n modules.
-ctx = std::find_if(ctx, ctx_end, [](const CompilerContext &ctx) {
- return ctx.kind != CompilerContextKind::Module;
-});
-continue;
- }
- // See if
[Lldb-commits] [lldb] [lldb] Refactor TypeQuery::ContextMatches, take 2 (PR #101333)
labath wrote: @Michael137, I'd appreciate it if you could give this a spin. If https://github.com/swiftlang/llvm-project/blob/ee8bc8b8d30eb99807adbcd3c1f044e00af66cdd/lldb/source/Plugins/TypeSystem/Swift/TypeSystemSwiftTypeRef.cpp#L219-L225 is the only use of `AnyModule`, then it should be straight-forward to replace that with a new flag. There's no rush since I'm going to be out for the rest of the week. https://github.com/llvm/llvm-project/pull/101333 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Refactor TypeQuery::ContextMatches, take 2 (PR #101333)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff d8b985c9490664f7ec923e59cf6c603d998179ae
e87b2b24cd673584aabd00eaf6ad8fc4c0c52c98 --extensions cpp,h,c --
lldb/include/lldb/Symbol/Type.h lldb/include/lldb/lldb-private-enumerations.h
lldb/source/Symbol/Type.cpp
lldb/test/Shell/SymbolFile/DWARF/clang-gmodules-type-lookup.c
lldb/tools/lldb-test/lldb-test.cpp lldb/unittests/Symbol/TestType.cpp
``
View the diff from clang-format here.
``diff
diff --git a/lldb/unittests/Symbol/TestType.cpp
b/lldb/unittests/Symbol/TestType.cpp
index 438f141ff3..e4b56b9ff0 100644
--- a/lldb/unittests/Symbol/TestType.cpp
+++ b/lldb/unittests/Symbol/TestType.cpp
@@ -94,7 +94,8 @@ TEST(Type, CompilerContextPattern) {
MatchesIgnoringModules(std::vector{make_module("B"), make_class("C")}));
EXPECT_THAT(
(std::vector{make_module("A"), make_module("B"), make_class("C")}),
- Not(MatchesIgnoringModules(std::vector{make_module("A"),
make_class("C")})));
+ Not(MatchesIgnoringModules(
+ std::vector{make_module("A"), make_class("C")})));
EXPECT_THAT((std::vector{make_module("A"), make_module("B"),
make_enum("C")}),
Matches(std::vector{make_module("A"), make_module("B"),
make_any_type("C")}));
``
https://github.com/llvm/llvm-project/pull/101333
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Refactor TypeQuery::ContextMatches, take 2 (PR #101333)
https://github.com/labath updated
https://github.com/llvm/llvm-project/pull/101333
>From e87b2b24cd673584aabd00eaf6ad8fc4c0c52c98 Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Tue, 16 Jul 2024 14:18:27 +
Subject: [PATCH 1/2] [lldb] Refactor TypeQuery::ContextMatches, take 2
This is an alternative, much simpler implementation of #99305. In this
version I replace the AnyModule wildcard match with a special TypeQuery
flag which achieves (mostly) the same thing.
It is a preparatory step for teaching ContextMatches about anonymous
namespaces. It started out as a way to remove the assumption that the
pattern and target contexts must be of the same length -- that's will
not be correct with anonymous namespaces, and probably isn't even
correct right now for AnyModule matches.
---
lldb/include/lldb/Symbol/Type.h | 13 +--
lldb/include/lldb/lldb-private-enumerations.h | 2 -
lldb/source/Symbol/Type.cpp | 74 ++
.../DWARF/clang-gmodules-type-lookup.c| 5 +-
.../SymbolFile/DWARF/x86/compilercontext.ll | 7 +-
lldb/tools/lldb-test/lldb-test.cpp| 18 +++-
lldb/unittests/Symbol/TestType.cpp| 97 ---
7 files changed, 119 insertions(+), 97 deletions(-)
diff --git a/lldb/include/lldb/Symbol/Type.h b/lldb/include/lldb/Symbol/Type.h
index c6f30cde81867..a35f9974fa39a 100644
--- a/lldb/include/lldb/Symbol/Type.h
+++ b/lldb/include/lldb/Symbol/Type.h
@@ -65,11 +65,6 @@ struct CompilerContext {
llvm::raw_ostream &operator<<(llvm::raw_ostream &os,
const CompilerContext &rhs);
-/// Match \p context_chain against \p pattern, which may contain "Any"
-/// kinds. The \p context_chain should *not* contain any "Any" kinds.
-bool contextMatches(llvm::ArrayRef context_chain,
-llvm::ArrayRef pattern);
-
FLAGS_ENUM(TypeQueryOptions){
e_none = 0u,
/// If set, TypeQuery::m_context contains an exact context that must match
@@ -79,10 +74,13 @@ FLAGS_ENUM(TypeQueryOptions){
/// If set, TypeQuery::m_context is a clang module compiler context. If not
/// set TypeQuery::m_context is normal type lookup context.
e_module_search = (1u << 1),
+/// If set, the query will ignore all Module entries in the type context,
+/// even for exact matches.
+e_ignore_modules = (1u << 2),
/// When true, the find types call should stop the query as soon as a
single
/// matching type is found. When false, the type query should find all
/// matching types.
-e_find_one = (1u << 2),
+e_find_one = (1u << 3),
};
LLDB_MARK_AS_BITMASK_ENUM(TypeQueryOptions)
@@ -264,6 +262,9 @@ class TypeQuery {
bool LanguageMatches(lldb::LanguageType language) const;
bool GetExactMatch() const { return (m_options & e_exact_match) != 0; }
+
+ bool GetIgnoreModules() const { return (m_options & e_ignore_modules) != 0; }
+
/// The \a m_context can be used in two ways: normal types searching with
/// the context containing a stanadard declaration context for a type, or
/// with the context being more complete for exact matches in clang modules.
diff --git a/lldb/include/lldb/lldb-private-enumerations.h
b/lldb/include/lldb/lldb-private-enumerations.h
index 9d18316dcea25..c24a3538f58da 100644
--- a/lldb/include/lldb/lldb-private-enumerations.h
+++ b/lldb/include/lldb/lldb-private-enumerations.h
@@ -207,8 +207,6 @@ enum class CompilerContextKind : uint16_t {
Builtin = 1 << 10,
Any = 1 << 15,
- /// Match 0..n nested modules.
- AnyModule = Any | Module,
/// Match any type.
AnyType = Any | ClassOrStruct | Union | Enum | Typedef | Builtin,
/// Math any declaration context.
diff --git a/lldb/source/Symbol/Type.cpp b/lldb/source/Symbol/Type.cpp
index e76574795733f..eb321407e3734 100644
--- a/lldb/source/Symbol/Type.cpp
+++ b/lldb/source/Symbol/Type.cpp
@@ -6,7 +6,9 @@
//
//===--===//
+#include
#include
+#include
#include
#include "lldb/Core/Module.h"
@@ -30,6 +32,7 @@
#include "lldb/Target/Process.h"
#include "lldb/Target/Target.h"
#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-private-enumerations.h"
#include "llvm/ADT/StringRef.h"
@@ -43,35 +46,6 @@ llvm::raw_ostream
&lldb_private::operator<<(llvm::raw_ostream &os,
return os << lldb_stream.GetString();
}
-bool lldb_private::contextMatches(llvm::ArrayRef
context_chain,
- llvm::ArrayRef pattern) {
- auto ctx = context_chain.begin();
- auto ctx_end = context_chain.end();
- for (const CompilerContext &pat : pattern) {
-// Early exit if the pattern is too long.
-if (ctx == ctx_end)
- return false;
-if (*ctx != pat) {
- // Skip any number of module matches.
- if (pat.kind == CompilerContextKind::AnyModule) {
-// Greedily match 0..n modules.
-ctx = std::find_if(ctx, ctx_end, [](const CompilerContext &ctx) {
-
[Lldb-commits] [lldb] [lldb] Refactor TypeQuery::ContextMatches, take 2 (PR #101333)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/101333 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Refactor TypeQuery::ContextMatches, take 2 (PR #101333)
https://github.com/Michael137 commented: Awesome, this feels much better. Thanks for doing this! I'll give this a try in the Swift plugin this week LGTM (modulo some stylistic comments). Will defer approval until I tried this with Swift https://github.com/llvm/llvm-project/pull/101333 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Refactor TypeQuery::ContextMatches, take 2 (PR #101333)
adrian-prantl wrote: @Michael137 Have you tried applying the patch to swift-lldb and running the tests? https://github.com/llvm/llvm-project/pull/101333 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Refactor TypeQuery::ContextMatches, take 2 (PR #101333)
Michael137 wrote: > @Michael137 Have you tried applying the patch to swift-lldb and running the > tests? Not yet, but planning to do so sometime this week https://github.com/llvm/llvm-project/pull/101333 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -336,3 +336,52 @@ TEST(MinidumpYAML, ExceptionStream_ExtraParameter) {
0xab, 0xad, 0xca, 0xfe}),
*ExpectedContext);
}
+
+TEST(MinidumpYAML, MemoryRegion_64bit) {
+ SmallString<0> Storage;
+ auto ExpectedFile = toBinary(Storage, R"(
+--- !minidump
+Streams:
+ - Type:Memory64List
+Memory Ranges:
+ - Start of Memory Range: 0x7FCF0818283
+Content: '68656c6c6f'
+ - Start of Memory Range: 0x7FFF0818283
+Content: '776f726c64'
+)");
+
+ ASSERT_THAT_EXPECTED(ExpectedFile, Succeeded());
+ object::MinidumpFile &File = **ExpectedFile;
+
+ ASSERT_EQ(1u, File.streams().size());
+
+ Expected> ExpectedMemoryList =
+ File.getMemory64List();
+
+ ASSERT_THAT_EXPECTED(ExpectedMemoryList, Succeeded());
+
+ ArrayRef MemoryList = *ExpectedMemoryList;
+ ASSERT_EQ(2u, MemoryList.size());
+
+ const minidump::MemoryDescriptor_64 &DescOne = MemoryList[0];
+ ASSERT_EQ(0x7FCF0818283u, DescOne.StartOfMemoryRange);
+ ASSERT_EQ(5u, DescOne.DataSize);
+
+ const minidump::MemoryDescriptor_64 &DescTwo = MemoryList[1];
+ ASSERT_EQ(0x7FFF0818283u, DescTwo.StartOfMemoryRange);
+ ASSERT_EQ(5u, DescTwo.DataSize);
+
+ const std::optional> ExpectedContent =
+ File.getRawStream(StreamType::Memory64List);
+ ASSERT_TRUE(ExpectedContent);
+ const size_t ExpectedStreamSize = sizeof(Memory64ListHeader) +
(sizeof(MemoryDescriptor_64) * 2);
+ ASSERT_EQ(ExpectedStreamSize, ExpectedContent->size());
+
+ Expected> DescOneExpectedContentSlice =
File.getRawData(DescOne);
+ ASSERT_THAT_EXPECTED(DescOneExpectedContentSlice, Succeeded());
+ ASSERT_EQ("hello", reinterpret_cast(DescOneExpectedContentSlice->data()));
Jlalond wrote:
Does LLVM prefer the ordering to be `Assert(Expected, Actual)`? Dotnet was very
strict about this and so I'm just curious
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -550,7 +588,7 @@ Stream::create(const Directory &StreamDesc, const
object::MinidumpFile &File) {
llvm_unreachable("Unhandled stream kind!");
}
-Expected Object::create(const object::MinidumpFile &File) {
+Expected Object::create(object::MinidumpFile &File) {
Jlalond wrote:
`fallible_iterator` is really cool! I'll refactor everything over
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [Obj2Yaml] Add support for minidump generation with 64b memory ranges. (PR #101272)
@@ -136,6 +136,22 @@ static size_t layout(BlobAllocator &File,
MinidumpYAML::ExceptionStream &S) {
return DataEnd;
}
+static size_t layout(BlobAllocator &File, MinidumpYAML::Memory64ListStream &S)
{
+ size_t BaseRVA = File.tell() + sizeof(minidump::Memory64ListHeader);
Jlalond wrote:
I agree with this, but because this entire class is on `size_t` is it a good
idea to refactor this one method to return a `uint64_t`? Below in
`layout(BlobAllocator &File, Stream &S)` everything either returns an `size_t`
or void.
If we refactor, I'd convert the local variable `DataEnd` in that layout
function to use `uint64_t` as well.
https://github.com/llvm/llvm-project/pull/101272
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Allow mapping object file paths (PR #101361)
https://github.com/aperez created
https://github.com/llvm/llvm-project/pull/101361
This introduces a `target.object-map` which allows us to remap module
locations, much in the same way as source mapping works today. This is useful,
for instance, when debugging coredumps, so we can replace some of the locations
where LLDB attempts to load shared libraries and executables from, without
having to setup an entire sysroot.
>From 5221c6f267fffd811c6dd39dbbcc211e2a93739c Mon Sep 17 00:00:00 2001
From: Alexandre Perez
Date: Wed, 31 Jul 2024 09:38:38 -0700
Subject: [PATCH] [lldb] Allow mapping object file paths
---
lldb/include/lldb/Target/Target.h | 2 ++
lldb/source/Target/Target.cpp | 21 ++--
lldb/source/Target/TargetProperties.td| 3 +++
.../postmortem/elf-core/TestLinuxCore.py | 24 +++
4 files changed, 48 insertions(+), 2 deletions(-)
diff --git a/lldb/include/lldb/Target/Target.h
b/lldb/include/lldb/Target/Target.h
index 5d5ae1bfcd3bd..119dff4d49819 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -141,6 +141,8 @@ class TargetProperties : public Properties {
PathMappingList &GetSourcePathMap() const;
+ PathMappingList &GetObjectPathMap() const;
+
bool GetAutoSourceMapRelative() const;
FileSpecList GetExecutableSearchPaths();
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index ec0da8a1378a8..129683c43f0c1 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -2155,12 +2155,21 @@ bool Target::ReadPointerFromMemory(const Address &addr,
Status &error,
return false;
}
-ModuleSP Target::GetOrCreateModule(const ModuleSpec &module_spec, bool notify,
- Status *error_ptr) {
+ModuleSP Target::GetOrCreateModule(const ModuleSpec &orig_module_spec,
+ bool notify, Status *error_ptr) {
ModuleSP module_sp;
Status error;
+ // Apply any remappings specified in target.object-map:
+ ModuleSpec module_spec(orig_module_spec);
+ PathMappingList &obj_mapping = GetObjectPathMap();
+ if (std::optional remapped_obj_file =
+ obj_mapping.RemapPath(orig_module_spec.GetFileSpec().GetPath(),
+true /* only_if_exists */)) {
+module_spec.GetFileSpec().SetPath(remapped_obj_file->GetPath());
+ }
+
// First see if we already have this module in our module list. If we do,
// then we're done, we don't need to consult the shared modules list. But
// only do this if we are passed a UUID.
@@ -4459,6 +4468,14 @@ PathMappingList &TargetProperties::GetSourcePathMap()
const {
return option_value->GetCurrentValue();
}
+PathMappingList &TargetProperties::GetObjectPathMap() const {
+ const uint32_t idx = ePropertyObjectMap;
+ OptionValuePathMappings *option_value =
+ m_collection_sp->GetPropertyAtIndexAsOptionValuePathMappings(idx);
+ assert(option_value);
+ return option_value->GetCurrentValue();
+}
+
bool TargetProperties::GetAutoSourceMapRelative() const {
const uint32_t idx = ePropertyAutoSourceMapRelative;
return GetPropertyAtIndexAs(
diff --git a/lldb/source/Target/TargetProperties.td
b/lldb/source/Target/TargetProperties.td
index 7f79218e0a6a4..4404a45492254 100644
--- a/lldb/source/Target/TargetProperties.td
+++ b/lldb/source/Target/TargetProperties.td
@@ -46,6 +46,9 @@ let Definition = "target" in {
def SourceMap: Property<"source-map", "PathMap">,
DefaultStringValue<"">,
Desc<"Source path remappings apply substitutions to the paths of source
files, typically needed to debug from a different host than the one that built
the target. The source-map property consists of an array of pairs, the first
element is a path prefix, and the second is its replacement. The syntax is
`prefix1 replacement1 prefix2 replacement2...`. The pairs are checked in
order, the first prefix that matches is used, and that prefix is substituted
with the replacement. A common pattern is to use source-map in conjunction
with the clang -fdebug-prefix-map flag. In the build, use
`-fdebug-prefix-map=/path/to/build_dir=.` to rewrite the host specific build
directory to `.`. Then for debugging, use `settings set target.source-map .
/path/to/local_dir` to convert `.` to a valid local path.">;
+ def ObjectMap: Property<"object-map", "PathMap">,
+DefaultStringValue<"">,
+Desc<"Object path remappings apply substitutions to the paths of object
files, typically needed to debug from a different host than the one that built
the target. The object-map property consists of an array of pairs, the first
element is a path prefix, and the second is its replacement. The syntax is
`prefix1 replacement1 prefix2 replacement2...`. The pairs are checked in
order, the first prefix that matches is used, and that prefix is substituted
with the replacement.">;
def AutoSourceMapRelative: Property<"
[Lldb-commits] [lldb] [lldb] Allow mapping object file paths (PR #101361)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Alexandre Perez (aperez)
Changes
This introduces a `target.object-map` which allows us to remap module
locations, much in the same way as source mapping works today. This is useful,
for instance, when debugging coredumps, so we can replace some of the locations
where LLDB attempts to load shared libraries and executables from, without
having to setup an entire sysroot.
---
Full diff: https://github.com/llvm/llvm-project/pull/101361.diff
4 Files Affected:
- (modified) lldb/include/lldb/Target/Target.h (+2)
- (modified) lldb/source/Target/Target.cpp (+19-2)
- (modified) lldb/source/Target/TargetProperties.td (+3)
- (modified) lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
(+24)
``diff
diff --git a/lldb/include/lldb/Target/Target.h
b/lldb/include/lldb/Target/Target.h
index 5d5ae1bfcd3bd..119dff4d49819 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -141,6 +141,8 @@ class TargetProperties : public Properties {
PathMappingList &GetSourcePathMap() const;
+ PathMappingList &GetObjectPathMap() const;
+
bool GetAutoSourceMapRelative() const;
FileSpecList GetExecutableSearchPaths();
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index ec0da8a1378a8..129683c43f0c1 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -2155,12 +2155,21 @@ bool Target::ReadPointerFromMemory(const Address &addr,
Status &error,
return false;
}
-ModuleSP Target::GetOrCreateModule(const ModuleSpec &module_spec, bool notify,
- Status *error_ptr) {
+ModuleSP Target::GetOrCreateModule(const ModuleSpec &orig_module_spec,
+ bool notify, Status *error_ptr) {
ModuleSP module_sp;
Status error;
+ // Apply any remappings specified in target.object-map:
+ ModuleSpec module_spec(orig_module_spec);
+ PathMappingList &obj_mapping = GetObjectPathMap();
+ if (std::optional remapped_obj_file =
+ obj_mapping.RemapPath(orig_module_spec.GetFileSpec().GetPath(),
+true /* only_if_exists */)) {
+module_spec.GetFileSpec().SetPath(remapped_obj_file->GetPath());
+ }
+
// First see if we already have this module in our module list. If we do,
// then we're done, we don't need to consult the shared modules list. But
// only do this if we are passed a UUID.
@@ -4459,6 +4468,14 @@ PathMappingList &TargetProperties::GetSourcePathMap()
const {
return option_value->GetCurrentValue();
}
+PathMappingList &TargetProperties::GetObjectPathMap() const {
+ const uint32_t idx = ePropertyObjectMap;
+ OptionValuePathMappings *option_value =
+ m_collection_sp->GetPropertyAtIndexAsOptionValuePathMappings(idx);
+ assert(option_value);
+ return option_value->GetCurrentValue();
+}
+
bool TargetProperties::GetAutoSourceMapRelative() const {
const uint32_t idx = ePropertyAutoSourceMapRelative;
return GetPropertyAtIndexAs(
diff --git a/lldb/source/Target/TargetProperties.td
b/lldb/source/Target/TargetProperties.td
index 7f79218e0a6a4..4404a45492254 100644
--- a/lldb/source/Target/TargetProperties.td
+++ b/lldb/source/Target/TargetProperties.td
@@ -46,6 +46,9 @@ let Definition = "target" in {
def SourceMap: Property<"source-map", "PathMap">,
DefaultStringValue<"">,
Desc<"Source path remappings apply substitutions to the paths of source
files, typically needed to debug from a different host than the one that built
the target. The source-map property consists of an array of pairs, the first
element is a path prefix, and the second is its replacement. The syntax is
`prefix1 replacement1 prefix2 replacement2...`. The pairs are checked in
order, the first prefix that matches is used, and that prefix is substituted
with the replacement. A common pattern is to use source-map in conjunction
with the clang -fdebug-prefix-map flag. In the build, use
`-fdebug-prefix-map=/path/to/build_dir=.` to rewrite the host specific build
directory to `.`. Then for debugging, use `settings set target.source-map .
/path/to/local_dir` to convert `.` to a valid local path.">;
+ def ObjectMap: Property<"object-map", "PathMap">,
+DefaultStringValue<"">,
+Desc<"Object path remappings apply substitutions to the paths of object
files, typically needed to debug from a different host than the one that built
the target. The object-map property consists of an array of pairs, the first
element is a path prefix, and the second is its replacement. The syntax is
`prefix1 replacement1 prefix2 replacement2...`. The pairs are checked in
order, the first prefix that matches is used, and that prefix is substituted
with the replacement.">;
def AutoSourceMapRelative: Property<"auto-source-map-relative", "Boolean">,
DefaultTrue,
Desc<"Automatically deduce source path mappings based on source file
bre
[Lldb-commits] [lldb] [lldb] Allow mapping object file paths (PR #101361)
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
12189f800585ab459afa20b4f159db93ae474b57...5221c6f267fffd811c6dd39dbbcc211e2a93739c
lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
``
View the diff from darker here.
``diff
--- TestLinuxCore.py2024-07-31 16:38:38.00 +
+++ TestLinuxCore.py2024-07-31 16:49:38.640161 +
@@ -258,11 +258,13 @@
lldbutil.mkdir_p(os.path.dirname(executable))
shutil.copyfile("linux-i386.out", executable)
# Replace the original module path at /home/labath/test and load the
core
self.runCmd(
-"settings set target.object-map /home/labath/test
{}".format(tmp_object_map_root)
+"settings set target.object-map /home/labath/test {}".format(
+tmp_object_map_root
+)
)
target = self.dbg.CreateTarget(None)
process = target.LoadCore("linux-i386.core")
``
https://github.com/llvm/llvm-project/pull/101361
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)
@@ -138,7 +191,7 @@ def use_support_substitutions(config):
# Set up substitutions for support tools. These tools can be overridden
at the CMake
# level (by specifying -DLLDB_LIT_TOOLS_DIR), installed, or as a last
resort, we can use
# the just-built version.
-host_flags = ["--target=" + config.host_triple]
+host_flags = ["--target=" + config.target_triple]
dzhidzhoev wrote:
Hmm. I thought it was named after the substitution name %clang_host (used by
tests not bound to the specific architecture).
I think it may be better to rename the substitution to %clang_target instead,
but it requires a huge change of all tests.
Otherwise, we get the line like
`config.substitutions.append(("%clang_host", "%clang " + target_flags))`
which also looks confusing IMO.
https://github.com/llvm/llvm-project/pull/95986
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Allow mapping object file paths (PR #101361)
https://github.com/aperez updated
https://github.com/llvm/llvm-project/pull/101361
>From 132b6fb0808385494a71c99533d7281420b743bd Mon Sep 17 00:00:00 2001
From: Alexandre Perez
Date: Wed, 31 Jul 2024 09:38:38 -0700
Subject: [PATCH] [lldb] Allow mapping object file paths
---
lldb/include/lldb/Target/Target.h | 2 ++
lldb/source/Target/Target.cpp | 21 +--
lldb/source/Target/TargetProperties.td| 3 +++
.../postmortem/elf-core/TestLinuxCore.py | 26 +++
4 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/lldb/include/lldb/Target/Target.h
b/lldb/include/lldb/Target/Target.h
index 5d5ae1bfcd3bd..119dff4d49819 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -141,6 +141,8 @@ class TargetProperties : public Properties {
PathMappingList &GetSourcePathMap() const;
+ PathMappingList &GetObjectPathMap() const;
+
bool GetAutoSourceMapRelative() const;
FileSpecList GetExecutableSearchPaths();
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index ec0da8a1378a8..129683c43f0c1 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -2155,12 +2155,21 @@ bool Target::ReadPointerFromMemory(const Address &addr,
Status &error,
return false;
}
-ModuleSP Target::GetOrCreateModule(const ModuleSpec &module_spec, bool notify,
- Status *error_ptr) {
+ModuleSP Target::GetOrCreateModule(const ModuleSpec &orig_module_spec,
+ bool notify, Status *error_ptr) {
ModuleSP module_sp;
Status error;
+ // Apply any remappings specified in target.object-map:
+ ModuleSpec module_spec(orig_module_spec);
+ PathMappingList &obj_mapping = GetObjectPathMap();
+ if (std::optional remapped_obj_file =
+ obj_mapping.RemapPath(orig_module_spec.GetFileSpec().GetPath(),
+true /* only_if_exists */)) {
+module_spec.GetFileSpec().SetPath(remapped_obj_file->GetPath());
+ }
+
// First see if we already have this module in our module list. If we do,
// then we're done, we don't need to consult the shared modules list. But
// only do this if we are passed a UUID.
@@ -4459,6 +4468,14 @@ PathMappingList &TargetProperties::GetSourcePathMap()
const {
return option_value->GetCurrentValue();
}
+PathMappingList &TargetProperties::GetObjectPathMap() const {
+ const uint32_t idx = ePropertyObjectMap;
+ OptionValuePathMappings *option_value =
+ m_collection_sp->GetPropertyAtIndexAsOptionValuePathMappings(idx);
+ assert(option_value);
+ return option_value->GetCurrentValue();
+}
+
bool TargetProperties::GetAutoSourceMapRelative() const {
const uint32_t idx = ePropertyAutoSourceMapRelative;
return GetPropertyAtIndexAs(
diff --git a/lldb/source/Target/TargetProperties.td
b/lldb/source/Target/TargetProperties.td
index 7f79218e0a6a4..4404a45492254 100644
--- a/lldb/source/Target/TargetProperties.td
+++ b/lldb/source/Target/TargetProperties.td
@@ -46,6 +46,9 @@ let Definition = "target" in {
def SourceMap: Property<"source-map", "PathMap">,
DefaultStringValue<"">,
Desc<"Source path remappings apply substitutions to the paths of source
files, typically needed to debug from a different host than the one that built
the target. The source-map property consists of an array of pairs, the first
element is a path prefix, and the second is its replacement. The syntax is
`prefix1 replacement1 prefix2 replacement2...`. The pairs are checked in
order, the first prefix that matches is used, and that prefix is substituted
with the replacement. A common pattern is to use source-map in conjunction
with the clang -fdebug-prefix-map flag. In the build, use
`-fdebug-prefix-map=/path/to/build_dir=.` to rewrite the host specific build
directory to `.`. Then for debugging, use `settings set target.source-map .
/path/to/local_dir` to convert `.` to a valid local path.">;
+ def ObjectMap: Property<"object-map", "PathMap">,
+DefaultStringValue<"">,
+Desc<"Object path remappings apply substitutions to the paths of object
files, typically needed to debug from a different host than the one that built
the target. The object-map property consists of an array of pairs, the first
element is a path prefix, and the second is its replacement. The syntax is
`prefix1 replacement1 prefix2 replacement2...`. The pairs are checked in
order, the first prefix that matches is used, and that prefix is substituted
with the replacement.">;
def AutoSourceMapRelative: Property<"auto-source-map-relative", "Boolean">,
DefaultTrue,
Desc<"Automatically deduce source path mappings based on source file
breakpoint resolution. It only deduces source mapping if source file breakpoint
request is using full path and if the debug info contains relative paths.">;
diff --git a/lldb/test/API/functionalities/postmortem/e
[Lldb-commits] [lldb] [lldb] Unify the way we get the Target in CommandObject (PR #101208)
https://github.com/JDevlieghere closed https://github.com/llvm/llvm-project/pull/101208 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 8398ad9 - [lldb] Unify the way we get the Target in CommandObject (#101208)
Author: Jonas Devlieghere
Date: 2024-07-31T09:57:10-07:00
New Revision: 8398ad9cb21736dc57ee4dd766bd0859ef9bd000
URL:
https://github.com/llvm/llvm-project/commit/8398ad9cb21736dc57ee4dd766bd0859ef9bd000
DIFF:
https://github.com/llvm/llvm-project/commit/8398ad9cb21736dc57ee4dd766bd0859ef9bd000.diff
LOG: [lldb] Unify the way we get the Target in CommandObject (#101208)
Currently, CommandObjects are obtaining a target in a variety of ways.
Often the command incorrectly operates on the selected target. As an
example, when a breakpoint command is running, the current target is
passed into the command but the target that hit the breakpoint is not
the selected target. In other places we use the CommandObject's
execution context, which is frozen during the execution of the command,
and comes with its own limitations. Finally, we often want to fall back
to the dummy target if no real target is available.
Instead of having to guess how to get the target, this patch introduces
one helper function in CommandObject to get the most relevant target. In
order of priority, that's the target from the command object's execution
context, from the interpreter's execution context, the selected target
or the dummy target.
rdar://110846511
Added:
Modified:
lldb/include/lldb/Interpreter/CommandObject.h
lldb/source/Commands/CommandObjectBreakpoint.cpp
lldb/source/Commands/CommandObjectBreakpointCommand.cpp
lldb/source/Commands/CommandObjectDisassemble.cpp
lldb/source/Commands/CommandObjectExpression.cpp
lldb/source/Commands/CommandObjectFrame.cpp
lldb/source/Commands/CommandObjectProcess.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Commands/CommandObjectWatchpoint.cpp
lldb/source/Commands/CommandObjectWatchpointCommand.cpp
lldb/source/Interpreter/CommandObject.cpp
lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
Removed:
diff --git a/lldb/include/lldb/Interpreter/CommandObject.h
b/lldb/include/lldb/Interpreter/CommandObject.h
index d48dbcdd5a5da..20c4769af9033 100644
--- a/lldb/include/lldb/Interpreter/CommandObject.h
+++ b/lldb/include/lldb/Interpreter/CommandObject.h
@@ -369,13 +369,14 @@ class CommandObject : public
std::enable_shared_from_this {
"currently stopped.";
}
- // This is for use in the command interpreter, when you either want the
- // selected target, or if no target is present you want to prime the dummy
- // target with entities that will be copied over to new targets.
- Target &GetSelectedOrDummyTarget(bool prefer_dummy = false);
- Target &GetSelectedTarget();
Target &GetDummyTarget();
+ // This is for use in the command interpreter, and returns the most relevant
+ // target. In order of priority, that's the target from the command object's
+ // execution context, the target from the interpreter's execution context,
the
+ // selected target or the dummy target.
+ Target &GetTarget();
+
// If a command needs to use the "current" thread, use this call. Command
// objects will have an ExecutionContext to use, and that may or may not have
// a thread in it. If it does, you should use that by default, if not, then
diff --git a/lldb/source/Commands/CommandObjectBreakpoint.cpp
b/lldb/source/Commands/CommandObjectBreakpoint.cpp
index 773f8ed2fa8af..aad03af11331c 100644
--- a/lldb/source/Commands/CommandObjectBreakpoint.cpp
+++ b/lldb/source/Commands/CommandObjectBreakpoint.cpp
@@ -539,7 +539,8 @@ class CommandObjectBreakpointSet : public
CommandObjectParsed {
protected:
void DoExecute(Args &command, CommandReturnObject &result) override {
-Target &target = GetSelectedOrDummyTarget(m_dummy_options.m_use_dummy);
+Target &target =
+m_dummy_options.m_use_dummy ? GetDummyTarget() : GetTarget();
// The following are the various types of breakpoints that could be set:
// 1). -f -l -p [-s -g] (setting breakpoint by source location)
@@ -839,7 +840,7 @@ class CommandObjectBreakpointModify : public
CommandObjectParsed {
protected:
void DoExecute(Args &command, CommandReturnObject &result) override {
-Target &target = GetSelectedOrDummyTarget(m_dummy_opts.m_use_dummy);
+Target &target = m_dummy_opts.m_use_dummy ? GetDummyTarget() : GetTarget();
std::unique_lock lock;
target.GetBreakpointList().GetListMutex(lock);
@@ -903,7 +904,7 @@ class CommandObjectBreakpointEnable : public
CommandObjectParsed {
protected:
void DoExecute(Args &command, CommandReturnObject &result) override {
-Target &target = GetSelectedOrDummyTarget();
+Target &target = GetTarget();
std::unique_lock lock;
target.GetBreakpointList().GetListMutex(lock);
@@ -1010,7 +1011,7 @@ the second re-enables the first location.");
protected:
void DoExecute(Args &command, CommandReturnObjec
[Lldb-commits] [lldb] Add a StackFrameRecognizer for the Darwin specific abort_with_payload… (PR #101365)
https://github.com/jimingham created
https://github.com/llvm/llvm-project/pull/101365
This is used by various system routines (the capabilities checker and dyld to
name a few) to add extra color to an abort. This patch adds a frame
recognizer so people can easily see the details, and also adds the information
to the ExtendedCrashInformation dictionary.
I also had to rework how the dictionary is held; previously it was created on
demand, but that was inconvenient since it meant all the entries had to be
produced at that same time. That didn't work for the recognizer.
>From 5b2cf0ab7e382e41ff3dfe88e4d7e35ddc99afd5 Mon Sep 17 00:00:00 2001
From: Jim Ingham
Date: Tue, 30 Jul 2024 09:32:24 -0700
Subject: [PATCH] Add a StackFrameRecognizer for the Darwin specific
abort_with_payload API.
---
lldb/include/lldb/Target/Process.h| 16 ++
.../Platform/MacOSX/PlatformDarwin.cpp| 45 ++--
.../AbortWithPayloadFrameRecognizer.cpp | 201 ++
.../MacOSX/AbortWithPayloadFrameRecognizer.h | 38
.../SystemRuntime/MacOSX/CMakeLists.txt | 1 +
.../MacOSX/SystemRuntimeMacOSX.cpp| 6 +-
lldb/source/Target/Process.cpp| 7 +-
.../API/macosx/abort_with_payload/Makefile| 4 +
.../TestAbortWithPayload.py | 146 +
.../test/API/macosx/abort_with_payload/main.c | 30 +++
10 files changed, 478 insertions(+), 16 deletions(-)
create mode 100644
lldb/source/Plugins/SystemRuntime/MacOSX/AbortWithPayloadFrameRecognizer.cpp
create mode 100644
lldb/source/Plugins/SystemRuntime/MacOSX/AbortWithPayloadFrameRecognizer.h
create mode 100644 lldb/test/API/macosx/abort_with_payload/Makefile
create mode 100644
lldb/test/API/macosx/abort_with_payload/TestAbortWithPayload.py
create mode 100644 lldb/test/API/macosx/abort_with_payload/main.c
diff --git a/lldb/include/lldb/Target/Process.h
b/lldb/include/lldb/Target/Process.h
index c8475db8ae160..84bbd8a52 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -2569,6 +2569,18 @@ void PruneThreadPlans();
/// A StructuredDataSP object which, if non-empty, will contain the
/// information related to the process.
virtual StructuredData::DictionarySP GetMetadata() { return nullptr; }
+
+ /// Fetch extended crash information held by the process. This will never be
+ /// an empty shared pointer, it will always have a dict, though it may be
+ /// empty.
+ StructuredData::DictionarySP GetExtendedCrashInfoDict() {
+return m_crash_info_dict_sp;
+ }
+
+ void ResetExtendedCrashInfoDict() {
+// StructuredData::Dictionary is add only, so we have to make a new one:
+m_crash_info_dict_sp.reset(new StructuredData::Dictionary());
+ }
size_t AddImageToken(lldb::addr_t image_ptr);
@@ -3185,6 +3197,10 @@ void PruneThreadPlans();
/// Per process source file cache.
SourceManager::SourceFileCache m_source_file_cache;
+
+ /// A repository for extra crash information, consulted in
+ /// GetExtendedCrashInformation.
+ StructuredData::DictionarySP m_crash_info_dict_sp;
size_t RemoveBreakpointOpcodesFromBuffer(lldb::addr_t addr, size_t size,
uint8_t *buf) const;
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 6fb5bbfbe417b..398b44c293614 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -856,21 +856,38 @@ PlatformDarwin::ParseVersionBuildDir(llvm::StringRef dir)
{
}
llvm::Expected
-PlatformDarwin::FetchExtendedCrashInformation(Process &process) {
- StructuredData::DictionarySP extended_crash_info =
- std::make_shared();
-
- StructuredData::ArraySP annotations = ExtractCrashInfoAnnotations(process);
- if (annotations && annotations->GetSize())
-extended_crash_info->AddItem("Crash-Info Annotations", annotations);
-
- StructuredData::DictionarySP app_specific_info =
- ExtractAppSpecificInfo(process);
- if (app_specific_info && app_specific_info->GetSize())
-extended_crash_info->AddItem("Application Specific Information",
- app_specific_info);
+PlatformDarwin::
+FetchExtendedCrashInformation(Process &process) {
+ static constexpr llvm::StringLiteral crash_info_key("Crash-Info
Annotations");
+ static constexpr llvm::StringLiteral asi_info_key("Application Specific
Information");
+
+ // We cache the information we find in the process extended info dict:
+ StructuredData::DictionarySP process_dict_sp
+ = process.GetExtendedCrashInfoDict();
+ StructuredData::Array *annotations = nullptr;
+ StructuredData::ArraySP new_annotations_sp;
+ if (!process_dict_sp->GetValueForKeyAsArray(crash_info_key, annotations)) {
+new_annotations_sp = ExtractCrashInfoAnnotations(process);
+if (new_annotations_sp && new_
[Lldb-commits] [lldb] Add a StackFrameRecognizer for the Darwin specific abort_with_payload… (PR #101365)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: None (jimingham)
Changes
This is used by various system routines (the capabilities checker and dyld to
name a few) to add extra color to an abort. This patch adds a frame
recognizer so people can easily see the details, and also adds the information
to the ExtendedCrashInformation dictionary.
I also had to rework how the dictionary is held; previously it was created on
demand, but that was inconvenient since it meant all the entries had to be
produced at that same time. That didn't work for the recognizer.
---
Patch is 26.07 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/101365.diff
10 Files Affected:
- (modified) lldb/include/lldb/Target/Process.h (+16)
- (modified) lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp (+31-14)
- (added)
lldb/source/Plugins/SystemRuntime/MacOSX/AbortWithPayloadFrameRecognizer.cpp
(+201)
- (added)
lldb/source/Plugins/SystemRuntime/MacOSX/AbortWithPayloadFrameRecognizer.h
(+38)
- (modified) lldb/source/Plugins/SystemRuntime/MacOSX/CMakeLists.txt (+1)
- (modified) lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
(+5-1)
- (modified) lldb/source/Target/Process.cpp (+6-1)
- (added) lldb/test/API/macosx/abort_with_payload/Makefile (+4)
- (added) lldb/test/API/macosx/abort_with_payload/TestAbortWithPayload.py
(+146)
- (added) lldb/test/API/macosx/abort_with_payload/main.c (+30)
``diff
diff --git a/lldb/include/lldb/Target/Process.h
b/lldb/include/lldb/Target/Process.h
index c8475db8ae160..84bbd8a52 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -2569,6 +2569,18 @@ void PruneThreadPlans();
/// A StructuredDataSP object which, if non-empty, will contain the
/// information related to the process.
virtual StructuredData::DictionarySP GetMetadata() { return nullptr; }
+
+ /// Fetch extended crash information held by the process. This will never be
+ /// an empty shared pointer, it will always have a dict, though it may be
+ /// empty.
+ StructuredData::DictionarySP GetExtendedCrashInfoDict() {
+return m_crash_info_dict_sp;
+ }
+
+ void ResetExtendedCrashInfoDict() {
+// StructuredData::Dictionary is add only, so we have to make a new one:
+m_crash_info_dict_sp.reset(new StructuredData::Dictionary());
+ }
size_t AddImageToken(lldb::addr_t image_ptr);
@@ -3185,6 +3197,10 @@ void PruneThreadPlans();
/// Per process source file cache.
SourceManager::SourceFileCache m_source_file_cache;
+
+ /// A repository for extra crash information, consulted in
+ /// GetExtendedCrashInformation.
+ StructuredData::DictionarySP m_crash_info_dict_sp;
size_t RemoveBreakpointOpcodesFromBuffer(lldb::addr_t addr, size_t size,
uint8_t *buf) const;
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 6fb5bbfbe417b..398b44c293614 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -856,21 +856,38 @@ PlatformDarwin::ParseVersionBuildDir(llvm::StringRef dir)
{
}
llvm::Expected
-PlatformDarwin::FetchExtendedCrashInformation(Process &process) {
- StructuredData::DictionarySP extended_crash_info =
- std::make_shared();
-
- StructuredData::ArraySP annotations = ExtractCrashInfoAnnotations(process);
- if (annotations && annotations->GetSize())
-extended_crash_info->AddItem("Crash-Info Annotations", annotations);
-
- StructuredData::DictionarySP app_specific_info =
- ExtractAppSpecificInfo(process);
- if (app_specific_info && app_specific_info->GetSize())
-extended_crash_info->AddItem("Application Specific Information",
- app_specific_info);
+PlatformDarwin::
+FetchExtendedCrashInformation(Process &process) {
+ static constexpr llvm::StringLiteral crash_info_key("Crash-Info
Annotations");
+ static constexpr llvm::StringLiteral asi_info_key("Application Specific
Information");
+
+ // We cache the information we find in the process extended info dict:
+ StructuredData::DictionarySP process_dict_sp
+ = process.GetExtendedCrashInfoDict();
+ StructuredData::Array *annotations = nullptr;
+ StructuredData::ArraySP new_annotations_sp;
+ if (!process_dict_sp->GetValueForKeyAsArray(crash_info_key, annotations)) {
+new_annotations_sp = ExtractCrashInfoAnnotations(process);
+if (new_annotations_sp && new_annotations_sp->GetSize()) {
+ process_dict_sp->AddItem(crash_info_key, new_annotations_sp);
+ annotations = new_annotations_sp.get();
+}
+ }
- return extended_crash_info->GetSize() ? extended_crash_info : nullptr;
+ StructuredData::Dictionary *app_specific_info;
+ StructuredData::DictionarySP new_app_specific_info_sp;
+ if (!process_dict_sp->GetValueForKe
[Lldb-commits] [lldb] Add a StackFrameRecognizer for the Darwin specific abort_with_payload… (PR #101365)
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
80c0e8d572d3b1c34d66faffc42bcfc9432583ec...5b2cf0ab7e382e41ff3dfe88e4d7e35ddc99afd5
lldb/test/API/macosx/abort_with_payload/TestAbortWithPayload.py
``
View the diff from darker here.
``diff
--- TestAbortWithPayload.py 2024-07-31 16:59:31.00 +
+++ TestAbortWithPayload.py 2024-07-31 17:12:24.107113 +
@@ -27,37 +27,43 @@
TestBase.setUp(self)
self.main_source_file = lldb.SBFileSpec("main.c")
def abort_with_test(self, with_payload):
"""If with_payload is True, we test the abort_with_payload call,
- if false, we test abort_with_reason."""
+if false, we test abort_with_reason."""
launch_info = lldb.SBLaunchInfo([])
if not with_payload:
launch_info.SetArguments(["use_reason"], True)
(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
-self, "Stop here before abort", self.main_source_file,
-launch_info=launch_info
+self,
+"Stop here before abort",
+self.main_source_file,
+launch_info=launch_info,
)
frame = thread.GetFrameAtIndex(0)
payload_str_var = frame.FindVariable("payload_string")
self.assertSuccess(payload_str_var.GetError(), "Got payload string
var")
payload_var_addr = payload_str_var.unsigned
-
+
payload_size_var = frame.FindVariable("payload_string_len")
self.assertSuccess(payload_size_var.GetError(), "Got payload string
len var")
payload_size_val = payload_size_var.unsigned
# Not let it run to crash:
process.Continue()
-
+
# At this point we should have stopped at the internal function.
# Make sure we selected the right thread:
sel_thread = process.GetSelectedThread()
self.assertEqual(thread, sel_thread, "Selected the original thread")
# Make sure the stop reason is right:
-self.assertEqual(thread.GetStopDescription(100), "abort with payload
or reason", "Description was right")
+self.assertEqual(
+thread.GetStopDescription(100),
+"abort with payload or reason",
+"Description was right",
+)
frame_0 = thread.frames[0]
self.assertEqual(frame_0.name, "__abort_with_payload", "Frame 0 was
right")
# Now check the recognized argument values and the
ExtendedCrashInformation version:
options = lldb.SBVariablesOptions()
@@ -67,80 +73,136 @@
options.SetIncludeStatics(False)
options.SetIncludeRuntimeSupportValues(False)
arguments = frame_0.GetVariables(options)
-correct_values = { "namespace" : 5,
- "code": 100,
- "payload_addr": payload_var_addr,
- "payload_size": payload_size_val,
- "payload_string" : '"This is a payload that
happens to be a string"',
- "reason_string" : '"This is the reason string"',
- "reason_no_quote" : "This is the reason string",
- "flags": 0x85 }
-
-# First check the recognized argument values:
+correct_values = {
+"namespace": 5,
+"code": 100,
+"payload_addr": payload_var_addr,
+"payload_size": payload_size_val,
+"payload_string": '"This is a payload that happens to be a
string"',
+"reason_string": '"This is the reason string"',
+"reason_no_quote": "This is the reason string",
+"flags": 0x85,
+}
+
+# First check the recognized argument values:
self.assertEqual(len(arguments), 6, "Got all six values")
self.runCmd("frame variable")
self.assertEqual(arguments[0].name, "namespace")
-self.assertEqual(arguments[0].unsigned, correct_values["namespace"],
"Namespace value correct")
+self.assertEqual(
+arguments[0].unsigned,
+correct_values["namespace"],
+"Namespace value correct",
+)
self.assertEqual(arguments[1].name, "code")
-self.assertEqual(arguments[1].unsigned, correct_values["code"], "code
value correct")
+self.assertEqual(
+arguments[1].unsigned, correct_values["code"], "code value correct"
+)
# FIXME: I'm always adding these recognized arguments, but that looks
wrong. Should only
# add the payload ones if it is the payload not the reason function.
self.assertEqual(arguments[2].name, "payload_addr")
self.assertEqual(arguments[3].name, "payloa
[Lldb-commits] [lldb] Add a StackFrameRecognizer for the Darwin specific abort_with_payload… (PR #101365)
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 80c0e8d572d3b1c34d66faffc42bcfc9432583ec
5b2cf0ab7e382e41ff3dfe88e4d7e35ddc99afd5 --extensions cpp,h,c --
lldb/source/Plugins/SystemRuntime/MacOSX/AbortWithPayloadFrameRecognizer.cpp
lldb/source/Plugins/SystemRuntime/MacOSX/AbortWithPayloadFrameRecognizer.h
lldb/test/API/macosx/abort_with_payload/main.c
lldb/include/lldb/Target/Process.h
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/source/Plugins/SystemRuntime/MacOSX/SystemRuntimeMacOSX.cpp
lldb/source/Target/Process.cpp
``
View the diff from clang-format here.
``diff
diff --git a/lldb/include/lldb/Target/Process.h
b/lldb/include/lldb/Target/Process.h
index 84bbd8..e1be76d494 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -2569,14 +2569,14 @@ void PruneThreadPlans();
/// A StructuredDataSP object which, if non-empty, will contain the
/// information related to the process.
virtual StructuredData::DictionarySP GetMetadata() { return nullptr; }
-
+
/// Fetch extended crash information held by the process. This will never be
/// an empty shared pointer, it will always have a dict, though it may be
/// empty.
StructuredData::DictionarySP GetExtendedCrashInfoDict() {
-return m_crash_info_dict_sp;
+return m_crash_info_dict_sp;
}
-
+
void ResetExtendedCrashInfoDict() {
// StructuredData::Dictionary is add only, so we have to make a new one:
m_crash_info_dict_sp.reset(new StructuredData::Dictionary());
@@ -3197,8 +3197,8 @@ protected:
/// Per process source file cache.
SourceManager::SourceFileCache m_source_file_cache;
-
- /// A repository for extra crash information, consulted in
+
+ /// A repository for extra crash information, consulted in
/// GetExtendedCrashInformation.
StructuredData::DictionarySP m_crash_info_dict_sp;
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 398b44c293..3d22e7122d 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -856,15 +856,15 @@ PlatformDarwin::ParseVersionBuildDir(llvm::StringRef dir)
{
}
llvm::Expected
-PlatformDarwin::
-FetchExtendedCrashInformation(Process &process) {
+PlatformDarwin::FetchExtendedCrashInformation(Process &process) {
static constexpr llvm::StringLiteral crash_info_key("Crash-Info
Annotations");
- static constexpr llvm::StringLiteral asi_info_key("Application Specific
Information");
+ static constexpr llvm::StringLiteral asi_info_key(
+ "Application Specific Information");
// We cache the information we find in the process extended info dict:
- StructuredData::DictionarySP process_dict_sp
- = process.GetExtendedCrashInfoDict();
- StructuredData::Array *annotations = nullptr;
+ StructuredData::DictionarySP process_dict_sp =
+ process.GetExtendedCrashInfoDict();
+ StructuredData::Array *annotations = nullptr;
StructuredData::ArraySP new_annotations_sp;
if (!process_dict_sp->GetValueForKeyAsArray(crash_info_key, annotations)) {
new_annotations_sp = ExtractCrashInfoAnnotations(process);
@@ -876,15 +876,15 @@ FetchExtendedCrashInformation(Process &process) {
StructuredData::Dictionary *app_specific_info;
StructuredData::DictionarySP new_app_specific_info_sp;
- if (!process_dict_sp->GetValueForKeyAsDictionary(asi_info_key,
app_specific_info))
- {
+ if (!process_dict_sp->GetValueForKeyAsDictionary(asi_info_key,
+ app_specific_info)) {
new_app_specific_info_sp = ExtractAppSpecificInfo(process);
if (new_app_specific_info_sp && new_app_specific_info_sp->GetSize()) {
process_dict_sp->AddItem(asi_info_key, new_app_specific_info_sp);
app_specific_info = new_app_specific_info_sp.get();
}
}
-
+
// Now get anything else that was in the process info dict, and add it to the
// return here:
return process_dict_sp->GetSize() ? process_dict_sp : nullptr;
diff --git
a/lldb/source/Plugins/SystemRuntime/MacOSX/AbortWithPayloadFrameRecognizer.cpp
b/lldb/source/Plugins/SystemRuntime/MacOSX/AbortWithPayloadFrameRecognizer.cpp
index 7f2aecd148..5434601f54 100644
---
a/lldb/source/Plugins/SystemRuntime/MacOSX/AbortWithPayloadFrameRecognizer.cpp
+++
b/lldb/source/Plugins/SystemRuntime/MacOSX/AbortWithPayloadFrameRecognizer.cpp
@@ -1,4 +1,4 @@
- //===-- AbortWithPayloadFrameRecognizer.cpp
---===//
+//===-- AbortWithPayloadFrameRecognizer.cpp
---===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
// See https://llvm.org/LICENSE.txt for license
[Lldb-commits] [lldb] [lldb][test] Support remote run of Shell tests (PR #95986)
dzhidzhoev wrote: > I'm not sure whether we need `LLDB_SHELL_TESTS_DISABLE_REMOTE`, can you > explain how that would be used? > > Would I set up a build with all the other flags, do my remote testing, then > set `LLDB_SHELL_TESTS_DISABLE_REMOTE=OFF` and run the shell tests locally, > using that same build? > > I guess I'm asking whether a given build should be all remote or all local or > a mix. I've added this flag to address the question raised about the support for different compilers. We've discussed that we're going to support Shell tests execution only with Clang compiler to avoid the burden of maintaining too many build flag combinations. However, for tests that use `%build` substitution, compiler detection is postponed to the test execution time (it happens in `lldb/test/Shell/helper/build.py` script itself), but it's necessary to know whether remote execution is on in advance (to disable local-only tests with `UNSUPPORTED: remote-linux` line). Thus, I've added this flag to disable remote Shell tests execution if tests fail due to unsupported compiler usage. The idea is that if this flag is turned on, tests are run locally as it happens right now. So no "mixed" executions are desired. It should be either "remote execution against the remote target" or "best-effort local execution against the localhost target" (best-effort since if cross toolchain for cross-compilation/remote execution of API tests is built that doesn't support host target and Shell tests can't find another compiler, they fail on current mainline). Also, I think `--target=` argument appended to %clang_host substitution should be changed to reflect that logic. https://github.com/llvm/llvm-project/pull/95986 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 9fe455f - [lldb] Add constant value mode for RegisterLocation in UnwindPlans (#100624)
Author: Felipe de Azevedo Piovezan
Date: 2024-07-31T10:25:31-07:00
New Revision: 9fe455fd0c7d6f2107b33b37c04bbd3b12fe65b3
URL:
https://github.com/llvm/llvm-project/commit/9fe455fd0c7d6f2107b33b37c04bbd3b12fe65b3
DIFF:
https://github.com/llvm/llvm-project/commit/9fe455fd0c7d6f2107b33b37c04bbd3b12fe65b3.diff
LOG: [lldb] Add constant value mode for RegisterLocation in UnwindPlans
(#100624)
This is useful for language runtimes that compute register values by
inspecting the state of the currently running process. Currently, there
are no mechanisms enabling these runtimes to set register values to
arbitrary values.
The alternative considered would involve creating a dwarf expression
that produces an arbitrary integer (e.g. using OP_constu). However, the
current data structure for Rows is such that they do not own any memory
associated with dwarf expressions, which implies any such expression
would need to have static storage and therefore could not contain a
runtime value.
Adding a new rule for constants leads to a simpler implementation. It's
also worth noting that this does not make the "Location" union any
bigger, since it already contains a pointer+size pair.
Added:
Modified:
lldb/include/lldb/Symbol/UnwindPlan.h
lldb/source/Symbol/UnwindPlan.cpp
lldb/source/Target/RegisterContextUnwind.cpp
Removed:
diff --git a/lldb/include/lldb/Symbol/UnwindPlan.h
b/lldb/include/lldb/Symbol/UnwindPlan.h
index ebb0ec421da72..a9e8406608ff3 100644
--- a/lldb/include/lldb/Symbol/UnwindPlan.h
+++ b/lldb/include/lldb/Symbol/UnwindPlan.h
@@ -68,7 +68,8 @@ class UnwindPlan {
isAFAPlusOffset, // reg = AFA + offset
inOtherRegister, // reg = other reg
atDWARFExpression, // reg = deref(eval(dwarf_expr))
-isDWARFExpression // reg = eval(dwarf_expr)
+isDWARFExpression, // reg = eval(dwarf_expr)
+isConstant // reg = constant
};
RegisterLocation() : m_location() {}
@@ -105,6 +106,15 @@ class UnwindPlan {
bool IsDWARFExpression() const { return m_type == isDWARFExpression; }
+ bool IsConstant() const { return m_type == isConstant; }
+
+ void SetIsConstant(uint64_t value) {
+m_type = isConstant;
+m_location.constant_value = value;
+ }
+
+ uint64_t GetConstant() const { return m_location.constant_value; }
+
void SetAtCFAPlusOffset(int32_t offset) {
m_type = atCFAPlusOffset;
m_location.offset = offset;
@@ -192,6 +202,8 @@ class UnwindPlan {
const uint8_t *opcodes;
uint16_t length;
} expr;
+// For m_type == isConstant
+uint64_t constant_value;
} m_location;
};
@@ -358,6 +370,9 @@ class UnwindPlan {
bool SetRegisterLocationToSame(uint32_t reg_num, bool must_replace);
+bool SetRegisterLocationToIsConstant(uint32_t reg_num, uint64_t constant,
+ bool can_replace);
+
// When this UnspecifiedRegistersAreUndefined mode is
// set, any register that is not specified by this Row will
// be described as Undefined.
diff --git a/lldb/source/Symbol/UnwindPlan.cpp
b/lldb/source/Symbol/UnwindPlan.cpp
index e258a4e3d82f2..e2dbd81a82c84 100644
--- a/lldb/source/Symbol/UnwindPlan.cpp
+++ b/lldb/source/Symbol/UnwindPlan.cpp
@@ -46,6 +46,8 @@ operator==(const UnwindPlan::Row::RegisterLocation &rhs)
const {
return !memcmp(m_location.expr.opcodes, rhs.m_location.expr.opcodes,
m_location.expr.length);
break;
+case isConstant:
+ return m_location.constant_value == rhs.m_location.constant_value;
}
}
return false;
@@ -153,6 +155,9 @@ void UnwindPlan::Row::RegisterLocation::Dump(Stream &s,
if (m_type == atDWARFExpression)
s.PutChar(']');
} break;
+ case isConstant:
+s.Printf("=0x%" PRIx64, m_location.constant_value);
+break;
}
}
@@ -351,6 +356,18 @@ bool UnwindPlan::Row::SetRegisterLocationToSame(uint32_t
reg_num,
return true;
}
+bool UnwindPlan::Row::SetRegisterLocationToIsConstant(uint32_t reg_num,
+ uint64_t constant,
+ bool can_replace) {
+ if (!can_replace &&
+ m_register_locations.find(reg_num) != m_register_locations.end())
+return false;
+ RegisterLocation reg_loc;
+ reg_loc.SetIsConstant(constant);
+ m_register_locations[reg_num] = reg_loc;
+ return true;
+}
+
bool UnwindPlan::Row::operator==(const UnwindPlan::Row &rhs) const {
return m_offset == rhs.m_offset && m_cfa_value == rhs.m_cfa_value &&
m_afa_value == rhs.m_afa_value &&
diff --git a/lldb/source/Target/RegisterContextUnwind.cpp
b/lldb/source/Target/RegisterContextUnwind.cpp
index bc8081f4e3b31..a61228d092d89 100644
--- a/lldb/source/Target/RegisterContextUnwind.cpp
+++ b/lldb
[Lldb-commits] [lldb] [lldb] Add constant value mode for RegisterLocation in UnwindPlans (PR #100624)
https://github.com/felipepiovezan closed https://github.com/llvm/llvm-project/pull/100624 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a StackFrameRecognizer for the Darwin specific abort_with_payload… (PR #101365)
https://github.com/jimingham updated
https://github.com/llvm/llvm-project/pull/101365
>From 5b2cf0ab7e382e41ff3dfe88e4d7e35ddc99afd5 Mon Sep 17 00:00:00 2001
From: Jim Ingham
Date: Tue, 30 Jul 2024 09:32:24 -0700
Subject: [PATCH 1/2] Add a StackFrameRecognizer for the Darwin specific
abort_with_payload API.
---
lldb/include/lldb/Target/Process.h| 16 ++
.../Platform/MacOSX/PlatformDarwin.cpp| 45 ++--
.../AbortWithPayloadFrameRecognizer.cpp | 201 ++
.../MacOSX/AbortWithPayloadFrameRecognizer.h | 38
.../SystemRuntime/MacOSX/CMakeLists.txt | 1 +
.../MacOSX/SystemRuntimeMacOSX.cpp| 6 +-
lldb/source/Target/Process.cpp| 7 +-
.../API/macosx/abort_with_payload/Makefile| 4 +
.../TestAbortWithPayload.py | 146 +
.../test/API/macosx/abort_with_payload/main.c | 30 +++
10 files changed, 478 insertions(+), 16 deletions(-)
create mode 100644
lldb/source/Plugins/SystemRuntime/MacOSX/AbortWithPayloadFrameRecognizer.cpp
create mode 100644
lldb/source/Plugins/SystemRuntime/MacOSX/AbortWithPayloadFrameRecognizer.h
create mode 100644 lldb/test/API/macosx/abort_with_payload/Makefile
create mode 100644
lldb/test/API/macosx/abort_with_payload/TestAbortWithPayload.py
create mode 100644 lldb/test/API/macosx/abort_with_payload/main.c
diff --git a/lldb/include/lldb/Target/Process.h
b/lldb/include/lldb/Target/Process.h
index c8475db8ae160..84bbd8a52 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -2569,6 +2569,18 @@ void PruneThreadPlans();
/// A StructuredDataSP object which, if non-empty, will contain the
/// information related to the process.
virtual StructuredData::DictionarySP GetMetadata() { return nullptr; }
+
+ /// Fetch extended crash information held by the process. This will never be
+ /// an empty shared pointer, it will always have a dict, though it may be
+ /// empty.
+ StructuredData::DictionarySP GetExtendedCrashInfoDict() {
+return m_crash_info_dict_sp;
+ }
+
+ void ResetExtendedCrashInfoDict() {
+// StructuredData::Dictionary is add only, so we have to make a new one:
+m_crash_info_dict_sp.reset(new StructuredData::Dictionary());
+ }
size_t AddImageToken(lldb::addr_t image_ptr);
@@ -3185,6 +3197,10 @@ void PruneThreadPlans();
/// Per process source file cache.
SourceManager::SourceFileCache m_source_file_cache;
+
+ /// A repository for extra crash information, consulted in
+ /// GetExtendedCrashInformation.
+ StructuredData::DictionarySP m_crash_info_dict_sp;
size_t RemoveBreakpointOpcodesFromBuffer(lldb::addr_t addr, size_t size,
uint8_t *buf) const;
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 6fb5bbfbe417b..398b44c293614 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -856,21 +856,38 @@ PlatformDarwin::ParseVersionBuildDir(llvm::StringRef dir)
{
}
llvm::Expected
-PlatformDarwin::FetchExtendedCrashInformation(Process &process) {
- StructuredData::DictionarySP extended_crash_info =
- std::make_shared();
-
- StructuredData::ArraySP annotations = ExtractCrashInfoAnnotations(process);
- if (annotations && annotations->GetSize())
-extended_crash_info->AddItem("Crash-Info Annotations", annotations);
-
- StructuredData::DictionarySP app_specific_info =
- ExtractAppSpecificInfo(process);
- if (app_specific_info && app_specific_info->GetSize())
-extended_crash_info->AddItem("Application Specific Information",
- app_specific_info);
+PlatformDarwin::
+FetchExtendedCrashInformation(Process &process) {
+ static constexpr llvm::StringLiteral crash_info_key("Crash-Info
Annotations");
+ static constexpr llvm::StringLiteral asi_info_key("Application Specific
Information");
+
+ // We cache the information we find in the process extended info dict:
+ StructuredData::DictionarySP process_dict_sp
+ = process.GetExtendedCrashInfoDict();
+ StructuredData::Array *annotations = nullptr;
+ StructuredData::ArraySP new_annotations_sp;
+ if (!process_dict_sp->GetValueForKeyAsArray(crash_info_key, annotations)) {
+new_annotations_sp = ExtractCrashInfoAnnotations(process);
+if (new_annotations_sp && new_annotations_sp->GetSize()) {
+ process_dict_sp->AddItem(crash_info_key, new_annotations_sp);
+ annotations = new_annotations_sp.get();
+}
+ }
- return extended_crash_info->GetSize() ? extended_crash_info : nullptr;
+ StructuredData::Dictionary *app_specific_info;
+ StructuredData::DictionarySP new_app_specific_info_sp;
+ if (!process_dict_sp->GetValueForKeyAsDictionary(asi_info_key,
app_specific_info))
+ {
+new_app_specific_info_sp = ExtractAppSpecificInfo(proc
[Lldb-commits] [lldb] [lldb] Change Module to have a concrete UnwindTable, update (PR #101130)
https://github.com/JDevlieghere approved this pull request. This makes sense to me. LGTM if Jim is happy. https://github.com/llvm/llvm-project/pull/101130 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add a StackFrameRecognizer for the Darwin specific abort_with_payload… (PR #101365)
https://github.com/jimingham updated
https://github.com/llvm/llvm-project/pull/101365
>From 5b2cf0ab7e382e41ff3dfe88e4d7e35ddc99afd5 Mon Sep 17 00:00:00 2001
From: Jim Ingham
Date: Tue, 30 Jul 2024 09:32:24 -0700
Subject: [PATCH 1/3] Add a StackFrameRecognizer for the Darwin specific
abort_with_payload API.
---
lldb/include/lldb/Target/Process.h| 16 ++
.../Platform/MacOSX/PlatformDarwin.cpp| 45 ++--
.../AbortWithPayloadFrameRecognizer.cpp | 201 ++
.../MacOSX/AbortWithPayloadFrameRecognizer.h | 38
.../SystemRuntime/MacOSX/CMakeLists.txt | 1 +
.../MacOSX/SystemRuntimeMacOSX.cpp| 6 +-
lldb/source/Target/Process.cpp| 7 +-
.../API/macosx/abort_with_payload/Makefile| 4 +
.../TestAbortWithPayload.py | 146 +
.../test/API/macosx/abort_with_payload/main.c | 30 +++
10 files changed, 478 insertions(+), 16 deletions(-)
create mode 100644
lldb/source/Plugins/SystemRuntime/MacOSX/AbortWithPayloadFrameRecognizer.cpp
create mode 100644
lldb/source/Plugins/SystemRuntime/MacOSX/AbortWithPayloadFrameRecognizer.h
create mode 100644 lldb/test/API/macosx/abort_with_payload/Makefile
create mode 100644
lldb/test/API/macosx/abort_with_payload/TestAbortWithPayload.py
create mode 100644 lldb/test/API/macosx/abort_with_payload/main.c
diff --git a/lldb/include/lldb/Target/Process.h
b/lldb/include/lldb/Target/Process.h
index c8475db8ae160..84bbd8a52 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -2569,6 +2569,18 @@ void PruneThreadPlans();
/// A StructuredDataSP object which, if non-empty, will contain the
/// information related to the process.
virtual StructuredData::DictionarySP GetMetadata() { return nullptr; }
+
+ /// Fetch extended crash information held by the process. This will never be
+ /// an empty shared pointer, it will always have a dict, though it may be
+ /// empty.
+ StructuredData::DictionarySP GetExtendedCrashInfoDict() {
+return m_crash_info_dict_sp;
+ }
+
+ void ResetExtendedCrashInfoDict() {
+// StructuredData::Dictionary is add only, so we have to make a new one:
+m_crash_info_dict_sp.reset(new StructuredData::Dictionary());
+ }
size_t AddImageToken(lldb::addr_t image_ptr);
@@ -3185,6 +3197,10 @@ void PruneThreadPlans();
/// Per process source file cache.
SourceManager::SourceFileCache m_source_file_cache;
+
+ /// A repository for extra crash information, consulted in
+ /// GetExtendedCrashInformation.
+ StructuredData::DictionarySP m_crash_info_dict_sp;
size_t RemoveBreakpointOpcodesFromBuffer(lldb::addr_t addr, size_t size,
uint8_t *buf) const;
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 6fb5bbfbe417b..398b44c293614 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -856,21 +856,38 @@ PlatformDarwin::ParseVersionBuildDir(llvm::StringRef dir)
{
}
llvm::Expected
-PlatformDarwin::FetchExtendedCrashInformation(Process &process) {
- StructuredData::DictionarySP extended_crash_info =
- std::make_shared();
-
- StructuredData::ArraySP annotations = ExtractCrashInfoAnnotations(process);
- if (annotations && annotations->GetSize())
-extended_crash_info->AddItem("Crash-Info Annotations", annotations);
-
- StructuredData::DictionarySP app_specific_info =
- ExtractAppSpecificInfo(process);
- if (app_specific_info && app_specific_info->GetSize())
-extended_crash_info->AddItem("Application Specific Information",
- app_specific_info);
+PlatformDarwin::
+FetchExtendedCrashInformation(Process &process) {
+ static constexpr llvm::StringLiteral crash_info_key("Crash-Info
Annotations");
+ static constexpr llvm::StringLiteral asi_info_key("Application Specific
Information");
+
+ // We cache the information we find in the process extended info dict:
+ StructuredData::DictionarySP process_dict_sp
+ = process.GetExtendedCrashInfoDict();
+ StructuredData::Array *annotations = nullptr;
+ StructuredData::ArraySP new_annotations_sp;
+ if (!process_dict_sp->GetValueForKeyAsArray(crash_info_key, annotations)) {
+new_annotations_sp = ExtractCrashInfoAnnotations(process);
+if (new_annotations_sp && new_annotations_sp->GetSize()) {
+ process_dict_sp->AddItem(crash_info_key, new_annotations_sp);
+ annotations = new_annotations_sp.get();
+}
+ }
- return extended_crash_info->GetSize() ? extended_crash_info : nullptr;
+ StructuredData::Dictionary *app_specific_info;
+ StructuredData::DictionarySP new_app_specific_info_sp;
+ if (!process_dict_sp->GetValueForKeyAsDictionary(asi_info_key,
app_specific_info))
+ {
+new_app_specific_info_sp = ExtractAppSpecificInfo(proc
[Lldb-commits] [lldb] [lldb] Change Module to have a concrete UnwindTable, update (PR #101130)
https://github.com/jimingham approved this pull request. LGTM I like that you also changed "Update" to "ModuleWasUpdated", that's more accurate. `m_initialized` isn't quite the right name, it means "needs work" not "initialized" which sounds like it should happen once. If you can think of a better name for that off-hand, that would be a little clearer, but I can't think of a good one, and it's not crucial. https://github.com/llvm/llvm-project/pull/101130 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Allow mapping object file paths (PR #101361)
https://github.com/clayborg approved this pull request. Looks good! This will be very useful for core files. https://github.com/llvm/llvm-project/pull/101361 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 0a01e8f - [lldb] Allow mapping object file paths (#101361)
Author: Alexandre Perez
Date: 2024-07-31T10:57:40-07:00
New Revision: 0a01e8ff530ab15277aa9fad5361297d7b55e247
URL:
https://github.com/llvm/llvm-project/commit/0a01e8ff530ab15277aa9fad5361297d7b55e247
DIFF:
https://github.com/llvm/llvm-project/commit/0a01e8ff530ab15277aa9fad5361297d7b55e247.diff
LOG: [lldb] Allow mapping object file paths (#101361)
This introduces a `target.object-map` which allows us to remap module
locations, much in the same way as source mapping works today. This is
useful, for instance, when debugging coredumps, so we can replace some
of the locations where LLDB attempts to load shared libraries and
executables from, without having to setup an entire sysroot.
Added:
Modified:
lldb/include/lldb/Target/Target.h
lldb/source/Target/Target.cpp
lldb/source/Target/TargetProperties.td
lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
Removed:
diff --git a/lldb/include/lldb/Target/Target.h
b/lldb/include/lldb/Target/Target.h
index 5d5ae1bfcd3bd..119dff4d49819 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -141,6 +141,8 @@ class TargetProperties : public Properties {
PathMappingList &GetSourcePathMap() const;
+ PathMappingList &GetObjectPathMap() const;
+
bool GetAutoSourceMapRelative() const;
FileSpecList GetExecutableSearchPaths();
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index ec0da8a1378a8..129683c43f0c1 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -2155,12 +2155,21 @@ bool Target::ReadPointerFromMemory(const Address &addr,
Status &error,
return false;
}
-ModuleSP Target::GetOrCreateModule(const ModuleSpec &module_spec, bool notify,
- Status *error_ptr) {
+ModuleSP Target::GetOrCreateModule(const ModuleSpec &orig_module_spec,
+ bool notify, Status *error_ptr) {
ModuleSP module_sp;
Status error;
+ // Apply any remappings specified in target.object-map:
+ ModuleSpec module_spec(orig_module_spec);
+ PathMappingList &obj_mapping = GetObjectPathMap();
+ if (std::optional remapped_obj_file =
+ obj_mapping.RemapPath(orig_module_spec.GetFileSpec().GetPath(),
+true /* only_if_exists */)) {
+module_spec.GetFileSpec().SetPath(remapped_obj_file->GetPath());
+ }
+
// First see if we already have this module in our module list. If we do,
// then we're done, we don't need to consult the shared modules list. But
// only do this if we are passed a UUID.
@@ -4459,6 +4468,14 @@ PathMappingList &TargetProperties::GetSourcePathMap()
const {
return option_value->GetCurrentValue();
}
+PathMappingList &TargetProperties::GetObjectPathMap() const {
+ const uint32_t idx = ePropertyObjectMap;
+ OptionValuePathMappings *option_value =
+ m_collection_sp->GetPropertyAtIndexAsOptionValuePathMappings(idx);
+ assert(option_value);
+ return option_value->GetCurrentValue();
+}
+
bool TargetProperties::GetAutoSourceMapRelative() const {
const uint32_t idx = ePropertyAutoSourceMapRelative;
return GetPropertyAtIndexAs(
diff --git a/lldb/source/Target/TargetProperties.td
b/lldb/source/Target/TargetProperties.td
index 7f79218e0a6a4..4404a45492254 100644
--- a/lldb/source/Target/TargetProperties.td
+++ b/lldb/source/Target/TargetProperties.td
@@ -46,6 +46,9 @@ let Definition = "target" in {
def SourceMap: Property<"source-map", "PathMap">,
DefaultStringValue<"">,
Desc<"Source path remappings apply substitutions to the paths of source
files, typically needed to debug from a
diff erent host than the one that built the target. The source-map property
consists of an array of pairs, the first element is a path prefix, and the
second is its replacement. The syntax is `prefix1 replacement1 prefix2
replacement2...`. The pairs are checked in order, the first prefix that
matches is used, and that prefix is substituted with the replacement. A common
pattern is to use source-map in conjunction with the clang -fdebug-prefix-map
flag. In the build, use `-fdebug-prefix-map=/path/to/build_dir=.` to rewrite
the host specific build directory to `.`. Then for debugging, use `settings
set target.source-map . /path/to/local_dir` to convert `.` to a valid local
path.">;
+ def ObjectMap: Property<"object-map", "PathMap">,
+DefaultStringValue<"">,
+Desc<"Object path remappings apply substitutions to the paths of object
files, typically needed to debug from a
diff erent host than the one that built the target. The object-map property
consists of an array of pairs, the first element is a path prefix, and the
second is its replacement. The syntax is `prefix1 replacement1 prefix2
replacement2...`. The pairs are checked in order, the first prefix that
matches is used, and that
[Lldb-commits] [lldb] [lldb] Allow mapping object file paths (PR #101361)
https://github.com/aperez closed https://github.com/llvm/llvm-project/pull/101361 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
