From patchwork Thu Jul 18 00:23:17 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Iain Sandoe X-Patchwork-Id: 259968 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]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 2420C2C0095 for ; Thu, 18 Jul 2013 10:23:40 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :content-type:subject:date:message-id:cc:to:mime-version; q=dns; s=default; b=PThK+iRP4xamwVCxvdmfDSoX6mG1a1/PppZp+DeWFsvQh2zGmu 5dYB5XHo7/DDw0o/VdFurrwDMmhNNCli8EAtjdlLGvQgfBxyBlaAYGkZlNGm5fMX g93AvC9yY3Ck45WdrPyN9ZvM307HV7KIUz6ek5TcpvF/Csj3z3A3rYCgU= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :content-type:subject:date:message-id:cc:to:mime-version; s= default; bh=BybnbgupSJwUFo15OzItR22T55o=; b=x6TtlkDuVWa4JpS03Qdp HldMg2Hi9ZaHUKSnRrarsDY9dll4pYBCF31FBMPURv2RVynDHSX/5Mf4AdK++43Q /Q1Lq/LV90bRVy/fZUlN2bD0xMzgmOnL4nYbIMDVas+OS/HRdpUBRnlhEFdGbKEd W0ctKq+4MGxMpsd9cHIshdU= Received: (qmail 18028 invoked by alias); 18 Jul 2013 00:23:33 -0000 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 Received: (qmail 18008 invoked by uid 89); 18 Jul 2013 00:23:32 -0000 X-Spam-SWARE-Status: No, score=0.5 required=5.0 tests=AWL, BAYES_50, KAM_STOCKTIP, KHOP_RCVD_UNTRUST, RCVD_IN_HOSTKARMA_W, RCVD_IN_HOSTKARMA_WL, RDNS_NONE autolearn=no version=3.3.1 Received: from Unknown (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Thu, 18 Jul 2013 00:23:30 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1Uzc0A-0007mo-Bx from Iain_Sandoe@mentor.com ; Wed, 17 Jul 2013 17:23:22 -0700 Received: from SVR-IES-FEM-02.mgc.mentorg.com ([137.202.0.106]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Wed, 17 Jul 2013 17:23:22 -0700 Received: from [127.0.0.1] (137.202.0.76) by SVR-IES-FEM-02.mgc.mentorg.com (137.202.0.106) with Microsoft SMTP Server id 14.2.247.3; Thu, 18 Jul 2013 01:23:20 +0100 From: Iain Sandoe Subject: [Patch x86/darwin] fix PR51784 'PIC register not correctly preserved...' Date: Thu, 18 Jul 2013 01:23:17 +0100 Message-ID: <1F77EC2A-536E-462E-B855-7B8818FC8D90@codesourcery.com> CC: Mike Stump , Richard Henderson To: GCC Patches MIME-Version: 1.0 (Apple Message framework v1085) X-Virus-Found: No Hi, The PR is logged against Darwin, and (as Jakub points out in the PR thread) indeed Darwin is missing a nonlocal_goto_receiver to restore the PIC reg in code that uses it (most of the patch). However, there is a second issue, and (if I've understood things correctly) this affects GOT targets too - thus there is a single non-darwin-specific hunk for which I need approval for X86 as a whole. consider (x86 -fPIC -m32) == int g42 = 42; int foo (void) <=== doesn't use EBX, so doesn't save it. { __label__ x; int z; int bar (int *zz) <== does use EBX, and saves it { *zz = g42; goto x; <== however, EBX is not restored here. } bar(&z); x: return z; } == ... this all works OK when the caller of foo and foo are in one object (and thus share the same GOT) .. however, suppose we build the code above as a shared lib and call it from a pie executable (or another so). Now, when the caller (with a different GOT value from the lib) calls foo() - EBX gets trashed (probably *boom*). The solution proposed here (for this aspect) is that, if a function contains a nonlocal label, then the PICbase register should be preserved. This is the only non-darwin-specific hunk in the patch. [For the Darwin case, it is always necessary to preserve and restore the PIC base, since a different base is used in each function]. The remainder of the patch is darwin-specific, to provide restoration of the pic register at non-local-goto-recievers. == I have verified that the patch works as expected on x86_64-linux (@m32), i686-darwin9 and x86_64-darwin12 (@m32) - and that, otherwise, there are no test changes (Ada and Java tested on Darwin, but not on Linux). (Other Darwin folks {thanks!} have tested this across a wider range of Darwin versions) == OK for trunk? OK open branches? (since this is a wrong code bug) thanks, Iain gcc/ PR target/51784 * config/i386/i386.c (output_set_got, DARWIN): Adjust to emit a second label for nonlocal goto receivers. Don't output pic base labels unless we're producing PIC; mark that action unreachable(). (ix86_save_reg): If the function contains a nonlocal label, save the PIC base reg. * config/darwin-protos.h (machopic_should_output_picbase_label): New. * gcc/config/darwin.c (emitted_pic_label_num): New GTY. (update_pic_label_number_if_needed): New. (machopic_output_function_base_name): Adjust for nonlocal receiver case. (machopic_should_output_picbase_label): New. * config/i386/i386.md (enum unspecv): UNSPECV_NLGR: New. (nonlocal_goto_receiver): New insn and split. gcc/config/darwin-protos.h | 1 + gcc/config/darwin.c | 30 +++++++++++++++++++++++++----- gcc/config/i386/i386.c | 23 ++++++++++++++--------- gcc/config/i386/i386.md | 34 +++++++++++++++++++++++++++++++++- 4 files changed, 73 insertions(+), 15 deletions(-) diff --git a/gcc/config/darwin-protos.h b/gcc/config/darwin-protos.h index 0755e94..70b7fb0 100644 --- a/gcc/config/darwin-protos.h +++ b/gcc/config/darwin-protos.h @@ -25,6 +25,7 @@ extern void machopic_validate_stub_or_non_lazy_ptr (const char *); extern void machopic_output_function_base_name (FILE *); extern const char *machopic_indirection_name (rtx, bool); extern const char *machopic_mcount_stub_name (void); +extern bool machopic_should_output_picbase_label (void); #ifdef RTX_CODE diff --git a/gcc/config/darwin.c b/gcc/config/darwin.c index a049a5d..7e63fdf 100644 --- a/gcc/config/darwin.c +++ b/gcc/config/darwin.c @@ -369,14 +369,13 @@ machopic_gen_offset (rtx orig) static GTY(()) const char * function_base_func_name; static GTY(()) int current_pic_label_num; +static GTY(()) int emitted_pic_label_num; -void -machopic_output_function_base_name (FILE *file) +static void +update_pic_label_number_if_needed (void) { const char *current_name; - /* If dynamic-no-pic is on, we should not get here. */ - gcc_assert (!MACHO_DYNAMIC_NO_PIC_P); /* When we are generating _get_pc thunks within stubs, there is no current function. */ if (current_function_decl) @@ -394,7 +393,28 @@ machopic_output_function_base_name (FILE *file) ++current_pic_label_num; function_base_func_name = "L_machopic_stub_dummy"; } - fprintf (file, "L%011d$pb", current_pic_label_num); +} + +void +machopic_output_function_base_name (FILE *file) +{ + /* If dynamic-no-pic is on, we should not get here. */ + gcc_assert (!MACHO_DYNAMIC_NO_PIC_P); + + update_pic_label_number_if_needed (); + fprintf (file, "L%d$pb", current_pic_label_num); +} + +bool +machopic_should_output_picbase_label (void) +{ + update_pic_label_number_if_needed (); + + if (current_pic_label_num == emitted_pic_label_num) + return false; + + emitted_pic_label_num = current_pic_label_num; + return true; } /* The suffix attached to non-lazy pointer symbols. */ diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 5df6ab7..f523c2a 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -8827,10 +8827,8 @@ output_set_got (rtx dest, rtx label ATTRIBUTE_UNUSED) output_asm_insn ("mov%z0\t{%2, %0|%0, %2}", xops); #if TARGET_MACHO - /* Output the Mach-O "canonical" label name ("Lxx$pb") here too. This - is what will be referenced by the Mach-O PIC subsystem. */ - if (!label) - ASM_OUTPUT_LABEL (asm_out_file, MACHOPIC_FUNCTION_BASE_NAME); + /* We don't need a pic base, we're not producing pic. */ + gcc_unreachable (); #endif targetm.asm_out.internal_label (asm_out_file, "L", @@ -8845,12 +8843,18 @@ output_set_got (rtx dest, rtx label ATTRIBUTE_UNUSED) xops[2] = gen_rtx_SYMBOL_REF (Pmode, ggc_strdup (name)); xops[2] = gen_rtx_MEM (QImode, xops[2]); output_asm_insn ("call\t%X2", xops); - /* Output the Mach-O "canonical" label name ("Lxx$pb") here too. This - is what will be referenced by the Mach-O PIC subsystem. */ + #if TARGET_MACHO - if (!label) + /* Output the Mach-O "canonical" pic base label name ("Lxx$pb") here. + This is what will be referenced by the Mach-O PIC subsystem. */ + if (machopic_should_output_picbase_label () || !label) ASM_OUTPUT_LABEL (asm_out_file, MACHOPIC_FUNCTION_BASE_NAME); - else + + /* When we are restoring the pic base at the site of a nonlocal label, + and we decided to emit the pic base above, we will still output a + local label used for calculating the correction offset (even though + the offset will be 0 in that case). */ + if (label) targetm.asm_out.internal_label (asm_out_file, "L", CODE_LABEL_NUMBER (label)); #endif @@ -8932,7 +8936,8 @@ ix86_save_reg (unsigned int regno, bool maybe_eh_return) && (df_regs_ever_live_p (REAL_PIC_OFFSET_TABLE_REGNUM) || crtl->profile || crtl->calls_eh_return - || crtl->uses_const_pool)) + || crtl->uses_const_pool + || cfun->has_nonlocal_label)) return ix86_select_alt_pic_regnum () == INVALID_REGNUM; if (crtl->calls_eh_return && maybe_eh_return) diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index 2777e9c..df9faf2 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -222,6 +222,8 @@ UNSPECV_XEND UNSPECV_XABORT UNSPECV_XTEST + + UNSPECV_NLGR ]) ;; Constants to represent rounding modes in the ROUND instruction @@ -16227,7 +16229,37 @@ emit_insn (gen_set_got (pic_offset_table_rtx)); DONE; }) - + +(define_insn_and_split "nonlocal_goto_receiver" + [(unspec_volatile [(const_int 0)] UNSPECV_NLGR)] + "TARGET_MACHO && !TARGET_64BIT && flag_pic" +{ + if (crtl->uses_pic_offset_table) + return "#"; + else + return "# not used"; /* just for debug. */ +} + "" + [(const_int 0)] +{ + if (crtl->uses_pic_offset_table) + { + rtx xops[3]; + rtx label_rtx = gen_label_rtx (); + rtx tmp; + + /* Get a new pic base. */ + emit_insn (gen_set_got_labelled (pic_offset_table_rtx, label_rtx)); + /* Correct this with the offset from the new to the old. */ + xops[0] = xops[1] = pic_offset_table_rtx; + label_rtx = gen_rtx_LABEL_REF (SImode, label_rtx); + tmp = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, label_rtx), UNSPEC_MACHOPIC_OFFSET); + xops[2] = gen_rtx_CONST (Pmode, tmp); + ix86_expand_binary_operator (MINUS, SImode, xops); + } + DONE; +}) + ;; Avoid redundant prefixes by splitting HImode arithmetic to SImode. (define_split