[PING][PATCH 00/31] Turn gdbserver's linux low targets into classes

[PING][PATCH 00/31] Turn gdbserver's linux low targets into classes

[Tankut Baris Aktemur]

> Hi,
>
> This is a series that converts the linux low target definitions of
> gdbserver into classes that derive from 'linux_process_target'.
> Previously, gdbserver's target definition was converted into a class
> and the following class hierarchy was obtained:
>
> process_stratum_target
> ^
> |-- linux_process_target
> |-- win32_process_target
> |-- nto_process_target
> |-- lynx_process_target
>
> This is a continuation of that work.  At the end of the series we
> obtain
>
> process_stratum_target
> ^
> |-- linux_process_target
>     ^
>     |-- x86_target (defined in linux-x86-low.cc)
>     |-- aarch64_target (defined in linux-aarch64-low.cc)
>     |-- riscv_target (defined in linux-riscv-low.cc)
>     |-- ...
> |-- win32_process_target
> |-- nto_process_target
> |-- lynx_process_target
>
> Please see Patch 03 for more details about the approach.  The series
> is also available at
>
> https://github.com/barisaktemur/gdb/commits/gdbserver/cppify/linux-low/v1
>
> in case one wants to check out.

Kindly pinging for the series available at
https://sourceware.org/legacy-ml/gdb-patches/2020-03/msg00155.html

Thanks
Baris

Note: for some reason, the submitted patches are not seen in the new
web interface of the archive.  The 'legacy-ml' link is available.

[PING^2][PATCH 00/31] Turn gdbserver's linux low targets into classes

[Tankut Baris Aktemur]

> Hi,
>
> This is a series that converts the linux low target definitions of
> gdbserver into classes that derive from 'linux_process_target'.
> Previously, gdbserver's target definition was converted into a class
> and the following class hierarchy was obtained:
>
> process_stratum_target
> ^
> |-- linux_process_target
> |-- win32_process_target
> |-- nto_process_target
> |-- lynx_process_target
>
> This is a continuation of that work.  At the end of the series we
> obtain
>
> process_stratum_target
> ^
> |-- linux_process_target
>     ^
>     |-- x86_target (defined in linux-x86-low.cc)
>     |-- aarch64_target (defined in linux-aarch64-low.cc)
>     |-- riscv_target (defined in linux-riscv-low.cc)
>     |-- ...
> |-- win32_process_target
> |-- nto_process_target
> |-- lynx_process_target
>
> Please see Patch 03 for more details about the approach.  The series
> is also available at
>
> https://github.com/barisaktemur/gdb/commits/gdbserver/cppify/linux-low/v1
>
> in case one wants to check out.

Kindly pinging for the series available at
https://sourceware.org/legacy-ml/gdb-patches/2020-03/msg00155.html

Thanks
Baris

Note: for some reason, the submitted patches are not seen in the new
web interface of the archive.  The 'legacy-ml' link above works though.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 10/31] gdbserver/linux-low: turn 'breakpoint_kind_from_{pc, current_state}' into methods

[Pedro Alves]

On 3/6/20 9:31 PM, Tankut Baris Aktemur wrote:
> @@ -257,7 +259,7 @@ riscv_breakpoint_kind_from_pc (CORE_ADDR *pcptr)
>      }
>    buf;
>  
> -  if (target_read_memory (*pcptr, buf.bytes, sizeof (buf.insn)) == 0
> +  if (read_memory (*pcptr, buf.bytes, sizeof (buf.insn)) == 0

Is this really correct?  It seems like an unrelated change.
target_read_memory masks off breakpoint instructions, while read_memory
doesn't, I believe.

Thanks,
Pedro Alves

RE: [PATCH 10/31] gdbserver/linux-low: turn 'breakpoint_kind_from_{pc, current_state}' into methods

[Aktemur, Tankut Baris]

On Wednesday, April 1, 2020 9:16 PM, Pedro Alves wrote:
> On 3/6/20 9:31 PM, Tankut Baris Aktemur wrote:
> > @@ -257,7 +259,7 @@ riscv_breakpoint_kind_from_pc (CORE_ADDR *pcptr)
> >      }
> >    buf;
> >
> > -  if (target_read_memory (*pcptr, buf.bytes, sizeof (buf.insn)) == 0
> > +  if (read_memory (*pcptr, buf.bytes, sizeof (buf.insn)) == 0
> 
> Is this really correct?  It seems like an unrelated change.
> target_read_memory masks off breakpoint instructions, while read_memory
> doesn't, I believe.

You're right.  I was under the impression that target_read_memory was a macro
for the_target->read_memory.  It's not.  I'll check the entire series for
other occurrences of this and also target_write_memory.

Thanks.
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 13/31] gdbserver/linux-low: move 'decr_pc_after_break' into linux_process_target

[Pedro Alves]

I think a method would be better than a data field here.
I think ctors with data fields like that just make it for
harder to read code.  (Plus, it's not like pedantically multiple
objects of the same linux_target_ops subclass type would
need different values for this field.)

Thanks,
Pedro Alves

On 3/6/20 9:31 PM, Tankut Baris Aktemur wrote:
> gdbserver/ChangeLog:
> 2020-03-06  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
> 
> 	Move the 'decr_pc_after_break' linux_target_ops field into
> 	linux_process_target.
> 
> 	* linux-low.h (struct linux_target_ops)
> 	<decr_pc_after_break>: Remove.
> 	(class linux_process_target): Declare a constructor that takes
> 	'decr_pc_after_break' as parameter.
> 	(class linux_process_target) <m_decr_pc_after_break>: New field.
> 	* linux-low.cc (linux_process_target::linux_process_target):
> 	Constructor implementation.
> 
> 	Update the users below.
> 
> 	(linux_process_target::save_stop_reason)
> 	(linux_process_target::wait_1)
> 	* linux-x86-low.cc (class x86_target):
> 	Define a zero-arg constructor.
> 	(the_low_target): Remove the field.
> 	* linux-aarch64-low.cc (class aarch64_target):
> 	Define a zero-arg constructor.
> 	(the_low_target): Remove the field.
> 	* linux-arm-low.cc (class arm_target):
> 	Define a zero-arg constructor.
> 	(the_low_target): Remove the field.
> 	* linux-bfin-low.cc (class bfin_target):
> 	Define a zero-arg constructor.
> 	(the_low_target): Remove the field.
> 	* linux-cris-low.cc (class cris_target):
> 	Define a zero-arg constructor.
> 	(the_low_target): Remove the field.
> 	* linux-crisv32-low.cc (class crisv32_target):
> 	Define a zero-arg constructor.
> 	(the_low_target): Remove the field.
> 	* linux-ia64-low.cc (class ia64_target):
> 	Define a zero-arg constructor.
> 	* linux-m32r-low.cc (class m32r_target):
> 	Define a zero-arg constructor.
> 	(the_low_target): Remove the field.
> 	* linux-m68k-low.cc (class m68k_target):
> 	Define a zero-arg constructor.
> 	(the_low_target): Remove the field.
> 	* linux-mips-low.cc (class mips_target):
> 	Define a zero-arg constructor.
> 	(the_low_target): Remove the field.
> 	* linux-nios2-low.cc (class nios2_target):
> 	Define a zero-arg constructor.
> 	(the_low_target): Remove the field.
> 	* linux-ppc-low.cc (class ppc_target):
> 	Define a zero-arg constructor.
> 	(the_low_target): Remove the field.
> 	* linux-riscv-low.cc (class riscv_target):
> 	Define a zero-arg constructor.
> 	(the_low_target): Remove the field.
> 	* linux-s390-low.cc (class s390_target):
> 	Define a zero-arg constructor.
> 	(the_low_target): Remove the field.
> 	* linux-sh-low.cc (class sh_target):
> 	Define a zero-arg constructor.
> 	(the_low_target): Remove the field.
> 	* linux-sparc-low.cc (class sparc_target):
> 	Define a zero-arg constructor.
> 	(the_low_target): Remove the field.
> 	* linux-tic6x-low.cc (class tic6x_target):
> 	Define a zero-arg constructor.
> 	(the_low_target): Remove the field.
> 	* linux-tile-low.cc (class tile_target):
> 	Define a zero-arg constructor.
> 	(the_low_target): Remove the field.
> 	* linux-xtensa-low.cc (class xtensa_target):
> 	Define a zero-arg constructor.
> 	(the_low_target): Remove the field.
> ---
>  gdbserver/linux-aarch64-low.cc |  2 +-
>  gdbserver/linux-arm-low.cc     |  3 ++-
>  gdbserver/linux-bfin-low.cc    |  3 ++-
>  gdbserver/linux-cris-low.cc    |  3 ++-
>  gdbserver/linux-crisv32-low.cc |  3 ++-
>  gdbserver/linux-ia64-low.cc    |  2 ++
>  gdbserver/linux-low.cc         | 10 ++++++++--
>  gdbserver/linux-low.h          |  6 +++++-
>  gdbserver/linux-m32r-low.cc    |  3 ++-
>  gdbserver/linux-m68k-low.cc    |  3 ++-
>  gdbserver/linux-mips-low.cc    |  3 ++-
>  gdbserver/linux-nios2-low.cc   |  3 ++-
>  gdbserver/linux-ppc-low.cc     |  3 ++-
>  gdbserver/linux-riscv-low.cc   |  3 ++-
>  gdbserver/linux-s390-low.cc    |  9 +++++----
>  gdbserver/linux-sh-low.cc      |  3 ++-
>  gdbserver/linux-sparc-low.cc   |  3 ++-
>  gdbserver/linux-tic6x-low.cc   |  3 ++-
>  gdbserver/linux-tile-low.cc    |  3 ++-
>  gdbserver/linux-x86-low.cc     |  3 ++-
>  gdbserver/linux-xtensa-low.cc  |  3 ++-
>  21 files changed, 53 insertions(+), 24 deletions(-)
> 
> diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc
> index 11c3296eb6b..d8b2402bd6b 100644
> --- a/gdbserver/linux-aarch64-low.cc
> +++ b/gdbserver/linux-aarch64-low.cc
> @@ -54,6 +54,7 @@
>  class aarch64_target : public linux_process_target
>  {
>  public:
> +  aarch64_target () : linux_process_target (0) { }
>  
>    const regs_info *get_regs_info () override;
>  
> @@ -3103,7 +3104,6 @@ aarch64_supports_hardware_single_step (void)
>  
>  struct linux_target_ops the_low_target =
>  {
> -  0,    /* decr_pc_after_break */
>    aarch64_breakpoint_at,
>    aarch64_supports_z_point_type,
>    aarch64_insert_point,
> diff --git a/gdbserver/linux-arm-low.cc b/gdbserver/linux-arm-low.cc
> index bd42feba1c4..0dca30fb58d 100644
> --- a/gdbserver/linux-arm-low.cc
> +++ b/gdbserver/linux-arm-low.cc
> @@ -60,6 +60,8 @@ class arm_target : public linux_process_target
>  {
>  public:
>  
> +  arm_target () : linux_process_target (0) { }
> +
>    const regs_info *get_regs_info () override;
>  
>    int breakpoint_kind_from_pc (CORE_ADDR *pcptr) override;
> @@ -1085,7 +1087,6 @@ arm_target::get_regs_info ()
>  }
>  
>  struct linux_target_ops the_low_target = {
> -  0,
>    arm_breakpoint_at,
>    arm_supports_z_point_type,
>    arm_insert_point,
> diff --git a/gdbserver/linux-bfin-low.cc b/gdbserver/linux-bfin-low.cc
> index 17948ed16f0..48373ccb178 100644
> --- a/gdbserver/linux-bfin-low.cc
> +++ b/gdbserver/linux-bfin-low.cc
> @@ -29,6 +29,8 @@ class bfin_target : public linux_process_target
>  {
>  public:
>  
> +  bfin_target () : linux_process_target (2) { }
> +
>    const regs_info *get_regs_info () override;
>  
>    const gdb_byte *sw_breakpoint_from_kind (int kind, int *size) override;
> @@ -161,7 +163,6 @@ bfin_target::get_regs_info ()
>  }
>  
>  struct linux_target_ops the_low_target = {
> -  2,
>    bfin_breakpoint_at,
>    NULL, /* supports_z_point_type */
>    NULL, /* insert_point */
> diff --git a/gdbserver/linux-cris-low.cc b/gdbserver/linux-cris-low.cc
> index 99060de1765..e7e389d049d 100644
> --- a/gdbserver/linux-cris-low.cc
> +++ b/gdbserver/linux-cris-low.cc
> @@ -26,6 +26,8 @@ class cris_target : public linux_process_target
>  {
>  public:
>  
> +  cris_target () : linux_process_target (0) { }
> +
>    const regs_info *get_regs_info () override;
>  
>    const gdb_byte *sw_breakpoint_from_kind (int kind, int *size) override;
> @@ -158,7 +160,6 @@ cris_target::get_regs_info ()
>  }
>  
>  struct linux_target_ops the_low_target = {
> -  0,
>    cris_breakpoint_at,
>  };
>  
> diff --git a/gdbserver/linux-crisv32-low.cc b/gdbserver/linux-crisv32-low.cc
> index c75e428885e..ad22ac0f9cc 100644
> --- a/gdbserver/linux-crisv32-low.cc
> +++ b/gdbserver/linux-crisv32-low.cc
> @@ -26,6 +26,8 @@ class crisv32_target : public linux_process_target
>  {
>  public:
>  
> +  crisv32_target () : linux_process_target (0) { }
> +
>    const regs_info *get_regs_info () override;
>  
>    const gdb_byte *sw_breakpoint_from_kind (int kind, int *size) override;
> @@ -455,7 +457,6 @@ crisv32_target::get_regs_info ()
>  }
>  
>  struct linux_target_ops the_low_target = {
> -  0,
>    cris_breakpoint_at,
>    cris_supports_z_point_type,
>    cris_insert_point,
> diff --git a/gdbserver/linux-ia64-low.cc b/gdbserver/linux-ia64-low.cc
> index 493c7e4f93d..0eb0ac53453 100644
> --- a/gdbserver/linux-ia64-low.cc
> +++ b/gdbserver/linux-ia64-low.cc
> @@ -29,6 +29,8 @@ class ia64_target : public linux_process_target
>  {
>  public:
>  
> +  ia64_target () : linux_process_target (0) { }
> +
>    const regs_info *get_regs_info () override;
>  
>    const gdb_byte *sw_breakpoint_from_kind (int kind, int *size) override;
> diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
> index 8eca077180e..2747c9f6b43 100644
> --- a/gdbserver/linux-low.cc
> +++ b/gdbserver/linux-low.cc
> @@ -281,6 +281,12 @@ static int check_ptrace_stopped_lwp_gone (struct lwp_info *lp);
>     being stepped.  */
>  ptid_t step_over_bkpt;
>  
> +linux_process_target::linux_process_target (int decr_pc_after_break)
> +  : m_decr_pc_after_break {decr_pc_after_break}
> +{
> +  /* Nop.  */
> +}
> +
>  /* True if the low target can hardware single-step.  */
>  
>  static int
> @@ -796,7 +802,7 @@ linux_process_target::save_stop_reason (lwp_info *lwp)
>      return false;
>  
>    pc = get_pc (lwp);
> -  sw_breakpoint_pc = pc - the_low_target.decr_pc_after_break;
> +  sw_breakpoint_pc = pc - this->m_decr_pc_after_break;
>  
>    /* breakpoint_at reads from the current thread.  */
>    saved_thread = current_thread;
> @@ -3694,7 +3700,7 @@ linux_process_target::wait_1 (ptid_t ptid, target_waitstatus *ourstatus,
>    if (event_child->stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT
>        && !cs.swbreak_feature)
>      {
> -      int decr_pc = the_low_target.decr_pc_after_break;
> +      int decr_pc = this->m_decr_pc_after_break;
>  
>        if (decr_pc != 0)
>  	{
> diff --git a/gdbserver/linux-low.h b/gdbserver/linux-low.h
> index 2202a658225..e289d5d06a0 100644
> --- a/gdbserver/linux-low.h
> +++ b/gdbserver/linux-low.h
> @@ -131,7 +131,6 @@ struct lwp_info;
>  
>  struct linux_target_ops
>  {
> -  int decr_pc_after_break;
>    int (*breakpoint_at) (CORE_ADDR pc);
>  
>    /* Breakpoint and watchpoint related functions.  See target.h for
> @@ -237,6 +236,8 @@ class linux_process_target : public process_stratum_target
>  {
>  public:
>  
> +  linux_process_target (int decr_pc_after_break);
> +
>    int create_inferior (const char *program,
>  		       const std::vector<char *> &program_args) override;
>  
> @@ -666,6 +667,9 @@ class linux_process_target : public process_stratum_target
>       Targets that override this method should also override
>       'supports_software_single_step' to return true.  */
>    virtual std::vector<CORE_ADDR> low_get_next_pcs (regcache *regcache);
> +
> +  /* How many bytes the PC should be decremented after a break.  */
> +  int m_decr_pc_after_break;
>  };
>  
>  extern linux_process_target *the_linux_target;
> diff --git a/gdbserver/linux-m32r-low.cc b/gdbserver/linux-m32r-low.cc
> index 78e002dfb24..4422951c342 100644
> --- a/gdbserver/linux-m32r-low.cc
> +++ b/gdbserver/linux-m32r-low.cc
> @@ -29,6 +29,8 @@ class m32r_target : public linux_process_target
>  {
>  public:
>  
> +  m32r_target () : linux_process_target (0) { }
> +
>    const regs_info *get_regs_info () override;
>  
>    const gdb_byte *sw_breakpoint_from_kind (int kind, int *size) override;
> @@ -160,7 +162,6 @@ m32r_target::get_regs_info ()
>  }
>  
>  struct linux_target_ops the_low_target = {
> -  0,
>    m32r_breakpoint_at,
>    NULL, /* supports_z_point_type */
>    NULL, /* insert_point */
> diff --git a/gdbserver/linux-m68k-low.cc b/gdbserver/linux-m68k-low.cc
> index 00851af5f02..5b6a1e391db 100644
> --- a/gdbserver/linux-m68k-low.cc
> +++ b/gdbserver/linux-m68k-low.cc
> @@ -25,6 +25,8 @@ class m68k_target : public linux_process_target
>  {
>  public:
>  
> +  m68k_target () : linux_process_target (2) { }
> +
>    const regs_info *get_regs_info () override;
>  
>    const gdb_byte *sw_breakpoint_from_kind (int kind, int *size) override;
> @@ -255,7 +257,6 @@ m68k_supports_hardware_single_step (void)
>  }
>  
>  struct linux_target_ops the_low_target = {
> -  2,
>    m68k_breakpoint_at,
>    NULL, /* supports_z_point_type */
>    NULL, /* insert_point */
> diff --git a/gdbserver/linux-mips-low.cc b/gdbserver/linux-mips-low.cc
> index 710245c1610..b02c9272764 100644
> --- a/gdbserver/linux-mips-low.cc
> +++ b/gdbserver/linux-mips-low.cc
> @@ -31,6 +31,8 @@ class mips_target : public linux_process_target
>  {
>  public:
>  
> +  mips_target () : linux_process_target (0) { }
> +
>    const regs_info *get_regs_info () override;
>  
>    const gdb_byte *sw_breakpoint_from_kind (int kind, int *size) override;
> @@ -966,7 +968,6 @@ mips_target::get_regs_info ()
>  }
>  
>  struct linux_target_ops the_low_target = {
> -  0,
>    mips_breakpoint_at,
>    mips_supports_z_point_type,
>    mips_insert_point,
> diff --git a/gdbserver/linux-nios2-low.cc b/gdbserver/linux-nios2-low.cc
> index 3cae8dac285..4fa80cd751b 100644
> --- a/gdbserver/linux-nios2-low.cc
> +++ b/gdbserver/linux-nios2-low.cc
> @@ -37,6 +37,8 @@ class nios2_target : public linux_process_target
>  {
>  public:
>  
> +  nios2_target () : linux_process_target (0) { }
> +
>    const regs_info *get_regs_info () override;
>  
>    const gdb_byte *sw_breakpoint_from_kind (int kind, int *size) override;
> @@ -277,7 +279,6 @@ nios2_target::get_regs_info ()
>  
>  struct linux_target_ops the_low_target =
>  {
> -  0,
>    nios2_breakpoint_at,
>  };
>  
> diff --git a/gdbserver/linux-ppc-low.cc b/gdbserver/linux-ppc-low.cc
> index af9dc0b2adf..66b5f9bc565 100644
> --- a/gdbserver/linux-ppc-low.cc
> +++ b/gdbserver/linux-ppc-low.cc
> @@ -50,6 +50,8 @@ class ppc_target : public linux_process_target
>  {
>  public:
>  
> +  ppc_target () : linux_process_target (0) { }
> +
>    const regs_info *get_regs_info () override;
>  
>    const gdb_byte *sw_breakpoint_from_kind (int kind, int *size) override;
> @@ -3406,7 +3408,6 @@ ppc_get_ipa_tdesc_idx (void)
>  }
>  
>  struct linux_target_ops the_low_target = {
> -  0,
>    ppc_breakpoint_at,
>    ppc_supports_z_point_type,
>    ppc_insert_point,
> diff --git a/gdbserver/linux-riscv-low.cc b/gdbserver/linux-riscv-low.cc
> index 31401dbdbbe..701c4370a32 100644
> --- a/gdbserver/linux-riscv-low.cc
> +++ b/gdbserver/linux-riscv-low.cc
> @@ -36,6 +36,8 @@ class riscv_target : public linux_process_target
>  {
>  public:
>  
> +  riscv_target () : linux_process_target (0) { }
> +
>    const regs_info *get_regs_info () override;
>  
>    int breakpoint_kind_from_pc (CORE_ADDR *pcptr) override;
> @@ -309,7 +311,6 @@ riscv_breakpoint_at (CORE_ADDR pc)
>  /* RISC-V/Linux target operations.  */
>  struct linux_target_ops the_low_target =
>  {
> -  0,    /* decr_pc_after_break */
>    riscv_breakpoint_at,
>  };
>  
> diff --git a/gdbserver/linux-s390-low.cc b/gdbserver/linux-s390-low.cc
> index 4c4b877bb75..68ab1ef7273 100644
> --- a/gdbserver/linux-s390-low.cc
> +++ b/gdbserver/linux-s390-low.cc
> @@ -51,12 +51,17 @@
>  
>  #define s390_num_regs 52
>  
> +static const gdb_byte s390_breakpoint[] = { 0, 1 };
> +#define s390_breakpoint_len 2
> +
>  /* Linux target op definitions for the S/390 architecture.  */
>  
>  class s390_target : public linux_process_target
>  {
>  public:
>  
> +  s390_target () : linux_process_target (s390_breakpoint_len) { }
> +
>    const regs_info *get_regs_info () override;
>  
>    const gdb_byte *sw_breakpoint_from_kind (int kind, int *size) override;
> @@ -450,9 +455,6 @@ static struct regset_info s390_regsets[] = {
>  };
>  
>  
> -static const gdb_byte s390_breakpoint[] = { 0, 1 };
> -#define s390_breakpoint_len 2
> -
>  /* Implementation of target ops method "sw_breakpoint_from_kind".  */
>  
>  const gdb_byte *
> @@ -2826,7 +2828,6 @@ s390_emit_ops (void)
>  }
>  
>  struct linux_target_ops the_low_target = {
> -  s390_breakpoint_len,
>    s390_breakpoint_at,
>    s390_supports_z_point_type,
>    NULL,
> diff --git a/gdbserver/linux-sh-low.cc b/gdbserver/linux-sh-low.cc
> index 47242dafe94..7f55f34a0bc 100644
> --- a/gdbserver/linux-sh-low.cc
> +++ b/gdbserver/linux-sh-low.cc
> @@ -25,6 +25,8 @@ class sh_target : public linux_process_target
>  {
>  public:
>  
> +  sh_target () : linux_process_target (0) { }
> +
>    const regs_info *get_regs_info () override;
>  
>    const gdb_byte *sw_breakpoint_from_kind (int kind, int *size) override;
> @@ -190,7 +192,6 @@ sh_target::low_arch_setup ()
>  }
>  
>  struct linux_target_ops the_low_target = {
> -  0,
>    sh_breakpoint_at,
>    NULL, /* supports_z_point_type */
>    NULL, /* insert_point */
> diff --git a/gdbserver/linux-sparc-low.cc b/gdbserver/linux-sparc-low.cc
> index f724cb88b71..1998267196d 100644
> --- a/gdbserver/linux-sparc-low.cc
> +++ b/gdbserver/linux-sparc-low.cc
> @@ -48,6 +48,8 @@ class sparc_target : public linux_process_target
>  {
>  public:
>  
> +  sparc_target () : linux_process_target (0) { }
> +
>    const regs_info *get_regs_info () override;
>  
>    const gdb_byte *sw_breakpoint_from_kind (int kind, int *size) override;
> @@ -339,7 +341,6 @@ sparc_target::get_regs_info ()
>  }
>  
>  struct linux_target_ops the_low_target = {
> -  0,
>    sparc_breakpoint_at,
>    NULL,  /* supports_z_point_type */
>    NULL, NULL, NULL, NULL,
> diff --git a/gdbserver/linux-tic6x-low.cc b/gdbserver/linux-tic6x-low.cc
> index 347b79e6519..596177e0b20 100644
> --- a/gdbserver/linux-tic6x-low.cc
> +++ b/gdbserver/linux-tic6x-low.cc
> @@ -44,6 +44,8 @@ class tic6x_target : public linux_process_target
>  {
>  public:
>  
> +  tic6x_target () : linux_process_target (0) { }
> +
>    const regs_info *get_regs_info () override;
>  
>    const gdb_byte *sw_breakpoint_from_kind (int kind, int *size) override;
> @@ -421,7 +423,6 @@ tic6x_target::get_regs_info ()
>  }
>  
>  struct linux_target_ops the_low_target = {
> -  0,
>    tic6x_breakpoint_at,
>    NULL, /* supports_z_point_type */
>    NULL, /* insert_point */
> diff --git a/gdbserver/linux-tile-low.cc b/gdbserver/linux-tile-low.cc
> index 86191b9bb1c..40350e026c8 100644
> --- a/gdbserver/linux-tile-low.cc
> +++ b/gdbserver/linux-tile-low.cc
> @@ -29,6 +29,8 @@ class tile_target : public linux_process_target
>  {
>  public:
>  
> +  tile_target () : linux_process_target (0) { }
> +
>    const regs_info *get_regs_info () override;
>  
>    const gdb_byte *sw_breakpoint_from_kind (int kind, int *size) override;
> @@ -222,7 +224,6 @@ tile_supports_hardware_single_step (void)
>  
>  struct linux_target_ops the_low_target =
>  {
> -  0,
>    tile_breakpoint_at,
>    NULL, /* supports_z_point_type */
>    NULL, /* insert_point */
> diff --git a/gdbserver/linux-x86-low.cc b/gdbserver/linux-x86-low.cc
> index 59c6b386d11..121b367e5fc 100644
> --- a/gdbserver/linux-x86-low.cc
> +++ b/gdbserver/linux-x86-low.cc
> @@ -100,6 +100,8 @@ class x86_target : public linux_process_target
>  {
>  public:
>  
> +  x86_target () : linux_process_target (1) { }
> +
>    /* Update all the target description of all processes; a new GDB
>       connected, and it may or not support xml target descriptions.  */
>    void update_xmltarget ();
> @@ -2899,7 +2901,6 @@ x86_get_ipa_tdesc_idx (void)
>  
>  struct linux_target_ops the_low_target =
>  {
> -  1,
>    x86_breakpoint_at,
>    x86_supports_z_point_type,
>    x86_insert_point,
> diff --git a/gdbserver/linux-xtensa-low.cc b/gdbserver/linux-xtensa-low.cc
> index 320ac92968d..d4d0780d894 100644
> --- a/gdbserver/linux-xtensa-low.cc
> +++ b/gdbserver/linux-xtensa-low.cc
> @@ -26,6 +26,8 @@ class xtensa_target : public linux_process_target
>  {
>  public:
>  
> +  xtensa_target () : linux_process_target (0) { }
> +
>    const regs_info *get_regs_info () override;
>  
>    const gdb_byte *sw_breakpoint_from_kind (int kind, int *size) override;
> @@ -328,7 +330,6 @@ xtensa_target::get_regs_info ()
>  }
>  
>  struct linux_target_ops the_low_target = {
> -  0,
>    xtensa_breakpoint_at,
>    NULL, /* supports_z_point_type */
>    NULL, /* insert_point */
> 

Re: [PATCH 00/31] Turn gdbserver's linux low targets into classes

[Pedro Alves]

On 3/6/20 9:31 PM, Tankut Baris Aktemur wrote:

> This is a series that converts the linux low target definitions of
> gdbserver into classes that derive from 'linux_process_target'.

This is excellent, thanks much for doing this.  Excellent work
splitting things up, and I like how you used the pattern
of calling the low virtual methods "low_xxx", like gdb/linux-nat.h does.

Other than the two patches I replied to, this all looks great to me.

Thanks,
Pedro Alves

RE: [PATCH 00/31] Turn gdbserver's linux low targets into classes

[Aktemur, Tankut Baris]

On Wednesday, April 1, 2020 11:17 PM, Pedro Alves wrote:
> On 3/6/20 9:31 PM, Tankut Baris Aktemur wrote:
> 
> > This is a series that converts the linux low target definitions of
> > gdbserver into classes that derive from 'linux_process_target'.
> 
> This is excellent, thanks much for doing this.  Excellent work
> splitting things up, and I like how you used the pattern
> of calling the low virtual methods "low_xxx", like gdb/linux-nat.h does.
> 
> Other than the two patches I replied to, this all looks great to me.
> 
> Thanks,
> Pedro Alves

Thank you for the thorough reviews.  I'll push after fixing those two things.

-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 14/31] gdbserver/linux-low: turn 'breakpoint_at' into a method

[Andreas Schwab]

../../gdbserver/linux-ia64-low.cc: In member function ‘virtual bool ia64_target::low_breakpoint_at(CORE_ADDR)’:
../../gdbserver/linux-ia64-low.cc:64:33: error: ‘gdb_assert_no_reached’ was not declared in this scope
     "implemented by this target");
                                 ^
../../gdbserver/linux-ia64-low.cc:65:1: warning: no return statement in function returning non-void [-Wreturn-type]
 }
 ^

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH 11/31] gdbserver/linux-low: turn 'sw_breakpoint_from_kind' into a method

[Andreas Schwab]

../../gdbserver/linux-ia64-low.cc: In member function ‘virtual const gdb_byte* ia64_target::sw_breakpoint_from_kind(int, int*)’:
../../gdbserver/linux-ia64-low.cc:57:33: error: ‘gdb_assert_no_reached’ was not declared in this scope
     "implemented by this target");
                                 ^
../../gdbserver/linux-ia64-low.cc:58:1: warning: no return statement in function returning non-void [-Wreturn-type]
 }
 ^

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

RE: [PATCH 14/31] gdbserver/linux-low: turn 'breakpoint_at' into a method

[Aktemur, Tankut Baris]

On Saturday, May 16, 2020 10:08 AM, Andreas Schwab wrote:
> ../../gdbserver/linux-ia64-low.cc: In member function ‘virtual bool ia64_target::low_breakpoint_at(CORE_ADDR)’:
> ../../gdbserver/linux-ia64-low.cc:64:33: error: ‘gdb_assert_no_reached’ was not declared in this scope
>      "implemented by this target");
>                                  ^
> ../../gdbserver/linux-ia64-low.cc:65:1: warning: no return statement in function returning non-void [-Wreturn-type]
>  }
>  ^

Oops.  That's the risk of applying a refactoring that applies to a wide
range of targets.  I pushed the patch below.  Thanks for the notification.

-Baris


From 8bbf03947dd594262e672c1fbc3462a81c811b6f Mon Sep 17 00:00:00 2001
From: Tankut Baris Aktemur <tankut.baris.aktemur@intel.com>
Date: Sat, 16 May 2020 10:24:24 +0200
Subject: [PATCH] gdbserver/linux-ia64-low: fix a build-breaking typo

During the gdbserver c++'ification refactoring, I apparently made a
typo that broke build in ia64 targets.

gdbserver/ChangeLog:
2020-05-16  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>

	* linux-ia64-low.cc (ia64_target::sw_breakpoint_from_kind):
	Fix incorrect 'gdb_assert_no_reached' to 'gdb_assert_not_reached'.
	(ia64_target::low_breakpoint_at): Ditto.
---
 gdbserver/ChangeLog         | 6 ++++++
 gdbserver/linux-ia64-low.cc | 8 ++++----
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/gdbserver/ChangeLog b/gdbserver/ChangeLog
index d2b2444d09b..3a27705f8df 100644
--- a/gdbserver/ChangeLog
+++ b/gdbserver/ChangeLog
@@ -1,3 +1,9 @@
+2020-05-16  Tankut Baris Aktemur  <tankut.baris.aktemur@intel.com>
+
+	* linux-ia64-low.cc (ia64_target::sw_breakpoint_from_kind):
+	Fix incorrect 'gdb_assert_no_reached' to 'gdb_assert_not_reached'.
+	(ia64_target::low_breakpoint_at): Ditto.
+
 2020-05-15  Hannes Domani  <ssbssa@yahoo.de>
 
 	* win32-i386-low.cc (i386_supports_z_point_type): Handle
diff --git a/gdbserver/linux-ia64-low.cc b/gdbserver/linux-ia64-low.cc
index 83a180871dc..ee255418b0f 100644
--- a/gdbserver/linux-ia64-low.cc
+++ b/gdbserver/linux-ia64-low.cc
@@ -53,15 +53,15 @@ static ia64_target the_ia64_target;
 const gdb_byte *
 ia64_target::sw_breakpoint_from_kind (int kind, int *size)
 {
-  gdb_assert_no_reached ("target op sw_breakpoint_from_kind is not "
-			 "implemented by this target");
+  gdb_assert_not_reached ("target op sw_breakpoint_from_kind is not "
+			  "implemented by this target");
 }
 
 bool
 ia64_target::low_breakpoint_at (CORE_ADDR pc)
 {
-  gdb_assert_no_reached ("linux target op low_breakpoint_at is not "
-			 "implemented by this target");
+  gdb_assert_not_reached ("linux target op low_breakpoint_at is not "
+			  "implemented by this target");
 }
 
 /* Defined in auto-generated file reg-ia64.c.  */
-- 
2.17.1

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928