diff mbox series

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

Message ID 20230522104623.214121-1-blarsen@redhat.com
State New
Headers
Series [v2] gdb/testsuite: change hardcoded assembly in gdb.arch/disp-step-insn-reloc.exp |

Commit Message

Guinevere Larsen May 22, 2023, 10:46 a.m. UTC 
  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.
---
 gdb/testsuite/gdb.arch/insn-reloc.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Andrew Burgess May 23, 2023, 8:36 a.m. UTC | #1 
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
diff mbox series

Patch

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)