This is the mail archive of the
binutils@sources.redhat.com
mailing list for the binutils project.
Re: mn10300 dynamic relocation clean up: remove dynamic PCREL32
- From: Alexandre Oliva <aoliva at redhat dot com>
- To: binutils at sources dot redhat dot com
- Date: 28 Jun 2004 04:55:10 -0300
- Subject: Re: mn10300 dynamic relocation clean up: remove dynamic PCREL32
- Organization: Red Hat Global Engineering Services Compiler Team
- References: <oreko1r9jo.fsf@livre.redhat.lsd.ic.unicamp.br>
Turned out the adoption of _bfd_elf_symbol_refs_local_p introduced
some ugly regressions in handling of common symbols, because it
doesn't handle them properly. The problem is that common symbols are
initially set up as bfd_link_hash_common, but then, when then linker
turns them into a definition, it doesn't set
ELF_LINK_HASH_DEF_REGULAR, which _bfd_elf_symbol_refs_local_p depends
on to tell whether a symbol is a definition.
This patch introduces ELF_LINK_HASH_COMMON, set for symbols read in as
common symbols. This bit remains set after the linker turns the
symbol into a definition, and is the new clean way to tell that the
symbol is a definition of a common symbol. Unfortunately, I had to
switch to a wider data type for elf_link_hash_flags.
After fixing _bfd_elf_symbol_refs_local_p, I switched to
SYMBOL_REFERENCES_LOCAL, that had the correct semantics for mn10300.
While at that, I also cleaned up an ugly bit in the FR-V code that
attempted to handle common symbols but didn't handle them correctly (a
globally-visible common symbol would be assumed to bind locally).
Ok to install?
Index: bfd/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
* elf-bfd.h (elf_link_hash_flags): Bump up from short to long.
(ELF_LINK_HASH_COMMON): New flag.
* elflink.c (_bfd_elf_symbol_refs_local_p): Handle common
definitions.
(elf_link_add_object_symbols): Set it for common symbols.
* elf-m10300.c: Use SYMBOL_REFERENCES_LOCAL instead of
_bfd_elf_symbol_refs_local_p.
* elf32-frv.c (FRVFDPIC_SYM_LOCAL): Remove hack for common
symbols.
Index: bfd/elf-bfd.h
===================================================================
RCS file: /cvs/uberbaum/./bfd/elf-bfd.h,v
retrieving revision 1.143
diff -u -p -r1.143 elf-bfd.h
--- bfd/elf-bfd.h 21 Jun 2004 14:45:41 -0000 1.143
+++ bfd/elf-bfd.h 28 Jun 2004 04:47:56 -0000
@@ -168,7 +168,7 @@ struct elf_link_hash_entry
unsigned char other;
/* Some flags; legal values follow. */
- unsigned short elf_link_hash_flags;
+ unsigned long elf_link_hash_flags;
/* Symbol is referenced by a non-shared object. */
#define ELF_LINK_HASH_REF_REGULAR 01
/* Symbol is defined by a non-shared object. */
@@ -203,6 +203,13 @@ struct elf_link_hash_entry
/* Symbol is referenced with a relocation where C/C++ pointer equality
matters. */
#define ELF_LINK_POINTER_EQUALITY_NEEDED 0100000
+ /* Symbol is, or originally was, a common definition. Necessary
+ because the linker clears all vestiges of common symbol
+ definitions, but doesn't set ELF_LINK_HASH_DEF_REGULAR. The way
+ to detect a common symbol that became defined is to test whether
+ the symbol is defined or defweak with HASH_REF_REGULAR and
+ HASH_COMMON. */
+#define ELF_LINK_HASH_COMMON 0200000
};
/* Will references to this symbol always reference the symbol
Index: bfd/elf-m10300.c
===================================================================
RCS file: /cvs/uberbaum/./bfd/elf-m10300.c,v
retrieving revision 1.55
diff -u -p -r1.55 elf-m10300.c
--- bfd/elf-m10300.c 27 Jun 2004 03:02:21 -0000 1.55
+++ bfd/elf-m10300.c 28 Jun 2004 04:47:59 -0000
@@ -1020,7 +1020,7 @@ mn10300_elf_final_link_relocate (howto,
if (info->shared
&& (input_section->flags & SEC_ALLOC) != 0
&& h != NULL
- && ! _bfd_elf_symbol_refs_local_p (h, info, 1))
+ && ! SYMBOL_REFERENCES_LOCAL (info, h))
return bfd_reloc_dangerous;
}
@@ -1079,7 +1079,7 @@ mn10300_elf_final_link_relocate (howto,
/* h->dynindx may be -1 if this symbol was marked to
become local. */
if (h == NULL
- || _bfd_elf_symbol_refs_local_p (h, info, 1))
+ || SYMBOL_REFERENCES_LOCAL (info, h))
{
relocate = TRUE;
outrel.r_info = ELF32_R_INFO (0, R_MN10300_RELATIVE);
@@ -1302,7 +1302,7 @@ mn10300_elf_final_link_relocate (howto,
BFD_ASSERT (off != (bfd_vma) -1);
if (! elf_hash_table (info)->dynamic_sections_created
- || _bfd_elf_symbol_refs_local_p (h, info, 1))
+ || SYMBOL_REFERENCES_LOCAL (info, h))
/* This is actually a static link, or it is a
-Bsymbolic link and the symbol is defined
locally, or the symbol was forced to be local
@@ -1461,9 +1461,9 @@ mn10300_elf_relocate_section (output_bfd
|| r_type == R_MN10300_GOT24
|| r_type == R_MN10300_GOT16)
&& elf_hash_table (info)->dynamic_sections_created
- && !_bfd_elf_symbol_refs_local_p (h, info, 1))
+ && !SYMBOL_REFERENCES_LOCAL (info, hh))
|| (r_type == R_MN10300_32
- && !_bfd_elf_symbol_refs_local_p (h, info, 1)
+ && !SYMBOL_REFERENCES_LOCAL (info, hh)
&& ((input_section->flags & SEC_ALLOC) != 0
/* DWARF will emit R_MN10300_32 relocations
in its sections against symbols defined
Index: bfd/elf32-frv.c
===================================================================
RCS file: /cvs/uberbaum/./bfd/elf32-frv.c,v
retrieving revision 1.26
diff -u -p -r1.26 elf32-frv.c
--- bfd/elf32-frv.c 24 Jun 2004 04:46:18 -0000 1.26
+++ bfd/elf32-frv.c 28 Jun 2004 04:48:01 -0000
@@ -661,17 +661,7 @@ frvfdpic_elf_link_hash_table_create (bfd
its function descriptor must be assigned by the dynamic linker. */
#define FRVFDPIC_SYM_LOCAL(INFO, H) \
(_bfd_elf_symbol_refs_local_p ((H), (INFO), 1) \
- || ! elf_hash_table (INFO)->dynamic_sections_created \
- || (/* The condition below is an ugly hack to get .scommon data to
- be regarded as local. For some reason the
- ELF_LINK_HASH_DEF_REGULAR bit is not set on such common
- symbols, and the SEC_IS_COMMON bit is not set any longer
- when we need to perform this test. Hopefully this
- approximation is good enough. */ \
- ((H)->root.type == bfd_link_hash_defined \
- || (H)->root.type == bfd_link_hash_defweak) \
- && (H)->root.u.def.section->output_section \
- && ((H)->root.u.def.section->flags & SEC_LINKER_CREATED)))
+ || ! elf_hash_table (INFO)->dynamic_sections_created)
#define FRVFDPIC_FUNCDESC_LOCAL(INFO, H) \
((H)->dynindx == -1 || ! elf_hash_table (INFO)->dynamic_sections_created)
Index: bfd/elflink.c
===================================================================
RCS file: /cvs/uberbaum/./bfd/elflink.c,v
retrieving revision 1.74
diff -u -p -r1.74 elflink.c
--- bfd/elflink.c 24 Jun 2004 04:46:22 -0000 1.74
+++ bfd/elflink.c 28 Jun 2004 04:48:06 -0000
@@ -2450,8 +2450,15 @@ _bfd_elf_symbol_refs_local_p (struct elf
return TRUE;
/* If we don't have a definition in a regular file, then we can't
- resolve locally. The sym is either undefined or dynamic. */
- if ((h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR) == 0)
+ resolve locally. The sym is either undefined or dynamic. Or a
+ common symbol turned into a local definition, so test that before
+ deciding it's not local. Note we shouldn't test for hash_common,
+ since we don't know beforehand whether they're going to become
+ definitions. */
+ if ((h->elf_link_hash_flags & ELF_LINK_HASH_DEF_REGULAR) == 0
+ && !((h->elf_link_hash_flags & ELF_LINK_HASH_COMMON) != 0
+ && (h->root.type == bfd_link_hash_defined
+ || h->root.type == bfd_link_hash_defweak)))
return FALSE;
/* Forced local symbols resolve locally. */
@@ -3776,6 +3783,11 @@ elf_link_add_object_symbols (bfd *abfd,
h->elf_link_hash_flags |= new_flag;
+ if (h->root.type == bfd_link_hash_common)
+ h->elf_link_hash_flags |= ELF_LINK_HASH_COMMON;
+ else
+ h->elf_link_hash_flags &= ~ELF_LINK_HASH_COMMON;
+
/* Check to see if we need to add an indirect symbol for
the default name. */
if (definition || h->root.type == bfd_link_hash_common)
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist oliva@{lsd.ic.unicamp.br, gnu.org}