Re: [PATCH 1/2] ld/testing: run_dump_test can now check linker mapfiles.

[Andrew Burgess <andrew dot burgess at embecosm dot com>]

* 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