Skip to content

Commit

Permalink
Record exit code when a detached proxy exits.
Browse files Browse the repository at this point in the history
This ensures we return the correct exit code when the main tracee is a detached proxy.
  • Loading branch information
rocallahan committed Nov 30, 2024
1 parent 59f1b7e commit 2c5a433
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 20 deletions.
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1719,6 +1719,7 @@ set(TESTS_WITHOUT_PROGRAM
nested_detach_kill
nested_detach_stop
nested_release
nested_release_exit_code
pack_dir
parent_no_break_child_bkpt
parent_no_stop_child_crash
Expand Down
14 changes: 3 additions & 11 deletions src/RecordSession.cc
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,6 @@ static void record_robust_futex_changes(RecordTask* t) {
RR_ARCH_FUNCTION(record_robust_futex_changes_arch, t->arch(), t);
}

static void record_exit_trace_event(RecordTask* t, WaitStatus exit_status) {
t->session().trace_writer().write_task_event(
TraceTaskEvent::for_exit(t->tid, exit_status));
if (t->thread_group()->tgid == t->tid) {
t->thread_group()->exit_status = exit_status;
}
}

static bool looks_like_syscall_entry(RecordTask* t) {
bool ok;
bool at_syscall = is_at_syscall_instruction(t,
Expand Down Expand Up @@ -298,7 +290,7 @@ static bool handle_ptrace_exit_event(RecordTask* t) {
// letting this task complete its exit.
bool may_wait_exit = !t->was_reaped() && !is_coredumping_signal(exit_status.fatal_sig()) &&
!t->waiting_for_pid_namespace_tasks_to_exit();
record_exit_trace_event(t, exit_status);
t->record_exit_trace_event(exit_status);
t->record_exit_event(
(!t->was_reaped() && !may_wait_exit) ? RecordTask::WRITE_CHILD_TID : RecordTask::KERNEL_WRITES_CHILD_TID);
if (!t->was_reaped()) {
Expand Down Expand Up @@ -766,7 +758,7 @@ bool RecordSession::handle_ptrace_event(RecordTask** t_ptr,
// address space.
pid_t tid = t->rec_tid;
WaitStatus status = t->status();
record_exit_trace_event(t, WaitStatus(0));
t->record_exit_trace_event(WaitStatus(0));
t->record_exit_event();
// Don't call RecordTask::destroy() because we don't want to
// PTRACE_DETACH.
Expand Down Expand Up @@ -1839,7 +1831,7 @@ bool RecordSession::signal_state_changed(RecordTask* t, StepState* step_state) {
// actually kill the task now that it isn't under ptrace control anymore.
t->destroy_buffers(nullptr, nullptr);
WaitStatus exit_status = WaitStatus::for_fatal_sig(sig);
record_exit_trace_event(t, exit_status);
t->record_exit_trace_event(exit_status);
// Allow writing child_tid now because otherwise the write will race
t->record_exit_event(RecordTask::WRITE_CHILD_TID);
// On a real affected kernel, we probably would have never gotten here,
Expand Down
8 changes: 8 additions & 0 deletions src/RecordTask.cc
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,14 @@ RecordTask::~RecordTask() {
session().on_destroy_record_task(this);
}

void RecordTask::record_exit_trace_event(WaitStatus exit_status) {
session().trace_writer().write_task_event(
TraceTaskEvent::for_exit(tid, exit_status));
if (thread_group()->tgid == tid) {
thread_group()->exit_status = exit_status;
}
}

void RecordTask::record_exit_event(WriteChildTid write_child_tid) {
// The kernel explicitly only clears the futex if the address space is shared.
// If the address space has no other users then the futex will not be cleared
Expand Down
2 changes: 2 additions & 0 deletions src/RecordTask.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@ class RecordTask final : public Task {
* If necessary, signal the ptracer that this task has exited.
*/
void do_ptrace_exit_stop(WaitStatus exit_status);

void record_exit_trace_event(WaitStatus exit_status);
/**
* Return the exit event.
* If write_child_tid is set, zero out child_tid now if applicable.
Expand Down
20 changes: 11 additions & 9 deletions src/Scheduler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -659,16 +659,18 @@ static RecordTask* find_waited_task(RecordSession& session, pid_t tid, WaitStatu
waited->emulated_stop_code = status;
parent->send_synthetic_SIGCHLD_if_necessary();
}

if (!parent &&
(status.type() == WaitStatus::EXIT || status.type() == WaitStatus::FATAL_SIGNAL)) {
// The task is now dead, but so is our parent, so none of our
// tasks care about this. We can now delete the proxy task.
// This will also reap the rec_tid of the proxy task.
delete waited;
// If there is a parent, we'll kill this task when the parent reaps it
// in our wait() emulation.
if (status.type() == WaitStatus::EXIT || status.type() == WaitStatus::FATAL_SIGNAL) {
waited->record_exit_trace_event(status);
if (!parent) {
// The task is now dead, but so is our parent, so none of our
// tasks care about this. We can now delete the proxy task.
// This will also reap the rec_tid of the proxy task.
delete waited;
// If there is a parent, we'll kill this task when the parent reaps it
// in our wait() emulation.
}
}

return nullptr;
}
return waited;
Expand Down
3 changes: 3 additions & 0 deletions src/test/nested_release_exit_code.run
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
source `dirname $0`/util.sh
just_record $RR_EXE "record --nested=release false" && failed "error not propagated"
passed

0 comments on commit 2c5a433

Please sign in to comment.