public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 0/2] ld: PROVIDE, undefined symbols, and mapfiles.
@ 2015-01-07 12:15 Andrew Burgess
  2015-01-07 12:15 ` [PATCH 1/2] ld/testing: run_dump_test can now check linker mapfiles Andrew Burgess
  2015-01-07 12:15 ` [PATCH 2/2] ld: Don't evaluate unneeded PROVIDE expressions Andrew Burgess
  0 siblings, 2 replies; 16+ messages in thread
From: Andrew Burgess @ 2015-01-07 12:15 UTC (permalink / raw)
  To: binutils; +Cc: Andrew Burgess

Consider the following snippet of linker script:

  PROVIDE (foo = 0x10);
  PROVIDE (bar = foo);

If both 'foo' and 'bar' are not needed then both of these PROVIDE
statements will have no effect.

However, when creating a linker mapfile, the expressions in both
PROVIDE statements are, currently, evaluated.  This is fine for the
'foo' case, but in the 'bar' case the foo symbol is undefined, and so
we get a fatal, undefined symbol, error.

The first patch in this series adds a new option to run_dump_test
called 'map' that allows for easier testing of generated linker scripts.

The second patch resolves the above problem by not evaluating the
PROVIDE expression when the symbol being provided is not needed.

Thanks,
Andrew

Andrew Burgess (2):
  ld/testing: run_dump_test can now check linker mapfiles.
  ld: Don't evaluate unneeded PROVIDE expressions.

 ld/ChangeLog                             |  7 +++++++
 ld/ldlang.c                              | 14 +++++++++++--
 ld/testsuite/ChangeLog                   | 14 +++++++++++++
 ld/testsuite/ld-scripts/overlay-size.d   |  1 +
 ld/testsuite/ld-scripts/overlay-size.exp |  9 ---------
 ld/testsuite/ld-scripts/provide-4-map.d  | 26 ++++++++++++++++++++++++
 ld/testsuite/ld-scripts/provide-4.d      | 10 ++++++++++
 ld/testsuite/ld-scripts/provide-4.t      | 16 +++++++++++++++
 ld/testsuite/ld-scripts/provide.exp      |  1 +
 ld/testsuite/lib/ld-lib.exp              | 34 ++++++++++++++++++++++++++++++++
 10 files changed, 121 insertions(+), 11 deletions(-)
 create mode 100644 ld/testsuite/ld-scripts/provide-4-map.d
 create mode 100644 ld/testsuite/ld-scripts/provide-4.d
 create mode 100644 ld/testsuite/ld-scripts/provide-4.t

-- 
1.9.3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/2] ld/testing: run_dump_test can now check linker mapfiles.
  2015-01-07 12:15 [PATCH 0/2] ld: PROVIDE, undefined symbols, and mapfiles Andrew Burgess
@ 2015-01-07 12:15 ` Andrew Burgess
  2015-01-08  1:21   ` Hans-Peter Nilsson
  2015-01-07 12:15 ` [PATCH 2/2] ld: Don't evaluate unneeded PROVIDE expressions Andrew Burgess
  1 sibling, 1 reply; 16+ messages in thread
From: Andrew Burgess @ 2015-01-07 12:15 UTC (permalink / raw)
  To: binutils; +Cc: Andrew Burgess

This is just a new mechanism to make it easier to test linker mapfiles.

OK to apply?

Thanks,
Andrew


--

Add a new option 'map' to the ld run_dump_test mechanism.  When the
'map' option is given run_dump_test will ensure that there is a
-Map=MAPFILE present in the linker command line, adding one if needed.

The MAPFILE is then compared with the file passed to the new 'map'
option using the regexp_diff function.  This should make it slightly
easier to write tests that check the linker mapfile output.

The only test I found that already compares mapfile content is updated
to use the new mechanism.

ld/testsuite/ChangeLog:

	* ld-scripts/overlay-size.d: Add 'map' option.
	* ld-scripts/overlay-size.exp: Remove manual check of mapfile.
	* lib/ld-lib.exp (run_dump_test): Add support for new 'map'
	option, checking linker mapfile output.
---
 ld/testsuite/ChangeLog                   |  7 +++++++
 ld/testsuite/ld-scripts/overlay-size.d   |  1 +
 ld/testsuite/ld-scripts/overlay-size.exp |  9 ---------
 ld/testsuite/lib/ld-lib.exp              | 34 ++++++++++++++++++++++++++++++++
 4 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
index c9b3ecb..266e996 100644
--- a/ld/testsuite/ChangeLog
+++ b/ld/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2015-01-07  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* ld-scripts/overlay-size.d: Add 'map' option.
+	* ld-scripts/overlay-size.exp: Remove manual check of mapfile.
+	* lib/ld-lib.exp (run_dump_test): Add support for new 'map'
+	option, checking linker mapfile output.
+
 2015-01-06  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* lib/ld-lib.exp (run_dump_test): Extend comment to mention
diff --git a/ld/testsuite/ld-scripts/overlay-size.d b/ld/testsuite/ld-scripts/overlay-size.d
index 78a9c92..4e60150 100644
--- a/ld/testsuite/ld-scripts/overlay-size.d
+++ b/ld/testsuite/ld-scripts/overlay-size.d
@@ -1,6 +1,7 @@
 # ld: -T overlay-size.t -Map tmpdir/overlay-size.map
 # name: overlay size
 # objdump: --headers
+# map: overlay-size-map.d
 # xfail: rx-*-*
 #   FAILS on the RX because the linker has to set LMA == VMA for the
 #   Renesas loader.
diff --git a/ld/testsuite/ld-scripts/overlay-size.exp b/ld/testsuite/ld-scripts/overlay-size.exp
index 4dde289..e8e7a91 100644
--- a/ld/testsuite/ld-scripts/overlay-size.exp
+++ b/ld/testsuite/ld-scripts/overlay-size.exp
@@ -23,12 +23,3 @@ if ![is_elf_format] {
 }
 
 run_dump_test overlay-size
-
-set testname "overlay size (map check)"
-
-if [regexp_diff "tmpdir/overlay-size.map" \
-	        "$srcdir/$subdir/overlay-size-map.d"] {
-    fail $testname
-} else {
-    pass $testname
-}
diff --git a/ld/testsuite/lib/ld-lib.exp b/ld/testsuite/lib/ld-lib.exp
index 3d980c9..045d2b7 100644
--- a/ld/testsuite/lib/ld-lib.exp
+++ b/ld/testsuite/lib/ld-lib.exp
@@ -555,6 +555,14 @@ proc ld_simple_link_defsyms {} {
 #	both "error" and "warning".  Multiple "warning" directives
 #	append to the expected linker warning message.
 #
+#   map: FILE
+#       Adding this option will cause the linker to generate a linker
+#       map file, using the -Map=MAPFILE command line option.  If thre
+#       is no -Map=MAPFILE in the 'ld: FLAGS' then one will be added
+#       to the linker command line.  The contents of the generated
+#       MAPFILE are then compared against the regexp lines in FILE
+#       using `regexp_diff' (see below for details).
+#
 # Each option may occur at most once unless otherwise mentioned.
 #
 # After the option lines come regexp lines.  `run_dump_test' calls
@@ -607,6 +615,7 @@ proc run_dump_test { name {extra_options {}} } {
     set opts(warning) {}
     set opts(objcopy_linked_file) {}
     set opts(objcopy_objects) {}
+    set opts(map) {}
 
     foreach i $opt_array {
 	set opt_name [lindex $i 0]
@@ -860,6 +869,20 @@ proc run_dump_test { name {extra_options {}} } {
 	set cmd "$LD $LDFLAGS -L$srcdir/$subdir \
 		   $opts(ld) -o $objfile $objfiles $opts(ld_after_inputfiles)"
 
+        # If needed then check for, or add a -Map option.
+        set mapfile ""
+        if { $opts(map) != "" } then {
+            if { [regexp -- "-Map=(\[^ \]+)" $cmd all mapfile] } then {
+                # Found existing mapfile option
+                verbose -log "Existing mapfile '$mapfile' found"
+            } else {
+                # No mapfile option.
+                set mapfile "tmpdir/dump.map"
+                verbose -log "Adding mapfile '$mapfile' found"
+                set cmd "$cmd -Map=$mapfile"
+            }
+        }
+
 	send_log "$cmd\n"
 	set cmdret [remote_exec host [concat sh -c [list "$cmd 2>&1"]] "" "/dev/null" "ld.tmp"]
 	remote_upload host "ld.tmp"
@@ -908,6 +931,17 @@ proc run_dump_test { name {extra_options {}} } {
 		return
 	    }
 	}
+
+        if { $opts(map) != "" } then {
+            # Check the map file matches.
+            set map_pattern_file $srcdir/$subdir/$opts(map)
+            verbose -log "Compare '$mapfile' against '$map_pattern_file'"
+            if { [regexp_diff $mapfile $map_pattern_file] } then {
+                fail "$testname (map file check)"
+            } else {
+                pass "$testname (map file check)"
+            }
+        }
     } else {
 	set objfile "tmpdir/dump0.o"
     }
-- 
1.9.3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 2/2] ld: Don't evaluate unneeded PROVIDE expressions.
  2015-01-07 12:15 [PATCH 0/2] ld: PROVIDE, undefined symbols, and mapfiles Andrew Burgess
  2015-01-07 12:15 ` [PATCH 1/2] ld/testing: run_dump_test can now check linker mapfiles Andrew Burgess
@ 2015-01-07 12:15 ` Andrew Burgess
  2015-01-08  1:25   ` Hans-Peter Nilsson
  1 sibling, 1 reply; 16+ messages in thread
From: Andrew Burgess @ 2015-01-07 12:15 UTC (permalink / raw)
  To: binutils; +Cc: Andrew Burgess

This is the patch that solves the issue with PROVIDE referencing an
undefined symbol.

The only thing I'm not sure about is my choice of text for when a
PROVIDE statement is not going to provide anything, right now I use
'[!provide]' the style of which was inspired by the '[unresolved]' a
few lines above.  I wanted to keep the length of the string inline
with the '*undef* ' string in the same block.  Any suggestions for
better formatting will be appreciated.

Otherwise, OK to apply?

Thanks,
Andrew

--

When creating a linker mapfile (using -Map=MAPFILE), we previously would
always try to evaluate the expression from a PROVIDE statement.

However, this is not always safe, consider:

  PROVIDE (foo = 0x10);
  PROVIDE (bar = foo);

In this example, if neither 'foo' or 'bar' is needed, then while
generating the linker mapfile evaluating the expression for 'foo' is
harmless (just the value 0x10).  However, evaluating the expression for
'bar' requires the symbol 'foo', which is undefined.  This used to cause
a fatal error.

This patch changes the behaviour, so that when the destination of the
PROVIDE is not defined (that is the PROVIDE is not going to provide
anything) the expression is not evaluated, and instead a special string
is displayed to indicate that the linker is discarding the PROVIDE
statement.

This change not only fixes the spurious undefined symbol error, but also
means that a user can now tell if a PROVIDE statement has provided
anything by inspecting the linker mapfile, something that could not be
done before.

ld/ChangeLog:

	* ldlang.c (print_assignment): Only evaluate the expression for a
	PROVIDE'd assignment when the destination is being defined.
	Display a special message for PROVIDE'd symbols that are not being
	provided.

ld/testsuite/ChangeLog:

	* ld-scripts/provide-4.d: New file.
	* ld-scripts/provide-4-map.d: New file.
	* ld-scripts/provide-4.t: New file.
	* ld-scripts/provide.exp: Run the provide-4.d test.
---
 ld/ChangeLog                            |  7 +++++++
 ld/ldlang.c                             | 14 ++++++++++++--
 ld/testsuite/ChangeLog                  |  7 +++++++
 ld/testsuite/ld-scripts/provide-4-map.d | 26 ++++++++++++++++++++++++++
 ld/testsuite/ld-scripts/provide-4.d     | 10 ++++++++++
 ld/testsuite/ld-scripts/provide-4.t     | 16 ++++++++++++++++
 ld/testsuite/ld-scripts/provide.exp     |  1 +
 7 files changed, 79 insertions(+), 2 deletions(-)
 create mode 100644 ld/testsuite/ld-scripts/provide-4-map.d
 create mode 100644 ld/testsuite/ld-scripts/provide-4.d
 create mode 100644 ld/testsuite/ld-scripts/provide-4.t

diff --git a/ld/ChangeLog b/ld/ChangeLog
index be4617f..fab597e 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,10 @@
+2015-01-07  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* ldlang.c (print_assignment): Only evaluate the expression for a
+	PROVIDE'd assignment when the destination is being defined.
+	Display a special message for PROVIDE'd symbols that are not being
+	provided.
+
 2015-01-01  Alan Modra  <amodra@gmail.com>
 
 	* ldver.c (ldversion): Just print current year.
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 9f3d209..a99febe 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -3983,7 +3983,14 @@ print_assignment (lang_assignment_statement_type *assignment,
   osec = output_section->bfd_section;
   if (osec == NULL)
     osec = bfd_abs_section_ptr;
-  exp_fold_tree (tree, osec, &print_dot);
+
+  if (assignment->exp->type.node_class != etree_provide
+      || bfd_link_hash_lookup (link_info.hash,
+                               assignment->exp->assign.dst,
+                               FALSE, FALSE, TRUE) != NULL)
+    exp_fold_tree (tree, osec, &print_dot);
+  else
+    expld.result.valid_p = FALSE;
   if (expld.result.valid_p)
     {
       bfd_vma value;
@@ -4021,7 +4028,10 @@ print_assignment (lang_assignment_statement_type *assignment,
     }
   else
     {
-      minfo ("*undef*   ");
+      if (assignment->exp->type.node_class == etree_provide)
+        minfo ("[!provide]");
+      else
+        minfo ("*undef*   ");
 #ifdef BFD64
       minfo ("        ");
 #endif
diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
index 266e996..521c819 100644
--- a/ld/testsuite/ChangeLog
+++ b/ld/testsuite/ChangeLog
@@ -1,5 +1,12 @@
 2015-01-07  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* ld-scripts/provide-4.d: New file.
+	* ld-scripts/provide-4-map.d: New file.
+	* ld-scripts/provide-4.t: New file.
+	* ld-scripts/provide.exp: Run the provide-4.d test.
+
+2015-01-07  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* ld-scripts/overlay-size.d: Add 'map' option.
 	* ld-scripts/overlay-size.exp: Remove manual check of mapfile.
 	* lib/ld-lib.exp (run_dump_test): Add support for new 'map'
diff --git a/ld/testsuite/ld-scripts/provide-4-map.d b/ld/testsuite/ld-scripts/provide-4-map.d
new file mode 100644
index 0000000..ec95715
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-4-map.d
@@ -0,0 +1,26 @@
+#...
+Linker script and memory map
+
+                0x0000000000000001                PROVIDE \(foo, 0x1\)
+                \[!provide\]                        PROVIDE \(bar, 0x2\)
+                0x0000000000000003                PROVIDE \(baz, 0x3\)
+
+\.text           0x0000000000000000        0x0
+ \.text          0x0000000000000000        0x0 tmpdir/dump0\.o
+
+\.iplt           0x0000000000000000        0x0
+ \.iplt          0x0000000000000000        0x0 tmpdir/dump0\.o
+
+\.rela\.dyn       0x0000000000000000        0x0
+ \.rela\.iplt     0x0000000000000000        0x0 tmpdir/dump0\.o
+ \.rela\.data     0x0000000000000000        0x0 tmpdir/dump0\.o
+
+\.data           0x0000000000002000       0x10
+ \*\(\.data\)
+ \.data          0x0000000000002000       0x10 tmpdir/dump0\.o
+                0x0000000000002000                foo
+                \[!provide\]                        PROVIDE \(loc1, ALIGN \(\., 0x10\)\)
+                0x0000000000002010                PROVIDE \(loc2, ALIGN \(\., 0x10\)\)
+                \[!provide\]                        PROVIDE \(loc3, \(loc1 \+ 0x20\)\)
+                0x0000000000002030                loc4 = \(loc2 \+ 0x20\)
+#...
\ No newline at end of file
diff --git a/ld/testsuite/ld-scripts/provide-4.d b/ld/testsuite/ld-scripts/provide-4.d
new file mode 100644
index 0000000..78482ea
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-4.d
@@ -0,0 +1,10 @@
+#source: provide-2.s
+#ld: -T provide-4.t
+#PROG: nm
+#map: provide-4-map.d
+
+0000000000000003 A baz
+0000000000002000 D foo
+0000000000002010 D loc2
+0000000000002030 A loc4
+
diff --git a/ld/testsuite/ld-scripts/provide-4.t b/ld/testsuite/ld-scripts/provide-4.t
new file mode 100644
index 0000000..424c238
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-4.t
@@ -0,0 +1,16 @@
+SECTIONS
+{
+  PROVIDE (foo = 1);
+  PROVIDE (bar = 2);
+  PROVIDE (baz = 3);
+  .data 0x2000 :
+  {
+    *(.data)
+
+    PROVIDE (loc1 = ALIGN (., 0x10));
+    PROVIDE (loc2 = ALIGN (., 0x10));
+  }
+
+  PROVIDE (loc3 = loc1 + 0x20);
+  loc4 = loc2 + 0x20;
+}
diff --git a/ld/testsuite/ld-scripts/provide.exp b/ld/testsuite/ld-scripts/provide.exp
index 7f45e58..baba0a4 100644
--- a/ld/testsuite/ld-scripts/provide.exp
+++ b/ld/testsuite/ld-scripts/provide.exp
@@ -40,5 +40,6 @@ run_dump_test provide-1
 run_dump_test provide-2
 setup_xfail *-*-*
 run_dump_test provide-3
+run_dump_test provide-4
 
 set LDFLAGS "$saved_LDFLAGS"
-- 
1.9.3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] ld/testing: run_dump_test can now check linker mapfiles.
  2015-01-07 12:15 ` [PATCH 1/2] ld/testing: run_dump_test can now check linker mapfiles Andrew Burgess
@ 2015-01-08  1:21   ` Hans-Peter Nilsson
  2015-01-08 10:16     ` Andrew Burgess
  0 siblings, 1 reply; 16+ messages in thread
From: Hans-Peter Nilsson @ 2015-01-08  1:21 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: binutils

On Wed, 7 Jan 2015, Andrew Burgess wrote:

> This is just a new mechanism to make it easier to test linker mapfiles.

(Not an approver, but:)  Cool.  Just a few nits:

> --- a/ld/testsuite/lib/ld-lib.exp
> +++ b/ld/testsuite/lib/ld-lib.exp
> @@ -555,6 +555,14 @@ proc ld_simple_link_defsyms {} {
>  #	both "error" and "warning".  Multiple "warning" directives
>  #	append to the expected linker warning message.
>  #
> +#   map: FILE
> +#       Adding this option will cause the linker to generate a linker
> +#       map file, using the -Map=MAPFILE command line option.  If thre

"there"

> +#       is no -Map=MAPFILE in the 'ld: FLAGS' then one will be added
> +#       to the linker command line.  The contents of the generated

Consider simplifying: always add it when "map:" is present.
(I just don't see the use in the more complex function.  Keep
it simple.)

> +            if { [regexp -- "-Map=(\[^ \]+)" $cmd all mapfile] } then {
> +                # Found existing mapfile option
> +                verbose -log "Existing mapfile '$mapfile' found"
> +            } else {
> +                # No mapfile option.
> +                set mapfile "tmpdir/dump.map"
> +                verbose -log "Adding mapfile '$mapfile' found"

Spurious cutnpasto "found", last?
(Moot if you delete this block. :)

brgds, H-P

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ld: Don't evaluate unneeded PROVIDE expressions.
  2015-01-07 12:15 ` [PATCH 2/2] ld: Don't evaluate unneeded PROVIDE expressions Andrew Burgess
@ 2015-01-08  1:25   ` Hans-Peter Nilsson
  2015-01-08 10:18     ` Andrew Burgess
  0 siblings, 1 reply; 16+ messages in thread
From: Hans-Peter Nilsson @ 2015-01-08  1:25 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: binutils

Having not looked at the rest, I just spotted:

On Wed, 7 Jan 2015, Andrew Burgess wrote:
> +++ b/ld/testsuite/ld-scripts/provide-4-map.d
> @@ -0,0 +1,26 @@
> +#...
> +Linker script and memory map
> +
> +                0x0000000000000001                PROVIDE \(foo, 0x1\)

For all these addresses, please cut them down to also match
builds with "32-bit bfd's":
+                0x0+1                PROVIDE \(foo, 0x1\)

> +#...
> \ No newline at end of file

Also, please do add one. ;)

brgds, H-P

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] ld/testing: run_dump_test can now check linker mapfiles.
  2015-01-08  1:21   ` Hans-Peter Nilsson
@ 2015-01-08 10:16     ` Andrew Burgess
  2015-01-12  0:38       ` Alan Modra
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Burgess @ 2015-01-08 10:16 UTC (permalink / raw)
  To: binutils; +Cc: Hans-Peter Nilsson

* Hans-Peter Nilsson <hp@bitrange.com> [2015-01-07 20:21:00 -0500]:

> On Wed, 7 Jan 2015, Andrew Burgess wrote:
>
> > This is just a new mechanism to make it easier to test linker mapfiles.
>
> (Not an approver, but:)  Cool.  Just a few nits:

Thanks for the review.  I fixed the typo and copy-'n'-paste-o, however...

>
> Consider simplifying: always add it when "map:" is present.
> (I just don't see the use in the more complex function.  Keep
> it simple.)

... I've left this in for now.  My reasons being, first, I don't think
it actually adds that much complexity (in fact, when I first wrote
this I did always add the -Map, but had an error check to ensure that
the user had not also specified -Map in the options, I realised I was
99% of the way to just using the users mapfile).  As for use case, if
a user wanted to perform any additional checks on the mapfile, the
easiest way would be to just specify -Map in the ld flags.

<shrug> I guess if there's one more vote for remove it then I'm happy
to do that.

Updated patch below.

Thanks,
Andrew

---
Add a new option 'map' to the ld run_dump_test mechanism.  When the
'map' option is given run_dump_test will ensure that there is a
-Map=MAPFILE present in the linker command line, adding one if needed.

The MAPFILE is then compared with the file passed to the new 'map'
option using the regexp_diff function.  This should make it slightly
easier to write tests that check the linker mapfile output.

The only test I found that already compares mapfile content is updated
to use the new mechanism.

ld/testsuite/ChangeLog:

	* ld-scripts/overlay-size.d: Add 'map' option.
	* ld-scripts/overlay-size.exp: Remove manual check of mapfile.
	* lib/ld-lib.exp (run_dump_test): Add support for new 'map'
	option, checking linker mapfile output.
---
 ld/testsuite/ChangeLog                   |  7 +++++++
 ld/testsuite/ld-scripts/overlay-size.d   |  1 +
 ld/testsuite/ld-scripts/overlay-size.exp |  9 ---------
 ld/testsuite/lib/ld-lib.exp              | 34 ++++++++++++++++++++++++++++++++
 4 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
index c9b3ecb..266e996 100644
--- a/ld/testsuite/ChangeLog
+++ b/ld/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2015-01-07  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* ld-scripts/overlay-size.d: Add 'map' option.
+	* ld-scripts/overlay-size.exp: Remove manual check of mapfile.
+	* lib/ld-lib.exp (run_dump_test): Add support for new 'map'
+	option, checking linker mapfile output.
+
 2015-01-06  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* lib/ld-lib.exp (run_dump_test): Extend comment to mention
diff --git a/ld/testsuite/ld-scripts/overlay-size.d b/ld/testsuite/ld-scripts/overlay-size.d
index 78a9c92..4e60150 100644
--- a/ld/testsuite/ld-scripts/overlay-size.d
+++ b/ld/testsuite/ld-scripts/overlay-size.d
@@ -1,6 +1,7 @@
 # ld: -T overlay-size.t -Map tmpdir/overlay-size.map
 # name: overlay size
 # objdump: --headers
+# map: overlay-size-map.d
 # xfail: rx-*-*
 #   FAILS on the RX because the linker has to set LMA == VMA for the
 #   Renesas loader.
diff --git a/ld/testsuite/ld-scripts/overlay-size.exp b/ld/testsuite/ld-scripts/overlay-size.exp
index 4dde289..e8e7a91 100644
--- a/ld/testsuite/ld-scripts/overlay-size.exp
+++ b/ld/testsuite/ld-scripts/overlay-size.exp
@@ -23,12 +23,3 @@ if ![is_elf_format] {
 }
 
 run_dump_test overlay-size
-
-set testname "overlay size (map check)"
-
-if [regexp_diff "tmpdir/overlay-size.map" \
-	        "$srcdir/$subdir/overlay-size-map.d"] {
-    fail $testname
-} else {
-    pass $testname
-}
diff --git a/ld/testsuite/lib/ld-lib.exp b/ld/testsuite/lib/ld-lib.exp
index 3d980c9..0a25f86 100644
--- a/ld/testsuite/lib/ld-lib.exp
+++ b/ld/testsuite/lib/ld-lib.exp
@@ -555,6 +555,14 @@ proc ld_simple_link_defsyms {} {
 #	both "error" and "warning".  Multiple "warning" directives
 #	append to the expected linker warning message.
 #
+#   map: FILE
+#       Adding this option will cause the linker to generate a linker
+#       map file, using the -Map=MAPFILE command line option.  If
+#       there is no -Map=MAPFILE in the 'ld: FLAGS' then one will be
+#       added to the linker command line.  The contents of the
+#       generated MAPFILE are then compared against the regexp lines
+#       in FILE using `regexp_diff' (see below for details).
+#
 # Each option may occur at most once unless otherwise mentioned.
 #
 # After the option lines come regexp lines.  `run_dump_test' calls
@@ -607,6 +615,7 @@ proc run_dump_test { name {extra_options {}} } {
     set opts(warning) {}
     set opts(objcopy_linked_file) {}
     set opts(objcopy_objects) {}
+    set opts(map) {}
 
     foreach i $opt_array {
 	set opt_name [lindex $i 0]
@@ -860,6 +869,20 @@ proc run_dump_test { name {extra_options {}} } {
 	set cmd "$LD $LDFLAGS -L$srcdir/$subdir \
 		   $opts(ld) -o $objfile $objfiles $opts(ld_after_inputfiles)"
 
+        # If needed then check for, or add a -Map option.
+        set mapfile ""
+        if { $opts(map) != "" } then {
+            if { [regexp -- "-Map=(\[^ \]+)" $cmd all mapfile] } then {
+                # Found existing mapfile option
+                verbose -log "Existing mapfile '$mapfile' found"
+            } else {
+                # No mapfile option.
+                set mapfile "tmpdir/dump.map"
+                verbose -log "Adding mapfile '$mapfile'"
+                set cmd "$cmd -Map=$mapfile"
+            }
+        }
+
 	send_log "$cmd\n"
 	set cmdret [remote_exec host [concat sh -c [list "$cmd 2>&1"]] "" "/dev/null" "ld.tmp"]
 	remote_upload host "ld.tmp"
@@ -908,6 +931,17 @@ proc run_dump_test { name {extra_options {}} } {
 		return
 	    }
 	}
+
+        if { $opts(map) != "" } then {
+            # Check the map file matches.
+            set map_pattern_file $srcdir/$subdir/$opts(map)
+            verbose -log "Compare '$mapfile' against '$map_pattern_file'"
+            if { [regexp_diff $mapfile $map_pattern_file] } then {
+                fail "$testname (map file check)"
+            } else {
+                pass "$testname (map file check)"
+            }
+        }
     } else {
 	set objfile "tmpdir/dump0.o"
     }
-- 
1.9.3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ld: Don't evaluate unneeded PROVIDE expressions.
  2015-01-08  1:25   ` Hans-Peter Nilsson
@ 2015-01-08 10:18     ` Andrew Burgess
  2015-01-08 11:08       ` Hans-Peter Nilsson
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Burgess @ 2015-01-08 10:18 UTC (permalink / raw)
  To: binutils; +Cc: Hans-Peter Nilsson

* Hans-Peter Nilsson <hp@bitrange.com> [2015-01-07 20:25:49 -0500]:

> Having not looked at the rest, I just spotted:

Thanks for the review.  I've fixed the issues you pointed out, and
attached an updated patch.

Andrew

---
When creating a linker mapfile (using -Map=MAPFILE), we previously would
always try to evaluate the expression from a PROVIDE statement.

However, this is not always safe, consider:

  PROVIDE (foo = 0x10);
  PROVIDE (bar = foo);

In this example, if neither 'foo' or 'bar' is needed, then while
generating the linker mapfile evaluating the expression for 'foo' is
harmless (just the value 0x10).  However, evaluating the expression for
'bar' requires the symbol 'foo', which is undefined.  This used to cause
a fatal error.

This patch changes the behaviour, so that when the destination of the
PROVIDE is not defined (that is the PROVIDE is not going to provide
anything) the expression is not evaluated, and instead a special string
is displayed to indicate that the linker is discarding the PROVIDE
statement.

This change not only fixes the spurious undefined symbol error, but also
means that a user can now tell if a PROVIDE statement has provided
anything by inspecting the linker mapfile, something that could not be
done before.

ld/ChangeLog:

	* ldlang.c (print_assignment): Only evaluate the expression for a
	PROVIDE'd assignment when the destination is being defined.
	Display a special message for PROVIDE'd symbols that are not being
	provided.

ld/testsuite/ChangeLog:

	* ld-scripts/provide-4.d: New file.
	* ld-scripts/provide-4-map.d: New file.
	* ld-scripts/provide-4.t: New file.
	* ld-scripts/provide.exp: Run the provide-4.d test.
---
 ld/ChangeLog                            |  7 +++++++
 ld/ldlang.c                             | 14 ++++++++++++--
 ld/testsuite/ChangeLog                  |  7 +++++++
 ld/testsuite/ld-scripts/provide-4-map.d | 26 ++++++++++++++++++++++++++
 ld/testsuite/ld-scripts/provide-4.d     | 10 ++++++++++
 ld/testsuite/ld-scripts/provide-4.t     | 16 ++++++++++++++++
 ld/testsuite/ld-scripts/provide.exp     |  1 +
 7 files changed, 79 insertions(+), 2 deletions(-)
 create mode 100644 ld/testsuite/ld-scripts/provide-4-map.d
 create mode 100644 ld/testsuite/ld-scripts/provide-4.d
 create mode 100644 ld/testsuite/ld-scripts/provide-4.t

diff --git a/ld/ChangeLog b/ld/ChangeLog
index be4617f..fab597e 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,10 @@
+2015-01-07  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* ldlang.c (print_assignment): Only evaluate the expression for a
+	PROVIDE'd assignment when the destination is being defined.
+	Display a special message for PROVIDE'd symbols that are not being
+	provided.
+
 2015-01-01  Alan Modra  <amodra@gmail.com>
 
 	* ldver.c (ldversion): Just print current year.
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 9f3d209..a99febe 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -3983,7 +3983,14 @@ print_assignment (lang_assignment_statement_type *assignment,
   osec = output_section->bfd_section;
   if (osec == NULL)
     osec = bfd_abs_section_ptr;
-  exp_fold_tree (tree, osec, &print_dot);
+
+  if (assignment->exp->type.node_class != etree_provide
+      || bfd_link_hash_lookup (link_info.hash,
+                               assignment->exp->assign.dst,
+                               FALSE, FALSE, TRUE) != NULL)
+    exp_fold_tree (tree, osec, &print_dot);
+  else
+    expld.result.valid_p = FALSE;
   if (expld.result.valid_p)
     {
       bfd_vma value;
@@ -4021,7 +4028,10 @@ print_assignment (lang_assignment_statement_type *assignment,
     }
   else
     {
-      minfo ("*undef*   ");
+      if (assignment->exp->type.node_class == etree_provide)
+        minfo ("[!provide]");
+      else
+        minfo ("*undef*   ");
 #ifdef BFD64
       minfo ("        ");
 #endif
diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
index 266e996..521c819 100644
--- a/ld/testsuite/ChangeLog
+++ b/ld/testsuite/ChangeLog
@@ -1,5 +1,12 @@
 2015-01-07  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* ld-scripts/provide-4.d: New file.
+	* ld-scripts/provide-4-map.d: New file.
+	* ld-scripts/provide-4.t: New file.
+	* ld-scripts/provide.exp: Run the provide-4.d test.
+
+2015-01-07  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* ld-scripts/overlay-size.d: Add 'map' option.
 	* ld-scripts/overlay-size.exp: Remove manual check of mapfile.
 	* lib/ld-lib.exp (run_dump_test): Add support for new 'map'
diff --git a/ld/testsuite/ld-scripts/provide-4-map.d b/ld/testsuite/ld-scripts/provide-4-map.d
new file mode 100644
index 0000000..da2ff66
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-4-map.d
@@ -0,0 +1,26 @@
+#...
+Linker script and memory map
+
+                0x0+1                PROVIDE \(foo, 0x1\)
+                \[!provide\]                        PROVIDE \(bar, 0x2\)
+                0x0+3                PROVIDE \(baz, 0x3\)
+
+\.text           0x0+        0x0
+ \.text          0x0+        0x0 tmpdir/dump0\.o
+
+\.iplt           0x0+        0x0
+ \.iplt          0x0+        0x0 tmpdir/dump0\.o
+
+\.rela\.dyn       0x0+        0x0
+ \.rela\.iplt     0x0+        0x0 tmpdir/dump0\.o
+ \.rela\.data     0x0+        0x0 tmpdir/dump0\.o
+
+\.data           0x0+2000       0x10
+ \*\(\.data\)
+ \.data          0x0+2000       0x10 tmpdir/dump0\.o
+                0x0+2000                foo
+                \[!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-4.d b/ld/testsuite/ld-scripts/provide-4.d
new file mode 100644
index 0000000..78482ea
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-4.d
@@ -0,0 +1,10 @@
+#source: provide-2.s
+#ld: -T provide-4.t
+#PROG: nm
+#map: provide-4-map.d
+
+0000000000000003 A baz
+0000000000002000 D foo
+0000000000002010 D loc2
+0000000000002030 A loc4
+
diff --git a/ld/testsuite/ld-scripts/provide-4.t b/ld/testsuite/ld-scripts/provide-4.t
new file mode 100644
index 0000000..424c238
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-4.t
@@ -0,0 +1,16 @@
+SECTIONS
+{
+  PROVIDE (foo = 1);
+  PROVIDE (bar = 2);
+  PROVIDE (baz = 3);
+  .data 0x2000 :
+  {
+    *(.data)
+
+    PROVIDE (loc1 = ALIGN (., 0x10));
+    PROVIDE (loc2 = ALIGN (., 0x10));
+  }
+
+  PROVIDE (loc3 = loc1 + 0x20);
+  loc4 = loc2 + 0x20;
+}
diff --git a/ld/testsuite/ld-scripts/provide.exp b/ld/testsuite/ld-scripts/provide.exp
index 7f45e58..baba0a4 100644
--- a/ld/testsuite/ld-scripts/provide.exp
+++ b/ld/testsuite/ld-scripts/provide.exp
@@ -40,5 +40,6 @@ run_dump_test provide-1
 run_dump_test provide-2
 setup_xfail *-*-*
 run_dump_test provide-3
+run_dump_test provide-4
 
 set LDFLAGS "$saved_LDFLAGS"
-- 
1.9.3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ld: Don't evaluate unneeded PROVIDE expressions.
  2015-01-08 10:18     ` Andrew Burgess
@ 2015-01-08 11:08       ` Hans-Peter Nilsson
  2015-01-08 20:03         ` Andrew Burgess
  0 siblings, 1 reply; 16+ messages in thread
From: Hans-Peter Nilsson @ 2015-01-08 11:08 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: binutils

On Thu, 8 Jan 2015, Andrew Burgess wrote:

> * Hans-Peter Nilsson <hp@bitrange.com> [2015-01-07 20:25:49 -0500]:
>
> > Having not looked at the rest, I just spotted:
>
> Thanks for the review.  I've fixed the issues you pointed out, and
> attached an updated patch.

Same thing in:

> +++ b/ld/testsuite/ld-scripts/provide-4.d
> @@ -0,0 +1,10 @@
> +#source: provide-2.s
> +#ld: -T provide-4.t
> +#PROG: nm
> +#map: provide-4-map.d
> +
> +0000000000000003 A baz

brgds, H-P

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ld: Don't evaluate unneeded PROVIDE expressions.
  2015-01-08 11:08       ` Hans-Peter Nilsson
@ 2015-01-08 20:03         ` Andrew Burgess
  2015-01-12  0:39           ` Alan Modra
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Burgess @ 2015-01-08 20:03 UTC (permalink / raw)
  To: binutils; +Cc: Hans-Peter Nilsson

* Hans-Peter Nilsson <hp@bitrange.com> [2015-01-08 06:08:21 -0500]:

> On Thu, 8 Jan 2015, Andrew Burgess wrote:
>
> > * Hans-Peter Nilsson <hp@bitrange.com> [2015-01-07 20:25:49 -0500]:
> >
> > > Having not looked at the rest, I just spotted:
> >
> > Thanks for the review.  I've fixed the issues you pointed out, and
> > attached an updated patch.
>
> Same thing in:

Ooops.  Also fixed.  Sorry for now spotting that.

Thanks,
Andrew

---

When creating a linker mapfile (using -Map=MAPFILE), we previously would
always try to evaluate the expression from a PROVIDE statement.

However, this is not always safe, consider:

  PROVIDE (foo = 0x10);
  PROVIDE (bar = foo);

In this example, if neither 'foo' or 'bar' is needed, then while
generating the linker mapfile evaluating the expression for 'foo' is
harmless (just the value 0x10).  However, evaluating the expression for
'bar' requires the symbol 'foo', which is undefined.  This used to cause
a fatal error.

This patch changes the behaviour, so that when the destination of the
PROVIDE is not defined (that is the PROVIDE is not going to provide
anything) the expression is not evaluated, and instead a special string
is displayed to indicate that the linker is discarding the PROVIDE
statement.

This change not only fixes the spurious undefined symbol error, but also
means that a user can now tell if a PROVIDE statement has provided
anything by inspecting the linker mapfile, something that could not be
done before.

ld/ChangeLog:

	* ldlang.c (print_assignment): Only evaluate the expression for a
	PROVIDE'd assignment when the destination is being defined.
	Display a special message for PROVIDE'd symbols that are not being
	provided.

ld/testsuite/ChangeLog:

	* ld-scripts/provide-4.d: New file.
	* ld-scripts/provide-4-map.d: New file.
	* ld-scripts/provide-4.t: New file.
	* ld-scripts/provide.exp: Run the provide-4.d test.
---
 ld/ChangeLog                            |  7 +++++++
 ld/ldlang.c                             | 14 ++++++++++++--
 ld/testsuite/ChangeLog                  |  7 +++++++
 ld/testsuite/ld-scripts/provide-4-map.d | 26 ++++++++++++++++++++++++++
 ld/testsuite/ld-scripts/provide-4.d     |  9 +++++++++
 ld/testsuite/ld-scripts/provide-4.t     | 16 ++++++++++++++++
 ld/testsuite/ld-scripts/provide.exp     |  1 +
 7 files changed, 78 insertions(+), 2 deletions(-)
 create mode 100644 ld/testsuite/ld-scripts/provide-4-map.d
 create mode 100644 ld/testsuite/ld-scripts/provide-4.d
 create mode 100644 ld/testsuite/ld-scripts/provide-4.t

diff --git a/ld/ChangeLog b/ld/ChangeLog
index be4617f..fab597e 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,10 @@
+2015-01-07  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* ldlang.c (print_assignment): Only evaluate the expression for a
+	PROVIDE'd assignment when the destination is being defined.
+	Display a special message for PROVIDE'd symbols that are not being
+	provided.
+
 2015-01-01  Alan Modra  <amodra@gmail.com>
 
 	* ldver.c (ldversion): Just print current year.
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 9f3d209..a99febe 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -3983,7 +3983,14 @@ print_assignment (lang_assignment_statement_type *assignment,
   osec = output_section->bfd_section;
   if (osec == NULL)
     osec = bfd_abs_section_ptr;
-  exp_fold_tree (tree, osec, &print_dot);
+
+  if (assignment->exp->type.node_class != etree_provide
+      || bfd_link_hash_lookup (link_info.hash,
+                               assignment->exp->assign.dst,
+                               FALSE, FALSE, TRUE) != NULL)
+    exp_fold_tree (tree, osec, &print_dot);
+  else
+    expld.result.valid_p = FALSE;
   if (expld.result.valid_p)
     {
       bfd_vma value;
@@ -4021,7 +4028,10 @@ print_assignment (lang_assignment_statement_type *assignment,
     }
   else
     {
-      minfo ("*undef*   ");
+      if (assignment->exp->type.node_class == etree_provide)
+        minfo ("[!provide]");
+      else
+        minfo ("*undef*   ");
 #ifdef BFD64
       minfo ("        ");
 #endif
diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
index 266e996..521c819 100644
--- a/ld/testsuite/ChangeLog
+++ b/ld/testsuite/ChangeLog
@@ -1,5 +1,12 @@
 2015-01-07  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* ld-scripts/provide-4.d: New file.
+	* ld-scripts/provide-4-map.d: New file.
+	* ld-scripts/provide-4.t: New file.
+	* ld-scripts/provide.exp: Run the provide-4.d test.
+
+2015-01-07  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* ld-scripts/overlay-size.d: Add 'map' option.
 	* ld-scripts/overlay-size.exp: Remove manual check of mapfile.
 	* lib/ld-lib.exp (run_dump_test): Add support for new 'map'
diff --git a/ld/testsuite/ld-scripts/provide-4-map.d b/ld/testsuite/ld-scripts/provide-4-map.d
new file mode 100644
index 0000000..da2ff66
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-4-map.d
@@ -0,0 +1,26 @@
+#...
+Linker script and memory map
+
+                0x0+1                PROVIDE \(foo, 0x1\)
+                \[!provide\]                        PROVIDE \(bar, 0x2\)
+                0x0+3                PROVIDE \(baz, 0x3\)
+
+\.text           0x0+        0x0
+ \.text          0x0+        0x0 tmpdir/dump0\.o
+
+\.iplt           0x0+        0x0
+ \.iplt          0x0+        0x0 tmpdir/dump0\.o
+
+\.rela\.dyn       0x0+        0x0
+ \.rela\.iplt     0x0+        0x0 tmpdir/dump0\.o
+ \.rela\.data     0x0+        0x0 tmpdir/dump0\.o
+
+\.data           0x0+2000       0x10
+ \*\(\.data\)
+ \.data          0x0+2000       0x10 tmpdir/dump0\.o
+                0x0+2000                foo
+                \[!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-4.d b/ld/testsuite/ld-scripts/provide-4.d
new file mode 100644
index 0000000..021bea2
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-4.d
@@ -0,0 +1,9 @@
+#source: provide-2.s
+#ld: -T provide-4.t
+#PROG: nm
+#map: provide-4-map.d
+
+0+3 A baz
+0+2000 D foo
+0+2010 D loc2
+0+2030 A loc4
diff --git a/ld/testsuite/ld-scripts/provide-4.t b/ld/testsuite/ld-scripts/provide-4.t
new file mode 100644
index 0000000..424c238
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-4.t
@@ -0,0 +1,16 @@
+SECTIONS
+{
+  PROVIDE (foo = 1);
+  PROVIDE (bar = 2);
+  PROVIDE (baz = 3);
+  .data 0x2000 :
+  {
+    *(.data)
+
+    PROVIDE (loc1 = ALIGN (., 0x10));
+    PROVIDE (loc2 = ALIGN (., 0x10));
+  }
+
+  PROVIDE (loc3 = loc1 + 0x20);
+  loc4 = loc2 + 0x20;
+}
diff --git a/ld/testsuite/ld-scripts/provide.exp b/ld/testsuite/ld-scripts/provide.exp
index 7f45e58..baba0a4 100644
--- a/ld/testsuite/ld-scripts/provide.exp
+++ b/ld/testsuite/ld-scripts/provide.exp
@@ -40,5 +40,6 @@ run_dump_test provide-1
 run_dump_test provide-2
 setup_xfail *-*-*
 run_dump_test provide-3
+run_dump_test provide-4
 
 set LDFLAGS "$saved_LDFLAGS"
-- 
1.9.3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] ld/testing: run_dump_test can now check linker mapfiles.
  2015-01-08 10:16     ` Andrew Burgess
@ 2015-01-12  0:38       ` Alan Modra
  0 siblings, 0 replies; 16+ messages in thread
From: Alan Modra @ 2015-01-12  0:38 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: binutils, Hans-Peter Nilsson

On Thu, Jan 08, 2015 at 10:16:15AM +0000, Andrew Burgess wrote:
> 	* ld-scripts/overlay-size.d: Add 'map' option.
> 	* ld-scripts/overlay-size.exp: Remove manual check of mapfile.
> 	* lib/ld-lib.exp (run_dump_test): Add support for new 'map'
> 	option, checking linker mapfile output.

OK.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ld: Don't evaluate unneeded PROVIDE expressions.
  2015-01-08 20:03         ` Andrew Burgess
@ 2015-01-12  0:39           ` Alan Modra
  2015-01-19 11:06             ` Andrew Burgess
  0 siblings, 1 reply; 16+ messages in thread
From: Alan Modra @ 2015-01-12  0:39 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: binutils, Hans-Peter Nilsson

On Thu, Jan 08, 2015 at 08:03:04PM +0000, Andrew Burgess wrote:
> ld/ChangeLog:
> 
> 	* ldlang.c (print_assignment): Only evaluate the expression for a
> 	PROVIDE'd assignment when the destination is being defined.
> 	Display a special message for PROVIDE'd symbols that are not being
> 	provided.

This is OK.

> ld/testsuite/ChangeLog:
> 
> 	* ld-scripts/provide-4.d: New file.
> 	* ld-scripts/provide-4-map.d: New file.
> 	* ld-scripts/provide-4.t: New file.
> 	* ld-scripts/provide.exp: Run the provide-4.d test.

This part isn't OK yet.  You need to make sure new tests pass on more
than just Linux x86.  I could see this would fail so applied your
patches and gave my usual set of targets a whirl, results below.  To
fix this you can either restrict the set of targets tested (the lazy
option), or relax the .d file matching to the minimum you need to
verify the feature being tested.

aarch64-linux  +FAIL: ld-scripts/provide-4 (map file check)
alpha-dec-vms  +FAIL: ld-scripts/provide-4
alpha-linux  +FAIL: ld-scripts/provide-4 (map file check)
alpha-linuxecoff  +FAIL: ld-scripts/provide-4 (map file check)
alpha-netbsd  +FAIL: ld-scripts/provide-4 (map file check)
alpha-unknown-freebsd4.7  +FAIL: ld-scripts/provide-4 (map file check)
am33_2.0-linux  +FAIL: ld-scripts/provide-4 (map file check)
arc-elf  +FAIL: ld-scripts/provide-4 (map file check)
arm-coff  +FAIL: ld-scripts/provide-4 (map file check)
arm-coff  +FAIL: ld-scripts/provide-4
arm-epoc-pe  +FAIL: ld-scripts/provide-4 (map file check)
arm-epoc-pe  +FAIL: ld-scripts/provide-4
arm-linuxeabi  +FAIL: ld-scripts/provide-4 (map file check)
arm-nacl  +FAIL: ld-scripts/provide-4 (map file check)
arm-netbsdelf  +FAIL: ld-scripts/provide-4 (map file check)
arm-nto  +FAIL: ld-scripts/provide-4 (map file check)
arm-pe  +FAIL: ld-scripts/provide-4 (map file check)
arm-pe  +FAIL: ld-scripts/provide-4
arm-symbianelf  +FAIL: ld-scripts/provide-4 (map file check)
arm-symbianelf  +FAIL: ld-scripts/provide-4
arm-vxworks  +FAIL: ld-scripts/provide-4 (map file check)
arm-wince-pe  +FAIL: ld-scripts/provide-4 (map file check)
arm-wince-pe  +FAIL: ld-scripts/provide-4
avr-elf  +FAIL: ld-scripts/provide-4 (map file check)
bfin-elf  +FAIL: ld-scripts/provide-4 (map file check)
cr16-elf  +FAIL: ld-scripts/provide-4 (map file check)
cris-elf  +FAIL: ld-scripts/provide-4 (map file check)
crisv32-linux  +FAIL: ld-scripts/provide-4 (map file check)
crx-elf  +FAIL: ld-scripts/provide-4 (map file check)
d10v-elf  +FAIL: ld-scripts/provide-4 (map file check)
d30v-elf  +FAIL: ld-scripts/provide-4 (map file check)
dlx-elf  +FAIL: ld-scripts/provide-4 (map file check)
fr30-elf  +FAIL: ld-scripts/provide-4 (map file check)
frv-elf  +FAIL: ld-scripts/provide-4 (map file check)
frv-linux  +FAIL: ld-scripts/provide-4 (map file check)
frv-linux  +FAIL: ld-scripts/provide-4
h8300-elf  +FAIL: ld-scripts/provide-4 (map file check)
hppa64-hp-hpux11.23  +FAIL: ld-scripts/provide-4 (map file check)
hppa64-hp-hpux11.23  +FAIL: ld-scripts/provide-4
hppa64-linux  +FAIL: ld-scripts/provide-4 (map file check)
hppa64-linux  +FAIL: ld-scripts/provide-4
hppa-linux  +FAIL: ld-scripts/provide-4 (map file check)
i370-linux  +FAIL: ld-scripts/provide-4 (map file check)
i386-lynxos  +FAIL: ld-scripts/provide-4 (map file check)
i386-netware  +FAIL: ld-scripts/provide-4 (map file check)
i586-coff  +FAIL: ld-scripts/provide-4 (map file check)
i586-coff  +FAIL: ld-scripts/provide-4
i586-linux  +FAIL: ld-scripts/provide-4 (map file check)
i686-nacl  +FAIL: ld-scripts/provide-4 (map file check)
i686-pc-beos  +FAIL: ld-scripts/provide-4 (map file check)
i686-pc-elf  +FAIL: ld-scripts/provide-4 (map file check)
i686-pe  +FAIL: ld-scripts/provide-4 (map file check)
i686-pe  +FAIL: ld-scripts/provide-4
i860-stardent-elf  +FAIL: ld-scripts/provide-4 (map file check)
i960-elf  +FAIL: ld-scripts/provide-4 (map file check)
ia64-elf  +FAIL: ld-scripts/provide-4 (map file check)
ia64-freebsd5  +FAIL: ld-scripts/provide-4 (map file check)
ia64-linux  +FAIL: ld-scripts/provide-4 (map file check)
ia64-netbsd  +FAIL: ld-scripts/provide-4 (map file check)
ip2k-elf  +FAIL: ld-scripts/provide-4 (map file check)
iq2000-elf  +FAIL: ld-scripts/provide-4 (map file check)
lm32-elf  +FAIL: ld-scripts/provide-4 (map file check)
m32c-elf  +FAIL: ld-scripts/provide-4 (map file check)
m32r-elf  +FAIL: ld-scripts/provide-4 (map file check)
m68hc11-elf  +FAIL: ld-scripts/provide-4 (map file check)
m68hc12-elf  +FAIL: ld-scripts/provide-4 (map file check)
m68k-elf  +FAIL: ld-scripts/provide-4 (map file check)
m68k-linux  +FAIL: ld-scripts/provide-4 (map file check)
mcore-elf  +FAIL: ld-scripts/provide-4 (map file check)
mcore-pe  +FAIL: ld-scripts/provide-4 (map file check)
mcore-pe  +FAIL: ld-scripts/provide-4
mep-elf  +FAIL: ld-scripts/provide-4 (map file check)
microblaze-elf  +FAIL: ld-scripts/provide-4 (map file check)
mips64-linux  +FAIL: ld-scripts/provide-4 (map file check)
mipsel-linux-gnu  +FAIL: ld-scripts/provide-4 (map file check)
mipsisa32el-linux  +FAIL: ld-scripts/provide-4 (map file check)
mips-linux  +FAIL: ld-scripts/provide-4 (map file check)
mmix  +FAIL: ld-scripts/provide-4 (map file check)
mmix  +FAIL: ld-scripts/provide-4
mn10200-elf  +FAIL: ld-scripts/provide-4 (map file check)
mn10300-elf  +FAIL: ld-scripts/provide-4 (map file check)
moxie-elf  +FAIL: ld-scripts/provide-4 (map file check)
ms1-elf  +FAIL: ld-scripts/provide-4 (map file check)
msp430-elf  +FAIL: ld-scripts/provide-4 (map file check)
msp430-elf  +FAIL: ld-scripts/provide-4
nios2-linux  +FAIL: ld-scripts/provide-4 (map file check)
or1k-elf  +FAIL: ld-scripts/provide-4 (map file check)
pj-elf  +FAIL: ld-scripts/provide-4 (map file check)
powerpc64le-elf  +FAIL: ld-scripts/provide-4 (map file check)
powerpc64-linux  +FAIL: ld-scripts/provide-4 (map file check)
powerpc-eabisim  +FAIL: ld-scripts/provide-4 (map file check)
powerpcle-cygwin  +FAIL: ld-scripts/provide-4
powerpcle-elf  +FAIL: ld-scripts/provide-4 (map file check)
powerpc-linux  +FAIL: ld-scripts/provide-4 (map file check)
powerpc-nto  +FAIL: ld-scripts/provide-4 (map file check)
powerpc-wrs-vxworks  +FAIL: ld-scripts/provide-4 (map file check)
ppc-lynxos  +FAIL: ld-scripts/provide-4 (map file check)
ppc-lynxos  +FAIL: ld-scripts/provide-4
rl78-elf  +FAIL: ld-scripts/provide-4 (map file check)
rx-elf  +XPASS: overlay size (map file check)
rx-elf  +FAIL: overlay size
rx-elf  +FAIL: ld-scripts/provide-4 (map file check)
sh64-elf  +FAIL: ld-scripts/provide-4 (map file check)
sh-linux  +FAIL: ld-scripts/provide-4 (map file check)
shl-unknown-netbsdelf  +FAIL: ld-scripts/provide-4 (map file check)
sh-nto  +FAIL: ld-scripts/provide-4 (map file check)
sh-pe  +FAIL: ld-scripts/provide-4 (map file check)
sh-pe  +FAIL: ld-scripts/provide-4
sh-rtems  +FAIL: ld-scripts/provide-4 (map file check)
sparc64-linux  +FAIL: ld-scripts/provide-4 (map file check)
sparc-coff  +FAIL: ld-scripts/provide-4 (map file check)
sparc-coff  +FAIL: ld-scripts/provide-4
sparc-linux  +FAIL: ld-scripts/provide-4 (map file check)
spu-elf  +FAIL: ld-scripts/provide-4 (map file check)
tic30-unknown-coff  +FAIL: ld-scripts/provide-4 (map file check)
tic30-unknown-coff  +FAIL: ld-scripts/provide-4
tic4x-coff  +FAIL: ld-scripts/provide-4 (map file check)
tic4x-coff  +FAIL: ld-scripts/provide-4
tic54x-coff  +FAIL: ld-scripts/provide-4
tic6x-elf  +FAIL: ld-scripts/provide-4 (map file check)
tilegx-linux  +FAIL: ld-scripts/provide-4 (map file check)
tilepro-linux  +FAIL: ld-scripts/provide-4 (map file check)
tx39-elf  +FAIL: ld-scripts/provide-4 (map file check)
v850-elf  +FAIL: ld-scripts/provide-4 (map file check)
vax-netbsdelf  +FAIL: ld-scripts/provide-4 (map file check)
x86_64-mingw32  +FAIL: ld-scripts/provide-4 (map file check)
x86_64-mingw32  +FAIL: ld-scripts/provide-4
xgate-elf  +FAIL: ld-scripts/provide-4 (map file check)
xstormy16-elf  +FAIL: ld-scripts/provide-4 (map file check)
xtensa-elf  +FAIL: ld-scripts/provide-4 (map file check)
z80-coff  +FAIL: ld-scripts/provide-4 (map file check)
z80-coff  +FAIL: ld-scripts/provide-4
z8k-coff  +FAIL: ld-scripts/provide-4 (map file check)
z8k-coff  +FAIL: ld-scripts/provide-4

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ld: Don't evaluate unneeded PROVIDE expressions.
  2015-01-12  0:39           ` Alan Modra
@ 2015-01-19 11:06             ` Andrew Burgess
  2015-01-19 12:50               ` Alan Modra
                                 ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Andrew Burgess @ 2015-01-19 11:06 UTC (permalink / raw)
  To: binutils; +Cc: Alan Modra, Hans-Peter Nilsson

* Alan Modra <amodra@gmail.com> [2015-01-12 11:08:53 +1030]:

> > ld/testsuite/ChangeLog:
> >
> > 	* ld-scripts/provide-4.d: New file.
> > 	* ld-scripts/provide-4-map.d: New file.
> > 	* ld-scripts/provide-4.t: New file.
> > 	* ld-scripts/provide.exp: Run the provide-4.d test.
>
> This part isn't OK yet.  You need to make sure new tests pass on more
> than just Linux x86.

Alan,

Thanks for taking the time to review my patch, and I'm sorry that the
change wasn't up to standard.

I've updated the expected results, and tested against the same set of
builds you suggest, I'm now seeing passes on all but 3 builds, and
these 3 failures are, I believe, not related to my changes.

I could look to XFAIL my new test for those targets, but the same
targets are FAIL-ing other tests from the 'provide.exp' file, it's
your call.

The 3 misbehaving builds are alpha-dec-vms, powerpcle-cygwin, and
tic54x-coff.  It is quite possible that I've not configured or built
these targets correctly, I have no experience with any of these.

Revised patch is below, the only change is to the testsuite directory.

Thanks again,
Andrew

---
When creating a linker mapfile (using -Map=MAPFILE), we previously would
always try to evaluate the expression from a PROVIDE statement.

However, this is not always safe, consider:

  PROVIDE (foo = 0x10);
  PROVIDE (bar = foo);

In this example, if neither 'foo' or 'bar' is needed, then while
generating the linker mapfile evaluating the expression for 'foo' is
harmless (just the value 0x10).  However, evaluating the expression for
'bar' requires the symbol 'foo', which is undefined.  This used to cause
a fatal error.

This patch changes the behaviour, so that when the destination of the
PROVIDE is not defined (that is the PROVIDE is not going to provide
anything) the expression is not evaluated, and instead a special string
is displayed to indicate that the linker is discarding the PROVIDE
statement.

This change not only fixes the spurious undefined symbol error, but also
means that a user can now tell if a PROVIDE statement has provided
anything by inspecting the linker mapfile, something that could not be
done before.

ld/ChangeLog:

	* ldlang.c (print_assignment): Only evaluate the expression for a
	PROVIDE'd assignment when the destination is being defined.
	Display a special message for PROVIDE'd symbols that are not being
	provided.

ld/testsuite/ChangeLog:

	* ld-scripts/provide-4.d: New file.
	* ld-scripts/provide-4-map.d: New file.
	* ld-scripts/provide-4.t: New file.
	* ld-scripts/provide.exp: Run the provide-4.d test.
---
 ld/ChangeLog                            |  7 +++++++
 ld/ldlang.c                             | 14 ++++++++++++--
 ld/testsuite/ChangeLog                  |  7 +++++++
 ld/testsuite/ld-scripts/provide-4-map.d | 13 +++++++++++++
 ld/testsuite/ld-scripts/provide-4.d     |  9 +++++++++
 ld/testsuite/ld-scripts/provide-4.t     | 16 ++++++++++++++++
 ld/testsuite/ld-scripts/provide.exp     |  1 +
 7 files changed, 65 insertions(+), 2 deletions(-)
 create mode 100644 ld/testsuite/ld-scripts/provide-4-map.d
 create mode 100644 ld/testsuite/ld-scripts/provide-4.d
 create mode 100644 ld/testsuite/ld-scripts/provide-4.t

diff --git a/ld/ChangeLog b/ld/ChangeLog
index be4617f..fab597e 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,10 @@
+2015-01-07  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* ldlang.c (print_assignment): Only evaluate the expression for a
+	PROVIDE'd assignment when the destination is being defined.
+	Display a special message for PROVIDE'd symbols that are not being
+	provided.
+
 2015-01-01  Alan Modra  <amodra@gmail.com>
 
 	* ldver.c (ldversion): Just print current year.
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 9f3d209..a99febe 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -3983,7 +3983,14 @@ print_assignment (lang_assignment_statement_type *assignment,
   osec = output_section->bfd_section;
   if (osec == NULL)
     osec = bfd_abs_section_ptr;
-  exp_fold_tree (tree, osec, &print_dot);
+
+  if (assignment->exp->type.node_class != etree_provide
+      || bfd_link_hash_lookup (link_info.hash,
+                               assignment->exp->assign.dst,
+                               FALSE, FALSE, TRUE) != NULL)
+    exp_fold_tree (tree, osec, &print_dot);
+  else
+    expld.result.valid_p = FALSE;
   if (expld.result.valid_p)
     {
       bfd_vma value;
@@ -4021,7 +4028,10 @@ print_assignment (lang_assignment_statement_type *assignment,
     }
   else
     {
-      minfo ("*undef*   ");
+      if (assignment->exp->type.node_class == etree_provide)
+        minfo ("[!provide]");
+      else
+        minfo ("*undef*   ");
 #ifdef BFD64
       minfo ("        ");
 #endif
diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
index 266e996..521c819 100644
--- a/ld/testsuite/ChangeLog
+++ b/ld/testsuite/ChangeLog
@@ -1,5 +1,12 @@
 2015-01-07  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* ld-scripts/provide-4.d: New file.
+	* ld-scripts/provide-4-map.d: New file.
+	* ld-scripts/provide-4.t: New file.
+	* ld-scripts/provide.exp: Run the provide-4.d test.
+
+2015-01-07  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* ld-scripts/overlay-size.d: Add 'map' option.
 	* ld-scripts/overlay-size.exp: Remove manual check of mapfile.
 	* lib/ld-lib.exp (run_dump_test): Add support for new 'map'
diff --git a/ld/testsuite/ld-scripts/provide-4-map.d b/ld/testsuite/ld-scripts/provide-4-map.d
new file mode 100644
index 0000000..59c84b5
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-4-map.d
@@ -0,0 +1,13 @@
+#...
+Linker script and memory map
+#...
+                0x0+1                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\)\)
+                0x0+2030                loc4 = \(loc2 \+ 0x20\)
+#...
diff --git a/ld/testsuite/ld-scripts/provide-4.d b/ld/testsuite/ld-scripts/provide-4.d
new file mode 100644
index 0000000..a7d37e3
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-4.d
@@ -0,0 +1,9 @@
+#source: provide-2.s
+#ld: -T provide-4.t
+#PROG: nm
+#map: provide-4-map.d
+#...
+0+3 A baz
+0+2000 D foo
+0+2010 D loc2
+0+2030 A loc4
diff --git a/ld/testsuite/ld-scripts/provide-4.t b/ld/testsuite/ld-scripts/provide-4.t
new file mode 100644
index 0000000..424c238
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-4.t
@@ -0,0 +1,16 @@
+SECTIONS
+{
+  PROVIDE (foo = 1);
+  PROVIDE (bar = 2);
+  PROVIDE (baz = 3);
+  .data 0x2000 :
+  {
+    *(.data)
+
+    PROVIDE (loc1 = ALIGN (., 0x10));
+    PROVIDE (loc2 = ALIGN (., 0x10));
+  }
+
+  PROVIDE (loc3 = loc1 + 0x20);
+  loc4 = loc2 + 0x20;
+}
diff --git a/ld/testsuite/ld-scripts/provide.exp b/ld/testsuite/ld-scripts/provide.exp
index 7f45e58..baba0a4 100644
--- a/ld/testsuite/ld-scripts/provide.exp
+++ b/ld/testsuite/ld-scripts/provide.exp
@@ -40,5 +40,6 @@ run_dump_test provide-1
 run_dump_test provide-2
 setup_xfail *-*-*
 run_dump_test provide-3
+run_dump_test provide-4
 
 set LDFLAGS "$saved_LDFLAGS"
-- 
1.9.3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ld: Don't evaluate unneeded PROVIDE expressions.
  2015-01-19 11:06             ` Andrew Burgess
@ 2015-01-19 12:50               ` Alan Modra
  2015-01-20 10:21               ` Andrew Burgess
  2015-01-20 14:21               ` Hans-Peter Nilsson
  2 siblings, 0 replies; 16+ messages in thread
From: Alan Modra @ 2015-01-19 12:50 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: binutils, Hans-Peter Nilsson

On Mon, Jan 19, 2015 at 11:06:20AM +0000, Andrew Burgess wrote:
> Revised patch is below, the only change is to the testsuite directory.

This is good, thanks!

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ld: Don't evaluate unneeded PROVIDE expressions.
  2015-01-19 11:06             ` Andrew Burgess
  2015-01-19 12:50               ` Alan Modra
@ 2015-01-20 10:21               ` Andrew Burgess
  2015-01-20 14:21               ` Hans-Peter Nilsson
  2 siblings, 0 replies; 16+ messages in thread
From: Andrew Burgess @ 2015-01-20 10:21 UTC (permalink / raw)
  To: binutils; +Cc: Alan Modra, Hans-Peter Nilsson

* Andrew Burgess <andrew.burgess@embecosm.com> [2015-01-19 11:06:20 +0000]:

>
> Revised patch is below, the only change is to the testsuite directory.
>

Just before pushing the change I realised that there was a bug in my
code.  I now have a revised version of my code, which is actually
simpler, I had previously failed to realise that an expression could
have class etree_provide (unused PROVIDE), or etree_provided (used
PROVIDE), the difference between these two tells me everything I need
to know.

I checked this against the same 110 targets as before and all pass.

Given how similar this version is to the previous, approved, version,
I'm just going to push this.

Thanks,
Andrew

---

When creating a linker mapfile (using -Map=MAPFILE), we previously would
always try to evaluate the expression from a PROVIDE statement.

However, this is not always safe, consider:

  PROVIDE (foo = 0x10);
  PROVIDE (bar = foo);

In this example, if neither 'foo' or 'bar' is needed, then while
generating the linker mapfile evaluating the expression for 'foo' is
harmless (just the value 0x10).  However, evaluating the expression for
'bar' requires the symbol 'foo', which is undefined.  This used to cause
a fatal error.

This patch changes the behaviour, so that when the destination of the
PROVIDE is not defined (that is the PROVIDE is not going to provide
anything) the expression is not evaluated, and instead a special string
is displayed to indicate that the linker is discarding the PROVIDE
statement.

This change not only fixes the spurious undefined symbol error, but also
means that a user can now tell if a PROVIDE statement has provided
anything by inspecting the linker mapfile, something that could not be
done before.

ld/ChangeLog:

	* ldlang.c (print_assignment): Only evaluate the expression for a
	PROVIDE'd assignment when the destination is being defined.
	Display a special message for PROVIDE'd symbols that are not being
	provided.

ld/testsuite/ChangeLog:

	* ld-scripts/provide-4.d: New file.
	* ld-scripts/provide-4-map.d: New file.
	* ld-scripts/provide-4.t: New file.
	* ld-scripts/provide-5.d: New file.
	* ld-scripts/provide-5.s: New file.
	* ld-scripts/provide-5-map.d: New file.
	* ld-scripts/provide-5.t: New file.
	* ld-scripts/provide.exp: Run the provide-4.d and provide-5.d
	tests.
---
 ld/ChangeLog                            |  7 +++++++
 ld/ldlang.c                             | 12 ++++++++++--
 ld/testsuite/ChangeLog                  | 12 ++++++++++++
 ld/testsuite/ld-scripts/provide-4-map.d | 13 +++++++++++++
 ld/testsuite/ld-scripts/provide-4.d     |  9 +++++++++
 ld/testsuite/ld-scripts/provide-4.t     | 16 ++++++++++++++++
 ld/testsuite/ld-scripts/provide-5-map.d |  6 ++++++
 ld/testsuite/ld-scripts/provide-5.d     |  6 ++++++
 ld/testsuite/ld-scripts/provide-5.s     |  4 ++++
 ld/testsuite/ld-scripts/provide-5.t     | 10 ++++++++++
 ld/testsuite/ld-scripts/provide.exp     |  2 ++
 11 files changed, 95 insertions(+), 2 deletions(-)
 create mode 100644 ld/testsuite/ld-scripts/provide-4-map.d
 create mode 100644 ld/testsuite/ld-scripts/provide-4.d
 create mode 100644 ld/testsuite/ld-scripts/provide-4.t
 create mode 100644 ld/testsuite/ld-scripts/provide-5-map.d
 create mode 100644 ld/testsuite/ld-scripts/provide-5.d
 create mode 100644 ld/testsuite/ld-scripts/provide-5.s
 create mode 100644 ld/testsuite/ld-scripts/provide-5.t

diff --git a/ld/ChangeLog b/ld/ChangeLog
index 1ca6fe3..4641ff0 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,10 @@
+2015-01-20  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* ldlang.c (print_assignment): Only evaluate the expression for a
+	PROVIDE'd assignment when the destination is being defined.
+	Display a special message for PROVIDE'd symbols that are not being
+	provided.
+
 2015-01-20  Alan Modra  <amodra@gmail.com>
 
 	* emulparams/elf64ppc.sh (BSS_PLT): Don't define.
diff --git a/ld/ldlang.c b/ld/ldlang.c
index 0c72333..3ea22c2 100644
--- a/ld/ldlang.c
+++ b/ld/ldlang.c
@@ -3983,7 +3983,12 @@ print_assignment (lang_assignment_statement_type *assignment,
   osec = output_section->bfd_section;
   if (osec == NULL)
     osec = bfd_abs_section_ptr;
-  exp_fold_tree (tree, osec, &print_dot);
+
+  if (assignment->exp->type.node_class != etree_provide)
+    exp_fold_tree (tree, osec, &print_dot);
+  else
+    expld.result.valid_p = FALSE;
+
   if (expld.result.valid_p)
     {
       bfd_vma value;
@@ -4021,7 +4026,10 @@ print_assignment (lang_assignment_statement_type *assignment,
     }
   else
     {
-      minfo ("*undef*   ");
+      if (assignment->exp->type.node_class == etree_provide)
+        minfo ("[!provide]");
+      else
+        minfo ("*undef*   ");
 #ifdef BFD64
       minfo ("        ");
 #endif
diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
index b6e3481..0b47494 100644
--- a/ld/testsuite/ChangeLog
+++ b/ld/testsuite/ChangeLog
@@ -1,5 +1,17 @@
 2015-01-20  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* ld-scripts/provide-4.d: New file.
+	* ld-scripts/provide-4-map.d: New file.
+	* ld-scripts/provide-4.t: New file.
+	* ld-scripts/provide-5.d: New file.
+	* ld-scripts/provide-5.s: New file.
+	* ld-scripts/provide-5-map.d: New file.
+	* ld-scripts/provide-5.t: New file.
+	* ld-scripts/provide.exp: Run the provide-4.d and provide-5.d
+	tests.
+
+2015-01-20  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* ld-scripts/overlay-size.d: Add 'map' option.
 	* ld-scripts/overlay-size.exp: Remove manual check of mapfile.
 	* lib/ld-lib.exp (run_dump_test): Add support for new 'map'
diff --git a/ld/testsuite/ld-scripts/provide-4-map.d b/ld/testsuite/ld-scripts/provide-4-map.d
new file mode 100644
index 0000000..d8e4a28
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-4-map.d
@@ -0,0 +1,13 @@
+#...
+Linker script and memory map
+#...
+                \[!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\)\)
+                0x0+2030                loc4 = \(loc2 \+ 0x20\)
+#...
diff --git a/ld/testsuite/ld-scripts/provide-4.d b/ld/testsuite/ld-scripts/provide-4.d
new file mode 100644
index 0000000..a7d37e3
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-4.d
@@ -0,0 +1,9 @@
+#source: provide-2.s
+#ld: -T provide-4.t
+#PROG: nm
+#map: provide-4-map.d
+#...
+0+3 A baz
+0+2000 D foo
+0+2010 D loc2
+0+2030 A loc4
diff --git a/ld/testsuite/ld-scripts/provide-4.t b/ld/testsuite/ld-scripts/provide-4.t
new file mode 100644
index 0000000..424c238
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-4.t
@@ -0,0 +1,16 @@
+SECTIONS
+{
+  PROVIDE (foo = 1);
+  PROVIDE (bar = 2);
+  PROVIDE (baz = 3);
+  .data 0x2000 :
+  {
+    *(.data)
+
+    PROVIDE (loc1 = ALIGN (., 0x10));
+    PROVIDE (loc2 = ALIGN (., 0x10));
+  }
+
+  PROVIDE (loc3 = loc1 + 0x20);
+  loc4 = loc2 + 0x20;
+}
diff --git a/ld/testsuite/ld-scripts/provide-5-map.d b/ld/testsuite/ld-scripts/provide-5-map.d
new file mode 100644
index 0000000..2271dfd
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-5-map.d
@@ -0,0 +1,6 @@
+#...
+Linker script and memory map
+#...
+                0x0+10                foo = 0x10
+                \[!provide\]                        PROVIDE \(foo, bar\)
+#...
diff --git a/ld/testsuite/ld-scripts/provide-5.d b/ld/testsuite/ld-scripts/provide-5.d
new file mode 100644
index 0000000..1b14fa6
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-5.d
@@ -0,0 +1,6 @@
+#source: provide-5.s
+#ld: -T provide-5.t
+#PROG: nm
+#map: provide-5-map.d
+#...
+0+10 A foo
diff --git a/ld/testsuite/ld-scripts/provide-5.s b/ld/testsuite/ld-scripts/provide-5.s
new file mode 100644
index 0000000..1d05efd
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-5.s
@@ -0,0 +1,4 @@
+        .data
+        .global baz
+baz:
+        .word 0
diff --git a/ld/testsuite/ld-scripts/provide-5.t b/ld/testsuite/ld-scripts/provide-5.t
new file mode 100644
index 0000000..eda741e
--- /dev/null
+++ b/ld/testsuite/ld-scripts/provide-5.t
@@ -0,0 +1,10 @@
+SECTIONS
+{
+  foo = 0x10;
+  PROVIDE (foo = bar);
+
+  .data 0x1000 :
+  {
+    *(.data)
+  }
+}
diff --git a/ld/testsuite/ld-scripts/provide.exp b/ld/testsuite/ld-scripts/provide.exp
index 7f45e58..3ddbb22 100644
--- a/ld/testsuite/ld-scripts/provide.exp
+++ b/ld/testsuite/ld-scripts/provide.exp
@@ -40,5 +40,7 @@ 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 LDFLAGS "$saved_LDFLAGS"
-- 
1.9.3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ld: Don't evaluate unneeded PROVIDE expressions.
  2015-01-19 11:06             ` Andrew Burgess
  2015-01-19 12:50               ` Alan Modra
  2015-01-20 10:21               ` Andrew Burgess
@ 2015-01-20 14:21               ` Hans-Peter Nilsson
  2015-01-20 16:08                 ` Andrew Burgess
  2 siblings, 1 reply; 16+ messages in thread
From: Hans-Peter Nilsson @ 2015-01-20 14:21 UTC (permalink / raw)
  To: andrew.burgess; +Cc: binutils, amodra

> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Date: Mon, 19 Jan 2015 12:06:20 +0100

> * Alan Modra <amodra@gmail.com> [2015-01-12 11:08:53 +1030]:
> 
> > > ld/testsuite/ChangeLog:
> > >
> > >     * ld-scripts/provide-4.d: New file.
> > >     * ld-scripts/provide-4-map.d: New file.
> > >     * ld-scripts/provide-4.t: New file.
> > >     * ld-scripts/provide.exp: Run the provide-4.d test.
> >
> > This part isn't OK yet.  You need to make sure new tests pass on more
> > than just Linux x86.
> 
> Alan,
> 
> Thanks for taking the time to review my patch, and I'm sorry that the
> change wasn't up to standard.
> 
> I've updated the expected results, and tested against the same set of
> builds you suggest, I'm now seeing passes on all but 3 builds, and
> these 3 failures are, I believe, not related to my changes.

I'd guess the testsuite adjustments were not double-checked that
they actually do pass on a 32-bit host, as these FAILs are
introduced by your patches, for targets cris-axis-linux-gnu,
arm-unknown-eabi, cris-axis-elf *on a 32-bit host* (so if
nothing else use 'CC=gcc -m32' on your x86_64-unknown-linux-gnu
host as my autotester does and double-check that you get a
'32-bit bfd_vma'):

Running /tmp/hpautotest-binutils/bsrc/src/ld/testsuite/ld-scripts/provide.exp ...
FAIL: ld-scripts/provide-4 (map file check)
FAIL: ld-scripts/provide-5 (map file check)

(Please fix.)

brgds, H-P

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] ld: Don't evaluate unneeded PROVIDE expressions.
  2015-01-20 14:21               ` Hans-Peter Nilsson
@ 2015-01-20 16:08                 ` Andrew Burgess
  0 siblings, 0 replies; 16+ messages in thread
From: Andrew Burgess @ 2015-01-20 16:08 UTC (permalink / raw)
  To: Hans-Peter Nilsson; +Cc: binutils, amodra

* Hans-Peter Nilsson <hans-peter.nilsson@axis.com> [2015-01-20 15:20:53 +0100]:

> I'd guess the testsuite adjustments were not double-checked that
> they actually do pass on a 32-bit host, as these FAILs are
> introduced by your patches,

I have pushed the patch below that I believe should fix the failures
you are seeing.

Please let me know if you are still seeing any problems.

Sorry for the wasted time, and thanks for bringing this to my
attention.

Andrew

---

Tests that I added in commit c05b575a8dfabab6af5d8586d1a5c0c67f819ac2
fails on 32-bit hosts due to differences in whitespace.

This patch updates the expected output patterns to be more accepting of
differences in whitespace, the tests should now pass.

ld/testsuite/ChangeLog:

	* ld-scripts/provide-4-map.d: Update expected output.
	* ld-scripts/provide-5-map.d: Likewise.
---
 ld/testsuite/ChangeLog                  |  5 +++++
 ld/testsuite/ld-scripts/provide-4-map.d | 18 +++++++++---------
 ld/testsuite/ld-scripts/provide-5-map.d |  4 ++--
 3 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/ld/testsuite/ChangeLog b/ld/testsuite/ChangeLog
index 0b47494..21cf40e 100644
--- a/ld/testsuite/ChangeLog
+++ b/ld/testsuite/ChangeLog
@@ -1,5 +1,10 @@
 2015-01-20  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* ld-scripts/provide-4-map.d: Update expected output.
+	* ld-scripts/provide-5-map.d: Likewise.
+
+2015-01-20  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* ld-scripts/provide-4.d: New file.
 	* ld-scripts/provide-4-map.d: New file.
 	* ld-scripts/provide-4.t: New file.
diff --git a/ld/testsuite/ld-scripts/provide-4-map.d b/ld/testsuite/ld-scripts/provide-4-map.d
index d8e4a28..189d1d0 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\)
-#...
-                0x0+2000                foo
-                \[!provide\]                        PROVIDE \(loc1, ALIGN \(\., 0x10\)\)
-                0x0+2010                PROVIDE \(loc2, ALIGN \(\., 0x10\)\)
-                \[!provide\]                        PROVIDE \(loc3, \(loc1 \+ 0x20\)\)
-                0x0+2030                loc4 = \(loc2 \+ 0x20\)
+                \[!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\)\)
+                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 2271dfd..a75e4aa 100644
--- a/ld/testsuite/ld-scripts/provide-5-map.d
+++ b/ld/testsuite/ld-scripts/provide-5-map.d
@@ -1,6 +1,6 @@
 #...
 Linker script and memory map
 #...
-                0x0+10                foo = 0x10
-                \[!provide\]                        PROVIDE \(foo, bar\)
+                0x0+10 +foo = 0x10
+                \[!provide\] +PROVIDE \(foo, bar\)
 #...
-- 
1.9.3

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2015-01-20 16:08 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-07 12:15 [PATCH 0/2] ld: PROVIDE, undefined symbols, and mapfiles Andrew Burgess
2015-01-07 12:15 ` [PATCH 1/2] ld/testing: run_dump_test can now check linker mapfiles Andrew Burgess
2015-01-08  1:21   ` Hans-Peter Nilsson
2015-01-08 10:16     ` Andrew Burgess
2015-01-12  0:38       ` Alan Modra
2015-01-07 12:15 ` [PATCH 2/2] ld: Don't evaluate unneeded PROVIDE expressions Andrew Burgess
2015-01-08  1:25   ` Hans-Peter Nilsson
2015-01-08 10:18     ` Andrew Burgess
2015-01-08 11:08       ` Hans-Peter Nilsson
2015-01-08 20:03         ` Andrew Burgess
2015-01-12  0:39           ` Alan Modra
2015-01-19 11:06             ` Andrew Burgess
2015-01-19 12:50               ` Alan Modra
2015-01-20 10:21               ` Andrew Burgess
2015-01-20 14:21               ` Hans-Peter Nilsson
2015-01-20 16:08                 ` Andrew Burgess

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).