Re: [PATCH v2 03/13] S390: Migrate to regcache_supply/collect_regset.

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Andreas Arnez wrote:

> gdb/
> 	* s390-linux-nat.c: Include "regset.h".
> 	(regmap_gregset): Delete macro.
> 	(s390_64_regmap_gregset): New register map for
> 	regcache_supply/_collect_regset.
> 	(s390_64_gregset): New regset.
> 	(S390_PSWM_OFFSET, S390_PSWA_OFFSET): New macros.
> 	(regmap_fpregset): Delete macro.
> 	(s390_native_supply, s390_native_collect): Delete functions.
> 	(supply_gregset, fill_gregset): Replace s390-specific regmap
> 	handling by a call to regcache_supply/_collect_regset.
> 	(supply_fpregset, fill_fpregset): Call regcache_supply/
> 	_collect_regset instead of s390_native_supply/_collect.
> 	(fetch_regset, store_regset): Likewise.  Also change the last
> 	parameter to a regset instead of a regmap.
> 	(s390_linux_fetch_inferior_registers)
> 	(390_linux_store_inferior_registers): Adjust last parameter in
> 	calls to fetch_regset and store_regset.
> 	* s390-linux-tdep.c (s390_regmap_gregset): Rename to...
> 	(s390_gregmap): ... this.  Also make static const and convert to
> 	regcache_map_entry format.
> 	(s390x_regmap_gregset): Delete.
> 	(s390_regmap_fpregset): Rename to...
> 	(s390_fpregmap): ... this.  Make static const and convert to
> 	regcache_map_entry format.
> 	(s390_regmap_upper, s390_regmap_last_break)
> 	(s390x_regmap_last_break, s390_regmap_system_call)
> 	(s390_regmap_tdb): Likewise.
> 	(s390_supply_regset, s390_collect_regset): Remove functions.
> 	(s390_supply_tdb_regset): Call regcache_supply_regset instead of
> 	s390_supply_regset.
> 	(s390_gregset, s390_fpregset, s390_upper_regset)
> 	(s390_last_break_regset, s390x_last_break_regset)
> 	(s390_system_call_regset, s390_tdb_regset): Make global and
> 	replace s390_supply/_collect_regset by regcache_supply/
> 	_collect_regset.
> 	(s390x_gregset): Delete.
> 	(s390_gdbarch_init): Replace s390x_gregset by s390_gregset.
> 	* s390-linux-tdep.h (s390_regmap_gregset, s390x_regmap_gregset)
> 	(s390_regmap_fpregset, s390_regmap_last_break)
> 	(s390x_regmap_last_break, s390_regmap_system_call)
> 	(s390_regmap_tdb): Delete global variable declarations.
> 	(s390_gregset, s390_fpregset, s390_last_break_regset)
> 	(s390x_last_break_regset, s390_system_call_regset)
> 	(s390_tdb_regset): New global variable declarations.

This is OK, except that:

> -const short s390_regmap_last_break[] =
> -  {
> -    0x04, S390_LAST_BREAK_REGNUM,
> -    -1, -1
> -  };
> -
> -const short s390x_regmap_last_break[] =
> -  {
> -    0x00, S390_LAST_BREAK_REGNUM,
> -    -1, -1
> -  };

These two are deliberately different, but the replacements
are identical:

> +static const struct regcache_map_entry s390_regmap_last_break[] =
> +  {
> +    { 1, S390_LAST_BREAK_REGNUM },
> +    { 0 }
> +  };
>  
> +static const struct regcache_map_entry s390x_regmap_last_break[] =
> +  {
> +    { 1, S390_LAST_BREAK_REGNUM },
> +    { 0 }
> +  };

That seems wrong, I think s390_regmap_last_break needs to
skip the initial 4 bytes.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com