From: Simon Marchi Date: Thu, 12 Mar 2020 18:22:23 +0000 (-0400) Subject: gdb: make gdb.arch/amd64-disp-step-avx.exp actually test displaced stepping X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=40310f30a51c1343b954d1fee7feb9a1d9455e9f;p=binutils-gdb.git gdb: make gdb.arch/amd64-disp-step-avx.exp actually test displaced stepping The test gdb.arch/amd64-disp-step-avx.exp is meant to test that doing a displaced step of an AVX instruction works correctly. However, I found (by pure coincidence) that the test instructions are not actually displaced stepped. Rather, they are inline-stepped, so the test is not actually testing what it's meant to test. This is what a portion of the test binary looks like: 0000000000400180 <_start>: 400180: 90 nop 0000000000400181
: 400181: 90 nop 0000000000400182 : 400182: c5 fb 10 05 0e 00 00 vmovsd 0xe(%rip),%xmm0 # 400198 400189: 00 000000000040018a : 40018a: 90 nop The instruction at 0x400182 is the one we want to test a displaced step for. A breakpoint is placed at 0x400182 and ran to. The execution is then resumed from there, forcing a step-over (which should normally be a displaced step) of the breakpoint. However, the displaced stepping buffer is at the _start label, and that means a breakpoint is present in the displaced stepping buffer. The breakpoint_in_range_p check in displaced_step_prepare_throw evaluates to true, which makes displaced_step_prepare_throw fail, forcing GDB to fall back on an in-line step. This can be easily observed by placing a `gdb_assert (false)` inside the breakpoint_in_range_p condition, in displaced_step_prepare_throw, and running gdb.arch/amd64-disp-step-avx.exp. The assertion will make the test fail. The proposed fix is to pad `_start` with a bunch of nops so that the test instruction is out of the displaced step buffer. I also think it would be good to enhance the test to make sure that we are testing displaced stepping as intended. I did that by enabling "set debug displaced on" while we step over the interesting instruction, and matching a message printed only when a displaced step is executed. gdb/testsuite/ChangeLog: * gdb.arch/amd64-disp-step-avx.S: Add nops after _start. * gdb.arch/amd64-disp-step-avx.exp: Enable "set debug displaced on" while stepping over the test instruction, match printed message. --- diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 5c5576ed1e9..48588a391bb 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2020-03-12 Simon Marchi + + * gdb.arch/amd64-disp-step-avx.S: Add nops after _start. + * gdb.arch/amd64-disp-step-avx.exp: Enable "set debug displaced + on" while stepping over the test instruction, match printed + message. + 2020-03-12 Tom de Vries * gdb.base/info-types.exp: Use exp_continue during matching of output diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step-avx.S b/gdb/testsuite/gdb.arch/amd64-disp-step-avx.S index 76747360b94..c72f6a5ca89 100644 --- a/gdb/testsuite/gdb.arch/amd64-disp-step-avx.S +++ b/gdb/testsuite/gdb.arch/amd64-disp-step-avx.S @@ -22,7 +22,12 @@ .global _start,main _start: + # The area at _start is used as the displaced stepping buffer. Put + # more than enough nop instructions so that the instructions under test + # below don't conflict with it. + .rept 200 nop + .endr main: nop diff --git a/gdb/testsuite/gdb.arch/amd64-disp-step-avx.exp b/gdb/testsuite/gdb.arch/amd64-disp-step-avx.exp index 23282f66780..ab83fe6770a 100644 --- a/gdb/testsuite/gdb.arch/amd64-disp-step-avx.exp +++ b/gdb/testsuite/gdb.arch/amd64-disp-step-avx.exp @@ -92,10 +92,16 @@ proc disp_step_func { func } { set value "0xdeadbeefd3adb33f" set_regs $value + # Turn "debug displaced" on to make sure a displaced step is actually + # executed, not an inline step. + gdb_test_no_output "set debug displaced on" + gdb_test "continue" \ - "Continuing.*Breakpoint.*, ${test_end_label} ().*" \ + "Continuing.*displaced: displaced pc to.*Breakpoint.*, ${test_end_label} ().*" \ "continue to ${test_end_label}" + gdb_test_no_output "set debug displaced off" + verify_regs $value }