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.
This commit is contained in:
Bruce Evans 2016-09-15 17:24:23 +00:00
parent 38605d7312
commit bd20334ca0
5 changed files with 52 additions and 14 deletions

View File

@ -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;

View File

@ -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

View File

@ -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 *is_breakpoint, bool *is_watchpoint)
#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 have_addr, db_expr_t count, char *modif)
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;

View File

@ -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;

View File

@ -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