This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
[PATCH] VAX/BFD: Fix GOT/PLT handling for non-preemptible symbols
- From: "Maciej W. Rozycki" <macro at linux-mips dot org>
- To: binutils at sourceware dot org
- Cc: Matt Thomas <matt at netbsd dot org>, Jan-Benedict Glaw <jbglaw at lug-owl dot de>
- Date: Mon, 20 May 2013 02:59:06 +0100 (BST)
- Subject: [PATCH] VAX/BFD: Fix GOT/PLT handling for non-preemptible symbols
Hi,
This change fixes a problem with GOT handling for non-preemptible symbols
(ones that have a non-default export class aka visibility). Being
non-preemptible such symbols do not require a GOT entry, because they
always resolve locally (there's a small complication around protected
function symbols where pointer equality is required; as it is handled
automagically I'm only mentioning it for completeness, the VAX backend
doesn't have to do anything special here), the VAX architecture provides
PC-relative addressing for all instructions involving address
calculations, and the PC-relative addressing mode supports an immediate
offset that spans the whole address space.
We currently handle this partially, GOT space is correctly not allocated
for them, however they are taken into account in GOT offset calculation.
As a result symbols that actually make it into the GOT may get assigned
slots beyond the end of the GOT space. The symptom is an assertion
failure on R_VAX_GOT32 calculation in elf_vax_relocate_section, at line
1464:
BFD_ASSERT (off < sgot->size);
-- because off, the offset calculated for the symbol being handled, is
outside the .got section. A test case that arranges for this to happen is
included.
The change below corrects the problem by removing all the non-default
export class checks for GOT and PLT relocs scattered throughout the
backend and relying on elf_vax_instantiate_got_entries setting the
respective refcounts to -1 for non-preemptible symbols (the
SYMBOL_REFERENCES_LOCAL macro takes care of both non-preemptible symbols
and these which have been forced local with a version script).
Consequently code is removed from elf_vax_relocate_section that allocates
GOT entries for forced-local symbols, there's no such need and it only
hits run-time performance. Complementing code is removed from
elf_vax_finish_dynamic_symbol that applied R_VAX_RELATIVE relocations to
such GOT entries.
The change makes some minor clean-ups as a side effect too. In
particular a check for h being NULL is removed from the R_VAX_GOT32 case
in elf_vax_check_relocs, because we already assert there it's not, and
contrariwise an assertion that h is NULL is removed from
elf_vax_relocate_section for R_VAX_GOT32, because we break out of the case
a few lines above if h is NULL (and similarly for off being unequal to
-1). And some conditions are consolidated or moved around for a better
code structure.
No regressions for vax-linux or vax-netbsdelf; the test case included
passes with the change applied. Questions or comments? Otherwise OK to
apply?
2013-05-20 Maciej W. Rozycki <macro@linux-mips.org>
bfd/
* elf32-vax.c (elf_vax_check_relocs) <R_VAX_GOT32, R_VAX_PLT32>:
Don't check symbol visibility here. Remove a check already
asserted for.
(elf_vax_instantiate_got_entries): Use SYMBOL_REFERENCES_LOCAL
instead of individual checks.
(elf_vax_relocate_section) <R_VAX_GOT32, R_VAX_PLT32>: Only
check the offset to decide if produce a GOT or PLT entry.
Remove redundant assertions. Remove code to produce GOT entries
for local symbols. Remove a duplicate comment and add a comment
on GOT relocations.
(elf_vax_finish_dynamic_symbol): Remove code to produce RELATIVE
dynamic relocs.
ld/testsuite/
* ld-vax-elf/got-local-exe.xd: New test.
* ld-vax-elf/got-local-lib.xd: New test.
* ld-vax-elf/got-local-aux.s: New test source.
* ld-vax-elf/got-local-def.s: New test source.
* ld-vax-elf/got-local-ref.s: New test source.
* ld-vax-elf/vax-elf.exp: Run the new tests.
Maciej
binutils-2.23.52-20130506-vax-got-local.patch
Index: binutils/bfd/elf32-vax.c
===================================================================
--- binutils.orig/bfd/elf32-vax.c
+++ binutils/bfd/elf32-vax.c
@@ -595,14 +595,12 @@ elf_vax_check_relocs (bfd *abfd, struct
{
case R_VAX_GOT32:
BFD_ASSERT (h != NULL);
- if (h->forced_local
- || h == elf_hash_table (info)->hgot
- || h == elf_hash_table (info)->hplt)
- break;
/* If this is a local symbol, we resolve it directly without
creating a global offset table entry. */
- if (h == NULL || ELF_ST_VISIBILITY (h->other) != STV_DEFAULT)
+ if (h->forced_local
+ || h == elf_hash_table (info)->hgot
+ || h == elf_hash_table (info)->hplt)
break;
/* This symbol requires a global offset table entry. */
@@ -672,11 +670,11 @@ elf_vax_check_relocs (bfd *abfd, struct
never referenced by a dynamic object, in which case we
don't need to generate a procedure linkage table entry
after all. */
+ BFD_ASSERT (h != NULL);
/* If this is a local symbol, we resolve it directly without
creating a procedure linkage table entry. */
- BFD_ASSERT (h != NULL);
- if (ELF_ST_VISIBILITY (h->other) != STV_DEFAULT || h->forced_local)
+ if (h->forced_local)
break;
h->needs_plt = 1;
@@ -1321,16 +1319,13 @@ elf_vax_instantiate_got_entries (struct
sgot = bfd_get_linker_section (dynobj, ".got");
srelgot = bfd_get_linker_section (dynobj, ".rela.got");
- if ((info->shared && info->symbolic)
- || h->forced_local)
+ if (SYMBOL_REFERENCES_LOCAL (info, h))
{
h->got.refcount = -1;
h->plt.refcount = -1;
}
else if (h->got.refcount > 0)
{
- bfd_boolean dyn;
-
/* Make sure this symbol is output as a dynamic symbol. */
if (h->dynindx == -1)
{
@@ -1338,15 +1333,9 @@ elf_vax_instantiate_got_entries (struct
return FALSE;
}
- dyn = elf_hash_table (info)->dynamic_sections_created;
/* Allocate space in the .got and .rela.got sections. */
- if (ELF_ST_VISIBILITY (h->other) == STV_DEFAULT
- && (info->shared
- || WILL_CALL_FINISH_DYNAMIC_SYMBOL (dyn, 0, h)))
- {
- sgot->size += 4;
- srelgot->size += sizeof (Elf32_External_Rela);
- }
+ sgot->size += 4;
+ srelgot->size += sizeof (Elf32_External_Rela);
}
return TRUE;
@@ -1471,17 +1460,14 @@ elf_vax_relocate_section (bfd *output_bf
case R_VAX_GOT32:
/* Relocation is to the address of the entry for this symbol
in the global offset table. */
+
+ /* Resolve a GOTxx reloc against a local symbol directly,
+ without using the global offset table. */
if (h == NULL
- || ELF_ST_VISIBILITY (h->other) != STV_DEFAULT
- || h->got.offset == (bfd_vma) -1
- || h->forced_local)
+ || h->got.offset == (bfd_vma) -1)
break;
- /* Relocation is the offset of the entry for this symbol in
- the global offset table. */
-
{
- bfd_boolean dyn;
bfd_vma off;
if (sgot == NULL)
@@ -1490,37 +1476,10 @@ elf_vax_relocate_section (bfd *output_bf
BFD_ASSERT (sgot != NULL);
}
- BFD_ASSERT (h != NULL);
off = h->got.offset;
- BFD_ASSERT (off != (bfd_vma) -1);
BFD_ASSERT (off < sgot->size);
- dyn = elf_hash_table (info)->dynamic_sections_created;
- if (! WILL_CALL_FINISH_DYNAMIC_SYMBOL (dyn, info->shared, h)
- || (info->shared
- && SYMBOL_REFERENCES_LOCAL (info, h)))
- {
- /* The symbol was forced to be local
- because of a version file.. We must initialize
- this entry in the global offset table. Since
- the offset must always be a multiple of 4, we
- use the least significant bit to record whether
- we have initialized it already.
-
- When doing a dynamic link, we create a .rela.got
- relocation entry to initialize the value. This
- is done in the finish_dynamic_symbol routine. */
- if ((off & 1) != 0)
- off &= ~1;
- else
- {
- bfd_put_32 (output_bfd, relocation + rel->r_addend,
- sgot->contents + off);
- h->got.offset |= 1;
- }
- } else {
- bfd_put_32 (output_bfd, rel->r_addend, sgot->contents + off);
- }
+ bfd_put_32 (output_bfd, rel->r_addend, sgot->contents + off);
relocation = sgot->output_offset + off;
/* The GOT relocation uses the addend. */
@@ -1547,19 +1506,9 @@ elf_vax_relocate_section (bfd *output_bf
/* Resolve a PLTxx reloc against a local symbol directly,
without using the procedure linkage table. */
if (h == NULL
- || ELF_ST_VISIBILITY (h->other) != STV_DEFAULT
- || h->forced_local)
+ || h->plt.offset == (bfd_vma) -1)
break;
- if (h->plt.offset == (bfd_vma) -1
- || !elf_hash_table (info)->dynamic_sections_created)
- {
- /* We didn't make a PLT entry for this symbol. This
- happens when statically linking PIC code, or when
- using -Bsymbolic. */
- break;
- }
-
if (splt == NULL)
{
splt = bfd_get_linker_section (dynobj, ".plt");
@@ -1894,25 +1843,10 @@ elf_vax_finish_dynamic_symbol (bfd *outp
rela.r_offset = (sgot->output_section->vma
+ sgot->output_offset
- + (h->got.offset &~ 1));
-
- /* If the symbol was forced to be local because of a version file
- locally we just want to emit a RELATIVE reloc. The entry in
- the global offset table will already have been initialized in
- the relocate_section function. */
- if (info->shared
- && h->dynindx == -1
- && h->def_regular)
- {
- rela.r_info = ELF32_R_INFO (0, R_VAX_RELATIVE);
- }
- else
- {
- rela.r_info = ELF32_R_INFO (h->dynindx, R_VAX_GLOB_DAT);
- }
+ + h->got.offset);
+ rela.r_info = ELF32_R_INFO (h->dynindx, R_VAX_GLOB_DAT);
rela.r_addend = bfd_get_signed_32 (output_bfd,
- (sgot->contents
- + (h->got.offset & ~1)));
+ sgot->contents + h->got.offset);
loc = srela->contents;
loc += srela->reloc_count++ * sizeof (Elf32_External_Rela);
Index: binutils/ld/testsuite/ld-vax-elf/got-local-aux.s
===================================================================
--- /dev/null
+++ binutils/ld/testsuite/ld-vax-elf/got-local-aux.s
@@ -0,0 +1,5 @@
+ .globl baz
+ .type baz, @object
+baz:
+ .byte 0
+ .size baz, . - baz
Index: binutils/ld/testsuite/ld-vax-elf/got-local-def.s
===================================================================
--- /dev/null
+++ binutils/ld/testsuite/ld-vax-elf/got-local-def.s
@@ -0,0 +1,12 @@
+ .data
+ .globl bar_hidden
+ .type bar_hidden, @object
+ .hidden bar_hidden
+bar_hidden:
+ .byte 0
+ .size bar_hidden, . - bar_hidden
+ .globl bar_visible
+ .type bar_visible, @object
+bar_visible:
+ .byte 0
+ .size bar_visible, . - bar_visible
Index: binutils/ld/testsuite/ld-vax-elf/got-local-exe.xd
===================================================================
--- /dev/null
+++ binutils/ld/testsuite/ld-vax-elf/got-local-exe.xd
@@ -0,0 +1,4 @@
+# Make sure there's only one GOT entry, for the symbol defined externally.
+Hex dump of section '\.got':
+ 0x[0-9a-f]+ ........ 00000000 00000000 00000000 .*
+ 0x[0-9a-f]+ 00000000 .*
Index: binutils/ld/testsuite/ld-vax-elf/got-local-lib.xd
===================================================================
--- /dev/null
+++ binutils/ld/testsuite/ld-vax-elf/got-local-lib.xd
@@ -0,0 +1,4 @@
+# Make sure there are only two GOT entries, for the preemptible symbols.
+Hex dump of section '\.got':
+ 0x[0-9a-f]+ ........ 00000000 00000000 00000000 .*
+ 0x[0-9a-f]+ 00000000 00000000 .*
Index: binutils/ld/testsuite/ld-vax-elf/got-local-ref.s
===================================================================
--- /dev/null
+++ binutils/ld/testsuite/ld-vax-elf/got-local-ref.s
@@ -0,0 +1,10 @@
+ .text
+ .globl foo
+ .type foo, @function
+foo:
+ .word 0
+ movab bar_hidden, %r0
+ movab bar_visible, %r1
+ movab baz, %r2
+ ret
+ .size foo, . - foo
Index: binutils/ld/testsuite/ld-vax-elf/vax-elf.exp
===================================================================
--- binutils.orig/ld/testsuite/ld-vax-elf/vax-elf.exp
+++ binutils/ld/testsuite/ld-vax-elf/vax-elf.exp
@@ -48,3 +48,37 @@ run_ld_link_tests [list \
{ plt-local.s } \
{ { objdump -d plt-local.dd } } \
"plt-local"]]
+
+# Global offset table tests. Make sure hidden symbols do not get GOT
+# assignments.
+run_ld_link_tests [list \
+ [list "GOT test (auxiliary shared library)" \
+ "-shared" "" \
+ "-k" \
+ { got-local-aux.s } \
+ {} \
+ "got-local-aux.so"] \
+ [list "GOT test (object 1)" \
+ "-r" "" \
+ "-k" \
+ { got-local-ref.s } \
+ {} \
+ "got-local-ref-r.o"] \
+ [list "GOT test (object 2)" \
+ "-r" "" \
+ "-k" \
+ { got-local-def.s } \
+ {} \
+ "got-local-def-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" "" \
+ "" \
+ {} \
+ { { readelf "-x .got" got-local-exe.xd } } \
+ "got-local-exe"] \
+ [list "GOT test (shared library)" \
+ "-shared tmpdir/got-local-aux.so tmpdir/got-local-ref-r.o tmpdir/got-local-def-r.o" "" \
+ "" \
+ {} \
+ { { readelf "-x .got" got-local-lib.xd } } \
+ "got-local-lib.so"]]