From patchwork Fri Jan 27 12:02:05 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Botcazou X-Patchwork-Id: 720653 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)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3v8y8P5Fvtz9tT8 for ; Fri, 27 Jan 2017 23:02:27 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="sRd8Yzvb"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:subject:date:message-id:mime-version:content-type :content-transfer-encoding; q=dns; s=default; b=d3D2mCsHpRFL/cCL ovV6H8n1hCYfT7z5kBZDQyf9L09F7/pHu9ABEczH/8eV1tJokhuNm/uCFCHBvliq yb/lObDQ5c5+NeFe5u9N8Vowha2QSotrR6q8iurrySRaZsJQjHglLUbjNh8d15il wZDcHL3VV88vrx+FWLZ/cIOBvm0= 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 :to:subject:date:message-id:mime-version:content-type :content-transfer-encoding; s=default; bh=8IN+OZlAsH5gcszDW3EI7A BJaLU=; b=sRd8YzvbuEraCWkrpQw30mLkISJfJR/3vnkOECAqgNWbD5aVhUJlFz GT8jOwsO7NRwze+E7Ge6tuBFtUFt5ws/8s8iMDtitGV3wMgHUKCDYMPiUZZ1f3ij 6IcTqhfKVgONhK5GVvxHZkwSt9WNDaxiXlQPjdwtnU+hQB9zKp6Jo= Received: (qmail 108645 invoked by alias); 27 Jan 2017 12:02:19 -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 108633 invoked by uid 89); 27 Jan 2017 12:02:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=sk:vogt@li, U*vogt, vogt@linux.vnet.ibm.com, sk:vogtli X-HELO: smtp.eu.adacore.com Received: from mel.act-europe.fr (HELO smtp.eu.adacore.com) (194.98.77.210) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 27 Jan 2017 12:02:08 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id AF77E814A9 for ; Fri, 27 Jan 2017 13:02:06 +0100 (CET) Received: from smtp.eu.adacore.com ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id V71yJ8k9CzHv for ; Fri, 27 Jan 2017 13:02:06 +0100 (CET) Received: from polaris.localnet (bon31-6-88-161-99-133.fbx.proxad.net [88.161.99.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.eu.adacore.com (Postfix) with ESMTPSA id 872578137A for ; Fri, 27 Jan 2017 13:02:06 +0100 (CET) From: Eric Botcazou To: gcc-patches@gcc.gnu.org Subject: [patch] Fix PR middle-end/78468 Date: Fri, 27 Jan 2017 13:02:05 +0100 Message-ID: <2127372.YidMXMCpNE@polaris> User-Agent: KMail/4.14.10 (Linux/3.16.7-53-desktop; KDE/4.14.9; x86_64; ; ) MIME-Version: 1.0 Hi, this is the regression introduced on 32-bit SPARC by this change: 2016-11-18 Dominik Vogt Re-apply after PR bootstrap/77359 is fixed: 2016-08-23 Dominik Vogt * explow.c (get_dynamic_stack_size): Take known alignment of stack pointer + STACK_DYNAMIC_OFFSET into account when calculating the size needed. and which causes the compiler to overwrite contents on the stack after a dynamic allocation in some cases. IMO this patch is backwards and should not have been approved: as explained by the comment in get_dynamic_stack_size: /* We will need to ensure that the address we return is aligned to REQUIRED_ALIGN. At this point in the compilation, we don't always know the final value of the STACK_DYNAMIC_OFFSET used in function.c (it might depend on the size of the outgoing parameter lists, for example), so we must preventively align the value. We leave space in SIZE for the hole that might result from the alignment operation. */ the function used to align the value to maintain the expected alignment of VIRTUAL_STACK_DYNAMIC_REGNUM. But the new version does the opposite (without changing the comment): unsigned known_align = REGNO_POINTER_ALIGN (VIRTUAL_STACK_DYNAMIC_REGNUM); it assumes that the alignment of VIRTUAL_STACK_DYNAMIC_REGNUM is already correct to optimize the dynamic allocation, i.e. to save a few bytes. This breaks the interface between middle-end and back-end and resulted in the breakage of PowerPC/AIX and now of the 32-bit SPARC port. And the failure mode is quite nasty: the entire testsuite of the compiler proper, including Ada, is clean on 32-bit SPARC and the issue only shows up in the libgomp testsuite, probably because of specific patterns. IMO it's very likely that other architectures among the ~50 supported ones are affected and the issue might show up only in very peculiar circumstances as on 32-bit SPARC so I think we should repair the breakage for GCC 7. The attached patch is a middle ground between the previously working and currently broken situations: if the back-end defines STACK_DYNAMIC_OFFSET, then the middle-end assumes that STACK_DYNAMIC_OFFSET maintains the alignment; if it doesn't, which means the default value of STACK_DYNAMIC_OFFSET computed in function.c is used instead, then the middle-end maintains the alignment itself. Tested on x86_64-suse-linux and SPARC/Solaris, OK for the mainline? 2017-01-27 Eric Botcazou PR middle-end/78468 * explow.c (get_dynamic_stack_size): Take known alignment of stack pointer + STACK_DYNAMIC_OFFSET into account only if the back-end defines STACK_DYNAMIC_OFFSET. Index: explow.c =================================================================== --- explow.c (revision 244917) +++ explow.c (working copy) @@ -1231,9 +1231,14 @@ get_dynamic_stack_size (rtx *psize, unsi know the final value of the STACK_DYNAMIC_OFFSET used in function.c (it might depend on the size of the outgoing parameter lists, for example), so we must preventively align the value. We leave space - in SIZE for the hole that might result from the alignment operation. */ - + in SIZE for the hole that might result from the alignment operation. + However, we assume that, if STACK_DYNAMIC_OFFSET is defined by the + back-end itself as opposed to function.c, it is properly aligned. */ +#ifdef STACK_DYNAMIC_OFFSET unsigned known_align = REGNO_POINTER_ALIGN (VIRTUAL_STACK_DYNAMIC_REGNUM); +#else + unsigned known_align = 0; +#endif if (known_align == 0) known_align = BITS_PER_UNIT; if (required_align > known_align)