From patchwork Tue Aug 3 23:53:11 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Henderson X-Patchwork-Id: 60806 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id BAC4DB70A8 for ; Wed, 4 Aug 2010 09:54:52 +1000 (EST) Received: (qmail 10271 invoked by alias); 3 Aug 2010 23:53:44 -0000 Received: (qmail 9802 invoked by uid 22791); 3 Aug 2010 23:53:36 -0000 X-SWARE-Spam-Status: No, hits=-1.6 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE X-Spam-Check-By: sourceware.org Received: from a.mail.sonic.net (HELO a.mail.sonic.net) (64.142.16.245) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 03 Aug 2010 23:53:22 +0000 Received: from are.twiddle.net (are.twiddle.net [75.101.38.216]) by a.mail.sonic.net (8.13.8.Beta0-Sonic/8.13.7) with ESMTP id o73NrK6C006100 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 3 Aug 2010 16:53:20 -0700 Received: from are.twiddle.net (localhost [127.0.0.1]) by are.twiddle.net (8.14.4/8.14.4) with ESMTP id o73NrK18001149; Tue, 3 Aug 2010 16:53:20 -0700 Received: (from rth@localhost) by are.twiddle.net (8.14.4/8.14.4/Submit) id o73NrKOk001148; Tue, 3 Aug 2010 16:53:20 -0700 From: Richard Henderson To: gcc-patches@gcc.gnu.org Cc: kai.tietz@onevision.com, ubizjak@gmail.com Subject: [PATCH 4/9] Cleanup 32-bit ms_hook code. Date: Tue, 3 Aug 2010 16:53:11 -0700 Message-Id: <1280879596-1089-5-git-send-email-rth@twiddle.net> In-Reply-To: <1280879596-1089-1-git-send-email-rth@twiddle.net> References: <1280879596-1089-1-git-send-email-rth@twiddle.net> Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org 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. --- gcc/config/i386/i386.c | 121 ++++++++++++++++++++++++++++------------------- gcc/config/i386/i386.md | 11 ---- 2 files changed, 72 insertions(+), 60 deletions(-) * 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. diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index efdb6c4..a77014c 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 d7fd78d..0041158 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.