Re: [PATCH v2] gdb/testsuite: change hardcoded assembly in gdb.arch/disp-step-insn-reloc.exp

[Andrew Burgess <aburgess@redhat.com>]

Bruno Larsen <blarsen@redhat.com> writes:

> When testing gdb.arch/disp-step-insn-reloc.exp with clang in an x86_64
> machine, the compiled test case would segfault when returning from
> the function can_relocate_call, with a suggestion of a broken stack.
> The example assembly in the commment was the following:
>
>    f:
>      MOV $1, %[ok]
>      JMP end
>    set_point0:
>      CALL f ; tracepoint here.
>    end:
>
> And the segmentation fault happening at the final "ret" instruction of
> can_relocate_call.  Looking at the disassembled version of the later
> half of the important function, we see:
>
> Clang version (f starting at 11a4):
>   00000000000011ae <set_point0>:
>       11ae:       e8 f1 ff ff ff          callq  11a4 <can_relocate_call+0x14>
>       11b3:       89 45 fc                mov    %eax,-0x4(%rbp)
>       11b6:       83 7d fc 01             cmpl   $0x1,-0x4(%rbp)
>       11ba:       0f 85 0a 00 00 00       jne    11ca <set_point0+0x1c>
>       11c0:       e8 5b 00 00 00          callq  1220 <pass>
>       11c5:       e9 05 00 00 00          jmpq   11cf <set_point0+0x21>
>       11ca:       e8 61 00 00 00          callq  1230 <fail>
>       11cf:       48 83 c4 10             add    $0x10,%rsp
>       11d3:       5d                      pop    %rbp
>       11d4:       c3                      retq
>       11d5:       66 66 2e 0f 1f 84 00    data16 nopw %cs:0x0(%rax,%rax,1)
>       11dc:       00 00 00 00
>
> gcc version (f starting at 401125):
>   000000000040112c <set_point0>:
>     40112c:       e8 f4 ff ff ff          callq  401125 <can_relocate_call+0x11>
>     401131:       89 45 fc                mov    %eax,-0x4(%rbp)
>     401134:       83 7d fc 01             cmpl   $0x1,-0x4(%rbp)
>     401138:       75 07                   jne    401141 <set_point0+0x15>
>     40113a:       e8 c7 ff ff ff          callq  401106 <pass>
>     40113f:       eb 05                   jmp    401146 <set_point0+0x1a>
>     401141:       e8 c7 ff ff ff          callq  40110d <fail>
>     401146:       90                      nop
>     401147:       c9                      leaveq
>     401148:       c3                      retq
>
> The epilogue of set_point0 (11cf for clang, 401146 for gcc) is the main
> difference: GCC's version uses the leaveq instruction, which resets rsp
> based on rbp, while clang adds the same constant to rsp that it
> subtracted in the prologue.  Clang fails because the return address that
> is added by the "call f" instruction isn't accounted for.
>
> This commit fixes that by adding a return instruction to f, which leaves
> the rsp as the compilers would expect.

Looks great.  Thanks for updating the commit message.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew


> ---
>  gdb/testsuite/gdb.arch/insn-reloc.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.arch/insn-reloc.c b/gdb/testsuite/gdb.arch/insn-reloc.c
> index f687c2c5631..365e6180057 100644
> --- a/gdb/testsuite/gdb.arch/insn-reloc.c
> +++ b/gdb/testsuite/gdb.arch/insn-reloc.c
> @@ -49,10 +49,9 @@ fail (void)
>       JMP set_point0
>     f:
>       MOV $1, %[ok]
> -     JMP end
> +     RET
>     set_point0:
>       CALL f ; tracepoint here.
> -   end:
>  
>     */
>  
> @@ -65,10 +64,9 @@ can_relocate_call (void)
>         "  jmp " SYMBOL (set_point0) "\n"
>         "0:\n"
>         "  mov $1, %[ok]\n"
> -       "  jmp 1f\n"
> +       "  ret\n"
>         SYMBOL (set_point0) ":\n"
>         "  call 0b\n"
> -       "1:\n"
>         : [ok] "=r" (ok));
>  
>    if (ok == 1)
> -- 
> 2.40.1