Re: RFH: Assignments folded too early

[Daniel Jacobowitz <dan at codesourcery dot com>]

On Wed, Apr 07, 2010 at 11:28:49PM -0400, Daniel Jacobowitz wrote:
> We could argue about the semantics of this, but I was also using this
> as shorthand for another potential problem.  Suppose we have the
> layout:
> 
> SECTIONS {
>   .text : {
>     *(.text)
>     *(.text1)
>     begin = DEFINED(begin) ? begin : fallback;
>   }
> }
> 
> Now suppose the linker needs to insert a long branch or mode change
> stub between .text and .text1.  We'll go back through section sizing,
> change all the output_offsets... but begin is now defined in terms of
> the .text *output* section.  It's harder to produce, but as dangerous.

Here's my WIP patch for this.  If folks like the approach, I'll try to
write a testcase which causes ARM veneers to be inserted before the
assignment point so that the bug I've described is tested.

I'd like to have a more general solution that solves cases other than
simple assignment.  I'm not sure how to do that, and comments are
welcome.  I think that this patch is at least a strict improvement.

I fixed the map file quirk too.

-- 
Daniel Jacobowitz
CodeSourcery

2010-04-08  Daniel Jacobowitz  <dan@codesourcery.com>

	ld/
	* ldlang.c (print_assignment): Use the symbol's section if we
	use its value.
	* ld/ldexp.c (exp_fold_tree_1): Skip self-assignment.  Expand
	comment on copying symbol type.

Index: ld/ldlang.c
===================================================================
--- ld/ldlang.c	(revision 281081)
+++ ld/ldlang.c	(working copy)
@@ -3899,9 +3899,8 @@ print_assignment (lang_assignment_statem
 	  if (h)
 	    {
 	      value = h->u.def.value;
-
-	      if (expld.result.section)
-		value += expld.result.section->vma;
+	      value += h->u.def.section->output_section->vma;
+	      value += h->u.def.section->output_offset;
 
 	      minfo ("[0x%V]", value);
 	    }
Index: ld/ldexp.c
===================================================================
--- ld/ldexp.c	(revision 281081)
+++ ld/ldexp.c	(working copy)
@@ -778,6 +778,8 @@ exp_fold_tree_1 (etree_type *tree)
 	}
       else
 	{
+	  etree_type *name;
+
 	  struct bfd_link_hash_entry *h = NULL;
 
 	  if (tree->type.node_class == etree_provide)
@@ -795,6 +797,23 @@ exp_fold_tree_1 (etree_type *tree)
 		}
 	    }
 
+	  name = tree->assign.src;
+	  if (name->type.node_class == etree_trinary)
+	    {
+	      exp_fold_tree_1 (name->trinary.cond);
+	      if (expld.result.valid_p)
+		name = (expld.result.value
+			? name->trinary.lhs : name->trinary.rhs);
+	    }
+
+	  if (name->type.node_class == etree_name
+	      && name->type.node_code == NAME
+	      && strcmp (tree->assign.dst, name->name.name) == 0)
+	    /* Leave it alone.  Do not replace a symbol with its own
+	       output address, in case there is another section sizing
+	       pass.  Folding does not preserve input sections.  */
+	    break;
+
 	  exp_fold_tree_1 (tree->assign.src);
 	  if (expld.result.valid_p)
 	    {
@@ -817,7 +836,8 @@ exp_fold_tree_1 (etree_type *tree)
 		tree->type.node_class = etree_provided;
 
 	      /* Copy the symbol type if this is a simple assignment of
-	         one symbol to annother.  */
+	         one symbol to another.  This could be more general
+		 (e.g. a ?: operator with NAMEs in each branch).  */
 	      if (tree->assign.src->type.node_class == etree_name)
 		{
 		  struct bfd_link_hash_entry *hsrc;