Add prms_id and bug_id to the ARI?

Add prms_id and bug_id to the ARI?

[Joel Brobecker]

Now that these are gone, I would like to prevent any accidental
re-introduction, so I'm thinking we could use the ARI for that?

Any objection?

-- 
Joel

Re: Add prms_id and bug_id to the ARI?

[Pedro Alves]

On Wednesday 05 May 2010 19:16:33, Joel Brobecker wrote:
> Now that these are gone, I would like to prevent any accidental
> re-introduction, so I'm thinking we could use the ARI for that?
> 
> Any objection?

No objection,

  though, if we're going to forbid its use, it should
be even more effective to check if it is set to anything other
than 0 somewhere in the testsuite common code, so that the person
writing the test gets a loud bark from the testsuite before even
posting the patch.  :-)

-- 
Pedro Alves

Re: Add prms_id and bug_id to the ARI?

[Joel Brobecker]

>   though, if we're going to forbid its use, it should
> be even more effective to check if it is set to anything other
> than 0 somewhere in the testsuite common code, so that the person
> writing the test gets a loud bark from the testsuite before even
> posting the patch.  :-)

I thought about that, but most testcases set it to zero...

-- 
Joel

Re: Add prms_id and bug_id to the ARI?

[Pedro Alves]

On Wednesday 05 May 2010 19:28:48, Joel Brobecker wrote:
> I thought about that, but most testcases set it to zero...

Oh, right, D'oh! :-)

-- 
Pedro Alves

Re: Add prms_id and bug_id to the ARI?

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> Now that these are gone, I would like to prevent any accidental
Joel> re-introduction, so I'm thinking we could use the ARI for that?
Joel> Any objection?

You could try putting this into gdb.exp somewhere:

    proc gdb_trace_error {name ignore op} {
      error "Attempt to write to $name."
    }

    trace variable prms_id w gdb_trace_error
    trace variable bug_id w gdb_trace_error

Maybe gdb.exp runs late enough that runtest won't trigger this error;
I'm not sure.

Tom

Re: Add prms_id and bug_id to the ARI?

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 641 bytes --]

> You could try putting this into gdb.exp somewhere:
> 
>     proc gdb_trace_error {name ignore op} {
>       error "Attempt to write to $name."
>     }
> 
>     trace variable prms_id w gdb_trace_error
>     trace variable bug_id w gdb_trace_error

Nice trick! Attached is a patch that implements something along those
lines...  Ideally, we would really like to check any use of these
variables (including reads and unsets), but these variable do get
accessed during the run (computing some pass strings). In practice,
all the uses I've found were setting them, so it is probably going
to be good enough.

Tested on x86_64-linux.

-- 
Joel

[-- Attachment #2: verbotten.diff --]
[-- Type: text/x-diff, Size: 1906 bytes --]

commit 35e9b1f2ec01514d8455c0366f77ab4e196da8f8
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Sat May 8 12:20:36 2010 -0700

    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.

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 8be2a72..70df5ea 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2480,6 +2480,11 @@ if ![info exists gdb_test_timeout] {
     set gdb_test_timeout $timeout
 }
 
+# A list of global variables that GDB testcases should not use.
+# We try to prevent their use by monitoring write accesses and raising
+# an error when that happens.
+set banned_variables { bug_id prms_id }
+
 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
@@ -2488,6 +2493,13 @@ proc gdb_init { args } {
     global timeout
     set timeout $gdb_test_timeout
 
+    # Block writes to all banned variables...
+    global banned_variables
+    foreach banned_var $banned_variables {
+        global "$banned_var"
+        trace variable "$banned_var" w error
+    }
+
     return [eval default_gdb_init $args];
 }
 
@@ -2501,6 +2513,14 @@ proc gdb_finish { } {
 	eval remote_file target delete $cleanfiles
 	set cleanfiles {}
     }
+
+    # 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 debug_format

Re: Add prms_id and bug_id to the ARI?

[Joel Brobecker]

> commit 35e9b1f2ec01514d8455c0366f77ab4e196da8f8
> Author: Joel Brobecker <brobecker@adacore.com>
> Date:   Sat May 8 12:20:36 2010 -0700
> 
>     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.

FYI: Now checked in.  New uses of these two variables were sneaking in again.

-- 
Joel