Re: PATCH: 6/6 [2nd try]: Add AVX support (gdbserver changes)

["H.J. Lu" <hjl.tools@gmail.com>]

On Sun, Mar 28, 2010 at 9:17 AM, Pedro Alves <pedro@codesourcery.com> wrote:
> On Sunday 28 March 2010 15:56:17, H.J. Lu wrote:
>> On Sun, Mar 28, 2010 at 12:55 AM, Pedro Alves <pedro@codesourcery.com> wrote:
>> > On Sunday 28 March 2010 02:11:31, H.J. Lu wrote:
>> >> > I guess you haven't tested this one :-)  You may want to add an AVX
>> >> > test to the testsuite, if it's not too much trouble.  You're checking
>> >> > for the "x86=xml" feature in the target, but only calling the target
>> >> > method for "x86:xstate=...".  I don't see how it could work.
>> >> >
>> >> > The problem we're solving by modifying qSupported is that older
>> >> > versions of GDB, which do not support XML registers at all, assume
>> >> > a specific layout for the g/G packet.  Newer versions, which do
>> >> > support XML, will use whatever the target supplies.  So, you only want
>> >> > the target to supply the registers via XML if GDB will understand
>> >> > them.  Is that accurate?
>> >>
>> >> Yes,
>> >>
>> >> > If that's the scope of the problem, then how about we handle
>> >> > this in a way we can reuse for other targets?  That doesn't have
>> >> > to change the implementation; just rename the feature to
>> >> > "xmlRegisters+".
>> >>
>> >> I will make the change.
>> >
>> > This (and the gdbarch_qsupported mechanism) worries me multi-arch
>> > design wise.  There's a bootstrapping problem here.  GDB sends qSupported
>> > to the target before knowing the target's target description.  The target
>> > sends the target description based on qSupported.
>> > As is, things only work correctly, when GDB already somehow knows the
>> > arch is some sort of x86 _before_ connecting to the target.  That's
>> > usually true if you give GDB a binary, but may not be true in some
>> > use cases.
>> >
>> > As a matter of example, if you have, say, a PPC --enable-targets=all
>> > GDB build, and you simply do:
>> >
>> >  $ gdb
>> >  (gdb) tar rem :9999
>> >
>> > to connect to a x86 linux gdbserver, then, the x86 target will not
>> > be sending the registers target description, because GDB wouldn't
>> > be sending the "x86=xml" feature (the target_gdbarch would be
>> > set to something not-x86 early in the connection, at the point
>> > gdbarch_qsupported it called).  With the "xmlRegisters+" change,
>> > it would be slightly even worse, as GDB would be sending a generic
>> > "xmlRegisters+", meaning "Hello target, I understand xml register
>> > descriptions for your arch", but, at a point when it may be
>> > mistaken what is the target's arch, and the target would
>> > have no way of knowing that.
>> >
>> > It seems to me that GDB should be sending "x86=xml" or something
>> > similar to the target unconditionally of whatever target_gdbarch is
>> > before having fetched the target description.
>> >
>>
>> I think current_target should be set to something sensible before
>> sending qSupported. It should match arch and OSABI of the executable.
>
> I can't agree with that.  That's against the goal of having the target
> fully self describe to GDB.  If that were true, then why would we
> support target descriptions that describe the OSABI?
> As I said and exampled above, you may not have a binary loaded in GDB
> at all.  A design that assumes you have, can't be correct in all
> supported cases.  GDB supports at least one x86 target that doesn't even
> have a notion of executables, only shared libraries --- DICOS.  I wouldn't
> want users of a non-x86 GDB build that supported that target to have
> to do "set architecture i386" or similar before connecting to be
> able to access the full register set as described by the target.
>
> What are your worries with doing something as I suggested?
>
> [To clear up confusions, this is about target_gdbarch, not
> current_target.  The current_target is always target
> remote / remote.c]
>

I guess it may be OK to always "xmlRegisters+" to gdb stub
and let each arch decide what to do.

One problem may be

1. We add XXX support to gdb 7.2.
2. We enable XML support for XXX in gdb 7.3.

What will happen when we run gdbserver
fom gdb 7.3 against gdb 7.2? gdb will always send
 "xmlRegisters+" to gdb stub which will send back
XML files. But gdb 7.2 doesn't support it.


-- 
H.J.