VAX/BFD: Do not warn about GOT addend mismatches if no GOT entry is made

Message ID alpine.LFD.2.21.2012031252020.656242@eddie.linux-mips.org
State New
Headers show
Series
  • VAX/BFD: Do not warn about GOT addend mismatches if no GOT entry is made
Related show

Commit Message

Maciej W. Rozycki Dec. 3, 2020, 1:32 p.m.
Match the condition used in `elf_vax_instantiate_got_entries' for the 
creation of GOT entries in the processing of R_VAX_GOT32 relocations in 
`elf_vax_check_relocs', removing incorrect warnings about a GOT addend 
mismatch like:

./ld-new: tmpdir/got-local-ref-off-r.o: warning: GOT addend of 1 to `bar_hidden' does not match previous GOT addend of 0
./ld-new: tmpdir/got-local-ref-off-r.o: warning: GOT addend of -1 to `bar_hidden' does not match previous GOT addend of 0

and corresponding failures with the test cases newly added here:

FAIL: GOT test (executable hidden reference with offset)
FAIL: GOT test (executable visible reference with offset)

for symbols that are considered local for reasons other than having been 
forced local with a version script, which is usually the ELF visibility. 
Correct code is produced regardless, but the warning breaks `-Werror' 
compilation and may upset people regardless.

Interestingly this shows with executable links only, because in shared 
library links code from `elf_link_add_object_symbols' triggers:

	    /* If the symbol already has a dynamic index, but
	       visibility says it should not be visible, turn it into
	       a local symbol.  */
	    switch (ELF_ST_VISIBILITY (h->other))
	      {
	      case STV_INTERNAL:
	      case STV_HIDDEN:
		(*bed->elf_backend_hide_symbol) (info, h, TRUE);
		dynsym = FALSE;
		break;
	      }

that sets `h->forced_local' like with a version script.

Add suitable test cases including disassembly to verify correct code has 
been produced where no warnings have been issued, and that warnings do 
get issued where necessary.  Do not verify (broken) code produced in the 
latter case; we should probably make the warning an error, or preferably 
actually start supporting GOT references with different addends as they
appear feasible with explicitly relocated GOT that we use.

	bfd/
	* elf32-vax.c (elf_vax_check_relocs) <R_VAX_GOT32>: Use 
	SYMBOL_REFERENCES_LOCAL rather than `h->forced_local' to check 
	whether the symbol referred is local or not.

	ld/
	* testsuite/ld-vax-elf/got-local-exe-off-hidden.dd: New test 
	dump.
	* testsuite/ld-vax-elf/got-local-exe-off-visible.dd: New test 
	dump.
	* testsuite/ld-vax-elf/got-local-lib-off-hidden.dd: New test 
	dump.
	* testsuite/ld-vax-elf/got-local-lib-off-visible.ed: New test 
	dump.
	* testsuite/ld-vax-elf/got-local-off-external.ed: New test dump.
	* testsuite/ld-vax-elf/got-local-exe-off.xd: New test dump.
	* testsuite/ld-vax-elf/got-local-lib-off.xd: New test dump.
	* testsuite/ld-vax-elf/got-local.ld: New test linker script.
	* testsuite/ld-vax-elf/got-local-aux-off.s: New test source.
	* testsuite/ld-vax-elf/got-local-def-off.s: New test source.
	* testsuite/ld-vax-elf/got-local-ref-off-external.s: New test 
	source.
	* testsuite/ld-vax-elf/got-local-ref-off-hidden.s: New test 
	source.
	* testsuite/ld-vax-elf/got-local-ref-off-visible.s: New test 
	source.
	* testsuite/ld-vax-elf/vax-elf.exp: Run the new tests.
---
 bfd/elf32-vax.c                                      |    2 
 ld/testsuite/ld-vax-elf/got-local-aux-off.s          |    5 +
 ld/testsuite/ld-vax-elf/got-local-def-off.s          |   12 ++
 ld/testsuite/ld-vax-elf/got-local-exe-off-hidden.dd  |   17 +++
 ld/testsuite/ld-vax-elf/got-local-exe-off-visible.dd |   17 +++
 ld/testsuite/ld-vax-elf/got-local-exe-off.xd         |    3 
 ld/testsuite/ld-vax-elf/got-local-lib-off-hidden.dd  |   17 +++
 ld/testsuite/ld-vax-elf/got-local-lib-off-visible.ed |    2 
 ld/testsuite/ld-vax-elf/got-local-lib-off.xd         |    3 
 ld/testsuite/ld-vax-elf/got-local-off-external.ed    |    2 
 ld/testsuite/ld-vax-elf/got-local-ref-off-external.s |   12 ++
 ld/testsuite/ld-vax-elf/got-local-ref-off-hidden.s   |   12 ++
 ld/testsuite/ld-vax-elf/got-local-ref-off-visible.s  |   12 ++
 ld/testsuite/ld-vax-elf/got-local.ld                 |   17 +++
 ld/testsuite/ld-vax-elf/vax-elf.exp                  |   85 ++++++++++++++++++-
 15 files changed, 215 insertions(+), 3 deletions(-)

binutils-vax-bfd-got32-local.diff

Comments

Jan-Benedict Glaw Dec. 4, 2020, 6:43 p.m. | #1
Hi Maciej!

On Thu, 2020-12-03 13:32:39 +0000, Maciej W. Rozycki <macro@linux-mips.org> wrote:
> 	bfd/

> 	* elf32-vax.c (elf_vax_check_relocs) <R_VAX_GOT32>: Use 

> 	SYMBOL_REFERENCES_LOCAL rather than `h->forced_local' to check 

> 	whether the symbol referred is local or not.

> 

> 	ld/

> 	* testsuite/ld-vax-elf/got-local-exe-off-hidden.dd: New test 

> 	dump.

> 	* testsuite/ld-vax-elf/got-local-exe-off-visible.dd: New test 

> 	dump.

> 	* testsuite/ld-vax-elf/got-local-lib-off-hidden.dd: New test 

> 	dump.

> 	* testsuite/ld-vax-elf/got-local-lib-off-visible.ed: New test 

> 	dump.

> 	* testsuite/ld-vax-elf/got-local-off-external.ed: New test dump.

> 	* testsuite/ld-vax-elf/got-local-exe-off.xd: New test dump.

> 	* testsuite/ld-vax-elf/got-local-lib-off.xd: New test dump.

> 	* testsuite/ld-vax-elf/got-local.ld: New test linker script.

> 	* testsuite/ld-vax-elf/got-local-aux-off.s: New test source.

> 	* testsuite/ld-vax-elf/got-local-def-off.s: New test source.

> 	* testsuite/ld-vax-elf/got-local-ref-off-external.s: New test 

> 	source.

> 	* testsuite/ld-vax-elf/got-local-ref-off-hidden.s: New test 

> 	source.

> 	* testsuite/ld-vax-elf/got-local-ref-off-visible.s: New test 

> 	source.

> 	* testsuite/ld-vax-elf/vax-elf.exp: Run the new tests.


Good to go!

Reminds me that I'd like a pure all-opcode test case as well as one
for all addressing modes.

Thanks a lot, JBG

--
Maciej W. Rozycki Dec. 4, 2020, 8:40 p.m. | #2
On Fri, 4 Dec 2020, Jan-Benedict Glaw wrote:

> > 	bfd/

> > 	* elf32-vax.c (elf_vax_check_relocs) <R_VAX_GOT32>: Use 

> > 	SYMBOL_REFERENCES_LOCAL rather than `h->forced_local' to check 

> > 	whether the symbol referred is local or not.

> > 

> > 	ld/

> > 	* testsuite/ld-vax-elf/got-local-exe-off-hidden.dd: New test 

> > 	dump.

> > 	* testsuite/ld-vax-elf/got-local-exe-off-visible.dd: New test 

> > 	dump.

> > 	* testsuite/ld-vax-elf/got-local-lib-off-hidden.dd: New test 

> > 	dump.

> > 	* testsuite/ld-vax-elf/got-local-lib-off-visible.ed: New test 

> > 	dump.

> > 	* testsuite/ld-vax-elf/got-local-off-external.ed: New test dump.

> > 	* testsuite/ld-vax-elf/got-local-exe-off.xd: New test dump.

> > 	* testsuite/ld-vax-elf/got-local-lib-off.xd: New test dump.

> > 	* testsuite/ld-vax-elf/got-local.ld: New test linker script.

> > 	* testsuite/ld-vax-elf/got-local-aux-off.s: New test source.

> > 	* testsuite/ld-vax-elf/got-local-def-off.s: New test source.

> > 	* testsuite/ld-vax-elf/got-local-ref-off-external.s: New test 

> > 	source.

> > 	* testsuite/ld-vax-elf/got-local-ref-off-hidden.s: New test 

> > 	source.

> > 	* testsuite/ld-vax-elf/got-local-ref-off-visible.s: New test 

> > 	source.

> > 	* testsuite/ld-vax-elf/vax-elf.exp: Run the new tests.

> 

> Good to go!


 Committed now, with a minor tweak to the description to match the current 
version of the relevant test case.

> Reminds me that I'd like a pure all-opcode test case as well as one

> for all addressing modes.


 Exactly my point:

On Mon, 30 Nov 2020, Maciej W. Rozycki wrote:

>  NB I wish we had better target-specific coverage across the testsuites, 

> like e.g. the MIPS port does.  We don't even have GAS coverage for basic 

> machine code generation, i.e. opcode and operand encoding!


 Thanks for your review!

  Maciej

Patch

Index: binutils-gdb/bfd/elf32-vax.c
===================================================================
--- binutils-gdb.orig/bfd/elf32-vax.c
+++ binutils-gdb/bfd/elf32-vax.c
@@ -598,7 +598,7 @@  elf_vax_check_relocs (bfd *abfd, struct
 
 	  /* If this is a local symbol, we resolve it directly without
 	     creating a global offset table entry.  */
-	  if (h->forced_local
+	  if (SYMBOL_REFERENCES_LOCAL (info, h)
 	      || h == elf_hash_table (info)->hgot
 	      || h == elf_hash_table (info)->hplt)
 	    break;
Index: binutils-gdb/ld/testsuite/ld-vax-elf/got-local-aux-off.s
===================================================================
--- /dev/null
+++ binutils-gdb/ld/testsuite/ld-vax-elf/got-local-aux-off.s
@@ -0,0 +1,5 @@ 
+	.globl	baz
+	.type	baz, @object
+baz:
+	.byte	0, 1, 2
+	.size	baz, . - baz
Index: binutils-gdb/ld/testsuite/ld-vax-elf/got-local-def-off.s
===================================================================
--- /dev/null
+++ binutils-gdb/ld/testsuite/ld-vax-elf/got-local-def-off.s
@@ -0,0 +1,12 @@ 
+	.data
+	.globl	bar_hidden
+	.type	bar_hidden, @object
+	.hidden	bar_hidden
+bar_hidden:
+	.byte	0, 1, 2
+	.size	bar_hidden, . - bar_hidden
+	.globl	bar_visible
+	.type	bar_visible, @object
+bar_visible:
+	.byte	0, 1, 2
+	.size	bar_visible, . - bar_visible
Index: binutils-gdb/ld/testsuite/ld-vax-elf/got-local-exe-off-hidden.dd
===================================================================
--- /dev/null
+++ binutils-gdb/ld/testsuite/ld-vax-elf/got-local-exe-off-hidden.dd
@@ -0,0 +1,17 @@ 
+.*: +file format .*vax.*
+
+Disassembly of section \.text:
+
+00000000 <foo>:
+   0:	00 00       	\.word 0x0000 # Entry mask: < >
+   2:	9e ef f8 1f 	movab 2000 <bar_hidden>,r0
+   6:	00 00 50 
+   9:	9e ef f2 1f 	movab 2001 <bar_hidden\+0x1>,r0
+   d:	00 00 50 
+  10:	9e ef ec 1f 	movab 2002 <bar_hidden\+0x2>,r0
+  14:	00 00 50 
+  17:	9e ef e6 1f 	movab 2003 <bar_visible>,r1
+  1b:	00 00 51 
+  1e:	9e ff dc 0f 	movab \*1000 <baz>,r2
+  22:	00 00 52 
+  25:	04          	ret
Index: binutils-gdb/ld/testsuite/ld-vax-elf/got-local-exe-off-visible.dd
===================================================================
--- /dev/null
+++ binutils-gdb/ld/testsuite/ld-vax-elf/got-local-exe-off-visible.dd
@@ -0,0 +1,17 @@ 
+.*: +file format .*vax.*
+
+Disassembly of section \.text:
+
+00000000 <foo>:
+   0:	00 00       	\.word 0x0000 # Entry mask: < >
+   2:	9e ef f8 1f 	movab 2000 <bar_hidden>,r0
+   6:	00 00 50 
+   9:	9e ef f4 1f 	movab 2003 <bar_visible>,r1
+   d:	00 00 51 
+  10:	9e ef ee 1f 	movab 2004 <bar_visible\+0x1>,r1
+  14:	00 00 51 
+  17:	9e ef e8 1f 	movab 2005 <bar_visible\+0x2>,r1
+  1b:	00 00 51 
+  1e:	9e ff dc 0f 	movab \*1000 <baz>,r2
+  22:	00 00 52 
+  25:	04          	ret
Index: binutils-gdb/ld/testsuite/ld-vax-elf/got-local-exe-off.xd
===================================================================
--- /dev/null
+++ binutils-gdb/ld/testsuite/ld-vax-elf/got-local-exe-off.xd
@@ -0,0 +1,3 @@ 
+# Make sure there's only one GOT entry, for the symbol defined externally.
+Hex dump of section '\.got':
+  0x00001000 00000000                            .*
Index: binutils-gdb/ld/testsuite/ld-vax-elf/got-local-lib-off-hidden.dd
===================================================================
--- /dev/null
+++ binutils-gdb/ld/testsuite/ld-vax-elf/got-local-lib-off-hidden.dd
@@ -0,0 +1,17 @@ 
+.*: +file format .*vax.*
+
+Disassembly of section \.text:
+
+00000000 <foo>:
+   0:	00 00       	\.word 0x0000 # Entry mask: < >
+   2:	9e ef f8 1f 	movab 2000 <bar_hidden>,r0
+   6:	00 00 50 
+   9:	9e ef f2 1f 	movab 2001 <bar_hidden\+0x1>,r0
+   d:	00 00 50 
+  10:	9e ef ec 1f 	movab 2002 <bar_hidden\+0x2>,r0
+  14:	00 00 50 
+  17:	9e ff e7 0f 	movab \*1004 <bar_visible-0xfff>,r1
+  1b:	00 00 51 
+  1e:	9e ff dc 0f 	movab \*1000 <baz>,r2
+  22:	00 00 52 
+  25:	04          	ret
Index: binutils-gdb/ld/testsuite/ld-vax-elf/got-local-lib-off-visible.ed
===================================================================
--- /dev/null
+++ binutils-gdb/ld/testsuite/ld-vax-elf/got-local-lib-off-visible.ed
@@ -0,0 +1,2 @@ 
+\A[^\n]*\.o: warning: GOT addend of 1 to `bar_visible' does not match previous GOT addend of 0
+[^\n]*\.o: warning: GOT addend of 2 to `bar_visible' does not match previous GOT addend of 0\Z
Index: binutils-gdb/ld/testsuite/ld-vax-elf/got-local-lib-off.xd
===================================================================
--- /dev/null
+++ binutils-gdb/ld/testsuite/ld-vax-elf/got-local-lib-off.xd
@@ -0,0 +1,3 @@ 
+# Make sure there are only two GOT entries, for the preemptible symbols.
+Hex dump of section '\.got':
+  0x00001000 00000000 00000000                   .*
Index: binutils-gdb/ld/testsuite/ld-vax-elf/got-local-off-external.ed
===================================================================
--- /dev/null
+++ binutils-gdb/ld/testsuite/ld-vax-elf/got-local-off-external.ed
@@ -0,0 +1,2 @@ 
+\A[^\n]*\.o: warning: GOT addend of 1 to `baz' does not match previous GOT addend of 0
+[^\n]*\.o: warning: GOT addend of 2 to `baz' does not match previous GOT addend of 0\Z
Index: binutils-gdb/ld/testsuite/ld-vax-elf/got-local-ref-off-external.s
===================================================================
--- /dev/null
+++ binutils-gdb/ld/testsuite/ld-vax-elf/got-local-ref-off-external.s
@@ -0,0 +1,12 @@ 
+	.text
+	.globl	foo
+	.type	foo, @function
+foo:
+	.word	0
+	movab	bar_hidden, %r0
+	movab	bar_visible, %r1
+	movab	baz, %r2
+	movab	baz + 1, %r2
+	movab	baz + 2, %r2
+	ret
+	.size	foo, . - foo
Index: binutils-gdb/ld/testsuite/ld-vax-elf/got-local-ref-off-hidden.s
===================================================================
--- /dev/null
+++ binutils-gdb/ld/testsuite/ld-vax-elf/got-local-ref-off-hidden.s
@@ -0,0 +1,12 @@ 
+	.text
+	.globl	foo
+	.type	foo, @function
+foo:
+	.word	0
+	movab	bar_hidden, %r0
+	movab	bar_hidden + 1, %r0
+	movab	bar_hidden + 2, %r0
+	movab	bar_visible, %r1
+	movab	baz, %r2
+	ret
+	.size	foo, . - foo
Index: binutils-gdb/ld/testsuite/ld-vax-elf/got-local-ref-off-visible.s
===================================================================
--- /dev/null
+++ binutils-gdb/ld/testsuite/ld-vax-elf/got-local-ref-off-visible.s
@@ -0,0 +1,12 @@ 
+	.text
+	.globl	foo
+	.type	foo, @function
+foo:
+	.word	0
+	movab	bar_hidden, %r0
+	movab	bar_visible, %r1
+	movab	bar_visible + 1, %r1
+	movab	bar_visible + 2, %r1
+	movab	baz, %r2
+	ret
+	.size	foo, . - foo
Index: binutils-gdb/ld/testsuite/ld-vax-elf/got-local.ld
===================================================================
--- /dev/null
+++ binutils-gdb/ld/testsuite/ld-vax-elf/got-local.ld
@@ -0,0 +1,17 @@ 
+SECTIONS
+{
+  .text : { *(.text) }
+  .hash : { *(.hash) }
+  .dynsym : { *(.dynsym) }
+  .dynstr : { *(.dynstr) }
+  .rela.dyn : { *(.rela.*) }
+  . = ALIGN (0x1000);
+  .got : { *(.got) }
+  . = ALIGN (0x1000);
+  .data : { *(.data) }
+  .dynamic : { *(.dynamic) }
+  .symtab : { *(.symtab) }
+  .strtab : { *(.strtab) }
+  .shstrtab : { *(.shstrtab) }
+  /DISCARD/ : { *(*) }
+}
Index: binutils-gdb/ld/testsuite/ld-vax-elf/vax-elf.exp
===================================================================
--- binutils-gdb.orig/ld/testsuite/ld-vax-elf/vax-elf.exp
+++ binutils-gdb/ld/testsuite/ld-vax-elf/vax-elf.exp
@@ -51,7 +51,7 @@  run_ld_link_tests [list \
 	  "plt-local"]]
 
 # Global offset table tests.  Make sure hidden symbols do not get GOT
-# assignments.
+# assignments.  Also verify offset references.
 run_ld_link_tests [list \
     [list "GOT test (auxiliary shared library)" \
 	  "-shared" "" \
@@ -59,18 +59,48 @@  run_ld_link_tests [list \
 	  { got-local-aux.s } \
 	  {} \
 	  "got-local-aux.so"] \
+    [list "GOT test (auxiliary shared library for offsets)" \
+	  "-shared" "" \
+	  "-k" \
+	  { got-local-aux-off.s } \
+	  {} \
+	  "got-local-aux-off.so"] \
     [list "GOT test (object 1)" \
 	  "-r" "" \
 	  "-k" \
 	  { got-local-ref.s } \
 	  {} \
 	  "got-local-ref-r.o"] \
+    [list "GOT test (object 1 hidden reference with offset)" \
+	  "-r" "" \
+	  "-k" \
+	  { got-local-ref-off-hidden.s } \
+	  {} \
+	  "got-local-ref-off-hidden-r.o"] \
+    [list "GOT test (object 1 visible reference with offset)" \
+	  "-r" "" \
+	  "-k" \
+	  { got-local-ref-off-visible.s } \
+	  {} \
+	  "got-local-ref-off-visible-r.o"] \
+    [list "GOT test (object 1 external reference with offset)" \
+	  "-r" "" \
+	  "-k" \
+	  { got-local-ref-off-external.s } \
+	  {} \
+	  "got-local-ref-off-external-r.o"] \
     [list "GOT test (object 2)" \
 	  "-r" "" \
 	  "-k" \
 	  { got-local-def.s } \
 	  {} \
 	  "got-local-def-r.o"] \
+    [list "GOT test (object 2 for offsets)" \
+	  "-r" "" \
+	  "-k" \
+	  { got-local-def-off.s } \
+	  {} \
+	  "got-local-def-off-r.o"] \
     [list "GOT test (executable)" \
 	  "-e 0 tmpdir/got-local-aux.so tmpdir/got-local-ref-r.o \
 	   tmpdir/got-local-def-r.o" "" \
@@ -84,7 +114,58 @@  run_ld_link_tests [list \
 	  "" \
 	  {} \
 	  { { readelf "-x .got" got-local-lib.xd } } \
-	  "got-local-lib.so"]]
+	  "got-local-lib.so"] \
+    [list "GOT test (executable hidden reference with offset)" \
+	  "-e 0 -T got-local.ld tmpdir/got-local-aux-off.so \
+	   tmpdir/got-local-ref-off-hidden-r.o \
+	   tmpdir/got-local-def-off-r.o" "" \
+	  "" \
+	  {} \
+	  { { readelf "-x .got" got-local-exe-off.xd } \
+	    { objdump -d got-local-exe-off-hidden.dd } } \
+	  "got-local-exe-off-hidden"] \
+    [list "GOT test (shared library hidden reference with offset)" \
+	  "-shared -T got-local.ld tmpdir/got-local-aux-off.so \
+	   tmpdir/got-local-ref-off-hidden-r.o \
+	   tmpdir/got-local-def-off-r.o" "" \
+	  "" \
+	  {} \
+	  { { readelf "-x .got" got-local-lib-off.xd } \
+	    { objdump -d got-local-lib-off-hidden.dd } } \
+	  "got-local-lib-off-hidden.so"] \
+    [list "GOT test (executable visible reference with offset)" \
+	  "-e 0 -T got-local.ld tmpdir/got-local-aux-off.so \
+	   tmpdir/got-local-ref-off-visible-r.o \
+	   tmpdir/got-local-def-off-r.o" "" \
+	  "" \
+	  {} \
+	  { { readelf "-x .got" got-local-exe-off.xd } \
+	    { objdump -d got-local-exe-off-visible.dd } } \
+	  "got-local-exe-off-visible"] \
+    [list "GOT test (shared library visible reference with offset)" \
+	  "-shared -T got-local.ld tmpdir/got-local-aux-off.so \
+	   tmpdir/got-local-ref-off-visible-r.o \
+	   tmpdir/got-local-def-off-r.o" "" \
+	  "" \
+	  {} \
+	  { { ld got-local-lib-off-visible.ed } } \
+	  "got-local-lib-off-visible.so"] \
+    [list "GOT test (executable external reference with offset)" \
+	  "-e 0 -T got-local.ld tmpdir/got-local-aux-off.so \
+	   tmpdir/got-local-ref-off-external-r.o \
+	   tmpdir/got-local-def-off-r.o" "" \
+	  "" \
+	  {} \
+	  { { ld got-local-off-external.ed } } \
+	  "got-local-exe-off-external"] \
+    [list "GOT test (shared library external reference with offset)" \
+	  "-shared -T got-local.ld tmpdir/got-local-aux-off.so \
+	   tmpdir/got-local-ref-off-external-r.o \
+	   tmpdir/got-local-def-off-r.o" "" \
+	  "" \
+	  {} \
+	  { { ld got-local-off-external.ed } } \
+	  "got-local-lib-off-external.so"]]
 
 # Export class relocation tests.
 run_ld_link_tests [list \