Skip to content

Commit

Permalink
kgdbts: (1 of 2) fix single step awareness to work correctly with SMP
Browse files Browse the repository at this point in the history
The do_fork and sys_open tests have never worked properly on anything
other than a UP configuration with the kgdb test suite.  This is
because the test suite did not fully implement the behavior of a real
debugger.  A real debugger tracks the state of what thread it asked to
single step and can correctly continue other threads of execution or
conditionally stop while waiting for the original thread single step
request to return.

Below is a simple method to cause a fatal kernel oops with the kgdb
test suite on a 4 processor x86 system:

while [ 1 ] ; do ls > /dev/null 2> /dev/null; done&
while [ 1 ] ; do ls > /dev/null 2> /dev/null; done&
while [ 1 ] ; do ls > /dev/null 2> /dev/null; done&
while [ 1 ] ; do ls > /dev/null 2> /dev/null; done&
echo V1I1F1000 > /sys/module/kgdbts/parameters/kgdbts

Very soon after starting the test the kernel will oops with a message like:

kgdbts: BP mismatch 3b7da66480 expected ffffffff8106a590
WARNING: at drivers/misc/kgdbts.c:303 check_and_rewind_pc+0xe0/0x100()
Call Trace:
 [<ffffffff812994a0>] check_and_rewind_pc+0xe0/0x100
 [<ffffffff81298945>] validate_simple_test+0x25/0xc0
 [<ffffffff81298f77>] run_simple_test+0x107/0x2c0
 [<ffffffff81298a18>] kgdbts_put_char+0x18/0x20

The warn will turn to a hard kernel crash shortly after that because
the pc will not get properly rewound to the right value after hitting
a breakpoint leading to a hard lockup.

This change is broken up into 2 pieces because archs that have hw
single stepping (2.6.26 and up) need different changes than archs that
do not have hw single stepping (3.0 and up).  This change implements
the correct behavior for an arch that supports hw single stepping.

A minor defect was fixed where sys_open should be do_sys_open
for the sys_open break point test.  This solves the problem of running
a 64 bit with a 32 bit user space.  The sys_open() never gets called
when using the 32 bit file system for the kgdb testsuite because the
32 bit binaries invoke the compat_sys_open() call leading to the test
never completing.

In order to mimic a real debugger, the kgdb test suite now tracks the
most recent thread that was continued (cont_thread_id), with the
intent to single step just this thread.  When the response to the
single step request stops in a different thread that hit the original
break point that thread will now get continued, while the debugger
waits for the thread with the single step pending.  Here is a high
level description of the sequence of events.

   cont_instead_of_sstep = 0;

1) set breakpoint at do_fork
2) continue
3)   Save the thread id where we stop to cont_thread_id
4) Remove breakpoint at do_fork
5) Reset the PC if needed depending on kernel exception type
6) if (cont_instead_of_sstep) { continue } else { single step }
7)   Check where we stopped
       if current thread != cont_thread_id {
           cont_instead_of_sstep = 1;
           goto step 5
       } else {
           cont_instead_of_sstep = 0;
       }
8) clean up and run test again if needed

Cc: [email protected] # >= 2.6.26
Signed-off-by: Jason Wessel <[email protected]>
  • Loading branch information
jwessel committed Mar 29, 2012
1 parent 456ca7f commit 486c598
Showing 1 changed file with 43 additions and 11 deletions.
54 changes: 43 additions & 11 deletions drivers/misc/kgdbts.c
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ static int force_hwbrks;
static int hwbreaks_ok;
static int hw_break_val;
static int hw_break_val2;
static int cont_instead_of_sstep;
static unsigned long cont_thread_id;
static unsigned long sstep_thread_id;
#if defined(CONFIG_ARM) || defined(CONFIG_MIPS) || defined(CONFIG_SPARC)
static int arch_needs_sstep_emulation = 1;
#else
Expand Down Expand Up @@ -211,7 +214,7 @@ static unsigned long lookup_addr(char *arg)
if (!strcmp(arg, "kgdbts_break_test"))
addr = (unsigned long)kgdbts_break_test;
else if (!strcmp(arg, "sys_open"))
addr = (unsigned long)sys_open;
addr = (unsigned long)do_sys_open;
else if (!strcmp(arg, "do_fork"))
addr = (unsigned long)do_fork;
else if (!strcmp(arg, "hw_break_val"))
Expand Down Expand Up @@ -283,6 +286,16 @@ static void hw_break_val_write(void)
hw_break_val++;
}

static int get_thread_id_continue(char *put_str, char *arg)
{
char *ptr = &put_str[11];

if (put_str[1] != 'T' || put_str[2] != '0')
return 1;
kgdb_hex2long(&ptr, &cont_thread_id);
return 0;
}

static int check_and_rewind_pc(char *put_str, char *arg)
{
unsigned long addr = lookup_addr(arg);
Expand Down Expand Up @@ -324,6 +337,18 @@ static int check_single_step(char *put_str, char *arg)
gdb_regs_to_pt_regs(kgdbts_gdb_regs, &kgdbts_regs);
v2printk("Singlestep stopped at IP: %lx\n",
instruction_pointer(&kgdbts_regs));

if (sstep_thread_id != cont_thread_id && !arch_needs_sstep_emulation) {
/*
* Ensure we stopped in the same thread id as before, else the
* debugger should continue until the original thread that was
* single stepped is scheduled again, emulating gdb's behavior.
*/
v2printk("ThrID does not match: %lx\n", cont_thread_id);
cont_instead_of_sstep = 1;
ts.idx -= 4;
return 0;
}
if (instruction_pointer(&kgdbts_regs) == addr) {
eprintk("kgdbts: SingleStep failed at %lx\n",
instruction_pointer(&kgdbts_regs));
Expand Down Expand Up @@ -368,7 +393,12 @@ static int got_break(char *put_str, char *arg)
static void emul_sstep_get(char *arg)
{
if (!arch_needs_sstep_emulation) {
fill_get_buf(arg);
if (cont_instead_of_sstep) {
cont_instead_of_sstep = 0;
fill_get_buf("c");
} else {
fill_get_buf(arg);
}
return;
}
switch (sstep_state) {
Expand Down Expand Up @@ -398,9 +428,11 @@ static void emul_sstep_get(char *arg)
static int emul_sstep_put(char *put_str, char *arg)
{
if (!arch_needs_sstep_emulation) {
if (!strncmp(put_str+1, arg, 2))
return 0;
return 1;
char *ptr = &put_str[11];
if (put_str[1] != 'T' || put_str[2] != '0')
return 1;
kgdb_hex2long(&ptr, &sstep_thread_id);
return 0;
}
switch (sstep_state) {
case 1:
Expand Down Expand Up @@ -502,10 +534,10 @@ static struct test_struct bad_read_test[] = {
static struct test_struct singlestep_break_test[] = {
{ "?", "S0*" }, /* Clear break points */
{ "kgdbts_break_test", "OK", sw_break, }, /* set sw breakpoint */
{ "c", "T0*", }, /* Continue */
{ "c", "T0*", NULL, get_thread_id_continue }, /* Continue */
{ "kgdbts_break_test", "OK", sw_rem_break }, /*remove breakpoint */
{ "g", "kgdbts_break_test", NULL, check_and_rewind_pc },
{ "write", "OK", write_regs }, /* Write registers */
{ "kgdbts_break_test", "OK", sw_rem_break }, /*remove breakpoint */
{ "s", "T0*", emul_sstep_get, emul_sstep_put }, /* Single step */
{ "g", "kgdbts_break_test", NULL, check_single_step },
{ "kgdbts_break_test", "OK", sw_break, }, /* set sw breakpoint */
Expand All @@ -523,10 +555,10 @@ static struct test_struct singlestep_break_test[] = {
static struct test_struct do_fork_test[] = {
{ "?", "S0*" }, /* Clear break points */
{ "do_fork", "OK", sw_break, }, /* set sw breakpoint */
{ "c", "T0*", }, /* Continue */
{ "c", "T0*", NULL, get_thread_id_continue }, /* Continue */
{ "do_fork", "OK", sw_rem_break }, /*remove breakpoint */
{ "g", "do_fork", NULL, check_and_rewind_pc }, /* check location */
{ "write", "OK", write_regs }, /* Write registers */
{ "do_fork", "OK", sw_rem_break }, /*remove breakpoint */
{ "s", "T0*", emul_sstep_get, emul_sstep_put }, /* Single step */
{ "g", "do_fork", NULL, check_single_step },
{ "do_fork", "OK", sw_break, }, /* set sw breakpoint */
Expand All @@ -541,10 +573,10 @@ static struct test_struct do_fork_test[] = {
static struct test_struct sys_open_test[] = {
{ "?", "S0*" }, /* Clear break points */
{ "sys_open", "OK", sw_break, }, /* set sw breakpoint */
{ "c", "T0*", }, /* Continue */
{ "c", "T0*", NULL, get_thread_id_continue }, /* Continue */
{ "sys_open", "OK", sw_rem_break }, /*remove breakpoint */
{ "g", "sys_open", NULL, check_and_rewind_pc }, /* check location */
{ "write", "OK", write_regs }, /* Write registers */
{ "sys_open", "OK", sw_rem_break }, /*remove breakpoint */
{ "s", "T0*", emul_sstep_get, emul_sstep_put }, /* Single step */
{ "g", "sys_open", NULL, check_single_step },
{ "sys_open", "OK", sw_break, }, /* set sw breakpoint */
Expand Down

0 comments on commit 486c598

Please sign in to comment.