gdb/testsuite: track nested caching proc calls

It was pointed out in this email:

  https://inbox.sourceware.org/gdb-patches/97973506-79f4-4216-9c0b-57401b3933f5@arm.com

that this commit:

  commit 0726729d344fecf98f8d138e688e77201cc3cece
  Date:   Mon Jun 3 13:56:54 2024 +0100

      gdb/testsuite: track if a caching proc calls gdb_exit or not

had broken some AArch64 tests.

What is going on is that there are two caching procs:

  allow_aarch64_sme_tests
  aarch64_initialize_sme_information

the allow_aarch64_sme_tests proc makes a call to
aarch64_initialize_sme_information, but
aarch64_initialize_sme_information is also called from other
non-caching procs, like aarch64_supports_sme_svl.

Both of the caching procs mentioned above compile and run a helper
program, and both of them call gdb_exit.

After the above commit, the first call to any caching proc, the body
of which calls gdb_exit, will result in a gdb_exit call even if the
body is not executed and the result is fetched from the cache.

What was observed is that in the first test script
allow_aarch64_sme_tests is called, the body of this caching proc is
run which calls gdb_exit.  Then allow_aarch64_sme_tests calls
aarch64_initialize_sme_information, the body of which is run and
gdb_exit is called again.  The results from both procs are added to
the cache.

In the next test script allow_aarch64_sme_tests is called.  This
results in a cache hit, but gdb_exit is also called as this is the
first call in this second test script.

Later in the test script aarch64_supports_sme_svl is called which
calls aarch64_initialize_sme_information.  As this is the first call
to aarch64_initialize_sme_information in this second test
script (remember the body of allow_aarch64_sme_tests was never run)
then gdb_exit is called.  This call to gdb_exit is new after the above
commit and is unexpected.

I think the idea behind the above commit is still sound.  If the call
to allow_aarch64_sme_tests was removed from the second test script
then we would want the extra gdb_exit call as this would expose a real
bug in the test.  The problem is that after the above commit the
nested nature of the caching proc calls becomes important: a call to
allow_aarch64_sme_tests should mean that we've also called
aarch64_initialize_sme_information, and that relationship isn't
currently captured.

So in this commit I'm adding another field to the global
gdb_data_cache (in lib/cache.exp).  This new field is 'also_called'.
For every caching proc we populate this field with a list of names,
these are the names of any nested caching procs that are called when
the body of a caching proc is executed.

Now when we get a cache hit in gdb_data_cache we mark every proc in
the 'also_called' list as having been called.  This means that further
calls to these procs will no longer trigger a gdb_exit call.

Approved-By: Luis Machado <luis.machado@arm.com>
Tested-By: Luis Machado <luis.machado@arm.com>

diff --git a/gdb/testsuite/lib/cache.exp b/gdb/testsuite/lib/cache.exp
index 092b7f136e86b89a80f21e8329d6582d7664495c..ca3dca22f96fe309c82905869dd5b29d29d665d9 100644 (file)
--- a/gdb/testsuite/lib/cache.exp
+++ b/gdb/testsuite/lib/cache.exp
@@ -59,13 +59,38 @@ proc gdb_exit_called { args } {
     set ::gdb_exit_called true
 }
 
-# If DO_EXIT is false then this proc does nothing.  If DO_EXIT is true
-# then call gdb_exit the first time this proc is called for each
-# unique value of NAME within a single test.  Every subsequent time
-# this proc is called within a single test (for a given value of
-# NAME), don't call gdb_exit.
+# While calling the implementation of a caching proc, that
+# implementation might itself call additional caching procs.  We need
+# to track all of the nested caching procs that are called and we do
+# that in this list which is a list containing the names of any nested
+# caching procs that are called.
 
-proc gdb_cache_maybe_gdb_exit { name do_exit } {
+set gdb_nested_caching_proc_calls {}
+
+# Called before returning from gdb_do_cache.  NAME is the name of the
+# caching proc that was called.
+#
+# When DO_EXIT is true then this proc should call gdb_exit before
+# returning, otherwise gdb_exit is not called.
+#
+# ALSO_CALLED is a list of the names all the nested caching procs that
+# the proc NAME called.  This proc appends NAME as well as everything
+# in ALSO_CALLED to the global GDB_NESTED_CACHING_PROC_CALLS, this
+# aids in tracking recursive caching proc calls.
+
+proc gdb_cache_maybe_gdb_exit { name do_exit also_called } {
+
+    # Record all the procs that have been called, but only if the exit
+    # trace is in place (this is done in gdb_do_cache) and indicates
+    # that we are in data gathering mode.
+    if { [info exists ::gdb_exit_trace_in_place] } {
+       set ::gdb_nested_caching_proc_calls \
+           [list {*}$::gdb_nested_caching_proc_calls $name {*}$also_called]
+    }
+
+    # The cache 'exit' entry will be true if this caching proc, or any
+    # caching proc that is recursively called from this caching proc,
+    # called exit.
     if { !$do_exit } {
        return
     }
@@ -76,7 +101,15 @@ proc gdb_cache_maybe_gdb_exit { name do_exit } {
     set global_name __${name}__cached_gdb_exit_called
     if { ![info exists ::${global_name}] } {
        gdb_exit
+       verbose -log "gdb_caching_proc $name caused gdb_exit to be called"
        set ::${global_name} true
+       verbose -log "  gdb_caching_proc $name marked as called"
+
+       foreach other_name $also_called {
+           verbose -log "  gdb_caching_proc $other_name marked as called"
+           set global_name __${other_name}__cached_gdb_exit_called
+           set ::${global_name} true
+       }
     }
 }
 
@@ -86,6 +119,8 @@ proc gdb_do_cache {name args} {
     global gdb_data_cache objdir
     global GDB_PARALLEL
 
+    verbose -log "gdb_do_cache: $name ( $args )"
+
     # Normally, if we have a cached value, we skip computation and return
     # the cached value.  If set to 1, instead don't skip computation and
     # verify against the cached value.
@@ -107,9 +142,10 @@ proc gdb_do_cache {name args} {
     if {[info exists gdb_data_cache(${cache_name},value)]} {
        set cached_value $gdb_data_cache(${cache_name},value)
        set cached_exit $gdb_data_cache(${cache_name},exit)
+       set cached_also_called $gdb_data_cache(${cache_name},also_called)
        verbose "$name: returning '$cached_value' from cache" 2
        if { $cache_verify == 0 } {
-           gdb_cache_maybe_gdb_exit $name $cached_exit
+           gdb_cache_maybe_gdb_exit $name $cached_exit $cached_also_called
            return $cached_value
        }
        set is_cached 1
@@ -123,11 +159,13 @@ proc gdb_do_cache {name args} {
            close $fd
            set gdb_data_cache(${cache_name},value) [lindex $content 0]
            set gdb_data_cache(${cache_name},exit) [lindex $content 1]
+           set gdb_data_cache(${cache_name},also_called) [lindex $content 2]
            set cached_value $gdb_data_cache(${cache_name},value)
            set cached_exit $gdb_data_cache(${cache_name},exit)
+           set cached_also_called $gdb_data_cache(${cache_name},also_called)
            verbose "$name: returning '$cached_value' from file cache" 2
            if { $cache_verify == 0 } {
-               gdb_cache_maybe_gdb_exit $name $cached_exit
+               gdb_cache_maybe_gdb_exit $name $cached_exit $cached_also_called
                return $cached_value
            }
            set is_cached 1
@@ -164,9 +202,29 @@ proc gdb_do_cache {name args} {
     set old_gdb_exit_called $::gdb_exit_called
     set ::gdb_exit_called false
 
+    # As with the exit tracking above we also need to track any nested
+    # caching procs that this proc might call.  To do this we backup
+    # the current global list of nested caching proc calls and reset
+    # the global back ot the empty list.  As nested caching procs are
+    # called their names are added to the global list, see
+    # gdb_cache_maybe_gdb_exit for where this is done.
+    set old_gdb_nested_caching_proc_calls $::gdb_nested_caching_proc_calls
+    set ::gdb_nested_caching_proc_calls {}
+
     set real_name gdb_real__$name
     set gdb_data_cache(${cache_name},value) [gdb_do_cache_wrap $real_name {*}$args]
     set gdb_data_cache(${cache_name},exit) $::gdb_exit_called
+    set gdb_data_cache(${cache_name},also_called) \
+       [lsort -unique $::gdb_nested_caching_proc_calls]
+
+    # Now that the actual implementation of this caching proc has been
+    # executed the gdb_nested_caching_proc_calls global will contain
+    # the names of any nested caching procs that were called.  We
+    # append this new set of names onto the set of names we backed up
+    # above.
+    set ::gdb_nested_caching_proc_calls \
+       [list {*}$old_gdb_nested_caching_proc_calls \
+            {*}$::gdb_nested_caching_proc_calls]
 
     # See comment above where OLD_GDB_EXIT_CALLED is set: if
     # GDB_EXIT_CALLED was previously true then this is a recursive
@@ -183,6 +241,7 @@ proc gdb_do_cache {name args} {
        trace remove execution gdb_exit enter gdb_exit_called
        unset ::gdb_exit_trace_in_place
        set ::gdb_exit_called false
+       set ::gdb_nested_caching_proc_calls {}
     }
 
     # If a value being stored in the cache contains a newline then
@@ -196,6 +255,7 @@ proc gdb_do_cache {name args} {
     if { $cache_verify == 1 && $is_cached == 1 } {
        set computed_value $gdb_data_cache(${cache_name},value)
        set computed_exit $gdb_data_cache(${cache_name},exit)
+       set computed_also_called $gdb_data_cache(${cache_name},also_called)
        if { $cached_value != $computed_value } {
            error [join [list "Inconsistent value results for $cache_name:"
                         "cached: $cached_value vs. computed: $computed_value"]]
@@ -204,6 +264,10 @@ proc gdb_do_cache {name args} {
            error [join [list "Inconsistent exit results for $cache_name:"
                         "cached: $cached_exit vs. computed: $computed_exit"]]
        }
+       if { $cached_also_called != $computed_also_called } {
+           error [join [list "Inconsistent also_called results for $cache_name:"
+                        "cached: $cached_also_called vs. computed: $computed_also_called"]]
+       }
     }
 
     if {[info exists GDB_PARALLEL]} {
@@ -213,10 +277,12 @@ proc gdb_do_cache {name args} {
        set fd [open $cache_filename.[pid] w]
        puts $fd $gdb_data_cache(${cache_name},value)
        puts $fd $gdb_data_cache(${cache_name},exit)
+       puts $fd $gdb_data_cache(${cache_name},also_called)
        close $fd
        file rename -force -- $cache_filename.[pid] $cache_filename
     }
-    gdb_cache_maybe_gdb_exit $name $gdb_data_cache(${cache_name},exit)
+    gdb_cache_maybe_gdb_exit $name $gdb_data_cache(${cache_name},exit) \
+       $gdb_data_cache(${cache_name},also_called)
     return $gdb_data_cache(${cache_name},value)
 }