Re: ld output_section_statment related tidy

[Alan Modra <amodra at bigpond dot net dot au>]

On Sun, Feb 15, 2009 at 03:23:18PM +0100, Andreas Schwab wrote:
> Alan Modra <amodra@bigpond.net.au> writes:
> > Can you tell me how to reproduce the crash?
> 
> See <https://bugzilla.novell.com/attachment.cgi?id=269567>.

process_insert_statements walks the statement list, setting
"constraint" negative for any output section statements it finds while
looking for an insert statement.  If no insert statement was found it
resets the constraints by walking the output statement list.  This is
a different list, but it ought to be in the same order as the full
statement list.  Of course, it wasn't.  The testcase is missing a -T
on the command line, and manages to find a place in ld where we allow
the lists to become inconsistent.

	* ldlang.c (push_stat_ptr, pop_stat_ptr): New functions.
	(stat_save, stat_save_ptr): New variables.
	(lang_insert_orphan): Use push_stat_ptr and pop_stat_ptr.
	(load_symbols): Likewise.  Delete dead "bad_load" code.
	(open_input_bfds): Warn on script containing output sections.
	(lang_enter_output_section_statement): Use push_stat_ptr.
	(lang_enter_group): Likewise.
	(lang_leave_output_section_statement): Use pop_stat_ptr.
	(lang_leave_group): Likewise.
	* ldlang.h (push_stat_ptr, pop_stat_ptr): Declare.
	* ldctor.c (ldctor_build_sets): Use push_stat_ptr and pop_stat_ptr.
	* emultempl/beos.em (gld_${EMULATION_NAME}_set_symbols): Likewise.
	* emultempl/pe.em (gld_${EMULATION_NAME}_set_symbols): Likewise.
	* emultempl/pep.em (gld_${EMULATION_NAME}_set_symbols): Likewise.
	* emultempl/spuelf.em (spu_place_special_section): Likewise.
	* emultempl/xtensaelf.em (ld_xtensa_insert_page_offsets): Likewise.

Index: ld/ldlang.c
===================================================================
RCS file: /cvs/src/src/ld/ldlang.c,v
retrieving revision 1.299
diff -u -p -r1.299 ldlang.c
--- ld/ldlang.c	9 Feb 2009 14:46:40 -0000	1.299
+++ ld/ldlang.c	16 Feb 2009 06:25:23 -0000
@@ -64,6 +64,8 @@ static const char *current_target;
 static const char *output_target;
 static lang_statement_list_type statement_list;
 static struct bfd_hash_table lang_definedness_table;
+static lang_statement_list_type *stat_save[10];
+static lang_statement_list_type **stat_save_ptr = &stat_save[0];
 
 /* Forward declarations.  */
 static void exp_init_os (etree_type *);
@@ -925,6 +927,23 @@ lang_list_init (lang_statement_list_type
   list->tail = &list->head;
 }
 
+void
+push_stat_ptr (lang_statement_list_type *new_ptr)
+{
+  if (stat_save_ptr >= stat_save + sizeof (stat_save) / sizeof (stat_save[0]))
+    abort ();
+  *stat_save_ptr++ = stat_ptr;
+  stat_ptr = new_ptr;
+}
+
+void
+pop_stat_ptr (void)
+{
+  if (stat_save_ptr <= stat_save)
+    abort ();
+  stat_ptr = *--stat_save_ptr;
+}
+
 /* Build a new statement node for the parse tree.  */
 
 static lang_statement_union_type *
@@ -1586,23 +1605,18 @@ lang_insert_orphan (asection *s,
 		    etree_type *address,
 		    lang_statement_list_type *add_child)
 {
-  lang_statement_list_type *old;
   lang_statement_list_type add;
   const char *ps;
   lang_output_section_statement_type *os;
   lang_output_section_statement_type **os_tail;
 
-  /* Start building a list of statements for this section.
-     First save the current statement pointer.  */
-  old = stat_ptr;
-
   /* If we have found an appropriate place for the output section
      statements for this orphan, add them to our own private list,
      inserting them later into the global statement list.  */
   if (after != NULL)
     {
-      stat_ptr = &add;
-      lang_list_init (stat_ptr);
+      lang_list_init (&add);
+      push_stat_ptr (&add);
     }
 
   ps = NULL;
@@ -1648,11 +1662,6 @@ lang_insert_orphan (asection *s,
     {
       char *symname;
 
-      /* lang_leave_ouput_section_statement resets stat_ptr.
-	 Put stat_ptr back where we want it.  */
-      if (after != NULL)
-	stat_ptr = &add;
-
       symname = (char *) xmalloc (ps - secname + sizeof "__stop_" + 1);
       symname[0] = bfd_get_symbol_leading_char (link_info.output_bfd);
       sprintf (symname + (symname[0] != 0), "__stop_%s", secname);
@@ -1663,7 +1672,7 @@ lang_insert_orphan (asection *s,
 
   /* Restore the global list pointer.  */
   if (after != NULL)
-    stat_ptr = old;
+    pop_stat_ptr ();
 
   if (after != NULL && os->bfd_section != NULL)
     {
@@ -1749,8 +1758,8 @@ lang_insert_orphan (asection *s,
 
 	  /* Fix the global list pointer if we happened to tack our
 	     new list at the tail.  */
-	  if (*old->tail == add.head)
-	    old->tail = add.tail;
+	  if (*stat_ptr->tail == add.head)
+	    stat_ptr->tail = add.tail;
 
 	  /* Save the end of this list.  */
 	  place->stmt = add.tail;
@@ -2481,8 +2490,6 @@ load_symbols (lang_input_statement_type 
       && ! bfd_check_format_matches (entry->the_bfd, bfd_object, &matching))
     {
       bfd_error_type err;
-      lang_statement_list_type *hold;
-      bfd_boolean bad_load = TRUE;
       bfd_boolean save_ldlang_sysrooted_script;
       bfd_boolean save_as_needed, save_add_needed;
 
@@ -2505,8 +2512,6 @@ load_symbols (lang_input_statement_type 
       else if (err != bfd_error_file_not_recognized
 	       || place == NULL)
 	einfo (_("%F%B: file not recognized: %E\n"), entry->the_bfd);
-      else
-	bad_load = FALSE;
 
       bfd_close (entry->the_bfd);
       entry->the_bfd = NULL;
@@ -2514,8 +2519,7 @@ load_symbols (lang_input_statement_type 
       /* Try to interpret the file as a linker script.  */
       ldfile_open_command_file (entry->filename);
 
-      hold = stat_ptr;
-      stat_ptr = place;
+      push_stat_ptr (place);
       save_ldlang_sysrooted_script = ldlang_sysrooted_script;
       ldlang_sysrooted_script = entry->sysrooted;
       save_as_needed = as_needed;
@@ -2534,9 +2538,9 @@ load_symbols (lang_input_statement_type 
       ldlang_sysrooted_script = save_ldlang_sysrooted_script;
       as_needed = save_as_needed;
       add_needed = save_add_needed;
-      stat_ptr = hold;
+      pop_stat_ptr ();
 
-      return ! bad_load;
+      return TRUE;
     }
 
   if (ldemul_recognized_file (entry))
@@ -3008,6 +3012,7 @@ open_input_bfds (lang_statement_union_ty
 	case lang_input_statement_enum:
 	  if (s->input_statement.real)
 	    {
+	      lang_statement_union_type **os_tail;
 	      lang_statement_list_type add;
 
 	      s->input_statement.target = current_target;
@@ -3023,6 +3028,7 @@ open_input_bfds (lang_statement_union_ty
 				       bfd_archive))
 		s->input_statement.loaded = FALSE;
 
+	      os_tail = lang_output_section_statement.tail;
 	      lang_list_init (&add);
 
 	      if (! load_symbols (&s->input_statement, &add))
@@ -3030,8 +3036,25 @@ open_input_bfds (lang_statement_union_ty
 
 	      if (add.head != NULL)
 		{
-		  *add.tail = s->header.next;
-		  s->header.next = add.head;
+		  /* If this was a script with output sections then
+		     tack any added statements on to the end of the
+		     list.  This avoids having to reorder the output
+		     section statement list.  Very likely the user
+		     forgot -T, and whatever we do here will not meet
+		     naive user expectations.  */
+		  if (os_tail != lang_output_section_statement.tail)
+		    {
+		      einfo (_("%P: warning: %s contains output sections;"
+			       " did you forget -T?\n"),
+			     s->input_statement.filename);
+		      *stat_ptr->tail = add.head;
+		      stat_ptr->tail = add.tail;
+		    }
+		  else
+		    {
+		      *add.tail = s->header.next;
+		      s->header.next = add.head;
+		    }
 		}
 	    }
 	  break;
@@ -5860,7 +5883,7 @@ lang_enter_output_section_statement (con
   os->block_value = 1;
 
   /* Make next things chain into subchain of this.  */
-  stat_ptr = &os->children;
+  push_stat_ptr (&os->children);
 
   os->subsection_alignment =
     topower (exp_get_value_int (subalign, -1, "subsection alignment"));
@@ -6468,7 +6491,7 @@ lang_leave_output_section_statement (fil
 		    current_section->addr_tree != NULL);
   current_section->fill = fill;
   current_section->phdrs = phdrs;
-  stat_ptr = &statement_list;
+  pop_stat_ptr ();
 }
 
 /* Create an absolute symbol with the given name with the value of the
@@ -6585,7 +6608,7 @@ lang_enter_group (void)
 
   g = new_stat (lang_group_statement, stat_ptr);
   lang_list_init (&g->children);
-  stat_ptr = &g->children;
+  push_stat_ptr (&g->children);
 }
 
 /* Leave a group.  This just resets stat_ptr to start writing to the
@@ -6596,7 +6619,7 @@ lang_enter_group (void)
 void
 lang_leave_group (void)
 {
-  stat_ptr = &statement_list;
+  pop_stat_ptr ();
 }
 
 /* Add a new program header.  This is called for each entry in a PHDRS
Index: ld/ldlang.h
===================================================================
RCS file: /cvs/src/src/ld/ldlang.h,v
retrieving revision 1.80
diff -u -p -r1.80 ldlang.h
--- ld/ldlang.h	4 Oct 2008 06:08:59 -0000	1.80
+++ ld/ldlang.h	16 Feb 2009 06:25:24 -0000
@@ -550,6 +550,10 @@ extern void lang_add_output_format
   (const char *, const char *, const char *, int);
 extern void lang_list_init
   (lang_statement_list_type *);
+extern void push_stat_ptr
+  (lang_statement_list_type *);
+extern void pop_stat_ptr
+  (void);
 extern void lang_add_data
   (int type, union etree_union *);
 extern void lang_add_reloc
Index: ld/ldctor.c
===================================================================
RCS file: /cvs/src/src/ld/ldctor.c,v
retrieving revision 1.14
diff -u -p -r1.14 ldctor.c
--- ld/ldctor.c	15 Feb 2008 03:35:53 -0000	1.14
+++ ld/ldctor.c	16 Feb 2009 06:25:19 -0000
@@ -198,7 +198,6 @@ void
 ldctor_build_sets (void)
 {
   static bfd_boolean called;
-  lang_statement_list_type *old;
   bfd_boolean header_printed;
   struct set_info *p;
 
@@ -244,10 +243,8 @@ ldctor_build_sets (void)
 	}
     }
 
-  old = stat_ptr;
-  stat_ptr = &constructor_list;
-
-  lang_list_init (stat_ptr);
+  lang_list_init (&constructor_list);
+  push_stat_ptr (&constructor_list);
 
   header_printed = FALSE;
   for (p = sets; p != NULL; p = p->next)
@@ -372,5 +369,5 @@ ldctor_build_sets (void)
       lang_add_data (size, exp_intop (0));
     }
 
-  stat_ptr = old;
+  pop_stat_ptr ();
 }
Index: ld/emultempl/beos.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/beos.em,v
retrieving revision 1.43
diff -u -p -r1.43 beos.em
--- ld/emultempl/beos.em	20 Oct 2008 12:14:29 -0000	1.43
+++ ld/emultempl/beos.em	16 Feb 2009 06:25:24 -0000
@@ -330,7 +330,6 @@ gld_${EMULATION_NAME}_set_symbols (void)
   /* Run through and invent symbols for all the
      names and insert the defaults. */
   int j;
-  lang_statement_list_type *save;
 
   if (!init[IMAGEBASEOFF].inited)
     {
@@ -347,9 +346,7 @@ gld_${EMULATION_NAME}_set_symbols (void)
     return;
 
   /* Glue the assignments into the abs section */
-  save = stat_ptr;
-
-  stat_ptr = &(abs_output_section->children);
+  push_stat_ptr (&abs_output_section->children);
 
   for (j = 0; init[j].ptr; j++)
     {
@@ -367,7 +364,7 @@ gld_${EMULATION_NAME}_set_symbols (void)
       else	abort();
     }
   /* Restore the pointer. */
-  stat_ptr = save;
+  pop_stat_ptr ();
 
   if (pe.FileAlignment >
       pe.SectionAlignment)
@@ -609,8 +606,6 @@ sort_sections (lang_statement_union_type
 static void
 gld_${EMULATION_NAME}_before_allocation (void)
 {
-  extern lang_statement_list_type *stat_ptr;
-
 #ifdef TARGET_IS_ppcpe
   /* Here we rummage through the found bfds to collect toc information */
   {
Index: ld/emultempl/pe.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/pe.em,v
retrieving revision 1.141
diff -u -p -r1.141 pe.em
--- ld/emultempl/pe.em	8 Jan 2009 13:28:48 -0000	1.141
+++ ld/emultempl/pe.em	16 Feb 2009 06:25:26 -0000
@@ -725,7 +725,6 @@ gld_${EMULATION_NAME}_set_symbols (void)
   /* Run through and invent symbols for all the
      names and insert the defaults.  */
   int j;
-  lang_statement_list_type *save;
 
   if (!init[IMAGEBASEOFF].inited)
     {
@@ -748,9 +747,7 @@ gld_${EMULATION_NAME}_set_symbols (void)
     return;
 
   /* Glue the assignments into the abs section.  */
-  save = stat_ptr;
-
-  stat_ptr = &(abs_output_section->children);
+  push_stat_ptr (&abs_output_section->children);
 
   for (j = 0; init[j].ptr; j++)
     {
@@ -772,7 +769,7 @@ gld_${EMULATION_NAME}_set_symbols (void)
 	image_base_statement = rv;
     }
   /* Restore the pointer.  */
-  stat_ptr = save;
+  pop_stat_ptr ();
 
   if (pe.FileAlignment >
       pe.SectionAlignment)
Index: ld/emultempl/pep.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/pep.em,v
retrieving revision 1.20
diff -u -p -r1.20 pep.em
--- ld/emultempl/pep.em	8 Jan 2009 13:28:48 -0000	1.20
+++ ld/emultempl/pep.em	16 Feb 2009 06:25:27 -0000
@@ -681,7 +681,6 @@ gld_${EMULATION_NAME}_set_symbols (void)
   /* Run through and invent symbols for all the
      names and insert the defaults.  */
   int j;
-  lang_statement_list_type *save;
 
   if (!init[IMAGEBASEOFF].inited)
     {
@@ -704,9 +703,7 @@ gld_${EMULATION_NAME}_set_symbols (void)
     return;
 
   /* Glue the assignments into the abs section.  */
-  save = stat_ptr;
-
-  stat_ptr = &(abs_output_section->children);
+  push_stat_ptr (&abs_output_section->children);
 
   for (j = 0; init[j].ptr; j++)
     {
@@ -728,7 +725,7 @@ gld_${EMULATION_NAME}_set_symbols (void)
 	image_base_statement = rv;
     }
   /* Restore the pointer.  */
-  stat_ptr = save;
+  pop_stat_ptr ();
 
   if (pep.FileAlignment > pep.SectionAlignment)
     {
Index: ld/emultempl/spuelf.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/spuelf.em,v
retrieving revision 1.33
diff -u -p -r1.33 spuelf.em
--- ld/emultempl/spuelf.em	4 Feb 2009 02:43:17 -0000	1.33
+++ ld/emultempl/spuelf.em	16 Feb 2009 06:25:27 -0000
@@ -160,12 +160,11 @@ spu_place_special_section (asection *s, 
 	  /* Pad this stub section so that it finishes at the
 	     end of the icache line.  */
 	  etree_type *e_size;
-	  lang_statement_list_type *save = stat_ptr;
 
-	  stat_ptr = &os->children;
+	  push_stat_ptr (&os->children);
 	  e_size = exp_intop (params.line_size - s->size);
 	  lang_add_assignment (exp_assop ('=', ".", e_size));
-	  stat_ptr = save;
+	  pop_stat_ptr ();
 	}
       lang_add_section (&os->children, s, os);
     }
@@ -558,7 +557,7 @@ embedded_spu_file (lang_input_statement_
   if (lang_add_input_file (oname, lang_input_file_is_file_enum, NULL) == NULL)
     return FALSE;
 
-  /* lang_add_input_file put the new list entry at the end of the statement
+  /* lang_add_input_file puts the new list entry at the end of the statement
      and input file lists.  Move it to just after the current entry.  */
   new_ent = *old_stat_tail;
   *old_stat_tail = NULL;
Index: ld/emultempl/xtensaelf.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/xtensaelf.em,v
retrieving revision 1.22
diff -u -p -r1.22 xtensaelf.em
--- ld/emultempl/xtensaelf.em	15 Feb 2008 03:35:53 -0000	1.22
+++ ld/emultempl/xtensaelf.em	16 Feb 2009 06:25:29 -0000
@@ -1951,20 +1951,17 @@ ld_xtensa_insert_page_offsets (bfd_vma d
 		lang_assignment_statement_type *assign_stmt;
 		lang_statement_union_type *assign_union;
 		lang_statement_list_type tmplist;
-		lang_statement_list_type *old_stat_ptr = stat_ptr;
 
 		/* There is hidden state in "lang_add_assignment".  It
 		   appends the new assignment statement to the stat_ptr
 		   list.  Thus, we swap it before and after the call.  */
 
-		tmplist.head = NULL;
-		tmplist.tail = &tmplist.head;
-
-		stat_ptr = &tmplist;
+		lang_list_init (&tmplist);
+		push_stat_ptr (&tmplist);
 		/* Warning: side effect; statement appended to stat_ptr.  */
 		assign_stmt = lang_add_assignment (assign_op);
 		assign_union = (lang_statement_union_type *) assign_stmt;
-		stat_ptr = old_stat_ptr;
+		pop_stat_ptr ();
 
 		assign_union->header.next = l;
 		*(*stack_p)->iterloc.loc = assign_union;


-- 
Alan Modra
Australia Development Lab, IBM