Re: [rfa/amd64] Zero fill 32-bit registers

[Andrew Cagney <cagney@gnu.org>]

>    Date: Sat, 28 Feb 2004 10:17:12 -0500
>    From: Andrew Cagney <cagney@gnu.org>
> 
>    >    Date: Thu, 26 Feb 2004 20:22:11 -0500
>    >    From: Andrew Cagney <ac131313@redhat.com>
>    > 
>    >    Hello,
>    > 
>    >    For a 64-bit gregset, the code was only modifying the low 32-bits of the 
>    >    register field - leaving the upper 64-bits undefined.
>    > 
>    > That's not completely unintentional.  The idea is to leave any
>    > "reserved" bits untouched, and in a sense for 32-bit stuff the upper
>    > 32 bits are "reserved"; they are not necessarily zero, at least not
>    > for all registers.
> 
>    Er, the upper 32-bits here aren't reserved.  The request was for a 
>    64-bit register, and this code is erreneously only supplying half that 
>    value - that leaves the upper 32-bits undefined.
> 
>    We've hit the same problem in the past with the MIPS.  When only 32-bits 
>    were available the value was expanded (in accordance with the ISA) to 
>    the full 64-bits.
> 
> I don't know enough about MIPS to be sure, but I really think AMD64 is
> different; the ISA doesn't magically extend 32-bit values to 64-bit
> values.

(That's why I zero extended).

The problem with MIPS is that the code, instead of always being locally 
consistent, assume that problems like this were handled elsewhere - they 
never were or were not handled consistently.  Only when MIPS started 
being locally consistent (and the mantra that addresses shall always be 
sign extended) did the code become robust, and the bugs disappear.  It 
became possible to mix 'n' match 32-bit and 64-bit code.

Here amd64 has the same problem - all 32x64-bit code needs to ensure 
that it is locally consistent.  If asked to supply a 64-bit register, 
supply the full 64-bits, and not hope something else is filling in the 
upper 32-bits.

>    > I guess the thread code isn't doing the equivalent of the PT_GETREGS
>    > call.  I think the correct way to fix this is to make sure the buffer
>    > is properly initialized before you pass it to
>    > amd64_collect_native_gregset.
> 
>    Don't look at me, the buffer originated in libthread-db.

> Short-term fix is probably to let fill_gregset() clear the register
> buffer before calling amd64_native_collect_gregset().
> store_inferior_registers() should call amd64_native_collect_gregset()
> then.

It doesn't address the underlying problem where fetching an individual 
register leaves half the register field undefined.   The code shouldn't 
assume that other code has magically initialized the rest of that 
register field.

>    > Another problem with your patch is that I'd rather like avoid assuming
>    > that the register buffer is an array of 8-byte registers.
> 
>    That code already assumems that, and that the values are little-endian.
> 
> Yes it assumes little-endianness, but the assumptions on the size of
> the slots in the register buffer are weaker.  The register buffer here
> corresponds to `struct reg' on the BSD's.  It would be prefectly well
> possible for some of its members to be 4 bytes in size.  The current
> code works for that case, whereas with your patch, it could thrash
> another member of the structure.

Do you know of any such an system?

Andrew