Differences between revisions 16 and 37 (spanning 21 versions)
Revision 16 as of 2013-07-31 11:53:22
Size: 5694
Editor: PedroAlves
Comment: update Build/Configure section
Revision 37 as of 2016-01-16 14:43:53
Size: 8828
Editor: PedroAlves
Comment: exceptions/cleanups are shared nowadays
Deletions are marked like this. Additions are marked like this.
Line 2: Line 2:
= Common part of GDB and GDBserver = = Common parts of GDB and GDBserver (a.k.a., The Stop the Duplication Madness! project) =
Line 5: Line 5:
This page describes the work to move duplicate code of GDB and GDBserver to files in the gdb/common/ directory. This page describes the work to eliminate duplicate code of GDB and GDBserver.
Line 8: Line 8:
There's a lot of code duplication between GDB and GDBserver. The project has been taking baby steps in reducing such duplication, by putting shared code in the gdb/common/ directory. The goal of this project is to reduce the duplication as much as possible, and to signify the contents of the gdb/common/ directory. There's a lot of code duplication between GDB and GDBserver. The project has been taking baby steps in reducing such duplication, by refactoring things, converging gdb and gdbserver's designs, and putting shared code in the shared directories. The goal of this project is to reduce the duplication as much as possible.

== Where to put shared code? ==

See these past discussions:

 [[https://sourceware.org/ml/gdb-patches/2012-11/msg00204.html]]

 [[https://sourceware.org/ml/gdb-patches/2013-07/msg00840.html]]

 [[https://sourceware.org/ml/gdb-patches/2013-08/msg00038.html]]

 [[https://sourceware.org/ml/gdb-patches/2014-06/msg00685.html]]

(there were more)


The "common" moniker for a directory name isn't that great. It is a bit artificial and an artifact. It tells us that the code is used by more than one something,
but it doesn't tell us really what's inside. It's not much different from saying the gdb/ is the new common and allowing gdb/gdbserver/ to refer to files under gdb/ . In the extreme, we could end up with half of gdb under common/ in a few years.

There are really two kinds of "common" stuff. There's the native target backends (ptrace and friends), but there's also the host side common stuff. If we stripped both gdb and gdbserver of their native target backends (e.g, in gdbserver, you end up with "main()", command line option processing, the event loop, the RSP marshalling, etc.., we'll still find things that are common between the two programs. The event loop is an obvious example. So we should take the opportunity to a establish a more meaningful directory structure.

So with that in mind, these are the subdirectories we're moving stuff to currently:

|| gdb/nat/ || native target backend files. Code that interfaces with the host debug API. E.g., ptrace code, Windows debug API code, procfs code should go here.||
|| gdb/target/ || Host-independent, target vector specific code (target_ops). Currently GDB's and GDBserver's target_ops vectors are different, though ideally they'd converge. ||
|| gdb/common/ || All other shared code. ||


== Header files in common code (defs.h vs server.h, etc.) ==

 * GDB has a defs.h file that includes declarations and definitions that end up necessary throughout most of GDB. This includes both core types and utility functions / macros, and GDB-the-application specific bits (that are of no interest to base/common code).

 * GDBserver has a server.h file that includes declarations and definitions that end up necessary throughout most of GDBserver. This includes both core types and utility functions / macros, and also GDBserver-the-application specific bits (that are of no interest to base/common code).

There's some obvious duplication above.

The current plan is to factor out the core/base/common code to a common/common-defs.h file that declares/defines the core types and both utility functions / macros, and then both gdb's defs.h and GDBserver's server.h includes that new file as the first line. This new file will also include the two config.h files (the application's and gnulib's).

Then .c files in the shared directories include common-defs.h as the first line, files in gdb will include defs.h and files in gdbserver include server.h as the first line.

In end the number of "#ifdef GDBSERVER"s in the shared directories codebase will be precisely one (to select the appropriate gnulib config.h in common-defs.h). We'll consider eliminating that one at a later stage.

See these past discussions:

 [[https://sourceware.org/ml/gdb-patches/2014-07/msg00656.html]]
Line 12: Line 57:
  * Target backends. These are the biggest duplication offenders. That is, the gdb/*-nat.c files and the gdb/gdbserver/*-low.c files accomplish pretty much the same. (Some targets do things differently enough, that they're not really duplication of code, but can be considered different implementations. The Linux support is one such case. Over the years we've been converging them though -- see the [[LocalRemoteFeatureParity]] page for more details.). GDBserver supports fewer target OSs than GDB does. Ideally, we'd finish the [[LocalRemoteFeatureParity]] project first, and then just dump the GDB-side backends. However, there are bits of the backends, most prominently, the arch specific bits, that can be shared between the implementations before then, as the interfaces are similar enough. E.g., debug registers/watchpoint support; code that detects which variant of a processor the program is running (the xml target description to send to GDB core), etc.. Sharing such arch specific code significantly reduces the effort for new ports (those usually don't need to touch common code). As is today most ports involve doing the same work twice. We should clean up some of the existing ports, laying grounds for good examples for new ports. Presently, there's overlap/duplication in: === Target backends ===

These are the biggest duplication offenders. That is, the gdb/*-nat.c files and the gdb/gdbserver/*-low.c files accomplish pretty much the same. (Some targets do things differently enough, that they're not really duplication of code, but can be considered different implementations. The Linux support is one such case. Over the years we've been converging them though -- see the [[LocalRemoteFeatureParity]] page for more details.). GDBserver supports fewer target OSs than GDB does. Ideally, we'd finish the [[LocalRemoteFeatureParity]] project first, and then just dump the GDB-side backends. Presently, there's overlap/duplication in:
Line 36: Line 83:
  * Terminal handling === Arch-specific bits of the target backends ===
Line 38: Line 85:
     || GDB || GDBserver || comments ||
     || gdb/terminal.h || gdb/gdbserver/terminal.h || (parts) ||
Independently of the local/remote parity project, however, there are bits of the backends, most prominently, the arch specific bits, that can be shared between the target backend implementations before then, as the interfaces are similar enough. E.g., code accessing hardware registers, debug registers/watchpoint support; code that detects which variant of a processor the program is running (the xml target description to send to GDB core), etc.. Sharing such arch specific code significantly reduces the effort for new ports (those usually don't need to touch common code). As is today most ports involve doing the same work twice. We should clean up some of the existing ports, laying grounds for good examples for new ports.
Line 41: Line 87:
  * Event loop machinery Move these duplicated part to nat/ directory, and come up with logically reasonable file names.
Line 43: Line 89:
     || GDB || GDBserver || comments ||
     || gdb/event-loop.c || gdb/gdbserver/event-loop.c || ||
--(For example, the code for accessing x86 debug registers is duplicated in gdb/gdbserver/i386-low.c and gdb/i386-nat.c, so it can be moved to gdb/nat/ directory with file name i386-dregs.c and i386-dregs.h. See [[https://sourceware.org/ml/gdb-patches/2014-06/msg00586.html| Refactor shared code in i386-{nat,low}.[ch]]].)-- Done in 7.9.
Line 46: Line 91:
  * Configury stuff. === Terminal handling ===
Line 48: Line 93:
     || GDB || GDBserver || comments ||
     || gdb/configure.ac, etc. || gdb/gdbserver/configure.ac, etc. || ||
Parts of terminal handling are duplicated.
Line 51: Line 95:
== Plan ==
Here is something like a plan on what we can do on this project,
|| GDB || GDBserver || comments ||
|| gdb/terminal.h || gdb/gdbserver/terminal.h || (parts) ||
Line 54: Line 98:
  * Move duplicated code among *-nat.c and *-low.c. Some macros, and code accessing hardware registers should be the same on both GDB and GDBserver. No reason to keep the same code in two copies respectively. Move these duplicated part to common/ directory, and give a logically reasonable file name. For example, the code for accessing debug registers are duplicated in i386-low.c and i386-nat.c, so they can be moved to common directory with file name i386-dbg-reg.c and i386-rdbg-reg.h. See [[http://sourceware.org/ml/gdb-patches/2011-03/msg00648.html| Move common macros to i386-dbg-reg.h]]. There is no automatic way to identify the duplications between *-nat.c and *-low.c except examining code manually.
  * Move duplicated code in gdb/linux-thread-db.c and gdb/gdbserver/thread-db.c. There should be some duplications, but still no idea on how to, due to lack of knowledge on these two files. As part of this piece of work, part of gdb_proc_serivce.h in GDB and GDBserver can be moved to common dir, and leave its own definition of 'struct ps_prochandle'.
  * Part of terminal.h is duplicated as well, and we can move them to common/ dir.
=== Event loop machinery ===

|| GDB || GDBserver || comments ||
|| gdb/event-loop.c || gdb/gdbserver/event-loop.c || ||

=== Build/Configure machinery ===

|| GDB || GDBserver || comments ||
|| gdb/configure.ac, etc. || gdb/gdbserver/configure.ac, etc. || Largely done ||

The current approach is to create Makefile fragments and m4 files in common/, which are then sourced by gdb and gdbserver's build machineries. See [[http://sourceware.org/ml/gdb-patches/2013-04/msg00739.html|this thread (RFC: introduce common.m4)]].

Eventually we plan to move gnulib, the common code, and gdbserver to the top-level. Then the configury becomes straightforward.

Note that at one point, common/ had its own configure. See the thread [[http://sourceware.org/ml/gdb-patches/2011-02/msg00209.html|here]]. The patches went into CVS, but were later reverted, because of various issues they caused.

=== Remote serial protocol ===

|| GDB || GDBserver || comments ||
|| gdb/remote.c || gdb/gdbserver/remote-utils.c || --(Factor out tohex/fromhex/getpkt/putpkt and more into rsp.c/h)-- ||

=== Target stack ===

|| GDB || GDBserver || comments ||
|| gdb/target.[ch] || gdb/gdbserver/target.[ch] || Merging the entire target stack is a long-term goal. ||
Line 62: Line 128:
  * Avoid "common" in file names in files under common/. Instead, name the files for what they contain, not for the fact that they're "common" to two programs (gdb, gdbserver) presently. The preferred direction is, when moving things to common/, take the opportunity to split them into smaller, more atomic, leaner units. E.g., that's how we ended up with ptid.h/ptid.c, instead of inferior-common.h (or some such). If the resulting file is just a dumping ground of miscellaneous things, then call it that. Say, foo-misc.h or foo-defs.h, not foo-common.h.   * Avoid "common" in file names in files under common/. Instead, name the files for what they contain, not for the fact that they're "common" to two programs (gdb, gdbserver) presently. If the resulting file is just a dumping ground of miscellaneous things, then call it that. Say, foo-misc.h or foo-defs.h, not foo-common.h. But ... (see below)
Line 64: Line 130:
== Build/Configure == === Split files if possible ===
Line 66: Line 132:
So far, we have three possible approaches:

  1. Don't have its own build stuff in common/, let gdb or gdbserver pick the pieces they want, and duplicate the necessary autoconf checks, etc.. This is the status quo.
  2. Make common/ have its own configure. In March 2011, Yao gave a try to build common/ as a library (libcommon.a), for both GDB and GDBserver to link. See the thread [[http://sourceware.org/ml/gdb-patches/2011-02/msg00209.html|here]]. The patches went into CVS, but were later reverted, because of various issues they caused.
  3. Create Makefile fragments and m4 files in common/, then sourced by gdb and gdbserver's build machineries. No new configure. This is the direction we're heading towards now. See [[http://sourceware.org/ml/gdb-patches/2013-04/msg00739.html|this thread (RFC: introduce common.m4)]].
When moving things to common/, take the opportunity to split them into smaller, more logically atomic, leaner units. E.g., that's how we ended up with ptid.h/ptid.c, instead of inferior-common.h (or some such). Avoid new kitchen sinks! Combining movement and refactoring is bad, but this is just movement: putting code into separate smaller new files is really no different from stuffing it all into one big new file.
Line 75: Line 137:

LocalRemoteFeatureParity

Common parts of GDB and GDBserver (a.k.a., The Stop the Duplication Madness! project)

This page describes the work to eliminate duplicate code of GDB and GDBserver.

1. Goal

There's a lot of code duplication between GDB and GDBserver. The project has been taking baby steps in reducing such duplication, by refactoring things, converging gdb and gdbserver's designs, and putting shared code in the shared directories. The goal of this project is to reduce the duplication as much as possible.

2. Where to put shared code?

See these past discussions:

(there were more)

The "common" moniker for a directory name isn't that great. It is a bit artificial and an artifact. It tells us that the code is used by more than one something, but it doesn't tell us really what's inside. It's not much different from saying the gdb/ is the new common and allowing gdb/gdbserver/ to refer to files under gdb/ . In the extreme, we could end up with half of gdb under common/ in a few years.

There are really two kinds of "common" stuff. There's the native target backends (ptrace and friends), but there's also the host side common stuff. If we stripped both gdb and gdbserver of their native target backends (e.g, in gdbserver, you end up with "main()", command line option processing, the event loop, the RSP marshalling, etc.., we'll still find things that are common between the two programs. The event loop is an obvious example. So we should take the opportunity to a establish a more meaningful directory structure.

So with that in mind, these are the subdirectories we're moving stuff to currently:

gdb/nat/

native target backend files. Code that interfaces with the host debug API. E.g., ptrace code, Windows debug API code, procfs code should go here.

gdb/target/

Host-independent, target vector specific code (target_ops). Currently GDB's and GDBserver's target_ops vectors are different, though ideally they'd converge.

gdb/common/

All other shared code.

3. Header files in common code (defs.h vs server.h, etc.)

  • GDB has a defs.h file that includes declarations and definitions that end up necessary throughout most of GDB. This includes both core types and utility functions / macros, and GDB-the-application specific bits (that are of no interest to base/common code).
  • GDBserver has a server.h file that includes declarations and definitions that end up necessary throughout most of GDBserver. This includes both core types and utility functions / macros, and also GDBserver-the-application specific bits (that are of no interest to base/common code).

There's some obvious duplication above.

The current plan is to factor out the core/base/common code to a common/common-defs.h file that declares/defines the core types and both utility functions / macros, and then both gdb's defs.h and GDBserver's server.h includes that new file as the first line. This new file will also include the two config.h files (the application's and gnulib's).

Then .c files in the shared directories include common-defs.h as the first line, files in gdb will include defs.h and files in gdbserver include server.h as the first line.

In end the number of "#ifdef GDBSERVER"s in the shared directories codebase will be precisely one (to select the appropriate gnulib config.h in common-defs.h). We'll consider eliminating that one at a later stage.

See these past discussions:

4. Where's the duplication then?

4.1. Target backends

These are the biggest duplication offenders. That is, the gdb/*-nat.c files and the gdb/gdbserver/*-low.c files accomplish pretty much the same. (Some targets do things differently enough, that they're not really duplication of code, but can be considered different implementations. The Linux support is one such case. Over the years we've been converging them though -- see the LocalRemoteFeatureParity page for more details.). GDBserver supports fewer target OSs than GDB does. Ideally, we'd finish the LocalRemoteFeatureParity project first, and then just dump the GDB-side backends. Presently, there's overlap/duplication in:

  • Linux
    • See the LocalRemoteFeatureParity page for more details.

      GDB

      GDBserver

      gdb/linux-nat.c

      gdb/gdbserver/linux-low.c

      gdb/proc-service.c

      gdb/gdbserver/proc-service.c

      gdb/linux-thread-db.c

      gdb/gdbserver/thread-db.c

      ... and related arch specific files ...

  • Windows
    • GDB

      GDBserver

      gdb/windows-nat.c

      gdb/gdbserver/win32-low.c

      ... and related arch specific files ...

  • NTO
    • GDB

      GDBserver

      gdb/nto-procfs.c

      gdb/gdbserver/nto-low.c

      ... and related arch specific files ...

4.2. Arch-specific bits of the target backends

Independently of the local/remote parity project, however, there are bits of the backends, most prominently, the arch specific bits, that can be shared between the target backend implementations before then, as the interfaces are similar enough. E.g., code accessing hardware registers, debug registers/watchpoint support; code that detects which variant of a processor the program is running (the xml target description to send to GDB core), etc.. Sharing such arch specific code significantly reduces the effort for new ports (those usually don't need to touch common code). As is today most ports involve doing the same work twice. We should clean up some of the existing ports, laying grounds for good examples for new ports.

Move these duplicated part to nat/ directory, and come up with logically reasonable file names.

For example, the code for accessing x86 debug registers is duplicated in gdb/gdbserver/i386-low.c and gdb/i386-nat.c, so it can be moved to gdb/nat/ directory with file name i386-dregs.c and i386-dregs.h. See Refactor shared code in i386-{nat,low}.[ch]. Done in 7.9.

4.3. Terminal handling

Parts of terminal handling are duplicated.

GDB

GDBserver

comments

gdb/terminal.h

gdb/gdbserver/terminal.h

(parts)

4.4. Event loop machinery

GDB

GDBserver

comments

gdb/event-loop.c

gdb/gdbserver/event-loop.c

4.5. Build/Configure machinery

GDB

GDBserver

comments

gdb/configure.ac, etc.

gdb/gdbserver/configure.ac, etc.

Largely done

The current approach is to create Makefile fragments and m4 files in common/, which are then sourced by gdb and gdbserver's build machineries. See this thread (RFC: introduce common.m4).

Eventually we plan to move gnulib, the common code, and gdbserver to the top-level. Then the configury becomes straightforward.

Note that at one point, common/ had its own configure. See the thread here. The patches went into CVS, but were later reverted, because of various issues they caused.

4.6. Remote serial protocol

GDB

GDBserver

comments

gdb/remote.c

gdb/gdbserver/remote-utils.c

Factor out tohex/fromhex/getpkt/putpkt and more into rsp.c/h

4.7. Target stack

GDB

GDBserver

comments

gdb/target.[ch]

gdb/gdbserver/target.[ch]

Merging the entire target stack is a long-term goal.

5. Guidelines

5.1. File Naming

  • Avoid "common" in file names in files under common/. Instead, name the files for what they contain, not for the fact that they're "common" to two programs (gdb, gdbserver) presently. If the resulting file is just a dumping ground of miscellaneous things, then call it that. Say, foo-misc.h or foo-defs.h, not foo-common.h. But ... (see below)

5.2. Split files if possible

When moving things to common/, take the opportunity to split them into smaller, more logically atomic, leaner units. E.g., that's how we ended up with ptid.h/ptid.c, instead of inferior-common.h (or some such). Avoid new kitchen sinks! Combining movement and refactoring is bad, but this is just movement: putting code into separate smaller new files is really no different from stuffing it all into one big new file.


OngoingWork

LocalRemoteFeatureParity

None: Common (last edited 2016-01-16 14:43:53 by PedroAlves)

All content (C) 2008 Free Software Foundation. For terms of use, redistribution, and modification, please see the WikiLicense page.