Patchwork [asan] Fix up dg-set-target-env-var

login
register
mail settings
Submitter Jakub Jelinek
Date Dec. 5, 2012, 11:28 p.m.
Message ID <20121205232858.GE2315@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/204040/
State New
Headers show

Comments

Jakub Jelinek - Dec. 5, 2012, 11:28 p.m.
Hi!

On Tue, Dec 04, 2012 at 10:00:35AM -0800, Wei Mi wrote:
> I updated the patch according to the comments. Please take a look. Thanks.

I've been testing your patch plus all the patches from me posted yesterday
just with asan.exp testing.  While doing full pair of bootstraps/regtests,
I've discovered dg-set-target-env-var changes don't really work,
gcc-dg-runtest is just one of the many ways to start testing, so
there were tons of errors like
ERROR: tcl error sourcing /usr/src/gcc/gcc/testsuite/gcc.c-torture/execute/ieee/ieee.exp.                                                          
ERROR: can't read "set_target_env_var": no such variable                                                                                           
ERROR: tcl error sourcing /usr/src/gcc/gcc/testsuite/gcc.dg/compat/struct-layout-1.exp.                                                            
ERROR: can't read "set_target_env_var": no such variable                                                                                           
ERROR: tcl error sourcing /usr/src/gcc/gcc/testsuite/gcc.dg/dg.exp.                                                                                
ERROR: can't read "set_target_env_var": no such variable                                                                                           
ERROR: tcl error sourcing /usr/src/gcc/gcc/testsuite/gcc.dg/guality/guality.exp.                                                                   
ERROR: can't read "set_target_env_var": no such variable                                                                                           
(and many more).

Here is an attempt to fix that up.  gcc-dg.exp overrides dg-test which is
the actual routine that parses dg-* directives from the testcases, performs
all the testing and then this overridden routine at the end does some
cleanup, so I'm unsetting set_target_env_var there, and not relying on it
being set.  Furthermore, I don't think unsetenv takes two arguments, and the
whole restore-target-env-var looked weird.  IMHO we want to record which env
vars were set to what values and which were unset, and restore that at the
end of ${tool}_load.  The error message change is because if there are no
args (which means only line will be there), I'm afraid it would show up
traceback.

So far successfully regtested on i686-linux, x86_64-linux regtest is almost
done.  Ok for trunk?

2012-12-05  Jakub Jelinek  <jakub@redhat.com>

	* lib/gcc-dg.exp (${tool}_load): Handle non-existing
	set_target_env_var the same as if it is empty list.
	(dg-set-target-env-var): Fix up error message.
	(set-target-env-var): Record both preexisting env var values
	as well as info that env wasn't set.
	(restore-target-env-var): Iterate on reversed list, if second
	sublist element is 1, setenv the env var to the third sublist
	element, otherwise unsetenv it.
	(gcc-dg-runtest): Don't initialize set_target_env_var.
	(dg-test): Unset set_target_env_var if it was set.



	Jakub
Mike Stump - Dec. 6, 2012, 12:23 a.m.
On Dec 5, 2012, at 3:28 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Here is an attempt to fix that up.

> Ok for trunk?

Ok.

Patch

--- gcc/testsuite/lib/gcc-dg.exp.jj	2012-12-05 20:02:20.000000000 +0100
+++ gcc/testsuite/lib/gcc-dg.exp	2012-12-05 23:23:05.766914151 +0100
@@ -257,14 +257,16 @@  if { [info procs ${tool}_load] != [list]
 	global set_target_env_var
 
 	set saved_target_env_var [list]
-	if { [llength $set_target_env_var] != 0 } {
+	if { [info exists set_target_env_var] \
+	     && [llength $set_target_env_var] != 0 } {
 	    if { [is_remote target] } {
 		return [list "unsupported" ""]
 	    }
 	    set-target-env-var
 	}
 	set result [eval [list saved_${tool}_load $program] $args]
-	if { [llength $set_target_env_var] != 0 } {
+	if { [info exists set_target_env_var] \
+	     && [llength $set_target_env_var] != 0 } {
 	    restore-target-env-var
 	}
 	if { $shouldfail != 0 } {
@@ -281,7 +283,7 @@  if { [info procs ${tool}_load] != [list]
 proc dg-set-target-env-var { args } {
     global set_target_env_var
     if { [llength $args] != 3 } {
-	error "[lindex $args 1]: need two arguments"
+	error "dg-set-target-env-var: need two arguments"
 	return
     }
     lappend set_target_env_var [list [lindex $args 1] [lindex $args 2]]
@@ -294,7 +296,9 @@  proc set-target-env-var { } {
 	set var [lindex $env_var 0]
 	set value [lindex $env_var 1]
 	if [info exists env($var)] {
-	    lappend saved_target_env_var [list $var $env($var)]
+	    lappend saved_target_env_var [list $var 1 $env($var)]
+	} else {
+	    lappend saved_target_env_var [list $var 0]
 	}
 	setenv $var $value
     }
@@ -302,10 +306,13 @@  proc set-target-env-var { } {
 
 proc restore-target-env-var { } {
     upvar 1 saved_target_env_var saved_target_env_var
-    foreach env_var $saved_target_env_var {
+    foreach env_var [lreverse $saved_target_env_var] {
 	set var [lindex $env_var 0]
-	set value [lindex $env_var 1]
-	unsetenv $var $value
+	if [lindex $env_var 1] {
+	    setenv $var [lindex $env_var 2]
+	} else {
+	    unsetenv $var
+	}
     }
 }
 
@@ -330,10 +337,6 @@  proc search_for { file pattern } {
 # as c-torture does.
 proc gcc-dg-runtest { testcases default-extra-flags } {
     global runtests
-    global set_target_env_var
-
-    # Init set_target_env_var
-    set set_target_env_var [list]
 
     # Some callers set torture options themselves; don't override those.
     set existing_torture_options [torture-options-exist]
@@ -724,6 +727,7 @@  if { [info procs saved-dg-test] == [list
 	global compiler_conditional_xfail_data
 	global shouldfail
 	global testname_with_flags
+	global set_target_env_var
 
 	if { [ catch { eval saved-dg-test $args } errmsg ] } {
 	    set saved_info $errorInfo
@@ -744,6 +748,9 @@  if { [info procs saved-dg-test] == [list
 	set additional_sources ""
 	set additional_prunes ""
 	set shouldfail 0
+	if [info exists set_target_env_var] {
+	    unset set_target_env_var
+	}
 	unset_timeout_vars
 	if [info exists compiler_conditional_xfail_data] {
 	    unset compiler_conditional_xfail_data