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

Ulrich Weigand
2014-07-15T09:06:55Z

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