Re: ugdb breakpoints

[Roland McGrath]

The traditional method is to restore the original instruction replaced by
the breakpoint in text, single-step over that instruction, then restore the
breakpoint in text, then continue.  That method requires all-stop so that
while you are stepping the thread that just hit the breakpoint, you can't
have another thread run past that instruction and miss the breakpoint.

Both this traditional in-place method, and the instruction-copying method,
depend on using single-step.  So stepi has to work before break can work.


Thanks,
Roland

Re: exit_ptrace() ptrace_report_signal() problems

[Oleg Nesterov]

On 09/14, Oleg Nesterov wrote:

 On 09/13, Roland McGrath wrote:
 
  Locks just between the tracer and ptrace_report_signal are not bad.

 OK, but let me think a bit more. I'd really like to avoid adding the
 new lock to avoid the very unlikely race with the exiting tracer.

Oh, please take a look at this (untested) patch.

But I think I should make another attempt, probably spinlock (-siglock ?)
would be cleaner.

Oleg.


--- kstub/kernel/ptrace-utrace.c~9_EXIT_PTRACE_CAN_LOSE_SIG 2010-08-24 
14:31:17.0 +0200
+++ kstub/kernel/ptrace-utrace.c2010-09-14 23:37:59.0 +0200
@@ -67,6 +67,7 @@ struct ptrace_context {
 #define PT_UTRACED 0x1000
 
 #define PTRACE_O_SYSEMU0x100
+#define PTRACE_O_REUSABLE  0x200
 
 #define PTRACE_EVENT_SYSCALL   (1  16)
 #define PTRACE_EVENT_SIGTRAP   (2  16)
@@ -115,7 +116,7 @@ ptrace_reuse_engine(struct task_struct *
return engine;
 
ctx = ptrace_context(engine);
-   if (unlikely(ctx-resume == UTRACE_DETACH)) {
+   if (unlikely(ctx-options == PTRACE_O_REUSABLE)) {
/*
 * Try to reuse this self-detaching engine.
 * The only caller which can hit this case is ptrace_attach(),
@@ -246,24 +247,48 @@ static void ptrace_detach_task(struct ta
 */
bool voluntary = (sig = 0);
struct utrace_engine *engine = ptrace_lookup_engine(tracee);
+   struct ptrace_context *ctx = ptrace_context(engine);
enum utrace_resume_action action = UTRACE_DETACH;
 
if (unlikely(IS_ERR(engine)))
return;
 
-   if (sig) {
-   struct ptrace_context *ctx = ptrace_context(engine);
+   if (!voluntary) {
+   int err;
+
+   ctx-resume = UTRACE_DETACH;
+   /* synchronize with ptrace_report_signal() */
+   do {
+   err = utrace_barrier(tracee, engine);
+   } while (err == -ERESTARTSYS);
+
+   if (!err) {
+   /*
+* The tracee has already reported a signal. If we
+* see -siginfo != NULL it is safe to mark this ctx
+* as reusable and resume the tracee. We can race
+* with ptrace_report_signal(UTRACE_SIGNAL_REPORT)
+* already in progress but this doesn't matter, it
+* must see -resume = UTRACE_DETACH.
+*/
+   if (ctx-siginfo) {
+   smp_wmb();
+   ctx-options = PTRACE_O_REUSABLE;
+   action = UTRACE_RESUME;
+   }
+   }
 
+   } else if (sig) {
switch (get_stop_event(ctx)) {
case PTRACE_EVENT_SYSCALL:
-   if (voluntary)
-   send_sig_info(sig, SEND_SIG_PRIV, tracee);
+   send_sig_info(sig, SEND_SIG_PRIV, tracee);
break;
 
case PTRACE_EVENT_SIGNAL:
-   if (voluntary)
-   ctx-signr = sig;
+   ctx-signr = sig;
ctx-resume = UTRACE_DETACH;
+   smp_wmb();
+   ctx-options = PTRACE_O_REUSABLE;
action = UTRACE_RESUME;
break;
}
@@ -410,6 +435,9 @@ static u32 ptrace_report_syscall_exit(u3
return UTRACE_STOP;
 
if (ctx-resume != UTRACE_RESUME) {
+   if (ctx-resume == UTRACE_DETACH)
+   return UTRACE_RESUME;
+
WARN_ON(ctx-resume != UTRACE_BLOCKSTEP 
ctx-resume != UTRACE_SINGLESTEP);
ctx-resume = UTRACE_RESUME;
@@ -502,6 +530,9 @@ static u32 ptrace_report_signal(u32 acti
ctx-siginfo = NULL;
 
if (resume != UTRACE_RESUME) {
+   if (resume == UTRACE_DETACH)
+   return action;
+
WARN_ON(resume != UTRACE_BLOCKSTEP 
resume != UTRACE_SINGLESTEP);
 
@@ -531,6 +562,11 @@ static u32 ptrace_report_signal(u32 acti
}
 
WARN_ON(ctx-siginfo);
+
+   /* Raced with the exiting tracer ? */
+   if (resume == UTRACE_DETACH)
+   return action;
+
ctx-siginfo = info;
/*
 * ctx-siginfo points to the caller's stack.
@@ -759,7 +795,7 @@ void exit_ptrace(struct task_struct *tra
 static int ptrace_set_options(struct task_struct *tracee,
struct utrace_engine *engine, long data)
 {
-   BUILD_BUG_ON(PTRACE_O_MASK  PTRACE_O_SYSEMU);
+   BUILD_BUG_ON(PTRACE_O_MASK  (PTRACE_O_SYSEMU | PTRACE_O_REUSABLE));