Re: [PATCH] align break instruction address in kgdb_set_sw_break

Bob,

Phew! Got that. This patch is correct, though I would prefer doing the same 
thing though a weak definition of kgdb_arch_validate_breakpoint as shown in 
attached patch. It also fixes remove_all_breakalong with kgdb_set_sw_break.

It includes some more minor changes. I haven't compiled it. Will compile it 
before checking in if no-one finds any problems with it.

-Amit

On Friday 13 Jan 2006 10:06 pm, Bob Picco wrote:
> Amit Kale wrote:      [Fri Jan 13 2006, 05:11:33AM EST]
>
> > Bob,
>
> Amit,
>
> Sorry for delay but fighting fires this morning.
>
> > Does gdb actually place breakpoints in middle of an instruction? If gdb
> > does ever ask for a non-aligned breakpoint, when the breakpoint occurs,
> > it'll be
>
> Well a bundle is three instructions. Slot 0 (first instruction) is 16 byte
> aligned but slot 1 and slot 2 are not byte aligned.  That is why gdb
> uses buddle-address+slot to identify where the breakpoint is set. To add
> to the complexity each slot could actually use a different kind of
> break instruction because the break instruction must match the exeuction
> unit of the instruction's slot.
>
> > on a different address, not the one which gdb asked. gdb won't know that
> > the breakpoint has occured. Won't it treat it as a spurious trap?
> >
> > Am I missing something?
>
> Hopefully the text above has helped.  If not, feel free to ask a
> different way.
>
> > I believe kgdb stub should refuse a non-aligned breakpoint.
> > -Amit
>
> bob
>
> > On Friday 13 Jan 2006 5:45 am, Bob Picco wrote:
> > > The check in kgdb_set_sw_break for reading the breakpoint location
> > > isn't exactly correct for ia64.  My brain hurts each time I must
> > > remember the ia64 instructionset layout but some description is
> > > required here. For a breakpoint address gdb sends kgdb the instruction
> > > bundle address+slot as the target address. An ia64 instruction is in a
> > > 128 bit bundle. Each bundle has three instruction slots and is 16 bytes
> > > aligned.  There are 41 bits per instruction and a 5
> > > bit template to identify what execution unit is used for each
> > > instruction slot; (41*3+5 = 128). Unless the breakpoint is in slot
> > > zero, which would be just luck, then the address sent to kgdb won't
> > > start on a bundle 16 byte aligned address. We need to align the address
> > > correctly before checking whether the entire ia64 bundle can be read.
> > >
> > > To summarize bundle slot 1 and slot 2 result in one byte and two bytes
> > > respectively being examined beyond the target bundle which is
> > > incorrect. In 2.6.14 and before this worked because the ia64 arch
> > > specific code, kgdb_arch_set_breakpoint, uses the correct magic.
> > >
> > > My other arch machines are busy.  I need to reinstall my i386 machine
> > > and then will test. Should someone test this patch then please email
> > > me.
> > >
> > > bob
> > >
> > > Signed-off-by: Bob Picco <bob.picco@xxxxxx>
> > >
> > >  include/asm-ia64/kgdb.h |    1 +
> > >  include/linux/kgdb.h    |    4 ++++
> > >  kernel/kgdb.c           |    4 ++--
> > >  3 files changed, 7 insertions(+), 2 deletions(-)
> > >
> > > Index: linux-2.6.15-kgdb/include/asm-ia64/kgdb.h
> > > ===================================================================
> > > --- linux-2.6.15-kgdb.orig/include/asm-ia64/kgdb.h        2006-01-11
> > > 13:45:23.000000000 -0500 +++
> > > linux-2.6.15-kgdb/include/asm-ia64/kgdb.h 2006-01-11 17:33:43.000000000
> > > -0500 @@ -26,6 +26,7 @@
> > >  #define KGDBBREAKNUM             0x6665UL
> > >  #define BREAKPOINT()             asm volatile ("break.m 0x6665")
> > >  #define BREAK_INSTR_SIZE 16
> > > +#define BREAK_INSTR_ALIGN        (~0xfUL)
> > >  #define CACHE_FLUSH_IS_SAFE      1
> > >
> > >  struct pt_regs;
> > > Index: linux-2.6.15-kgdb/include/linux/kgdb.h
> > > ===================================================================
> > > --- linux-2.6.15-kgdb.orig/include/linux/kgdb.h   2006-01-11
> > > 13:45:23.000000000 -0500 +++
> > > linux-2.6.15-kgdb/include/linux/kgdb.h    2006-01-11 17:33:43.000000000
> > > -0500 @@ -46,6 +46,10 @@ extern atomic_t kgdb_sync_softlockup[NR_
> > >
> > >  extern struct task_struct *kgdb_usethread, *kgdb_contthread;
> > >
> > > +#ifndef BREAK_INSTR_ALIGN
> > > +#define BREAK_INSTR_ALIGN        (~0UL)
> > > +#endif
> > > +
> > >  enum kgdb_bptype {
> > >   bp_breakpoint = '0',
> > >   bp_hardware_breakpoint,
> > > Index: linux-2.6.15-kgdb/kernel/kgdb.c
> > > ===================================================================
> > > --- linux-2.6.15-kgdb.orig/kernel/kgdb.c  2006-01-11 13:45:23.000000000
> > > -0500 +++ linux-2.6.15-kgdb/kernel/kgdb.c 2006-01-11 17:34:35.000000000
> > > -0500 @@ -829,8 +829,8 @@ static int kgdb_set_sw_break(unsigned lo
> > >   int i, breakno = -1;
> > >   int error = 0;
> > >   char tmp_variable[BREAK_INSTR_SIZE];
> > > - if ((error = kgdb_get_mem((char *)addr, tmp_variable,
> > > -                         BREAK_INSTR_SIZE)) < 0)
> > > + if ((error = kgdb_get_mem((char *)(addr & BREAK_INSTR_ALIGN),
> > > +         tmp_variable, BREAK_INSTR_SIZE)) < 0)
> > >           return error;
> > >   for (i = 0; i < MAX_BREAKPOINTS; i++) {
> > >           if ((kgdb_break[i].state == bp_set) &&
> > >
> > >
> > >
> > > -------------------------------------------------------
> > > This SF.net email is sponsored by: Splunk Inc. Do you grep through log
> > > files for problems?  Stop!  Download the new AJAX search engine that
> > > makes searching your log files as easy as surfing the  web.  DOWNLOAD
> > > SPLUNK! http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
> > > _______________________________________________
> > > Kgdb-bugreport mailing list
> > > Kgdb-bugreport@xxxxxxxxxxxxxxxxxxxxx
> > > https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport

break-align.patch
Description: Text Data