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

H.J. Lu
2010-03-28T14:56:24Z

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.

--
H.J.