[testsuite] Fix ld action in run_dump_test

Message ID CAKnkMGvabYzGTXh=yhokYx8mmm+dnHh7ct-4XTz5dWd92=dveg@mail.gmail.com
State New
Headers show
Series
  • [testsuite] Fix ld action in run_dump_test
Related show

Commit Message

Thomas Preudhomme Oct. 26, 2018, 1:31 p.m.
[Resending with proper subject tag]

On Fri, 26 Oct 2018 at 14:28, Thomas Preudhomme
<thomas.preudhomme@linaro.org> wrote:
>

> Hi,

>

> run_dump_test proposes an ld action but when trying to make use of it in

> a gas test it gave me some Tcl error. It turns out that it references

> the check_shared_lib_support procedure and ld_elf_shared_opt variable

> both only available in ld-lib.exp. I've thus moved the procedure in

> binutils-common.exp and defined the variable needed in the various

> default.exp of testsuite that seem to be using run_dump_test.

>

> Since check_shared_lib_support itself references the ld variable not

> defined in binutils-common I've defined it from LD in run_dump_test and

> fixed LD and LDFLAGS to be defined as expected by run_dump_test in the

> various default.exp of testsuite using run_dump_test.

>

> ChangeLog entries are as follows:

>

> *** binutils/ChangeLog ***

>

> 2018-10-26  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

>

> * testsuite/config/default.exp: Define LD, LDFLAGS and

> ld_elf_shared_opt.

> * testsuite/lib/binutils-common.exp (check_shared_lib_support): Moved

> from ld-lib.exp.

> (run_dump_test): Set ld to $LD.

>

> *** gas/ChangeLog ***

>

> 2018-10-26  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

>

> * testsuite/config/default.exp: Define LD, LDFLAGS and

> ld_elf_shared_opt.

>

> *** ld/ChangeLog ***

>

> 2018-10-26  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

>

> * testsuite/lib/ld-lib.exp (check_shared_lib_support): Moved to

> binutils-common.exp.

>

> Testing: Successfully ran tests using the ld action in gas testsuite. No

> regression in the testsuite when targeting arm-none-eabi.

>

> Is this ok for master?

>

> Best regards,

>

> Thomas

Comments

Alan Modra Oct. 29, 2018, 10:55 p.m. | #1
On Fri, 26 Oct 2018 at 14:28, Thomas Preudhomme <thomas.preudhomme@linaro.org> wrote:
> run_dump_test proposes an ld action but when trying to make use of it in

> a gas test it gave me some Tcl error.


I have to wonder why you are using ld in the gas testsuite (making a
test of the linker which should perhaps be moved to the ld testsuite)?

Anyway, patch is OK.

-- 
Alan Modra
Australia Development Lab, IBM
Thomas Preudhomme Nov. 1, 2018, 5:19 p.m. | #2
Hi Alan,

Sorry for the late reply. See
https://sourceware.org/ml/binutils/2018-10/msg00289.html. In this case
I'm testing that both GAS and objcopy are doing the right thing and it
made sense to have only one set of test to reduce duplication. It also
need to run objcopy twice (once to remove a section, once to check its
behavior without that section) and I don't think the ld tests allow
this, but the run_dump_test allow it if linking. I don't technically
need linking, it was just a convenient way to get objcopy to run
twice. I welcome any suggestion on how to do this in a better way. In
the meantime I've committed this patch as if the feature is there it
should be working IMO.

Best regards,

Thomas
On Mon, 29 Oct 2018 at 22:55, Alan Modra <amodra@gmail.com> wrote:
>

> On Fri, 26 Oct 2018 at 14:28, Thomas Preudhomme <thomas.preudhomme@linaro.org> wrote:

> > run_dump_test proposes an ld action but when trying to make use of it in

> > a gas test it gave me some Tcl error.

>

> I have to wonder why you are using ld in the gas testsuite (making a

> test of the linker which should perhaps be moved to the ld testsuite)?

>

> Anyway, patch is OK.

>

> --

> Alan Modra

> Australia Development Lab, IBM

Patch

From f95da8ba8149df5d79afc7cccc9db7d2e91db9f6 Mon Sep 17 00:00:00 2001
From: Thomas Preud'homme <thomas.preudhomme@linaro.org>
Date: Fri, 26 Oct 2018 11:43:33 +0100
Subject: [PATCH 1/2] [testsuite] Fix ld action in run_dump_test

Hi,

run_dump_test proposes an ld action but when trying to make use of it in
a gas test it gave me some Tcl error. It turns out that it references
the check_shared_lib_support procedure and ld_elf_shared_opt variable
both only available in ld-lib.exp. I've thus moved the procedure in
binutils-common.exp and defined the variable needed in the various
default.exp of testsuite that seem to be using run_dump_test.

Since check_shared_lib_support itself references the ld variable not
defined in binutils-common I've defined it from LD in run_dump_test and
fixed LD and LDFLAGS to be defined as expected by run_dump_test in the
various default.exp of testsuite using run_dump_test.

ChangeLog entries are as follows:

*** binutils/ChangeLog ***

2018-10-26  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

	* testsuite/config/default.exp: Define LD, LDFLAGS and
	ld_elf_shared_opt.
	* testsuite/lib/binutils-common.exp (check_shared_lib_support): Moved
	from ld-lib.exp.
	(run_dump_test): Set ld to $LD.

*** gas/ChangeLog ***

2018-10-26  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

	* testsuite/config/default.exp: Define LD, LDFLAGS and
	ld_elf_shared_opt.

*** ld/ChangeLog ***

2018-10-26  Thomas Preud'homme  <thomas.preudhomme@linaro.org>

	* testsuite/lib/ld-lib.exp (check_shared_lib_support): Moved to
	binutils-common.exp.

Testing: Successfully ran tests using the ld action in gas testsuite. No
regression in the testsuite when targeting arm-none-eabi.

Is this ok for master?

Best regards,

Thomas
---
 binutils/testsuite/config/default.exp      |  8 ++++++++
 binutils/testsuite/lib/binutils-common.exp | 19 +++++++++++++++++++
 gas/testsuite/config/default.exp           |  8 ++++++++
 ld/testsuite/lib/ld-lib.exp                | 17 -----------------
 4 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/binutils/testsuite/config/default.exp b/binutils/testsuite/config/default.exp
index 6c55be66f5..1bfe72e8c8 100644
--- a/binutils/testsuite/config/default.exp
+++ b/binutils/testsuite/config/default.exp
@@ -28,6 +28,14 @@  if ![info exists ASFLAGS] then {
     set ASFLAGS ""
 }
 
+if ![info exists LD] then {
+    set LD [findfile $base_dir/../ld/ld-new $base_dir/../ld/ld-new [transform ld]]
+}
+if ![info exists LDFLAGS] then {
+    set LDFLAGS ""
+}
+set ld_elf_shared_opt "-z norelro"
+
 if ![info exists NM] then {
     set NM [findfile $base_dir/nm-new $base_dir/nm-new [transform nm]]
 }
diff --git a/binutils/testsuite/lib/binutils-common.exp b/binutils/testsuite/lib/binutils-common.exp
index 08d57ae963..fcd2c8ee0c 100644
--- a/binutils/testsuite/lib/binutils-common.exp
+++ b/binutils/testsuite/lib/binutils-common.exp
@@ -248,6 +248,23 @@  proc is_bad_symtab {} {
     return 0;
 }
 
+# Returns true if -shared is supported on the target
+
+proc check_shared_lib_support { } {
+    global shared_available_saved
+    global ld
+
+    if {![info exists shared_available_saved]} {
+	set ld_output [remote_exec host $ld "-shared"]
+	if { [ string first "not supported" $ld_output ] >= 0 } {
+	    set shared_available_saved 0
+	} else {
+	    set shared_available_saved 1
+	}
+    }
+    return $shared_available_saved
+}
+
 # Compare two files line-by-line.  FILE_1 is the actual output and FILE_2
 # is the expected output.  Ignore blank lines in either file.
 #
@@ -1002,6 +1019,8 @@  proc run_dump_test { name {extra_options {}} } {
 	catch "exec rm -f $objfile" exec_output
 
 	set ld_extra_opt ""
+	global ld
+	set ld "$LD"
 	if { [is_elf_format] && [check_shared_lib_support] } {
 	    set ld_extra_opt "$ld_elf_shared_opt"
 	}
diff --git a/gas/testsuite/config/default.exp b/gas/testsuite/config/default.exp
index 888e90d178..b76c5baa44 100644
--- a/gas/testsuite/config/default.exp
+++ b/gas/testsuite/config/default.exp
@@ -26,6 +26,14 @@  if ![info exists ASFLAGS] then {
     set ASFLAGS ""
 }
 
+if ![info exists LD] then {
+    set LD [findfile $base_dir/../../ld/ld-new $base_dir/../../ld/ld-new [transform ld]]
+}
+if ![info exists LDFLAGS] then {
+    set LDFLAGS ""
+}
+set ld_elf_shared_opt "-z norelro"
+
 if ![info exists OBJDUMP] then {
     set OBJDUMP [findfile $base_dir/../../binutils/objdump \
 			  $base_dir/../../binutils/objdump \
diff --git a/ld/testsuite/lib/ld-lib.exp b/ld/testsuite/lib/ld-lib.exp
index d6453f1995..3fb1e58a0c 100644
--- a/ld/testsuite/lib/ld-lib.exp
+++ b/ld/testsuite/lib/ld-lib.exp
@@ -1069,23 +1069,6 @@  proc check_gc_sections_available { } {
     return $gc_sections_available_saved
 }
 
-# Returns true if -shared is supported on the target
-
-proc check_shared_lib_support { } {
-    global shared_available_saved
-    global ld
-
-    if {![info exists shared_available_saved]} {
-	set ld_output [remote_exec host $ld "-shared"]
-	if { [ string first "not supported" $ld_output ] >= 0 } {
-	    set shared_available_saved 0
-	} else {
-	    set shared_available_saved 1
-	}
-    }
-    return $shared_available_saved
-}
-
 # Return true if target uses genelf.em (assuming it is ELF).
 proc is_generic_elf { } {
     if { [istarget "d30v-*-*"]
-- 
2.19.1