Re: [PATCH] TARGET_ADJUST_BREAKPOINT_ADDRESS - patch 4 of 4

[Michael Snyder <msnyder@redhat.com>]

Kevin Buettner wrote:
> Below is patch 4 of 4 of my current TARGET_ADJUST_BREAKPOINT_ADDRESS
> patch set.  To better understand what this is all about, see patch 3
> at:
> 
>     http://sources.redhat.com/ml/gdb-patches/2003-10/msg00073.html
> 
> I'll commit this patch sometime after patch 1 is committed.  (In order
> to actually be used, it depends on patch 3, but no harm will occur if
> it goes in before patch 3.)
> 
> 	* frv-tdep.c (frv_target_adjust_breakpoint_address): New function.
> 	(frv_gdbarch_init): Initialize TARGET_ADJUST_BREAKPOINT_ADDRESS
> 	method.

Hi Kevin,

My first feedback is that "count" should be set to a const
rather than to 8 -- since we've seen it change from 4 to 8
just recently, and who knows when it will change again.
That'll also provide an opportunity to explain (via the
name of the const, if nothing else) what 'count' is.

Maybe "bpaddr - 4" could also be "bpaddr - sizeof (something)".

Michael

> 
> Index: frv-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/frv-tdep.c,v
> retrieving revision 1.53
> diff -u -p -r1.53 frv-tdep.c
> --- frv-tdep.c	19 Sep 2003 16:22:38 -0000	1.53
> +++ frv-tdep.c	3 Oct 2003 23:38:25 -0000
> @@ -37,6 +37,7 @@ static gdbarch_init_ftype frv_gdbarch_in
>  
>  static gdbarch_register_name_ftype frv_register_name;
>  static gdbarch_breakpoint_from_pc_ftype frv_breakpoint_from_pc;
> +static gdbarch_target_adjust_breakpoint_address_ftype frv_target_adjust_breakpoint_address;
>  static gdbarch_skip_prologue_ftype frv_skip_prologue;
>  static gdbarch_deprecated_extract_return_value_ftype frv_extract_return_value;
>  static gdbarch_deprecated_extract_struct_value_address_ftype frv_extract_struct_value_address;
> @@ -272,6 +273,42 @@ frv_breakpoint_from_pc (CORE_ADDR *pcptr
>  }
>  
>  
> +static CORE_ADDR
> +frv_target_adjust_breakpoint_address (CORE_ADDR bpaddr)
> +{
> +  int count = 8;
> +  CORE_ADDR addr = bpaddr - 4;
> +  CORE_ADDR func_start = get_pc_function_start (bpaddr);
> +
> +  /* Find the end of the previous packing sequence.  This will be indicated
> +     by either attempting to access some inaccessible memory or by finding
> +     an instruction word whose packing bit is set to one. */
> +  while (count-- > 0 && addr >= func_start)
> +    {
> +      int status;
> +      char instr[4];
> +
> +      status = read_memory_nobpt (addr, instr, sizeof instr);
> +
> +      if (status != 0)
> +	break;
> +
> +      /* This is a big endian architecture, so byte zero will have most
> +         significant byte.  The most significant bit of this byte is the
> +         packing bit.  */
> +      if (instr[0] & 0x80)
> +	break;
> +
> +      addr -= 4;
> +    }
> +
> +  if (count > 0)
> +    bpaddr = addr + 4;
> +
> +  return bpaddr;
> +}
> +
> +
>  /* Return true if REG is a caller-saves ("scratch") register,
>     false otherwise.  */
>  static int
> @@ -1114,6 +1151,7 @@ frv_gdbarch_init (struct gdbarch_info in
>  
>    set_gdbarch_skip_prologue (gdbarch, frv_skip_prologue);
>    set_gdbarch_breakpoint_from_pc (gdbarch, frv_breakpoint_from_pc);
> +  set_gdbarch_target_adjust_breakpoint_address (gdbarch, frv_target_adjust_breakpoint_address);
>  
>    set_gdbarch_frame_args_skip (gdbarch, 0);
>    set_gdbarch_frameless_function_invocation (gdbarch, frv_frameless_function_invocation);
>