[RFA:] Prefer to re-associate symbols in discarded sections with a SEC_LOAD section

[Hans-Peter Nilsson <hp at bitrange dot com>]

Currently, the test-case ld-mmix/bpo-10 fails, not as much due to
weird object formats or target details but because of a bug in the
heuristic to associate a used symbol in a GC:ed section to another nearby
section (i.e., whether to choose the previous or the next section).
Well, ok, one odd detail is involved but not a unique one.

For the case at hand, the "next section" is the .MMIX.reg_contents
register section, which is not suitable to choose for a symbol
originally in the .text section, at least not in preference to the
.init section!  Maybe it can be argued that the .text section should
not have been GC:ed when a symbol in there is referenced.  On the
other hand, the way the symbol ("_start.", sic) is defined and used,
in the default linker script, is intended to work in the absence of a
.text section so I'd say that is as it should be.

When fix_syms look at section details, it looks at SEC_ALLOC and
SEC_THREAD_LOCAL, but the flag used to control whether it matters that
sections overlap (i.e. used whether to emit an error when they
collide, elsewhere) or if they "don't count", is SEC_LOAD.
Unfortunately, an early return in lang_add_section has skipped the
processing of it for the discarded .text section.  For the case at
hand, there's also SEC_LINKER_CREATED for the register section, but
that seems inappropriate to use as a controlling factor; a discarded
code section symbol could very well be re-associated to e.g. a .plt
section.

So, it seems appropriate to use SEC_LOAD in an all-else-being-equal
test to choose the section with which to associate the symbol.  An
alternative would be to rewrite lang_add_section to process SEC_LOAD
(and presumably similar others to avoid special cases) and to fold the
handling of SEC_LOAD into those of SEC_ALLOC and SEC_THREAD_LOCAL in
fix_syms.

Tested mmix-knuth-mmixware and also cross to mips-unknown-linux-gnu
and native i686-pc-linux-gnu.

Ok to commit?

bfd:
	* linker.c (fix_syms): Consider SEC_LOAD when choosing section.

Index: linker.c
===================================================================
RCS file: /cvs/src/src/bfd/linker.c,v
retrieving revision 1.69
diff -p -u -r1.69 linker.c
--- linker.c	4 May 2009 12:09:29 -0000	1.69
+++ linker.c	30 Jul 2009 15:17:52 -0000
@@ -3149,10 +3149,16 @@ fix_syms (struct bfd_link_hash_entry *h,
 	  else if (op == NULL)
 	    op = op1;
 	  else if (((op1->flags ^ op->flags)
-		    & (SEC_ALLOC | SEC_THREAD_LOCAL)) != 0)
+		    & (SEC_ALLOC | SEC_THREAD_LOCAL | SEC_LOAD)) != 0)
 	    {
 	      if (((op->flags ^ s->flags)
-		   & (SEC_ALLOC | SEC_THREAD_LOCAL)) != 0)
+		   & (SEC_ALLOC | SEC_THREAD_LOCAL)) != 0
+		  /* We prefer to choose a loaded section.  Section S
+		     doesn't have SEC_LOAD set (it being excluded, that
+		     part of the flag processing didn't happen) so we
+		     can't compare that flag to those of OP and OP1.  */
+		  || ((op1->flags & SEC_LOAD) != 0
+		      && (op->flags & SEC_LOAD) == 0))
 		op = op1;
 	    }
 	  else if (((op1->flags ^ op->flags) & SEC_READONLY) != 0)

brgds, H-P