Re: [RFA:] Fix DEFINED in ld scripts.

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

On Fri, 10 Oct 2003, Alan Modra wrote:
> On Fri, Oct 10, 2003 at 04:12:39AM -0400, Hans-Peter Nilsson wrote:
> > +   /* The value "3" is ad-hoc, somewhat related to the expected number of
> > +      DEFINED expressions in a linker script.  For most default linker
> > +      scripts, there are none.  */
> > +   if (bfd_hash_table_init_n (&lang_definedness_table,
> > + 			     lang_definedness_newfunc, 3) != TRUE)
>
> So why bother with a hash table?  Wouldn't a simple list do?

Hopefully covered by the updated comment.  To wit: why make it a
simple list when using the existing hash table code is simpler. ;-)
I can't really goof on the pointers or the list walk this way.

> > ! 	     Callers of exp_fold_tree need to increment
> > ! 	     lang_statement_iteration before starting to iterate over the
> > ! 	     statement list.  We need to do it before each call instead of
> > ! 	     in lang_do_assignments because we can't tell the root call
> > ! 	     from its recursions.  */
>
> Split out the body of lang_do_assignments to lang_do_assignments_1.
> Then you can twiddle your iteration count in lang_do_assignments.

I forgot to mention that I'd planned to do the
lang_do_assignments as a future independent change.  But since
you asked, done.

Bootstrapped and checked as before on ia64-unknown-linux-gnu.  Built and
checked cross to mmix-knuth-mmixware (binutils, gas, ld, gcc).

This is only the updated ldlang.c patch: the other code is as in the
previous email.

Ok to commit?

ld:
	* ldlang.c (lang_definedness_table): New variable.
	(lang_definedness_newfunc, lang_track_definedness)
	(lang_symbol_definition_iteration, lang_update_definedness): New
	functions.
	(lang_init): Initialize lang_definedness_table and
	lang_statement_iteration.
	(lang_finish): Destroy bfd_hash_table_free.
	(lang_size_sections): Increment lang_statement_iteration.
	(lang_do_assignments_1): New function with former
	lang_do_assignments contents.  Change recursive calls to call this
	function.
	(lang_do_assignments): Evacuate contents.  Increment
	lang_statement_iteration, then just call lang_do_assignments_1.

Index: ldlang.c
===================================================================
RCS file: /cvs/src/src/ld/ldlang.c,v
retrieving revision 1.118
diff -p -c -u -2 -p -r1.118 ldlang.c
cvs server: conflicting specifications of output style
--- ldlang.c	8 Oct 2003 22:17:35 -0000	1.118
+++ ldlang.c	11 Oct 2003 02:22:57 -0000
@@ -61,4 +61,5 @@ static const char *output_target;
 static lang_statement_list_type statement_list;
 static struct lang_phdr *lang_phdr_list;
+static struct bfd_hash_table lang_definedness_table;

 /* Forward declarations.  */
@@ -68,4 +69,6 @@ static lang_input_statement_type *lookup
 static bfd_boolean load_symbols (lang_input_statement_type *,
 				 lang_statement_list_type *);
+static struct bfd_hash_entry *lang_definedness_newfunc
+ (struct bfd_hash_entry *, struct bfd_hash_table *, const char *);
 static void insert_undefined (const char *);
 static void print_statement (lang_statement_union_type *,
@@ -96,4 +99,5 @@ struct lang_nocrossrefs *nocrossref_list
 struct unique_sections *unique_section_list;
 static bfd_boolean ldlang_sysrooted_script = FALSE;
+int lang_statement_iteration = 0;

 etree_type *base; /* Relocation base - or null */
@@ -478,4 +482,17 @@ lang_init (void)

   abs_output_section->bfd_section = bfd_abs_section_ptr;
+
+  /* The value "3" is ad-hoc, somewhat related to the expected number of
+     DEFINED expressions in a linker script.  For most default linker
+     scripts, there are none.  Why a hash table then?  Well, it's somewhat
+     simpler to re-use working machinery than using a linked list in terms
+     of code-complexity here in ld, besides the initialization which just
+     looks like other code here.  */
+  if (bfd_hash_table_init_n (&lang_definedness_table,
+			     lang_definedness_newfunc, 3) != TRUE)
+    einfo (_("%P%F: out of memory during initialization"));
+
+  /* Callers of exp_fold_tree need to increment this.  */
+  lang_statement_iteration = 0;
 }

@@ -1868,4 +1885,83 @@ lang_reasonable_defaults (void)
 }

+/* Add a symbol to a hash of symbols used in DEFINED (NAME) expressions.  */
+
+void
+lang_track_definedness (const char *name)
+{
+  if (bfd_hash_lookup (&lang_definedness_table, name, TRUE, FALSE) == NULL)
+    einfo (_("%P%F: bfd_hash_lookup failed creating symbol %s\n"), name);
+}
+
+/* New-function for the definedness hash table.  */
+
+static struct bfd_hash_entry *
+lang_definedness_newfunc (struct bfd_hash_entry *entry,
+			  struct bfd_hash_table *table ATTRIBUTE_UNUSED,
+			  const char *name ATTRIBUTE_UNUSED)
+{
+  struct lang_definedness_hash_entry *ret
+    = (struct lang_definedness_hash_entry *) entry;
+
+  if (ret == NULL)
+    ret = (struct lang_definedness_hash_entry *)
+      bfd_hash_allocate (table, sizeof (struct lang_definedness_hash_entry));
+
+  if (ret == NULL)
+    einfo (_("%P%F: bfd_hash_allocate failed creating symbol %s\n"), name);
+
+  ret->iteration = -1;
+  return &ret->root;
+}
+
+/* Return the iteration when the definition of NAME was last updated.  A
+   value of -1 means that the symbol is not defined in the linker script
+   or the command line, but may be defined in the linker symbol table.  */
+
+int
+lang_symbol_definition_iteration (const char *name)
+{
+  struct lang_definedness_hash_entry *defentry
+    = (struct lang_definedness_hash_entry *)
+    bfd_hash_lookup (&lang_definedness_table, name, FALSE, FALSE);
+
+  /* We've already created this one on the presence of DEFINED in the
+     script, so it can't be NULL unless something is borked elsewhere in
+     the code.  */
+  if (defentry == NULL)
+    FAIL ();
+
+  return defentry->iteration;
+}
+
+/* Update the definedness state of NAME.  */
+
+void
+lang_update_definedness (const char *name, struct bfd_link_hash_entry *h)
+{
+  struct lang_definedness_hash_entry *defentry
+    = (struct lang_definedness_hash_entry *)
+    bfd_hash_lookup (&lang_definedness_table, name, FALSE, FALSE);
+
+  /* We don't keep track of symbols not tested with DEFINED.  */
+  if (defentry == NULL)
+    return;
+
+  /* If the symbol was already defined, and not from an earlier statement
+     iteration, don't update the definedness iteration, because that'd
+     make the symbol seem defined in the linker script at this point, and
+     it wasn't; it was defined in some object.  If we do anyway, DEFINED
+     would start to yield false before this point and the construct "sym =
+     DEFINED (sym) ? sym : X;" would change sym to X despite being defined
+     in an object.  */
+  if (h->type != bfd_link_hash_undefined
+      && h->type != bfd_link_hash_common
+      && h->type != bfd_link_hash_new
+      && defentry->iteration == -1)
+    return;
+
+  defentry->iteration = lang_statement_iteration;
+}
+
 /* Add the supplied name to the symbol table as an undefined reference.
    This is a two step process as the symbol table doesn't even exist at
@@ -3133,4 +3229,7 @@ lang_size_sections
   bfd_vma result;

+  /* Callers of exp_fold_tree need to increment this.  */
+  lang_statement_iteration++;
+
   exp_data_seg.phase = exp_dataseg_none;
   result = lang_size_sections_1 (s, output_section_statement, prev, fill,
@@ -3158,6 +3257,8 @@ lang_size_sections
 }

-bfd_vma
-lang_do_assignments
+/* Worker function for lang_do_assignments.  Recursiveness goes here.  */
+
+static bfd_vma
+lang_do_assignments_1
   (lang_statement_union_type *s,
    lang_output_section_statement_type *output_section_statement,
@@ -3173,8 +3274,8 @@ lang_do_assignments
 	{
 	case lang_constructors_statement_enum:
-	  dot = lang_do_assignments (constructor_list.head,
-				     output_section_statement,
-				     fill,
-				     dot);
+	  dot = lang_do_assignments_1 (constructor_list.head,
+				       output_section_statement,
+				       fill,
+				       dot);
 	  break;

@@ -3187,6 +3288,6 @@ lang_do_assignments
 	      {
 		dot = os->bfd_section->vma;
-		(void) lang_do_assignments (os->children.head, os,
-					    os->fill, dot);
+		(void) lang_do_assignments_1 (os->children.head, os,
+					      os->fill, dot);
 		dot = os->bfd_section->vma + os->bfd_section->_raw_size / opb;

@@ -3207,7 +3308,7 @@ lang_do_assignments
 	case lang_wild_statement_enum:

-	  dot = lang_do_assignments (s->wild_statement.children.head,
-				     output_section_statement,
-				     fill, dot);
+	  dot = lang_do_assignments_1 (s->wild_statement.children.head,
+				       output_section_statement,
+				       fill, dot);

 	  break;
@@ -3302,7 +3403,7 @@ lang_do_assignments

 	case lang_group_statement_enum:
-	  dot = lang_do_assignments (s->group_statement.children.head,
-				     output_section_statement,
-				     fill, dot);
+	  dot = lang_do_assignments_1 (s->group_statement.children.head,
+				       output_section_statement,
+				       fill, dot);

 	  break;
@@ -3319,4 +3420,16 @@ lang_do_assignments
 }

+bfd_vma
+lang_do_assignments (lang_statement_union_type *s,
+		     lang_output_section_statement_type
+		     *output_section_statement,
+		     fill_type *fill,
+		     bfd_vma dot)
+{
+  /* Callers of exp_fold_tree need to increment this.  */
+  lang_statement_iteration++;
+  lang_do_assignments_1 (s, output_section_statement, fill, dot);
+}
+
 /* Fix any .startof. or .sizeof. symbols.  When the assemblers see the
    operator .startof. (section_name), it produces an undefined symbol
@@ -3446,4 +3559,6 @@ lang_finish (void)
 	}
     }
+
+  bfd_hash_table_free (&lang_definedness_table);
 }

brgds, H-P