[prev in list] [next in list] [prev in thread] [next in thread] 

List:       gdb-patches
Subject:    Re: [PATCH v2 2/2] ppc: recognize all program traps
From:       Pedro Alves <pedro () palves ! net>
Date:       2021-11-30 12:17:35
Message-ID: d03bd578-798a-ff4b-f8f9-9034a11a5738 () palves ! net
[Download RAW message or body]

On 2021-11-24 13:09, Jan Vrany via Gdb-patches wrote:
> +# Number of expected SIGTRAP's to get.  This needs to be kept in sync
> +# with the source file.
> +set expected_traps 3
> +set keep_going 1
> +set count 0
> +
> +# Make sure we have a lower timeout in case GDB doesn't support a particular
> +# instruction.  Such instruction will cause GDB to loop infinitely.
> +while {$keep_going} {
> +    # Continue to next program breakpoint instruction.
> +    gdb_test_multiple "continue" "trap instruction $count causes SIGTRAP" {
> +	-re "Program received signal SIGTRAP, Trace/breakpoint trap.*$gdb_prompt $" {
> +	    pass $gdb_test_name
> +
> +	    # Advance PC to next instruction
> +	    gdb_test "set \$pc = \$pc + 4" "" "advance past trap instruction $count"
> +
> +	    incr count
> +	}
> +	# We've reached the end of the test.
> +	-re "exited with code 01.*$gdb_prompt $" {
> +	    set keep_going 0
> +	}
> +	timeout {
> +	    fail $gdb_test_name
> +	    set keep_going 0
> +	}
> +    }
> +}
> +


This while loop will iterate forever if gdb_test_multiple trips on some internal match, like
some unexpected text out of gdb that ends with a prompt, or an internal error.  The logic of
"keep_going" should be reversed so that the loop breaks if anything goes wrong.   Also, the
last continue to "exited with code 01" doesn't itself issue a pass unlike the other continues,
which seems inconsistent.  BTW, if we flip the logic, we no longer need the timeout check.

Something like this (completely untested):

~~~~~~~~~~~~
# Number of expected SIGTRAP's to get.  This needs to be kept in sync
# with the source file.
set expected_traps 3
set count 0
set keep_going 1

# Make sure we have a lower timeout in case GDB doesn't support a particular
# instruction.  Such instruction will cause GDB to loop infinitely.
while {$keep_going} {

    set keep_going 0

    # Continue to next program breakpoint instruction.
    gdb_test_multiple "continue" "trap instruction $count causes SIGTRAP" {
	-re "Program received signal SIGTRAP, Trace/breakpoint trap.*$gdb_prompt $" {
	    pass $gdb_test_name

	    # Advance PC to next instruction
	    gdb_test "set \$pc = \$pc + 4" "" "advance past trap instruction $count"

	    incr count
	    if {$count < $expected_traps} {
	       set keep_going 1
	    }
	}
    }

# Verify we stopped at the expected number of SIGTRAP's.
gdb_assert {$count == $expected_traps} "all trap instructions triggered"

# One last continue to reach the end of the test, to make sure we don't get
# another SIGTRAP.
gdb_test "continue" "exited with code 01.*" "continue to end"
~~~~~~~~~~~~


Some thing in the other file.  Or you could merge the files, and make the exp file
set a different .S filename and different "expected_traps" depending on arch, to
avoid duplication.


BTW, I don't understand what the "Make sure we have a lower timeout" comment is referring
to -- I don't see anything changing the timeout.

BTW ², seems strange to expect that the program exits with exit code 1 instead of 0 on a success run.
I can't write PPC assembly to save my life, but I don't see any "1", or writing to r3 in in
the assembly, so I guess that is assuming the value is already 1 on entry.  I wonder whether that's a
good assumption in all environments.
[prev in list] [next in list] [prev in thread] [next in thread]