PATCH: PR ld/2065: "ld -r" is broken

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

PATCH: PR ld/2065: "ld -r" is broken

H.J. Lu-27
I am resending this with proper subject.

H.J.
---
On Sun, Dec 18, 2005 at 03:42:23PM -0800, H. J. Lu wrote:

> On Sat, Dec 17, 2005 at 12:46:44PM +1030, Alan Modra wrote:
> > On Fri, Dec 16, 2005 at 05:46:40PM -0800, H. J. Lu wrote:
> > > bad.o is generated by the new linker. Alan, can you take a look?
> >
> > Sorry, not immediately.  I'm just about to leave for a week at the
> > beach with my family.
> >
>
> I opened a PR:
>
> http://sourceware.org/bugzilla/show_bug.cgi?id=2065
>
> and I am checking this testcase.
>

The problem is introduced by

http://sourceware.org/ml/binutils/2005-11/msg00239.html

where the new prev field is never really set properly since the last
field used to set it points to the next field of the last element on
the list.

This hack seems to work for me. But I'd like to see the tail field in
lang_statement_list_type be removed if possible. Alan, can you take
a look after you come back?

Thanks.


H.J.
----
2005-12-18  H.J. Lu  <[hidden email]>

        PR ld/2065
        * ldlang.h (lang_statement_list_type): Add last.
        * ldlang.c (file_chain): Set the last field.
        (lang_list_init): Likewise.
        (lang_insert_orphan): Likewise.
        (lang_statement_append): Likewise.
        (output_statement_newfunc): Set os.prev with last.

--- ld/ldlang.c.prev 2005-12-18 09:40:59.000000000 -0800
+++ ld/ldlang.c 2005-12-18 14:29:57.000000000 -0800
@@ -89,7 +89,7 @@ static void lang_do_version_exports_sect
 lang_output_section_statement_type *abs_output_section;
 lang_statement_list_type lang_output_section_statement;
 lang_statement_list_type *stat_ptr = &statement_list;
-lang_statement_list_type file_chain = { NULL, NULL };
+lang_statement_list_type file_chain = { NULL, NULL, NULL };
 struct bfd_sym_chain entry_symbol = { NULL, NULL };
 static const char *entry_symbol_default = "start";
 const char *entry_section = ".text";
@@ -742,6 +742,7 @@ void
 lang_list_init (lang_statement_list_type *list)
 {
   list->head = NULL;
+  list->last = NULL;
   list->tail = &list->head;
 }
 
@@ -912,8 +913,10 @@ output_statement_newfunc (struct bfd_has
  (lang_statement_union_type *) &ret->os,
  &ret->os.header.next);
 
-  ret->os.prev = &((*lang_output_section_statement.tail)
-   ->output_section_statement);
+  if (lang_output_section_statement.last)
+    ret->os.prev
+      = &(lang_output_section_statement.last->output_section_statement);
+
   /* GCC's strict aliasing rules prevent us from just casting the
      address, so we store the pointer in a variable and cast that
      instead.  */
@@ -1531,7 +1534,10 @@ lang_insert_orphan (lang_input_statement
   /* 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;
+    {
+      old->last = add.last;
+      old->tail = add.tail;
+    }
 
   /* Save the end of this list.  */
   place->stmt = add.tail;
@@ -5836,6 +5842,7 @@ lang_statement_append (lang_statement_li
        lang_statement_union_type **field)
 {
   *(list->tail) = element;
+  list->last = element;
   list->tail = field;
 }
 
--- ld/ldlang.h.prev 2005-12-18 09:40:59.000000000 -0800
+++ ld/ldlang.h 2005-12-18 14:12:14.000000000 -0800
@@ -44,6 +44,7 @@ struct _fill_type
 typedef struct statement_list
 {
   union lang_statement_union *head;
+  union lang_statement_union *last;
   union lang_statement_union **tail;
 } lang_statement_list_type;
 

Reply | Threaded
Open this post in threaded view
|

Re: PATCH: PR ld/2065: "ld -r" is broken

Alan Modra
On Sun, Dec 18, 2005 at 03:59:19PM -0800, H. J. Lu wrote:
> The problem is introduced by
> http://sourceware.org/ml/binutils/2005-11/msg00239.html
> where the new prev field is never really set properly

Yes, I slipped up.  I missed adjusting "prev" in lang_insert_orphan
too.  Obviously not enough testing.  Thanks for adding the new test, and
reverting the bad patch.

        * ldlang.h (lang_output_section_statement_type): Add prev.
        * ldlang.c (output_statement_newfunc): Set os.prev.
        (lang_insert_orphan): Likewise.
        (output_prev_sec_find): Use os.prev.

Index: ld/ldlang.h
===================================================================
RCS file: /cvs/src/src/ld/ldlang.h,v
retrieving revision 1.59
diff -u -p -r1.59 ldlang.h
--- ld/ldlang.h 19 Dec 2005 15:06:40 -0000 1.59
+++ ld/ldlang.h 24 Dec 2005 07:24:34 -0000
@@ -130,6 +130,7 @@ typedef struct lang_output_section_state
   lang_statement_header_type header;
   lang_statement_list_type children;
   struct lang_output_section_statement_struct *next;
+  struct lang_output_section_statement_struct *prev;
   const char *name;
   asection *bfd_section;
   lang_memory_region_type *region;
Index: ld/ldlang.c
===================================================================
RCS file: /cvs/src/src/ld/ldlang.c,v
retrieving revision 1.211
diff -u -p -r1.211 ldlang.c
--- ld/ldlang.c 19 Dec 2005 15:06:40 -0000 1.211
+++ ld/ldlang.c 24 Dec 2005 07:24:34 -0000
@@ -912,6 +912,14 @@ output_statement_newfunc (struct bfd_has
  (lang_statement_union_type *) &ret->os,
  &ret->os.header.next);
 
+  /* For every output section statement added to the list, except the
+     first one, lang_output_section_statement.tail points to the "next"
+     field of the last element of the list.  */
+  if (lang_output_section_statement.head != NULL)
+    ret->os.prev = (lang_output_section_statement_type *)
+      ((char *) lang_output_section_statement.tail
+       - offsetof (lang_output_section_statement_type, next));
+
   /* GCC's strict aliasing rules prevent us from just casting the
      address, so we store the pointer in a variable and cast that
      instead.  */
@@ -1290,20 +1298,15 @@ lang_output_section_find_by_flags (const
 static asection *
 output_prev_sec_find (lang_output_section_statement_type *os)
 {
-  asection *s = (asection *) NULL;
   lang_output_section_statement_type *lookup;
 
-  for (lookup = &lang_output_section_statement.head->output_section_statement;
-       lookup != NULL;
-       lookup = lookup->next)
+  for (lookup = os->prev; lookup != NULL; lookup = lookup->prev)
     {
       if (lookup->constraint == -1)
  continue;
-      if (lookup == os)
- return s;
 
       if (lookup->bfd_section != NULL && lookup->bfd_section->owner != NULL)
- s = lookup->bfd_section;
+ return lookup->bfd_section;
     }
 
   return NULL;
@@ -1544,7 +1547,12 @@ lang_insert_orphan (asection *s,
   /* Do the same for the list of output section statements.  */
   newly_added_os = *os_tail;
   *os_tail = NULL;
+  newly_added_os->prev = (lang_output_section_statement_type *)
+    ((char *) place->os_tail
+     - offsetof (lang_output_section_statement_type, next));
   newly_added_os->next = *place->os_tail;
+  if (newly_added_os->next != NULL)
+    newly_added_os->next->prev = newly_added_os;
   *place->os_tail = newly_added_os;
   place->os_tail = &newly_added_os->next;
 
--
Alan Modra
IBM OzLabs - Linux Technology Centre