Date: Thu, 15 Sep 2016 17:24:23 +0000 (UTC) From: Bruce Evans <bde@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r305840 - in head/sys: amd64/amd64 amd64/include ddb i386/i386 i386/include Message-ID: <201609151724.u8FHONFq081367@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: bde Date: Thu Sep 15 17:24:23 2016 New Revision: 305840 URL: https://svnweb.freebsd.org/changeset/base/305840 Log: Abort single stepping in ddb if the trap is not for single-stepping. This is not very easy to do, since ddb didn't know when traps are for single-stepping. It more or less assumed that traps are either breakpoints or single-step, but even for x86 this became inadequate with the release of the i386 in ~1986, and FreeBSD passes it other trap types for NMIs and panics. On x86, teach ddb when a trap is for single stepping using the %dr6 register. Unknown traps are now treated almost the same as breakpoints instead of as the same as single-steps. Previously, the classification of breakpoints was almost correct and everything else was unknown so had to be treated as a single-step. Now the classification of single- steps is precise, the classification of breakpoints is almost correct (as before) and everything else is unknown and treated like a breakpoint. This fixes: - breakpoints not set by ddb, including the main one in kdb_enter(), were treated as single-steps and not stopped on when stepping (except for the usual, simple case of a step with residual count 1). As special cases, kdb_enter() didn't stop for fatal traps or panics - similarly for "hardware breakpoints". Use a new MD macro IS_SSTEP_TRAP(type, code) to code to classify single-steps. This is excessively complicated for bug-for-bug and backwards compatibilty. Design errors apparently started in Mach in ~1990 or perhaps in the FreeBSD interface in ~1993. Common trap types like single steps should have a unique MI code (like the TRAP* codes for user SIGTRAP) so that debuggers don't need macros like IS_SSTEP_TRAP() to decode them. But 'type' is actually an ambiguous MD trap number, and code was always 0 (now it is (int)%dr6 on x86). So it was impossible to determine the trap type from the args. Global variables had to be used. There is already a classification macro db_pc_is_single_step(), but this just gets in the way. It is only used to recover from bugs in IS_BREAKPOINT_TRAP(). On some arches, IS_BREAKPOINT_TRAP() just duplicates the ambiguity in 'type' and misclassifies single-steps as breakpoints. It defaults to 'false', which is the opposite of what is needed for bug-for-bug compatibility. When this is cleaned up, MI classification bits should be passed in 'code'. This could be done now for positive-logic bits, since 'code' was always 0, but some negative logic is needed for compatibility so a simple MI classificition is not usable yet. After reading %dr6, clear the single-step bit in it so that the type of the next debugger trap can be decoded. This is a little ddb-specific. ddb doesn't understand the need to clear this bit and doing it before calling kdb is easiest. gdb would need to reverse this to support hardware breakpoints, but it just doesn't support them now since gdbstub doesn't support %dr*. Fix a bug involving %dr6: when emulating a single-step trap for vm86, set the bit for it in %dr6. Userland debuggers need this. ddb now needs this for vm86 bios calls. The bit gets copied to 'code' then cleared again. Fix related style bugs: - when clearing bits for hardware breakpoints in %dr6, spell the mask as ~0xf on both amd64 and i386 to get the correct number of bits using sign extension and not need a comment about using the wrong mask on amd64 (amd64 traps for invalid results but clearing the reserved top bits didn't trap since they are 0). - rewrite my old wrong comments about using %dr6 for ddb watchpoints. Modified: head/sys/amd64/amd64/trap.c head/sys/amd64/include/db_machdep.h head/sys/ddb/db_run.c head/sys/i386/i386/trap.c head/sys/i386/include/db_machdep.h Modified: head/sys/amd64/amd64/trap.c ============================================================================== --- head/sys/amd64/amd64/trap.c Thu Sep 15 17:24:11 2016 (r305839) +++ head/sys/amd64/amd64/trap.c Thu Sep 15 17:24:23 2016 (r305840) @@ -176,6 +176,7 @@ trap(struct trapframe *frame) #endif struct thread *td = curthread; struct proc *p = td->td_proc; + register_t dr6; int i = 0, ucode = 0, code; u_int type; register_t addr = 0; @@ -540,8 +541,7 @@ trap(struct trapframe *frame) * Reset breakpoint bits because the * processor doesn't */ - /* XXX check upper bits here */ - load_dr6(rdr6() & 0xfffffff0); + load_dr6(rdr6() & ~0xf); goto out; } /* @@ -553,7 +553,10 @@ trap(struct trapframe *frame) * Otherwise, debugger traps "can't happen". */ #ifdef KDB - if (kdb_trap(type, 0, frame)) + /* XXX %dr6 is not quite reentrant. */ + dr6 = rdr6(); + load_dr6(dr6 & ~0x4000); + if (kdb_trap(type, dr6, frame)) goto out; #endif break; Modified: head/sys/amd64/include/db_machdep.h ============================================================================== --- head/sys/amd64/include/db_machdep.h Thu Sep 15 17:24:11 2016 (r305839) +++ head/sys/amd64/include/db_machdep.h Thu Sep 15 17:24:23 2016 (r305840) @@ -56,12 +56,16 @@ do { \ #define db_clear_single_step kdb_cpu_clear_singlestep #define db_set_single_step kdb_cpu_set_singlestep -#define IS_BREAKPOINT_TRAP(type, code) ((type) == T_BPTFLT) /* - * Watchpoints are not supported. The debug exception type is in %dr6 - * and not yet in the args to this macro. + * The debug exception type is copied from %dr6 to 'code' and used to + * disambiguate single step traps. Watchpoints have no special support. + * Our hardware breakpoints are not well integrated with ddb and are too + * different from watchpoints. ddb treats them as unknown traps with + * unknown addresses and doesn't turn them off while it is running. */ -#define IS_WATCHPOINT_TRAP(type, code) 0 +#define IS_BREAKPOINT_TRAP(type, code) ((type) == T_BPTFLT) +#define IS_SSTEP_TRAP(type, code) ((type) == T_TRCTRAP && (code) & 0x4000) +#define IS_WATCHPOINT_TRAP(type, code) 0 #define I_CALL 0xe8 #define I_CALLI 0xff Modified: head/sys/ddb/db_run.c ============================================================================== --- head/sys/ddb/db_run.c Thu Sep 15 17:24:11 2016 (r305839) +++ head/sys/ddb/db_run.c Thu Sep 15 17:24:23 2016 (r305840) @@ -57,6 +57,7 @@ static int db_run_mode; #define STEP_INVISIBLE 5 #define STEP_COUNT 6 +static bool db_sstep_multiple; static bool db_sstep_print; static int db_loop_count; static int db_call_depth; @@ -133,7 +134,25 @@ db_stop_at_pc(int type, int code, bool * #endif } - *is_breakpoint = false; + *is_breakpoint = false; /* might be a breakpoint, but not ours */ + + /* + * If stepping, then abort if the trap type is unexpected. + * Breakpoints owned by us are expected and were handled above. + * Single-steps are expected and are handled below. All others + * are unexpected. + * + * If the MD layer doesn't tell us when it is stepping, use the + * bad historical default that all unexepected traps. + */ +#ifndef IS_SSTEP_TRAP +#define IS_SSTEP_TRAP(type, code) true +#endif + if (db_run_mode != STEP_CONTINUE && !IS_SSTEP_TRAP(type, code)) { + printf("Stepping aborted\n"); + db_run_mode = STEP_NONE; + return (true); + } if (db_run_mode == STEP_INVISIBLE) { db_run_mode = STEP_CONTINUE; @@ -194,6 +213,7 @@ db_restart_at_pc(bool watchpt) db_addr_t pc = PC_REGS(); if ((db_run_mode == STEP_COUNT) || + ((db_run_mode == STEP_ONCE) && db_sstep_multiple) || (db_run_mode == STEP_RETURN) || (db_run_mode == STEP_CALLT)) { /* @@ -321,6 +341,7 @@ db_single_step_cmd(db_expr_t addr, bool db_run_mode = STEP_ONCE; db_loop_count = count; + db_sstep_multiple = (count != 1); db_sstep_print = print; db_inst_count = 0; db_load_count = 0; Modified: head/sys/i386/i386/trap.c ============================================================================== --- head/sys/i386/i386/trap.c Thu Sep 15 17:24:11 2016 (r305839) +++ head/sys/i386/i386/trap.c Thu Sep 15 17:24:23 2016 (r305840) @@ -189,6 +189,7 @@ trap(struct trapframe *frame) #endif struct thread *td = curthread; struct proc *p = td->td_proc; + register_t dr6; int i = 0, ucode = 0, code; u_int type; register_t addr = 0; @@ -362,6 +363,7 @@ user_trctrap_out: i = vm86_emulate((struct vm86frame *)frame); if (i == SIGTRAP) { type = T_TRCTRAP; + load_dr6(rdr6() | 0x4000); goto user_trctrap_out; } if (i == 0) @@ -572,6 +574,7 @@ user_trctrap_out: i = vm86_emulate((struct vm86frame *)frame); if (i == SIGTRAP) { type = T_TRCTRAP; + load_dr6(rdr6() | 0x4000); goto kernel_trctrap; } if (i != 0) @@ -696,7 +699,7 @@ kernel_trctrap: * Reset breakpoint bits because the * processor doesn't */ - load_dr6(rdr6() & 0xfffffff0); + load_dr6(rdr6() & ~0xf); goto out; } /* @@ -708,7 +711,10 @@ kernel_trctrap: * Otherwise, debugger traps "can't happen". */ #ifdef KDB - if (kdb_trap(type, 0, frame)) + /* XXX %dr6 is not quite reentrant. */ + dr6 = rdr6(); + load_dr6(dr6 & ~0x4000); + if (kdb_trap(type, dr6, frame)) goto out; #endif break; Modified: head/sys/i386/include/db_machdep.h ============================================================================== --- head/sys/i386/include/db_machdep.h Thu Sep 15 17:24:11 2016 (r305839) +++ head/sys/i386/include/db_machdep.h Thu Sep 15 17:24:23 2016 (r305840) @@ -59,12 +59,16 @@ do { \ #define db_clear_single_step kdb_cpu_clear_singlestep #define db_set_single_step kdb_cpu_set_singlestep -#define IS_BREAKPOINT_TRAP(type, code) ((type) == T_BPTFLT) /* - * Watchpoints are not supported. The debug exception type is in %dr6 - * and not yet in the args to this macro. + * The debug exception type is copied from %dr6 to 'code' and used to + * disambiguate single step traps. Watchpoints have no special support. + * Our hardware breakpoints are not well integrated with ddb and are too + * different from watchpoints. ddb treats them as unknown traps with + * unknown addresses and doesn't turn them off while it is running. */ -#define IS_WATCHPOINT_TRAP(type, code) 0 +#define IS_BREAKPOINT_TRAP(type, code) ((type) == T_BPTFLT) +#define IS_SSTEP_TRAP(type, code) ((type) == T_TRCTRAP && (code) & 0x4000) +#define IS_WATCHPOINT_TRAP(type, code) 0 #define I_CALL 0xe8 #define I_CALLI 0xff
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201609151724.u8FHONFq081367>