From 1cdfd7849d1fb2d5f622b3e70b05ce05d25eec57 Mon Sep 17 00:00:00 2001 From: Kyle Huey Date: Thu, 28 Mar 2024 13:52:46 +0000 Subject: [PATCH] Rejigger output mechanism from script to just use pipes. --- scripts/rr-gdb-script-host.py | 58 ++++++++++------------ src/SourcesCommand.cc | 91 +++++++++++++++++++---------------- 2 files changed, 74 insertions(+), 75 deletions(-) diff --git a/scripts/rr-gdb-script-host.py b/scripts/rr-gdb-script-host.py index 34c878a69c6..22a9647452d 100755 --- a/scripts/rr-gdb-script-host.py +++ b/scripts/rr-gdb-script-host.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -""" rr-gdb-script-host.py """ +""" rr-gdb-script-host.py """ # Performs a limited emulation of the runtime environment of gdb scripts so that # we can run them for `rr sources` to locate debug information. from typing import Optional, List, Callable @@ -157,36 +157,28 @@ def events(self) -> GdbApiEvents: self._events = GdbApiEvents(self.gdb) return self._events -def synchronize(): - os.kill(os.getpid(), signal.SIGSTOP) - if __name__ == '__main__': - with open(sys.argv[1], 'w') as output: - with open(sys.argv[2], 'r') as user_script_file: - user_script = user_script_file.read() - host = GdbScriptHost(sys.argv[3]) - try: - host.execute_script(user_script) - except NameError as e: - if getattr(e, "name", None) == "python" or e == "NameError: name 'python' is not defined": - # This might be a gdb script that wraps a python script. - start = user_script.find("python") + len("python") - end = user_script.find("end") - if end == -1: - raise(e) - user_script = user_script[start:end] - try: - host.execute_script(user_script) - except: - raise(e) - - print(host.debug_file_directory, file=output, flush=True) - print(host._dir, file=output, flush=True) - synchronize() - for line in sys.stdin: - line = line.rstrip() - logging.debug("Processing %s"%line) - host.new_objfile(line) - print(host.debug_file_directory, file=output, flush=True) - print(host._dir, file=output, flush=True) - synchronize() + with open(sys.argv[1], 'r') as user_script_file: + user_script = user_script_file.read() + host = GdbScriptHost(sys.argv[2]) + try: + host.execute_script(user_script) + except NameError as e: + if getattr(e, "name", None) == "python" or e == "NameError: name 'python' is not defined": + # This might be a gdb script that wraps a python script. + start = user_script.find("python") + len("python") + end = user_script.find("end") + if end == -1: + raise(e) + user_script = user_script[start:end] + try: + host.execute_script(user_script) + except: + raise(e) + + print("%s\n%s" % (host.debug_file_directory, host._dir), flush=True) + for line in sys.stdin: + line = line.rstrip() + logging.debug("Processing %s"%line) + host.new_objfile(line) + print("%s\n%s" % (host.debug_file_directory, host._dir), flush=True) diff --git a/src/SourcesCommand.cc b/src/SourcesCommand.cc index cda6419acb0..aae6f55c915 100644 --- a/src/SourcesCommand.cc +++ b/src/SourcesCommand.cc @@ -148,8 +148,8 @@ struct DebugDirs { }; /// Manages integration with rr-gdb-script-host.py to allow a gdb script to -/// control which directories we search. If pipe_fd is open we have a -/// python child process. If pipe_fd is closed then everything here +/// control which directories we search. If input_pipe_fd is open we have a +/// python child process. If input_pipe_fd is closed then everything here /// becomes a no-op. class DebugDirManager { public: @@ -165,44 +165,42 @@ class DebugDirManager { DebugDirManager(const DebugDirManager&) = delete; DebugDirManager& operator=(const DebugDirManager&) = delete; - void synchronize(); DebugDirs read_result(); - ScopedFd pipe_fd; + ScopedFd input_pipe_fd; + ScopedFd output_pipe_fd; pid_t pid; - FILE* output_file; }; DebugDirManager::~DebugDirManager() { - if (!pipe_fd.is_open()) { + if (!input_pipe_fd.is_open()) { return; } - pipe_fd.close(); + input_pipe_fd.close(); + output_pipe_fd.close(); int status; if (waitpid(pid, &status, 0) == -1) { FATAL() << "Failed to wait on gdb script host"; } - - if (!(WIFEXITED(status) && WEXITSTATUS(status) == 0)) { - FATAL() << "gdb script host in unexpected state " << HEX(status); - } } DebugDirManager::DebugDirManager(const string& program, const string& gdb_script) - : pid(-1), output_file(nullptr) + : pid(-1) { if (gdb_script.empty()) { return; } - TempFile output_file = create_temporary_file("rr-gdb-script-host-output-XXXXXX"); - int stdin_pipe_fds[2]; if (pipe(stdin_pipe_fds) == -1) { FATAL(); } + int stdout_pipe_fds[2]; + if (pipe(stdout_pipe_fds) == -1) { + FATAL(); + } posix_spawn_file_actions_t file_actions; int ret = posix_spawn_file_actions_init(&file_actions); @@ -216,15 +214,27 @@ DebugDirManager::DebugDirManager(const string& program, const string& gdb_script FATAL() << "posix_spawn_file_actions_addclose failed with " << ret; } + // Close unused read end in the child. + ret = posix_spawn_file_actions_addclose(&file_actions, stdout_pipe_fds[0]); + if (ret != 0) { + FATAL() << "posix_spawn_file_actions_addclose failed with " << ret; + } + // Replace child's stdin with the read end. ret = posix_spawn_file_actions_adddup2(&file_actions, stdin_pipe_fds[0], 0); if (ret != 0) { FATAL() << "posix_spawn_file_actions_adddup2 failed with " << ret; } + // Replace child's stdout with the write end. + ret = posix_spawn_file_actions_adddup2(&file_actions, stdout_pipe_fds[1], 1); + if (ret != 0) { + FATAL() << "posix_spawn_file_actions_adddup2 failed with " << ret; + } + string gdb_script_host_path = resource_path() + "bin/rr-gdb-script-host.py"; pid_t pid; - vector gdb_script_host_argv_vec = { gdb_script_host_path, output_file.name, gdb_script, program }; + vector gdb_script_host_argv_vec = { gdb_script_host_path, gdb_script, program }; StringVectorToCharArray gdb_script_host_argv(gdb_script_host_argv_vec); ret = posix_spawn(&pid, gdb_script_host_path.c_str(), &file_actions, nullptr, gdb_script_host_argv.get(), environ); @@ -236,33 +246,28 @@ DebugDirManager::DebugDirManager(const string& program, const string& gdb_script posix_spawn_file_actions_destroy(&file_actions); close(stdin_pipe_fds[0]); + close(stdout_pipe_fds[1]); this->pid = pid; - this->pipe_fd = ScopedFd(stdin_pipe_fds[1]); - this->output_file = fopen(output_file.name.c_str(), "r"); - if (!this->output_file) { - FATAL() << "Failed to open gdb script host result file " << output_file.name; - } - - synchronize(); + this->input_pipe_fd = ScopedFd(stdin_pipe_fds[1]); + this->output_pipe_fd = ScopedFd(stdout_pipe_fds[0]); } DebugDirs DebugDirManager::process_one_binary(const string& binary_path) { - if (!pipe_fd.is_open()) { + if (!input_pipe_fd.is_open()) { return DebugDirs(); } auto len = binary_path.length(); - size_t written = write(pipe_fd, binary_path.c_str(), len); + size_t written = write(input_pipe_fd, binary_path.c_str(), len); if (written != len) { FATAL() << "Failed to write filename"; } - written = write(pipe_fd, "\n", 1); + written = write(input_pipe_fd, "\n", 1); if (written != 1) { FATAL() << "Failed to write trailing newline"; } - synchronize(); return read_result(); } @@ -271,12 +276,26 @@ DebugDirs DebugDirManager::read_result() { DebugDirs result; size_t index; const char delimiter[2] = ":"; + int output_fd; + FILE* output = NULL; - if (!pipe_fd.is_open()) { + if (!input_pipe_fd.is_open()) { return result; } - if (!fgets(buf, sizeof(buf) - 1, output_file)) { + // Convert our fd to a FILE so we can use fgets instead of writing + // our own buffering code. + output_fd = dup(output_pipe_fd); + if (output_fd < 0) { + FATAL() << "Failed to dup output_pipe_fd"; + } + + output = fdopen(output_fd, "r"); + if (!output) { + FATAL() << "Failed to fdopen(output_fd)"; + } + + if (!fgets(buf, sizeof(buf) - 1, output)) { FATAL() << "Failed to read gdb script output"; } index = strcspn(buf, "\n"); @@ -291,7 +310,7 @@ DebugDirs DebugDirManager::read_result() { token = strtok(nullptr, delimiter); } - if (!fgets(buf, sizeof(buf) - 1, output_file)) { + if (!fgets(buf, sizeof(buf) - 1, output)) { FATAL() << "Failed to read gdb script output"; } index = strcspn(buf, "\n"); @@ -311,22 +330,10 @@ DebugDirs DebugDirManager::read_result() { token = strtok(nullptr, delimiter); } + fclose(output); return result; } -void DebugDirManager::synchronize() { - int status; - if (waitpid(pid, &status, WUNTRACED) == -1) { - FATAL() << "Failed to wait on gdb script host"; - } - - if (!(WIFSTOPPED(status) && WSTOPSIG(status) == SIGSTOP)) { - FATAL() << "gdb script host in unexpected state " << HEX(status); - } - - kill(pid, SIGCONT); -} - // Resolve a file name relative to a compilation directory and relative directory. // file_name cannot be null, but the others can be. // Takes into account the original file name as follows: