| Message ID | 1373062265-4267-3-git-send-email-James.Yang@freescale.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
On Fri, 2013-07-05 at 17:11 -0500, James Yang wrote: > A BookE branch taken debug exception followed by a single step does not > accurately simulate Server's branch execute debug exception. BookE's > branch taken debug exception stops before the branch is to be executed > and only happens if the branch will actually be taken. Server's branch > execute trace exception stops on the instruction after the branch > executes, regardless of whether or not the branch redirected the program > counter. > > The existing PTRACE_SINGLEBLOCK support for BookE hardcodes a single > step after the branch taken exception is taken in order to simulate > Server's behavior, but this misses fall-through branch instructions > (i.e., branches that are NOT taken). Also, the si_code became masked as > TRAP_TRACE instead of TRAP_BRANCH. But that changes the user visible behaviour, won't that break gdb expectations ? Another way to "fix" it is to instead use lib/sstep.c to emulate the single step maybe ? On the other hand, I tend to think that trapping before the branch is actually more useful especially if you don't have the CFAR register. Cheers, Ben.
On Sat, 6 Jul 2013, Benjamin Herrenschmidt wrote: > On Fri, 2013-07-05 at 17:11 -0500, James Yang wrote: > > A BookE branch taken debug exception followed by a single step does not > > accurately simulate Server's branch execute debug exception. BookE's > > branch taken debug exception stops before the branch is to be executed > > and only happens if the branch will actually be taken. Server's branch > > execute trace exception stops on the instruction after the branch > > executes, regardless of whether or not the branch redirected the program > > counter. > > > > The existing PTRACE_SINGLEBLOCK support for BookE hardcodes a single > > step after the branch taken exception is taken in order to simulate > > Server's behavior, but this misses fall-through branch instructions > > (i.e., branches that are NOT taken). Also, the si_code became masked as > > TRAP_TRACE instead of TRAP_BRANCH. > > But that changes the user visible behaviour, won't that break gdb > expectations ? I am having a difficult time finding any use of PTRACE_SINGLEBLOCK in any arch in the various versions of gdb source trees I downloaded. Would you please provide a pointer or at least a hint as to where you think it would be? I don't know gdb internals at all, but grepping the sources for PTRACE_SINGLEBLOCK returns nothing. > Another way to "fix" it is to instead use lib/sstep.c to emulate the > single step maybe ? I don't think there's any issue any more with the hard coded single step with the fixes that Scott and Bharat recently provided. Actually, I don't even know if this feature was practically useful on BookE before those fixes due to the hangs and non-deterministic behavior. Hypothetically, sstep.c could let server to emulate BookE's behavior.. > On the other hand, I tend to think that trapping before the branch is > actually more useful especially if you don't have the CFAR register. And there's no exception for fall-through branches. So, yeah, this really is the question: are there actually any tools that rely on PTRACE_SINGLEBLOCK behaving in the different ways it currently does on the two Power subarchiectures? For BookE targes, how do they handle not being able to catch fall-through branches? For server targets, how do they capture the old LR value for blrl after the branch? (I'm guessing not even sstep emulation can provide this information, though it might not be practically useful.)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c index c3ceaa2..5837d7f 100644 --- a/arch/powerpc/kernel/traps.c +++ b/arch/powerpc/kernel/traps.c @@ -1518,12 +1518,21 @@ void __kprobes DebugException(struct pt_regs *regs, unsigned long debug_status) { current->thread.dbsr = debug_status; - /* Hack alert: On BookE, Branch Taken stops on the branch itself, while - * on server, it stops on the target of the branch. In order to simulate - * the server behaviour, we thus restart right away with a single step - * instead of stopping here when hitting a BT + /* BookE's Branch Taken Debug Exception stops on the branch iff the + * branch is resolved to be taken. No exception occurs if the branch + * is not taken (no exception if the branch does not redirect the PC). + * This is unlike Classic/Server's behavior where the exception occurs + * after the branch executes, regardless of whether or not the branch + * redirected the PC. + * + * The past behavior of this function was to simulate Classic/Server's + * behavior by performing a single-step upon a branch taken exception. + * However, the simulation is not accurate because fall-through non- + * taken branches would not result in a SIGTRAP. Also, that SIGTRAP's + * si_code would be reported as a TRAP_TRACE instead of a TRAP_BRANCH. */ - if (debug_status & DBSR_BT) { + + if (debug_status & DBSR_BT) { /* Branch Taken */ regs->msr &= ~MSR_DE; /* Disable BT */ @@ -1531,20 +1540,25 @@ void __kprobes DebugException(struct pt_regs *regs, unsigned long debug_status) /* Clear the BT event */ mtspr(SPRN_DBSR, DBSR_BT); - /* Do the single step trick only when coming from userspace */ - if (user_mode(regs)) { - current->thread.dbcr0 &= ~DBCR0_BT; - current->thread.dbcr0 |= DBCR0_IDM | DBCR0_IC; - regs->msr |= MSR_DE; - return; - } - if (notify_die(DIE_SSTEP, "block_step", regs, 5, 5, SIGTRAP) == NOTIFY_STOP) { return; } + if (debugger_sstep(regs)) return; + + if (user_mode(regs)) { + current->thread.dbcr0 &= ~DBCR0_BT; + if (DBCR_ACTIVE_EVENTS(current->thread.dbcr0, + current->thread.dbcr1)) + regs->msr |= MSR_DE; + else + /* Make sure the IDM bit is off */ + current->thread.dbcr0 &= ~DBCR0_IDM; + } + + _exception(SIGTRAP, regs, TRAP_BRANCH, regs->nip); } else if (debug_status & DBSR_IC) { /* Instruction complete */ regs->msr &= ~MSR_DE;
A BookE branch taken debug exception followed by a single step does not accurately simulate Server's branch execute debug exception. BookE's branch taken debug exception stops before the branch is to be executed and only happens if the branch will actually be taken. Server's branch execute trace exception stops on the instruction after the branch executes, regardless of whether or not the branch redirected the program counter. The existing PTRACE_SINGLEBLOCK support for BookE hardcodes a single step after the branch taken exception is taken in order to simulate Server's behavior, but this misses fall-through branch instructions (i.e., branches that are NOT taken). Also, the si_code became masked as TRAP_TRACE instead of TRAP_BRANCH. This patch provides native support for the BookE branch taken debug exception's behavior: PTRACE_SINGLEBLOCK stops with a SIGTRAP before a branch-that-would-be-taken would execute. Userspace software will be able to examine the process state upon catching the SIGTRAP, and it will need to issue a PTRACE_SINGLESTEP or PTRACE_CONT to resume program execution past the branch. Signed-off-by: James Yang <James.Yang@freescale.com> --- arch/powerpc/kernel/traps.c | 40 +++++++++++++++++++++++++++------------- 1 files changed, 27 insertions(+), 13 deletions(-)