> In short: I no longer understand why utrace->resume can be UTRACE_*STEP > when the tracee is stopped, and in fact I think this is not right.
I think we didn't have that originally. The rationale that I remember is the idea that you should be able to implement a debugger interface feature like PTRACE_SINGLESTEP without noticeably more overhead than it had in pre-utrace ptrace. That is, in vanilla ptrace, the tracer thread calls user_enable_single_step itself and then just lets the tracee resume directly to user mode. We wanted to avoid always having an additional trip through tracehook_notify_resume and the utrace reporting loop and then back through the return-to-user assembly path a second time, between the tracee waking up and actually going to user mode. > For example. Consider two engines, E1 and E2. To simplify the discussion > suppose that both engines have engine->data->resume and ->report_quiesce() > just returns ->resume. So, if the debugger wants, say, single-step, it > does > > engine->data->resume = UTRACE_SINGLESTEP; > utrace_control(UTRACE_SINGLESTEP); > > Actually, any engine should do something like this. Right. > Now suppose that E1 wants UTRACE_SINGLESTEP, E2 asks for UTRACE_STOP. > In this case utrace_resume() results in utrace_stop() which sets > utrace->resume = UTRACE_SINGLESTEP before sleeping. > > First of all - why? Yes, the theory is that we have another reporting > loop after wake_up, but utrace->resume = UTRACE_REPORT is enough, E1 > should always acquire UTRACE_SINGLESTEP if needed or it is buggy. If E2 uses UTRACE_RESUME rather than UTRACE_REPORT, nobody actually cared about another callback before user mode. So indeed the theory is that this lets E1 get its single-step without the overhead of a reporting loop that serves no other purpose than that. But you are right that any engine is buggy if it doesn't dtrt in such a reporting loop, since some other engine might have used UTRACE_REPORT. > But more importantly, this doesn't look right or I missed something. > Suppose that, before E2 resumes the tracee, E1 does utrace_control(STOP) > which immediately succeeds and allows E1 to play with the stopped tracee. > Again, to simplify the discussion, suppose that E2 does UTRACE_RESUME > and nothing more after that. > > Now. E1 wants to resume the tracee too and it does > > engine->data->resume = UTRACE_RESUME; > utrace_control(UTRACE_RESUME); > > utrace_control(RESUME) correctly clears TIF_SINGLESTEP, but it doesn't > change utrace->resume. This means utrace_resume() doesn't do the > reporting loop, it just calls user_enable_single_step() and returns > to user-mode. Yes, that sounds wrong. > So. Could you explain why utrace_stop() should ever keep UTRACE_STEP > in utrace->resume? finish_report() does the same but this looks fine. I cannot. > Also. I am not sure utrace_control(UTRACE_STEP) should always set > utrace->resume = UTRACE_STEP. If the tracee is stopped but there > is another ENGINE_STOP engine (iow, the tracee won't be resumed > right now) we have the same problem. Right. Said another way, any time an engine that previously used UTRACE_*STEP now uses UTRACE_RESUME, we must degrade utrace->resume back to UTRACE_REPORT at most. Since we don't keep track of which engine it was that put UTRACE_*STEP into utrace->resume before, we'd have to just always degrade it any time anybody uses UTRACE_RESUME. That has almost the whole effect of punting utrace->resume back to just a utrace->report flag. But that's how it used to be, and then we changed it. So it merits digging up our old discussion threads that led to doing that and see what all we had in mind then. > Off-topic, > > #define UTRACE_RESUME_BITS (ilog2(UTRACE_RESUME_MAX) + 1) > > why UTRACE_RESUME_MAX? it should be (ilog2(UTRACE_RESUME_MAX - 1) + 1), no? Yes, I think you're right. Thanks, Roland
