Changeset View
Standalone View
sys/kern/subr_syscall.c
| Show First 20 Lines • Show All 62 Lines • ▼ Show 20 Lines | syscallenter(struct thread *td, struct syscall_args *sa) | ||||
| td->td_pticks = 0; | td->td_pticks = 0; | ||||
| if (td->td_cowgen != p->p_cowgen) | if (td->td_cowgen != p->p_cowgen) | ||||
| thread_cow_update(td); | thread_cow_update(td); | ||||
| if (p->p_flag & P_TRACED) { | if (p->p_flag & P_TRACED) { | ||||
| traced = 1; | traced = 1; | ||||
| PROC_LOCK(p); | PROC_LOCK(p); | ||||
| td->td_dbgflags &= ~TDB_USERWR; | td->td_dbgflags &= ~TDB_USERWR; | ||||
| td->td_dbgflags |= TDB_SCE; | td->td_dbgflags |= TDB_SCE; | ||||
| td->td_dbgsa = sa; | |||||
kib: You cannot do it this way. struct syscall_args is typically allocated on the thread stack… | |||||
Not Done Inline ActionsSo one slightly annoying thing here is that sa->code and sa->narg aren't valid yet when we set TDB_SCE. For now I've deferred setting the td_dbg_sc_* fields until we call ptracestop(). If we want these fields to always be valid when TDB_SCE is set, then we either need to defer setting TDB_SCE or move this section after the call to sv_fetch_syscall_args so that sa->code and sa->narg are valid. I would probably prefer the latter, but I'm not sure of the implications of deferring the set of TDB_SCE until after the call to fetch_syscall_args, especially in regards to TDB_USERWR. jhb: So one slightly annoying thing here is that sa->code and sa->narg aren't valid yet when we set… | |||||
Not Done Inline ActionsBut TDB_ flags are not really usable until the debugee is at the stop point. That is, I do not see much difference between having TDB_SCE set where it is now, and moving it to the ptracestop() location. Except possibly one less PROC_LOCK(). kib: But TDB_ flags are not really usable until the debugee is at the stop point. That is, I do not… | |||||
Not Done Inline ActionsAfter talking more on IRC, we realized that moving TDB_SCE down would be a bug because it would not properly report PL_FLAG_SCE if a stop inside of a system call triggered but the debugger had not enabled system call stops, so I will leave TDB_SCE where it is. jhb: After talking more on IRC, we realized that moving TDB_SCE down would be a bug because it would… | |||||
| PROC_UNLOCK(p); | PROC_UNLOCK(p); | ||||
| } else | } else | ||||
| traced = 0; | traced = 0; | ||||
| error = (p->p_sysent->sv_fetch_syscall_args)(td, sa); | error = (p->p_sysent->sv_fetch_syscall_args)(td, sa); | ||||
| #ifdef KTRACE | #ifdef KTRACE | ||||
| if (KTRPOINT(td, KTR_SYSCALL)) | if (KTRPOINT(td, KTR_SYSCALL)) | ||||
| ktrsyscall(sa->code, sa->narg, sa->args); | ktrsyscall(sa->code, sa->narg, sa->args); | ||||
| #endif | #endif | ||||
| ▲ Show 20 Lines • Show All 73 Lines • ▼ Show 20 Lines | |||||
| retval: | retval: | ||||
| KTR_STOP4(KTR_SYSC, "syscall", syscallname(p, sa->code), | KTR_STOP4(KTR_SYSC, "syscall", syscallname(p, sa->code), | ||||
| (uintptr_t)td, "pid:%d", td->td_proc->p_pid, "error:%d", error, | (uintptr_t)td, "pid:%d", td->td_proc->p_pid, "error:%d", error, | ||||
| "retval0:%#lx", td->td_retval[0], "retval1:%#lx", | "retval0:%#lx", td->td_retval[0], "retval1:%#lx", | ||||
| td->td_retval[1]); | td->td_retval[1]); | ||||
| if (traced) { | if (traced) { | ||||
| PROC_LOCK(p); | PROC_LOCK(p); | ||||
| td->td_dbgflags &= ~TDB_SCE; | td->td_dbgflags &= ~TDB_SCE; | ||||
| td->td_dbgsa = NULL; | |||||
kibUnsubmitted Done Inline ActionsWhy not keeping the (analog of) td_dbgsa for the whole syscall processing duration ? Setting pl_syscall_* should be directed by TDB_SCE/TDB_SCX, then you do not need to temporary NULLify the data. kib: Why not keeping the (analog of) td_dbgsa for the whole syscall processing duration ? Setting… | |||||
| PROC_UNLOCK(p); | PROC_UNLOCK(p); | ||||
| } | } | ||||
| (p->p_sysent->sv_set_syscall_retval)(td, error); | (p->p_sysent->sv_set_syscall_retval)(td, error); | ||||
| return (error); | return (error); | ||||
| } | } | ||||
| static inline void | static inline void | ||||
| syscallret(struct thread *td, int error, struct syscall_args *sa __unused) | syscallret(struct thread *td, int error, struct syscall_args *sa) | ||||
Done Inline ActionsIs this now stray change ? kib: Is this now stray change ? | |||||
Not Done Inline ActionsYes, though it is actually used, so it is more correct. I can do it separately. jhb: Yes, though it is actually used, so it is more correct. I can do it separately. | |||||
Not Done Inline ActionsI am fine with either. It is more logical to make a separate commit. kib: I am fine with either. It is more logical to make a separate commit. | |||||
| { | { | ||||
| struct proc *p, *p2; | struct proc *p, *p2; | ||||
| int traced; | int traced; | ||||
| p = td->td_proc; | p = td->td_proc; | ||||
| /* | /* | ||||
| * Handle reschedule and other end-of-syscall issues | * Handle reschedule and other end-of-syscall issues | ||||
| */ | */ | ||||
| userret(td, td->td_frame); | userret(td, td->td_frame); | ||||
| #ifdef KTRACE | #ifdef KTRACE | ||||
| if (KTRPOINT(td, KTR_SYSRET)) { | if (KTRPOINT(td, KTR_SYSRET)) { | ||||
| ktrsysret(sa->code, (td->td_pflags & TDP_NERRNO) == 0 ? | ktrsysret(sa->code, (td->td_pflags & TDP_NERRNO) == 0 ? | ||||
| error : td->td_errno, td->td_retval[0]); | error : td->td_errno, td->td_retval[0]); | ||||
| } | } | ||||
| #endif | #endif | ||||
| td->td_pflags &= ~TDP_NERRNO; | td->td_pflags &= ~TDP_NERRNO; | ||||
| if (p->p_flag & P_TRACED) { | if (p->p_flag & P_TRACED) { | ||||
| traced = 1; | traced = 1; | ||||
| PROC_LOCK(p); | PROC_LOCK(p); | ||||
| td->td_dbgflags |= TDB_SCX; | td->td_dbgflags |= TDB_SCX; | ||||
| td->td_dbgsa = sa; | |||||
| PROC_UNLOCK(p); | PROC_UNLOCK(p); | ||||
| } else | } else | ||||
| traced = 0; | traced = 0; | ||||
| /* | /* | ||||
| * This works because errno is findable through the | * This works because errno is findable through the | ||||
| * register set. If we ever support an emulation where this | * register set. If we ever support an emulation where this | ||||
| * is not the case, this code will need to be revisited. | * is not the case, this code will need to be revisited. | ||||
| */ | */ | ||||
| STOPEVENT(p, S_SCX, sa->code); | STOPEVENT(p, S_SCX, sa->code); | ||||
| if (traced || (td->td_dbgflags & (TDB_EXEC | TDB_FORK)) != 0) { | if (traced || (td->td_dbgflags & (TDB_EXEC | TDB_FORK)) != 0) { | ||||
| PROC_LOCK(p); | PROC_LOCK(p); | ||||
| /* | /* | ||||
| * If tracing the execed process, trap to the debugger | * If tracing the execed process, trap to the debugger | ||||
| * so that breakpoints can be set before the program | * so that breakpoints can be set before the program | ||||
| * executes. If debugger requested tracing of syscall | * executes. If debugger requested tracing of syscall | ||||
| * returns, do it now too. | * returns, do it now too. | ||||
| */ | */ | ||||
| if (traced && | if (traced && | ||||
| ((td->td_dbgflags & (TDB_FORK | TDB_EXEC)) != 0 || | ((td->td_dbgflags & (TDB_FORK | TDB_EXEC)) != 0 || | ||||
| (p->p_stops & S_PT_SCX) != 0)) | (p->p_stops & S_PT_SCX) != 0)) | ||||
| ptracestop(td, SIGTRAP); | ptracestop(td, SIGTRAP); | ||||
| td->td_dbgflags &= ~(TDB_SCX | TDB_EXEC | TDB_FORK); | td->td_dbgflags &= ~(TDB_SCX | TDB_EXEC | TDB_FORK); | ||||
| td->td_dbgsa = NULL; | |||||
| PROC_UNLOCK(p); | PROC_UNLOCK(p); | ||||
| } | } | ||||
| if (td->td_pflags & TDP_RFPPWAIT) { | if (td->td_pflags & TDP_RFPPWAIT) { | ||||
| /* | /* | ||||
| * Preserve synchronization semantics of vfork. If | * Preserve synchronization semantics of vfork. If | ||||
| * waiting for child to exec or exit, fork set | * waiting for child to exec or exit, fork set | ||||
| * P_PPWAIT on child, and there we sleep on our proc | * P_PPWAIT on child, and there we sleep on our proc | ||||
| Show All 25 Lines | |||||
You cannot do it this way. struct syscall_args is typically allocated on the thread stack, which means that it could be swapped out. I do not remember any reason why stopped thread cannot be swapped.