[RFC] alpha-elf vs copy_indirect_symbol

[RFC] alpha-elf vs copy_indirect_symbol

[Richard Henderson]

[-- Attachment #1: Type: text/plain, Size: 1381 bytes --]

A while ago I (finally) added support for --gc-sections to the
alpha backend.  Unfortunately at the time I'd been testing GCC
with --disable-shared, to make things easier to manage in my
cross-build setup.  While the original patch worked for the ld
testsuite, it ran into problems linking the shared libstdc++.

Alpha had been using its own fixup routine for versioned symbols,
which it ran during always_size_sections.  Unfortunately, this
runs after the mark+sweep, which means that the gc_sweep_hook
was seeing invalid data, which lead directly to the crash.

I presume without any archaeology that Alpha's version symbol
fixup routine pre-dates the copy_indirect_symbol hook.  Not
that it matters, I suppose, it's got to be fixed now.

The following patch appears to fix the problem.  Certainly there's
no longer an ld crash building gcc+binutils w/ all languages.
However, I've yet to adjust my cross-built setup to handle the
shared libraries, so I havn't actually tested everything properly.

What concerns me is that copy_indirect_symbol is called for things
that aren't indirect symbols.  In particular, we call this hook for
defweak and defined symbols.  Which might be fine, I suppose, if we
then *replaced* the defweak symbol in the sym_hashes table so that
we never ever see it again.  I have no idea what the current usage
is supposed to accomplish.

Comments?


r~

[-- Attachment #2: d-alpha-cis --]
[-- Type: text/plain, Size: 2968 bytes --]

	* elf64-alpha.c (elf64_alpha_copy_indirect_symbol): Rename from
	elf64_alpha_merge_ind_symbols; adjust for the generic interface.
	(elf64_alpha_always_size_sections): Don't call
	elf64_alpha_merge_ind_symbols.
	(elf_backend_copy_indirect_symbol): New.

Index: elf64-alpha.c
===================================================================
RCS file: /cvs/src/src/bfd/elf64-alpha.c,v
retrieving revision 1.177
diff -u -p -r1.177 elf64-alpha.c
--- elf64-alpha.c	13 Jun 2011 00:59:12 -0000	1.177
+++ elf64-alpha.c	14 Jun 2011 15:41:37 -0000
@@ -2148,23 +2148,29 @@ elf64_alpha_merge_symbol_attribute (stru
    indirect to the new ones.  Consolidate the got and reloc information
    in these situations.  */
 
-static bfd_boolean
-elf64_alpha_merge_ind_symbols (struct alpha_elf_link_hash_entry *hi,
-			       PTR dummy ATTRIBUTE_UNUSED)
-{
-  struct alpha_elf_link_hash_entry *hs;
+static void
+elf64_alpha_copy_indirect_symbol (struct bfd_link_info *info,
+				  struct elf_link_hash_entry *dir,
+				  struct elf_link_hash_entry *ind)
+{
+  struct alpha_elf_link_hash_entry *hi
+    = (struct alpha_elf_link_hash_entry *) ind;
+  struct alpha_elf_link_hash_entry *hs
+    = (struct alpha_elf_link_hash_entry *) dir;
 
-  if (hi->root.root.type != bfd_link_hash_indirect)
-    return TRUE;
-  hs = hi;
-  do {
-    hs = (struct alpha_elf_link_hash_entry *)hs->root.root.u.i.link;
-  } while (hs->root.root.type == bfd_link_hash_indirect);
+  /* Do the merging in the superclass.  */
+  _bfd_elf_link_hash_copy_indirect(info, dir, ind);
 
   /* Merge the flags.  Whee.  */
-
   hs->flags |= hi->flags;
 
+  /* ??? It's unclear to me what's really supposed to happen when
+     "merging" defweak and defined symbols, given that we don't
+     actually throw away the defweak.  This more-or-less copies
+     the logic related to got and plt entries in the superclass.  */
+  if (ind->root.type != bfd_link_hash_indirect)
+    return;
+
   /* Merge the .got entries.  Cannibalize the old symbol's list in
      doing so, since we don't need it anymore.  */
 
@@ -2217,8 +2223,6 @@ elf64_alpha_merge_ind_symbols (struct al
 	}
     }
   hi->reloc_entries = NULL;
-
-  return TRUE;
 }
 
 /* Is it possible to merge two object file's .got tables?  */
@@ -2625,10 +2629,6 @@ elf64_alpha_always_size_sections (bfd *o
   if (htab == NULL)
     return FALSE;
 
-  /* First, take care of the indirect symbols created by versioning.  */
-  alpha_elf_link_hash_traverse (htab, elf64_alpha_merge_ind_symbols,
-				NULL);
-
   if (!elf64_alpha_size_got_sections (info))
     return FALSE;
 
@@ -5443,6 +5443,8 @@ static const struct elf_size_info alpha_
   elf64_alpha_adjust_dynamic_symbol
 #define elf_backend_merge_symbol_attribute \
   elf64_alpha_merge_symbol_attribute
+#define elf_backend_copy_indirect_symbol \
+  elf64_alpha_copy_indirect_symbol
 #define elf_backend_always_size_sections \
   elf64_alpha_always_size_sections
 #define elf_backend_size_dynamic_sections \

Re: [RFC] alpha-elf vs copy_indirect_symbol

[Alan Modra]

On Tue, Jun 14, 2011 at 09:14:02AM -0700, Richard Henderson wrote:
> What concerns me is that copy_indirect_symbol is called for things
> that aren't indirect symbols.  In particular, we call this hook for
> defweak and defined symbols.

The weak and strong symbols in question are both non-function symbols
defined in a shared lib, eg. timezone and _timezone.

>  Which might be fine, I suppose, if we
> then *replaced* the defweak symbol in the sym_hashes table so that
> we never ever see it again.  I have no idea what the current usage
> is supposed to accomplish.

It started life as a means to copy over symbol flags from the weak to
strong sym.  Without doing some archaeology I'm not sure why we
also transfer over dyn_relocs on most targets.

-- 
Alan Modra
Australia Development Lab, IBM