From patchwork Tue Nov 6 23:04:30 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Lance Taylor X-Patchwork-Id: 197572 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 061E92C00F8 for ; Wed, 7 Nov 2012 10:04:48 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1352847890; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:From:To:Subject:Date:Message-ID:User-Agent:MIME-Version: Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:Sender:Delivered-To; bh=8Usyx+x O5ruz7Ht0iepuWb6/fl4=; b=MmWKBxREMzU6s9b6ucAvA8S4ylbHrSMn62vZgWS 1L3o21QB0t812+Ykr7n7b3o/zMHyBCJ51jjAjFuikPqI5ycBpcKrBTTHo5js3yW7 u/pDwds1rDMTFpVFDhbXMQj0wqe2q7wsuoJ1iIjsCMQuTd9rFf6T/Q7lQR2T1QYg hk4g= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:X-Google-DKIM-Signature:Received:Received:From:To:Subject:Date:Message-ID:User-Agent:MIME-Version:Content-Type:X-Gm-Message-State:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=E90rFecvdamLkQfyq6++tkvjE4+7mgcBMIWmuvq55qtHf74ElvObXK6PYCuWiD D0a+WdDbPan8J+7mDptEfUigiEJOv0c2EjG0nC+pO5wM/BAluHgjDoZKOsS5UpRV GVQk/iSLBvUPxtw9JXPla1s+SsJwZ7nQI3y5fohxJN/5g=; Received: (qmail 20153 invoked by alias); 6 Nov 2012 23:04:41 -0000 Received: (qmail 20137 invoked by uid 22791); 6 Nov 2012 23:04:40 -0000 X-SWARE-Spam-Status: No, hits=-4.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, KHOP_RCVD_TRUST, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_YE, RP_MATCHES_RCVD, TW_CP, TW_DD, TW_OV, T_TVD_MIME_NO_HEADERS X-Spam-Check-By: sourceware.org Received: from mail-da0-f47.google.com (HELO mail-da0-f47.google.com) (209.85.210.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 06 Nov 2012 23:04:33 +0000 Received: by mail-da0-f47.google.com with SMTP id s35so381534dak.20 for ; Tue, 06 Nov 2012 15:04:32 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:subject:date:message-id:user-agent:mime-version :content-type:x-gm-message-state; bh=9fMLvSVS4VVf6cR7y7PIx/tvyCDbI9G5hyooYMOQuDU=; b=IILOgqZrsJ7cOmMpA8z3/yKPL5HUoXVS/J+tDFpNVD4RsY67NoXfrYZMjEhaW/EXn4 npIDoxywwMDwH26raspiKWTq+uOdVDx8jl5kWo4NsItGKHGs9yCKucxgZMqZsgNZwi0Q T9jlEzCuaclFgV7pHdXHHT4zxG5yBOOw5gFJQISaCh8QjuVYCxQy7KJRnUVo0KYwcFfL Hr4INuCdG3DDhWqh6MYaRx0IwqvQIFlBqX6asixACO98M2a5a329e3hqaU/zIj57puEm owVPKwk9Oe8Irw8EDRV9lFsOnYiq02efVXIa0UjnFR9S/WJAPYBzwmtbx84WYOyIIm2c RQew== Received: by 10.68.129.227 with SMTP id nz3mr7780673pbb.111.1352243072122; Tue, 06 Nov 2012 15:04:32 -0800 (PST) Received: from coign.google.com ([2620:0:1000:1804:224:d7ff:fe8f:f634]) by mx.google.com with ESMTPS id t1sm13138684paw.11.2012.11.06.15.04.31 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 06 Nov 2012 15:04:31 -0800 (PST) From: Ian Lance Taylor To: gcc-patches@gcc.gnu.org Subject: libgcc patch committed: Fix split-stack stack alignment Date: Tue, 06 Nov 2012 15:04:30 -0800 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 X-Gm-Message-State: ALoCoQlp1DBJGY0xtdmzSj0QrjBM08CahnJrqJ5TKC2lp5Ryv1CWPvvO+A+xCuWa36Rej1eNwQThGTI77ICA3LtnPiTwDK7rDSacGJkzi85dVjOW6rY1JekP2v2i+HjttJuQThheX/iH3LCuZPbpIvXxBYP7TvbkeACR93RtW5faQHioijeY+9eUrfz1lMdrMwJPKBMjr99ApWbaTkjAUZ2m87/dDPOkkg== X-IsSubscribed: yes 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 This patch fixes two stack alignment bugs in the split-stack support. The first is that I managed to miscalculate the stack alignment in the assembly code. I was treating that code as though it were a normal function. That is wrong, because __morestack is actually called without any stack adjustment in the caller, which means that when __morestack is entered the stack % 16 will == 8 in 32-bit mode and 0 in 64-bit mode, unlike the usual case of 12 and 8, respectively. This patch corrects the code to align the stack correctly when calling the C split-stack functions. Previously the stack was misaligned when calling those functions. Since those functions don't happen to use any vector registers, this did not matter except for performance. In any case, this patch fixes it. The second bug is that the C functions were not aligning the returned stack. The result was coming back to an alignment determined by the parameter size. This is simply wrong. Since the C code doesn't know the required stack alignment, I simply made it always align to a 32-byte boundary. If split stack support is added for more processors, this may need to become processor dependent. Along the way I noticed that the 32-bit __morestack_non_split support was mishandling the return address when called by a varargs function, and I fixed that too. Bootstrapped and ran split-stack and Go tests on x86_64-unknown-linux-gnu, both 64-bit and 32-bit mode. Committed to mainline. Ian 2012-11-06 Ian Lance Taylor * generic-morestack.c (__generic_morestack): Align the returned stack pointer to a 32 byte boundary. * config/i386/morestack.S (__morestack_non_split) [32-bit]: Don't increment the return address until we have decided that we don't have a varargs function. (__morestack) [32-bit]: Align stack correctly when calling C functions. (__morestack) [64-bit]: Likewise. Index: generic-morestack.c =================================================================== --- generic-morestack.c (revision 193263) +++ generic-morestack.c (working copy) @@ -549,6 +549,7 @@ __generic_morestack (size_t *pframe_size char *to; void *ret; size_t i; + size_t aligned; current = __morestack_current_segment; @@ -580,15 +581,19 @@ __generic_morestack (size_t *pframe_size *pframe_size = current->size - param_size; + /* Align the returned stack to a 32-byte boundary. */ + aligned = (param_size + 31) & ~ (size_t) 31; + #ifdef STACK_GROWS_DOWNWARD { char *bottom = (char *) (current + 1) + current->size; - to = bottom - param_size; - ret = bottom - param_size; + to = bottom - aligned; + ret = bottom - aligned; } #else to = current + 1; - ret = (char *) (current + 1) + param_size; + to += aligned - param_size; + ret = (char *) (current + 1) + aligned; #endif /* We don't call memcpy to avoid worrying about the dynamic linker Index: config/i386/morestack.S =================================================================== --- config/i386/morestack.S (revision 193263) +++ config/i386/morestack.S (working copy) @@ -200,18 +200,19 @@ __morestack_non_split: jb 2f # Get more space if we need it. - # This breaks call/return prediction, as described above. - incq 8(%rsp) # Increment the return address. - # If the instruction that we return to is # leaq 24(%rbp), %r11n # then we have been called by a varargs function that expects # %ebp to hold a real value. That can only work if we do the # full stack split routine. FIXME: This is fragile. movq 8(%rsp),%rax + incq %rax # Skip ret instruction in caller. cmpl $0x185d8d4c,(%rax) je 2f + # This breaks call/return prediction, as described above. + incq 8(%rsp) # Increment the return address. + popq %rax # Restore register. .cfi_adjust_cfa_offset -8 # Adjust for popped register. @@ -296,9 +297,13 @@ __morestack: # argument size is pushed then the new stack frame size is # pushed. - # Align stack to 16-byte boundary with enough space for saving - # registers and passing parameters to functions we call. - subl $40,%esp + # In the body of a non-leaf function, the stack pointer will + # be aligned to a 16-byte boundary. That is CFA + 12 in the + # stack picture above: (CFA + 12) % 16 == 0. At this point we + # have %esp == CFA - 8, so %esp % 16 == 12. We need some + # space for saving registers and passing parameters, and we + # need to wind up with %esp % 16 == 0. + subl $44,%esp # Because our cleanup code may need to clobber %ebx, we need # to save it here so the unwinder can restore the value used @@ -393,13 +398,15 @@ __morestack: movl %ebp,%esp # Restore stack pointer. + # As before, we now have %esp % 16 == 12. + pushl %eax # Push return value on old stack. pushl %edx - subl $8,%esp # Align stack to 16-byte boundary. + subl $4,%esp # Align stack to 16-byte boundary. call __morestack_unblock_signals - addl $8,%esp + addl $4,%esp popl %edx # Restore return value. popl %eax @@ -485,15 +492,21 @@ __morestack: pushq %r9 pushq %r11 - pushq $0 # For alignment. + + # We entered morestack with the stack pointer aligned to a + # 16-byte boundary (the call to morestack's caller used 8 + # bytes, and the call to morestack used 8 bytes). We have now + # pushed 10 registers, so we are still aligned to a 16-byte + # boundary. call __morestack_block_signals leaq -8(%rbp),%rdi # Address of new frame size. leaq 24(%rbp),%rsi # The caller's parameters. - addq $8,%rsp popq %rdx # The size of the parameters. + subq $8,%rsp # Align stack. + call __generic_morestack movq -8(%rbp),%r10 # Reload modified frame size @@ -564,6 +577,9 @@ __morestack: movq %rbp,%rsp # Restore stack pointer. + # Now (%rsp & 16) == 8. + + subq $8,%rsp # For alignment. pushq %rax # Push return value on old stack. pushq %rdx @@ -571,6 +587,7 @@ __morestack: popq %rdx # Restore return value. popq %rax + addq $8,%rsp .cfi_remember_state popq %rbp