From 1fcaa83692d4ebe9cb97ca181758827cf4a67418 Mon Sep 17 00:00:00 2001 From: Saleem Abdulrasool Date: Tue, 28 Jul 2026 15:13:33 -0700 Subject: [PATCH] GDBRemote: resolve relative vFile paths against the session's working directory onFileOpen and its siblings in FileOperationsMixin passed vFile paths straight through to Host::File, never consulting the working directory set via QSetWorkingDir, unlike qPlatform_shell and process launch. This is why lldbutil.wait_for_file_on_target's ls-then-Get() sequence diverged: the shell-based ls resolves relative paths against the platform working directory (qPlatform_shell carries an explicit cwd field), but the vFile:open packet that immediately follows carries no such field, so ds2 has no way to know it unless it tracks the field from QSetWorkingDir itself. Resolve relative vFile paths against each session's own _workingDirectory rather than ds2's actual process cwd: PlatformMain in Sources/main.cpp serves every connected platform client on its own thread with its own PlatformSessionImpl, so the process cwd is shared, mutable state across all of them, and mutating it per-session would let concurrent clients stomp on each other's working directory. onSetWorkingDirectory canonicalizes the incoming path to absolute the moment it's set (a relative path only means something relative to ds2's own starting directory, since ds2's cwd never changes), rather than storing it verbatim. This also keeps ProcessSpawner's own chdir() in the forked child working correctly for a relative QSetWorkingDir: storing the raw relative string and letting both the vFile join and the spawner's chdir() apply it independently would resolve it against two different bases and risk applying it twice. Verified against a rebuilt android-x86_64 ds2 on the local emulator: TestHelloWorld.test_with_attach_to_process_with_id_api passes (was failing with "unable to open source file"); a raw protocol probe confirms a relative QSetWorkingDir resolves once (no doubling) and that two concurrent sessions keep independent working directories; and the sanity/api/android categories show no regressions. --- .../GDBRemote/Mixins/FileOperationsMixin.h | 11 +++++ .../GDBRemote/Mixins/FileOperationsMixin.hpp | 40 ++++++++++++++----- .../GDBRemote/Mixins/ProcessLaunchMixin.hpp | 19 ++++++++- 3 files changed, 59 insertions(+), 11 deletions(-) diff --git a/Headers/DebugServer2/GDBRemote/Mixins/FileOperationsMixin.h b/Headers/DebugServer2/GDBRemote/Mixins/FileOperationsMixin.h index 522fb363..dbe12616 100644 --- a/Headers/DebugServer2/GDBRemote/Mixins/FileOperationsMixin.h +++ b/Headers/DebugServer2/GDBRemote/Mixins/FileOperationsMixin.h @@ -30,6 +30,17 @@ template class FileOperationsMixin : public T { explicit FileOperationsMixin(Args &&... args) : T(std::forward(args)...) {} +private: + // vFile paths are frequently relative (e.g. a bare filename an inferior + // writes into its own working directory); unlike qPlatform_shell, the + // vFile packets carry no separate working-directory field, so ds2 has to + // apply the platform session's own configured working directory itself. + // This has to stay session-local rather than mutating ds2's actual + // process cwd: platform mode serves each connected client on its own + // thread with its own session (see PlatformMain in Sources/main.cpp), and + // the process cwd is shared state across all of them. + std::string resolvePath(Session &session, std::string const &path) const; + protected: ErrorCode onFileOpen(Session &session, std::string const &path, OpenFlags flags, uint32_t mode, int &fd) override; diff --git a/Sources/GDBRemote/Mixins/FileOperationsMixin.hpp b/Sources/GDBRemote/Mixins/FileOperationsMixin.hpp index c067d7a3..96f5817f 100644 --- a/Sources/GDBRemote/Mixins/FileOperationsMixin.hpp +++ b/Sources/GDBRemote/Mixins/FileOperationsMixin.hpp @@ -13,6 +13,7 @@ #include "DebugServer2/GDBRemote/Mixins/FileOperationsMixin.h" #include "DebugServer2/Host/Platform.h" +#include #include #include @@ -23,12 +24,30 @@ namespace ds2 { namespace GDBRemote { template -ErrorCode FileOperationsMixin::onFileOpen(Session &, std::string const &path, +std::string FileOperationsMixin::resolvePath(Session &session, + std::string const &path) const { + if (std::filesystem::path(path).is_absolute()) { + return path; + } + + std::string workingDirectory; + if (this->onQueryWorkingDirectory(session, workingDirectory) != kSuccess || + workingDirectory.empty()) { + return path; + } + + // onSetWorkingDirectory canonicalizes to an absolute path before storing + // it, so this join always yields an absolute result. + return (std::filesystem::path(workingDirectory) / path).string(); +} + +template +ErrorCode FileOperationsMixin::onFileOpen(Session &session, std::string const &path, OpenFlags flags, uint32_t mode, int &fd) { static int fileIdx = 0; - Host::File file(path, flags, mode); + Host::File file(resolvePath(session, path), flags, mode); if (!file.valid()) { return file.lastError(); } @@ -76,29 +95,30 @@ ErrorCode FileOperationsMixin::onFileWrite(Session &session, int fd, } template -ErrorCode FileOperationsMixin::onFileCreateDirectory(Session &, +ErrorCode FileOperationsMixin::onFileCreateDirectory(Session &session, std::string const &path, uint32_t flags) { - return Host::File::createDirectory(path, flags); + return Host::File::createDirectory(resolvePath(session, path), flags); } template -ErrorCode FileOperationsMixin::onFileExists(Session &, +ErrorCode FileOperationsMixin::onFileExists(Session &session, std::string const &path) { - return Host::Platform::IsFilePresent(path) ? kSuccess : kErrorNotFound; + return Host::Platform::IsFilePresent(resolvePath(session, path)) ? kSuccess + : kErrorNotFound; } template ErrorCode FileOperationsMixin::onFileGetSize(Session &session, std::string const &path, uint64_t &size){ - return Host::File::fileSize(path, size); + return Host::File::fileSize(resolvePath(session, path), size); } template ErrorCode FileOperationsMixin::onFileGetMode(Session &session, std::string const &path, uint32_t &mode) const { - return Host::File::fileMode(path, mode); + return Host::File::fileMode(resolvePath(session, path), mode); } template @@ -114,14 +134,14 @@ ErrorCode FileOperationsMixin::onFileFstat(Session &session, int fd, template ErrorCode FileOperationsMixin::onFileRemove(Session &session, std::string const &path) { - return Host::File::unlink(path); + return Host::File::unlink(resolvePath(session, path)); } template ErrorCode FileOperationsMixin::onFileSetPermissions(Session &session, std::string const &path, uint32_t mode) { - return Host::File::chmod(path, mode); + return Host::File::chmod(resolvePath(session, path), mode); } template diff --git a/Sources/GDBRemote/Mixins/ProcessLaunchMixin.hpp b/Sources/GDBRemote/Mixins/ProcessLaunchMixin.hpp index 5a78fd14..00b5809d 100644 --- a/Sources/GDBRemote/Mixins/ProcessLaunchMixin.hpp +++ b/Sources/GDBRemote/Mixins/ProcessLaunchMixin.hpp @@ -13,6 +13,7 @@ #include "DebugServer2/GDBRemote/Mixins/ProcessLaunchMixin.h" #include +#include namespace ds2 { namespace GDBRemote { @@ -35,7 +36,23 @@ template ErrorCode ProcessLaunchMixin::onSetWorkingDirectory(Session &, std::string const &path) { - _workingDirectory = path; + // ds2 serves each platform client on its own thread with its own session + // (see PlatformMain in Sources/main.cpp), so the working directory has to + // stay session-local state rather than ds2's actual process cwd, which is + // shared by every concurrently-connected client. + // + // Canonicalize to an absolute path here, once, rather than storing + // whatever the client sent verbatim: a relative path only means + // something relative to ds2's own starting directory (there is nothing + // else for it to be relative to, since ds2's cwd never changes), and + // resolving it eagerly means every later consumer of _workingDirectory + // (vFile path resolution, ProcessSpawner) can treat it as absolute + // without redoing this resolution or applying it a second time. + std::filesystem::path resolved(path); + if (!resolved.is_absolute()) + resolved = std::filesystem::path(Platform::GetWorkingDirectory()) / resolved; + + _workingDirectory = resolved.string(); return kSuccess; }