diff --git a/src/AutoRemoteSyscalls.cc b/src/AutoRemoteSyscalls.cc index b547b91814f..2b60536e543 100644 --- a/src/AutoRemoteSyscalls.cc +++ b/src/AutoRemoteSyscalls.cc @@ -67,14 +67,7 @@ AutoRestoreMem::~AutoRestoreMem() { remote.task()->write_bytes_helper(addr, len, data.data()); } remote.regs().set_sp(remote.regs().sp() + len); - if (remote.task()->is_stopped()) { - remote.task()->set_regs(remote.regs()); - // If the task is not stopped it must have been kicked out of a - // stop via SIGKILL or equivalent. In that case we should not be - // trying to do anything more with it until restore_state_to(), - // which will clean things up, so it doesn't matter if we - // set the task's registers here. - } + remote.task()->set_regs(remote.regs()); } static bool is_SIGTRAP_default_and_unblocked(Task* t) { @@ -270,7 +263,7 @@ void AutoRemoteSyscalls::restore_state_to(Task* t) { // Don't restore status; callers need to see the task is exiting. // And the other stuff we don't below won't work. // But do restore registers so it looks like the exit happened in a clean state. - t->override_regs_during_exit(regs); + t->set_regs(regs); return; } diff --git a/src/Task.cc b/src/Task.cc index a27333ef7fb..fdc449ddb5b 100644 --- a/src/Task.cc +++ b/src/Task.cc @@ -90,6 +90,7 @@ Task::Task(Session& session, pid_t _tid, pid_t _rec_tid, uint32_t serial, last_resume_orig_cx(0), did_set_breakpoint_after_cpuid(false), is_stopped_(false), + in_unexpected_exit(false), seccomp_bpf_enabled(false), registers_dirty(false), orig_syscallno_dirty(false), @@ -976,7 +977,7 @@ bool Task::exit_syscall_and_prepare_restart() { // make it look like we really entered the syscall. Then // handle_ptrace_exit_event will record something appropriate. r.emulate_syscall_entry(); - override_regs_during_exit(r); + set_regs(r); return false; } LOG(debug) << "exit_syscall_and_prepare_restart done"; @@ -1100,7 +1101,10 @@ string Task::read_c_str(remote_ptr child_addr, bool *ok) { } const Registers& Task::regs() const { - ASSERT(this, is_stopped_ || was_reaped_); + // If we're in an unexpected exit then the tracee may + // not be stopped but we know its registers won't change again, + // so it's safe to ask for them here. + ASSERT(this, is_stopped_ || was_reaped_ || in_unexpected_exit); return registers; } @@ -1566,7 +1570,8 @@ bool Task::resume_execution(ResumeRequest how, WaitRequest wait_how, } void Task::set_regs(const Registers& regs) { - ASSERT(this, is_stopped_); + // Only allow registers to be set while our copy is the source of truth. + ASSERT(this, is_stopped_ || in_unexpected_exit); if (registers.original_syscallno() != regs.original_syscallno()) { orig_syscallno_dirty = true; } @@ -1577,12 +1582,6 @@ void Task::set_regs(const Registers& regs) { } } -void Task::override_regs_during_exit(const Registers& regs) { - orig_syscallno_dirty = true; - registers_dirty = true; - registers = regs; -} - void Task::flush_regs() { if (registers_dirty) { LOG(debug) << "Flushing registers for tid " << tid << " " << registers; @@ -2171,6 +2170,7 @@ bool Task::did_waitpid(WaitStatus status) { LOG(debug) << "Unexpected process death getting siginfo for " << tid; // Let's pretend this stop never happened. set_stopped(false); + in_unexpected_exit = true; return false; } } @@ -2231,6 +2231,7 @@ bool Task::did_waitpid(WaitStatus status) { // but in that case we're going to ignore this signal-stop // so it doesn't matter. set_stopped(false); + in_unexpected_exit = true; return false; } } diff --git a/src/Task.h b/src/Task.h index 81b5ac1dc4b..cd144d926b6 100644 --- a/src/Task.h +++ b/src/Task.h @@ -598,13 +598,6 @@ class Task { /** Set the tracee's registers to |regs|. Lazy. */ void set_regs(const Registers& regs); - /** Set the tracee's registers to |regs|. The task - * is known to be exiting (running towards, or actually in, a - * not-yet-reported PTRACE_EVENT_EXIT or reap) so its real registers - * won't change underneath us, but we need to override those - * registers with our values for recording purposes. */ - void override_regs_during_exit(const Registers& regs); - /** Ensure registers are flushed back to the underlying task. */ void flush_regs(); @@ -1275,7 +1268,12 @@ class Task { // Count of all ticks seen by this task since tracees became // consistent and the task last wait()ed. Ticks ticks; - // When |is_stopped_|, these are our child registers. + // Copy of the child registers. + // When is_stopped_ or in_unexpected_exit, these are the source of + // truth. Otherwise the child is running and the registers could be + // changed by the kernel or user-space execution, and the values here + // are meaningless. + // See also registers_dirty. Registers registers; // Where we last resumed execution remote_code_ptr address_of_last_execution_resume; @@ -1298,6 +1296,9 @@ class Task { // without our knowledge, due to a SIGKILL or equivalent such as // zap_pid_ns_processes. bool is_stopped_; + // True when we've been kicked out of a ptrace-stop via SIGKILL or + // equivalent. + bool in_unexpected_exit; /* True when the seccomp filter has been enabled via prctl(). This happens * in the first system call issued by the initial tracee (after it returns * from kill(SIGSTOP) to synchronize with the tracer). */ diff --git a/src/record_syscall.cc b/src/record_syscall.cc index 15aeff64710..8e4ae189e22 100644 --- a/src/record_syscall.cc +++ b/src/record_syscall.cc @@ -6970,11 +6970,7 @@ void rec_process_syscall(RecordTask* t) { aarch64_kernel_bug_workaround(t, syscall_state); - if (t->is_stopped()) { - t->on_syscall_exit(sys_ev.number, sys_ev.arch(), t->regs()); - // If t was killed via SIGKILL or equivalent, these post-syscall - // state updates don't matter. - } + t->on_syscall_exit(sys_ev.number, sys_ev.arch(), t->regs()); t->syscall_state = nullptr; MonitoredSharedMemory::check_all(t);