Re: [PATCH] gdb.reverse/time-reverse.exp: test both time syscall and C time function (was: Re: [pushed] [gdb/testsuite] Require syscall time in gdb.reverse/time-reverse.exp)

[Tom de Vries <tdevries@suse.de>]

On 2/22/23 17:12, Pedro Alves wrote:
> On 2023-02-21 1:10 p.m., Tom de Vries via Gdb-patches wrote:
>> On aarch64-linux, I run into:
>> ...
>> Running gdb.reverse/time-reverse.exp ...
>> gdb compile failed, gdb.reverse/time-reverse.c: In function 'main':
>> gdb.reverse/time-reverse.c:39:12: error: 'SYS_time' undeclared \
>>    (first use in this function); did you mean 'SYS_times'?
>>     syscall (SYS_time, &time_global);
>>              ^~~~~~~~
>>              SYS_times
>> gdb.reverse/time-reverse.c:39:12: note: each undeclared identifier is \
>>    reported only once for each function it appears in
>> UNTESTED: gdb.reverse/time-reverse.exp: failed to prepare
>> ...
>>
>> Fix this by adding a new proc have_syscall, and requiring syscall time, such
>> that we have instead:
>> ...
>> UNSUPPORTED: gdb.reverse/time-reverse.exp: require failed: \
>>    expr [have_syscall time]
>> ...
>>
>> Tested on x86_64-linux and aarch64-linux.
> 
> I think the patch below would be even better.  Does it work on aarch64?
> 

It does for me.

FWIW, doing an strace on the exec, I don't see any syscall related to time.

LGTM.

Thanks,
- Tom

>  From 433d6856be51a9df39e0285af9ae1520af59346a Mon Sep 17 00:00:00 2001
> From: Pedro Alves <pedro@palves.net>
> Date: Wed, 22 Feb 2023 15:40:58 +0000
> Subject: [PATCH] gdb.reverse/time-reverse.exp: test both time syscall and C
>   time function
> 
> Instead of only testing this on systems that have a SYS_time syscall,
> test it everywhere using the time(2) C function, and in addition, run
> the tests again using the SYS_time syscall.
> 
> The C variant ensures that if some platform uses some syscall we are
> not aware of yet, we'll still exercise it, and likely fail, at which
> point we should teach GDB about the syscall.
> 
> The explicit syscall variant is useful on platforms where the C
> function does not call a syscall at all by default, e.g., on some
> systems the C time function wraps an implementation provided by the
> vDSO.
> 
> Change-Id: Id4b755d76577d02c46b8acbfa249d9c31b587633
> ---
>   gdb/testsuite/gdb.reverse/time-reverse.c   |  8 ++-
>   gdb/testsuite/gdb.reverse/time-reverse.exp | 71 +++++++++++++++-------
>   2 files changed, 57 insertions(+), 22 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.reverse/time-reverse.c b/gdb/testsuite/gdb.reverse/time-reverse.c
> index 668fb102ad2..43f5762d447 100644
> --- a/gdb/testsuite/gdb.reverse/time-reverse.c
> +++ b/gdb/testsuite/gdb.reverse/time-reverse.c
> @@ -20,6 +20,12 @@
>   #include <sys/syscall.h>
>   #include <unistd.h>
>   
> +#ifdef USE_SYSCALL
> +# define my_time(TLOC) syscall (SYS_time, &time_global)
> +#else
> +# define my_time(TLOC) time (TLOC)
> +#endif
> +
>   void
>   marker1 (void)
>   {
> @@ -36,7 +42,7 @@ int
>   main (void)
>   {
>     marker1 ();
> -  syscall (SYS_time, &time_global);
> +  my_time (&time_global);
>     marker2 ();
>     return 0;
>   }
> diff --git a/gdb/testsuite/gdb.reverse/time-reverse.exp b/gdb/testsuite/gdb.reverse/time-reverse.exp
> index befda65d836..91f9911c33a 100644
> --- a/gdb/testsuite/gdb.reverse/time-reverse.exp
> +++ b/gdb/testsuite/gdb.reverse/time-reverse.exp
> @@ -23,33 +23,62 @@ require supports_reverse
>   
>   standard_testfile
>   
> -require {expr [have_syscall time]}
> +# MODE is either "syscall" for testing the time syscall explicitly, or
> +# "c" for testing the C time(2) function.
> +proc test {mode} {
> +    set options {debug}
>   
> -if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
> -    return -1
> -}
> +    if {$mode == "syscall"} {
> +	lappend options additional_flags=-DUSE_SYSCALL
> +    } elseif {$mode != "c"} {
> +	error "unrecognized mode: $mode"
> +    }
>   
> -runto_main
> +    if { [prepare_for_testing "failed to prepare" $::testfile-$mode $::srcfile $options] } {
> +	return
> +    }
>   
> -if [supports_process_record] {
> -    # Activate process record/replay
> -    gdb_test_no_output "record" "turn on process record"
> -}
> +    runto_main
> +
> +    if [supports_process_record] {
> +	# Activate process record/replay
> +	gdb_test_no_output "record" "turn on process record"
> +    }
> +
> +    gdb_test "break marker2" \
> +	"Breakpoint $::decimal at $::hex: file .*$::srcfile, line $::decimal.*" \
> +	"set breakpoint at marker2"
> +
> +    gdb_continue_to_breakpoint "marker2" ".*$::srcfile:.*"
>   
> -gdb_test "break marker2" \
> -    "Breakpoint $decimal at $hex: file .*$srcfile, line $decimal.*" \
> -    "set breakpoint at marker2"
> +    gdb_test "break marker1" \
> +	"Breakpoint $::decimal at $::hex: file .*$::srcfile, line $::decimal.*" \
> +	"set breakpoint at marker1"
>   
> -gdb_continue_to_breakpoint "marker2" ".*$srcfile:.*"
> +    gdb_test "reverse-continue" ".*$::srcfile:$::decimal.*" "reverse to marker1"
>   
> -gdb_test "break marker1" \
> -    "Breakpoint $decimal at $hex: file .*$srcfile, line $decimal.*" \
> -    "set breakpoint at marker1"
> +    # If the variable was recorded properly, the old contents (-1)
> +    # will be remembered.  If not, new contents (current time) will be
> +    # used, and the test will fail.
>   
> -gdb_test "reverse-continue" ".*$srcfile:$decimal.*" "reverse to marker1"
> +    gdb_test "print time_global" ".* = -1" "check time record"
> +}
>   
> -# If the variable was recorded properly on syscall, the old contents (-1)
> -# will be remembered.  If not, new contents (current time) will be used,
> -# and the test will fail.
> +# Test both using the syscall explicitly, and using the time(2) C
> +# function.
> +#
> +# The C variant ensures that if some platform uses some syscall we are
> +# not aware of yet, we'll still exercise it (and likely fail).
> +#
> +# The explicit syscall variant is useful on platforms where the C
> +# function does not call a syscall at all by default, e.g., on some
> +# systems the C time function wraps an implementation provided by the
> +# vDSO.
>   
> -gdb_test "print time_global" ".* = -1" "check time record"
> +foreach_with_prefix mode {syscall c} {
> +    if {$mode == "syscall" && ![have_syscall time]} {
> +	continue
> +    }
> +
> +    test $mode
> +}
> 
> base-commit: 5e39600a691e3ba76acf6ab94edb24844c2e82b7