Re: [PATCH v3] Fix reverse stepping multiple contiguous PC ranges over the line table.

[Bruno Larsen <blarsen@redhat.com>]

On 10/05/2023 19:16, Carl Love wrote:
>>> +
>>> +require supports_reverse
>>> +
>>> +# This test uses the gcc no-column-info command which was added in
>>> gcc 7.1.
>>> +require get_compiler_info  "gcc-7-*"
> I put the compiler check in last.  When I ran it, I obviously didn't
> double check gdb/testsuite/gdb.log to make sure it really worked.  I
> normally try to make a point of double checking the log file.  I have
> been burned before thinking it was OK when there were no errors visible
> on the command line. The above command fails if you check the log file.
>> By constructing your regex like this, you are only allowing this test
>> to
>> be run on gcc 7. Anything later is also not accepted. I would do
>> something like (Warning, untested)
>>
>> require get_compiler_info "gcc"
>> require !get_compiler_info "gcc-[1-6]-*"
> I couldn't get require to work like that.  The get_compiler_info
> doesn't seem take "gcc" as an argument.
>
> I changed the test to:
>
> if {![test_compiler_info {gcc-*}]
>      || [test_compiler_info {gcc-[1-6]-*}]} {
>      return
> }
>
> With this, I do see the correct number of passes in
> gdb/testsuite/gdb.log.
Yeah, this is perfectly fine IMO, since this is a more complicated use 
of "require".
>
>> Which requires gcc, but does not allow versions 1 to 6. There is
>> probably a way to do it with a single require line, but I'm not the
>> best
>> with regexes.
>>
>>> +
>>> +proc run_tests {msg} {
>>> +    global srcfile
>>> +    global executable
>>> +
>>> +    runto_main
>>> +    set target_remote [gdb_is_target_remote]
>> When probing for target remote, GDB will emit pass/fails with
>> hardcoded
>> names, so the current proc setup gives us some duplicated test names.
>>
>> I would suggest that, instead of passing a message as a parameter,
>> you
>> wrapped all function calls in a with_test_prefix scope, like:
>>
>> with_test_prefix "with-column-info" {
>>       run_test
>> }
> OK, I changed from passing in an argument and did the wrapped calls to
> run_test instead. Note, this still didn't fix the duplicate test names
> for turning on record.
>
>>> +
>>> +    gdb_test_no_output "record" "turn on process record"
>>> +
>>> +    # This regression test verifies the reverse-step and reverse-
>>> next commands
>>> +    # work properly when executing backwards thru a source line
>>> containing
>>> +    # two function calls on the same source line, i.e. func1 ();
>>> func2 ();
>>> +    # This test is compiled so the dwarf info not contain the line
>>> table
>>> +    # information.
>>> +
>>> +    # Test 1, reverse-next command
>>> +    # Set breakpoint at the line after the function calls.
>>> +    set bp_start_reverse_test [gdb_get_line_number "START REVERSE
>>> TEST" \
>>> +				   $srcfile]
>>> +    gdb_breakpoint $srcfile:$bp_start_reverse_test temporary
>>> +
>>> +    # Continue to break point for reverse-next test.
>>> +    # Command definition:  reverse-next [count]
>>> +    #   Run backward to the beginning of the previous line
>>> executed in the
>>> +    #   current (innermost) stack frame. If the line contains
>>> function calls,
>>> +    #   they will be “un-executed” without stopping. Starting from
>>> the first
>>> +    #   line of a function, reverse-next will take you back to the
>>> caller of
>>> +    #   that function, before the function was called, just as the
>>> normal next
>>> +    #   command would take you from the last line of a function
>>> back to its
>>> +    #   return to its caller 2 .
>>> +    gdb_continue_to_breakpoint \
>>> +	"$msg: test1: stopped at command reverse-next test start
>>> location" \
>>> +	".*$srcfile:$bp_start_reverse_test\r\n.*"
>>> +
>>> +    # The reverse-next should step all the way back to the
>>> beginning of the
>>> +    # line, i.e. at the beginning of the func1 call.
>>> +    gdb_test "reverse-next" ".*func1 \\(\\); func2 \\(\\);.*" \
>>> +	"$msg: test1: reverse-next to line with two functions"
>>> +
>>> +    # We should be stopped at the first instruction of the line. A
>>> reverse-step
>>> +    # should step back and stop at the beginning of the previous
>>> line b = 2,
>>> +    # i.e. not in func1 ().
>>> +    gdb_test "reverse-stepi" ".*b = 2;.*" \
>>> +	"$msg: test1: reverse-stepi to previous line b = 2"
>>> +
>>> +
>>> +    # Setup for test 2
>>> +    clean_restart $executable
>>> +    runto_main
>>> +
>>> +    gdb_test_no_output "record" "turn on process record"
>> This gives a duplicate test name from setting up for the first test.
>> Adding "test 2:(...)" in here solves it.
> I couldn't figure out how to get the above syntax to work.  So I used
> the with_test_prefix instead, i.e.
oops, sorry, that's what I get for sending a patch review at the end of 
a long work day. That wasn't some arcane magic of TCL, it was just me 
being too lazy to type out the whole message. Sorry for the wild goose 
chase.
>
>      with_test_prefix "test2" {
>          gdb_test_no_output "record" "turn on process record"
>      }
>
> That fixed the duplicate names.  I also wrapped the first record with
> "test1" for consistency.
>

-- 
Cheers,
Bruno