This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

PR14962 fix


When I first looked at PR14962 I was inclined to close the bug as
WONTFIX, but I like the clever hack used to avoid cross-reference
warnings on a subset of symbols.  The reporter uses self-assignment in
a linker script to convert symbols to absolute, thus avoiding the
cross-reference check.

The problem is that ld now runs lang_do_assignments early, before
sections are mapped to output sections.  Trying to convert symbols
to absolute results in wrong values because section VMAs are not set.
Of course, once the symbol is absolute it loses its original section,
so a later lang_do_assingments won't correct the value.  So we need to
prevent self-assignment until section placement is complete.

Interestingly, ldexp.c already contains code to catch a very limited
set of self-assignment expressions, and ldlang.c has a more
sophisticated recursive descent check for self-assignment.  This patch
replaces both lots of code with a simple modification to the normal
expression evaluation code.  Besides fixing the PR, I think this
behaves better than the old scan_for_self_assignment(): The old code
saw a reference on the false branch of "?:" as a self-assignment, and
references in DEFINED(), and could get confused by symbols that
happened to have the same name as a section.

Not that the new code will catch all self-assignments.  For example,
writing
	x = ld_eval_count + 1;
	ld_eval_count = x;
will circumvent the checks.

	PR ld/14962
	* ldexp.h (struct ldexp_control): Add "assign_name".
	* ldexp.c (fold_name <NAME>): Compare and clear assign_name on match.
	(exp_fold_tree_1): Remove existing code testing for self assignment.
	Instead set and test expld.assign_name.
	* ldlang.c (scan_for_self_assignment): Delete.
	(print_assignment): Instead set and test expld.assign_name.

Index: ld/ldexp.h
===================================================================
RCS file: /cvs/src/src/ld/ldexp.h,v
retrieving revision 1.32
diff -u -p -r1.32 ldexp.h
--- ld/ldexp.h	6 Aug 2012 22:27:52 -0000	1.32
+++ ld/ldexp.h	15 Dec 2012 05:46:49 -0000
@@ -139,6 +139,11 @@ struct ldexp_control {
 
   /* Principally used for diagnostics.  */
   bfd_boolean assigning_to_dot;
+  /* If evaluating an assignment, the destination.  Cleared if an
+     etree_name NAME matches this, to signal a self-assignment.
+     Note that an etree_name DEFINED does not clear this field, nor
+     does the false branch of a trinary expression.  */
+  const char *assign_name;
 
   /* Working results.  */
   etree_value_type result;
Index: ld/ldexp.c
===================================================================
RCS file: /cvs/src/src/ld/ldexp.c,v
retrieving revision 1.102
diff -u -p -r1.102 ldexp.c
--- ld/ldexp.c	6 Aug 2012 22:27:52 -0000	1.102
+++ ld/ldexp.c	15 Dec 2012 13:55:29 -0000
@@ -572,6 +572,9 @@ fold_name (etree_type *tree)
       break;
 
     case NAME:
+      if (expld.assign_name != NULL
+	  && strcmp (expld.assign_name, tree->name.name) == 0)
+	expld.assign_name = NULL;
       if (expld.phase == lang_first_phase_enum)
 	;
       else if (tree->name.name[0] == '.' && tree->name.name[1] == 0)
@@ -852,8 +855,6 @@ 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)
@@ -871,25 +872,20 @@ 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;
-
+	  expld.assign_name = tree->assign.dst;
 	  exp_fold_tree_1 (tree->assign.src);
-	  if (expld.result.valid_p
+	  /* expld.assign_name remaining equal to tree->assign.dst
+	     below indicates the evaluation of tree->assign.src did
+	     not use the value of tree->assign.dst.  We don't allow
+	     self assignment until the final phase for two reasons:
+	     1) Expressions are evaluated multiple times.  With
+	     relaxation, the number of times may vary.
+	     2) Section relative symbol values cannot be correctly
+	     converted to absolute values, as is required by many
+	     expressions, until final section sizing is complete.  */
+	  if ((expld.result.valid_p
+	       && (expld.phase == lang_final_phase_enum
+		   || expld.assign_name != NULL))
 	      || (expld.phase <= lang_mark_phase_enum
 		  && tree->type.node_class == etree_assign
 		  && tree->assign.defsym))
@@ -937,6 +933,7 @@ exp_fold_tree_1 (etree_type *tree)
 		  && h->type == bfd_link_hash_new)
 		h->type = bfd_link_hash_undefined;
 	    }
+	  expld.assign_name = NULL;
 	}
       break;
 
Index: ld/ldlang.c
===================================================================
RCS file: /cvs/src/src/ld/ldlang.c,v
retrieving revision 1.402
diff -u -p -r1.402 ldlang.c
--- ld/ldlang.c	21 Nov 2012 19:56:37 -0000	1.402
+++ ld/ldlang.c	15 Dec 2012 08:15:57 -0000
@@ -3951,63 +3951,12 @@ print_output_section_statement
 			output_section_statement);
 }
 
-/* Scan for the use of the destination in the right hand side
-   of an expression.  In such cases we will not compute the
-   correct expression, since the value of DST that is used on
-   the right hand side will be its final value, not its value
-   just before this expression is evaluated.  */
-
-static bfd_boolean
-scan_for_self_assignment (const char * dst, etree_type * rhs)
-{
-  if (rhs == NULL || dst == NULL)
-    return FALSE;
-
-  switch (rhs->type.node_class)
-    {
-    case etree_binary:
-      return (scan_for_self_assignment (dst, rhs->binary.lhs)
-	      || scan_for_self_assignment (dst, rhs->binary.rhs));
-
-    case etree_trinary:
-      return (scan_for_self_assignment (dst, rhs->trinary.lhs)
-	      || scan_for_self_assignment (dst, rhs->trinary.rhs));
-
-    case etree_assign:
-    case etree_provided:
-    case etree_provide:
-      if (strcmp (dst, rhs->assign.dst) == 0)
-	return TRUE;
-      return scan_for_self_assignment (dst, rhs->assign.src);
-
-    case etree_unary:
-      return scan_for_self_assignment (dst, rhs->unary.child);
-
-    case etree_value:
-      if (rhs->value.str)
-	return strcmp (dst, rhs->value.str) == 0;
-      return FALSE;
-
-    case etree_name:
-      if (rhs->name.name)
-	return strcmp (dst, rhs->name.name) == 0;
-      return FALSE;
-
-    default:
-      break;
-    }
-
-  return FALSE;
-}
-
-
 static void
 print_assignment (lang_assignment_statement_type *assignment,
 		  lang_output_section_statement_type *output_section)
 {
   unsigned int i;
   bfd_boolean is_dot;
-  bfd_boolean computation_is_valid = TRUE;
   etree_type *tree;
   asection *osec;
 
@@ -4018,15 +3967,14 @@ print_assignment (lang_assignment_statem
     {
       is_dot = FALSE;
       tree = assignment->exp->assert_s.child;
-      computation_is_valid = TRUE;
     }
   else
     {
       const char *dst = assignment->exp->assign.dst;
 
       is_dot = (dst[0] == '.' && dst[1] == 0);
+      expld.assign_name = dst;
       tree = assignment->exp->assign.src;
-      computation_is_valid = is_dot || !scan_for_self_assignment (dst, tree);
     }
 
   osec = output_section->bfd_section;
@@ -4037,7 +3985,9 @@ print_assignment (lang_assignment_statem
     {
       bfd_vma value;
 
-      if (computation_is_valid)
+      if (assignment->exp->type.node_class == etree_assert
+	  || is_dot
+	  || expld.assign_name != NULL)
 	{
 	  value = expld.result.value;
 
@@ -4073,6 +4023,7 @@ print_assignment (lang_assignment_statem
       minfo ("        ");
 #endif
     }
+  expld.assign_name = NULL;
 
   minfo ("                ");
   exp_print_tree (assignment->exp);

-- 
Alan Modra
Australia Development Lab, IBM


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]