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

[Marcin KoÅcielnicki <koriakin at 0x04 dot net>]

On 26/01/16 19:12, Andreas Arnez wrote:
> 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.

OK, will do.
> > +{
> > +  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.

If write_guessed_tracepoint_pc is called, we explicitely have no 
registers data other than PC itself - there's no original PSWA to speak 
of.  The state of PSWA high bit probably doesn't matter all that much, 
but since we want to mark PSWA as known, we'd better write the 
reasonable value to it.
> > +
> > +  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);