This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


   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



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]