This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [BFD][PR21703]Override the new defined symbol with the old normal symbol when --allow-multiple-definition is provided


Hi Alan,

Thanks for the review! I updated the patch with your suggestions.

Allow the "ld -r" test for targets without shared library support.
Relax pr21703-r.sd and pr21703-shared.sd with additional "#..." pattern lines.

All targets checked again in cross environment. No failures.

Regards,
Renlin

ld/ChangeLog:

2017-10-26  Renlin Li  <renlin.li@arm.com>

	* testsuite/ld-elf/elf.exp: xfail pr21703 tests on specific targets.
	Only run shared lib test for targets which support it.
	* testsuite/ld-elf/pr21703-r.sd: Adjust the expected output.
	* testsuite/ld-elf/pr21703-shared.sd: Likewise.



On 25/10/17 22:12, Alan Modra wrote:
On Wed, Oct 25, 2017 at 09:24:21PM +0100, Renlin Li wrote:
Hi,

This patch fixes the test cases failures on target which don't support share library.
Additional, targets which use _bfd_generic_link_add_symbols instead of
bfd_elf_link_add_symbols are expected to fail on those tests.

all the failed targets mentioned by Alan have been checked. All fixed.
However, all the checks are done on cross-target environment as I don't
native environment except arm, aarch64 and x86.

Is it Okay to commit?

No, there isn't any reason why the ld -r test shouldn't run on targets
without shared lib support.  If I take a look at the first target in
my list that failed the ld -r test, avr-elf, I see the readelf -s
output is

Symbol table '.symtab' contains 17 entries:
    Num:    Value  Size Type    Bind   Vis      Ndx Name
      0: 00000000     0 NOTYPE  LOCAL  DEFAULT  UND
      1: 00000000     0 SECTION LOCAL  DEFAULT    1
      2: 00000000     0 SECTION LOCAL  DEFAULT    2
      3: 00000000     0 SECTION LOCAL  DEFAULT    3
      4: 0000ffa0     0 NOTYPE  GLOBAL DEFAULT  ABS __DATA_REGION_LENGTH__
      5: 00000000     4 FUNC    GLOBAL DEFAULT    1 foo@FOO
      6: 00000400     0 NOTYPE  GLOBAL DEFAULT  ABS __LOCK_REGION_LENGTH__
      7: 00000004    32 FUNC    GLOBAL DEFAULT    1 foo1
      8: 00000400     0 NOTYPE  GLOBAL DEFAULT  ABS __SIGNATURE_REGION_LENGTH
      9: 00000004    32 FUNC    GLOBAL DEFAULT    1 foo@@FOO1
     10: 00000034     8 FUNC    GLOBAL DEFAULT    1 bar1
     11: 00000400     0 NOTYPE  GLOBAL DEFAULT  ABS __USER_SIGNATURE_REGION_L
     12: 00000000     4 FUNC    GLOBAL DEFAULT    1 foo
     13: 00010000     0 NOTYPE  GLOBAL DEFAULT  ABS __EEPROM_REGION_LENGTH__
     14: 00000024    16 FUNC    GLOBAL DEFAULT    1 bar
     15: 00000400     0 NOTYPE  GLOBAL DEFAULT  ABS __FUSE_REGION_LENGTH__
     16: 00002000     0 NOTYPE  GLOBAL DEFAULT  ABS __TEXT_REGION_LENGTH__

The foo and bar symbols are actually the right size, but we have extra
symbols.  pr21703-r.sd ought to be modified to make this pass, by
inserting "#..." lines between the symbols you need to check.

diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp
index eac29e0..1fe38eb 100644
--- a/ld/testsuite/ld-elf/elf.exp
+++ b/ld/testsuite/ld-elf/elf.exp
@@ -70,17 +70,23 @@ run_ld_link_tests [list \
 	{symbol3w.s} {} "symbol3w.a" ] \
 ]
 
-run_ld_link_tests [list \
-    [list "PR ld/21703" \
+# These targets use _bfd_generic_link_add_symbols instead of
+# bfd_elf_link_add_symbols
+setup_xfail "d30v-*-*" "dlx-*-*" "pj-*-*"
+run_ld_link_tests {
+      {"PR ld/21703"
 	"--allow-multiple-definition tmpdir/pr21703-1.o tmpdir/pr21703-2.o" "" "" \
-	{pr21703-1.s pr21703-2.s} {{readelf {-s} pr21703.sd}} "pr21703" ] \
-    [list "PR ld/21703 -r" \
+	{pr21703-1.s pr21703-2.s} {{readelf {-s} pr21703.sd}} "pr21703" }
+}
+
+# These targets use _bfd_generic_link_add_symbols instead of
+# bfd_elf_link_add_symbols
+setup_xfail "d30v-*-*" "dlx-*-*" "pj-*-*"
+run_ld_link_tests {
+      {"PR ld/21703 -r"
 	"-r --allow-multiple-definition tmpdir/pr21703-3.o tmpdir/pr21703-4.o" "" "" \
-	{pr21703-3.s pr21703-4.s} {{readelf {-s} pr21703-r.sd}} "pr21703.o" ] \
-    [list "PR ld/21703 shared" \
-	"-shared --allow-multiple-definition --version-script pr21703.ver tmpdir/pr21703-3.o tmpdir/pr21703-4.o" "" "" \
-	{pr21703-3.s pr21703-4.s} {{readelf {--dyn-syms} pr21703-shared.sd}} "pr21703.so" ] \
-]
+	{pr21703-3.s pr21703-4.s} {{readelf {-s} pr21703-r.sd}} "pr21703.o" }
+}
 
 if { [check_shared_lib_support] } then {
     run_ld_link_tests {
@@ -98,6 +104,17 @@ if { [check_shared_lib_support] } then {
 	    "--no-dynamic-linker tmpdir/pr14170a.o tmpdir/pr14170.so" "" $hpux \
 	     {pr14170c.s} { } "pr14170" ] \
     ]
+
+    # These targets use _bfd_generic_link_add_symbols instead of
+    # bfd_elf_link_add_symbols
+    setup_xfail "d30v-*-*" "dlx-*-*" "pj-*-*"
+    setup_xfail "tic6x-*-*" "hppa64-*-*"
+    run_ld_link_tests {
+	{"PR ld/21703 shared"
+	 "-shared --allow-multiple-definition --version-script pr21703.ver tmpdir/pr21703-3.o tmpdir/pr21703-4.o" "" "" \
+	 {pr21703-3.s pr21703-4.s} {{readelf {--dyn-syms} pr21703-shared.sd}} "pr21703.so" }
+    }
+
 }
 
 # Only run these tests on targets that support creating shared libraries.
diff --git a/ld/testsuite/ld-elf/pr21703-r.sd b/ld/testsuite/ld-elf/pr21703-r.sd
index 6758088..fba756d 100644
--- a/ld/testsuite/ld-elf/pr21703-r.sd
+++ b/ld/testsuite/ld-elf/pr21703-r.sd
@@ -1,9 +1,14 @@
 Symbol table '.symtab' contains .* entries:
 #...
 .*: [0-9a-fA-F]* +4 +FUNC +GLOBAL +DEFAULT +. foo@FOO
+#...
 .*: [0-9a-fA-F]* +32 +FUNC +GLOBAL +DEFAULT +. foo1
+#...
 .*: [0-9a-fA-F]* +32 +FUNC +GLOBAL +DEFAULT +. foo@@FOO1
+#...
 .*: [0-9a-fA-F]* +8 +FUNC +GLOBAL +DEFAULT +. bar1
+#...
 .*: [0-9a-fA-F]* +4 +FUNC +GLOBAL +DEFAULT +. foo
+#...
 .*: [0-9a-fA-F]* +16 +FUNC +GLOBAL +DEFAULT +. bar
 #pass
diff --git a/ld/testsuite/ld-elf/pr21703-shared.sd b/ld/testsuite/ld-elf/pr21703-shared.sd
index 9b6b1b9..9ca541c 100644
--- a/ld/testsuite/ld-elf/pr21703-shared.sd
+++ b/ld/testsuite/ld-elf/pr21703-shared.sd
@@ -2,7 +2,10 @@ Symbol table '\.dynsym' contains [0-9]+ entries:
  +Num: +Value +Size Type +Bind +Vis +Ndx Name
 #...
  +[0-9]+: +[0-9a-f]+ +4 +FUNC +GLOBAL +DEFAULT +[0-9] +foo@FOO
+#...
  +[0-9]+: +[0-9a-f]+ +0 +OBJECT +GLOBAL +DEFAULT +ABS +FOO1
+#...
  +[0-9]+: +[0-9a-f]+ +32 +FUNC +GLOBAL +DEFAULT +[0-9] +foo@@FOO1
+#...
  +[0-9]+: +[0-9a-f]+ +0 +OBJECT +GLOBAL +DEFAULT +ABS +FOO
 #...

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]