re PR c/7054 (#pragma pack handled incorrectly)

PR c/7054
        * defaults.h (TARGET_DEFAULT_PACK_STRUCT): Provide default.
        * tree.h (initial_max_fld_align): Declare
        * stor-layout.c (initial_max_fld_align): Define and initialize.
        (maximum_field_alignment): Initialize to the same value.
        * common.opt: Add -fpack-struct= variant of switch.
        * opts.c: Handle -fpack-struct= variant of switch.
        * c-pragma.c: Change #pragma pack() handling so that it becomes
        compatible to other compilers: accept individual 'push' argument,
        make final pop restore (command line) default, correct interaction
        of push/pop and sole specification of a new alignment (so that the
        sequence #pragma pack(push) - #pragma pack(<n>) becomes identical
        to #pragma pack(push, <n>).
        * doc/extend.texi: New node "Structure-Packing Pragmas" under
        "Pragmas", describing #pragma pack.
        * doc/invoke.texi: Document -fpack-struct=<n> variant of switch.
        * doc/tm.texi: Adjust description for HANDLE_PRAGMA_PACK_PUSH_POP.
        Document new TARGET_DEFAULT_PACK_STRUCT.
testsuite:
        * gcc.dg/pack-test-2.c: Adjust to permit and check #pragma pack(push).
        * gcc.dg/c99-flex-array-4.c: Add -fpack-struct=8 to provide a
        deterministic starting point for the alignment of structure fields.
        * gcc.dg/Wpadded.c: Dito.
        * g++.dg/abi/vbase10.C: Dito.

From-SVN: r87037

gcc/ChangeLog

@@ -1,3 +1,24 @@
+2004-09-03  Jan Beulich  <jbeulich@novell.com>
+
+	PR c/7054
+	* defaults.h (TARGET_DEFAULT_PACK_STRUCT): Provide default.
+	* tree.h (initial_max_fld_align): Declare
+	* stor-layout.c (initial_max_fld_align): Define and initialize.
+	(maximum_field_alignment): Initialize to the same value.
+	* common.opt: Add -fpack-struct= variant of switch.
+	* opts.c: Handle -fpack-struct= variant of switch.
+	* c-pragma.c: Change #pragma pack() handling so that it becomes
+	compatible to other compilers: accept individual 'push' argument,
+	make final pop restore (command line) default, correct interaction
+	of push/pop and sole specification of a new alignment (so that the
+	sequence #pragma pack(push) - #pragma pack(<n>) becomes identical
+	to #pragma pack(push, <n>).
+	* doc/extend.texi: New node "Structure-Packing Pragmas" under
+	"Pragmas", describing #pragma pack.
+	* doc/invoke.texi: Document -fpack-struct=<n> variant of switch.
+	* doc/tm.texi: Adjust description for HANDLE_PRAGMA_PACK_PUSH_POP.
+	Document new TARGET_DEFAULT_PACK_STRUCT.
+
 2004-09-03  Devang Patel  <dpatel@apple.com>
 
 	* dwarf2out.c (gen_field_die). Equate decl number to die.

gcc/c-pragma.c

@@ -42,7 +42,6 @@ Software Foundation, 59 Temple Place - Suite 330, Boston, MA
 typedef struct align_stack GTY(())
 {
   int                  alignment;
-  unsigned int         num_pushes;
   tree                 id;
   struct align_stack * prev;
 } align_stack;
@@ -59,8 +58,9 @@ static void handle_pragma_pack (cpp_reader *);
    happens, we restore the value to this, not to a value of 0 for
    maximum_field_alignment.  Value is in bits.  */
 static int default_alignment;
-#define SET_GLOBAL_ALIGNMENT(ALIGN) \
-  (default_alignment = maximum_field_alignment = (ALIGN))
+#define SET_GLOBAL_ALIGNMENT(ALIGN) (maximum_field_alignment = *(alignment_stack == NULL \
+	? &default_alignment \
+	: &alignment_stack->alignment) = (ALIGN))
 
 static void push_alignment (int, tree);
 static void pop_alignment (tree);
@@ -69,31 +69,23 @@ static void pop_alignment (tree);
 static void
 push_alignment (int alignment, tree id)
 {
-  if (alignment_stack == NULL
-      || alignment_stack->alignment != alignment
-      || id != NULL_TREE)
-    {
-      align_stack * entry;
+  align_stack * entry;
 
-      entry = ggc_alloc (sizeof (* entry));
+  entry = ggc_alloc (sizeof (* entry));
 
-      entry->alignment  = alignment;
-      entry->num_pushes = 1;
-      entry->id         = id;
-      entry->prev       = alignment_stack;
-      
-      /* The current value of maximum_field_alignment is not necessarily 
-	 0 since there may be a #pragma pack(<n>) in effect; remember it 
-	 so that we can restore it after the final #pragma pop().  */
-      if (alignment_stack == NULL)
-	default_alignment = maximum_field_alignment;
-      
-      alignment_stack = entry;
+  entry->alignment  = alignment;
+  entry->id         = id;
+  entry->prev       = alignment_stack;
+       
+  /* The current value of maximum_field_alignment is not necessarily 
+     0 since there may be a #pragma pack(<n>) in effect; remember it 
+     so that we can restore it after the final #pragma pop().  */
+  if (alignment_stack == NULL)
+    default_alignment = maximum_field_alignment;
+ 
+  alignment_stack = entry;
 
-      maximum_field_alignment = alignment;
-    }
-  else
-    alignment_stack->num_pushes ++;
+  maximum_field_alignment = alignment;
 }
 
 /* Undo a push of an alignment onto the stack.  */
@@ -103,12 +95,7 @@ pop_alignment (tree id)
   align_stack * entry;
       
   if (alignment_stack == NULL)
-    {
-      warning ("\
-#pragma pack (pop) encountered without matching #pragma pack (push, <n>)"
-	       );
-      return;
-    }
+    GCC_BAD("#pragma pack (pop) encountered without matching #pragma pack (push)");
 
   /* If we got an identifier, strip away everything above the target
      entry so that the next step will restore the state just below it.  */
@@ -117,27 +104,20 @@ pop_alignment (tree id)
       for (entry = alignment_stack; entry; entry = entry->prev)
 	if (entry->id == id)
 	  {
-	    entry->num_pushes = 1;
 	    alignment_stack = entry;
 	    break;
 	  }
       if (entry == NULL)
 	warning ("\
-#pragma pack(pop, %s) encountered without matching #pragma pack(push, %s, <n>)"
+#pragma pack(pop, %s) encountered without matching #pragma pack(push, %s)"
 		 , IDENTIFIER_POINTER (id), IDENTIFIER_POINTER (id));
     }
 
-  if (-- alignment_stack->num_pushes == 0)
-    {
-      entry = alignment_stack->prev;
+  entry = alignment_stack->prev;
 
-      if (entry == NULL)
-	maximum_field_alignment = default_alignment;
-      else
-	maximum_field_alignment = entry->alignment;
+  maximum_field_alignment = entry ? entry->alignment : default_alignment;
 
-      alignment_stack = entry;
-    }
+  alignment_stack = entry;
 }
 #else  /* not HANDLE_PRAGMA_PACK_PUSH_POP */
 #define SET_GLOBAL_ALIGNMENT(ALIGN) (maximum_field_alignment = (ALIGN))
@@ -150,7 +130,9 @@ pop_alignment (tree id)
 /* #pragma pack ()
    #pragma pack (N)
    
+   #pragma pack (push)
    #pragma pack (push, N)
+   #pragma pack (push, ID)
    #pragma pack (push, ID, N)
    #pragma pack (pop)
    #pragma pack (pop, ID) */
@@ -169,7 +151,7 @@ handle_pragma_pack (cpp_reader * ARG_UNUSED (dummy))
   if (token == CPP_CLOSE_PAREN)
     {
       action = set;
-      align = 0;
+      align = initial_max_fld_align;
     }
   else if (token == CPP_NUMBER)
     {
@@ -180,8 +162,8 @@ handle_pragma_pack (cpp_reader * ARG_UNUSED (dummy))
     }
   else if (token == CPP_NAME)
     {
-#define GCC_BAD_ACTION do { if (action == push) \
-	  GCC_BAD ("malformed '#pragma pack(push[, id], <n>)' - ignored"); \
+#define GCC_BAD_ACTION do { if (action != pop) \
+	  GCC_BAD ("malformed '#pragma pack(push[, id][, <n>])' - ignored"); \
 	else \
 	  GCC_BAD ("malformed '#pragma pack(pop[, id])' - ignored"); \
 	} while (0)
@@ -194,31 +176,21 @@ handle_pragma_pack (cpp_reader * ARG_UNUSED (dummy))
       else
 	GCC_BAD2 ("unknown action '%s' for '#pragma pack' - ignored", op);
 
-      token = c_lex (&x);
-      if (token != CPP_COMMA && action == push)
-	GCC_BAD_ACTION;
-
-      if (token == CPP_COMMA)
+      while ((token = c_lex (&x)) == CPP_COMMA)
 	{
 	  token = c_lex (&x);
-	  if (token == CPP_NAME)
+	  if (token == CPP_NAME && id == 0)
 	    {
 	      id = x;
-	      if (action == push && c_lex (&x) != CPP_COMMA)
-		GCC_BAD_ACTION;
-	      token = c_lex (&x);
 	    }
-
-	  if (action == push)
+	  else if (token == CPP_NUMBER && action == push && align == -1)
 	    {
-	      if (token == CPP_NUMBER)
-		{
-		  align = TREE_INT_CST_LOW (x);
-		  token = c_lex (&x);
-		}
-	      else
-		GCC_BAD_ACTION;
+	      align = TREE_INT_CST_LOW (x);
+	      if (align == -1)
+		action = set;
 	    }
+	  else
+	    GCC_BAD_ACTION;
 	}
 
       if (token != CPP_CLOSE_PAREN)
@@ -231,6 +203,9 @@ handle_pragma_pack (cpp_reader * ARG_UNUSED (dummy))
   if (c_lex (&x) != CPP_EOF)
     warning ("junk at end of '#pragma pack'");
 
+  if (flag_pack_struct)
+    GCC_BAD ("#pragma pack has no effect with -fpack-struct - ignored");
+
   if (action != pop)
     switch (align)
       {
@@ -242,6 +217,12 @@ handle_pragma_pack (cpp_reader * ARG_UNUSED (dummy))
       case 16:
 	align *= BITS_PER_UNIT;
 	break;
+      case -1:
+	if (action == push)
+	  {
+	    align = maximum_field_alignment;
+	    break;
+	  }
       default:
 	GCC_BAD2 ("alignment must be a small power of two, not %d", align);
       }

gcc/common.opt

@@ -575,6 +575,10 @@ fpack-struct
 Common Report Var(flag_pack_struct)
 Pack structure members together without holes
 
+fpack-struct=
+Common RejectNegative Joined UInteger
+-fpack-struct=<number>	Set initial maximum structure member alignment
+
 fpcc-struct-return
 Common Report Var(flag_pcc_struct_return,1) VarExists
 Return small aggregates in memory, not registers

gcc/defaults.h

@@ -476,6 +476,10 @@ do { fputs (integer_asm_op (POINTER_SIZE / BITS_PER_UNIT, TRUE), FILE); \
 #define PREFERRED_STACK_BOUNDARY STACK_BOUNDARY
 #endif
 
+#ifndef TARGET_DEFAULT_PACK_STRUCT
+#define TARGET_DEFAULT_PACK_STRUCT 0
+#endif
+
 /* By default, the C++ compiler will use function addresses in the
    vtable entries.  Setting this nonzero tells the compiler to use
    function descriptors instead.  The value of this macro says how

gcc/doc/extend.texi

@@ -7672,6 +7672,7 @@ for further explanation.
 * Darwin Pragmas::
 * Solaris Pragmas::
 * Symbol-Renaming Pragmas::
+* Structure-Packing Pragmas::
 @end menu
 
 @node ARM Pragmas
@@ -7848,6 +7849,30 @@ labels, but if @code{#pragma extern_prefix} triggers first we have no
 way of knowing that that happened.)
 @end enumerate
 
+@node Structure-Packing Pragmas
+@subsection Structure-Packing Pragmas
+
+For compatibility with Win32, GCC supports as set of @code{#pragma}
+directives which change the maximum alignment of members of structures,
+unions, and classes subsequently defined.  The @var{n} value below always
+is required to be a small power of two and specifies the new alignment
+in bytes.
+
+@enumerate
+@item @code{#pragma pack(@var{n})} simply sets the new alignment.
+@item @code{#pragma pack()} sets the alignment to the one that was in
+effect when compilation started (see also command line option
+@option{-fpack-struct[=<n>]} @pxref{Code Gen Options}).
+@item @code{#pragma pack(push[,@var{n}])} pushes the current alignment
+setting on an internal stack and then optionally sets the new alignment.
+@item @code{#pragma pack(pop)} restores the alignment setting to the one
+saved at the top of the internal stack (and removes that stack entry).
+Note that @code{#pragma pack([@var{n}])} does not influence this internal
+stack; thus it is possible to have @code{#pragma pack(push)} followed by
+multiple @code{#pragma pack(@var{n})} instances and finalized by a single
+@code{#pragma pack(pop)}.
+@end enumerate
+
 @node Unnamed Fields
 @section Unnamed struct/union fields within structs/unions.
 @cindex struct

gcc/doc/invoke.texi

@@ -699,7 +699,7 @@ See S/390 and zSeries Options.
 -fpcc-struct-return  -fpic  -fPIC -fpie -fPIE @gol
 -freg-struct-return  -fshared-data  -fshort-enums @gol
 -fshort-double  -fshort-wchar @gol
--fverbose-asm  -fpack-struct  -fstack-check @gol
+-fverbose-asm  -fpack-struct[=@var{n}]  -fstack-check @gol
 -fstack-limit-register=@var{reg}  -fstack-limit-symbol=@var{sym} @gol
 -fargument-alias  -fargument-noalias @gol
 -fargument-noalias-global  -fleading-underscore @gol
@@ -11789,9 +11789,13 @@ a register in which function values may be returned.
 This flag does not have a negative form, because it specifies a
 three-way choice.
 
-@item -fpack-struct
+@item -fpack-struct[=@var{n}]
 @opindex fpack-struct
-Pack all structure members together without holes.
+Without a value specified, pack all structure members together without
+holes. When a value is specified (which must be a small power of two), pack
+structure members according to this value, representing the maximum
+alignment (that is, objects with default alignment requirements larger than
+this will be output potentially unaligned at the next fitting location.
 
 @strong{Warning:} the @option{-fpack-struct} switch causes GCC to generate
 code that is not binary compatible with code generated without that switch.

gcc/doc/tm.texi

@@ -9017,16 +9017,23 @@ of specifically named weak labels, optionally with a value.
 @findex pragma
 @defmac HANDLE_PRAGMA_PACK_PUSH_POP
 Define this macro (to a value of 1) if you want to support the Win32
-style pragmas @samp{#pragma pack(push,@var{n})} and @samp{#pragma
-pack(pop)}.  The @samp{pack(push,@var{n})} pragma specifies the maximum alignment
-(in bytes) of fields within a structure, in much the same way as the
-@samp{__aligned__} and @samp{__packed__} @code{__attribute__}s do.  A
+style pragmas @samp{#pragma pack(push[,@var{n}])} and @samp{#pragma
+pack(pop)}.  The @samp{pack(push,[@var{n}])} pragma specifies the maximum
+alignment (in bytes) of fields within a structure, in much the same way as
+the @samp{__aligned__} and @samp{__packed__} @code{__attribute__}s do.  A
 pack value of zero resets the behavior to the default.  Successive
 invocations of this pragma cause the previous values to be stacked, so
 that invocations of @samp{#pragma pack(pop)} will return to the previous
 value.
 @end defmac
 
+@defmac TARGET_DEFAULT_PACK_STRUCT
+If your target requires a structure packing default other than 0 (meaning
+the machine default), define this macro the the necessary value (in bytes).
+This must be a value that would also valid to be used with
+@samp{#pragma pack()} (that is, a small power of two).
+@end defmac
+
 @defmac DOLLARS_IN_IDENTIFIERS
 Define this macro to control use of the character @samp{$} in
 identifier names for the C family of languages.  0 means @samp{$} is

gcc/opts.c

@@ -810,6 +810,16 @@ common_handle_option (size_t scode, const char *arg, int value)
       pp_set_line_maximum_length (global_dc->printer, value);
       break;
 
+    case OPT_fpack_struct_:
+      if (value <= 0 || (value & (value - 1)) || value > 16)
+	error("structure alignment must be a small power of two, not %d", value);
+      else
+	{
+	  initial_max_fld_align = value;
+	  maximum_field_alignment = value * BITS_PER_UNIT;
+	}
+      break;
+
     case OPT_fpeel_loops:
       flag_peel_loops_set = true;
       break;

gcc/stor-layout.c

@@ -43,7 +43,9 @@ tree sizetype_tab[(int) TYPE_KIND_LAST];
 
 /* If nonzero, this is an upper limit on alignment of structure fields.
    The value is measured in bits.  */
-unsigned int maximum_field_alignment;
+unsigned int maximum_field_alignment = TARGET_DEFAULT_PACK_STRUCT * BITS_PER_UNIT;
+/* ... and its original value in bytes, specified via -fpack-struct=<value>. */
+unsigned int initial_max_fld_align = TARGET_DEFAULT_PACK_STRUCT;
 
 /* If nonzero, the alignment of a bitstring or (power-)set value, in bits.
    May be overridden by front-ends.  */

gcc/testsuite/ChangeLog

@@ -1,3 +1,11 @@
+2004-09-03  Jan Beulich  <jbeulich@novell.com>
+
+	* gcc.dg/pack-test-2.c: Adjust to permit and check #pragma pack(push).
+	* gcc.dg/c99-flex-array-4.c: Add -fpack-struct=8 to provide a
+	deterministic starting point for the alignment of structure fields.
+	* gcc.dg/Wpadded.c: Dito.
+	* g++.dg/abi/vbase10.C: Dito.
+
 2004-09-03  Devang Patel  <dpatel@apple.com>
 
 	* g++.dg/debug/pr15736.cc: New test.

gcc/testsuite/g++.dg/abi/vbase10.C

@@ -1,5 +1,7 @@
 // { dg-do compile }
-// { dg-options "-Wabi -fabi-version=1" }
+// -fpack-struct is necessary because the code below assumes the initial
+// packing is larger than 1, which cannot ge guaranteed for all targets.
+// { dg-options "-Wabi -fabi-version=1 -fpack-struct=8" }
 // On ARM processors, the alignment of B will be 4 even though it
 // contains only a single "char".  That would avoids the situation
 // that the warning below is designed to catch.  We therefore

gcc/testsuite/gcc.dg/Wpadded.c

@@ -1,7 +1,9 @@
 /* Source: EMC.  */
 
 /* { dg-do compile } */
-/* { dg-options "-Wpadded" } */
+/* -fpack-struct is necessary because the warning expected requires the initial
+   packing to be larger than 1, which cannot ge guaranteed for all targets. */
+/* { dg-options "-Wpadded -fpack-struct=8" } */
 
 struct foo {
   char bar;

gcc/testsuite/gcc.dg/c99-flex-array-4.c

@@ -5,7 +5,9 @@
    from Tony Finch <dot@dotat.at>, adapted to a testcase by Joseph Myers
    <jsm28@cam.ac.uk>.  See also WG14 reflector messages 9571-3.  */
 /* { dg-do compile } */
-/* { dg-options "-std=iso9899:1999 -pedantic-errors" } */
+/* -fpack-struct is necessary because the code below assumes the initial
+   packing is larger than 1, which cannot ge guaranteed for all targets. */
+/* { dg-options "-std=iso9899:1999 -fpack-struct=8 -pedantic-errors" } */
 
 #include <stddef.h>
 

gcc/testsuite/gcc.dg/pack-test-2.c

@@ -3,9 +3,11 @@
 
 /* { dg-do compile { target *-*-linux* *-*-cygwin* powerpc*-*-eabi* } } */
 
-#pragma pack(push)              /* { dg-error "malformed" } */
 #pragma pack(pop)               /* { dg-error "without matching" } */
 
+#pragma pack(push)
+#pragma pack(pop)               /* reset */
+
 #pragma pack(push, foo, 1)
 #pragma pack(pop, foo, 1)       /* { dg-error "malformed" } (/
 #pragma pack(pop)               /* reset */

gcc/tree.h

@@ -3102,8 +3102,10 @@ extern void put_pending_sizes (tree);
    + (BITS_PER_UNIT > 8) + (BITS_PER_UNIT > 16) + (BITS_PER_UNIT > 32) \
    + (BITS_PER_UNIT > 64) + (BITS_PER_UNIT > 128) + (BITS_PER_UNIT > 256))
 
-/* If nonzero, an upper limit on alignment of structure fields, in bits.  */
+/* If nonzero, an upper limit on alignment of structure fields, in bits,  */
 extern unsigned int maximum_field_alignment;
+/* and its original value in bytes, specified via -fpack-struct=<value>. */
+extern unsigned int initial_max_fld_align;
 
 /* If nonzero, the alignment of a bitstring or (power-)set value, in bits.  */
 extern unsigned int set_alignment;