This is the mail archive of the binutils-cvs@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]

[binutils-gdb/binutils-2_28-branch] PR gas/20649: MIPS: Fix GOT16/LO16 reloc pairing with comdat sections

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=65060a78866f374e25f4668d12efc783235d19d1

commit 65060a78866f374e25f4668d12efc783235d19d1
Author: Maciej W. Rozycki <macro@imgtec.com>
Date:   Wed Jan 18 18:18:21 2017 +0000

    PR gas/20649: MIPS: Fix GOT16/LO16 reloc pairing with comdat sections
    
    Correct a regression from commit 8614eeee67f9 ("Traditional MIPS
    patches"), <https://sourceware.org/ml/binutils/2000-07/msg00018.html>,
    which caused symbols in linkonce or what is these days known as comdat
    sections to be treated as external for the purpose of PIC relocation
    generation even if their binding remains STB_LOCAL.  This in turn
    disabled GOT16/LO16 relocation pairing with references to such symbols,
    as no complementing LO16 relocation is expected for external GOT16
    references in the o32 ABI, which ultimately leads to link errors, e.g.:
    
    ld: comdat-reloc.o: Can't find matching LO16 reloc against `foo' for R_MIPS_GOT16 at 0x24 in section `.text.bar[bar]'
    
    as with the LD test case included with this change.
    
    Revert the special case for symbols in comdat sections then, making code
    actually match `adjust_reloc_syms' as indicated in its explanatory
    comment, and adjust calling code accordingly.  Also bring back the
    corresponding description of what now is `s_is_linkonce', lost with
    commit 5f0fe04bc550 ("Improved MIPS16/MIPS32 code intermixing for
    gas."), <https://www.sourceware.org/ml/binutils/2006-07/msg00039.html>.
    
    	gas/
    	PR gas/20649
    	* config/tc-mips.c (pic_need_relax): Don't check for linkonce
    	symbols, remove the `segtype' parameter.
    	(mips_frob_file, md_estimate_size_before_relax): Adjust
    	accordingly.
    	(s_is_linkonce): Add an explanatory comment.
    	* testsuite/gas/mips/comdat-reloc.d: New test.
    	* testsuite/gas/mips/comdat-reloc.s: New test source.
    	* testsuite/gas/mips/mips.exp: Run the new test.
    
    	ld/
    	PR gas/20649
    	* testsuite/ld-mips-elf/mips-elf.exp: Add PIC comdat GOT16/LO16
    	relocation pairing link test.
    
    (cherry picked from commit 9e009953a54bfbf79d83f37797f846c923aeea43)

Diff:
---
 gas/ChangeLog                         | 12 +++++++++++
 gas/config/tc-mips.c                  | 11 +++++-----
 gas/testsuite/gas/mips/comdat-reloc.d | 31 ++++++++++++++++++++++++++++
 gas/testsuite/gas/mips/comdat-reloc.s | 38 +++++++++++++++++++++++++++++++++++
 gas/testsuite/gas/mips/mips.exp       |  2 ++
 ld/ChangeLog                          |  6 ++++++
 ld/testsuite/ld-mips-elf/mips-elf.exp |  7 +++++++
 7 files changed, 102 insertions(+), 5 deletions(-)

diff --git a/gas/ChangeLog b/gas/ChangeLog
index 8f3cf3e..bc406af 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,15 @@
+2017-01-18  Maciej W. Rozycki  <macro@imgtec.com>
+
+	PR gas/20649
+	* config/tc-mips.c (pic_need_relax): Don't check for linkonce
+	symbols, remove the `segtype' parameter.
+	(mips_frob_file, md_estimate_size_before_relax): Adjust
+	accordingly.
+	(s_is_linkonce): Add an explanatory comment.
+	* testsuite/gas/mips/comdat-reloc.d: New test.
+	* testsuite/gas/mips/comdat-reloc.s: New test source.
+	* testsuite/gas/mips/mips.exp: Run the new test.
+
 2017-01-18  Bernhard Rosenkranzer  <bero@lindev.ch>
 
 	PR 21059
diff --git a/gas/config/tc-mips.c b/gas/config/tc-mips.c
index 79958c8..1487e73 100644
--- a/gas/config/tc-mips.c
+++ b/gas/config/tc-mips.c
@@ -1353,7 +1353,7 @@ static void s_mips_stab (int);
 static void s_mips_weakext (int);
 static void s_mips_file (int);
 static void s_mips_loc (int);
-static bfd_boolean pic_need_relax (symbolS *, asection *);
+static bfd_boolean pic_need_relax (symbolS *);
 static int relaxed_branch_length (fragS *, asection *, int);
 static int relaxed_micromips_16bit_branch_length (fragS *, asection *, int);
 static int relaxed_micromips_32bit_branch_length (fragS *, asection *, int);
@@ -4267,6 +4267,8 @@ mips_move_text_labels (void)
   mips_move_labels (seg_info (now_seg)->label_list, TRUE);
 }
 
+/* Duplicate the test for LINK_ONCE sections as in `adjust_reloc_syms'.  */
+
 static bfd_boolean
 s_is_linkonce (symbolS *sym, segT from_seg)
 {
@@ -14895,7 +14897,7 @@ mips_frob_file (void)
 	 constants; we'll report an error for those later.  */
       if (got16_reloc_p (l->fixp->fx_r_type)
 	  && !(l->fixp->fx_addsy
-	       && pic_need_relax (l->fixp->fx_addsy, l->seg)))
+	       && pic_need_relax (l->fixp->fx_addsy)))
 	continue;
 
       /* Check quickly whether the next fixup happens to be a matching %lo.  */
@@ -17115,7 +17117,7 @@ nopic_need_relax (symbolS *sym, int before_relaxing)
 /* Return true if the given symbol should be considered local for SVR4 PIC.  */
 
 static bfd_boolean
-pic_need_relax (symbolS *sym, asection *segtype)
+pic_need_relax (symbolS *sym)
 {
   asection *symsec;
 
@@ -17140,7 +17142,6 @@ pic_need_relax (symbolS *sym, asection *segtype)
   return (!bfd_is_und_section (symsec)
 	  && !bfd_is_abs_section (symsec)
 	  && !bfd_is_com_section (symsec)
-	  && !s_is_linkonce (sym, segtype)
 	  /* A global or weak symbol is treated as external.  */
 	  && (!S_IS_WEAK (sym) && !S_IS_EXTERNAL (sym)));
 }
@@ -17579,7 +17580,7 @@ md_estimate_size_before_relax (fragS *fragp, asection *segtype)
   if (mips_pic == NO_PIC)
     change = nopic_need_relax (fragp->fr_symbol, 0);
   else if (mips_pic == SVR4_PIC)
-    change = pic_need_relax (fragp->fr_symbol, segtype);
+    change = pic_need_relax (fragp->fr_symbol);
   else if (mips_pic == VXWORKS_PIC)
     /* For vxworks, GOT16 relocations never have a corresponding LO16.  */
     change = 0;
diff --git a/gas/testsuite/gas/mips/comdat-reloc.d b/gas/testsuite/gas/mips/comdat-reloc.d
new file mode 100644
index 0000000..12d092a
--- /dev/null
+++ b/gas/testsuite/gas/mips/comdat-reloc.d
@@ -0,0 +1,31 @@
+#readelf: -gr
+#name: MIPS ELF o32 PIC comdat GOT16/LO16 relocation pairing
+#as: -32 -mno-pdr
+
+# Make sure the orphan GOT16 relocation is paired with LO16 for a local
+# symbol in a comdat section, i.e. rather than this:
+#
+# 00000014  00000509 R_MIPS_GOT16      00000000   foo
+# 00000020  00000506 R_MIPS_LO16       00000000   foo
+# 0000001c  00000509 R_MIPS_GOT16      00000000   foo
+#
+# we have this:
+#
+# 00000014  00000509 R_MIPS_GOT16      00000000   foo
+# 00000024  00000509 R_MIPS_GOT16      00000000   foo
+# 0000001c  00000506 R_MIPS_LO16       00000000   foo
+
+#...
+COMDAT group section \[.....\] `\.group' \[bar\] contains .+ sections:
+   \[Index\]    Name
+   \[.....\]   \.text\.foo
+   \[.....\]   \.text\.bar
+#...
+Relocation section '\.rel\.text\.bar' at offset .+ contains .+ entries:
+ Offset     Info    Type            Sym\.Value  Sym\. Name
+00000000  ......05 R_MIPS_HI16       00000000   _gp_disp
+00000004  ......06 R_MIPS_LO16       00000000   _gp_disp
+00000014  ......09 R_MIPS_GOT16      00000000   foo
+00000024  ......09 R_MIPS_GOT16      00000000   foo
+0000001c  ......06 R_MIPS_LO16       00000000   foo
+#pass
diff --git a/gas/testsuite/gas/mips/comdat-reloc.s b/gas/testsuite/gas/mips/comdat-reloc.s
new file mode 100644
index 0000000..be972bd
--- /dev/null
+++ b/gas/testsuite/gas/mips/comdat-reloc.s
@@ -0,0 +1,38 @@
+	.abicalls
+
+	.section	.text.foo, "axG", @progbits, bar, comdat
+	.align	2
+	.ent	foo
+	.type	foo, @function
+foo:
+	.frame	$sp, 0, $31
+	.mask	0x00000000, 0
+	.fmask	0x00000000, 0
+	jr	$31
+	.end	foo
+	.size	foo, . - foo
+
+	.section	.text.bar, "axG", @progbits, bar, comdat
+	.align	2
+	.globl	bar
+	.ent	bar
+	.type	bar, @function
+bar:
+	.frame	$sp, 0, $31
+	.mask	0x00000000, 0
+	.fmask	0x00000000, 0
+	.set	noreorder
+	.cpload	$25
+	.set	reorder
+	beqz	$4, 1f
+	.set	noreorder
+	lw	$2, %got(foo)($28)
+0:
+	jr	$31
+	 addiu	$2, $2, %lo(foo)
+1:
+	b	0b
+	 lw	$2, %got(foo)($28)
+	.set	reorder
+	.end	bar
+	.size	bar, . - bar
diff --git a/gas/testsuite/gas/mips/mips.exp b/gas/testsuite/gas/mips/mips.exp
index 6a1aa6c..106bd7b 100644
--- a/gas/testsuite/gas/mips/mips.exp
+++ b/gas/testsuite/gas/mips/mips.exp
@@ -1168,6 +1168,8 @@ if { [istarget mips*-*-vxworks*] } {
     }
     run_list_test_arches "elf-rel30" "-32" [mips_arch_list_all]
 
+    run_dump_test "comdat-reloc"
+
     run_dump_test "${tmips}mips${el}16-e"
     run_dump_test "${tmips}mips${el}16-f"
 
diff --git a/ld/ChangeLog b/ld/ChangeLog
index 479d678..080697e 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,9 @@
+2017-01-18  Maciej W. Rozycki  <macro@imgtec.com>
+
+	PR gas/20649
+	* testsuite/ld-mips-elf/mips-elf.exp: Add PIC comdat GOT16/LO16
+	relocation pairing link test.
+
 2017-01-18  Bernhard Rosenkranzer  <bero@lindev.ch>
 
 	PR 21059
diff --git a/ld/testsuite/ld-mips-elf/mips-elf.exp b/ld/testsuite/ld-mips-elf/mips-elf.exp
index 9946708..5639c84 100644
--- a/ld/testsuite/ld-mips-elf/mips-elf.exp
+++ b/ld/testsuite/ld-mips-elf/mips-elf.exp
@@ -573,6 +573,13 @@ if { $has_newabi } {
 }
 
 run_dump_test "reloc-local-overflow" [list [list ld $abi_ldflags(o32)]]
+run_ld_link_tests [list \
+    [list \
+	"MIPS link ELF o32 PIC comdat GOT16/LO16 relocation pairing" \
+	"$abi_ldflags(o32) -e bar" "" "$abi_asflags(o32) -mno-pdr" \
+	"../../../gas/testsuite/gas/mips/comdat-reloc.s" \
+	{} \
+	"comdat-reloc"]]
 
 if {$has_newabi && $linux_gnu} {
     run_dump_test "eh-frame1-n32"
Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]