[PATCH] gdbserver short-circuit-argument-list failures
Joel Brobecker
brobecker@adacore.com
Sun Dec 23 06:34:00 GMT 2018
Hello,
It looks like noone has reviewed this patch, yet? If not, I apologize
for the delay.
> This patch fixes test case failures observed when running
> short-circuit-argument-list.exp with gdb server boards. Thanks to Sergio
> Durigan Junior for pointing this out.
>
> Assertions failed with the native{,-extended}-gdbserver boards as the
> standard output from the test program appears in a different location
> than observed on non-gdbserver boards. This standard output was used to
> determine whether a function, which had been logically short-circuited,
> was called or not. Since the location of the standard out cannot be
> relied upon to verify this, a new mechanism was needed.
Agreed.
> The test program now records function calls in module variables named the
> same as the function with a "_called" suffix. These variables can then be
> queried from the test case to verify the occurrence of a call.
Good approach.
> A method "reset_called_flags" has also been added to the test program, so
> that any future assertions added to this test can ensure a fresh set of
> zeros before proceeding.
My initial reaction was that you really didn't need the reset at all,
just making sure that the counters are as expect at all times.
However, there is indeed value in resetting them all before each test,
so that one test failing doesn't cause subsequent tests to also fail.
Given that GDB's behavior should be purely independent of the target,
I find that the risk of this happening is very low; but resetting
the counters does make the test more robust.
I would like to suggest a different approach, where you avoid a function
call. Why not define a function inside the test script itself, which
resets the counters for you.
that the value of those counters is in fact zero.
Another suggestion: Why not put all the counters in a singe struct
(or the fortan equivalent). That way, you can print them all in one
single print command. And, if supported by GDB, maybe even set
the whole thing through one single command.
Finally, zero tends to be a very common value in memory. What do you
think of using an arbitrary "magick value"? That way, if for some
reason we're reading the variables at the wrong location, we are
less likely to miss a regression, just because the wrong memory location
we read from also happens to be zero-ed out.
>
> Regression tested on x86_64, aarch64 and ppc64le.
> Regression tested with Ada on x86_64.
> Regression tested with the native{,-extended}-gdbserver boards on x86_64.
>
> gdb/testsuite/ChangeLog:
>
> 2018-12-03 Richard Bunt <richard.bunt@arm.com>
>
> * gdb.fortran/short-circuit-argument-list.exp: Remove reliance
> on user program standard output.
> * gdb.fortran/short-circuit-argument-list.f90: Record function
> calls.
A few comments below.
> ---
> .../gdb.fortran/short-circuit-argument-list.exp | 50 +++++++++++++++-------
> .../gdb.fortran/short-circuit-argument-list.f90 | 33 +++++++++++---
> 2 files changed, 63 insertions(+), 20 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.fortran/short-circuit-argument-list.exp b/gdb/testsuite/gdb.fortran/short-circuit-argument-list.exp
> index 1b948bb19d289e5327cb2caa3bf665e33cd6d98d..7032dbeea2eb487c5b98713bbf1c2b9807b692c9 100644
> --- a/gdb/testsuite/gdb.fortran/short-circuit-argument-list.exp
> +++ b/gdb/testsuite/gdb.fortran/short-circuit-argument-list.exp
> @@ -49,20 +49,32 @@ foreach_with_prefix arg {"No" "One" "Two"} {
> set trimmed_args [string trimright $argument_list ,]
> set arg_lower [string tolower $arg]
> gdb_test "p function_no_arg_false() .OR. function_${arg_lower}_arg($trimmed_args)" \
> - " $arg, return true.\r\n\\\$$decimal = .TRUE."
> - # Check the skipped function has not printed anything by asserting the
> - # absence of the full stop from its message.
> + "\\\$$decimal = .TRUE."
A small note: for "print" tests, we typically don't bother matching
the number before the '=', eg:
gdb_test "print 42" " = 42"
This simplifies reading of the test.
> + gdb_test "p reset_called_flags()" "VOID"
If you follow my propsal, this is going to become moot, but if not,
my comment would have been: This is fine, but I would change
the above to:
gdb_test_no_output "call reset_called_flags()"
> gdb_test "p .TRUE. .OR. function_${arg_lower}_arg($trimmed_args)" \
> - "\[^.\]\r\n\\\$$decimal = .TRUE."
> + "\\\$$decimal = .TRUE."
> + # Check that none of the short-circuited functions have been called.
> + gdb_test "p function_no_arg_called" "0"
Be sure to match the equal sign in the expected output also.
In this case, anything that ends with a '0' would return a PASS.
gdb_test "p function_no_arg_called" " = <val>"
Same comments for the rest of the testcase.
> + gdb_test "p function_one_arg_called" "0"
> + gdb_test "p function_two_arg_called" "0"
> append argument_list " .TRUE.,"
> }
>
> -# Check nested calls
> -gdb_test "p function_one_arg(.FALSE. .OR. function_no_arg())" \
> - " No, return true.\r\n One, return true.\r\n\\\$$decimal = .TRUE."
> +with_test_prefix "nested call not skipped" {
> + gdb_test "p reset_called_flags()" "VOID"
> + # Check nested calls
> + gdb_test "p function_one_arg(.FALSE. .OR. function_no_arg())" \
> + "\\\$$decimal = .TRUE."
> + gdb_test "p function_no_arg_called" "1"
> + gdb_test "p function_one_arg_called" "1"
> +}
>
> -gdb_test "p function_one_arg(.TRUE. .OR. function_no_arg())" \
> - "\[^.\]\r\n One, return true.\r\n\\\$$decimal = .TRUE."
> +with_test_prefix "nested call skipped" {
> + gdb_test "p function_one_arg(.TRUE. .OR. function_no_arg())" \
> + "\\\$$decimal = .TRUE."
> + gdb_test "p function_no_arg_called" "1"
> + gdb_test "p function_one_arg_called" "2"
> +}
>
> # Vary number of components in the expression to skip.
> set expression "p .TRUE."
> @@ -97,10 +109,18 @@ foreach_with_prefix range1 {"1:2" ":" ":2" "1:"} {
> # Check evaluation of substring operations in logical expressions.
> gdb_test "p .FALSE. .OR. binary_string(1)" "\\\$$decimal = .FALSE."
>
> -# Function call and substring skip.
> -gdb_test "p .TRUE. .OR. function_one_arg(binary_string(1))" \
> - "\\\$$decimal = .TRUE."
> +with_test_prefix "binary string skip" {
> + gdb_test "p reset_called_flags()" "VOID"
> + # Function call and substring skip.
> + gdb_test "p .TRUE. .OR. function_one_arg(binary_string(1))" \
> + "\\\$$decimal = .TRUE."
> + gdb_test "p function_one_arg_called" "0"
> +}
>
> -# Function call and array skip.
> -gdb_test "p .TRUE. .OR. function_array(binary_string)" \
> - "\\\$$decimal = .TRUE."
> +with_test_prefix "array skip" {
> + # Function call and array skip.
> + gdb_test "p reset_called_flags()" "VOID"
> + gdb_test "p .TRUE. .OR. function_array(binary_string)" \
> + "\\\$$decimal = .TRUE."
> + gdb_test "p function_array_called" "0"
> +}
> diff --git a/gdb/testsuite/gdb.fortran/short-circuit-argument-list.f90 b/gdb/testsuite/gdb.fortran/short-circuit-argument-list.f90
> index 5d8b9c73a705c598b513fd85a8d710c7a7dabebf..bd1f94ad6db3c43135c9aa4476d33e694fb52405 100644
> --- a/gdb/testsuite/gdb.fortran/short-circuit-argument-list.f90
> +++ b/gdb/testsuite/gdb.fortran/short-circuit-argument-list.f90
> @@ -15,36 +15,59 @@
>
> ! Source code for short-circuit-argument-list.exp.
>
> +module called_flags
> + implicit none
> + integer :: function_no_arg_called = 0
> + integer :: function_no_arg_false_called = 0
> + integer :: function_one_arg_called = 0
> + integer :: function_two_arg_called = 0
> + integer :: function_array_called = 0
> +contains
> + subroutine reset_called_flags
> + function_no_arg_called = 0
> + function_no_arg_false_called = 0
> + function_one_arg_called = 0
> + function_two_arg_called = 0
> + function_array_called = 0
> + end subroutine reset_called_flags
> +end module called_flags
> +
> logical function function_no_arg()
> - print *, "No, return true."
> + use called_flags
> + function_no_arg_called = function_no_arg_called + 1
> function_no_arg = .TRUE.
> end function function_no_arg
>
> logical function function_no_arg_false()
> + use called_flags
> + function_no_arg_false_called = function_no_arg_false_called + 1
> function_no_arg_false = .FALSE.
> end function function_no_arg_false
>
> logical function function_one_arg(x)
> + use called_flags
> logical, intent(in) :: x
> - print *, "One, return true."
> + function_one_arg_called = function_one_arg_called + 1
> function_one_arg = .TRUE.
> end function function_one_arg
>
> logical function function_two_arg(x, y)
> + use called_flags
> logical, intent(in) :: x, y
> - print *, "Two, return true."
> + function_two_arg_called = function_two_arg_called + 1
> function_two_arg = .TRUE.
> end function function_two_arg
>
> logical function function_array(logical_array)
> + use called_flags
> logical, dimension(4,2), target, intent(in) :: logical_array
> logical, dimension(:,:), pointer :: p
> - p => logical_array
> - print *, "Array, return true.", p(1,1), logical_array(1,1)
> + function_array_called = function_array_called + 1
> function_array = .TRUE.
> end function function_array
>
> program generate_truth_table
> + use called_flags
> implicit none
> interface
> logical function function_no_arg()
> --
> 2.7.4
--
Joel
More information about the Gdb-patches
mailing list