* [PATCH 0/2] ld: PROVIDE, undefined symbols, and mapfiles. @ 2015-01-07 12:15 Andrew Burgess 2015-01-07 12:15 ` [PATCH 2/2] ld: Don't evaluate unneeded PROVIDE expressions Andrew Burgess 2015-01-07 12:15 ` [PATCH 1/2] ld/testing: run_dump_test can now check linker mapfiles 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 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 ` Andrew Burgess 2015-01-08 1:25 ` Hans-Peter Nilsson 2015-01-07 12:15 ` [PATCH 1/2] ld/testing: run_dump_test can now check linker mapfiles 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 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 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 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 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
* [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 ` [PATCH 2/2] ld: Don't evaluate unneeded PROVIDE expressions Andrew Burgess @ 2015-01-07 12:15 ` Andrew Burgess 2015-01-08 1:21 ` 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 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
* 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 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 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
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 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 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
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).