The debug_info section for the interrupt function looks ok. I tried to call it from assembler code to check it in gdb.
pushl $0x333 ;eflags
pushl $0x111 ;cs
pushl $0x222 ;eip
jmp foo ;interrupt function
#define uword_t unsigned int
struct interrupt_frame
{
uword_t ip;
uword_t cs;
uword_t flags;
};
I get this inside the interrupt function:
Breakpoint 1, foo (frame=0xffffd560) at interrupt-1.c:7
7 a = (struct interrupt_frame*) frame;
(gdb) p/x ((struct interrupt_frame*)frame)->ip
$1 = 0x222
(gdb) p/x ((struct interrupt_frame*)frame)->cs
$3 = 0x111
(gdb) p/x ((struct interrupt_frame*)frame)->flags
$4 = 0x333
Frame pointer info looks ok.
On Tue, Oct 13, 2015 at 3:18 PM, Yulia Koval <vaalfr...@gmail.com> wrote:
>
> Here is the current version of the patch with all the fixes.
> Regtested\bootstraped it on 64 bit.
>
> We need a pointer since interrupt handler will update data pointing
> to by frame. Since error_code isn't at the normal location where the
> parameter is passed on stack and frame isn't in a hard register, we
> changed ix86_function_arg:
>
> + if (!cum->caller && cfun->machine->func_type != TYPE_NORMAL)
> + {
> + /* The first argument of interrupt handler is a pointer and
> + points to the return address slot on stack. The optional
> + second argument is an integer for error code on stack. */
> + gcc_assert (type != NULL_TREE);
> + if (POINTER_TYPE_P (type))
> + {
> + if (cfun->machine->func_type == TYPE_EXCEPTION)
> + /* (AP) in the current frame in exception handler. */
> + arg = arg_pointer_rtx;
> + else
> + /* -WORD(AP) in the current frame in interrupt handler. */
> + arg = force_reg (Pmode,
> + plus_constant (Pmode, arg_pointer_rtx,
> + -UNITS_PER_WORD));
> + if (mode != Pmode)
> + arg = convert_to_mode (mode, arg, 1);
> + }
> + else
> + {
> + gcc_assert (TREE_CODE (type) == INTEGER_TYPE
> + && cfun->machine->func_type == TYPE_EXCEPTION
> + && mode == word_mode);
> + /* The error code is at -WORD(AP) in the current frame in
> + exception handler. */
> + arg = gen_rtx_MEM (word_mode,
> + plus_constant (Pmode, arg_pointer_rtx,
> + -UNITS_PER_WORD));
> + }
> +
> + return arg;
> + }
> +
>
> to return a pseudo register. It violates
>
> Return where to put the arguments to a function.
> Return zero to push the argument on the stack, or a hard register in
> which to store the argument.
>
> Register allocator has no problem with parameters in pseudo registers.
> But GCC crashes when it tries to access DECL_INCOMING_RTL as a hard
> register when generating debug information. We worked around it by
> doing
>
> +
> + if (cfun->machine->func_type != TYPE_NORMAL)
> + {
> + /* Since the pointer argument of interrupt handler isn't a real
> + argument, adjust DECL_INCOMING_RTL for debug output. */
> + tree arg = DECL_ARGUMENTS (current_function_decl);
> + gcc_assert (arg != NULL_TREE
> + && POINTER_TYPE_P (TREE_TYPE (arg)));
> + if (cfun->machine->func_type == TYPE_EXCEPTION)
> + /* (AP) in the current frame in exception handler. */
> + DECL_INCOMING_RTL (arg) = arg_pointer_rtx;
> + else
> + /* -WORD(AP) in the current frame in interrupt handler. */
> + DECL_INCOMING_RTL (arg) = plus_constant (Pmode,
> + arg_pointer_rtx,
> + -UNITS_PER_WORD);
> + }
>
>
> On Mon, Oct 5, 2015 at 12:29 PM, Uros Bizjak <ubiz...@gmail.com> wrote:
> > On Mon, Oct 5, 2015 at 1:17 AM, H.J. Lu <hjl.to...@gmail.com> wrote:
> >
> >>>>>>> Looking a bit deeper into the code, it looks that we want to realign
> >>>>>>> the stack in the interrupt handler. Let's assume that interrupt
> >>>>>>> handler is calling some other function that saves SSE vector regs to
> >>>>>>> the stack. According to the x86 ABI, incoming stack of the called
> >>>>>>> function is assumed to be aligned to 16 bytes. But, interrupt handler
> >>>>>>> violates this assumption, since the stack could be aligned to only 4
> >>>>>>> bytes for 32bit and 8 bytes for 64bit targets. Entering the called
> >>>>>>> function with stack, aligned to less than 16 bytes will certainly
> >>>>>>> violate ABI.
> >>>>>>>
> >>>>>>> So, it looks to me that we need to realign the stack in the interrupt
> >>>>>>> handler unconditionally to 16bytes. In this case, we also won't need
> >>>>>>> the following changes:
> >>>>>>>
> >>>>>>
> >>>>>> Current stack alignment implementation requires at least
> >>>>>> one, maybe two, scratch registers:
> >>>>>>
> >>>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67841
> >>>>>>
> >>>>>> Extend it to the interrupt handler, which doesn't have any scratch
> >>>>>> registers may require significant changes in backend as well as
> >>>>>> register allocator.
> >>>>>
> >>>>> But without realignment, the handler is unusable for anything but
> >>>>> simple functions. The handler will crash when called function will try
> >>>>> to save vector reg to stack.
> >>>>>
> >>>>
> >>>> We can use unaligned load and store to avoid crash.
> >>>
> >>> Oh, sorry, I meant "called function will crash", like:
> >>>
> >>> -> interrupt when %rsp = 0x...8 ->
> >>> -> interrupt handler ->
> >>> -> calls some function that tries to save xmm reg to stack
> >>> -> crash in the called function
> >>>
> >>
> >> It should be fixed by this patch. But we need to fix stack
> >> alignment in interrupt handler to avoid scratch register.
> >>
> >>
> >> --
> >> H.J.
> >> ---
> >> commit 15f48be1dc7ff48207927d0b835e593d058f695b
> >> Author: H.J. Lu <hjl.to...@gmail.com>
> >> Date: Sun Oct 4 16:14:03 2015 -0700
> >>
> >> Correctly set incoming stack boundary for interrupt handler
> >>
> >> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> >> index 7ebdcd9..0f0cc3c 100644
> >> --- a/gcc/config/i386/i386.c
> >> +++ b/gcc/config/i386/i386.c
> >> @@ -12037,8 +12037,11 @@ ix86_minimum_incoming_stack_boundary (bool
> >> sibcall)
> >> {
> >> unsigned int incoming_stack_boundary;
> >>
> >> + /* Stack of interrupt handler is always aligned to word_mode. */
> >> + if (cfun->machine->func_type != TYPE_NORMAL)
> >> + incoming_stack_boundary = TARGET_64BIT ? 64 : 32;
> >
> > Just a heads up that in order to support stack realignmnent on x86_64,
> > MIN_STACK_BOUNDARY will soon be changed to BITS_PER_WORD, so you can
> > use it in the line above. Please see comment #5 and #6 of PR 66697
> > [1].
> >
> >> /* Prefer the one specified at command line. */
> >> - if (ix86_user_incoming_stack_boundary)
> >> + else if (ix86_user_incoming_stack_boundary)
> >> incoming_stack_boundary = ix86_user_incoming_stack_boundary;
> >> /* In 32bit, use MIN_STACK_BOUNDARY for incoming stack boundary
> >> if -mstackrealign is used, it isn't used for sibcall check and
> >
> > [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66697
> >
> > Uros.
patch_repost
Description: Binary data
