Bug 27945 - [4.0/4.1/4.2/4.3 Regression] Packed struct of variable length has wrong size
Summary: [4.0/4.1/4.2/4.3 Regression] Packed struct of variable length has wrong size
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: unknown
: P1 normal
Target Milestone: 4.1.3
Assignee: Jason Merrill
URL:
Keywords: ABI
Depends on:
Blocks:
 
Reported: 2006-06-08 02:26 UTC by AlpT
Modified: 2007-09-24 21:00 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work: 3.3.3 4.3.0
Known to fail: 3.4.0 4.0.0 4.1.0 4.1.1 4.1.2 4.2.0
Last reconfirmed: 2006-11-14 01:10:19


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description AlpT 2006-06-08 02:26:40 UTC
Consider this test: http://hinezumilabs.org/alpt/test-pack.c

#define _PACKED_ __attribute__ ((__packed__))
int func(int levels) 
{
	struct foo {
		u_char		a;
		int32_t		b[levels];
		struct timeval _PACKED_ c[levels];
		u_int		d[4];
	}_PACKED_ pkt;

	struct bar {
		u_char		a;
		int32_t		b[levels];
		struct timeval  c[levels];
		u_int		d[4];
	}_PACKED_ pktII;

	printf("pkt %d, foo %d\n", sizeof(pkt), sizeof(struct foo));
	printf("pktII %d, bar %d\n", sizeof(pktII), sizeof(struct bar));
}

On a x86 with gcc-3.4.x/3.3.6 it prints:
pkt 65, foo 65
pktII 65, bar 65
(this is the right output)

while compiled for mips and runned on a BCM3302 (mips) with a gcc-3.4.4
it gives:
pkt 68, foo 68
pktII 68, bar 68

On a gentoo's 4.1.1 amd64:
pkt 104, foo 104
pktII 104, bar 104

On a x86 with the same version of gcc:
pkt 68, foo 68
pktII 68, bar 68

That's all ;)
Comment 1 AlpT 2006-06-08 02:33:21 UTC
On gcc 4.0.2, PIII:
pkt 68, foo 68
pktII 68, bar 68
Comment 2 Andrew Pinski 2006-06-08 03:00:54 UTC
> On a x86 with gcc-3.4.x/3.3.6 it prints:
> pkt 65, foo 65
> pktII 65, bar 65
> (this is the right output)

I don't get 65 with 3.4.0, I get 68 like 4.0.0 and above.
earth:~>~/ia32_linux_gcc3_4/bin/gcc -O2 t.c
earth:~>./a.out
pkt 68, foo 68
pktII 68, bar 68
earth:~>~/ia32_linux_gcc3_3/bin/gcc -O2 t.c
earth:~>./a.out
pkt 65, foo 65
pktII 65, bar 65

You should note this is extension even on top of C99 so it might be correct to change it.
Comment 3 AlpT 2006-06-08 03:14:21 UTC
You're right. on 3.4.x I get wrong results too.

The only positive result I get is on 3.3.6.

~> You should note this is extension even on top of C99 so it might be correct
~> to change it.

Yep, but it is a very elegant extension ;)
Comment 4 Andrew Pinski 2006-06-09 07:45:54 UTC
Confirmed.
Comment 5 Mark Mitchell 2006-07-05 17:59:04 UTC
We should understand if there was an intentional ABI change.
Comment 6 Janis Johnson 2006-08-08 16:05:05 UTC
A regression hunt on powerpc-linux showed that the behavior changed with this patch:

    http://gcc.gnu.org/viewcvs?view=rev&rev=65103

    r65103 | jason | 2003-03-31 20:25:11 +0000 (Mon, 31 Mar 2003)

There was discussion on the mailing list about this, referenced from http://gcc.gnu.org/ml/gcc-patches/2003-03/msg02550.html.
Comment 7 Kazu Hirata 2006-08-25 19:31:33 UTC
Reproduced.  Looking at this now.
Comment 8 Jason Merrill 2006-11-14 01:10:17 UTC
Ah, I see the problem.  The code I removed from the C and C++ front ends was redundant with code in layout_decl, except that the code in layout_decl deliberately ignores DECL_PACKED for fields of variable size.

      /* If the field is of variable size, we can't misalign it since we               
         have no way to make a temporary to align the result.  But this                
         isn't an issue if the decl is not addressable.  Likewise if it                
         is of unknown size.                                                           
                                                                                       
         Note that do_type_align may set DECL_USER_ALIGN, so we need to                
         check old_user_align instead.  */
=>    if (packed_p
          && !old_user_align
          && (DECL_NONADDRESSABLE_P (decl)
              || DECL_SIZE_UNIT (decl) == 0
              || TREE_CODE (DECL_SIZE_UNIT (decl)) == INTEGER_CST))
        DECL_ALIGN (decl) = MIN (DECL_ALIGN (decl), BITS_PER_UNIT);

This dates back to a change of Kenner's from 2001:

Sat Dec 29 15:48:54 2001  Richard Kenner  <kenner@vlsi1.ultra.nyu.edu>

        * stor-layout.c (layout_decl): Don't misalign field of variable size
        for packed record.

Richard, do you have any input?  Do you think there a way to make that test more specific to the case were fixing?
Comment 9 Gabriel Dos Reis 2007-02-03 17:34:27 UTC
Won't fix in GCC-4.0.x.  Adjusting milestone.
Comment 10 Jason Merrill 2007-09-24 21:00:36 UTC
Fixed in 4.3.0.  Not sure if the fix should go into 4.2.x, since it's an ABI change.