Re: [PATCH v3 2/5] Support breakpoint kinds for software breakpoints in GDBServer.

[Pedro Alves <palves at redhat dot com>]

On 10/20/2015 05:48 PM, Antoine Tremblay wrote:

> As the kind is now a differentiating factor equivalent to size for the
> breakpoint struct and that it's size can be queryed using

(queried)

> sw_breakpoint_from_kind, the size field has been replaced with the kind field.
> All references to size are now replaced by kind or a call to bp_size that wraps
> sw_breakpoing_from_kind and returns the size of the breakpoint in memory.
> 

> gdb/gdbserver/ChangeLog:
> 	* linux-low.c (initialize_low): Ajdust for breakpoint global variables

"Adjust"

> 	removal.
> 	* mem-break.c : Remove breakpoint_data/breakpoint_len global variables.

 	* mem-break.c (breakpoint_data, breakpoint_len): Delete.

> 	(struct raw_breakpoint) <size>: Remove.
> 	(struct raw_breakpoint) <kind>: Add.
> 	(bp_size): New function.
> 	(bp_opcode): Likewise.
> 	(find_raw_breakpoint_at): Adjust for kind.
> 	(insert_memory_breakpoint): Ajust for kind call bp_size,bp_opcode.
> 	(remove_memory_breakpoint): Ajust for kind call bp_size.

"Adjust".

> 	(set_raw_breakpoint_at): Adjust for kind.
> 	(set_breakpoint): Likewise.
> 	(set_breakpoint_at): Call breakpoint_kind_from_pc.
> 	(delete_raw_breakpoint): Adjust for kind.
> 	(delete_breakpoint): Likewise.
> 	(find_gdb_breakpoint): Likewise.
> 	(set_gdb_breakpoint_1): Likewise.
> 	(set_gdb_breakpoint): Likewise.
> 	(delete_gdb_breakpoint_1): Likewise.
> 	(delete_gdb_breakpoint): Likewise.
> 	(uninsert_raw_breakpoint): Likewise.
> 	(reinsert_raw_breakpoint): Likewise.
> 	(set_breakpoint_data): Remove.
> 	(validate_inserted_breakpoint): Adjust for kind call bp_size,bp_opcode.
> 	(check_mem_read): Adjust for kind call bp_size.
> 	(check_mem_write): Adjust for kind call bp_size,bp_opcode.

 	(check_mem_read): Adjust for kind.  Call bp_size.
 	(check_mem_write): Adjust for kind.  Call bp_size, bp_opcode.

> 	(clone_one_breakpoint): Adjust for kind.
> 	* mem-break.h (set_gdb_breakpoint): Likewise.
> 	(delete_gdb_breakpoint): Likewise.
> 	* server.c (process_serial_event): Likewise.
> ---
>  gdb/gdbserver/linux-low.c |  10 ----
>  gdb/gdbserver/mem-break.c | 150 +++++++++++++++++++++++-----------------------
>  gdb/gdbserver/mem-break.h |   8 +--
>  gdb/gdbserver/server.c    |   8 +--
>  4 files changed, 83 insertions(+), 93 deletions(-)
> 
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index 398c0aa..c20e257 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -7082,20 +7082,10 @@ void
>  initialize_low (void)
>  {
>    struct sigaction sigchld_action;
> -  int breakpoint_kind = 0;
> -  int breakpoint_size = 0;
> -  const gdb_byte *breakpoint = NULL;
>  
>    memset (&sigchld_action, 0, sizeof (sigchld_action));
>    set_target_ops (&linux_target_ops);
>  
> -  breakpoint_kind = the_target->breakpoint_kind_from_pc (NULL);
> -  breakpoint = the_target->sw_breakpoint_from_kind (breakpoint_kind,
> -						    &breakpoint_size);
> -
> -  set_breakpoint_data (breakpoint,
> -		       breakpoint_size);
> -
>    linux_init_signals ();
>    linux_ptrace_init_warnings ();
>  
> diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
> index f077497..85ef7b0 100644
> --- a/gdb/gdbserver/mem-break.c
> +++ b/gdb/gdbserver/mem-break.c
> @@ -21,8 +21,6 @@
>  #include "server.h"
>  #include "regcache.h"
>  #include "ax.h"
> -const unsigned char *breakpoint_data;
> -int breakpoint_len;
>  
>  #define MAX_BREAKPOINT_LEN 8
>  
> @@ -100,8 +98,8 @@ struct raw_breakpoint
>       breakpoint for a given PC.  */
>    CORE_ADDR pc;
>  
> -  /* The breakpoint's size.  */
> -  int size;
> +  /* The breakpoint's kind.  */
> +  int kind;

This comment should be expanded.  Something like:

  /* The software breakpoint's kind.  This is target specific.
     Most architectures only use one specific instruction for software
     breakpoints, while others may use more than one.  E.g., on ARM, we
     need to set different breakpoint instructions on Thumb, Thumb-2,
     and ARM code.  */
  int kind;

> -/* Locate a GDB breakpoint of type Z_TYPE and size SIZE placed at
> -   address ADDR and return a pointer to its structure.  If SIZE is -1,
> -   the breakpoints' sizes are ignored.  */
> +/* Locate a GDB breakpoint of type Z_TYPE and kind KIND placed at
> +   address ADDR and return a pointer to its structure.  If KIND is -1,
> +   the breakpoint's kind are ignored.  */

Either:

 "breakpoint's kind is ignored".

or:

 "breakpoints' kinds are ignored".

Thanks,
Pedro Alves