Change gdbserver to use existing gdbsupport

Message ID 20200219170643.10725-1-tom@tromey.com
State New
Headers show
Series
  • Change gdbserver to use existing gdbsupport
Related show

Commit Message

Tom Tromey Feb. 19, 2020, 5:06 p.m.
This changes the gdbserver build to use the gdbsupport that was built
for gdb.

The most notable change here is that CORE_ADDR is always a 64-bit
type.  This makes it so that gdb acts as if it were always built in
64-bit mode.

I tested this locally, and I tried it with a few crosses using
AdaCore's internal test suite (including a 32-bit build).  However, I
don't have a very clear idea of what code might break due to the
switch from a 32- to 64-bit CORE_ADDR.

ChangeLog
2020-02-19  Tom Tromey  <tom@tromey.com>

	* Makefile.in: Rebuild.
	* Makefile.def (gdbsupport): Don't depend on bfd.
	(gdbserver): Depend on gdbsupport.

gdbserver/ChangeLog
2020-02-19  Tom Tromey  <tom@tromey.com>

	* configure: Rebuild.
	* configure.ac (GDBSERVER_DEPFILES): Remove srv_selftest_objs.
	* Makefile.in (SFILES, OBS, TAGS, GDBREPLAY_OBS): Remove
	gdbsupport files.
	(gdbsupport/%.o): Remove target.
	(GDBSUPPORT_BUILDDIR, GDBSUPPORT): New variables.
	(gdbserver$(EXEEXT), gdbreplay$(EXEEXT)): Add GDBSUPPORT.

gdbsupport/ChangeLog
2020-02-19  Tom Tromey  <tom@tromey.com>

	* common-types.h: Remove GDBSERVER code.
	(gdb_byte, CORE_ADDR, LONGEST, ULONGEST): Redefine.
	* common-defs.h: Remove GDBSERVER code.
---
 ChangeLog                 |  6 +++
 Makefile.def              |  3 +-
 Makefile.in               |  4 +-
 gdbserver/ChangeLog       | 10 +++++
 gdbserver/Makefile.in     | 86 +++++----------------------------------
 gdbserver/configure       |  4 +-
 gdbserver/configure.ac    |  6 +--
 gdbsupport/ChangeLog      |  6 +++
 gdbsupport/common-defs.h  | 16 --------
 gdbsupport/common-types.h | 32 +++------------
 10 files changed, 43 insertions(+), 130 deletions(-)

-- 
2.17.2

Comments

Pedro Alves Feb. 19, 2020, 5:42 p.m. | #1
On 2/19/20 5:06 PM, Tom Tromey wrote:
> This changes the gdbserver build to use the gdbsupport that was built

> for gdb.

> 

> The most notable change here is that CORE_ADDR is always a 64-bit

> type.  This makes it so that gdb acts as if it were always built in

> 64-bit mode.

> 

> I tested this locally, and I tried it with a few crosses using

> AdaCore's internal test suite (including a 32-bit build).  However, I

> don't have a very clear idea of what code might break due to the

> switch from a 32- to 64-bit CORE_ADDR.


I'm surprised you didn't hit any problem, because last time we chatted
about it, I quickly tried it, and ran into a small number of places
that needed adjustment.

I just tried it again, and I get, for example, with
a i686-pc-linux-gnu build:

$ make V=1 remote.o
g++ -m32 -x c++  -g3 -O0 (....) -c -o remote.o -MT remote.o -MMD -MP -MF ./.deps/remote.Tpo /home/pedro/gdb/binutils-gdb/src/gdb/remote.c
In file included from /home/pedro/gdb/binutils-gdb/src/gdb/defs.h:37:0,
                 from /home/pedro/gdb/binutils-gdb/src/gdb/remote.c:22:
/home/pedro/gdb/binutils-gdb/src/gdb/remote.c: In member function ‘virtual void remote_target::download_tracepoint(bp_location*)’:
../bfd/bfd.h:198:62: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘CORE_ADDR {aka long long unsigned int}’ [-Werror=format=]
 #define sprintf_vma(s,x) sprintf (s, "%08" BFD_VMA_FMT "x", x)
                                                              ^
/home/pedro/brno/pedro/gdb/binutils-gdb/src/gdb/remote.c:12817:3: note: in expansion of macro ‘sprintf_vma’
   sprintf_vma (addrbuf, tpaddr);
   ^~~~~~~~~~~
/home/pedro/gdb/binutils-gdb/src/gdb/remote.c: In member function ‘virtual void remote_target::enable_tracepoint(bp_location*)’:
../bfd/bfd.h:198:62: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘CORE_ADDR {aka long long unsigned int}’ [-Werror=format=]
 #define sprintf_vma(s,x) sprintf (s, "%08" BFD_VMA_FMT "x", x)
...

If I use "make -k", I run into a few more like the above in two or three
more files.

I'd prefer to see this conversion in two separate patches.  One (or more) that
removes the bfd dependency from gdb's gdbsupport, which potentially is a material
change for making GDB always use 64-bit types, and then a separate change
to make gdbserver use top level gdbsupport.

Thanks,
Pedro Alves
Tom Tromey Feb. 19, 2020, 7:18 p.m. | #2
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:


Pedro> I'm surprised you didn't hit any problem, because last time we chatted
Pedro> about it, I quickly tried it, and ran into a small number of places
Pedro> that needed adjustment.

Yeah, I'm surprised too.

I just tried a mingw build and it found some issues.  So, I will redo it.

Pedro> I'd prefer to see this conversion in two separate patches.  One (or more) that
Pedro> removes the bfd dependency from gdb's gdbsupport, which potentially is a material
Pedro> change for making GDB always use 64-bit types, and then a separate change
Pedro> to make gdbserver use top level gdbsupport.

Will do.

Tom

Patch

diff --git a/Makefile.def b/Makefile.def
index 2fe8204366c..52714924f46 100644
--- a/Makefile.def
+++ b/Makefile.def
@@ -412,6 +412,7 @@  dependencies = { module=all-gdb; on=all-libctf; };
 
 // Host modules specific to gdbserver.
 dependencies = { module=configure-gdbserver; on=all-gnulib; };
+dependencies = { module=all-gdbserver; on=all-gdbsupport; };
 dependencies = { module=all-gdbserver; on=all-gnulib; };
 dependencies = { module=all-gdbserver; on=all-libiberty; };
 
@@ -421,9 +422,7 @@  dependencies = { module=all-libgui; on=all-tcl; };
 dependencies = { module=all-libgui; on=all-tk; };
 dependencies = { module=all-libgui; on=all-itcl; };
 
-dependencies = { module=configure-gdbsupport; on=configure-bfd; };
 dependencies = { module=configure-gdbsupport; on=configure-gnulib; };
-dependencies = { module=all-gdbsupport; on=all-bfd; };
 dependencies = { module=all-gdbsupport; on=all-gnulib; };
 
 // Host modules specific to binutils.
diff --git a/Makefile.in b/Makefile.in
index be38b34e9bf..0d756545a3c 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -51977,6 +51977,7 @@  all-gdb: maybe-all-build-bison
 all-gdb: maybe-all-sim
 all-gdb: maybe-all-libtermcap
 configure-gdbserver: maybe-all-gnulib
+all-gdbserver: maybe-all-gdbsupport
 all-gdbserver: maybe-all-gnulib
 configure-libgui: maybe-configure-tcl
 configure-libgui: maybe-configure-tk
@@ -52430,7 +52431,6 @@  configure-libcc1: stage_last
 configure-utils: stage_last
 configure-gdb: stage_last
 configure-gdbserver: stage_last
-configure-gdbsupport: stage_last
 configure-gprof: stage_last
 configure-sid: stage_last
 configure-sim: stage_last
@@ -52454,8 +52454,6 @@  all-gdb: maybe-all-opcodes
 all-gdb: maybe-all-libdecnumber
 all-gdb: maybe-all-libctf
 all-gdbserver: maybe-all-libiberty
-configure-gdbsupport: maybe-configure-bfd
-all-gdbsupport: maybe-all-bfd
 configure-gprof: maybe-configure-intl
 all-gprof: maybe-all-libiberty
 all-gprof: maybe-all-bfd
diff --git a/gdbserver/Makefile.in b/gdbserver/Makefile.in
index 1baebba0f56..0804743c9ae 100644
--- a/gdbserver/Makefile.in
+++ b/gdbserver/Makefile.in
@@ -100,6 +100,9 @@  INCLUDE_DEP = $$(INCLUDE_DIR)
 LIBIBERTY_BUILDDIR = ../libiberty
 LIBIBERTY = $(LIBIBERTY_BUILDDIR)/libiberty.a
 
+GDBSUPPORT_BUILDDIR = ../gdbsupport
+GDBSUPPORT = $(GDBSUPPORT_BUILDDIR)/libgdbsupport.a
+
 # Where is ust?  These will be empty if ust was not available.
 ustlibs = @ustlibs@
 ustinc = @ustinc@
@@ -205,32 +208,6 @@  SFILES = \
 	$(srcdir)/../gdb/arch/arm-linux.c \
 	$(srcdir)/../gdb/arch/ppc-linux-common.c \
 	$(srcdir)/../gdb/arch/riscv.c \
-	$(srcdir)/../gdbsupport/btrace-common.cc \
-	$(srcdir)/../gdbsupport/buffer.cc \
-	$(srcdir)/../gdbsupport/cleanups.cc \
-	$(srcdir)/../gdbsupport/common-debug.cc \
-	$(srcdir)/../gdbsupport/common-exceptions.cc \
-	$(srcdir)/../gdbsupport/common-inferior.cc \
-	$(srcdir)/../gdbsupport/common-regcache.cc \
-	$(srcdir)/../gdbsupport/common-utils.cc \
-	$(srcdir)/../gdbsupport/errors.cc \
-	$(srcdir)/../gdbsupport/environ.cc \
-	$(srcdir)/../gdbsupport/fileio.cc \
-	$(srcdir)/../gdbsupport/filestuff.cc \
-	$(srcdir)/../gdbsupport/job-control.cc \
-	$(srcdir)/../gdbsupport/gdb-dlfcn.cc \
-	$(srcdir)/../gdbsupport/gdb_tilde_expand.cc \
-	$(srcdir)/../gdbsupport/gdb_vecs.cc \
-	$(srcdir)/../gdbsupport/gdb_wait.cc \
-	$(srcdir)/../gdbsupport/netstuff.cc \
-	$(srcdir)/../gdbsupport/new-op.cc \
-	$(srcdir)/../gdbsupport/pathstuff.cc \
-	$(srcdir)/../gdbsupport/print-utils.cc \
-	$(srcdir)/../gdbsupport/ptid.cc \
-	$(srcdir)/../gdbsupport/rsp-low.cc \
-	$(srcdir)/../gdbsupport/safe-strerror.cc \
-	$(srcdir)/../gdbsupport/tdesc.cc \
-	$(srcdir)/../gdbsupport/xml-utils.cc \
 	$(srcdir)/../gdb/nat/aarch64-sve-linux-ptrace.c \
 	$(srcdir)/../gdb/nat/linux-btrace.c \
 	$(srcdir)/../gdb/nat/linux-namespaces.c \
@@ -252,36 +229,6 @@  TAGFILES = $(SOURCES) ${HFILES} ${ALLPARAM} ${POSSLIBS}
 OBS = \
 	alloc.o \
 	ax.o \
-	gdbsupport/agent.o \
-	gdbsupport/btrace-common.o \
-	gdbsupport/buffer.o \
-	gdbsupport/cleanups.o \
-	gdbsupport/common-debug.o \
-	gdbsupport/common-exceptions.o \
-	gdbsupport/common-inferior.o \
-	gdbsupport/job-control.o \
-	gdbsupport/common-regcache.o \
-	gdbsupport/common-utils.o \
-	gdbsupport/errors.o \
-	gdbsupport/environ.o \
-	gdbsupport/fileio.o \
-	gdbsupport/filestuff.o \
-	gdbsupport/format.o \
-	gdbsupport/gdb-dlfcn.o \
-	gdbsupport/gdb_tilde_expand.o \
-	gdbsupport/gdb_vecs.o \
-	gdbsupport/gdb_wait.o \
-	gdbsupport/netstuff.o \
-	gdbsupport/new-op.o \
-	gdbsupport/pathstuff.o \
-	gdbsupport/print-utils.o \
-	gdbsupport/ptid.o \
-	gdbsupport/rsp-low.o \
-	gdbsupport/safe-strerror.o \
-	gdbsupport/signals.o \
-	gdbsupport/signals-state-save-restore.o \
-	gdbsupport/tdesc.o \
-	gdbsupport/xml-utils.o \
 	debug.o \
 	dll.o \
 	event-loop.o \
@@ -304,14 +251,6 @@  OBS = \
 	$(XML_BUILTIN)
 
 GDBREPLAY_OBS = \
-	gdbsupport/cleanups.o \
-	gdbsupport/common-exceptions.o \
-	gdbsupport/common-utils.o \
-	gdbsupport/rsp-low.o \
-	gdbsupport/errors.o \
-	gdbsupport/netstuff.o \
-	gdbsupport/print-utils.o \
-	gdbsupport/safe-strerror.o \
 	gdbreplay.o \
 	utils.o \
 	version.o
@@ -403,17 +342,19 @@  html:
 install-html:
 clean-info:
 
-gdbserver$(EXEEXT): $(sort $(OBS)) ${CDEPS} $(LIBGNU) $(LIBIBERTY)
+gdbserver$(EXEEXT): $(sort $(OBS)) ${CDEPS} $(LIBGNU) $(LIBIBERTY) \
+		$(GDBSUPPORT)
 	$(SILENCE) rm -f gdbserver$(EXEEXT)
 	$(ECHO_CXXLD) $(CC_LD) $(INTERNAL_CFLAGS) $(INTERNAL_LDFLAGS) \
-		-o gdbserver$(EXEEXT) $(OBS) $(LIBGNU) $(LIBIBERTY) \
-		$(GDBSERVER_LIBS) $(XM_CLIBS)
+		-o gdbserver$(EXEEXT) $(OBS) $(GDBSUPPORT) $(LIBGNU) \
+		$(LIBIBERTY) $(GDBSERVER_LIBS) $(XM_CLIBS)
 
-gdbreplay$(EXEEXT): $(sort $(GDBREPLAY_OBS)) $(LIBGNU) $(LIBIBERTY)
+gdbreplay$(EXEEXT): $(sort $(GDBREPLAY_OBS)) $(LIBGNU) $(LIBIBERTY) \
+		$(GDBSUPPORT)
 	$(SILENCE) rm -f gdbreplay$(EXEEXT)
 	$(ECHO_CXXLD) $(CC_LD) $(INTERNAL_CFLAGS) $(INTERNAL_LDFLAGS) \
-		-o gdbreplay$(EXEEXT) $(GDBREPLAY_OBS) $(XM_CLIBS) $(LIBGNU) \
-		$(LIBIBERTY)
+		-o gdbreplay$(EXEEXT) $(GDBREPLAY_OBS) $(XM_CLIBS) \
+		$(GDBSUPPORT) $(LIBGNU) $(LIBIBERTY)
 
 IPA_OBJS = \
 	alloc-ipa.o \
@@ -449,7 +390,6 @@  TAGS:	${TAGFILES}
 	     if [ x$$i != xyzzy ]; then \
 	       echo ${srcdir}/$$i | sed -e 's/\.o$$/\.cc/' \
 		 -e 's,/\(arch\|nat\|target\)/,/../\1/,' \
-		 -e 's,/\(gdbsupport\)/,/../../\1/,'; \
 	     fi; \
 	   done` \
 	  ${TAGFILES}
@@ -585,10 +525,6 @@  arch/%.o: ../gdb/arch/%.c
 	$(COMPILE) -x c++ $<
 	$(POSTCOMPILE)
 
-gdbsupport/%.o: ../gdbsupport/%.cc
-	$(COMPILE) $<
-	$(POSTCOMPILE)
-
 %.o: %-generated.cc
 	$(COMPILE) $<
 	$(POSTCOMPILE)
diff --git a/gdbserver/configure b/gdbserver/configure
index 0ae464644c9..0d35ec8cb09 100755
--- a/gdbserver/configure
+++ b/gdbserver/configure
@@ -6098,8 +6098,6 @@  if $enable_unittests; then
 $as_echo "#define GDB_SELF_TEST 1" >>confdefs.h
 
 
-  srv_selftest_objs="gdbsupport/selftest.o"
-
 fi
 
 
@@ -10526,7 +10524,7 @@  $as_echo "#define USE_XML 1" >>confdefs.h
   done
 fi
 
-GDBSERVER_DEPFILES="$srv_regobj $srv_tgtobj $srv_hostio_err_objs $srv_thread_depfiles $srv_selftest_objs"
+GDBSERVER_DEPFILES="$srv_regobj $srv_tgtobj $srv_hostio_err_objs $srv_thread_depfiles"
 GDBSERVER_LIBS="$srv_libs"
 
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether the target supports __sync_*_compare_and_swap" >&5
diff --git a/gdbserver/configure.ac b/gdbserver/configure.ac
index be2284b26c5..65155afa471 100644
--- a/gdbserver/configure.ac
+++ b/gdbserver/configure.ac
@@ -46,9 +46,7 @@  AC_HEADER_STDC
 # Set the 'development' global.
 . $srcdir/../bfd/development.sh
 
-GDB_AC_SELFTEST([
-  srv_selftest_objs="gdbsupport/selftest.o"
-])
+GDB_AC_SELFTEST
 
 ACX_NONCANONICAL_TARGET
 ACX_NONCANONICAL_HOST
@@ -357,7 +355,7 @@  if test "$srv_xmlfiles" != ""; then
   done
 fi
 
-GDBSERVER_DEPFILES="$srv_regobj $srv_tgtobj $srv_hostio_err_objs $srv_thread_depfiles $srv_selftest_objs"
+GDBSERVER_DEPFILES="$srv_regobj $srv_tgtobj $srv_hostio_err_objs $srv_thread_depfiles"
 GDBSERVER_LIBS="$srv_libs"
 
 dnl Check whether the target supports __sync_*_compare_and_swap.
diff --git a/gdbsupport/common-defs.h b/gdbsupport/common-defs.h
index a5c57de2f27..65500ce7634 100644
--- a/gdbsupport/common-defs.h
+++ b/gdbsupport/common-defs.h
@@ -20,20 +20,6 @@ 
 #ifndef COMMON_COMMON_DEFS_H
 #define COMMON_COMMON_DEFS_H
 
-#ifdef GDBSERVER
-
-#include <gnulib/config.h>
-
-#undef PACKAGE_NAME
-#undef PACKAGE
-#undef PACKAGE_VERSION
-#undef PACKAGE_STRING
-#undef PACKAGE_TARNAME
-
-#include <config.h>
-
-#else  /* GDBSERVER */
-
 #include <gdbsupport/config.h>
 
 #undef PACKAGE_NAME
@@ -44,8 +30,6 @@ 
 
 #include "gnulib/config.h"
 
-#endif	/* GDBSERVER */
-
 /* From:
     https://www.gnu.org/software/gnulib/manual/html_node/stdint_002eh.html
 
diff --git a/gdbsupport/common-types.h b/gdbsupport/common-types.h
index 61099b4e25d..f5b2f3d2491 100644
--- a/gdbsupport/common-types.h
+++ b/gdbsupport/common-types.h
@@ -20,40 +20,18 @@ 
 #ifndef COMMON_COMMON_TYPES_H
 #define COMMON_COMMON_TYPES_H
 
-#ifdef GDBSERVER
+#include <inttypes.h>
 
 /* * A byte from the program being debugged.  */
 typedef unsigned char gdb_byte;
 
-typedef unsigned long long CORE_ADDR;
-
-typedef long long LONGEST;
-typedef unsigned long long ULONGEST;
-
-#else /* GDBSERVER */
-
-#include "bfd.h"
-
-/* * A byte from the program being debugged.  */
-typedef bfd_byte gdb_byte;
-
 /* * An address in the program being debugged.  Host byte order.  */
-typedef bfd_vma CORE_ADDR;
-
-/* This is to make sure that LONGEST is at least as big as CORE_ADDR.  */
-
-#ifdef BFD64
-
-typedef BFD_HOST_64_BIT LONGEST;
-typedef BFD_HOST_U_64_BIT ULONGEST;
-
-#else /* No BFD64 */
+typedef uint64_t CORE_ADDR;
 
-typedef long long LONGEST;
-typedef unsigned long long ULONGEST;
+/* LONGEST must be at least as big as CORE_ADDR.  */
 
-#endif /* No BFD64 */
-#endif /* GDBSERVER */
+typedef int64_t LONGEST;
+typedef uint64_t ULONGEST;
 
 /* * The largest CORE_ADDR value.  */
 #define CORE_ADDR_MAX (~(CORE_ADDR) 0)