[Cygwin problem]:with banned variables

[Cygwin problem]:with banned variables

[Pierre Muller]

  Following this patch, 
I can't run the testsuite for Cygwin anymore :(

See this:
$ make check RUNTESTFLAGS="gdb.base/a*.exp"
Nothing to be done for all...
rootme=`pwd`; export rootme; srcdir=../../../src/gdb/testsuite ; export
srcdir ;
 EXPECT=`if [ -f ${rootme}/../../expect/expect ] ; then echo
${rootme}/../../exp
ect/expect ; else echo expect ; fi` ; export EXPECT ; EXEEXT=.exe ; export
EXEEX
T ;
LD_LIBRARY_PATH=$rootme/../../expect:$rootme/../../libstdc++:$rootme/../../t
k/unix:$rootme/../../tcl/unix:$rootme/../../bfd:$rootme/../../opcodes:$LD_LI
BRAR
Y_PATH; export LD_LIBRARY_PATH; if [ -f ${rootme}/../../expect/expect ] ;
then T
CL_LIBRARY=${srcdir}/../../tcl/library ; export TCL_LIBRARY ; fi ; ` if [ -f
${s
rcdir}/../../dejagnu/runtest ]; then echo ${srcdir}/../../dejagnu/runtest;
else
if [ "i686-pc-cygwin" = "i686-pc-cygwin" ]; then echo runtest; else
t='s,y,y,';
echo runtest | sed -e $t; fi; fi` gdb.base/a*.exp
WARNING: Couldn't find the global config file.
Test Run By Pierre on Sat Jun 19 06:46:45 2010
Native configuration is i686-pc-cygwin

                === gdb tests ===

Schedule of variations:
    unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for
targe
t.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for
target.
Using ../../../src/gdb/testsuite/config/unix.exp as tool-and-target-specific
int
erface file.
Running ../../../src/gdb/testsuite/gdb.base/a2-run.exp ...
FAIL: gdb.base/a2-run.exp: run "a2-run" with shell (the program exited)
Running ../../../src/gdb/testsuite/gdb.base/advance.exp ...
can't set "prms_id": prms_id
    while executing
"set prms_id    0"
    (procedure "runtest" line 10)
    invoked from within
"runtest $test_name"
    ("foreach" body line 24)
    invoked from within
"foreach test_name $testlist {
                if { ${ignoretests} != "" } {


  It appears that pmrs_id and bug_id are reset to 0 inside
/usr/share/dejagnu/runtest.exp
in proc runtest
  I know that Cygwin uses an outdated expect,
but I don't understand why this error only shows up
for Cygwin.
  The lastest 


If I modify lib/gdb.exp
Index: lib/gdb.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
retrieving revision 1.150
diff -r1.150 gdb.exp
100a101,106
> # The variable transform_gdb_expect_code can be set to the name of
> # a procedure that will transform the code parameter of gdb_expect call
> # in order to cope for some target dependant problems
> # it can also be reset to an empty string to disable that operation
> global transform_gdb_expect_code
>
2180a2187,2194
>     global transform_gdb_expect_code;
>     if [info exists transform_gdb_expect_code] {
>       if { "[info procs $transform_gdb_expect_code]" != "" } {
>           set expcode [$transform_gdb_expect_code $expcode];
>       } elseif { "$transform_gdb_expect_code" != "" } {
>           perror "Procedure $transform_gdb_expect_code unknown"
>       }
>     }
2522a2537,2541
> proc trace_write { varname args } {
>   upvar #0 $varname var
>   puts "Error: trying to write $varname to \"$var\""
> }
>
2535c2554
<         trace variable "$banned_var" w error
---
>         trace variable "$banned_var" w "trace_write $banned_var"

  Then I also see, on x86_64 linux machine:

muller@gcc16:~/gdb/build-norm/gdb/testsuite$ make check
RUNTESTFLAGS="gdb.base/
a*.exp"
Nothing to be done for all...
rootme=`pwd`; export rootme; srcdir=../../../src/gdb/testsuite ; export
srcdir ;
 EXPECT=`if [ -f ${rootme}/../../expect/expect ] ; then echo
${rootme}/../../exp
ect/expect ; else echo expect ; fi` ; export EXPECT ; EXEEXT= ; export
EXEEXT ;
LD_LIBRARY_PATH=$rootme/../../expect:$rootme/../../libstdc++:$rootme/../../t
k/un
ix:$rootme/../../tcl/unix:$rootme/../../bfd:$rootme/../../opcodes:$LD_LIBRAR
Y_PA
TH; export LD_LIBRARY_PATH; if [ -f ${rootme}/../../expect/expect ] ; then
TCL_L
IBRARY=${srcdir}/../../tcl/library ; export TCL_LIBRARY ; fi ; ` if [ -f
${srcdi
r}/../../dejagnu/runtest ]; then echo ${srcdir}/../../dejagnu/runtest; else
if [
 "x86_64-unknown-linux-gnu" = "x86_64-unknown-linux-gnu" ]; then echo
runtest; e
lse t='s,y,y,'; echo runtest | sed -e $t; fi; fi` gdb.base/a*.exp
Test Run By muller on Sat Jun 19 09:48:09 2010
Native configuration is x86_64-unknown-linux-gnu

                === gdb tests ===

Schedule of variations:
    unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for
targe
t.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for
target.
Using ../../../src/gdb/testsuite/config/unix.exp as tool-and-target-specific
int
erface file.
Running ../../../src/gdb/testsuite/gdb.base/auxv.exp ...
WARNING: can't generate a core file - core tests suppressed - check ulimit
-c
FAIL: gdb.base/auxv.exp: generate native core dump
Running ../../../src/gdb/testsuite/gdb.base/args.exp ...
Error: trying to write prms_id to "0"
Error: trying to write bug_id to "0"
Running ../../../src/gdb/testsuite/gdb.base/assign.exp ...
Error: trying to write prms_id to "0"
Error: trying to write prms_id to "0"
Error: trying to write bug_id to "0"
Error: trying to write bug_id to "0"
Running ../../../src/gdb/testsuite/gdb.base/all-bin.exp ...

So it seems that the difference is in the "error" internal command...


Pierre

  

> -----Message d'origine-----
> De : gdb-cvs-owner@sourceware.org [mailto:gdb-cvs-owner@sourceware.org]
> De la part de brobecke@sourceware.org
> Envoyé : Monday, May 17, 2010 7:11 PM
> À : gdb-cvs@sourceware.org
> Objet : src/gdb/testsuite ChangeLog lib/gdb.exp
> 
> CVSROOT:	/cvs/src
> Module name:	src
> Changes by:	brobecke@sourceware.org	2010-05-17 17:11:28
> 
> Modified files:
> 	gdb/testsuite  : ChangeLog
> 	gdb/testsuite/lib: gdb.exp
> 
> Log message:
> 	testsuite: Prevent writes to prms_id and bug_id.
> 
> 	gdb/testsuite/ChangeLog:
> 
> 	* lib/gdb.exp (banned_variables): New variable/constant.
> 	(gdb_init): Add write trace on variables listed in
> banned_variables.
> 	(gdb_finish): Remove write traces on variables listed in
> 	banned_variables.
> 
> Patches:
> http://sourceware.org/cgi-
> bin/cvsweb.cgi/src/gdb/testsuite/ChangeLog.diff?cvsroot=src&r1=1.2267&r
> 2=1.2268
> http://sourceware.org/cgi-
> bin/cvsweb.cgi/src/gdb/testsuite/lib/gdb.exp.diff?cvsroot=src&r1=1.146&
> r2=1.147

[RFA] Fix Cygwin problem with banned variables

[Pierre Muller]

  The problem may have two origins,
and I didn't spend time to completely analyse it,
but found a fix that seems to work.

  One problem is that while gdb_finish procedure
is never used inside testsuite '.exp' files,
gdb_init is present in lots of '.exp' files.

  But gdb_init and gdb_finsih are also called
automatically by runtest procedure
before and after sourcing the .exp file itself.
  This generates multiple calls to 'trace variable '
and only one call to 'trace remove variable '.
  But another problem is that we current implementation
uses the old way for trace insertion 'trace variable var_name w error'
and the new way for deletion 'trace remove variable var_name write error'.

  Apparently Cygwin's implementation of tcl doesn't like this
mix, and does not remove the trace.
  Uses new version 'trace add variable var_name write error'
allows the removal to work correctly.

So here is a patch that adds a global variable avoiding
multiple setting of those traces and using new
`trace add variable ' syntax.

  I checked this manually on gcc16 (as automated
tests are impossible currently due to the byte-code optimizer problem).

Is this patch OK?


Pierre Muller


2010-06-19  Pierre Muller  <muller@ics.u-strasbg.fr>

	* lib/gdb.exp (banned_variables_traced): New global variable.
	(gdb_init, gdb_finish): Use new variable to avoid multiple
	tracing.
	(gdb_init): Use `trace add variable ' instead of obsolete
	`trace variable '.

Index: src/gdb/testsuite/lib/gdb.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
retrieving revision 1.150
diff -u -p -r1.150 gdb.exp
--- src/gdb/testsuite/lib/gdb.exp	11 Jun 2010 21:58:20 -0000
1.150
+++ src/gdb/testsuite/lib/gdb.exp	19 Jun 2010 12:53:19 -0000
@@ -2519,6 +2519,7 @@ if ![info exists gdb_test_timeout] {
 # We try to prevent their use by monitoring write accesses and raising
 # an error when that happens.
 set banned_variables { bug_id prms_id }
+set banned_variables_traced 0
 
 proc gdb_init { args } {
     # Reset the timeout value to the default.  This way, any testcase
@@ -2530,9 +2531,13 @@ proc gdb_init { args } {
 
     # Block writes to all banned variables...
     global banned_variables
-    foreach banned_var $banned_variables {
-        global "$banned_var"
-        trace variable "$banned_var" w error
+    global banned_variables_traced
+    if (!$banned_variables_traced) {
+    	foreach banned_var $banned_variables {
+            global "$banned_var"
+            trace add variable "$banned_var" write error
+	}
+	set banned_variables_traced 1
     }
 
     return [eval default_gdb_init $args];
@@ -2552,9 +2557,13 @@ proc gdb_finish { } {
     # Unblock write access to the banned variables.  Dejagnu typically
     # resets some of them between testcases.
     global banned_variables
-    foreach banned_var $banned_variables {
-        global "$banned_var"
-        trace remove variable "$banned_var" write error
+    global banned_variables_traced
+    if ($banned_variables_traced) {
+    	foreach banned_var $banned_variables {
+            global "$banned_var"
+            trace remove variable "$banned_var" write error
+	}
+	set banned_variables_traced 0
     }
 }
 

Re: [RFA] Fix Cygwin problem with banned variables

[Joel Brobecker]

>   This generates multiple calls to 'trace variable ' and only one call
>   to 'trace remove variable '.

The TCL documentation is not directly explicit about that, but
some experimentation shows that this is indeed a problem. And
rereading the docuemntation with that in mind, the duplicate traces
are probably treated as distinct traces.  Thus, when doing only one
remove for the trace, we're still left with at least one trace.

> 2010-06-19  Pierre Muller  <muller@ics.u-strasbg.fr>
> 
> 	* lib/gdb.exp (banned_variables_traced): New global variable.
> 	(gdb_init, gdb_finish): Use new variable to avoid multiple
> 	tracing.
> 	(gdb_init): Use `trace add variable ' instead of obsolete
> 	`trace variable '.

Just one detail, but otherwise OK.

> @@ -2519,6 +2519,7 @@ if ![info exists gdb_test_timeout] {
>  # We try to prevent their use by monitoring write accesses and raising
>  # an error when that happens.
>  set banned_variables { bug_id prms_id }
> +set banned_variables_traced 0

Please add a comment explaining what this global is used for and
why it is necessary.

Thanks,
-- 
Joel

RE: [RFA] Fix Cygwin problem with banned variables

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : Monday, June 21, 2010 7:08 PM
> À : Pierre Muller
> Cc : brobecke@sourceware.org; gdb-patches@sourceware.org; 'Pedro Alves'
> Objet : Re: [RFA] Fix Cygwin problem with banned variables
> 
> >   This generates multiple calls to 'trace variable ' and only one
> call
> >   to 'trace remove variable '.
> 
> The TCL documentation is not directly explicit about that, but
> some experimentation shows that this is indeed a problem. And
> rereading the docuemntation with that in mind, the duplicate traces
> are probably treated as distinct traces.  Thus, when doing only one
> remove for the trace, we're still left with at least one trace.
> 
> > 2010-06-19  Pierre Muller  <muller@ics.u-strasbg.fr>
> >
> > 	* lib/gdb.exp (banned_variables_traced): New global variable.
> > 	(gdb_init, gdb_finish): Use new variable to avoid multiple
> > 	tracing.
> > 	(gdb_init): Use `trace add variable ' instead of obsolete
> > 	`trace variable '.
> 
> Just one detail, but otherwise OK.

  Thanks for the approval
 
> > @@ -2519,6 +2519,7 @@ if ![info exists gdb_test_timeout] {
> >  # We try to prevent their use by monitoring write accesses and
> raising
> >  # an error when that happens.
> >  set banned_variables { bug_id prms_id }
> > +set banned_variables_traced 0
> 
> Please add a comment explaining what this global is used for and
> why it is necessary.

Here is what I checked in,
with an explanation added for the new variable `banned_variables_traced'.

  Please feel free to improve the comment,

Pierre Muller

testsuite/ChangLog entry:

2010-06-22  Pierre Muller  <muller@ics.u-strasbg.fr>

       * lib/gdb.exp (banned_variables_traced): New global variable.
       (gdb_init, gdb_finish): Use new variable to avoid multiple tracing.
       (gdb_init): Use `trace add variable' instead of obsolete
       `trace variable'.

Index: lib/gdb.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
retrieving revision 1.150
diff -u -p -r1.150 gdb.exp
--- lib/gdb.exp 11 Jun 2010 21:58:20 -0000      1.150
+++ lib/gdb.exp 22 Jun 2010 07:16:58 -0000
@@ -2520,6 +2520,15 @@ if ![info exists gdb_test_timeout] {
 # an error when that happens.
 set banned_variables { bug_id prms_id }

+# gdb_init is called by runtest at start, but also by several
+# tests directly; gdb_finish is only called from within runtest after
+# each test source execution.
+# Placing several traces by repetitive calls to gdb_init leads
+# to problems, as only one trace is removed in gdb_finish.
+# To overcome this possible problem, we add a variable that records
+# if the banned variables are traced.
+set banned_variables_traced 0
+
 proc gdb_init { args } {
     # Reset the timeout value to the default.  This way, any testcase
     # that changes the timeout value without resetting it cannot affect
@@ -2530,9 +2539,13 @@ proc gdb_init { args } {

     # Block writes to all banned variables...
     global banned_variables
-    foreach banned_var $banned_variables {
-        global "$banned_var"
-        trace variable "$banned_var" w error
+    global banned_variables_traced
+    if (!$banned_variables_traced) {
+       foreach banned_var $banned_variables {
+            global "$banned_var"
+            trace add variable "$banned_var" write error
+       }
+       set banned_variables_traced 1
     }

     return [eval default_gdb_init $args];
@@ -2552,9 +2565,13 @@ proc gdb_finish { } {
     # Unblock write access to the banned variables.  Dejagnu typically
     # resets some of them between testcases.
     global banned_variables
-    foreach banned_var $banned_variables {
-        global "$banned_var"
-        trace remove variable "$banned_var" write error
+    global banned_variables_traced
+    if ($banned_variables_traced) {
+       foreach banned_var $banned_variables {
+            global "$banned_var"
+            trace remove variable "$banned_var" write error
+       }
+       set banned_variables_traced 0
     }
 }