From patchwork Mon Nov 11 14:34:24 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ulrich Weigand X-Patchwork-Id: 290316 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.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 015FF2C0096 for ; Tue, 12 Nov 2013 01:37:22 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:subject:to:date:from:mime-version:content-type :content-transfer-encoding; q=dns; s=default; b=WCvVgPSjclYcFHl7 mn4gu2BoHCL3iClvW0NsamDV/vcqaH0cFYyouBC8q7GP6X1paZrEfxfM4uD0PoaI VVn9m8ijBSrmhY+FsV+s9pHMYKG0SPQgX1FNqPDrdzEGl5iybanJOdtXWIeJdwAp Be3GK+4Vc2bbzLCjRlGti4iwgXU= 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 :message-id:subject:to:date:from:mime-version:content-type :content-transfer-encoding; s=default; bh=VzWthulWbbAzFLoX5b6lWW 1TePc=; b=jxcaOIq9TyEfoYALoLpE7dBRsJcLZMVtyWpzUTdeUClDevheDWBoIG ErWMSMe9G6mf1SjO3r+KYHV3XE3btO6DJQtqXKuv5JX6S7DAs+vvaStgHY0Igzq/ tgrGOqs9JuCSVuKfenxqnSGqWseTRUIF6nPAVIa31QxXM6cb3GXe8= Received: (qmail 22677 invoked by alias); 11 Nov 2013 14:36:04 -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 22659 invoked by uid 89); 11 Nov 2013 14:36:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.0 required=5.0 tests=AWL, BAYES_50, MSGID_FROM_MTA_HEADER, RDNS_NONE, SPF_PASS, URIBL_BLOCKED autolearn=no version=3.3.2 X-HELO: e06smtp18.uk.ibm.com Received: from Unknown (HELO e06smtp18.uk.ibm.com) (195.75.94.114) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 11 Nov 2013 14:35:53 +0000 Received: from /spool/local by e06smtp18.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 11 Nov 2013 14:35:44 -0000 Received: from d06dlp01.portsmouth.uk.ibm.com (9.149.20.13) by e06smtp18.uk.ibm.com (192.168.101.148) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Mon, 11 Nov 2013 14:35:41 -0000 Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by d06dlp01.portsmouth.uk.ibm.com (Postfix) with ESMTP id C117D17D8059 for ; Mon, 11 Nov 2013 14:35:20 +0000 (GMT) Received: from d06av02.portsmouth.uk.ibm.com (d06av02.portsmouth.uk.ibm.com [9.149.37.228]) by b06cxnps4075.portsmouth.uk.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id rABEYDa259834492 for ; Mon, 11 Nov 2013 14:34:13 GMT Received: from d06av02.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id rABEYPpA020416 for ; Mon, 11 Nov 2013 07:34:25 -0700 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d06av02.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with SMTP id rABEYOeh020345 for ; Mon, 11 Nov 2013 07:34:24 -0700 Message-Id: <201311111434.rABEYOeh020345@d06av02.portsmouth.uk.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Mon, 11 Nov 2013 15:34:24 +0100 Subject: [PATCH, rs6000] Fix corner case in unwinding CR values To: gcc-patches@gcc.gnu.org Date: Mon, 11 Nov 2013 15:34:24 +0100 (CET) From: "Ulrich Weigand" MIME-Version: 1.0 X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 13111114-6892-0000-0000-000006DCF6B5 Hello, this patch fixes a corner case bug in unwinding CR values. The underlying problem is as follows: In order to save the CR in the prologue, two insns are necessary. The first is a mfcr that loads the CR value into a GPR, and the second is a store of the GPR to the stack. The back-end currently tags the first insn with a REG_FRAME_RELATED_EXPR, and marks both as RTX_FRAME_RELATED_P. This causes the middle-end to emit two CFI records, showing the CR saved first in the GPR, and later in its stack slot. (The first record is omitted if there is no possible exception in between.) Now, assume we use -fnon-call-exceptions, and get a signal on an instruction in between those two points, and the signal handler attempts to throw. The unwind logic will read CFI records and determine that in this frame, CR was saved into a GPR; and in the frame below (which must be a signal frame), that GPR was saved onto the stack. What the unwind logic then decides to do is to restore CR from the save slot of that latter GPR; i.e. the unwinder will copy that GPR save slot over the CR save slot of the unwind routine that calls __builtin_eh_return. Unfortunately, this is a problem on a 64-bit big-endian platform, since that GPR save slot is 8 bytes, while the CR save slot is just 4 bytes. The unwinder therefore copies the wrong half of the GPR save slot over the CR save slot ... As second, related, problem will come up with the new ABI. Here, we want to track each CR field in a separate CFI record, even though the values end up sharing the same stack slot. This is not a problem for a CFI record pointing to memory. However, the current dwarf2out.c middle-end logic is unable to handle the situation where multiple registers are saved in the same *register* at the same time ... Both problems can be fixed in the same way, by never emitting a CFI record showing CR saved into a GPR. This is easily done by simply marking only the store to memory as RTX_FRAME_RELATED_P. However, to avoid generating incorrect code we must then prevent and exception point to occur in between the two insns to save CR. The easiest way to do so is to mark the store to the stack slot as an additional user of all CR fields that need to be saved by the current function. This does restrict scheduling other instructions in between the prologue a bit more than otherwise. But since this affects only very special cases (e.g. an instruction that may triggers a floating- point exception, while -fnon-call-exceptions is active), this seems not too bad ... Note that in the epilogue, this is not really a problem, even though we also need two instructions to restore CR. We can (and do) simply leave the CFI record point to the stack slot all the time, which remains valid even after the CR value was read from there (and even after the stack pointer itself was modified). The patch below fixes this problem as described above, and adds a test case that fails without the patch. Tested on powerpc64-linux and powerpc64le-linux. OK for mainline? Bye, Ulrich ChangeLog: 2013-11-11 Ulrich Weigand * config/rs6000/rs6000.c (rs6000_emit_prologue): Do not place a RTX_FRAME_RELATED_P marker on the UNSPEC_MOVESI_FROM_CR insn. Instead, add USEs of all modified call-saved CR fields to the insn storing the result to the stack slot, and provide an appropriate REG_FRAME_RELATED_EXPR for that insn. * config/rs6000/rs6000.md ("*crsave"): New insn pattern. * config/rs6000/predicates.md ("crsave_operation"): New predicate. testsuite/ChangeLog: 2013-11-11 Ulrich Weigand * g++.dg/eh/ppc64-sighandle-cr.C: New test. Index: gcc/gcc/config/rs6000/rs6000.c =================================================================== --- gcc.orig/gcc/config/rs6000/rs6000.c +++ gcc/gcc/config/rs6000/rs6000.c @@ -21761,21 +21761,9 @@ rs6000_emit_prologue (void) && REGNO (frame_reg_rtx) != cr_save_regno && !(using_static_chain_p && cr_save_regno == 11)) { - rtx set; - cr_save_rtx = gen_rtx_REG (SImode, cr_save_regno); START_USE (cr_save_regno); - insn = emit_insn (gen_movesi_from_cr (cr_save_rtx)); - RTX_FRAME_RELATED_P (insn) = 1; - /* Now, there's no way that dwarf2out_frame_debug_expr is going - to understand '(unspec:SI [(reg:CC 68) ...] UNSPEC_MOVESI_FROM_CR)'. - But that's OK. All we have to do is specify that _one_ condition - code register is saved in this stack slot. The thrower's epilogue - will then restore all the call-saved registers. - We use CR2_REGNO (70) to be compatible with gcc-2.95 on Linux. */ - set = gen_rtx_SET (VOIDmode, cr_save_rtx, - gen_rtx_REG (SImode, CR2_REGNO)); - add_reg_note (insn, REG_FRAME_RELATED_EXPR, set); + emit_insn (gen_movesi_from_cr (cr_save_rtx)); } /* Do any required saving of fpr's. If only one or two to save, do @@ -22086,26 +22074,62 @@ rs6000_emit_prologue (void) rtx addr = gen_rtx_PLUS (Pmode, frame_reg_rtx, GEN_INT (info->cr_save_offset + frame_off)); rtx mem = gen_frame_mem (SImode, addr); - /* See the large comment above about why CR2_REGNO is used. */ - rtx magic_eh_cr_reg = gen_rtx_REG (SImode, CR2_REGNO); /* If we didn't copy cr before, do so now using r0. */ if (cr_save_rtx == NULL_RTX) { - rtx set; - START_USE (0); cr_save_rtx = gen_rtx_REG (SImode, 0); - insn = emit_insn (gen_movesi_from_cr (cr_save_rtx)); - RTX_FRAME_RELATED_P (insn) = 1; - set = gen_rtx_SET (VOIDmode, cr_save_rtx, magic_eh_cr_reg); - add_reg_note (insn, REG_FRAME_RELATED_EXPR, set); + emit_insn (gen_movesi_from_cr (cr_save_rtx)); } - insn = emit_move_insn (mem, cr_save_rtx); + + /* Saving CR requires a two-instruction sequence: one instruction + to move the CR to a general-purpose register, and a second + instruction that stores the GPR to memory. + + We do not emit any DWARF CFI records for the first of these, + because we cannot properly represent the fact that CR is saved in + a register. One reason is that we cannot express that multiple + CR fields are saved; another reason is that on 64-bit, the size + of the CR register in DWARF (4 bytes) differs from the size of + a general-purpose register. + + This means if any intervening instruction were to clobber one of + the call-saved CR fields, we'd have incorrect CFI. To prevent + this from happening, we mark the store to memory as a use of + those CR fields, which prevents any such instruction from being + scheduled in between the two instructions. */ + rtx crsave_v[9]; + int n_crsave = 0; + int i; + + crsave_v[n_crsave++] = gen_rtx_SET (VOIDmode, mem, cr_save_rtx); + for (i = 0; i < 8; i++) + if (save_reg_p (CR0_REGNO + i)) + crsave_v[n_crsave++] + = gen_rtx_USE (VOIDmode, gen_rtx_REG (CCmode, CR0_REGNO + i)); + + insn = emit_insn (gen_rtx_PARALLEL (VOIDmode, + gen_rtvec_v (n_crsave, crsave_v))); END_USE (REGNO (cr_save_rtx)); - rs6000_frame_related (insn, frame_reg_rtx, sp_off - frame_off, - NULL_RTX, NULL_RTX); + /* Now, there's no way that dwarf2out_frame_debug_expr is going to + understand '(unspec:SI [(reg:CC 68) ...] UNSPEC_MOVESI_FROM_CR)', + so we need to construct a frame expression manually. */ + RTX_FRAME_RELATED_P (insn) = 1; + + /* Update address to be stack-pointer relative, like + rs6000_frame_related would do. */ + addr = gen_rtx_PLUS (Pmode, gen_rtx_REG (Pmode, STACK_POINTER_REGNUM), + GEN_INT (info->cr_save_offset + sp_off)); + mem = gen_frame_mem (SImode, addr); + + /* We still cannot express that multiple CR fields are saved in the + CR save slot. By convention, we use a single CR regnum to represent + the fact that all call-saved CR fields are saved. We use CR2_REGNO + to be compatible with gcc-2.95 on Linux. */ + rtx set = gen_rtx_SET (VOIDmode, mem, gen_rtx_REG (SImode, CR2_REGNO)); + add_reg_note (insn, REG_FRAME_RELATED_EXPR, set); } /* Update stack and set back pointer unless this is V.4, Index: gcc/gcc/config/rs6000/rs6000.md =================================================================== --- gcc.orig/gcc/config/rs6000/rs6000.md +++ gcc/gcc/config/rs6000/rs6000.md @@ -15058,6 +15058,14 @@ "mfcr %0" [(set_attr "type" "mfcr")]) +(define_insn "*crsave" + [(match_parallel 0 "crsave_operation" + [(set (match_operand:SI 1 "memory_operand" "=m") + (match_operand:SI 2 "gpc_reg_operand" "r"))])] + "" + "stw %2,%1" + [(set_attr "type" "store")]) + (define_insn "*stmw" [(match_parallel 0 "stmw_operation" [(set (match_operand:SI 1 "memory_operand" "=m") Index: gcc/gcc/config/rs6000/predicates.md =================================================================== --- gcc.orig/gcc/config/rs6000/predicates.md +++ gcc/gcc/config/rs6000/predicates.md @@ -1538,6 +1538,26 @@ return 1; }) +;; Return 1 if OP is valid for crsave insn, known to be a PARALLEL. +(define_predicate "crsave_operation" + (match_code "parallel") +{ + int count = XVECLEN (op, 0); + int i; + + for (i = 1; i < count; i++) + { + rtx exp = XVECEXP (op, 0, i); + + if (GET_CODE (exp) != USE + || GET_CODE (XEXP (exp, 0)) != REG + || GET_MODE (XEXP (exp, 0)) != CCmode + || ! CR_REGNO_P (REGNO (XEXP (exp, 0)))) + return 0; + } + return 1; +}) + ;; Return 1 if OP is valid for lmw insn, known to be a PARALLEL. (define_predicate "lmw_operation" (match_code "parallel") Index: gcc/gcc/testsuite/g++.dg/eh/ppc64-sighandle-cr.C =================================================================== --- /dev/null +++ gcc/gcc/testsuite/g++.dg/eh/ppc64-sighandle-cr.C @@ -0,0 +1,54 @@ +// { dg-do run { target { powerpc64*-*-linux* } } } +// { dg-options "-fexceptions -fnon-call-exceptions" } + +#include +#include +#include + +#define SET_CR(R,V) __asm__ __volatile__ ("mtcrf %0,%1" : : "n" (1<<(7-R)), "r" (V<<(4*(7-R))) : "cr" #R) +#define GET_CR(R) ({ int tmp; __asm__ __volatile__ ("mfcr %0" : "=r" (tmp)); (tmp >> 4*(7-R)) & 15; }) + +void sighandler (int signo, siginfo_t * si, void * uc) +{ + SET_CR(2, 3); + SET_CR(3, 2); + SET_CR(4, 1); + + throw 0; +} + +float test (float a, float b) __attribute__ ((__noinline__)); +float test (float a, float b) +{ + float x; + asm ("mtcrf %1,%2" : "=f" (x) : "n" (1 << (7-3)), "r" (0), "0" (b) : "cr3"); + return a / x; +} + +int main () +{ + struct sigaction sa; + int status; + + sa.sa_sigaction = sighandler; + sa.sa_flags = SA_SIGINFO; + + status = sigaction (SIGFPE, & sa, NULL); + + feenableexcept (FE_DIVBYZERO); + + SET_CR(2, 6); + SET_CR(3, 9); + SET_CR(4, 12); + + try { + test (1, 0); + } + catch (...) { + return GET_CR(2) != 6 || GET_CR(3) != 9 || GET_CR(4) != 12; + } + + return 1; +} + +