From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id a1XXIrgZ1l9TLgAAWB0awg (envelope-from ) for ; Sun, 13 Dec 2020 08:40:08 -0500 Received: by simark.ca (Postfix, from userid 112) id 81C641F0AA; Sun, 13 Dec 2020 08:40:08 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.0 required=5.0 tests=MAILING_LIST_MULTI, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 174AC1E99A for ; Sun, 13 Dec 2020 08:40:07 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A6A6C385800A; Sun, 13 Dec 2020 13:40:06 +0000 (GMT) Received: from rock.gnat.com (rock.gnat.com [IPv6:2620:20:4000:0:a9e:1ff:fe9b:1d1]) by sourceware.org (Postfix) with ESMTP id 3AE5E385800A for ; Sun, 13 Dec 2020 13:40:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 3AE5E385800A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=brobecker@adacore.com Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 1A0835606E; Sun, 13 Dec 2020 08:40:04 -0500 (EST) X-Virus-Scanned: Debian amavisd-new at gnat.com Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id md5rnWKE8REL; Sun, 13 Dec 2020 08:40:04 -0500 (EST) Received: from float.home (localhost.localdomain [127.0.0.1]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPS id AF0BC116CE5; Sun, 13 Dec 2020 08:40:03 -0500 (EST) Received: by float.home (Postfix, from userid 1000) id C5B21A1880; Sun, 13 Dec 2020 17:39:58 +0400 (+04) Date: Sun, 13 Dec 2020 17:39:58 +0400 From: Joel Brobecker To: Tom de Vries Subject: Re: [PATCH][gdb/testsuite] Don't pass -fPIC to gdb_compile_shlib Message-ID: <20201213133958.GC366101@adacore.com> References: <20201207122016.GA14320@delia> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201207122016.GA14320@delia> X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" Hi Tom, > When running test-case gdb.base/info-shared.exp, I see in gdb.log: > ... > Executing on host: \ > gcc ... -fPIC -fpic -c -o info-shared-solib1.c.o info-shared-solib1.c > ... > > The -fPIC comes from the test-case: > ... > if { [gdb_compile_shlib $srcfile_lib1 $binfile_lib1 \ > [list additional_flags=-fPIC]] != "" } { > ... > but the -fpic, which overrides the -fPIC comes from gdb_compile_shlib. > > The proc gdb_compile_shlib adds the -fpic or similar dependent on platform > and compiler. However, in some cases it doesn't add anything, which is > probably why all those test-case pass -fPIC. > > Fix this by removing -fPIC from all the calls to gdb_compile_shlib, and > ensuring that gdb_compile_shlib takes care of adding it, if required. > > Tested on x86_64-linux. > > Any comments? I think it makes sense in general. We should rely on a function to tell us which "pic" or "PIC" (or whichever) is needed to generate a shared librariy, so as to avoid duplicating all the code making that decision across those testcases. One slightly open question is whether any of those testcases that you are modifying actually specifically needs the "pic" that they specified. I can speak of gdb.ada/catch_ex_std.exp, which I think I wrote, as well as gdb.base/dso2dso.exp: The testcase does not need a specific PIC so this patch is a nice improvement. I think the risk is low for the other testcases. I looked at the changes in gdb_compile_shlib, and for me, they looked OK, but this is the kind of change that perhaps could use a third pair of eyes. FWIW, I did double-check that for MinGW/cygwin, we use -fPIC. > > [gdb/testsuite] Don't pass -fPIC to gdb_compile_shlib > > gdb/testsuite/ChangeLog: > > 2020-12-07 Tom de Vries > > * lib/gdb.exp (gdb_compile_shlib): Make sure it's not necessary to > pass -fPIC. > * gdb.ada/catch_ex_std.exp: Don't pass -fPIC to gdb_compile_shlib. > * gdb.base/break-probes.exp: Same. > * gdb.base/ctxobj.exp: Same. > * gdb.base/dso2dso.exp: Same. > * gdb.base/global-var-nested-by-dso.exp: Same. > * gdb.base/info-shared.exp: Same. > * gdb.base/jit-reader-simple.exp: Same. > * gdb.base/print-file-var.exp: Same. > * gdb.base/skip-solib.exp: Same. > * gdb.btrace/dlopen.exp: Same. > > --- > gdb/testsuite/gdb.ada/catch_ex_std.exp | 3 +-- > gdb/testsuite/gdb.base/break-probes.exp | 3 +-- > gdb/testsuite/gdb.base/ctxobj.exp | 4 ++-- > gdb/testsuite/gdb.base/dso2dso.exp | 4 ++-- > gdb/testsuite/gdb.base/global-var-nested-by-dso.exp | 4 ++-- > gdb/testsuite/gdb.base/info-shared.exp | 6 ++---- > gdb/testsuite/gdb.base/jit-reader-simple.exp | 2 +- > gdb/testsuite/gdb.base/print-file-var.exp | 2 +- > gdb/testsuite/gdb.base/skip-solib.exp | 2 +- > gdb/testsuite/gdb.btrace/dlopen.exp | 3 +-- > gdb/testsuite/lib/gdb.exp | 13 +++++++++---- > 11 files changed, 23 insertions(+), 23 deletions(-) > > diff --git a/gdb/testsuite/gdb.ada/catch_ex_std.exp b/gdb/testsuite/gdb.ada/catch_ex_std.exp > index c8acd70241..5ed0b4d172 100644 > --- a/gdb/testsuite/gdb.ada/catch_ex_std.exp > +++ b/gdb/testsuite/gdb.ada/catch_ex_std.exp > @@ -29,8 +29,7 @@ set sofile [standard_output_file libsome_package.so] > set outdir [file dirname $binfile] > > # Create the shared library. > -if {[gdb_compile_shlib $srcfile2 $sofile \ > - {ada debug additional_flags=-fPIC}] != ""} { > +if {[gdb_compile_shlib $srcfile2 $sofile {ada debug}] != ""} { > return -1 > } > > diff --git a/gdb/testsuite/gdb.base/break-probes.exp b/gdb/testsuite/gdb.base/break-probes.exp > index 08513ddbb2..96ef93d9f5 100644 > --- a/gdb/testsuite/gdb.base/break-probes.exp > +++ b/gdb/testsuite/gdb.base/break-probes.exp > @@ -30,8 +30,7 @@ if { [istarget "*bsd*"] } { > } > set probes_bp "dl_main" > > -if { [gdb_compile_shlib $srcfile_lib $binfile_lib \ > - [list additional_flags=-fPIC]] != "" } { > +if { [gdb_compile_shlib $srcfile_lib $binfile_lib {}] != "" } { > untested "failed to compile shared library" > return -1 > } > diff --git a/gdb/testsuite/gdb.base/ctxobj.exp b/gdb/testsuite/gdb.base/ctxobj.exp > index b8931bafb4..faf209cc0d 100644 > --- a/gdb/testsuite/gdb.base/ctxobj.exp > +++ b/gdb/testsuite/gdb.base/ctxobj.exp > @@ -33,10 +33,10 @@ set libsrc [list "${srcdir}/${subdir}/ctxobj-v.c" \ > set libobj1 [standard_output_file libctxobj1.so] > set libobj2 [standard_output_file libctxobj2.so] > > -set libobj1_opts { debug additional_flags=-fPIC > +set libobj1_opts { debug > additional_flags=-DVERSION=104 > additional_flags=-DGET_VERSION=get_version_1 } > -set libobj2_opts { debug additional_flags=-fPIC > +set libobj2_opts { debug > additional_flags=-DVERSION=203 > additional_flags=-DGET_VERSION=get_version_2 } > > diff --git a/gdb/testsuite/gdb.base/dso2dso.exp b/gdb/testsuite/gdb.base/dso2dso.exp > index 7d8ea7057c..4c0b27955d 100644 > --- a/gdb/testsuite/gdb.base/dso2dso.exp > +++ b/gdb/testsuite/gdb.base/dso2dso.exp > @@ -40,13 +40,13 @@ set srcfile_libdso1 $srcdir/$subdir/$libdso1.c > set binfile_libdso1 [standard_output_file $libdso1.so] > > if { [gdb_compile_shlib $srcfile_libdso2 $binfile_libdso2 \ > - [list debug additional_flags=-fPIC]] != "" } { > + [list debug]] != "" } { > untested "failed to compile shared library 2" > return -1 > } > > if { [gdb_compile_shlib $srcfile_libdso1 $binfile_libdso1 \ > - [list debug additional_flags=-fPIC]] != "" } { > + [list debug]] != "" } { > untested "failed to compile shared library 1" > return -1 > } > diff --git a/gdb/testsuite/gdb.base/global-var-nested-by-dso.exp b/gdb/testsuite/gdb.base/global-var-nested-by-dso.exp > index f88c5d094f..c277595015 100644 > --- a/gdb/testsuite/gdb.base/global-var-nested-by-dso.exp > +++ b/gdb/testsuite/gdb.base/global-var-nested-by-dso.exp > @@ -28,13 +28,13 @@ set srcfile_lib2 $srcdir/$subdir/$lib2name.c > set binfile_lib2 [standard_output_file $lib2name.so] > > if { [gdb_compile_shlib $srcfile_lib1 $binfile_lib1 \ > - [list debug additional_flags=-fPIC]] != "" } { > + [list debug]] != "" } { > untested "failed to compile shared library 1" > return -1 > } > > if { [gdb_compile_shlib $srcfile_lib2 $binfile_lib2 \ > - [list debug additional_flags=-fPIC]] != "" } { > + [list debug]] != "" } { > untested "failed to compile shared library 2" > return -1 > } > diff --git a/gdb/testsuite/gdb.base/info-shared.exp b/gdb/testsuite/gdb.base/info-shared.exp > index 9a54100f8a..7b8ce5b11e 100644 > --- a/gdb/testsuite/gdb.base/info-shared.exp > +++ b/gdb/testsuite/gdb.base/info-shared.exp > @@ -29,14 +29,12 @@ set srcfile_lib2 $srcdir/$subdir/$lib2name.c > set binfile_lib2 [standard_output_file $lib2name.so] > set define2 -DSHLIB2_NAME=\"$binfile_lib2\" > > -if { [gdb_compile_shlib $srcfile_lib1 $binfile_lib1 \ > - [list additional_flags=-fPIC]] != "" } { > +if { [gdb_compile_shlib $srcfile_lib1 $binfile_lib1 {}] != "" } { > untested "failed to compile shared library 1" > return -1 > } > > -if { [gdb_compile_shlib $srcfile_lib2 $binfile_lib2 \ > - [list additional_flags=-fPIC]] != "" } { > +if { [gdb_compile_shlib $srcfile_lib2 $binfile_lib2 {}] != "" } { > untested "failed to compile shared library 2" > return -1 > } > diff --git a/gdb/testsuite/gdb.base/jit-reader-simple.exp b/gdb/testsuite/gdb.base/jit-reader-simple.exp > index a8f33c6d7a..99c4dcf732 100644 > --- a/gdb/testsuite/gdb.base/jit-reader-simple.exp > +++ b/gdb/testsuite/gdb.base/jit-reader-simple.exp > @@ -56,7 +56,7 @@ proc build_shared_jit {{options ""}} { > global testfile > global srcfile_lib binfile_lib binfile_lib2 > > - lappend options "debug additional_flags=-fPIC" > + lappend options "debug" > if { [gdb_compile_shlib $srcfile_lib $binfile_lib $options] != "" } { > return -1 > } > diff --git a/gdb/testsuite/gdb.base/print-file-var.exp b/gdb/testsuite/gdb.base/print-file-var.exp > index 62e5f230eb..219bedcde4 100644 > --- a/gdb/testsuite/gdb.base/print-file-var.exp > +++ b/gdb/testsuite/gdb.base/print-file-var.exp > @@ -42,7 +42,7 @@ proc test {hidden dlopen version_id_main lang} { > set libobj1 [standard_output_file ${lib1}$suffix.so] > set libobj2 [standard_output_file ${lib2}$suffix.so] > > - set lib_opts { debug additional_flags=-fPIC $lang } > + set lib_opts { debug $lang } > lappend lib_opts "additional_flags=-DHIDDEN=$hidden" > > if { [gdb_compile_shlib ${srcdir}/${subdir}/${lib1}.c \ > diff --git a/gdb/testsuite/gdb.base/skip-solib.exp b/gdb/testsuite/gdb.base/skip-solib.exp > index 528410a0d5..8ecba70aa0 100644 > --- a/gdb/testsuite/gdb.base/skip-solib.exp > +++ b/gdb/testsuite/gdb.base/skip-solib.exp > @@ -39,7 +39,7 @@ set binfile_lib [standard_output_file ${libname}.so] > # the main program. > # > > -if {[gdb_compile_shlib ${srcdir}/${subdir}/${srcfile_lib} ${binfile_lib} [list debug additional_flags=-fPIC -Wl,-soname,${libname}.so]] != ""} { > +if {[gdb_compile_shlib ${srcdir}/${subdir}/${srcfile_lib} ${binfile_lib} [list debug -Wl,-soname,${libname}.so]] != ""} { > return -1 > } > > diff --git a/gdb/testsuite/gdb.btrace/dlopen.exp b/gdb/testsuite/gdb.btrace/dlopen.exp > index fc910a933a..6839b0f4f4 100644 > --- a/gdb/testsuite/gdb.btrace/dlopen.exp > +++ b/gdb/testsuite/gdb.btrace/dlopen.exp > @@ -31,8 +31,7 @@ set basename_lib dlopen-dso > set srcfile_lib $srcdir/$subdir/$basename_lib.c > set binfile_lib [standard_output_file $basename_lib.so] > > -if { [gdb_compile_shlib $srcfile_lib $binfile_lib \ > - [list additional_flags=-fPIC]] != "" } { > +if { [gdb_compile_shlib $srcfile_lib $binfile_lib {}] != "" } { > untested "failed to prepare shlib" > return -1 > } > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp > index e413bab93c..e35d236018 100644 > --- a/gdb/testsuite/lib/gdb.exp > +++ b/gdb/testsuite/lib/gdb.exp > @@ -4304,17 +4304,21 @@ proc gdb_compile_shlib {sources dest options} { > lappend obj_options "additional_flags=-qpic" > } > "clang-*" { > - if { !([istarget "*-*-cygwin*"] > - || [istarget "*-*-mingw*"]) } { > + if { [istarget "*-*-cygwin*"] > + || [istarget "*-*-mingw*"] } { > + lappend obj_options "additional_flags=-fPIC" > + } else { > lappend obj_options "additional_flags=-fpic" > } > } > "gcc-*" { > - if { !([istarget "powerpc*-*-aix*"] > + if { [istarget "powerpc*-*-aix*"] > || [istarget "rs6000*-*-aix*"] > || [istarget "*-*-cygwin*"] > || [istarget "*-*-mingw*"] > - || [istarget "*-*-pe*"]) } { > + || [istarget "*-*-pe*"] } { > + lappend obj_options "additional_flags=-fPIC" > + } else { > lappend obj_options "additional_flags=-fpic" > } > } > @@ -4323,6 +4327,7 @@ proc gdb_compile_shlib {sources dest options} { > } > default { > # don't know what the compiler is... > + lappend obj_options "additional_flags=-fPIC" > } > } > -- Joel