[PATCH 1/3] binutils/testsuite: Support stderr options with `run_dump_test'

["Maciej W. Rozycki" <macro at mips dot com>]

Add support for the `error', `error_output', `warning' and 
`warning_output' options for `run_dump_test' input files, based on the 
version of the procedure in ld/testsuite/lib/ld-lib.exp and providing 
compatible semantics.  These options apply to PROG under test and let 
test cases specify output expected on stderr as well as express a 
requirement for PROG to exit unsuccessfully.  Messages to match against 
can be supplied either inline or fetched from a named file.  Update 
procedure description in the introductory comment accordingly.

As the exit status from `remote_exec' is regrettably lost in our default 
implementation of `binutils_run', which is user-overridable, avoid 
changing this procedure's API and use a global `binutils_run_status' 
variable to pass the status up to the caller, similarly to how 
`binutils_run_failed' is handled.  Document the new variable in the 
respective introductory comments.

	binutils/
	* testsuite/config/default.exp (binutils_run): Document 
	`binutils_run_status'.
	* testsuite/lib/utils-lib.exp (default_binutils_run): Likewise, 
	and set it.
	(run_dump_test): Add `error', `error_output', `warning' and
	`warning_output' options.  Update documentation accordingly.
---
Hi,

 Apparently with `testsuite/binutils-all/wasm32/invalid-wasm-1.d' someone 
has already tried to use `error', figured out support was missing and 
disabled the test rather than adding such support.  Interestingly this 
test, unusually, expects an error in the dumper tool to be handled rather 
than one in the tool under test, which is unlike the GAS and LD test 
frameworks work, and unlike the binutils framework will with this change 
in place.

 NB source for `testsuite/binutils-all/wasm32/invalid-wasm-2.d' is 
additionally missing.

 I'm leaving this and the other issue to sort out by the WebAssembly port 
submitter; obviously no one else can provide the missing test source.

 With these observations noted I have verified this change not to cause 
regressions across my usual targets.  OK to apply?

  Maciej
---
 binutils/testsuite/config/default.exp |    1 
 binutils/testsuite/lib/utils-lib.exp  |  190 ++++++++++++++++++++++++++--------
 2 files changed, 148 insertions(+), 43 deletions(-)

binutils-binutils-test-warnerr.diff
Index: binutils/binutils/testsuite/config/default.exp
===================================================================
--- binutils.orig/binutils/testsuite/config/default.exp	2018-01-17 15:26:14.139371898 +0000
+++ binutils/binutils/testsuite/config/default.exp	2018-01-17 15:26:24.240779220 +0000
@@ -112,6 +112,7 @@ if {[file isfile tmpdir/gas/as[exe_ext]]
 # binutils_run
 #	run a program, returning the output
 #	sets binutils_run_failed if the program does not exist
+#	sets binutils_run_status to the exit status of the program
 #
 proc binutils_run { prog progargs } {
     default_binutils_run $prog $progargs
Index: binutils/binutils/testsuite/lib/utils-lib.exp
===================================================================
--- binutils.orig/binutils/testsuite/lib/utils-lib.exp	2018-01-10 20:12:52.000000000 +0000
+++ binutils/binutils/testsuite/lib/utils-lib.exp	2018-01-17 15:21:03.954697988 +0000
@@ -51,12 +51,17 @@ proc binutil_version { prog } {
 # default_binutils_run
 #	run a program, returning the output
 #	sets binutils_run_failed if the program does not exist
+#	sets binutils_run_status to the exit status of the program
 #
 proc default_binutils_run { prog progargs } {
     global binutils_run_failed
+    global binutils_run_status
     global host_triplet
 
     set binutils_run_failed 0
+    if [info exists binutils_run_status] {
+	unset binutils_run_status
+    }
 
     if ![is_remote host] {
 	if {[which $prog] == 0} then {
@@ -83,6 +88,7 @@ proc default_binutils_run { prog progarg
     regsub -all "\\$" "$progargs" "\\$" progargs
 
     set state [remote_exec host $prog $progargs]
+    set binutils_run_status [lindex $state 0]
     set exec_output [prune_warnings [lindex $state 1]]
     if {![string match "" $exec_output]} then {
 	send_log "$exec_output\n"
@@ -241,22 +247,22 @@ proc exe_ext {} {
 #
 #   error: REGEX
 #	An error with message matching REGEX must be emitted for the test
-#	to pass.  The PROG, objdump, nm and objcopy options have no
-#	meaning and need not supplied if this is present.
+#	to pass.  The DUMPPROG, addr2line, nm, objdump, readelf and size
+#	options have no meaning and need not supplied if this is present.
+#	Multiple "error" directives append to the expected error message.
 #
-#   warning: REGEX
-#	Expect a gas warning matching REGEX.  It is an error to issue
-#	both "error" and "warning".
+#   error_output: FILE
+#	Means the same as 'error', except the regular expression lines
+#	are contains in FILE.
 #
-#   stderr: FILE
-#       FILE contains regexp lines to be matched against the diagnostic
-#       output of the assembler.  This does not preclude the use of
-#       PROG, nm, objdump, or objcopy.
+#   warning: REGEX
+#	Expect a warning matching REGEX.  It is an error to issue both
+#	"error" and "warning".  Multiple "warning" directives append to
+#	the expected linker warning message.
 #
-#   error-output: FILE
-#       Means the same as 'stderr', but also indicates that the assembler
-#       is expected to exit unsuccessfully (therefore PROG, objdump, nm,
-#       and objcopy have no meaning and should not be supplied).
+#   warning_output: FILE
+#	Means the same as 'warning', except the regular expression
+#	lines are contains in FILE.
 #
 # Each option may occur at most once.
 #
@@ -270,6 +276,7 @@ proc run_dump_test { name {extra_options
     global OBJDUMP NM OBJCOPY READELF STRIP
     global OBJDUMPFLAGS NMFLAGS OBJCOPYFLAGS READELFFLAGS STRIPFLAGS
     global ELFEDIT ELFEDITFLAGS
+    global binutils_run_status
     global host_triplet
     global env
     global copyfile
@@ -304,6 +311,10 @@ proc run_dump_test { name {extra_options
     set opts(DUMPPROG) {}
     set opts(source) {}
     set opts(dump) {}
+    set opts(error) {}
+    set opts(warning) {}
+    set opts(error_output) {}
+    set opts(warning_output) {}
     set opts(target) {}
     set opts(not-target) {}
     set opts(skip) {}
@@ -322,12 +333,18 @@ proc run_dump_test { name {extra_options
 	# directory.
 	regsub -all "\\\$srcdir" "$opt_val" "$srcdir/$subdir" opt_val
 
-	if [string length $opts($opt_name)] {
-	    perror "option $opt_name multiply set in $file.d"
-	    unresolved $subdir/$name
-	    return
+	switch -- $opt_name {
+	    warning {}
+	    error {}
+	    default {
+		if [string length $opts($opt_name)] {
+		    perror "option $opt_name multiply set in $file.d"
+		    unresolved $subdir/$name
+		    return
+		}
+	    }
 	}
-	set opts($opt_name) $opt_val
+	append opts($opt_name) $opt_val
     }
 
     foreach i $extra_options {
@@ -345,7 +362,8 @@ proc run_dump_test { name {extra_options
 
 	# add extra option to end of existing option, adding space
 	# if necessary.
-	if [string length $opts($opt_name)] {
+	if { ![regexp "warning|error" $opt_name]
+	     && [string length $opts($opt_name)] } {
 	    append opts($opt_name) " "
 	}
 	append opts($opt_name) $opt_val
@@ -383,28 +401,35 @@ proc run_dump_test { name {extra_options
     }
 
     set dumpprogram ""
-    if { $opts(DUMPPROG) != "" } {
-	switch -- $opts(DUMPPROG) {
-	    addr2line	{ set dumpprogram addr2line }
-	    nm		{ set dumpprogram nm }
-	    objdump	{ set dumpprogram objdump }
-	    readelf	{ set dumpprogram readelf }
-	    size	{ set dumpprogram size }
-	    default	{
-		perror "unrecognized dump program option $opts(DUMPPROG) in $file.d"
-		unresolved $testname
-		return }
-	}
-    } else {
-	# Guess which program to run, by seeing which option was specified.
-	foreach p {addr2line nm objdump readelf size} {
-	    if {$opts($p) != ""} {
-		if {$dumpprogram != ""} {
-		    perror "more than one possible dump program specified in $file.d"
+    # It's meaningless to require an output-testing method when we
+    # expect an error.
+    if { $opts(error) == "" && $opts(error_output) == "" } {
+	if { $opts(DUMPPROG) != "" } {
+	    switch -- $opts(DUMPPROG) {
+		addr2line	{ set dumpprogram addr2line }
+		nm		{ set dumpprogram nm }
+		objdump		{ set dumpprogram objdump }
+		readelf		{ set dumpprogram readelf }
+		size		{ set dumpprogram size }
+		default		{
+		    perror "unrecognized dump program option $opts(DUMPPROG)\
+			    in $file.d"
 		    unresolved $testname
 		    return
-		} else {
-		    set dumpprogram $p
+		}
+	    }
+	} else {
+	    # Guess which program to run, by seeing which option was specified.
+	    foreach p {addr2line nm objdump readelf size} {
+		if {$opts($p) != ""} {
+		    if {$dumpprogram != ""} {
+			perror "more than one possible dump program specified\
+				in $file.d"
+			unresolved $testname
+			return
+		    } else {
+			set dumpprogram $p
+		    }
 		}
 	    }
 	}
@@ -487,16 +512,95 @@ proc run_dump_test { name {extra_options
 	}
     }
 
+    if { (($opts(warning) != "") && ($opts(error) != "")) \
+	 || (($opts(warning) != "") && ($opts(error_output) != "")) \
+	 || (($opts(warning) != "") && ($opts(warning_output) != "")) \
+	 || (($opts(error) != "") && ($opts(warning_output) != "")) \
+	 || (($opts(error) != "") && ($opts(error_output) != "")) \
+	 || (($opts(warning_output) != "") && ($opts(error_output) != "")) } {
+	perror "bad mix of warning, error, warning_output, and error_output\
+		test-directives"
+	unresolved $testname
+	return
+    }
+
+    set check_prog(source) ""
+    set check_prog(terminal) 0
+    if { $opts(error) != "" \
+	 || $opts(warning) != "" \
+	 || $opts(error_output) != "" \
+	 || $opts(warning_output) != "" } {
+
+	if { $opts(error) != "" || $opts(error_output) != "" } {
+	    set check_prog(terminal) 1
+	} else {
+	    set check_prog(terminal) 0
+	}
+
+	if { $opts(error) != "" || $opts(warning) != "" } {
+	    set check_prog(source) "regex"
+	    if { $opts(error) != "" } {
+		set check_prog(regex) $opts(error)
+	    } else {
+		set check_prog(regex) $opts(warning)
+	    }
+	} else {
+	    set check_prog(source) "file"
+	    if { $opts(error_output) != "" } {
+		set check_prog(file) $opts(error_output)
+	    } else {
+		set check_prog(file) $opts(warning_output)
+	    }
+	}
+    }
+
     set progopts1 $opts($program)
     eval set progopts \$[string toupper $program]FLAGS
     eval set binary \$[string toupper $program]
 
     set exec_output [binutils_run $binary "$progopts $progopts1 $tempfile $destopt ${copyfile}.o"]
-    if ![string match "" $exec_output] {
-	send_log "$exec_output\n"
+    set cmdret 0
+    if [info exists binutils_run_status] {
+	set cmdret $binutils_run_status
+    }
+
+    regsub "\n$" $exec_output "" exec_output
+    if { $cmdret != 0 || $exec_output != "" || $check_prog(source) != "" } {
+	set exitstat "succeeded"
+	if { $cmdret != 0 } {
+	    set exitstat "failed"
+	}
+
+	if { $check_prog(source) == "regex" } {
+	    verbose -log "$exitstat with: <$exec_output>,\
+			  expected: <$check_prog(regex)>"
+	} elseif { $check_prog(source) == "file" } {
+	    verbose -log "$exitstat with: <$exec_output>,\
+			  expected in file $check_prog(file)"
+	    set_file_contents "tmpdir/prog.messages" "$exec_output"
+	} else {
+	    verbose -log "$exitstat with: <$exec_output>, no expected output"
+	}
+	send_log -- "$exec_output\n"
 	verbose "$exec_output"
-	fail $testname
-	return
+
+	if { (($check_prog(source) == "") == ($exec_output == "")) \
+	     && (($cmdret == 0) == ($check_prog(terminal) == 0)) \
+	     && ((($check_prog(source) == "regex") \
+		  && ($check_prog(regex) == "") == ($exec_output == "") \
+		  && [regexp -- $check_prog(regex) $exec_output]) \
+		 || (($check_prog(source) == "file") \
+		     && (![regexp_diff "tmpdir/prog.messages" \
+			   "$srcdir/$subdir/$check_prog(file)"]))) } {
+	    # We have the expected output from prog.
+	    if { $check_prog(terminal) || $program == "" } {
+		pass $testname
+		return
+	    }
+	} else {
+	    fail $testname
+	    return
+	}
     }
 
     set progopts1 $opts($dumpprogram)