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..33a38b4 100644 --- a/src/sdb_process.cpp +++ b/src/sdb_process.cpp @@ -8,7 +8,6 @@ #include #include #include -#include #include namespace sdb @@ -29,6 +28,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 +65,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 +81,7 @@ Process::~Process() std::cout << "Unable to detach from process, rc=" << rc << std::endl; return; } + kill(d_pid, SIGCONT); } @@ -157,11 +182,14 @@ 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::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; - 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..d6fea5b 100644 --- a/test/sdb_process.t.cpp +++ b/test/sdb_process.t.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -45,6 +46,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 +76,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 +87,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); +}