Messages in this thread | | | From | Roland McGrath <> | Subject | Re: s390 && user_enable_single_step() (Was: odd utrace testing results on s390x) | Date | Mon, 4 Jan 2010 13:11:47 -0800 (PST) |
| |
> This probably means that copy_process()->user_disable_single_step() > is not enough to clear the "this task wants single-stepping" copied > from parent.
I would suspect s390's TIF_SINGLE_STEP flag here. That flag means "a single-step trap occurred". This is what causes do_single_step to be called before returning to user mode, rather than the machine trap doing it directly as is done in the other arch implementations.
If I'm right, then "this task wants single-stepping" is not the problem, and that really is fully cleared. In fact, looking at s390's copy_thread (arch/s390/kernel/process.c) it clears out all the state that is actually touched by user_enable_single_step and user_disable_single_step. So for s390 the new fork.c call is actually superfluous AFAICT.
The problem is that the copied parent state includes the "this task has a pending single-step to report" flag. IMHO it clearly makes sense for s390's copy_thread to clear this flag in a new task, which it does not do now.
An alternative to that would be just to make its user_disable_single_step clear the flag. That could in theory also have an effect on e.g. the (authentic) pending step report of a tracee that was stopped with TIF_SINGLE_STEP set when its tracer detached. This might be considered a good thing, but since every other arch posts the SIGTRAP immediately they all have the equivalent issue and s390 doesn't need to be any "better" than they are before we have a generic resolution to the whole subject of tracer-induced signals (which we won't get into now). I'm not even sure from my insufficient reading of the s390 assembly code whether this path is even possible, i.e. do_signal called before do_single_step.
Martin, I suggest having copy_thread clear TIF_SINGLE_STEP. That bit is always task-private state that should not be copied.
Btw, given the complexity of FixPerRegisters (and its new additional cost on task==current), you might want to make user_*_single_step bail out if per_info.single_step is already set/clear on entry.
Thanks, Roland
| |