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

Mark Kettenis
2004-02-28T17:35:13Z

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.

> 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.

Which is in dire need of a proper maintainer. The fact that I'm still
listed as threads maintainer doesn't mean that I've got any interest
in the crappy GNU/Linux threads implementation and the support for it
in GDB. I'm inclined to rename thread-db.c into linux-thread-db.c and
let it rot.

That said, I really think the problem lies in libthread-db and/or
GDB's support code for libthread-db, and that it should be solved
there, not in the generic AMD64 native support code that's also used
on other platforms.

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.

> 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.

Mark