Re: [PATCH gnumach] Add x86_64 registers to i386_thread_state

[Samuel Thibault]

Hello,

Sergey Bugaev, le mar. 14 févr. 2023 14:03:22 +0300, a ecrit:
> On Sun, Feb 12, 2023 at 9:26 PM Flavio Cruz <flaviocruz@gmail.com> wrote:
> >
> > This is required to implement ptrace.
> > ---
> >  i386/i386/pcb.c                        | 42 +++++++++++++++++++++++++-
> >  i386/include/mach/i386/thread_status.h | 28 +++++++++++++++++
> >  2 files changed, 69 insertions(+), 1 deletion(-)
> >
> > diff --git a/i386/i386/pcb.c b/i386/i386/pcb.c
> > index 9ac55a1c..ba856523 100644
> > --- a/i386/i386/pcb.c
> > +++ b/i386/i386/pcb.c
> > @@ -500,6 +500,25 @@ kern_return_t thread_setstatus(
> >                 /*
> >                  * General registers
> >                  */
> > +#if defined(__x86_64__) && !defined(USER32)
> > +               saved_state->r8 = state->r8;
> > +               saved_state->r9 = state->r9;
> > +               saved_state->r10 = state->r10;
> > +               saved_state->r11 = state->r11;
> > +               saved_state->r12 = state->r12;
> > +               saved_state->r13 = state->r13;
> > +               saved_state->r14 = state->r14;
> > +               saved_state->r15 = state->r15;
> > +               saved_state->edi = state->rdi;
> > +               saved_state->esi = state->rsi;
> > +               saved_state->ebp = state->rbp;
> > +               saved_state->uesp = state->ursp;
> > +               saved_state->ebx = state->rbx;
> > +               saved_state->edx = state->rdx;
> > +               saved_state->ecx = state->rcx;
> > +               saved_state->eax = state->rax;
> > +               saved_state->eip = state->rip;
> 
> Do I understand correctly that the names of the i386_saved_state
> registers are not updated for x86_64 because it's an internal header,
> whereas i386_thread_state is public and it has to look right?

For an internal header we can do whatever we prefer indeed, while for a
public header we better use proper names.

> Still, I would find it cleaner / less surprising if both structures
> were updated the same way.
> 
> > +#if defined(__x86_64__) && !defined(USER32)
> > +       uint64_t        efl;
> 
> Should this not be 'rfl' (for 'rflags')?

In both cases, that would make the gnumach code have to distinguish the
two names, in the debugger and various other places.

Samuel