From 3555e694a278efdcd5a53dd0d18413176a3f3ecb Mon Sep 17 00:00:00 2001 From: Okwudili Pat-Nebe <15070069+patnebe@users.noreply.github.com> Date: Mon, 25 Aug 2025 20:18:20 +0100 Subject: [PATCH 1/3] Test resuming a process --- include/libsdb/sdb_process.h | 11 ++++--- include/libsdb/sdb_process_state.h | 2 ++ src/sdb_pipe.cpp | 3 +- src/sdb_process.cpp | 46 ++++++++++++++++++++++++------ test/CMakeLists.txt | 2 ++ test/sdb_process.t.cpp | 45 ++++++++++++++++++++++++++--- test/testbin.m.cpp | 9 ++++++ 7 files changed, 97 insertions(+), 21 deletions(-) create mode 100644 test/testbin.m.cpp diff --git a/include/libsdb/sdb_process.h b/include/libsdb/sdb_process.h index ddbf168..6364fa3 100644 --- a/include/libsdb/sdb_process.h +++ b/include/libsdb/sdb_process.h @@ -41,19 +41,18 @@ class Process Process(pid_t pid, bool cleanupOnExit, bool isBeingTraced); private: - const pid_t d_pid; - + pid_t d_pid; ProcessState d_state; - const bool d_cleanupOnExit; - const bool d_isBeingTraced; + bool d_cleanupOnExit; + bool d_isBeingTraced; public: ~Process(); static ProcessUPtr attach(pid_t pid); static ProcessUPtr launch(std::filesystem::path path, bool traceProc = true); - Process(Process&&) = default; - Process& operator=(Process&&) = default; + Process(Process&&); + Process& operator=(Process&&); /* Resume the debugee process */ void resume(); diff --git a/include/libsdb/sdb_process_state.h b/include/libsdb/sdb_process_state.h index 81a4a68..eac174e 100644 --- a/include/libsdb/sdb_process_state.h +++ b/include/libsdb/sdb_process_state.h @@ -15,6 +15,8 @@ enum struct ProcessState e_TERMINATED }; +std::ostream& operator<<(std::ostream& os, ProcessState state); + } // sdb #endif diff --git a/src/sdb_pipe.cpp b/src/sdb_pipe.cpp index 4fb3e4a..451a1cd 100644 --- a/src/sdb_pipe.cpp +++ b/src/sdb_pipe.cpp @@ -9,7 +9,6 @@ namespace sdb { - namespace { static const int EMPTY_PIPE_FD = -1; @@ -17,7 +16,7 @@ static const int EMPTY_PIPE_FD = -1; Pipe::Pipe(bool closeOnExec) { - if (auto rc = pipe2(d_fds, 0 | (closeOnExec ? O_CLOEXEC : 0)); rc < 0) + if (auto rc = pipe2(d_fds, closeOnExec ? O_CLOEXEC : 0); rc < 0) { std::stringstream ss; ss << "Failed to create pipe, rc=" << rc; diff --git a/src/sdb_process.cpp b/src/sdb_process.cpp index ab4e28e..00b7afe 100644 --- a/src/sdb_process.cpp +++ b/src/sdb_process.cpp @@ -29,6 +29,31 @@ Process::Process(pid_t pid, bool cleanupOnExit, bool isBeingTraced) { } +Process::Process(Process&& other) + : d_pid(other.d_pid), d_state(other.d_state), + d_cleanupOnExit(other.d_cleanupOnExit), + d_isBeingTraced(other.d_isBeingTraced) +{ + // Ensure the underlying process is not cleaned + // when the object moved out of is destructed + other.d_pid = 0; + other.d_cleanupOnExit = false; +} + +Process& Process::operator=(Process&& other) +{ + d_pid = other.d_pid; + d_state = other.d_state; + d_cleanupOnExit = other.d_cleanupOnExit; + d_isBeingTraced = other.d_isBeingTraced; + + // Ensure the underlying process is not cleaned + // when the object moved out of is destructed + other.d_pid = 0; + other.d_cleanupOnExit = false; + return *this; +} + Process::~Process() { if (d_pid == 0) @@ -41,15 +66,15 @@ Process::~Process() return; } - if (d_state == ProcessState::e_RUNNING) - { - kill(d_pid, SIGSTOP); - waitOnSignal(); - } - - // Detach if (d_isBeingTraced) { + if (d_state == ProcessState::e_RUNNING) + { + kill(d_pid, SIGSTOP); + waitOnSignal(); + } + + // Detach if (auto rc = ptrace(PTRACE_DETACH, d_pid, /*addr=*/nullptr, /*data=*/nullptr); rc < 0) @@ -57,6 +82,7 @@ Process::~Process() std::cout << "Unable to detach from process, rc=" << rc << std::endl; return; } + kill(d_pid, SIGCONT); } @@ -157,11 +183,13 @@ void Process::resume() case ProcessState::e_STOPPED: if (int rc = ptrace(PTRACE_CONT, d_pid, nullptr, nullptr); rc < 0) { - std::cerr << "Unable to continue, rc=" << rc << std::endl; + std::cerr << "Unable to continue proc with pid=" << d_pid + << ", rc=" << rc << std::endl; + perror(""); std::exit(-1); } d_state = ProcessState::e_RUNNING; - waitOnSignal(); + // waitOnSignal(); break; default: std::cout << "Inferior process is not running" << std::endl; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index c32945b..97490c5 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -19,3 +19,5 @@ target_link_libraries(sdb.u.t include(GoogleTest) gtest_discover_tests(sdb.u.t) + +add_executable(testbin.tsk testbin.m.cpp) diff --git a/test/sdb_process.t.cpp b/test/sdb_process.t.cpp index f57ba8e..eac4231 100644 --- a/test/sdb_process.t.cpp +++ b/test/sdb_process.t.cpp @@ -1,10 +1,13 @@ #include +#include #include #include #include #include #include +#include +#include #include #include @@ -15,6 +18,7 @@ namespace { using namespace testing; +using namespace std::chrono_literals; bool processExists(int pid) { @@ -45,6 +49,12 @@ char getProcessStatus(int pid) const auto idxOfProcStatus = idxOfLastParen + 2; return buf[idxOfProcStatus]; } + +const auto processInTracingStoppedStatus + = [](int pid) { return getProcessStatus(pid) == 't'; }; + +const auto processInRunningStatus + = [](int pid) { return getProcessStatus(pid) == 'R'; }; } // namespace TEST(ProcessTest, processLaunchedSuccessfully) @@ -69,7 +79,9 @@ TEST(ProcessTest, launchNonExistentProgram) TEST(ProcessTest, attachToProcess) { // Given - const std::filesystem::path procname{ "ls" }; + const std::filesystem::path procname{ "./testbin.tsk" }; + // Don't trace the process on launch + // since we'll attach to it below const auto proc = Process::launch(procname, /*traceProc*/ false); ASSERT_THAT(proc, NotNull()); @@ -78,10 +90,35 @@ TEST(ProcessTest, attachToProcess) // Then ASSERT_EQ(proc->pid(), attachedProc->pid()); - auto processInTracingStoppedStatus - = [](int pid) { return getProcessStatus(pid) == 't'; }; - ASSERT_TRUE(processInTracingStoppedStatus(attachedProc->pid())); } +TEST(ProcessTest, attachToInvalidPid) +{ + // Given / When / Then + EXPECT_THROW(Process::attach(0), ProcessAttachError); +} + +TEST(ProcessTest, resumeAProcess) +{ + // Given + auto proc = Process::launch("./testbin.tsk"); + ASSERT_THAT(proc, NotNull()); + const auto pid = proc->pid(); + ASSERT_TRUE(processInTracingStoppedStatus(pid)); + const auto procState = proc->state(); + ASSERT_EQ(procState, ProcessState::e_STOPPED) + << "unexpected proc state=" << procState; + + // When + // TODO: Understand why this fails with the following + // error when I move it to a different thread:: + // Unable to continue proc with pid=137233, rc=-1 + // No such process + proc->resume(); + + // Then + ASSERT_TRUE(processInRunningStatus(pid)); +} + } // namespace sdb diff --git a/test/testbin.m.cpp b/test/testbin.m.cpp new file mode 100644 index 0000000..2dd2c72 --- /dev/null +++ b/test/testbin.m.cpp @@ -0,0 +1,9 @@ +#include +#include + +using namespace std::chrono_literals; + +int main() +{ + std::this_thread::sleep_for(5s); +} From 66ae5e0d2d295a9418589bb34789bfffa29798ba Mon Sep 17 00:00:00 2001 From: Okwudili Pat-Nebe <15070069+patnebe@users.noreply.github.com> Date: Mon, 25 Aug 2025 20:22:50 +0100 Subject: [PATCH 2/3] . --- src/sdb_process.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/sdb_process.cpp b/src/sdb_process.cpp index 00b7afe..4f4ec0f 100644 --- a/src/sdb_process.cpp +++ b/src/sdb_process.cpp @@ -183,9 +183,10 @@ void Process::resume() case ProcessState::e_STOPPED: if (int rc = ptrace(PTRACE_CONT, d_pid, nullptr, nullptr); rc < 0) { - std::cerr << "Unable to continue proc with pid=" << d_pid - << ", rc=" << rc << std::endl; - perror(""); + std::stringstream ss; + ss << "Unable to continue proc with pid=" << d_pid << ", rc=" << rc + << ". perror="; + perror(ss.str().c_str()); std::exit(-1); } d_state = ProcessState::e_RUNNING; From d957ccf5d8971a956fc0904da25dc7a8ef3e44d2 Mon Sep 17 00:00:00 2001 From: Okwudili Pat-Nebe <15070069+patnebe@users.noreply.github.com> Date: Mon, 25 Aug 2025 20:26:37 +0100 Subject: [PATCH 3/3] . --- src/sdb_process.cpp | 1 - test/sdb_process.t.cpp | 3 --- 2 files changed, 4 deletions(-) diff --git a/src/sdb_process.cpp b/src/sdb_process.cpp index 4f4ec0f..33a38b4 100644 --- a/src/sdb_process.cpp +++ b/src/sdb_process.cpp @@ -8,7 +8,6 @@ #include #include #include -#include #include namespace sdb diff --git a/test/sdb_process.t.cpp b/test/sdb_process.t.cpp index eac4231..d6fea5b 100644 --- a/test/sdb_process.t.cpp +++ b/test/sdb_process.t.cpp @@ -1,13 +1,11 @@ #include -#include #include #include #include #include #include #include -#include #include #include @@ -18,7 +16,6 @@ namespace { using namespace testing; -using namespace std::chrono_literals; bool processExists(int pid) {