From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 145D03858C36 for ; Sat, 5 Nov 2022 16:38:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 145D03858C36 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca Received: from [10.0.0.11] (unknown [217.28.27.60]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id E6FAD1E0D3; Sat, 5 Nov 2022 12:38:31 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1667666312; bh=xfiImYeZSCbAQIhlq6W5xt5SSUZq+rGCIvItjFa+omI=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=AnwPnZyMNl3aooThnj9hKLRS7mIqEqty6KhXs8mmautZA7RIsV5HtuKvDMFvTAkId ug3Rj17jcl5GX17Z2lB3cd2YZLuS9nP2UDgxadUd6XmIwX8iLK/qf2IiFDaNGW+CJt C88OIx3P+LB891Bm5225e+Ox/SuHcExxKEY4Gs7Q= Message-ID: <242f99e5-2c41-155a-3bd9-e0c96dea28f5@simark.ca> Date: Sat, 5 Nov 2022 12:38:30 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.4.1 Subject: Re: [PATCH V2] gdb: link executables with libtool Content-Language: en-US To: "Jose E. Marchesi" , gdb-patches@sourceware.org Cc: indu.bhagat@oracle.com, elena.zannoni@oracle.com References: <20221105140233.1197237-1-jose.marchesi@oracle.com> From: Simon Marchi In-Reply-To: <20221105140233.1197237-1-jose.marchesi@oracle.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_SHORT,NICE_REPLY_A,SPF_HELO_PASS,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 . 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 > > * 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 > + > + * 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 > > * src-release.sh: Add "-r " 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