parsic/sh/sparc tracehook breakage when tracing signals

when i ported the Blackfin code to the tracehook framework, i copied a latent bug from the sparc port. trying to trace another process while handling signals no longer worked (and subsequently broke some of the gdb tests).

this was due to calling tracehook_signal_handler() with the last argument (stepping) always as 0. if we look at the definition of this function in linux/tracehook.h, we see that calling the function stepping=0 is pointless:

	if (stepping)
		ptrace_notify(SIGTRAP);

after Roland pointed out some more stuff, i went back and looked at all the tracehook arches in the tree. it seems like these arches are all broken in the same way:

	parisc (arch/parisc/kernel/signal.c)
	SuperH (64bit only) (arch/sh/kernel/signal_64.c)
	Sparc (all bits) (arch/sparc/kernel/signal{_32,32,_64}.c)

seems like you guys should just change the last argument to:
test_thread_flag(TIF_SINGLESTEP)
-mike

From: Mike Frysinger <[email protected]>
Date: Thu, 11 Feb 2010 19:10:47 -0500

What you seem to be missing is that on Sparc TIF_SINGLESTEP will never be set, because it does not support hardware single step.

This thread flag is not even defined on that platform.

my point is that these arches never call ptrace_notify(SIGTRAP) from their signal handlers. maybe the arch is unable to due to some port/hardware limitation and this is currently the expected behavior.

dont get me wrong ... i dont particularly care if your arch has a bug in it here. my arch did have a bug and on the off chance that others did too, i thought i'd drop an e-mail to people. if your arch is in the "limited" category, feel free to ignore this.
-mike

What you mean is that single-step into a signal handler fails to stop at the first instruction of the handler. (Instead it stops after the first instruction in the handler's prologue.)

Indeed, it doesn't do anything else right now. But the reason to call it regardless is so that every arch is consistent in telling the generic code what is going on. If we add a future feature to ptrace (or something else) to track signal handler setups, that feature may very well not be one that fires only when single-step is in use. An arch that is set up now to call tracehook_signal_handler() exactly as that function's kerneldoc says to do will be prepared for such things to work without later arch changes.

Whether there is a TIF_SINGLESTEP and what it means is arch-specific. If arch_has_single_step(), then the argument should be nonzero if user_enable_single_step() is in force at the time of handler setup.

In parisc, it should test for either TIF_SINGLESTEP or TIF_BLOCKSTEP.

In sh, going from what signal_32.c does, it should indeed do as you say.

In sparc, arch_has_single_step()==0, so there is nothing to do.

Thanks,
Roland

Good catch, I've committed a patch to fix this, will send it upstream today.

regards, Kyle