Re: [PATCH 2/8] gdb/s390: Fill write_guessed_tracepoint_pc hook.

Andreas Arnez
2016-01-26T18:12:43Z

On Sun, Jan 24 2016, Marcin Kościelnicki wrote:

> diff --git a/gdb/s390-linux-tdep.c b/gdb/s390-linux-tdep.c
> index e827684..c4d25d2 100644
> --- a/gdb/s390-linux-tdep.c
> +++ b/gdb/s390-linux-tdep.c
> @@ -167,6 +167,22 @@ s390_write_pc (struct regcache *regcache, CORE_ADDR pc)
> regcache_cooked_write_unsigned (regcache, S390_SYSTEM_CALL_REGNUM, 0);
> }
>
> +static void
> +s390_write_guessed_tracepoint_pc (struct regcache *regcache, CORE_ADDR pc)

Please add documentation for this function, as described here:

https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Document_Every_Subprogram

Note that this was not always done in the past; thus there are still
many undocumented functions in this file. But for new functions we
should stick to it.

> +{
> + struct gdbarch *gdbarch = get_regcache_arch (regcache);
> + struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
> + int sz = register_size (gdbarch, S390_PSWA_REGNUM);
> + gdb_byte *reg = (gdb_byte *) alloca (sz);
> +
> + /* 31-bit PSWA needs high bit set. */
> + if (tdep->abi == ABI_LINUX_S390)
> + pc |= 0x80000000;

This is done differently in s390_pseudo_register_write: the high bit is
copied from the original PSWA. Of course, this only makes a difference
if a program ever switches to 24-bit mode (yikes). I just wonder
whether both cases should be treated the same, or whether there's good
reason not to.

> +
> + store_unsigned_integer (reg, sz, gdbarch_byte_order (gdbarch), pc);
> + regcache_raw_supply (regcache, S390_PSWA_REGNUM, reg);
> +}
> +
>
> /* DWARF Register Mapping. */
>
> @@ -7857,6 +7873,7 @@ s390_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches)
> s390_iterate_over_regset_sections);
> set_gdbarch_cannot_store_register (gdbarch, s390_cannot_store_register);
> set_gdbarch_write_pc (gdbarch, s390_write_pc);
> + set_gdbarch_write_guessed_tracepoint_pc (gdbarch, s390_write_guessed_tracepoint_pc);
> set_gdbarch_pseudo_register_read (gdbarch, s390_pseudo_register_read);
> set_gdbarch_pseudo_register_write (gdbarch, s390_pseudo_register_write);
> set_tdesc_pseudo_register_name (gdbarch, s390_pseudo_register_name);