Re: [PATCH 1/2] Port gdbserver to GNU/Hurd

[Pedro Alves <palves@redhat.com>]

On 09/03/2013 10:38 AM, Thomas Schwinge wrote:
> Hi!
> 
> For context, Yue Lu is a student participating in this year's Google
> Summer of Code program, to port gdbserver to GNU Hurd, and is both a GDB
> as well as a GNU Hurd newbie.  (So, be gentle.)  ;-)
> 
> On Tue, 3 Sep 2013 16:00:32 +0800, Yue Lu <hacklu.newborn@gmail.com> wrote:
>> This is my patch to port gdbserver to GNU/Hurd. Most of code are
>> copied  from [gdb]/gdb/gnu-nat.c.
> 
> ... and elsewhere.  Our strategy was to first get this into a basic
> functional state:
> 
>> Now the gdbserver on GNU/Hurd can set breakpoint and check memory or
>> register(but without float-register support).
> 
> So, this initial port just posted is a great milestone!  Especially so
> given your previous lack of experience with both GDB and the Hurd -- both
> of which are not always easy to grasp.

Indeed!

> 
> There are lots of things to be polished (Yue, don't worry -- this
> entirely was to be expected), such as GDB coding standard, and changes
> that are unrelated to your port, which all has to be cleared out before I
> can commit this initial port to GDB upstream.
> 
> There is missing functionality, but we decided this can be enhanced piece
> by piece once the initial port is accepted.
> 
> There is the issue of code sharing between GDB proper and gdbserver, a
> strategy for which has been briefly discussed in
> <http://news.gmane.org/find-root.php?message_id=%3CCAB8fV%3Djzv_rPHP3-HQVBA-pCNZNat6PNbh%2BOJEU7tZgQdKX3%2Bw%40mail.gmail.com%3E>.

That has been further discussed recently:

 https://sourceware.org/ml/gdb-patches/2013-07/msg00788.html

and we ended up with putting native target files under the
new gdb/nat/ directory, rather than in gdb/common/:

 https://sourceware.org/ml/gdb-patches/2013-08/msg00618.html

> Likewise for code sharing between the new Hurd gdbserver port and the
> existing x86 Linux kernel port.  Again I suggest this to be done *after*
> the initial port is accepted: this will turn into a nice (and easily
> reviewable) series of cleanup patches à la: remove from GDB proper
> gdb/gnu-nat.c:[function] and from gdbserver
> gdb/gdbserver/gnu-low.c:[function], and add
> gdb/common/gnu-low.c:[function], and so on.  Likewise for build
> infrastructure that can be shared.
> 
> Does this strategy generally make sense to you GDB maintainers?

I've been thinking about this this morning, after seeing these
patches.

For new gdbserver ports, this path just seems to swim further away from
a full sharing approach, by adding lots duplication as first step, and
actually making it hard to see what exactly needed to be changed/adapted
for gdbserver use, and puts the tree in a state where any further changes
for the GNU/Hurd target will need to be considered twice going further,
exactly what we're fighting against with the existing ports.  I think
that strategy ultimately is slower to get at where we want to, and
is actually more work than an alternative that does things the other
way around.

I checked out Yue's git branch, and diffed gdb/gnu-nat.c vs
gdbserver/gnu-low.c, and gdb/i386gnu-nat.c vs gdbserver/gnu-i386-low.c.
I didn't diff the rest of the files, as I assume they'll probably have
even less divergence.  There are several spurious formatting differences,
and some reordering of functions, but for the most port, the code is
mostly identical.  There's some expected necessary adjustment to GDBserver's
interfaces, but it turns out it's not that much.  We've been converging
gdb's and gdbserver's interfaces throughout the years, and it now shows.

So my idea would be, instead of adding the new files under gdbserver,
to remove the spurious differences (formatting, reordering, etc.) that
were introduced in the gdbserver copies of the files, eliminating the
unnecessary divergence, and then fold back the resulting differences into
the original gdb/gnu-nat.c etc. files, guarded by #ifdef GDBSERVER.  Some
cleanups might have been identified and done in the gdbserver files, and
it might make sense to do those as preparatory work, in the original files.
This should result in smaller patches, and will actually avoid
the need for most of the polishing Thomas mentioned, and as consequence
review burden -- reviewing the new gnu-low.c etc., for GNU conventions,
formatting, or even appropriate use of the Hurd's debug APIs etc., is
just unnecessary that way, by design, and we'll be able to focus on the
bits that are the real new code -- the glue between the target and gdb, and
the target and gdbserver.

The current state of the work isn't wasted at all!  And I don't
think following this direction is that much work.  I'd do this my
literally moving gdbserver/gnu-low.c on top of gdb/gnu-nat.c (etc.), and
use git diff to guide me through, in identifying what would need to
be restored, and guarded with #if[n]def GDBSERVER.  #ifdef GDBSERVER
is how we've adapting shared code under gdb/common/ and gdb/nat/
to work on both programs.  Medium/long term, core gdb and core
gdbserver target interface differences should converge further, and
the #ifdefs disappear, but for now that's a necessary evil.

It's fine to leave bits of functionality disabled on gdbserver,
wrapped in #ifndef GDBSERVER.  After that initial work is committed,
we can then easily progress the gdbserver port by just looking for
those #ifdefs.

It's fine with me to leave the existing native files under gdb/ while
doing this; it's probably easier that way.  Moving them under gdb/nat/
can be left for a cleanup step further down the road.

Could we try that approach?

-- 
Pedro Alves