[PATCH V2] gdb: link executables with libtool

[PATCH V2] gdb: link executables with libtool

[Jose E. Marchesi]

This patch changes the GDB build system in order to use libtool to
link the several built executables.  This makes it possible to refer
to libtool libraries (.la files) in CLIBS.

As an application of the above,

  LIBBFD           new refers to ../libbfd/libbfd.la
  LIBBACKTRACE_LIB now refers to ../libbacktrace/libbacktrace.la
  LIBCTF           now refers to ../libctf/libctf.la

NOTE1: The addition of libtool adds a few new configure-time options
       to GDB.  Among these, --enable-shared and --disable-shared,
       which were previously ignored.  Now GDB shall honor these
       options when linking, picking up the right version of the
       referred libtool libraries automagically.

NOTE2: I have not tested the insight build.

NOTE3: For regenerating configure I used an environment with Autoconf
       2.69 and Automake 1.15.1.  This should match the previously
       used version as announced in the configure script.

NOTE4: Now the installed shared object libbfd.so is used by gdb
       if binutils is installed with --enable-shared.

Testing performed:

- --enable-shared and --disable-shared (the default in binutils) work
  as expected: the linked executables link with the archive or shared
  libraries transparently.

- Makefile.in modified for EXEEXT = .exe.  It installs the binaries
  just fine.  The installed gdb.exe runs fine.

- Native build regtested in x86_64. The installed gdb runs fine.

  In the regression testing I'm observing that the following tests
  doesn't seem to be deterministic:

    gdb.base/step-over-syscall.exp
    gdb.threads/process-dies-while-detaching.exp
    gdb.threads/process-dies-while-handling-bp.exp

  Sometimes some of the the tests in these files unexpectedly fail,
  like in:

  -PASS: gdb.threads/process-dies-while-detaching.exp: single-process: \
         continue: detach: continue
  +FAIL: gdb.threads/process-dies-while-detaching.exp: single-process: \
         continue: detach: continue

  Sometimes they unexpectedly pass:

  -KFAIL: gdb.base/step-over-syscall.exp: clone: displaced=on: \
          check_pc_after_cross_syscall: single step over clone \
          final pc (PRMS: gdb/19675)
  +PASS: gdb.base/step-over-syscall.exp: clone: displaced=on: \
         check_pc_after_cross_syscall: single step over clone final pc

  -KFAIL: gdb.threads/process-dies-while-handling-bp.exp: \
          non_stop=on: cond_bp_target=0: inferior 1 exited \
          (prompt) (PRMS: gdb/18749)
  +PASS: gdb.threads/process-dies-while-handling-bp.exp: \
          non_stop=on: cond_bp_target=0: inferior 1 exited

- Cross build for aarch64-linux-gnu built to exercise
  program_transform_name and friends.  The installed
  aarch64-linux-gnu-gdb runs fine.

2022-11-05  Jose E. Marchesi  <jose.marchesi@oracle.com>

	* gdb/aclocal.m4: include libtool macros from the top-level
	directory.
	* gdb/Makefile.in (LIBTOOL): Define.
	(CC_LD): Use libtool for linking.
	(install-only): Use libtool for installing GDB$(EXEEXT).
	(install-gdbtk): Likewise for insight$(EXEEXT).
	* gdb/configure.ac: Call LT_INIT.
	Refer to libctf.la and libbacktrace.la.
	* gdb/configure: Regenerate.
	* gdb/testsuite/lib/gdb.exp: Run uninstalled GDB with libtool
	--mode=execute.
---
 ChangeLog                 |    15 +
 gdb/Makefile.in           |    12 +-
 gdb/aclocal.m4            |     5 +
 gdb/configure             | 31544 ++++++++++++++++++++++++------------
 gdb/configure.ac          |    15 +-
 gdb/testsuite/lib/gdb.exp |     2 +-
 6 files changed, 21142 insertions(+), 10451 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 73c3a006881..fea3f7c6877 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2022-11-05  Jose E. Marchesi  <jose.marchesi@oracle.com>
+
+	* gdb/aclocal.m4: include libtool macros from the top-level
+	directory.
+	* gdb/Makefile.in (LIBTOOL): Define.
+	(CC_LD): Use libtool for linking.
+	(install-only): Use libtool for installing GDB$(EXEEXT).
+	(install-gdbtk): Likewise for insight$(EXEEXT).
+	Use libbfd.la instead of libbfd.a.
+	* gdb/configure.ac: Call LT_INIT.
+	Refer to libctf.la and libbacktrace.la.
+	* gdb/configure: Regenerate.
+	* gdb/testsuite/lib/gdb.exp: Run uninstalled GDB with libtool
+	--mode=execute.
+
 2022-10-10  Nick Clifton  <nickc@redhat.com>
 
 	* src-release.sh: Add "-r <date>" option to create reproducible
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
index c528ee5aa80..b03eedd4d2f 100644
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -143,10 +143,12 @@ MAKEINFO_CMD = $(MAKEINFO) $(MAKEINFOFLAGS) $(MAKEINFO_EXTRA_FLAGS)
 MAKEHTML = $(MAKEINFO_CMD) --html
 MAKEHTMLFLAGS =
 
+LIBTOOL = @LIBTOOL@
+
 # Set this up with gcc if you have gnu ld and the loader will print out
 # line numbers for undefined references.
 #CC_LD = g++ -static
-CC_LD = $(CXX) $(CXX_DIALECT)
+CC_LD = $(LIBTOOL) --mode=link $(CXX) $(CXX_DIALECT)
 
 # Where is our "include" directory?  Typically $(srcdir)/../include.
 # This is essentially the header file directory for the library
@@ -163,7 +165,7 @@ CTF_DEPS = @CTF_DEPS@
 
 # Where is the BFD library?  Typically in ../bfd.
 BFD_DIR = ../bfd
-BFD = $(BFD_DIR)/libbfd.a
+BFD = $(BFD_DIR)/libbfd.la
 BFD_SRC = $(srcdir)/$(BFD_DIR)
 BFD_CFLAGS = -I$(BFD_DIR) -I$(BFD_SRC)
 
@@ -2017,7 +2019,8 @@ install-only: $(CONFIG_INSTALL)
 		  true ; \
 		fi ; \
 		$(SHELL) $(srcdir)/../mkinstalldirs $(DESTDIR)$(bindir) ; \
-		$(INSTALL_PROGRAM_ENV) $(INSTALL_PROGRAM) gdb$(EXEEXT) \
+		$(LIBTOOL) --mode=install $(INSTALL_PROGRAM_ENV) $(INSTALL_PROGRAM) \
+			gdb$(EXEEXT) \
 			$(DESTDIR)$(bindir)/$$transformed_name$(EXEEXT) ; \
 		$(SHELL) $(srcdir)/../mkinstalldirs $(DESTDIR)$(includedir)/gdb ; \
 		$(INSTALL_DATA) jit-reader.h $(DESTDIR)$(includedir)/gdb/jit-reader.h
@@ -2529,7 +2532,8 @@ install-gdbtk:
 	  true ; \
 	fi ; \
 	$(SHELL) $(srcdir)/../mkinstalldirs $(DESTDIR)$(bindir); \
-	$(INSTALL_PROGRAM_ENV) $(INSTALL_PROGRAM) insight$(EXEEXT) \
+	$(LIBTOOL) --mode=install $(INSTALL_PROGRAM_ENV) $(INSTALL_PROGRAM) \
+		insight$(EXEEXT) \
 		$(DESTDIR)$(bindir)/$$transformed_name$(EXEEXT) ; \
 	$(SHELL) $(srcdir)/../mkinstalldirs \
 		$(DESTDIR)$(GDBTK_LIBRARY) ; \
diff --git a/gdb/aclocal.m4 b/gdb/aclocal.m4
index 3ed4a58d39f..4aac87b52ed 100644
--- a/gdb/aclocal.m4
+++ b/gdb/aclocal.m4
@@ -212,4 +212,9 @@ m4_include([../config/override.m4])
 m4_include([../config/pkg.m4])
 m4_include([../config/plugins.m4])
 m4_include([../config/tcl.m4])
+m4_include([../libtool.m4])
+m4_include([../ltoptions.m4])
+m4_include([../ltsugar.m4])
+m4_include([../ltversion.m4])
+m4_include([../lt~obsolete.m4])
 m4_include([acinclude.m4])
diff --git a/gdb/configure.ac b/gdb/configure.ac
index fceb80e8c9d..4fd77e48576 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -46,6 +46,10 @@ ACX_NONCANONICAL_TARGET
 
 AC_ARG_PROGRAM
 
+# We require libtool to link with the in-tree libtool libraries
+# the proper way.
+LT_INIT
+
 # We require a C++11 compiler.  Check if one is available, and if
 # necessary, set CXX_DIALECT to some -std=xxx switch.
 AX_CXX_COMPILE_STDCXX(11, , mandatory)
@@ -2092,7 +2096,7 @@ AC_ARG_ENABLE([libbacktrace],
 
 if test "${enable_libbacktrace}" = "yes"; then
   LIBBACKTRACE_INC="-I$srcdir/../libbacktrace/ -I../libbacktrace/"
-  LIBBACKTRACE_LIB=../libbacktrace/.libs/libbacktrace.a
+  LIBBACKTRACE_LIB=../libbacktrace/libbacktrace.la
   AC_DEFINE(HAVE_LIBBACKTRACE, 1, [Define if libbacktrace is being used.])
 else
   LIBBACKTRACE_INC=
@@ -2151,15 +2155,10 @@ AC_ARG_WITH(xxhash,
   [], [with_xxhash=auto])
 
 GCC_ENABLE([libctf], [yes], [], [Handle .ctf type-info sections])
-if test x${enable_static} = xno; then
-  LIBCTF="-Wl,--rpath,../libctf/.libs ../libctf/.libs/libctf.so"
-  CTF_DEPS="../libctf/.libs/libctf.so"
-else
-  LIBCTF="../libctf/.libs/libctf.a"
-  CTF_DEPS="$LIBCTF"
-fi
 if test "${enable_libctf}" = yes; then
   AC_DEFINE(ENABLE_LIBCTF, 1, [Handle .ctf type-info sections])
+  LIBCTF="../libctf/libctf.la"
+  CTF_DEPS="../libctf/libctf.la"
 else
   LIBCTF=
   CTF_DEPS=
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index e2cda30b95a..63d57581d13 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -154,7 +154,7 @@ global GDB_DATA_DIRECTORY
 global inferior_spawn_id
 
 if [info exists TOOL_EXECUTABLE] {
-    set GDB $TOOL_EXECUTABLE
+    set GDB "libtool --mode=execute $TOOL_EXECUTABLE"
 }
 if ![info exists GDB] {
     if ![is_remote host] {
-- 
2.30.2

Re: [PATCH V2] gdb: link executables with libtool

[Simon Marchi]

On 11/5/22 10:02, Jose E. Marchesi via Gdb-patches wrote:
> This patch changes the GDB build system in order to use libtool to
> link the several built executables.  This makes it possible to refer
> to libtool libraries (.la files) in CLIBS.
> 
> As an application of the above,
> 
>   LIBBFD           new refers to ../libbfd/libbfd.la

new -> now

>   LIBBACKTRACE_LIB now refers to ../libbacktrace/libbacktrace.la
>   LIBCTF           now refers to ../libctf/libctf.la

The patch linked in this bug:

https://sourceware.org/bugzilla/show_bug.cgi?id=29372

also updated OPCODES to use $(OPCODES_DIR)/libopcodes.la, should you do
it too?

> 
> NOTE1: The addition of libtool adds a few new configure-time options
>        to GDB.  Among these, --enable-shared and --disable-shared,
>        which were previously ignored.  Now GDB shall honor these
>        options when linking, picking up the right version of the
>        referred libtool libraries automagically.
> 
> NOTE2: I have not tested the insight build.
> 
> NOTE3: For regenerating configure I used an environment with Autoconf
>        2.69 and Automake 1.15.1.  This should match the previously
>        used version as announced in the configure script.

Your patch appears to be missing the changes to gdb/configure, however
the diffstat in your email shows:

 gdb/configure             | 31544 ++++++++++++++++++++++++------------

When running `autoreconf -f` in the gdb/ directory, I get some changes
that revert your changes to gdb/aclocal.m4, and makes some changes to
gdb/configure.  But I don't think my changes are right, because LT_INIT
stays unexpanded in the generated configure.

Does the method of regenerating the build system change with this patch?

If I run libtoolize in gdb/, it adds a bunch of .m4 files to
$top_level/config, and then if I run autoreconf, LT_INIT gets properly
expanded.  But these .m4 files already exist at the top-level such as
$top_level/libtool.m4, I guess we don't want them twice.  Should
gdb/configure.ac be modified to find the existing files?  For instance,
this makes it work for me:


diff --git a/gdb/configure.ac b/gdb/configure.ac
index 4fd77e485762..12561d4d2de6 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -19,7 +19,7 @@ dnl along with this program.  If not, see <http://www.gnu.org/licenses/>.
 dnl Process this file with autoconf to produce a configure script.
 
 AC_INIT
-AC_CONFIG_MACRO_DIRS([../config])
+AC_CONFIG_MACRO_DIRS([.. ../config])
 AC_CONFIG_SRCDIR([main.c])
 AC_CONFIG_HEADERS(config.h:config.in, [echo > stamp-h])
 AM_MAINTAINER_MODE



> NOTE4: Now the installed shared object libbfd.so is used by gdb
>        if binutils is installed with --enable-shared.

Do you mean bfd instead of binutils?  Or do you mean binutils in the
sense that bfd is part of binutils?

> Testing performed:
> 
> - --enable-shared and --disable-shared (the default in binutils) work
>   as expected: the linked executables link with the archive or shared
>   libraries transparently.
> 
> - Makefile.in modified for EXEEXT = .exe.  It installs the binaries
>   just fine.  The installed gdb.exe runs fine.
> 
> - Native build regtested in x86_64. The installed gdb runs fine.
> 
>   In the regression testing I'm observing that the following tests
>   doesn't seem to be deterministic:
> 
>     gdb.base/step-over-syscall.exp
>     gdb.threads/process-dies-while-detaching.exp
>     gdb.threads/process-dies-while-handling-bp.exp
> 
>   Sometimes some of the the tests in these files unexpectedly fail,
>   like in:
> 
>   -PASS: gdb.threads/process-dies-while-detaching.exp: single-process: \
>          continue: detach: continue
>   +FAIL: gdb.threads/process-dies-while-detaching.exp: single-process: \
>          continue: detach: continue
> 
>   Sometimes they unexpectedly pass:
> 
>   -KFAIL: gdb.base/step-over-syscall.exp: clone: displaced=on: \
>           check_pc_after_cross_syscall: single step over clone \
>           final pc (PRMS: gdb/19675)
>   +PASS: gdb.base/step-over-syscall.exp: clone: displaced=on: \
>          check_pc_after_cross_syscall: single step over clone final pc
> 
>   -KFAIL: gdb.threads/process-dies-while-handling-bp.exp: \
>           non_stop=on: cond_bp_target=0: inferior 1 exited \
>           (prompt) (PRMS: gdb/18749)
>   +PASS: gdb.threads/process-dies-while-handling-bp.exp: \
>           non_stop=on: cond_bp_target=0: inferior 1 exited

It's unfortunately not the only ones.  It's not good, but it's known.

> - Cross build for aarch64-linux-gnu built to exercise
>   program_transform_name and friends.  The installed
>   aarch64-linux-gnu-gdb runs fine.

Great, thanks for the details on the testing.

> 
> 2022-11-05  Jose E. Marchesi  <jose.marchesi@oracle.com>
> 
> 	* gdb/aclocal.m4: include libtool macros from the top-level
> 	directory.
> 	* gdb/Makefile.in (LIBTOOL): Define.
> 	(CC_LD): Use libtool for linking.
> 	(install-only): Use libtool for installing GDB$(EXEEXT).
> 	(install-gdbtk): Likewise for insight$(EXEEXT).
> 	* gdb/configure.ac: Call LT_INIT.
> 	Refer to libctf.la and libbacktrace.la.
> 	* gdb/configure: Regenerate.
> 	* gdb/testsuite/lib/gdb.exp: Run uninstalled GDB with libtool
> 	--mode=execute.

Could you add the git trailer:

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29372

?

> ---
>  ChangeLog                 |    15 +

I don't think you should add gdb changes to the top-level ChangeLog.  We
simply don't use ChangeLogs now.

>  gdb/Makefile.in           |    12 +-
>  gdb/aclocal.m4            |     5 +
>  gdb/configure             | 31544 ++++++++++++++++++++++++------------
>  gdb/configure.ac          |    15 +-
>  gdb/testsuite/lib/gdb.exp |     2 +-
>  6 files changed, 21142 insertions(+), 10451 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 73c3a006881..fea3f7c6877 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,18 @@
> +2022-11-05  Jose E. Marchesi  <jose.marchesi@oracle.com>
> +
> +	* gdb/aclocal.m4: include libtool macros from the top-level
> +	directory.
> +	* gdb/Makefile.in (LIBTOOL): Define.
> +	(CC_LD): Use libtool for linking.
> +	(install-only): Use libtool for installing GDB$(EXEEXT).
> +	(install-gdbtk): Likewise for insight$(EXEEXT).
> +	Use libbfd.la instead of libbfd.a.
> +	* gdb/configure.ac: Call LT_INIT.
> +	Refer to libctf.la and libbacktrace.la.
> +	* gdb/configure: Regenerate.
> +	* gdb/testsuite/lib/gdb.exp: Run uninstalled GDB with libtool
> +	--mode=execute.
> +
>  2022-10-10  Nick Clifton  <nickc@redhat.com>
>  
>  	* src-release.sh: Add "-r <date>" option to create reproducible
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index c528ee5aa80..b03eedd4d2f 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -143,10 +143,12 @@ MAKEINFO_CMD = $(MAKEINFO) $(MAKEINFOFLAGS) $(MAKEINFO_EXTRA_FLAGS)
>  MAKEHTML = $(MAKEINFO_CMD) --html
>  MAKEHTMLFLAGS =
>  
> +LIBTOOL = @LIBTOOL@
> +
>  # Set this up with gcc if you have gnu ld and the loader will print out
>  # line numbers for undefined references.
>  #CC_LD = g++ -static
> -CC_LD = $(CXX) $(CXX_DIALECT)
> +CC_LD = $(LIBTOOL) --mode=link $(CXX) $(CXX_DIALECT)
>  
>  # Where is our "include" directory?  Typically $(srcdir)/../include.
>  # This is essentially the header file directory for the library
> @@ -163,7 +165,7 @@ CTF_DEPS = @CTF_DEPS@
>  
>  # Where is the BFD library?  Typically in ../bfd.
>  BFD_DIR = ../bfd
> -BFD = $(BFD_DIR)/libbfd.a
> +BFD = $(BFD_DIR)/libbfd.la
>  BFD_SRC = $(srcdir)/$(BFD_DIR)
>  BFD_CFLAGS = -I$(BFD_DIR) -I$(BFD_SRC)
>  
> @@ -2017,7 +2019,8 @@ install-only: $(CONFIG_INSTALL)
>  		  true ; \
>  		fi ; \
>  		$(SHELL) $(srcdir)/../mkinstalldirs $(DESTDIR)$(bindir) ; \
> -		$(INSTALL_PROGRAM_ENV) $(INSTALL_PROGRAM) gdb$(EXEEXT) \
> +		$(LIBTOOL) --mode=install $(INSTALL_PROGRAM_ENV) $(INSTALL_PROGRAM) \
> +			gdb$(EXEEXT) \
>  			$(DESTDIR)$(bindir)/$$transformed_name$(EXEEXT) ; \
>  		$(SHELL) $(srcdir)/../mkinstalldirs $(DESTDIR)$(includedir)/gdb ; \
>  		$(INSTALL_DATA) jit-reader.h $(DESTDIR)$(includedir)/gdb/jit-reader.h
> @@ -2529,7 +2532,8 @@ install-gdbtk:
>  	  true ; \
>  	fi ; \
>  	$(SHELL) $(srcdir)/../mkinstalldirs $(DESTDIR)$(bindir); \
> -	$(INSTALL_PROGRAM_ENV) $(INSTALL_PROGRAM) insight$(EXEEXT) \
> +	$(LIBTOOL) --mode=install $(INSTALL_PROGRAM_ENV) $(INSTALL_PROGRAM) \
> +		insight$(EXEEXT) \
>  		$(DESTDIR)$(bindir)/$$transformed_name$(EXEEXT) ; \
>  	$(SHELL) $(srcdir)/../mkinstalldirs \
>  		$(DESTDIR)$(GDBTK_LIBRARY) ; \
> diff --git a/gdb/aclocal.m4 b/gdb/aclocal.m4
> index 3ed4a58d39f..4aac87b52ed 100644
> --- a/gdb/aclocal.m4
> +++ b/gdb/aclocal.m4
> @@ -212,4 +212,9 @@ m4_include([../config/override.m4])
>  m4_include([../config/pkg.m4])
>  m4_include([../config/plugins.m4])
>  m4_include([../config/tcl.m4])
> +m4_include([../libtool.m4])
> +m4_include([../ltoptions.m4])
> +m4_include([../ltsugar.m4])
> +m4_include([../ltversion.m4])
> +m4_include([../lt~obsolete.m4])
>  m4_include([acinclude.m4])
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index fceb80e8c9d..4fd77e48576 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -46,6 +46,10 @@ ACX_NONCANONICAL_TARGET
>  
>  AC_ARG_PROGRAM
>  
> +# We require libtool to link with the in-tree libtool libraries
> +# the proper way.
> +LT_INIT
> +
>  # We require a C++11 compiler.  Check if one is available, and if
>  # necessary, set CXX_DIALECT to some -std=xxx switch.
>  AX_CXX_COMPILE_STDCXX(11, , mandatory)
> @@ -2092,7 +2096,7 @@ AC_ARG_ENABLE([libbacktrace],
>  
>  if test "${enable_libbacktrace}" = "yes"; then
>    LIBBACKTRACE_INC="-I$srcdir/../libbacktrace/ -I../libbacktrace/"
> -  LIBBACKTRACE_LIB=../libbacktrace/.libs/libbacktrace.a
> +  LIBBACKTRACE_LIB=../libbacktrace/libbacktrace.la
>    AC_DEFINE(HAVE_LIBBACKTRACE, 1, [Define if libbacktrace is being used.])
>  else
>    LIBBACKTRACE_INC=
> @@ -2151,15 +2155,10 @@ AC_ARG_WITH(xxhash,
>    [], [with_xxhash=auto])
>  
>  GCC_ENABLE([libctf], [yes], [], [Handle .ctf type-info sections])
> -if test x${enable_static} = xno; then
> -  LIBCTF="-Wl,--rpath,../libctf/.libs ../libctf/.libs/libctf.so"
> -  CTF_DEPS="../libctf/.libs/libctf.so"
> -else
> -  LIBCTF="../libctf/.libs/libctf.a"
> -  CTF_DEPS="$LIBCTF"
> -fi
>  if test "${enable_libctf}" = yes; then
>    AC_DEFINE(ENABLE_LIBCTF, 1, [Handle .ctf type-info sections])
> +  LIBCTF="../libctf/libctf.la"
> +  CTF_DEPS="../libctf/libctf.la"
>  else
>    LIBCTF=
>    CTF_DEPS=
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index e2cda30b95a..63d57581d13 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -154,7 +154,7 @@ global GDB_DATA_DIRECTORY
>  global inferior_spawn_id
>  
>  if [info exists TOOL_EXECUTABLE] {
> -    set GDB $TOOL_EXECUTABLE
> +    set GDB "libtool --mode=execute $TOOL_EXECUTABLE"

Can you explain why this is needed?  My understanding is that
TOOL_EXECUTABLE can be used to point to an arbitrary gdb executable to
test.  If it's a gdb that has been "make install"ed, then libtool
doesn't have to be involved.  If pointing to a gdb executable in its
build directory, then "gdb" will be a libtool script that sets all the
right runtime library paths.  The only moment I can imagine this would
be useful would be if TOOL_EXECUTABLE consists of a wrapper around
gdb, e.g. "valgrind /path/to/build/gdb".

Simon

Re: [PATCH V2] gdb: link executables with libtool

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 826 bytes --]

On 05 Nov 2022 12:38, Simon Marchi via Gdb-patches wrote:
> On 11/5/22 10:02, Jose E. Marchesi via Gdb-patches wrote:
> >   LIBBACKTRACE_LIB now refers to ../libbacktrace/libbacktrace.la
> >   LIBCTF           now refers to ../libctf/libctf.la
> 
> The patch linked in this bug:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=29372
> 
> also updated OPCODES to use $(OPCODES_DIR)/libopcodes.la, should you do
> it too?

yeah, and i think we can drop zlib & zstd logic too since they're in there
only for bfd.  less sure about gettext usage ... if gdb uses gettext, we
want to keep it, but if it was pulled in only for bfd too, we can purge it
from the gdb side.

although i'm totally fine with doing the zlib & zstd cleanup as a followup
commit since Jose's patch here improves the status quo.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH V2] gdb: link executables with libtool

[Jose E. Marchesi]

> On 11/5/22 10:02, Jose E. Marchesi via Gdb-patches wrote:
>> This patch changes the GDB build system in order to use libtool to
>> link the several built executables.  This makes it possible to refer
>> to libtool libraries (.la files) in CLIBS.
>> 
>> As an application of the above,
>> 
>>   LIBBFD           new refers to ../libbfd/libbfd.la
>
> new -> now
>
>>   LIBBACKTRACE_LIB now refers to ../libbacktrace/libbacktrace.la
>>   LIBCTF           now refers to ../libctf/libctf.la
>
> The patch linked in this bug:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=29372
>
> also updated OPCODES to use $(OPCODES_DIR)/libopcodes.la, should you do
> it too?

Indeed I should.  Thanks for pointing that out :)


>> 
>> NOTE1: The addition of libtool adds a few new configure-time options
>>        to GDB.  Among these, --enable-shared and --disable-shared,
>>        which were previously ignored.  Now GDB shall honor these
>>        options when linking, picking up the right version of the
>>        referred libtool libraries automagically.
>> 
>> NOTE2: I have not tested the insight build.
>> 
>> NOTE3: For regenerating configure I used an environment with Autoconf
>>        2.69 and Automake 1.15.1.  This should match the previously
>>        used version as announced in the configure script.
>
> Your patch appears to be missing the changes to gdb/configure, however
> the diffstat in your email shows:
>
>  gdb/configure             | 31544 ++++++++++++++++++++++++------------
>
> When running `autoreconf -f` in the gdb/ directory, I get some changes
> that revert your changes to gdb/aclocal.m4, and makes some changes to
> gdb/configure.  But I don't think my changes are right, because LT_INIT
> stays unexpanded in the generated configure.
>
> Does the method of regenerating the build system change with this patch?
>
> If I run libtoolize in gdb/, it adds a bunch of .m4 files to
> $top_level/config, and then if I run autoreconf, LT_INIT gets properly
> expanded.  But these .m4 files already exist at the top-level such as
> $top_level/libtool.m4, I guess we don't want them twice.  Should
> gdb/configure.ac be modified to find the existing files?  For instance,
> this makes it work for me:
>
>
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index 4fd77e485762..12561d4d2de6 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -19,7 +19,7 @@ dnl along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  dnl Process this file with autoconf to produce a configure script.
>  
>  AC_INIT
> -AC_CONFIG_MACRO_DIRS([../config])
> +AC_CONFIG_MACRO_DIRS([.. ../config])
>  AC_CONFIG_SRCDIR([main.c])
>  AC_CONFIG_HEADERS(config.h:config.in, [echo > stamp-h])
>  AM_MAINTAINER_MODE

Hmm, the libtool that is in the binutils tree (in the top-level .m4
files, which are macros that generate the actual `libtool' scripts at
configure time) is actually:

1) A phantom release 2.2.7a that doesn't exist anyhwere else, or at
   least I haven't managed to find it.  It certainly doesn't exist in
   ftp.gnu.org nor in alpha.gnu.org.
   
2) Locally patched.

Indeed `autoreconf' runs libtoolize, so to avoid it to install other .m4
files your fix above makes sense.


>> NOTE4: Now the installed shared object libbfd.so is used by gdb
>>        if binutils is installed with --enable-shared.
>
> Do you mean bfd instead of binutils?  Or do you mean binutils in the
> sense that bfd is part of binutils?
>
>> Testing performed:
>> 
>> - --enable-shared and --disable-shared (the default in binutils) work
>>   as expected: the linked executables link with the archive or shared
>>   libraries transparently.
>> 
>> - Makefile.in modified for EXEEXT = .exe.  It installs the binaries
>>   just fine.  The installed gdb.exe runs fine.
>> 
>> - Native build regtested in x86_64. The installed gdb runs fine.
>> 
>>   In the regression testing I'm observing that the following tests
>>   doesn't seem to be deterministic:
>> 
>>     gdb.base/step-over-syscall.exp
>>     gdb.threads/process-dies-while-detaching.exp
>>     gdb.threads/process-dies-while-handling-bp.exp
>> 
>>   Sometimes some of the the tests in these files unexpectedly fail,
>>   like in:
>> 
>>   -PASS: gdb.threads/process-dies-while-detaching.exp: single-process: \
>>          continue: detach: continue
>>   +FAIL: gdb.threads/process-dies-while-detaching.exp: single-process: \
>>          continue: detach: continue
>> 
>>   Sometimes they unexpectedly pass:
>> 
>>   -KFAIL: gdb.base/step-over-syscall.exp: clone: displaced=on: \
>>           check_pc_after_cross_syscall: single step over clone \
>>           final pc (PRMS: gdb/19675)
>>   +PASS: gdb.base/step-over-syscall.exp: clone: displaced=on: \
>>          check_pc_after_cross_syscall: single step over clone final pc
>> 
>>   -KFAIL: gdb.threads/process-dies-while-handling-bp.exp: \
>>           non_stop=on: cond_bp_target=0: inferior 1 exited \
>>           (prompt) (PRMS: gdb/18749)
>>   +PASS: gdb.threads/process-dies-while-handling-bp.exp: \
>>           non_stop=on: cond_bp_target=0: inferior 1 exited
>
> It's unfortunately not the only ones.  It's not good, but it's known.

Ah ok.  I would say it it not bad at all, considering both the size and
character of the GDB testsuite.

>> - Cross build for aarch64-linux-gnu built to exercise
>>   program_transform_name and friends.  The installed
>>   aarch64-linux-gnu-gdb runs fine.
>
> Great, thanks for the details on the testing.
>
>> 
>> 2022-11-05  Jose E. Marchesi  <jose.marchesi@oracle.com>
>> 
>> 	* gdb/aclocal.m4: include libtool macros from the top-level
>> 	directory.
>> 	* gdb/Makefile.in (LIBTOOL): Define.
>> 	(CC_LD): Use libtool for linking.
>> 	(install-only): Use libtool for installing GDB$(EXEEXT).
>> 	(install-gdbtk): Likewise for insight$(EXEEXT).
>> 	* gdb/configure.ac: Call LT_INIT.
>> 	Refer to libctf.la and libbacktrace.la.
>> 	* gdb/configure: Regenerate.
>> 	* gdb/testsuite/lib/gdb.exp: Run uninstalled GDB with libtool
>> 	--mode=execute.
>
> Could you add the git trailer:
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=29372
>
> ?

Will do.

>
>> ---
>>  ChangeLog                 |    15 +
>
> I don't think you should add gdb changes to the top-level ChangeLog.  We
> simply don't use ChangeLogs now.

Oops, that was Emacs C-x4a.  I saw ChangeLog entries in the buffer with
reasonably recent entries and assumed it was GDB's.  Sorry about that :)

>
>>  gdb/Makefile.in           |    12 +-
>>  gdb/aclocal.m4            |     5 +
>>  gdb/configure             | 31544 ++++++++++++++++++++++++------------
>>  gdb/configure.ac          |    15 +-
>>  gdb/testsuite/lib/gdb.exp |     2 +-
>>  6 files changed, 21142 insertions(+), 10451 deletions(-)
>> 
>> diff --git a/ChangeLog b/ChangeLog
>> index 73c3a006881..fea3f7c6877 100644
>> --- a/ChangeLog
>> +++ b/ChangeLog
>> @@ -1,3 +1,18 @@
>> +2022-11-05  Jose E. Marchesi  <jose.marchesi@oracle.com>
>> +
>> +	* gdb/aclocal.m4: include libtool macros from the top-level
>> +	directory.
>> +	* gdb/Makefile.in (LIBTOOL): Define.
>> +	(CC_LD): Use libtool for linking.
>> +	(install-only): Use libtool for installing GDB$(EXEEXT).
>> +	(install-gdbtk): Likewise for insight$(EXEEXT).
>> +	Use libbfd.la instead of libbfd.a.
>> +	* gdb/configure.ac: Call LT_INIT.
>> +	Refer to libctf.la and libbacktrace.la.
>> +	* gdb/configure: Regenerate.
>> +	* gdb/testsuite/lib/gdb.exp: Run uninstalled GDB with libtool
>> +	--mode=execute.
>> +
>>  2022-10-10  Nick Clifton  <nickc@redhat.com>
>>  
>>  	* src-release.sh: Add "-r <date>" option to create reproducible
>> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
>> index c528ee5aa80..b03eedd4d2f 100644
>> --- a/gdb/Makefile.in
>> +++ b/gdb/Makefile.in
>> @@ -143,10 +143,12 @@ MAKEINFO_CMD = $(MAKEINFO) $(MAKEINFOFLAGS) $(MAKEINFO_EXTRA_FLAGS)
>>  MAKEHTML = $(MAKEINFO_CMD) --html
>>  MAKEHTMLFLAGS =
>>  
>> +LIBTOOL = @LIBTOOL@
>> +
>>  # Set this up with gcc if you have gnu ld and the loader will print out
>>  # line numbers for undefined references.
>>  #CC_LD = g++ -static
>> -CC_LD = $(CXX) $(CXX_DIALECT)
>> +CC_LD = $(LIBTOOL) --mode=link $(CXX) $(CXX_DIALECT)
>>  
>>  # Where is our "include" directory?  Typically $(srcdir)/../include.
>>  # This is essentially the header file directory for the library
>> @@ -163,7 +165,7 @@ CTF_DEPS = @CTF_DEPS@
>>  
>>  # Where is the BFD library?  Typically in ../bfd.
>>  BFD_DIR = ../bfd
>> -BFD = $(BFD_DIR)/libbfd.a
>> +BFD = $(BFD_DIR)/libbfd.la
>>  BFD_SRC = $(srcdir)/$(BFD_DIR)
>>  BFD_CFLAGS = -I$(BFD_DIR) -I$(BFD_SRC)
>>  
>> @@ -2017,7 +2019,8 @@ install-only: $(CONFIG_INSTALL)
>>  		  true ; \
>>  		fi ; \
>>  		$(SHELL) $(srcdir)/../mkinstalldirs $(DESTDIR)$(bindir) ; \
>> -		$(INSTALL_PROGRAM_ENV) $(INSTALL_PROGRAM) gdb$(EXEEXT) \
>> +		$(LIBTOOL) --mode=install $(INSTALL_PROGRAM_ENV) $(INSTALL_PROGRAM) \
>> +			gdb$(EXEEXT) \
>>  			$(DESTDIR)$(bindir)/$$transformed_name$(EXEEXT) ; \
>>  		$(SHELL) $(srcdir)/../mkinstalldirs $(DESTDIR)$(includedir)/gdb ; \
>>  		$(INSTALL_DATA) jit-reader.h $(DESTDIR)$(includedir)/gdb/jit-reader.h
>> @@ -2529,7 +2532,8 @@ install-gdbtk:
>>  	  true ; \
>>  	fi ; \
>>  	$(SHELL) $(srcdir)/../mkinstalldirs $(DESTDIR)$(bindir); \
>> -	$(INSTALL_PROGRAM_ENV) $(INSTALL_PROGRAM) insight$(EXEEXT) \
>> +	$(LIBTOOL) --mode=install $(INSTALL_PROGRAM_ENV) $(INSTALL_PROGRAM) \
>> +		insight$(EXEEXT) \
>>  		$(DESTDIR)$(bindir)/$$transformed_name$(EXEEXT) ; \
>>  	$(SHELL) $(srcdir)/../mkinstalldirs \
>>  		$(DESTDIR)$(GDBTK_LIBRARY) ; \
>> diff --git a/gdb/aclocal.m4 b/gdb/aclocal.m4
>> index 3ed4a58d39f..4aac87b52ed 100644
>> --- a/gdb/aclocal.m4
>> +++ b/gdb/aclocal.m4
>> @@ -212,4 +212,9 @@ m4_include([../config/override.m4])
>>  m4_include([../config/pkg.m4])
>>  m4_include([../config/plugins.m4])
>>  m4_include([../config/tcl.m4])
>> +m4_include([../libtool.m4])
>> +m4_include([../ltoptions.m4])
>> +m4_include([../ltsugar.m4])
>> +m4_include([../ltversion.m4])
>> +m4_include([../lt~obsolete.m4])
>>  m4_include([acinclude.m4])
>> diff --git a/gdb/configure.ac b/gdb/configure.ac
>> index fceb80e8c9d..4fd77e48576 100644
>> --- a/gdb/configure.ac
>> +++ b/gdb/configure.ac
>> @@ -46,6 +46,10 @@ ACX_NONCANONICAL_TARGET
>>  
>>  AC_ARG_PROGRAM
>>  
>> +# We require libtool to link with the in-tree libtool libraries
>> +# the proper way.
>> +LT_INIT
>> +
>>  # We require a C++11 compiler.  Check if one is available, and if
>>  # necessary, set CXX_DIALECT to some -std=xxx switch.
>>  AX_CXX_COMPILE_STDCXX(11, , mandatory)
>> @@ -2092,7 +2096,7 @@ AC_ARG_ENABLE([libbacktrace],
>>  
>>  if test "${enable_libbacktrace}" = "yes"; then
>>    LIBBACKTRACE_INC="-I$srcdir/../libbacktrace/ -I../libbacktrace/"
>> -  LIBBACKTRACE_LIB=../libbacktrace/.libs/libbacktrace.a
>> +  LIBBACKTRACE_LIB=../libbacktrace/libbacktrace.la
>>    AC_DEFINE(HAVE_LIBBACKTRACE, 1, [Define if libbacktrace is being used.])
>>  else
>>    LIBBACKTRACE_INC=
>> @@ -2151,15 +2155,10 @@ AC_ARG_WITH(xxhash,
>>    [], [with_xxhash=auto])
>>  
>>  GCC_ENABLE([libctf], [yes], [], [Handle .ctf type-info sections])
>> -if test x${enable_static} = xno; then
>> -  LIBCTF="-Wl,--rpath,../libctf/.libs ../libctf/.libs/libctf.so"
>> -  CTF_DEPS="../libctf/.libs/libctf.so"
>> -else
>> -  LIBCTF="../libctf/.libs/libctf.a"
>> -  CTF_DEPS="$LIBCTF"
>> -fi
>>  if test "${enable_libctf}" = yes; then
>>    AC_DEFINE(ENABLE_LIBCTF, 1, [Handle .ctf type-info sections])
>> +  LIBCTF="../libctf/libctf.la"
>> +  CTF_DEPS="../libctf/libctf.la"
>>  else
>>    LIBCTF=
>>    CTF_DEPS=
>> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
>> index e2cda30b95a..63d57581d13 100644
>> --- a/gdb/testsuite/lib/gdb.exp
>> +++ b/gdb/testsuite/lib/gdb.exp
>> @@ -154,7 +154,7 @@ global GDB_DATA_DIRECTORY
>>  global inferior_spawn_id
>>  
>>  if [info exists TOOL_EXECUTABLE] {
>> -    set GDB $TOOL_EXECUTABLE
>> +    set GDB "libtool --mode=execute $TOOL_EXECUTABLE"
>
> Can you explain why this is needed?  My understanding is that
> TOOL_EXECUTABLE can be used to point to an arbitrary gdb executable to
> test.  If it's a gdb that has been "make install"ed, then libtool
> doesn't have to be involved.  If pointing to a gdb executable in its
> build directory, then "gdb" will be a libtool script that sets all the
> right runtime library paths.  The only moment I can imagine this would
> be useful would be if TOOL_EXECUTABLE consists of a wrapper around
> gdb, e.g. "valgrind /path/to/build/gdb".

Hm, I actually changed that to see if it would help with the failing
tests, since these (related to forking etc) may have been sensible to
the fact the libtool wrapper script introduced an additional process in
the chain.  I agree with you, will remove this thunk from the next
version of the patch.

Thanks for the review!

Re: [PATCH V2] gdb: link executables with libtool

[Jose E. Marchesi]

Hi Simon.

>> NOTE4: Now the installed shared object libbfd.so is used by gdb
>>        if binutils is installed with --enable-shared.
>
> Do you mean bfd instead of binutils?  Or do you mean binutils in the
> sense that bfd is part of binutils?

Sorry I failed to answer this: I mean the second.