[PATCH RFA]: Fix i386-linux-nat.c fill_gregset()

[PATCH RFA]: Fix i386-linux-nat.c fill_gregset()

[Kevin Buettner]

The patch below fixes a number of linux-dp failures that seem to
have crept in recently.

The following diagram (inverted stack trace) may be helpful in
understanding the discussion below:

    store_regs () ->
    fill_gregset () ->
    read_register_gen () ->
    legacy_read_register_gen () ->
    fetch_register () ->
    target_fetch_registers () which is thread_db_fetch_registers () ->
    lin_lwp_fetch_registers() ->
    fetch_regs () ->
    ptrace (PTRACE_GETREGS,...) & supply_gregset ()

On Linux/x86, store_regs() is supposed to write some of the
contents of GDB's regcache to the inferior process via ptrace().
It does this by first calling ptrace(PTRACE_GETREGS,...) to fetch
the process's registers.  Next it invokes fill_gregset() to copy
certain register(s) from GDB's regcache into the set of registers
just retrieved from the process.  Finally, it writes these registers
to the inferior via the PTRACE_SETREGS operation.

The fill_gregset() operation must not modify GDB's regcache.  It
should only change the contents of the gregset_t that it's asked
to fill using the contents of GDB's regcache.

However, this is not what's happening.  As noted in the diagram
above, the current version of Linux/x86 fill_gregset() is
invoking read_register_gen() which eventually winds up invoking
fetch_regs().  fetch_regs() calls ptrace() and supply_gregset()
which modifies GDB's regcache.  (Which is not what we want.)

The solution to this problem is to not call read_register_gen() from
fill_gregset().  Instead, we must fetch the regcache contents using
a more direct means.  That's what the patch below does.  (And, in
fact, it uses the same mechanism that's used to fetch the rest of
the registers in the gregset.)

Okay to commit?

	* i386-linux-nat.c (fill_gregset): Don't invoke read_register_gen()
	when fetching ORIG_EAX.

Index: i386-linux-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-linux-nat.c,v
retrieving revision 1.29
diff -u -p -r1.29 i386-linux-nat.c
--- i386-linux-nat.c	2001/11/04 14:30:42	1.29
+++ i386-linux-nat.c	2001/11/17 09:14:41
@@ -325,7 +325,8 @@ fill_gregset (elf_gregset_t *gregsetp, i
       *(regp + regmap[i]) = *(elf_greg_t *) &registers[REGISTER_BYTE (i)];
 
   if (regno == -1 || regno == I386_LINUX_ORIG_EAX_REGNUM)
-    read_register_gen (I386_LINUX_ORIG_EAX_REGNUM, (char *) (regp + ORIG_EAX));
+    *(regp + regmap[ORIG_EAX]) =
+      *(elf_greg_t *) &registers[REGISTER_BYTE (I386_LINUX_ORIG_EAX_REGNUM)];
 }
 
 #ifdef HAVE_PTRACE_GETREGS

Re: [PATCH RFA]: Fix i386-linux-nat.c fill_gregset()

[Mark Kettenis]

Kevin Buettner <kevinb@cygnus.com> writes:

> The patch below fixes a number of linux-dp failures that seem to
> have crept in recently.

I wonder why I didn't see those failures...

Anyway, your reasoning seems sound to me so...

> Okay to commit?

... go ahead.

Thanks,

Mark

Re: [PATCH RFA]: Fix i386-linux-nat.c fill_gregset()

[Andrew Cagney]

> Okay to commit?
> 
> 	* i386-linux-nat.c (fill_gregset): Don't invoke read_register_gen()
> 	when fetching ORIG_EAX.
> 
> Index: i386-linux-nat.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/i386-linux-nat.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 i386-linux-nat.c
> --- i386-linux-nat.c	2001/11/04 14:30:42	1.29
> +++ i386-linux-nat.c	2001/11/17 09:14:41
> @@ -325,7 +325,8 @@ fill_gregset (elf_gregset_t *gregsetp, i
>        *(regp + regmap[i]) = *(elf_greg_t *) &registers[REGISTER_BYTE (i)];
>  
>    if (regno == -1 || regno == I386_LINUX_ORIG_EAX_REGNUM)
> -    read_register_gen (I386_LINUX_ORIG_EAX_REGNUM, (char *) (regp + ORIG_EAX));
> +    *(regp + regmap[ORIG_EAX]) =
> +      *(elf_greg_t *) &registers[REGISTER_BYTE (I386_LINUX_ORIG_EAX_REGNUM)];
>  }
>  
>  #ifdef HAVE_PTRACE_GETREGS

FYI,

Using registers[] is wrong.  The code should be using regcache_collect().

Andrew

Re: [PATCH RFA]: Fix i386-linux-nat.c fill_gregset()

[Kevin Buettner]

On Nov 17,  3:17am, Kevin Buettner wrote:

> 	* i386-linux-nat.c (fill_gregset): Don't invoke read_register_gen()
> 	when fetching ORIG_EAX.
[...]
> -    read_register_gen (I386_LINUX_ORIG_EAX_REGNUM, (char *) (regp + ORIG_EAX));
> +    *(regp + regmap[ORIG_EAX]) =
> +      *(elf_greg_t *) &registers[REGISTER_BYTE (I386_LINUX_ORIG_EAX_REGNUM)];

Committed.  (Thanks to Mark K. for the quick approval.)

I'm going to convert fill_gregset() to use regcache_collect() as a
separate step.  I'm testing it now and will commit it as "obvious"
assuming my testing goes okay...

Kevin