From a10b3cf14888c2778261cba92f52055716b6d69d Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Sun, 30 Jul 2017 14:10:32 +0000 Subject: [PATCH] i386: Update INCOMING_FRAME_SP_OFFSET for exception handler Since there is an extra error code passed to the exception handler, INCOMING_FRAME_SP_OFFSET is return address plus error code for the exception handler. This patch updates INCOMING_FRAME_SP_OFFSET to the correct value for the exception handler. This patch exposed a bug in DWARF stack frame CFI generation, which assumes that INCOMING_FRAME_SP_OFFSET is the same for all functions: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81570 It sets and caches the incoming stack frame offset with the same INCOMING_FRAME_SP_OFFSET for all functions. When there are both exception handler and normal function in the same input, the wrong incoming stack frame offset is used for exception handler or normal function, which leads to FAIL: gcc.dg/guality/pr68037-1.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects line 33 error == 0x12345670 FAIL: gcc.dg/guality/pr68037-1.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects line 33 frame->ip == 0x12345671 FAIL: gcc.dg/guality/pr68037-1.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects line 33 frame->cs == 0x12345672 FAIL: gcc.dg/guality/pr68037-1.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects line 33 frame->flags == 0x12345673 FAIL: gcc.dg/guality/pr68037-1.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects line 33 frame->sp == 0x12345674 FAIL: gcc.dg/guality/pr68037-1.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects line 33 frame->ss == 0x12345675 With the patch for PR 81570: https://gcc.gnu.org/ml/gcc-patches/2017-07/msg01851.html applied, there are no regressions on i686 and x86-64. gcc/ PR target/79793 * config/i386/i386.c (ix86_function_arg): Update arguments for exception handler. (ix86_compute_frame_layout): Set the initial stack offset to INCOMING_FRAME_SP_OFFSET. Update red-zone offset with INCOMING_FRAME_SP_OFFSET. (ix86_expand_epilogue): Don't pop the 'ERROR_CODE' off the stack before exception handler returns. * config/i386/i386.h (INCOMING_FRAME_SP_OFFSET): Add the the 'ERROR_CODE' for exception handler. gcc/testsuite/ PR target/79793 * gcc.dg/guality/pr68037-1.c: Update gdb breakpoints. * gcc.target/i386/interrupt-5.c (interrupt_frame): New struct. (foo): Check the builtin return address against the return address in interrupt frame. * gcc.target/i386/pr79793-1.c: New test. * gcc.target/i386/pr79793-2.c: Likewise. From-SVN: r250721 --- gcc/ChangeLog | 13 +++++++ gcc/config/i386/i386.c | 39 +++++++-------------- gcc/config/i386/i386.h | 7 ++-- gcc/testsuite/ChangeLog | 10 ++++++ gcc/testsuite/gcc.dg/guality/pr68037-1.c | 12 +++---- gcc/testsuite/gcc.target/i386/interrupt-5.c | 13 +++++-- gcc/testsuite/gcc.target/i386/pr79793-1.c | 14 ++++++++ gcc/testsuite/gcc.target/i386/pr79793-2.c | 16 +++++++++ 8 files changed, 88 insertions(+), 36 deletions(-) create mode 100644 gcc/testsuite/gcc.target/i386/pr79793-1.c create mode 100644 gcc/testsuite/gcc.target/i386/pr79793-2.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 300857938d1..5a2e5fac42c 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,16 @@ +2017-07-30 H.J. Lu + + PR target/79793 + * config/i386/i386.c (ix86_function_arg): Update arguments for + exception handler. + (ix86_compute_frame_layout): Set the initial stack offset to + INCOMING_FRAME_SP_OFFSET. Update red-zone offset with + INCOMING_FRAME_SP_OFFSET. + (ix86_expand_epilogue): Don't pop the 'ERROR_CODE' off the + stack before exception handler returns. + * config/i386/i386.h (INCOMING_FRAME_SP_OFFSET): Add the + the 'ERROR_CODE' for exception handler. + 2017-07-30 Uros Bizjak * config/i386/i386.h (ASM_PRINTF_EXTENSIONS): New macro. diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index c0213a66ae0..47e2ae8ce2e 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -10421,25 +10421,22 @@ ix86_function_arg (cumulative_args_t cum_v, machine_mode omode, { /* This is the pointer argument. */ gcc_assert (TYPE_MODE (type) == Pmode); - if (cfun->machine->func_type == TYPE_INTERRUPT) - /* -WORD(AP) in the current frame in interrupt handler. */ - arg = plus_constant (Pmode, arg_pointer_rtx, - -UNITS_PER_WORD); - else - /* (AP) in the current frame in exception handler. */ - arg = arg_pointer_rtx; + /* It is at -WORD(AP) in the current frame in interrupt and + exception handlers. */ + arg = plus_constant (Pmode, arg_pointer_rtx, -UNITS_PER_WORD); } else { gcc_assert (cfun->machine->func_type == TYPE_EXCEPTION && TREE_CODE (type) == INTEGER_TYPE && TYPE_MODE (type) == word_mode); - /* The integer argument is the error code at -WORD(AP) in - the current frame in exception handler. */ + /* The error code is the word-mode integer argument at + -2 * WORD(AP) in the current frame of the exception + handler. */ arg = gen_rtx_MEM (word_mode, plus_constant (Pmode, arg_pointer_rtx, - -UNITS_PER_WORD)); + -2 * UNITS_PER_WORD)); } return arg; } @@ -12914,8 +12911,8 @@ ix86_compute_frame_layout (void) the registers need to be saved before allocating the frame. */ && flag_stack_check != STATIC_BUILTIN_STACK_CHECK); - /* Skip return address. */ - offset = UNITS_PER_WORD; + /* Skip return address and error code in exception handler. */ + offset = INCOMING_FRAME_SP_OFFSET; /* Skip pushed static chain. */ if (ix86_static_chain_on_stack) @@ -15220,8 +15217,9 @@ ix86_expand_epilogue (int style) m->fs.red_zone_offset = 0; if (ix86_using_red_zone () && crtl->args.pops_args < 65536) { - /* The red-zone begins below the return address. */ - m->fs.red_zone_offset = RED_ZONE_SIZE + UNITS_PER_WORD; + /* The red-zone begins below return address and error code in + exception handler. */ + m->fs.red_zone_offset = RED_ZONE_SIZE + INCOMING_FRAME_SP_OFFSET; /* When the register save area is in the aligned portion of the stack, determine the maximum runtime displacement that @@ -15516,18 +15514,7 @@ ix86_expand_epilogue (int style) } if (cfun->machine->func_type != TYPE_NORMAL) - { - /* Return with the "IRET" instruction from interrupt handler. - Pop the 'ERROR_CODE' off the stack before the 'IRET' - instruction in exception handler. */ - if (cfun->machine->func_type == TYPE_EXCEPTION) - { - rtx r = plus_constant (Pmode, stack_pointer_rtx, - UNITS_PER_WORD); - emit_insn (gen_rtx_SET (stack_pointer_rtx, r)); - } - emit_jump_insn (gen_interrupt_return ()); - } + emit_jump_insn (gen_interrupt_return ()); else if (crtl->args.pops_args && crtl->args.size) { rtx popc = GEN_INT (crtl->args.pops_args); diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h index 61501dc8c0b..bdea37b7313 100644 --- a/gcc/config/i386/i386.h +++ b/gcc/config/i386/i386.h @@ -2177,8 +2177,11 @@ extern int const svr4_dbx_register_map[FIRST_PSEUDO_REGISTER]; /* PC is dbx register 8; let's use that column for RA. */ #define DWARF_FRAME_RETURN_COLUMN (TARGET_64BIT ? 16 : 8) -/* Before the prologue, the top of the frame is at 4(%esp). */ -#define INCOMING_FRAME_SP_OFFSET UNITS_PER_WORD +/* Before the prologue, there are return address and error code for + exception handler on the top of the frame. */ +#define INCOMING_FRAME_SP_OFFSET \ + (cfun->machine->func_type == TYPE_EXCEPTION \ + ? 2 * UNITS_PER_WORD : UNITS_PER_WORD) /* Describe how we implement __builtin_eh_return. */ #define EH_RETURN_DATA_REGNO(N) ((N) <= DX_REG ? (N) : INVALID_REGNUM) diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 20ea9de7555..a6a378263c1 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,13 @@ +2017-07-30 H.J. Lu + + PR target/79793 + * gcc.dg/guality/pr68037-1.c: Update gdb breakpoints. + * gcc.target/i386/interrupt-5.c (interrupt_frame): New struct. + (foo): Check the builtin return address against the return address + in interrupt frame. + * gcc.target/i386/pr79793-1.c: New test. + * gcc.target/i386/pr79793-2.c: Likewise. + 2017-07-29 Eric Botcazou * c-c++-common/dump-ada-spec-7.c: New test. diff --git a/gcc/testsuite/gcc.dg/guality/pr68037-1.c b/gcc/testsuite/gcc.dg/guality/pr68037-1.c index 74f61ec5f96..44cab58659f 100644 --- a/gcc/testsuite/gcc.dg/guality/pr68037-1.c +++ b/gcc/testsuite/gcc.dg/guality/pr68037-1.c @@ -59,9 +59,9 @@ main () return 0; } -/* { dg-final { gdb-test 31 "error" "0x12345670" } } */ -/* { dg-final { gdb-test 31 "frame->ip" "0x12345671" } } */ -/* { dg-final { gdb-test 31 "frame->cs" "0x12345672" } } */ -/* { dg-final { gdb-test 31 "frame->flags" "0x12345673" } } */ -/* { dg-final { gdb-test 31 "frame->sp" "0x12345674" } } */ -/* { dg-final { gdb-test 31 "frame->ss" "0x12345675" } } */ +/* { dg-final { gdb-test 33 "error" "0x12345670" } } */ +/* { dg-final { gdb-test 33 "frame->ip" "0x12345671" } } */ +/* { dg-final { gdb-test 33 "frame->cs" "0x12345672" } } */ +/* { dg-final { gdb-test 33 "frame->flags" "0x12345673" } } */ +/* { dg-final { gdb-test 33 "frame->sp" "0x12345674" } } */ +/* { dg-final { gdb-test 33 "frame->ss" "0x12345675" } } */ diff --git a/gcc/testsuite/gcc.target/i386/interrupt-5.c b/gcc/testsuite/gcc.target/i386/interrupt-5.c index 803c0636299..5742b6f4743 100644 --- a/gcc/testsuite/gcc.target/i386/interrupt-5.c +++ b/gcc/testsuite/gcc.target/i386/interrupt-5.c @@ -7,12 +7,21 @@ extern void link_error (void); typedef unsigned int uword_t __attribute__ ((mode (__word__))); +struct interrupt_frame +{ + uword_t ip; + uword_t cs; + uword_t flags; + uword_t sp; + uword_t ss; +}; + __attribute__ ((used, interrupt)) void -foo (void *frame, uword_t error) +foo (struct interrupt_frame *frame, uword_t error) { void *ra = __builtin_return_address (0); - if ((uintptr_t) ra != (uintptr_t) error) + if ((uintptr_t) ra != (uintptr_t) frame->ip) link_error (); } diff --git a/gcc/testsuite/gcc.target/i386/pr79793-1.c b/gcc/testsuite/gcc.target/i386/pr79793-1.c new file mode 100644 index 00000000000..a382fe9c5e2 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr79793-1.c @@ -0,0 +1,14 @@ +/* { dg-do compile { target { *-*-linux* && { ! ia32 } } } } */ +/* { dg-options "-O2 -mgeneral-regs-only" } */ + +void + __attribute__ ((interrupt)) +fn1 (void *frame) +{ + char fxsave_region [512] __attribute__((aligned(16))); + __builtin_ia32_fxsave64 (fxsave_region); +} + +/* { dg-final { scan-assembler-times "sub\[lq\]\[\t \]*\\\$400,\[\t \]*%\[re\]sp" 1 } } */ +/* { dg-final { scan-assembler-times "fxsave64\[\t \]*-120\\(%\[re\]sp\\)" 1 } } */ +/* { dg-final { scan-assembler-times "add\[lq\]\[\t \]*\\\$400,\[\t \]*%\[re\]sp" 1 } } */ diff --git a/gcc/testsuite/gcc.target/i386/pr79793-2.c b/gcc/testsuite/gcc.target/i386/pr79793-2.c new file mode 100644 index 00000000000..f6ae5aed33a --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr79793-2.c @@ -0,0 +1,16 @@ +/* { dg-do compile { target { *-*-linux* && { ! ia32 } } } } */ +/* { dg-options "-O2 -mgeneral-regs-only" } */ + +typedef unsigned int uword_t __attribute__ ((mode (__word__))); + +void + __attribute__ ((interrupt)) +fn1 (void *frame, uword_t error) +{ + char fxsave_region [512] __attribute__((aligned(16))); + __builtin_ia32_fxsave64 (fxsave_region); +} + +/* { dg-final { scan-assembler-times "sub\[lq\]\[\t \]*\\\$392,\[\t \]*%\[re\]sp" 1 } } */ +/* { dg-final { scan-assembler-times "fxsave64\[\t \]*-120\\(%\[re\]sp\\)" 1 } } */ +/* { dg-final { scan-assembler-times "add\[lq\]\[\t \]*\\\$400,\[\t \]*%\[re\]sp" 1 } } */ -- 2.30.2