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 0726729d34
  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>
This commit is contained in:
Andrew Burgess 2024-08-07 14:51:06 +01:00
parent be14b683af
commit 40a1603112

View File

@ -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)
}