From patchwork Thu Dec 13 12:02:21 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kai Tietz X-Patchwork-Id: 205817 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 7930E2C0079 for ; Thu, 13 Dec 2012 23:02:40 +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=1356004961; h=Comment: DomainKey-Signature:Received:Received:Received:Received: MIME-Version:Received:Received:In-Reply-To:References:Date: Message-ID:Subject:From:To:Cc:Content-Type:Mailing-List: Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:Sender:Delivered-To; bh=jji0PeDGruaQEhqDqk/zgGqAmNw=; b=bFjVsJ7Nit5GEoPN9k7544bd1NgCzqDhBU+sRVngRQERyLv2kk9wb8klwYhJb6 xxLdtg3lUMJNMN9FWNcL1LrSVpElexl1cUNYWakQullqto/e094cEc/4ApC+CMl4 j2TUoQjrJCkYQyWvDvknG14knmqyz2uqnmVo6oyvKPAus= 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:MIME-Version:Received:Received:In-Reply-To:References:Date:Message-ID:Subject:From:To:Cc:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=Fly5JLXXvuePy7+v/7OIK3wksDPIOASlwSrds0vhD8+WD46r6PaJn3g5+ck4jr +x1RRguLjKWdQvotxVZ5PK5L3dWcZG2R0EU6NbRDnNxt51cl5Yn7wMYIJWeun24x nEpldh3eMjLAU02Z+CCBrmb9q3dCsQzOJGb86UkjjM1Kc=; Received: (qmail 18710 invoked by alias); 13 Dec 2012 12:02:35 -0000 Received: (qmail 18699 invoked by uid 22791); 13 Dec 2012 12:02:33 -0000 X-SWARE-Spam-Status: No, hits=-3.9 required=5.0 tests=AWL, BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, DKIM_VALID, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, KHOP_RCVD_TRUST, KHOP_THREADED, NML_ADSP_CUSTOM_MED, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_YE X-Spam-Check-By: sourceware.org Received: from mail-wi0-f175.google.com (HELO mail-wi0-f175.google.com) (209.85.212.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 13 Dec 2012 12:02:22 +0000 Received: by mail-wi0-f175.google.com with SMTP id hm11so3807765wib.8 for ; Thu, 13 Dec 2012 04:02:21 -0800 (PST) MIME-Version: 1.0 Received: by 10.180.101.99 with SMTP id ff3mr766458wib.21.1355400141283; Thu, 13 Dec 2012 04:02:21 -0800 (PST) Received: by 10.216.153.132 with HTTP; Thu, 13 Dec 2012 04:02:21 -0800 (PST) In-Reply-To: <20121213105657.GP2315@tucnak.redhat.com> References: <50C89E96.4090603@redhat.com> <20121213100738.GO2315@tucnak.redhat.com> <20121213105657.GP2315@tucnak.redhat.com> Date: Thu, 13 Dec 2012 13:02:21 +0100 Message-ID: Subject: Re: [patch c/c++]: Fix for PR c/52991 issue about ignored packed-attribute for ms-structure-layout From: Kai Tietz To: Jakub Jelinek Cc: Richard Biener , Richard Henderson , GCC Patches , "Joseph S. Myers" 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 2012/12/13 Jakub Jelinek : > On Thu, Dec 13, 2012 at 11:29:47AM +0100, Richard Biener wrote: >> On Thu, Dec 13, 2012 at 11:07 AM, Jakub Jelinek wrote: >> > On Thu, Dec 13, 2012 at 11:03:58AM +0100, Richard Biener wrote: >> >> struct X >> >> { >> >> char c; >> >> short s; >> >> char c2; >> >> short s2; >> >> } __attribute__((packed,aligned(2))); >> > >> > As struct-layout-1.exp tests show, this is something that was ABI-wise >> > changed already several times. That said, for non-ms-bitfield-layout I'd >> > strongly prefer if we could avoid yet another ABI change for it. >> >> Probably not exactly this case - 2.95, 3.2, 3.3, 4.1 ... all show the same >> behavior. And Kai should have seen regressions, no? > > Maybe my memory is too weak, dunno if it was packed,aligned(N) or aligned(N),packed, > but I remember seeing FAILs in such tests when testing against older > compilers. Well, I see the point about having packed and aligned in combination only for final-element in struct/union. As packed enforces unaligned accesses due it tries to save memory-use. If an user alignment is present the packing can just happen on last member for struct, as otherwise the alignment has to be applied and padding happens. But I agree that I didn't intended to change sysv_abi here by this patch. So I added to the change in start_record_layout a check to ms-bitfields use. 2012-12-13 Kai Tietz PR c/52991 * stor-layout.c (start_record_layout): Handle packed-attribute for ms-structure-layout. (update_alignment_for_field): Likewise. (place_field): Likewise. 2012-12-12 Kai Tietz PR c/52991 * gcc.dg/attr-ms_struct-packed2.c: New file. * gcc.dg/attr-ms_struct-packed3.c: New file. Ok for apply? Regards, Kai +} Index: gcc/gcc/stor-layout.c =================================================================== --- gcc.orig/gcc/stor-layout.c +++ gcc/gcc/stor-layout.c @@ -756,7 +756,10 @@ start_record_layout (tree t) /* If the type has a minimum specified alignment (via an attribute declaration, for example) use it -- otherwise, start with a one-byte alignment. */ - rli->record_align = MAX (BITS_PER_UNIT, TYPE_ALIGN (t)); + if (targetm.ms_bitfield_layout_p (t) && TYPE_PACKED (t)) + rli->record_align = BITS_PER_UNIT; + else + rli->record_align = MAX (BITS_PER_UNIT, TYPE_ALIGN (t)); rli->unpacked_align = rli->record_align; rli->offset_align = MAX (rli->record_align, BIGGEST_ALIGNMENT); @@ -952,15 +955,20 @@ update_alignment_for_field (record_layou meaningless. */ if (targetm.ms_bitfield_layout_p (rli->t)) { + if (rli->t && TYPE_PACKED (rli->t) + && (is_bitfield || !DECL_PACKED (field) + || DECL_SIZE (field) == NULL_TREE + || !integer_zerop (DECL_SIZE (field)))) + desired_align = BITS_PER_UNIT; /* Here, the alignment of the underlying type of a bitfield can affect the alignment of a record; even a zero-sized field can do this. The alignment should be to the alignment of the type, except that for zero-size bitfields this only applies if there was an immediately prior, nonzero-size bitfield. (That's the way it is, experimentally.) */ - if ((!is_bitfield && !DECL_PACKED (field)) - || ((DECL_SIZE (field) == NULL_TREE - || !integer_zerop (DECL_SIZE (field))) + else if ((!is_bitfield && !DECL_PACKED (field)) + || ((DECL_SIZE (field) == NULL_TREE + || !integer_zerop (DECL_SIZE (field))) ? !DECL_PACKED (field) : (rli->prev_field && DECL_BIT_FIELD_TYPE (rli->prev_field) @@ -1414,7 +1422,12 @@ place_field (record_layout_info rli, tre } /* Now align (conventionally) for the new type. */ - type_align = TYPE_ALIGN (TREE_TYPE (field)); + if (!TYPE_PACKED (rli->t)) + { + type_align = TYPE_ALIGN (TREE_TYPE (field)); + if (DECL_PACKED (field)) + type_align = MIN (type_align, BITS_PER_UNIT); + } if (maximum_field_alignment != 0) type_align = MIN (type_align, maximum_field_alignment); Index: gcc/gcc/testsuite/gcc.dg/attr-ms_struct-packed2.c =================================================================== --- /dev/null +++ gcc/gcc/testsuite/gcc.dg/attr-ms_struct-packed2.c @@ -0,0 +1,31 @@ +/* Test for MS structure with packed attribute. */ +/* { dg-do run { target *-*-interix* *-*-mingw* *-*-cygwin* i?86-*-darwin* i?86-*-linux* x86_64-*-linux* } } +/* { dg-options "-std=gnu99" } */ + +extern void abort (); + +struct A { + short s; + struct { int i; } __attribute__((__ms_struct__)); +} __attribute__((__ms_struct__, __packed__)); + +struct B { + short s; + struct { int i; } __attribute__((__ms_struct__, __packed__)); +} __attribute__((__ms_struct__, __packed__)); + +struct C { + struct { int i; } __attribute__((__ms_struct__)); + short s; +} __attribute__((__ms_struct__, __packed__)); + +int +main (void) +{ + if (sizeof (struct C) != sizeof (struct B) + || sizeof (struct A) != sizeof (struct B) + || sizeof (struct B) != 6) + abort (); + + return 0;