Re: [RFC] hppa/multiarch - PUSH_DUMMY_FRAME depends on inf_status

[Andrew Cagney <ac131313@redhat.com>]

> tm-hppa.h contains the following macro definition for PUSH_DUMMY_FRAME:
> 
>     /* FIXME: brobecker 2002-12-26.  This macro definition takes advantage
>        of the fact that PUSH_DUMMY_FRAME is called within a function where
>        a variable inf_status of type struct inferior_status * is defined.
>        Ugh!  Until this is fixed, we will not be able to move to multiarch
>        partial.  */
>     #define PUSH_DUMMY_FRAME hppa_push_dummy_frame (inf_status)
>     extern void hppa_push_dummy_frame (struct inferior_status *);
> 
> As said in the comment, there is this nasty dependency of the fact that
> there is a variable called inf_status in the scope where this macro is
> "invoked". This dependency is getting in the way of the multiarch conversion,
> so I'd like to remove it.
> 
> I looked at the only place where PUSH_DUMMY_FRAME is used, and found
> the following code (function hand_function_call in valops.c):
> 
>     /* A cleanup for the inferior status.  Create this AFTER the retbuf
>        so that this can be discarded or applied without interfering with
>        the regbuf.  */
>     inf_status = save_inferior_status (1);
>     inf_status_cleanup = make_cleanup_restore_inferior_status (inf_status);

[tangent]

This makes a copy of the inferior's registers.

Hmm, this means that there are potentially two copies of the system 
registers - inf_status and dummy-frame's (but HP doesn't yet use dummy 
frames).

In fact, this means that all targets, even the ones that don't use 
generic dummy frames, are restoring their registers from a regcache and 
_not_ from the the register values pushed onto the stack!

>     /* PUSH_DUMMY_FRAME is responsible for saving the inferior registers
>        (and POP_FRAME for restoring them).  (At least on most machines)
>        they are saved on the stack in the inferior.  */
>     PUSH_DUMMY_FRAME;
> 
> So inf_status is obtained by a call to save_inferior_status. OK.
> 
> HP's push_dummy_frame uses it exclusively to hack in some of the
> registers.  A comment explains why this is necessary:
> 
>   /* Oh, what a hack.  If we're trying to perform an inferior call
>      while the inferior is asleep, we have to make sure to clear
>      the "in system call" bit in the flag register (the call will
>      start after the syscall returns, so we're no longer in the system
>      call!)  This state is kept in "inf_status", change it there.
>   
>      We also need a number of horrid hacks to deal with lossage in the
>      PC queue registers (apparently they're not valid when the in syscall
>      bit is set).  */


> They do it like this:
> 
>       write_inferior_status_register (inf_status, 0, int_buffer);
>       write_inferior_status_register (inf_status, PCOQ_HEAD_REGNUM, pc + 0);
>       write_inferior_status_register (inf_status, PCOQ_TAIL_REGNUM, pc + 4);

Joel,

I think I'm missing something pretty fundamental here.  This shouldn't 
work.  I can't figure out why it doesn't end up trying to do a write to 
a read-only copy of current_regcache?

- inf_status->registers is created using regcache_dup (current_regcache)
gdb_assert (inf_status->registers != current_registers) true?
gdb_assert (inf_status->registers->readonly_p) true?

- inf_status->registers->regcache_raw_write() is calling (you've got 
legacy code :-) legacy_write_register_gen(), but first it checks:
gdb_assert (!regcache->readonly_p);
gdb_assert (regcache == current_regcache);

- legacy_write_register_gen() is then both writing the value into the 
current_regcache and direct to the target.

i.e., those writes are hitting the target directly.

Any idea what I'm missing?

confused,
Andrew