[PATCH 0/4] Fix issue with PROVIDE in linker scripts

[PATCH 0/4] Fix issue with PROVIDE in linker scripts

[Andrew Burgess]

This series works is really all about patch #3, that fixes an issue in
linker scripts where PROVIDE can override a "normal" symbol.  The
issue is described in more detail in patch #3.

The other patches #1, #3, and #4, are all setup and cleanup.

I've tested this whole series against a set of 245 targets, there are
no new regressions.

---

Andrew Burgess (4):
  ld: In map file use '=' in PROVIDE statements
  ld: Find and run some tests using a wildcard pattern
  ld: Fix issue where PROVIDE overrides defined symbol
  ld: Remove unused expression state

 ld/ChangeLog                                       | 41 +++++++++
 ld/ldexp.c                                         | 99 +++++++++++-----------
 ld/ldexp.h                                         |  1 -
 ld/ldlang.c                                        |  1 +
 ld/testsuite/ld-scripts/provide-3.d                |  1 +
 ld/testsuite/ld-scripts/provide-4-map.d            | 13 ---
 ld/testsuite/ld-scripts/provide-4.d                |  2 +-
 ld/testsuite/ld-scripts/provide-4.map              | 13 +++
 ld/testsuite/ld-scripts/provide-5.d                |  2 +-
 .../ld-scripts/{provide-5-map.d => provide-5.map}  |  2 +-
 ld/testsuite/ld-scripts/provide-6.d                |  9 ++
 ld/testsuite/ld-scripts/provide-6.t                | 11 +++
 ld/testsuite/ld-scripts/provide-7.d                |  8 ++
 ld/testsuite/ld-scripts/provide-7.t                | 11 +++
 ld/testsuite/ld-scripts/provide-8.d                |  8 ++
 ld/testsuite/ld-scripts/provide-8.t                | 14 +++
 ld/testsuite/ld-scripts/provide.exp                | 12 +--
 17 files changed, 177 insertions(+), 71 deletions(-)
 delete mode 100644 ld/testsuite/ld-scripts/provide-4-map.d
 create mode 100644 ld/testsuite/ld-scripts/provide-4.map
 rename ld/testsuite/ld-scripts/{provide-5-map.d => provide-5.map} (60%)
 create mode 100644 ld/testsuite/ld-scripts/provide-6.d
 create mode 100644 ld/testsuite/ld-scripts/provide-6.t
 create mode 100644 ld/testsuite/ld-scripts/provide-7.d
 create mode 100644 ld/testsuite/ld-scripts/provide-7.t
 create mode 100644 ld/testsuite/ld-scripts/provide-8.d
 create mode 100644 ld/testsuite/ld-scripts/provide-8.t

-- 
2.14.3

[PATCH 4/4] ld: Remove unused expression state

[Andrew Burgess]

Previous commit removed all uses of the defsym field within the linker
expression union.  This commit cleans up the now redundant state.

ld/ChangeLog:

	* ldexp.h (union etree_union): Remove defsym field.
	* ldexp.c (exp_assop): Remove defsym parameter, and use of defsym
	parameter.
	(exp_assign): Remove passing of defsym parameter.
	(exp_defsym): Likewise.
	(exp_provide): Likewise.
---
 ld/ChangeLog | 9 +++++++++
 ld/ldexp.c   | 8 +++-----
 ld/ldexp.h   | 1 -
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/ld/ldexp.c b/ld/ldexp.c
index 9508bad4a56..832f1dfb9f6 100644
--- a/ld/ldexp.c
+++ b/ld/ldexp.c
@@ -1339,7 +1339,6 @@ static etree_type *
 exp_assop (const char *dst,
 	   etree_type *src,
 	   enum node_tree_enum class,
-	   bfd_boolean defsym,
 	   bfd_boolean hidden)
 {
   etree_type *n;
@@ -1351,7 +1350,6 @@ exp_assop (const char *dst,
   n->assign.type.node_class = class;
   n->assign.src = src;
   n->assign.dst = dst;
-  n->assign.defsym = defsym;
   n->assign.hidden = hidden;
   return n;
 }
@@ -1361,7 +1359,7 @@ exp_assop (const char *dst,
 etree_type *
 exp_assign (const char *dst, etree_type *src, bfd_boolean hidden)
 {
-  return exp_assop (dst, src, etree_assign, FALSE, hidden);
+  return exp_assop (dst, src, etree_assign, hidden);
 }
 
 /* Handle --defsym command-line option.  */
@@ -1369,7 +1367,7 @@ exp_assign (const char *dst, etree_type *src, bfd_boolean hidden)
 etree_type *
 exp_defsym (const char *dst, etree_type *src)
 {
-  return exp_assop (dst, src, etree_assign, TRUE, FALSE);
+  return exp_assop (dst, src, etree_assign, FALSE);
 }
 
 /* Handle PROVIDE.  */
@@ -1377,7 +1375,7 @@ exp_defsym (const char *dst, etree_type *src)
 etree_type *
 exp_provide (const char *dst, etree_type *src, bfd_boolean hidden)
 {
-  return exp_assop (dst, src, etree_provide, FALSE, hidden);
+  return exp_assop (dst, src, etree_provide, hidden);
 }
 
 /* Handle ASSERT.  */
diff --git a/ld/ldexp.h b/ld/ldexp.h
index 572b703c203..d58cacba1c5 100644
--- a/ld/ldexp.h
+++ b/ld/ldexp.h
@@ -66,7 +66,6 @@ typedef union etree_union {
     node_type type;
     const char *dst;
     union etree_union *src;
-    bfd_boolean defsym;
     bfd_boolean hidden;
   } assign;
   struct {
-- 
2.14.3

[PATCH 2/4] ld: Find and run some tests using a wildcard pattern

[Andrew Burgess]

Find the ld-script/provide-*.d tests using a wildcard, then run them in
a loop.  This will make it easier to add more tests in the future.  Some
associated clean up is required.

ld/ChangeLog:

	* testsuite/ld-scripts/provide-3.d: Add xfail directive.
	* testsuite/ld-scripts/provide-4.d: Use new map file name.
	* testsuite/ld-scripts/provide-5.d: Use new map file name.
	* testsuite/ld-scripts/provide-4-map.d: Renamed to...
	* testsuite/ld-scripts/provide-4.map: ...this.
	* testsuite/ld-scripts/provide-5-map.d: Renamed to...
	* testsuite/ld-scripts/provide-5.map: ...this.
	* testsuite/ld-scripts/provide.exp: Move xfail into provide-3.d
	file, and run tests in a loop.
---
 ld/ChangeLog                                               | 12 ++++++++++++
 ld/testsuite/ld-scripts/provide-3.d                        |  1 +
 ld/testsuite/ld-scripts/provide-4.d                        |  2 +-
 ld/testsuite/ld-scripts/{provide-4-map.d => provide-4.map} |  0
 ld/testsuite/ld-scripts/provide-5.d                        |  2 +-
 ld/testsuite/ld-scripts/{provide-5-map.d => provide-5.map} |  0
 ld/testsuite/ld-scripts/provide.exp                        | 12 ++++++------
 7 files changed, 21 insertions(+), 8 deletions(-)
 rename ld/testsuite/ld-scripts/{provide-4-map.d => provide-4.map} (100%)
 rename ld/testsuite/ld-scripts/{provide-5-map.d => provide-5.map} (100%)

diff --git a/ld/testsuite/ld-scripts/provide-3.d b/ld/testsuite/ld-scripts/provide-3.d
index c8b12dafe7f..a5855e6b473 100644
--- a/ld/testsuite/ld-scripts/provide-3.d
+++ b/ld/testsuite/ld-scripts/provide-3.d
@@ -1,3 +1,4 @@
 #source: provide-3.s
 #ld: -T provide-3.t
 #error: symbol defined in linker script and object file
+#xfail: "*-*-*"
diff --git a/ld/testsuite/ld-scripts/provide-4.d b/ld/testsuite/ld-scripts/provide-4.d
index a7d37e3c347..18699f221da 100644
--- a/ld/testsuite/ld-scripts/provide-4.d
+++ b/ld/testsuite/ld-scripts/provide-4.d
@@ -1,7 +1,7 @@
 #source: provide-2.s
 #ld: -T provide-4.t
 #PROG: nm
-#map: provide-4-map.d
+#map: provide-4.map
 #...
 0+3 A baz
 0+2000 D foo
diff --git a/ld/testsuite/ld-scripts/provide-4-map.d b/ld/testsuite/ld-scripts/provide-4.map
similarity index 100%
rename from ld/testsuite/ld-scripts/provide-4-map.d
rename to ld/testsuite/ld-scripts/provide-4.map
diff --git a/ld/testsuite/ld-scripts/provide-5.d b/ld/testsuite/ld-scripts/provide-5.d
index 1b14fa6a2fe..2a8baec605b 100644
--- a/ld/testsuite/ld-scripts/provide-5.d
+++ b/ld/testsuite/ld-scripts/provide-5.d
@@ -1,6 +1,6 @@
 #source: provide-5.s
 #ld: -T provide-5.t
 #PROG: nm
-#map: provide-5-map.d
+#map: provide-5.map
 #...
 0+10 A foo
diff --git a/ld/testsuite/ld-scripts/provide-5-map.d b/ld/testsuite/ld-scripts/provide-5.map
similarity index 100%
rename from ld/testsuite/ld-scripts/provide-5-map.d
rename to ld/testsuite/ld-scripts/provide-5.map
diff --git a/ld/testsuite/ld-scripts/provide.exp b/ld/testsuite/ld-scripts/provide.exp
index 3a0b2e8070f..01bf934d17e 100644
--- a/ld/testsuite/ld-scripts/provide.exp
+++ b/ld/testsuite/ld-scripts/provide.exp
@@ -36,11 +36,11 @@ if [istarget "x86_64-*-mingw*"] then {
   set LDFLAGS "$LDFLAGS --image-base 0"
 }
 
-run_dump_test provide-1
-run_dump_test provide-2
-setup_xfail *-*-*
-run_dump_test provide-3
-run_dump_test provide-4
-run_dump_test provide-5
+set test_list [lsort [glob -nocomplain $srcdir/$subdir/provide-*.d]]
+foreach test_file $test_list {
+    set test_name [file rootname $test_file]
+    verbose $test_name
+    run_dump_test $test_name
+}
 
 set LDFLAGS "$saved_LDFLAGS"
-- 
2.14.3

[PATCH 1/4] ld: In map file use '=' in PROVIDE statements

[Andrew Burgess]

Currently when recording a PROVIDE statement in a linker script we
display something like:

    PROVIDE (SYMBOL, VALUE)

However, in linker scripts we write these statements like this:

    PROVIDE (SYMBOL = VALUE);

This commit changes the format in the map file to be closer to linker
script format, the map file now contains:

    PROVIDE (SYMBOL = VALUE)

ld/ChangeLog:

	* ldexp.c (exp_print_tree): Use '=' instead of ',' when printing
	PROVIDE statements.
	* testsuite/ld-scripts/provide-4.map: Update expected output.
	* testsuite/ld-scripts/provide-5.map: Likewise.
---
 ld/ChangeLog                            |  7 +++++++
 ld/ldexp.c                              |  2 +-
 ld/testsuite/ld-scripts/provide-4-map.d | 12 ++++++------
 ld/testsuite/ld-scripts/provide-5-map.d |  2 +-
 4 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/ld/ldexp.c b/ld/ldexp.c
index 9fbc892e4b5..800c7f80859 100644
--- a/ld/ldexp.c
+++ b/ld/ldexp.c
@@ -1423,7 +1423,7 @@ exp_print_tree (etree_type *tree)
       break;
     case etree_provide:
     case etree_provided:
-      fprintf (config.map_file, "PROVIDE (%s, ", tree->assign.dst);
+      fprintf (config.map_file, "PROVIDE (%s = ", tree->assign.dst);
       exp_print_tree (tree->assign.src);
       fputc (')', config.map_file);
       break;
diff --git a/ld/testsuite/ld-scripts/provide-4-map.d b/ld/testsuite/ld-scripts/provide-4-map.d
index 189d1d04bcc..6ef936c8592 100644
--- a/ld/testsuite/ld-scripts/provide-4-map.d
+++ b/ld/testsuite/ld-scripts/provide-4-map.d
@@ -1,13 +1,13 @@
 #...
 Linker script and memory map
 #...
-                \[!provide\] +PROVIDE \(foo, 0x1\)
-                \[!provide\] +PROVIDE \(bar, 0x2\)
-                0x0+3 +PROVIDE \(baz, 0x3\)
+                \[!provide\] +PROVIDE \(foo = 0x1\)
+                \[!provide\] +PROVIDE \(bar = 0x2\)
+                0x0+3 +PROVIDE \(baz = 0x3\)
 #...
                 0x0+2000 +foo
-                \[!provide\] +PROVIDE \(loc1, ALIGN \(\., 0x10\)\)
-                0x0+2010 +PROVIDE \(loc2, ALIGN \(\., 0x10\)\)
-                \[!provide\] +PROVIDE \(loc3, \(loc1 \+ 0x20\)\)
+                \[!provide\] +PROVIDE \(loc1 = ALIGN \(\., 0x10\)\)
+                0x0+2010 +PROVIDE \(loc2 = ALIGN \(\., 0x10\)\)
+                \[!provide\] +PROVIDE \(loc3 = \(loc1 \+ 0x20\)\)
                 0x0+2030 +loc4 = \(loc2 \+ 0x20\)
 #...
diff --git a/ld/testsuite/ld-scripts/provide-5-map.d b/ld/testsuite/ld-scripts/provide-5-map.d
index a75e4aa73c5..78b0bcbbdd3 100644
--- a/ld/testsuite/ld-scripts/provide-5-map.d
+++ b/ld/testsuite/ld-scripts/provide-5-map.d
@@ -2,5 +2,5 @@
 Linker script and memory map
 #...
                 0x0+10 +foo = 0x10
-                \[!provide\] +PROVIDE \(foo, bar\)
+                \[!provide\] +PROVIDE \(foo = bar\)
 #...
-- 
2.14.3

[PATCH 3/4] ld: Fix issue where PROVIDE overrides defined symbol

[Andrew Burgess]

In a linker script, a sequence like this:

  foo = ADDR (.some_section);
  bar = foo;
  PROVIDE (foo = 0);

will result in 'bar = ADDR (.some_section)' and 'foo = 0', which seems
like incorrect behaviour, foo is clearly defined elsewhere, and so the
PROVIDE should not trigger.

The problem is that an expression like this:

    foo = ADDR (.some_section);

can't be evaluated until a late phase of the linker, due to the need
for the section '.some_section' to have been placed, then the PROVIDE
was being marked as being used during an earlier phase.  At the end of
the link, both lines:

    foo = ADDR (.some_section);
    PROVIDE (foo = 0);

are active, and this causes the final value of 'foo' to be 0.

The solution proposed in this commit is that, during earlier phases of
the linker, when we see the expression 'foo = ADDR (.some_section);',
instead of ignoring the expression, we create a "fake" definition of
'foo'.  The existence of this "fake" definition prevents the PROVIDE
from being marked used, and during the final phase the real definition
of 'foo' will replace the "fake" definition.

The new test provide-6 covers the exact case described above.  The
provide-7 test is similar to the above, but using constant
expressions, this was never broken, but is added here to increase
coverage.

The provide-8 case also didn't fail before this commit, but I did
manage to break this case during development of this patch.  This case
was only covered by a mmix test before, so I've added this here to
increase coverage.

ld/ChangeLog:

	* ldexp.c (exp_fold_tree_1): Rework condition underwhich provide
	nodes are ignored in the tree walk, and move the location at which
	we change provide nodes into provided nodes.
	(exp_init_os): Add etree_provided.
	* testsuite/ld-scripts/provide-6.d: New file.
	* testsuite/ld-scripts/provide-6.t: New file.
	* testsuite/ld-scripts/provide-7.d: New file.
	* testsuite/ld-scripts/provide-7.t: New file.
	* testsuite/ld-scripts/provide-8.d: New file.
	* testsuite/ld-scripts/provide-8.t: New file.
---
 ld/ChangeLog                        | 13 ++++++
 ld/ldexp.c                          | 89 ++++++++++++++++++++-----------------
 ld/ldlang.c                         |  1 +
 ld/testsuite/ld-scripts/provide-6.d |  9 ++++
 ld/testsuite/ld-scripts/provide-6.t | 11 +++++
 ld/testsuite/ld-scripts/provide-7.d |  8 ++++
 ld/testsuite/ld-scripts/provide-7.t | 11 +++++
 ld/testsuite/ld-scripts/provide-8.d |  8 ++++
 ld/testsuite/ld-scripts/provide-8.t | 14 ++++++
 9 files changed, 122 insertions(+), 42 deletions(-)
 create mode 100644 ld/testsuite/ld-scripts/provide-6.d
 create mode 100644 ld/testsuite/ld-scripts/provide-6.t
 create mode 100644 ld/testsuite/ld-scripts/provide-7.d
 create mode 100644 ld/testsuite/ld-scripts/provide-7.t
 create mode 100644 ld/testsuite/ld-scripts/provide-8.d
 create mode 100644 ld/testsuite/ld-scripts/provide-8.t

diff --git a/ld/ldexp.c b/ld/ldexp.c
index 800c7f80859..9508bad4a56 100644
--- a/ld/ldexp.c
+++ b/ld/ldexp.c
@@ -1153,13 +1153,12 @@ exp_fold_tree_1 (etree_type *tree)
 	     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))
+	  if ((expld.phase == lang_final_phase_enum
+	       || expld.assign_name != NULL))
 	    {
+	      if (tree->type.node_class == etree_provide)
+		tree->type.node_class = etree_provided;
+
 	      if (h == NULL)
 		{
 		  h = bfd_link_hash_lookup (link_info.hash, tree->assign.dst,
@@ -1169,44 +1168,50 @@ exp_fold_tree_1 (etree_type *tree)
 			   tree->assign.dst);
 		}
 
-	      if (expld.result.section == NULL)
-		expld.result.section = expld.section;
-	      if (!update_definedness (tree->assign.dst, h) && 0)
+              /* If the expression is not valid then fake a zero value.  In
+                 the final phase any errors will already have been raised,
+                 in earlier phases we want to create this definition so
+                 that it can be seen by other expressions.  */
+              if (!expld.result.valid_p
+                  && h->type == bfd_link_hash_new)
+                {
+                  expld.result.value = 0;
+                  expld.result.section = NULL;
+                  expld.result.valid_p = TRUE;
+                }
+
+	      if (expld.result.valid_p)
 		{
-		  /* Symbol was already defined.  For now this error
-		     is disabled because it causes failures in the ld
-		     testsuite: ld-elf/var1, ld-scripts/defined5, and
-		     ld-scripts/pr14962.  Some of these no doubt
-		     reflect scripts used in the wild.  */
-		  (*link_info.callbacks->multiple_definition)
-		    (&link_info, h, link_info.output_bfd,
-		     expld.result.section, expld.result.value);
+		  if (expld.result.section == NULL)
+		    expld.result.section = expld.section;
+		  if (!update_definedness (tree->assign.dst, h) && 0)
+		    {
+		      /* Symbol was already defined.  For now this error
+			 is disabled because it causes failures in the ld
+			 testsuite: ld-elf/var1, ld-scripts/defined5, and
+			 ld-scripts/pr14962.  Some of these no doubt
+			 reflect scripts used in the wild.  */
+		      (*link_info.callbacks->multiple_definition)
+			(&link_info, h, link_info.output_bfd,
+			 expld.result.section, expld.result.value);
+		    }
+		  h->type = bfd_link_hash_defined;
+		  h->u.def.value = expld.result.value;
+		  h->u.def.section = expld.result.section;
+		  h->linker_def = ! tree->assign.type.lineno;
+		  h->ldscript_def = 1;
+
+		  /* Copy the symbol type if this is an expression only
+		     referencing a single symbol.  (If the expression
+		     contains ternary conditions, ignoring symbols on
+		     false branches.)  */
+		  if (expld.result.valid_p
+		      && expld.assign_src != NULL
+		      && (expld.assign_src
+			  != (struct bfd_link_hash_entry *) 0 - 1))
+		    bfd_copy_link_hash_symbol_type (link_info.output_bfd, h,
+						    expld.assign_src);
 		}
-	      h->type = bfd_link_hash_defined;
-	      h->u.def.value = expld.result.value;
-	      h->u.def.section = expld.result.section;
-	      h->linker_def = ! tree->assign.type.lineno;
-	      h->ldscript_def = 1;
-	      if (tree->type.node_class == etree_provide)
-		tree->type.node_class = etree_provided;
-
-	      /* Copy the symbol type if this is an expression only
-		 referencing a single symbol.  (If the expression
-		 contains ternary conditions, ignoring symbols on
-		 false branches.)  */
-	      if (expld.result.valid_p
-		  && expld.assign_src != NULL
-		  && expld.assign_src != (struct bfd_link_hash_entry *) 0 - 1)
-		bfd_copy_link_hash_symbol_type (link_info.output_bfd, h,
-						expld.assign_src);
-	    }
-	  else if (expld.phase == lang_final_phase_enum)
-	    {
-	      h = bfd_link_hash_lookup (link_info.hash, tree->assign.dst,
-					FALSE, FALSE, TRUE);
-	      if (h != NULL
-		  && h->type == bfd_link_hash_new)
-		h->type = bfd_link_hash_undefined;
 	    }
 	  expld.assign_name = NULL;
 	}
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 294e6323012..1526d7b2ec1 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -2207,6 +2207,7 @@ exp_init_os (etree_type *exp)
     {
     case etree_assign:
     case etree_provide:
+    case etree_provided:
       exp_init_os (exp->assign.src);
       break;
 
diff --git a/ld/testsuite/ld-scripts/provide-6.d b/ld/testsuite/ld-scripts/provide-6.d
new file mode 100644
index 00000000000..dd405151197
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-6.d
@@ -0,0 +1,9 @@
+#source: provide-5.s
+#ld: -T provide-6.t
+#PROG: nm
+#xfail: x86_64-*-cygwin
+
+#...
+0+1000 D foo
+0+1000 D foo_2
+#...
diff --git a/ld/testsuite/ld-scripts/provide-6.t b/ld/testsuite/ld-scripts/provide-6.t
new file mode 100644
index 00000000000..6b5d11c04bd
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-6.t
@@ -0,0 +1,11 @@
+SECTIONS
+{
+  .data 0x1000 :
+  {
+    *(.data)
+  }
+
+  foo = ADDR (.data);
+  foo_2 = foo;
+  PROVIDE (foo = 0x20);
+}
diff --git a/ld/testsuite/ld-scripts/provide-7.d b/ld/testsuite/ld-scripts/provide-7.d
new file mode 100644
index 00000000000..c524fe428ab
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-7.d
@@ -0,0 +1,8 @@
+#source: provide-5.s
+#ld: -T provide-7.t
+#PROG: nm
+
+#...
+0+10 A foo
+0+10 A foo_2
+#...
diff --git a/ld/testsuite/ld-scripts/provide-7.t b/ld/testsuite/ld-scripts/provide-7.t
new file mode 100644
index 00000000000..882883eda05
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-7.t
@@ -0,0 +1,11 @@
+SECTIONS
+{
+  .data 0x1000 :
+  {
+    *(.data)
+  }
+
+  foo = 0x10;
+  foo_2 = foo;
+  PROVIDE (foo = 0x20);
+}
diff --git a/ld/testsuite/ld-scripts/provide-8.d b/ld/testsuite/ld-scripts/provide-8.d
new file mode 100644
index 00000000000..a4029370ca2
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-8.d
@@ -0,0 +1,8 @@
+#source: provide-5.s
+#ld: -T provide-8.t
+#PROG: nm
+#xfail: x86_64-*-cygwin mmix-*-* sh-*-pe spu-*-*
+
+#...
+0+4000 D __FOO
+#...
diff --git a/ld/testsuite/ld-scripts/provide-8.t b/ld/testsuite/ld-scripts/provide-8.t
new file mode 100644
index 00000000000..ffc3467911f
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-8.t
@@ -0,0 +1,14 @@
+SECTIONS
+{
+  .data 0x1000 :
+  {
+    *(.data)
+    QUAD (__FOO);
+  }
+
+  .foo 0x4000 :
+  {
+    PROVIDE (__FOO = .);
+    *(.foo)
+  }
+}
-- 
2.14.3

Re: [PATCH 1/4] ld: In map file use '=' in PROVIDE statements

[Alan Modra]

On Tue, Jan 09, 2018 at 09:08:56PM +0000, Andrew Burgess wrote:
> 	* ldexp.c (exp_print_tree): Use '=' instead of ',' when printing
> 	PROVIDE statements.
> 	* testsuite/ld-scripts/provide-4.map: Update expected output.
> 	* testsuite/ld-scripts/provide-5.map: Likewise.

OK.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 2/4] ld: Find and run some tests using a wildcard pattern

[Alan Modra]

On Tue, Jan 09, 2018 at 09:08:57PM +0000, Andrew Burgess wrote:
> 	* testsuite/ld-scripts/provide-3.d: Add xfail directive.
> 	* testsuite/ld-scripts/provide-4.d: Use new map file name.
> 	* testsuite/ld-scripts/provide-5.d: Use new map file name.
> 	* testsuite/ld-scripts/provide-4-map.d: Renamed to...
> 	* testsuite/ld-scripts/provide-4.map: ...this.
> 	* testsuite/ld-scripts/provide-5-map.d: Renamed to...
> 	* testsuite/ld-scripts/provide-5.map: ...this.
> 	* testsuite/ld-scripts/provide.exp: Move xfail into provide-3.d
> 	file, and run tests in a loop.

OK.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 3/4] ld: Fix issue where PROVIDE overrides defined symbol

[Alan Modra]

On Tue, Jan 09, 2018 at 09:08:58PM +0000, Andrew Burgess wrote:
> 	* ldexp.c (exp_fold_tree_1): Rework condition underwhich provide
> 	nodes are ignored in the tree walk, and move the location at which
> 	we change provide nodes into provided nodes.
> 	(exp_init_os): Add etree_provided.
> 	* testsuite/ld-scripts/provide-6.d: New file.
> 	* testsuite/ld-scripts/provide-6.t: New file.
> 	* testsuite/ld-scripts/provide-7.d: New file.
> 	* testsuite/ld-scripts/provide-7.t: New file.
> 	* testsuite/ld-scripts/provide-8.d: New file.
> 	* testsuite/ld-scripts/provide-8.t: New file.

OK, with a few nits fixed.

> +	  if ((expld.phase == lang_final_phase_enum
> +	       || expld.assign_name != NULL))

Excess parens here.

> +		  /* Copy the symbol type if this is an expression only
> +		     referencing a single symbol.  (If the expression
> +		     contains ternary conditions, ignoring symbols on
> +		     false branches.)  */
> +		  if (expld.result.valid_p

Testing valid_p here is redundant.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 4/4] ld: Remove unused expression state

[Alan Modra]

On Tue, Jan 09, 2018 at 09:08:59PM +0000, Andrew Burgess wrote:
> 	* ldexp.h (union etree_union): Remove defsym field.
> 	* ldexp.c (exp_assop): Remove defsym parameter, and use of defsym
> 	parameter.
> 	(exp_assign): Remove passing of defsym parameter.
> 	(exp_defsym): Likewise.
> 	(exp_provide): Likewise.

OK.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 0/4] Fix issue with PROVIDE in linker scripts

[Andrew Burgess]

* Andrew Burgess <andrew.burgess@embecosm.com> [2018-01-09 21:08:55 +0000]:

> This series works is really all about patch #3, that fixes an issue in
> linker scripts where PROVIDE can override a "normal" symbol.  The
> issue is described in more detail in patch #3.
> 
> The other patches #1, #3, and #4, are all setup and cleanup.
> 
> I've tested this whole series against a set of 245 targets, there are
> no new regressions.
> 
> ---
> 
> Andrew Burgess (4):
>   ld: In map file use '=' in PROVIDE statements
>   ld: Find and run some tests using a wildcard pattern
>   ld: Fix issue where PROVIDE overrides defined symbol
>   ld: Remove unused expression state

Alan - Thanks for the review.

All pushed with the improvements Alan suggested.

Thanks,
Andrew