[PING][PATCH PR gdb/21870] aarch64: Leftover uncleared debug registers

[Wei-min Pan <weimin.pan@oracle.com>]

On 10/6/2017 3:18 PM, Weimin Pan wrote:
>      The root cause is that ptrace() does not validate either
>      address or size when setting a hardware watchpoint/breakpoint.
>      As a result, watchpoints were set at address 0, the initial value of
>      aarch64_debug_reg_state in aarch64_process_info, when the
>      PTRACE_SETREGSET request was first made in
>      aarch64_linux_set_debug_regs(), in preparation for resuming the thread.
>
>      Other than changing the kernel ptrace() implementation, first attempt to
>      fix this problem in gdb was to focus on aarch64_linux_new_thread().
>      Instead of marking all hardware breakpoint/watchpoint register pairs for
>      the new thread that have changed, tried to reflect the state by using
>      either DR_MARK_ALL_CHANGED() if they have really been changed or
>      DR_CLEAR_CHANGED() otherwise. But finding whether or not the registers
>      have been changed by using parent's lwp_info or aarch64_process_info
>      proved to be hard or incorrect, especially the latter which caused
>      gdbserver to crash in the middle of the ptid_of_lwp() call.
>
>      Another approach was then taken - add function initial_control_length()
>      to validate the contents in the control registers, basing on the fact that
>      the kernel only supports Byte Address Select (BAS) values of 0x1, 0x3, 0xf
>      and 0xff, before calling ptrace() in aarch64_linux_set_debug_regs().
>
>      Tested on aarch64-linux-gnu. No regressions.
> ---
>   gdb/ChangeLog                    |  7 +++++++
>   gdb/nat/aarch64-linux-hw-point.c | 18 +++++++++++++++++-
>   2 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index c4f55a8137..543e1a0487 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,10 @@
> +2017-10-06  Weimin Pan  <weimin.pan@oracle.com>
> +
> +	PR gdb/21870
> +	* nat/aarch64-linux-hw-point.c (aarch64_linux_set_debug_regs):
> +	Call new function to validate the length in control registers.
> +	(initial_control_length): New function.
> +
>   2017-09-15  Pedro Alves  <palves@redhat.com>
>   
>   	* compile/compile-c-types.c (convert_enum, convert_int)
> diff --git a/gdb/nat/aarch64-linux-hw-point.c b/gdb/nat/aarch64-linux-hw-point.c
> index 9800d9a59c..22c0a48c14 100644
> --- a/gdb/nat/aarch64-linux-hw-point.c
> +++ b/gdb/nat/aarch64-linux-hw-point.c
> @@ -548,6 +548,22 @@ aarch64_handle_watchpoint (enum target_hw_bp_type type, CORE_ADDR addr,
>   						state);
>   }
>   
> +/* Validate the lengths of breakpoints/watchpoints, according to the
> +   contents of these hardware debug control registers, and return
> +   true if all these registers contain zero length.  */
> +
> +static bool
> +initial_control_length (const unsigned int *ctrl, int count)
> +{
> +  for (int i = 0; i < count; i++)
> +    {
> +      if (DR_CONTROL_LENGTH (ctrl[i]))
> +        return false;
> +    }
> +
> +  return true;
> +}
> +
>   /* Call ptrace to set the thread TID's hardware breakpoint/watchpoint
>      registers with data from *STATE.  */
>   
> @@ -566,7 +582,7 @@ aarch64_linux_set_debug_regs (const struct aarch64_debug_reg_state *state,
>     count = watchpoint ? aarch64_num_wp_regs : aarch64_num_bp_regs;
>     addr = watchpoint ? state->dr_addr_wp : state->dr_addr_bp;
>     ctrl = watchpoint ? state->dr_ctrl_wp : state->dr_ctrl_bp;
> -  if (count == 0)
> +  if (count == 0 || initial_control_length (ctrl, count))
>       return;
>     iov.iov_len = (offsetof (struct user_hwdebug_state, dbg_regs)
>   		 + count * sizeof (regs.dbg_regs[0]));