Re: [PATCH 3/3] Create arch_lwp_info class hierarchy

[Simon Marchi <simon dot marchi at ericsson dot com>]

On 2017-07-24 12:39 PM, Simon Marchi wrote:
> From: Simon Marchi <simon.marchi@polymtl.ca>
> 
> I am trying to "poison" the XNEW family of functions as well as xfree to
> help catch their uses on non-POD types.  It trips on this:
> 
> /usr/include/c++/5/type_traits: In instantiation of ‘struct std::is_pod<arch_lwp_info>’:
> /home/emaisin/src/binutils-gdb/gdb/common/traits.h:66:8:   required from ‘struct gdb::Or<std::is_pod<arch_lwp_info>, std::is_void<arch_lwp_info> >’
> /home/emaisin/src/binutils-gdb/gdb/common/common-utils.h:59:15:   required from ‘void xfree(T*) [with T = arch_lwp_info]’
> /home/emaisin/src/binutils-gdb/gdb/linux-nat.c:840:26:   required from here
> /usr/include/c++/5/type_traits:656:12: error: invalid use of incomplete type ‘struct arch_lwp_info’
>      struct is_pod
>             ^
> 
> The problem is that the arch_lwp_info type is defined internally by the
> various arch-specific linux-nat implementations in GDB and GDBserver,
> but opaque to linux-nat.c.  When linux-nat.c tries to xfree an object of
> incomplete type arch_lwp_info, it can't validate whether it is POD or
> not.  This patch converts this to a proper class hierarchy, with
> arch_lwp_info being an abstract class, and various arches defining their
> own type extending it.
> 
> This will allow using C++ features in these objects, if we ever need to.
> 
> gdb/ChangeLog:
> 
> 	* nat/linux-nat.h (arch_lwp_info): Define type, add virtual pure
> 	destructor.
> 	* linux-nat.h (struct arch_lwp_info): Remove forward
> 	declaration.
> 	(lwp_info) <arch_private>: Change type to use unique_ptr.
> 	* linux-nat.c (arch_lwp_info::~arch_lwp_info): Provide default
> 	implementation.
> 	(lwp_set_arch_private_info, lwp_arch_private_info): Adapt to use
> 	unique_ptr.
> 	(lwp_free): Don't free lp->arch_private manually.
> 	* arm-linux-nat.c (struct arch_lwp_info): Rename to ...
> 	(struct arm_lwp_info): ... this, inherit arch_lwp_info,
> 	initialize fields.
> 	(update_registers_callback): Allocate arm_lwp_info with new,
> 	adjust to unique_ptr usage.
> 	(arm_linux_new_thread): Likewise.
> 	(arm_linux_prepare_to_resume): Adjust to unique_ptr usage.
> 	* s390-linux-nat.c (struct arch_lwp_info): Rename to ...
> 	(s390_lwp_info): ... this, inherit arch_lwp_info, initialize
> 	fields.
> 	(s390_prepare_to_resume): Add cast.
> 	(s390_mark_per_info_changed): Allocate s390_lwp_info with new,
> 	add cast.
> 	* nat/aarch64-linux-hw-point.h (struct arch_lwp_info): Rename
> 	to ...
> 	(struct aarch64_lwp_info): ... this, inherit arch_lwp_info,
> 	initialize fields.
> 	* nat/aarch64-linux-hw-point.c (debug_reg_change_callback): Add
> 	cast, allocate aarch64_lwp_info with new.
> 	* nat/aarch64-linux.c (aarch64_linux_prepare_to_resume): Add
> 	cast.
> 	(aarch64_linux_new_thread): Allocate aarch64_lwp_info with new.
> 	* x86-linux.c (struct arch_lwp_info): Rename to ...
> 	(struct x86_lwp_info): ... this, inherit arch_lwp_info, initialize
> 	fields.
> 	(lwp_set_debug_registers_changed): Allocate x86_lwp_info with
> 	new, add cast.
> 	(lwp_debug_registers_changed): Add cast.
> 
> gdb/gdbserver/ChangeLog:
> 
> 	* linux-low.h (struct lwp_info) <arch_private> Change type to
> 	unique_ptr.
> 	* linux-arm-low.c (struct arch_lwp_info): Rename to ...
> 	(struct arm_lwp_info): ... this, inherit arch_lwp_info, initialize
> 	fields.
> 	(update_registers_callback, arm_stopped_by_watchpoint,
> 	arm_stopped_data_address): Use lwp_arch_private_info and add cast.
> 	(arm_new_thread): Allocate arm_lwp_info with new, use
> 	lwp_set_arch_private_info.
> 	(arm_new_fork, arm_prepare_to_resume): Use
> 	lwp_arch_private_info and add cast.
> 	* linux-low.c (delete_lwp): Don't manually free arch_private.
> 	(lwp_set_arch_private_info, lwp_arch_private_info): Adapt to use
> 	unique_ptr.
> 	(arch_lwp_info::~arch_lwp_info): Provide default implementation.
> 	* linux-mips-low.c (struct arch_lwp_info): Rename to ...
> 	(struct mips_lwp_info): ... this, inherit arch_lwp_info, initialize
> 	fields.
> 	(update_watch_registers_callback): Use lwp_arch_private_info and
> 	add cast.
> 	(mips_linux_new_thread): Allocate mips_lwp_info with new, use
> 	lwp_arch_private_info.
> 	(mips_linux_prepare_to_resume): Use lwp_arch_private_info and add cast.
> ---
>  gdb/arm-linux-nat.c              | 21 ++++++++++++---------
>  gdb/gdbserver/linux-arm-low.c    | 31 +++++++++++++++++--------------
>  gdb/gdbserver/linux-low.c        |  7 ++++---
>  gdb/gdbserver/linux-low.h        |  2 +-
>  gdb/gdbserver/linux-mips-low.c   | 16 +++++++++-------
>  gdb/linux-nat.c                  |  7 ++++---
>  gdb/linux-nat.h                  |  4 +---
>  gdb/nat/aarch64-linux-hw-point.c |  4 ++--
>  gdb/nat/aarch64-linux-hw-point.h |  6 +++---
>  gdb/nat/aarch64-linux.c          |  4 ++--
>  gdb/nat/linux-nat.h              |  7 ++++++-
>  gdb/nat/x86-linux.c              | 12 +++++++-----
>  gdb/s390-linux-nat.c             | 11 ++++++-----
>  13 files changed, 74 insertions(+), 58 deletions(-)
> 
> diff --git a/gdb/arm-linux-nat.c b/gdb/arm-linux-nat.c
> index ad3085a..19f9926 100644
> --- a/gdb/arm-linux-nat.c
> +++ b/gdb/arm-linux-nat.c
> @@ -726,11 +726,11 @@ struct arm_linux_process_info
>  };
>  
>  /* Per-thread arch-specific data we want to keep.  */
> -struct arch_lwp_info
> +struct arm_lwp_info : public arch_lwp_info
>  {
>    /* Non-zero if our copy differs from what's recorded in the thread.  */
> -  char bpts_changed[MAX_BPTS];
> -  char wpts_changed[MAX_WPTS];
> +  char bpts_changed[MAX_BPTS] = {0};
> +  char wpts_changed[MAX_WPTS] = {0};
>  };
>  
>  static struct arm_linux_process_info *arm_linux_process_list = NULL;
> @@ -928,14 +928,16 @@ update_registers_callback (struct lwp_info *lwp, void *arg)
>    struct update_registers_data *data = (struct update_registers_data *) arg;
>  
>    if (lwp->arch_private == NULL)
> -    lwp->arch_private = XCNEW (struct arch_lwp_info);
> +    lwp->arch_private.reset (new arm_lwp_info);
> +
> +  struct arm_lwp_info *info = (arm_lwp_info *) lwp->arch_private.get ();
>  
>    /* The actual update is done later just before resuming the lwp,
>       we just mark that the registers need updating.  */
>    if (data->watch)
> -    lwp->arch_private->wpts_changed[data->index] = 1;
> +    info->wpts_changed[data->index] = 1;
>    else
> -    lwp->arch_private->bpts_changed[data->index] = 1;
> +    info->bpts_changed[data->index] = 1;
>  
>    /* If the lwp isn't stopped, force it to momentarily pause, so
>       we can update its breakpoint registers.  */
> @@ -1186,7 +1188,7 @@ static void
>  arm_linux_new_thread (struct lwp_info *lp)
>  {
>    int i;
> -  struct arch_lwp_info *info = XCNEW (struct arch_lwp_info);
> +  struct arm_lwp_info *info = new arm_lwp_info;
>  
>    /* Mark that all the hardware breakpoint/watchpoint register pairs
>       for this thread need to be initialized.  */
> @@ -1197,7 +1199,7 @@ arm_linux_new_thread (struct lwp_info *lp)
>        info->wpts_changed[i] = 1;
>      }
>  
> -  lp->arch_private = info;
> +  lp->arch_private.reset (info);
>  }
>  
>  /* Called when resuming a thread.
> @@ -1208,7 +1210,8 @@ arm_linux_prepare_to_resume (struct lwp_info *lwp)
>  {
>    int pid, i;
>    struct arm_linux_hw_breakpoint *bpts, *wpts;
> -  struct arch_lwp_info *arm_lwp_info = lwp->arch_private;
> +  struct arm_lwp_info *arm_lwp_info
> +    = (struct arm_lwp_info *) lwp->arch_private.get ();
>  
>    pid = ptid_get_lwp (lwp->ptid);
>    bpts = arm_linux_get_debug_reg_state (ptid_get_pid (lwp->ptid))->bpts;
> diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
> index 5a3f465..9e28388 100644
> --- a/gdb/gdbserver/linux-arm-low.c
> +++ b/gdb/gdbserver/linux-arm-low.c
> @@ -110,13 +110,14 @@ struct arch_process_info
>  };
>  
>  /* Per-thread arch-specific data we want to keep.  */
> -struct arch_lwp_info
> +struct arm_lwp_info : public arch_lwp_info
>  {
>    /* Non-zero if our copy differs from what's recorded in the thread.  */
> -  char bpts_changed[MAX_BPTS];
> -  char wpts_changed[MAX_WPTS];
> +  char bpts_changed[MAX_BPTS] = {0};
> +  char wpts_changed[MAX_WPTS] = {0};
> +
>    /* Cached stopped data address.  */
> -  CORE_ADDR stopped_data_address;
> +  CORE_ADDR stopped_data_address = 0;
>  };
>  
>  /* These are in <asm/elf.h> in current kernels.  */
> @@ -471,6 +472,7 @@ update_registers_callback (struct inferior_list_entry *entry, void *arg)
>  {
>    struct thread_info *thread = (struct thread_info *) entry;
>    struct lwp_info *lwp = get_thread_lwp (thread);
> +  arm_lwp_info *arm_lwp = (arm_lwp_info *) lwp_arch_private_info (lwp);
>    struct update_registers_data *data = (struct update_registers_data *) arg;
>  
>    /* Only update the threads of the current process.  */
> @@ -479,9 +481,9 @@ update_registers_callback (struct inferior_list_entry *entry, void *arg)
>        /* The actual update is done later just before resuming the lwp,
>           we just mark that the registers need updating.  */
>        if (data->watch)
> -	lwp->arch_private->wpts_changed[data->i] = 1;
> +	arm_lwp->wpts_changed[data->i] = 1;
>        else
> -	lwp->arch_private->bpts_changed[data->i] = 1;
> +	arm_lwp->bpts_changed[data->i] = 1;
>  
>        /* If the lwp isn't stopped, force it to momentarily pause, so
>           we can update its breakpoint registers.  */
> @@ -594,6 +596,7 @@ static int
>  arm_stopped_by_watchpoint (void)
>  {
>    struct lwp_info *lwp = get_thread_lwp (current_thread);
> +  arm_lwp_info *arm_lwp = (arm_lwp_info *) lwp_arch_private_info (lwp);
>    siginfo_t siginfo;
>  
>    /* We must be able to set hardware watchpoints.  */
> @@ -617,8 +620,7 @@ arm_stopped_by_watchpoint (void)
>      return 0;
>  
>    /* Cache stopped data address for use by arm_stopped_data_address.  */
> -  lwp->arch_private->stopped_data_address
> -    = (CORE_ADDR) (uintptr_t) siginfo.si_addr;
> +  arm_lwp->stopped_data_address = (CORE_ADDR) (uintptr_t) siginfo.si_addr;
>  
>    return 1;
>  }
> @@ -629,7 +631,8 @@ static CORE_ADDR
>  arm_stopped_data_address (void)
>  {
>    struct lwp_info *lwp = get_thread_lwp (current_thread);
> -  return lwp->arch_private->stopped_data_address;
> +  arm_lwp_info *arm_lwp = (arm_lwp_info *) lwp_arch_private_info (lwp);
> +  return arm_lwp->stopped_data_address;
>  }
>  
>  /* Called when a new process is created.  */
> @@ -644,7 +647,7 @@ arm_new_process (void)
>  static void
>  arm_new_thread (struct lwp_info *lwp)
>  {
> -  struct arch_lwp_info *info = XCNEW (struct arch_lwp_info);
> +  arm_lwp_info *info = new arm_lwp_info;
>    int i;
>  
>    for (i = 0; i < MAX_BPTS; i++)
> @@ -652,7 +655,7 @@ arm_new_thread (struct lwp_info *lwp)
>    for (i = 0; i < MAX_WPTS; i++)
>      info->wpts_changed[i] = 1;
>  
> -  lwp->arch_private = info;
> +  lwp_set_arch_private_info (lwp, info);
>  }
>  
>  static void
> @@ -661,7 +664,6 @@ arm_new_fork (struct process_info *parent, struct process_info *child)
>    struct arch_process_info *parent_proc_info;
>    struct arch_process_info *child_proc_info;
>    struct lwp_info *child_lwp;
> -  struct arch_lwp_info *child_lwp_info;
>    int i;
>  
>    /* These are allocated by linux_add_process.  */
> @@ -692,7 +694,8 @@ arm_new_fork (struct process_info *parent, struct process_info *child)
>    /* Mark all the hardware breakpoints and watchpoints as changed to
>       make sure that the registers will be updated.  */
>    child_lwp = find_lwp_pid (ptid_of (child));
> -  child_lwp_info = child_lwp->arch_private;
> +  arm_lwp_info *child_lwp_info
> +    = (arm_lwp_info *) lwp_arch_private_info (child_lwp);
>    for (i = 0; i < MAX_BPTS; i++)
>      child_lwp_info->bpts_changed[i] = 1;
>    for (i = 0; i < MAX_WPTS; i++)
> @@ -708,7 +711,7 @@ arm_prepare_to_resume (struct lwp_info *lwp)
>    int pid = lwpid_of (thread);
>    struct process_info *proc = find_process_pid (pid_of (thread));
>    struct arch_process_info *proc_info = proc->priv->arch_private;
> -  struct arch_lwp_info *lwp_info = lwp->arch_private;
> +  arm_lwp_info *lwp_info = (arm_lwp_info *) lwp_arch_private_info (lwp);
>    int i;
>  
>    for (i = 0; i < arm_linux_get_hw_breakpoint_count (); i++)
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index e650b0d..5decca7 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -152,7 +152,7 @@ void
>  lwp_set_arch_private_info (struct lwp_info *lwp,
>  			   struct arch_lwp_info *info)
>  {
> -  lwp->arch_private = info;
> +  lwp->arch_private.reset (info);
>  }
>  
>  /* See nat/linux-nat.h.  */
> @@ -160,7 +160,7 @@ lwp_set_arch_private_info (struct lwp_info *lwp,
>  struct arch_lwp_info *
>  lwp_arch_private_info (struct lwp_info *lwp)
>  {
> -  return lwp->arch_private;
> +  return lwp->arch_private.get ()	;
>  }
>  
>  /* See nat/linux-nat.h.  */
> @@ -414,7 +414,6 @@ delete_lwp (struct lwp_info *lwp)
>      debug_printf ("deleting %ld\n", lwpid_of (thr));
>  
>    remove_thread (thr);
> -  free (lwp->arch_private);
>    delete lwp;
>  }
>  
> @@ -7588,6 +7587,8 @@ linux_get_pc_64bit (struct regcache *regcache)
>    return pc;
>  }
>  
> +arch_lwp_info::~arch_lwp_info () = default;
> +
>  
>  static struct target_ops linux_target_ops = {
>    linux_create_inferior,
> diff --git a/gdb/gdbserver/linux-low.h b/gdb/gdbserver/linux-low.h
> index dcc9315..43c1735 100644
> --- a/gdb/gdbserver/linux-low.h
> +++ b/gdb/gdbserver/linux-low.h
> @@ -377,7 +377,7 @@ struct lwp_info
>  #endif
>  
>    /* Arch-specific additions.  */
> -  struct arch_lwp_info *arch_private = NULL;
> +  std::unique_ptr<arch_lwp_info> arch_private;
>  };
>  
>  int linux_pid_exe_is_elf_64_file (int pid, unsigned int *machine);
> diff --git a/gdb/gdbserver/linux-mips-low.c b/gdb/gdbserver/linux-mips-low.c
> index b4a83b0..b09a099 100644
> --- a/gdb/gdbserver/linux-mips-low.c
> +++ b/gdb/gdbserver/linux-mips-low.c
> @@ -188,10 +188,10 @@ struct arch_process_info
>  
>  /* Per-thread arch-specific data we want to keep.  */
>  
> -struct arch_lwp_info
> +struct mips_lwp_info : public arch_lwp_info
>  {
>    /* Non-zero if our copy differs from what's recorded in the thread.  */
> -  int watch_registers_changed;
> +  int watch_registers_changed = 0;
>  };
>  
>  /* From mips-linux-nat.c.  */
> @@ -298,6 +298,7 @@ update_watch_registers_callback (struct inferior_list_entry *entry,
>  {
>    struct thread_info *thread = (struct thread_info *) entry;
>    struct lwp_info *lwp = get_thread_lwp (thread);
> +  mips_lwp_info *mips_lwp = (mips_lwp_info *) lwp_arch_private_info (lwp);
>    int pid = *(int *) pid_p;
>  
>    /* Only update the threads of this process.  */
> @@ -305,7 +306,7 @@ update_watch_registers_callback (struct inferior_list_entry *entry,
>      {
>        /* The actual update is done later just before resuming the lwp,
>  	 we just mark that the registers need updating.  */
> -      lwp->arch_private->watch_registers_changed = 1;
> +      mips_lwp->watch_registers_changed = 1;
>  
>        /* If the lwp isn't stopped, force it to momentarily pause, so
>  	 we can update its watch registers.  */
> @@ -334,11 +335,11 @@ mips_linux_new_process (void)
>  static void
>  mips_linux_new_thread (struct lwp_info *lwp)
>  {
> -  struct arch_lwp_info *info = XCNEW (struct arch_lwp_info);
> +  mips_lwp_info *info = new mips_lwp_info;
>  
>    info->watch_registers_changed = 1;
>  
> -  lwp->arch_private = info;
> +  lwp_set_arch_private_info (lwp, info);
>  }
>  
>  /* Create a new mips_watchpoint and add it to the list.  */
> @@ -413,8 +414,9 @@ mips_linux_prepare_to_resume (struct lwp_info *lwp)
>    ptid_t ptid = ptid_of (get_lwp_thread (lwp));
>    struct process_info *proc = find_process_pid (ptid_get_pid (ptid));
>    struct arch_process_info *priv = proc->priv->arch_private;
> +  mips_lwp_info *mips_lwp = (mips_lwp_info *) lwp_arch_private_info (lwp);
>  
> -  if (lwp->arch_private->watch_registers_changed)
> +  if (mips_lwp->watch_registers_changed)
>      {
>        /* Only update the watch registers if we have set or unset a
>  	 watchpoint already.  */
> @@ -428,7 +430,7 @@ mips_linux_prepare_to_resume (struct lwp_info *lwp)
>  	    perror_with_name ("Couldn't write watch register");
>  	}
>  
> -      lwp->arch_private->watch_registers_changed = 0;
> +      mips_lwp->watch_registers_changed = 0;
>      }
>  }
>  
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index 9378276..130a4d4 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -322,13 +322,15 @@ ptid_of_lwp (struct lwp_info *lwp)
>    return lwp->ptid;
>  }
>  
> +arch_lwp_info::~arch_lwp_info () = default;
> +
>  /* See nat/linux-nat.h.  */
>  
>  void
>  lwp_set_arch_private_info (struct lwp_info *lwp,
>  			   struct arch_lwp_info *info)
>  {
> -  lwp->arch_private = info;
> +  lwp->arch_private.reset (info);
>  }
>  
>  /* See nat/linux-nat.h.  */
> @@ -336,7 +338,7 @@ lwp_set_arch_private_info (struct lwp_info *lwp,
>  struct arch_lwp_info *
>  lwp_arch_private_info (struct lwp_info *lwp)
>  {
> -  return lwp->arch_private;
> +  return lwp->arch_private.get ();
>  }
>  
>  /* See nat/linux-nat.h.  */
> @@ -837,7 +839,6 @@ static int check_ptrace_stopped_lwp_gone (struct lwp_info *lp);
>  static void
>  lwp_free (struct lwp_info *lp)
>  {
> -  xfree (lp->arch_private);
>    delete lp;
>  }
>  
> diff --git a/gdb/linux-nat.h b/gdb/linux-nat.h
> index a783674..57fab42 100644
> --- a/gdb/linux-nat.h
> +++ b/gdb/linux-nat.h
> @@ -21,8 +21,6 @@
>  #include "target.h"
>  #include <signal.h>
>  
> -struct arch_lwp_info;
> -
>  /* Structure describing an LWP.  This is public only for the purposes
>     of ALL_LWPS; target-specific code should generally not access it
>     directly.  */
> @@ -103,7 +101,7 @@ struct lwp_info
>    int core = -1;
>  
>    /* Arch-specific additions.  */
> -  arch_lwp_info *arch_private = NULL;
> +  std::unique_ptr<arch_lwp_info> arch_private;
>  
>    /* Previous and next pointers in doubly-linked list of known LWPs,
>       sorted by reverse creation order.  */
> diff --git a/gdb/nat/aarch64-linux-hw-point.c b/gdb/nat/aarch64-linux-hw-point.c
> index 9800d9a..bba2200 100644
> --- a/gdb/nat/aarch64-linux-hw-point.c
> +++ b/gdb/nat/aarch64-linux-hw-point.c
> @@ -258,13 +258,13 @@ debug_reg_change_callback (struct lwp_info *lwp, void *ptr)
>    int tid = ptid_get_lwp (ptid_of_lwp (lwp));
>    int idx = param_p->idx;
>    int is_watchpoint = param_p->is_watchpoint;
> -  struct arch_lwp_info *info = lwp_arch_private_info (lwp);
> +  aarch64_lwp_info *info = (aarch64_lwp_info *) lwp_arch_private_info (lwp);
>    dr_changed_t *dr_changed_ptr;
>    dr_changed_t dr_changed;
>  
>    if (info == NULL)
>      {
> -      info = XCNEW (struct arch_lwp_info);
> +      info = new aarch64_lwp_info;
>        lwp_set_arch_private_info (lwp, info);
>      }
>  
> diff --git a/gdb/nat/aarch64-linux-hw-point.h b/gdb/nat/aarch64-linux-hw-point.h
> index 610a5f1..6547594 100644
> --- a/gdb/nat/aarch64-linux-hw-point.h
> +++ b/gdb/nat/aarch64-linux-hw-point.h
> @@ -154,13 +154,13 @@ struct aarch64_debug_reg_state
>  
>  /* Per-thread arch-specific data we want to keep.  */
>  
> -struct arch_lwp_info
> +struct aarch64_lwp_info : public arch_lwp_info
>  {
>    /* When bit N is 1, it indicates the Nth hardware breakpoint or
>       watchpoint register pair needs to be updated when the thread is
>       resumed; see aarch64_linux_prepare_to_resume.  */
> -  dr_changed_t dr_changed_bp;
> -  dr_changed_t dr_changed_wp;
> +  dr_changed_t dr_changed_bp = 0;
> +  dr_changed_t dr_changed_wp = 0;
>  };
>  
>  extern int aarch64_num_bp_regs;
> diff --git a/gdb/nat/aarch64-linux.c b/gdb/nat/aarch64-linux.c
> index 388eee8..12999da 100644
> --- a/gdb/nat/aarch64-linux.c
> +++ b/gdb/nat/aarch64-linux.c
> @@ -33,7 +33,7 @@
>  void
>  aarch64_linux_prepare_to_resume (struct lwp_info *lwp)
>  {
> -  struct arch_lwp_info *info = lwp_arch_private_info (lwp);
> +    aarch64_lwp_info *info = (aarch64_lwp_info *) lwp_arch_private_info (lwp);
>  
>    /* NULL means this is the main thread still going through the shell,
>       or, no watchpoint has been set yet.  In that case, there's
> @@ -73,7 +73,7 @@ aarch64_linux_prepare_to_resume (struct lwp_info *lwp)
>  void
>  aarch64_linux_new_thread (struct lwp_info *lwp)
>  {
> -  struct arch_lwp_info *info = XNEW (struct arch_lwp_info);
> +  aarch64_lwp_info *info = new aarch64_lwp_info;
>  
>    /* Mark that all the hardware breakpoint/watchpoint register pairs
>       for this thread need to be initialized (with data from
> diff --git a/gdb/nat/linux-nat.h b/gdb/nat/linux-nat.h
> index 7dd18fe..751f32c 100644
> --- a/gdb/nat/linux-nat.h
> +++ b/gdb/nat/linux-nat.h
> @@ -23,7 +23,12 @@
>  #include "target/waitstatus.h"
>  
>  struct lwp_info;
> -struct arch_lwp_info;
> +
> +/* Base class for arch-specific lwp data.  */
> +struct arch_lwp_info
> +{
> +  virtual ~arch_lwp_info () = 0;
> +};
>  
>  /* This is the kernel's hard limit.  Not to be confused with SIGRTMIN.  */
>  #ifndef __SIGRTMIN
> diff --git a/gdb/nat/x86-linux.c b/gdb/nat/x86-linux.c
> index b499e74..b35ed19 100644
> --- a/gdb/nat/x86-linux.c
> +++ b/gdb/nat/x86-linux.c
> @@ -23,11 +23,11 @@
>  
>  /* Per-thread arch-specific data we want to keep.  */
>  
> -struct arch_lwp_info
> +struct x86_lwp_info : public arch_lwp_info
>  {
>    /* Non-zero if our copy differs from what's recorded in the
>       thread.  */
> -  int debug_registers_changed;
> +  int debug_registers_changed = 0;
>  };
>  
>  /* See nat/x86-linux.h.  */
> @@ -36,9 +36,11 @@ void
>  lwp_set_debug_registers_changed (struct lwp_info *lwp, int value)
>  {
>    if (lwp_arch_private_info (lwp) == NULL)
> -    lwp_set_arch_private_info (lwp, XCNEW (struct arch_lwp_info));
> +    lwp_set_arch_private_info (lwp, new x86_lwp_info ());
>  
> -  lwp_arch_private_info (lwp)->debug_registers_changed = value;
> +  x86_lwp_info *info = (x86_lwp_info *) lwp_arch_private_info (lwp);
> +
> +  info->debug_registers_changed = value;
>  }
>  
>  /* See nat/x86-linux.h.  */
> @@ -46,7 +48,7 @@ lwp_set_debug_registers_changed (struct lwp_info *lwp, int value)
>  int
>  lwp_debug_registers_changed (struct lwp_info *lwp)
>  {
> -  struct arch_lwp_info *info = lwp_arch_private_info (lwp);
> +  x86_lwp_info *info = (x86_lwp_info *) lwp_arch_private_info (lwp);
>  
>    /* NULL means either that this is the main thread still going
>       through the shell, or that no watchpoint has been set yet.
> diff --git a/gdb/s390-linux-nat.c b/gdb/s390-linux-nat.c
> index 90c73c4..4e38f16 100644
> --- a/gdb/s390-linux-nat.c
> +++ b/gdb/s390-linux-nat.c
> @@ -44,10 +44,10 @@
>  
>  /* Per-thread arch-specific data.  */
>  
> -struct arch_lwp_info
> +struct s390_lwp_info : public arch_lwp_info
>  {
>    /* Non-zero if the thread's PER info must be re-written.  */
> -  int per_info_changed;
> +  int per_info_changed = 0;
>  };
>  
>  static int have_regset_last_break = 0;
> @@ -665,7 +665,7 @@ s390_prepare_to_resume (struct lwp_info *lp)
>    CORE_ADDR watch_lo_addr = (CORE_ADDR)-1, watch_hi_addr = 0;
>    unsigned ix;
>    s390_watch_area *area;
> -  struct arch_lwp_info *lp_priv = lwp_arch_private_info (lp);
> +  s390_lwp_info *lp_priv = (s390_lwp_info *) lwp_arch_private_info (lp);
>    struct s390_debug_reg_state *state = s390_get_debug_reg_state (pid);
>    int step = lwp_is_stepping (lp);
>  
> @@ -763,9 +763,10 @@ static void
>  s390_mark_per_info_changed (struct lwp_info *lp)
>  {
>    if (lwp_arch_private_info (lp) == NULL)
> -    lwp_set_arch_private_info (lp, XCNEW (struct arch_lwp_info));
> +    lwp_set_arch_private_info (lp, new s390_lwp_info);
>  
> -  lwp_arch_private_info (lp)->per_info_changed = 1;
> +  s390_lwp_info *lp_private = (s390_lwp_info *) lwp_arch_private_info (lp);
> +  lp_private->per_info_changed = 1;
>  }
>  
>  /* When attaching to a new thread, mark its PER info as changed.  */
> 

Yao, did you have any comments on patch 3/3?

Simon