From e8dc4658fa3910188ee5d7c66a801a14025df408 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Wed, 4 Aug 2010 16:58:08 -0700 Subject: [PATCH] Cleanup 32-bit ms_hook code. Emit the entire required hook code sequence via ASM_BYTE; emit unwind info onto a blockage insn. Remove the vswapmov pattern. Fix indentation in several places. From-SVN: r162886 --- gcc/ChangeLog | 10 ++++ gcc/config/i386/i386.c | 121 ++++++++++++++++++++++++---------------- gcc/config/i386/i386.md | 11 ---- 3 files changed, 82 insertions(+), 60 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index e08157f8a45..1d78cce9c5e 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,5 +1,15 @@ 2010-08-04 Richard Henderson + * config/i386/i386.c (ix86_function_ms_hook_prologue): Fix + argument name to reflect the expected tree; fix indentation. + (ix86_asm_output_function_label): Output the entire 32-bit + ms_hook here as bytes ... + (ix86_expand_prologue): ... not here as insns. Attach the + unwind info for the ms_hook to a blockage insn. + (ix86_handle_fndecl_attribute): Don't check HAVE_AS_IX86_SWAP. + (ix86_ms_bitfield_layout_p): Fix indentation. + * config/i386/i386.md (UNSPECV_VSWAPMOV, vswapmov): Remove. + * config/i386/i386.c (ix86_using_red_zone): New. (ix86_compute_frame_layout, ix86_add_cfa_restore_note, ix86_expand_prologue, ix86_force_to_memory): Use it. diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index efdb6c473d5..a77014c9557 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -5117,17 +5117,15 @@ ix86_function_type_abi (const_tree fntype) } static bool -ix86_function_ms_hook_prologue (const_tree fntype) +ix86_function_ms_hook_prologue (const_tree fn) { - if (fntype && lookup_attribute ("ms_hook_prologue", DECL_ATTRIBUTES (fntype))) + if (fn && lookup_attribute ("ms_hook_prologue", DECL_ATTRIBUTES (fn))) { - if (decl_function_context (fntype) != NULL_TREE) - { - error_at (DECL_SOURCE_LOCATION (fntype), - "ms_hook_prologue is not compatible with nested function"); - } - - return true; + if (decl_function_context (fn) != NULL_TREE) + error_at (DECL_SOURCE_LOCATION (fn), + "ms_hook_prologue is not compatible with nested function"); + else + return true; } return false; } @@ -5169,18 +5167,23 @@ ix86_asm_output_function_label (FILE *asm_out_file, const char *fname, ASM_OUTPUT_LABEL (asm_out_file, fname); - /* Output magic byte marker, if hot-patch attribute is set. - For x86 case frame-pointer prologue will be emitted in - expand_prologue. */ + /* Output magic byte marker, if hot-patch attribute is set. */ if (is_ms_hook) { if (TARGET_64BIT) - /* leaq [%rsp + 0], %rsp */ - asm_fprintf (asm_out_file, ASM_BYTE - "0x48, 0x8d, 0xa4, 0x24, 0x00, 0x00, 0x00, 0x00\n"); + { + /* leaq [%rsp + 0], %rsp */ + asm_fprintf (asm_out_file, ASM_BYTE + "0x48, 0x8d, 0xa4, 0x24, 0x00, 0x00, 0x00, 0x00\n"); + } else - /* movl.s %edi, %edi. */ - asm_fprintf (asm_out_file, ASM_BYTE "0x8b, 0xff\n"); + { + /* movl.s %edi, %edi + push %ebp + movl.s %esp, %ebp */ + asm_fprintf (asm_out_file, ASM_BYTE + "0x8b, 0xff, 0x55, 0x8b, 0xec\n"); + } } } @@ -9200,7 +9203,7 @@ ix86_expand_prologue (void) bool pic_reg_used; struct ix86_frame frame; HOST_WIDE_INT allocate; - int gen_frame_pointer = frame_pointer_needed; + bool gen_frame_pointer = frame_pointer_needed; bool int_registers_saved = false; ix86_finalize_stack_realign_flags (); @@ -9216,51 +9219,78 @@ ix86_expand_prologue (void) if (!TARGET_64BIT && ix86_function_ms_hook_prologue (current_function_decl)) { - rtx push, mov; + /* We should have already generated an error for any use of + ms_hook on a nested function. */ + gcc_checking_assert (!ix86_static_chain_on_stack); /* Check if profiling is active and we shall use profiling before prologue variant. If so sorry. */ if (crtl->profile && flag_fentry != 0) sorry ("ms_hook_prologue attribute isn't compatible with -mfentry for 32-bit"); - /* Make sure the function starts with - 8b ff movl.s %edi,%edi (emited by ix86_asm_output_function_label) + /* In ix86_asm_output_function_label we emitted: + 8b ff movl.s %edi,%edi 55 push %ebp 8b ec movl.s %esp,%ebp This matches the hookable function prologue in Win32 API functions in Microsoft Windows XP Service Pack 2 and newer. Wine uses this to enable Windows apps to hook the Win32 API - functions provided by Wine. */ - push = emit_insn (gen_push (hard_frame_pointer_rtx)); - mov = emit_insn (gen_vswapmov (hard_frame_pointer_rtx, - stack_pointer_rtx)); + functions provided by Wine. - if (frame_pointer_needed && !(crtl->drap_reg - && crtl->stack_realign_needed)) + What that means is that we've already set up the frame pointer. */ + + if (frame_pointer_needed + && !(crtl->drap_reg && crtl->stack_realign_needed)) { - /* The push %ebp and movl.s %esp, %ebp already set up - the frame pointer. No need to do this again. */ - gen_frame_pointer = 0; + rtx push, mov; + + /* We've decided to use the frame pointer already set up. + Describe this to the unwinder by pretending that both + push and mov insns happen right here. + + Putting the unwind info here at the end of the ms_hook + is done so that we can make absolutely certain we get + the required byte sequence at the start of the function, + rather than relying on an assembler that can produce + the exact encoding required. + + However it does mean (in the unpatched case) that we have + a 1 insn window where the asynchronous unwind info is + incorrect. However, if we placed the unwind info at + its correct location we would have incorrect unwind info + in the patched case. Which is probably all moot since + I don't expect Wine generates dwarf2 unwind info for the + system libraries that use this feature. */ + + insn = emit_insn (gen_blockage ()); + + push = gen_push (hard_frame_pointer_rtx); + mov = gen_rtx_SET (VOIDmode, hard_frame_pointer_rtx, + stack_pointer_rtx); RTX_FRAME_RELATED_P (push) = 1; RTX_FRAME_RELATED_P (mov) = 1; + + RTX_FRAME_RELATED_P (insn) = 1; + add_reg_note (insn, REG_FRAME_RELATED_EXPR, + gen_rtx_PARALLEL (VOIDmode, gen_rtvec (2, push, mov))); + if (ix86_cfa_state->reg == stack_pointer_rtx) ix86_cfa_state->reg = hard_frame_pointer_rtx; + gen_frame_pointer = false; } else - /* If the frame pointer is not needed, pop %ebp again. This - could be optimized for cases where ebp needs to be backed up - for some other reason. If stack realignment is needed, pop - the base pointer again, align the stack, and later regenerate - the frame pointer setup. The frame pointer generated by the - hook prologue is not aligned, so it can't be used. */ - insn = emit_insn (ix86_gen_pop1 (hard_frame_pointer_rtx)); + { + /* The frame pointer is not needed so pop %ebp again. + This leaves us with a pristine state. */ + emit_insn (ix86_gen_pop1 (hard_frame_pointer_rtx)); + } } /* The first insn of a function that accepts its static chain on the stack is to push the register that would be filled in by a direct call. This insn will be skipped by the trampoline. */ - if (ix86_static_chain_on_stack) + else if (ix86_static_chain_on_stack) { rtx t; @@ -27010,23 +27040,16 @@ ix86_handle_fndecl_attribute (tree *node, tree name, warning (OPT_Wattributes, "%qE attribute only applies to functions", name); *no_add_attrs = true; - return NULL_TREE; } - -#ifndef HAVE_AS_IX86_SWAP - if (!TARGET_64BIT) - sorry ("ms_hook_prologue attribute needs assembler swap suffix support"); -#endif - - return NULL_TREE; + return NULL_TREE; } static bool ix86_ms_bitfield_layout_p (const_tree record_type) { - return (TARGET_MS_BITFIELD_LAYOUT && - !lookup_attribute ("gcc_struct", TYPE_ATTRIBUTES (record_type))) - || lookup_attribute ("ms_struct", TYPE_ATTRIBUTES (record_type)); + return ((TARGET_MS_BITFIELD_LAYOUT + && !lookup_attribute ("gcc_struct", TYPE_ATTRIBUTES (record_type))) + || lookup_attribute ("ms_struct", TYPE_ATTRIBUTES (record_type))); } /* Returns an expression indicating where the this parameter is diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index d7fd78dc61e..00411583a6d 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -250,7 +250,6 @@ UNSPECV_RDTSC UNSPECV_RDTSCP UNSPECV_RDPMC - UNSPECV_VSWAPMOV UNSPECV_LLWP_INTRINSIC UNSPECV_SLWP_INTRINSIC UNSPECV_LWPVAL_INTRINSIC @@ -11573,16 +11572,6 @@ (set_attr "length_immediate" "0") (set_attr "modrm" "0")]) -(define_insn "vswapmov" - [(set (match_operand:SI 0 "register_operand" "=r") - (match_operand:SI 1 "register_operand" "r")) - (unspec_volatile [(const_int 0)] UNSPECV_VSWAPMOV)] - "" - "movl.s\t{%1, %0|%0, %1}" - [(set_attr "length" "2") - (set_attr "length_immediate" "0") - (set_attr "modrm" "0")]) - ;; Pad to 16-byte boundary, max skip in op0. Used to avoid ;; branch prediction penalty for the third jump in a 16-byte ;; block on K8. -- 2.30.2