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

[Pedro Alves <pedro at codesourcery dot com>]

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]

-- 
Pedro Alves