[PATCH] Fix PR66805 - #pragma pack affecting gcov_info_type layout

[PATCH] Fix PR66805 - #pragma pack affecting gcov_info_type layout

[Richard Biener]

The following fixes #pragma pack effect leaking to all types built
from the middle-end (so possibly even vector types built by the
vectorizer?).  The PR in question is about gcov_info_type where
layout is affected and inconsistency between that and the libgcov.a
copy causes libgcov to crash.

As the way to communicate #pragma pack to stor-layout.c is already
a hack I couldn't think of a better solution like that below.

Bootstrap and regtest running on x86_64-unknown-linux-gnu.

Ok?

Thanks,
Richard.

2015-07-08  Richard Biener  <rguenther@suse.de>

	* stor-layout.h (reset_maximum_field_alignment): Declare.
	* stor-layout.c (reset_maximum_field_alignment): New function.
	* toplev.c: Include stor-layout.h.
	(compile_file): Reset maximum_field_alignment after parsing.

Index: gcc/stor-layout.c
===================================================================
--- gcc/stor-layout.c	(revision 225534)
+++ gcc/stor-layout.c	(working copy)
@@ -59,6 +59,12 @@ tree sizetype_tab[(int) stk_type_kind_la
    The value is measured in bits.  */
 unsigned int maximum_field_alignment = TARGET_DEFAULT_PACK_STRUCT * BITS_PER_UNIT;
 
+void
+reset_maximum_field_alignment (void)
+{
+  maximum_field_alignment = TARGET_DEFAULT_PACK_STRUCT * BITS_PER_UNIT;
+}
+
 /* Nonzero if all REFERENCE_TYPEs are internal and hence should be allocated
    in the address spaces' address_mode, not pointer_mode.   Set only by
    internal_reference_types called only by a front end.  */
Index: gcc/stor-layout.h
===================================================================
--- gcc/stor-layout.h	(revision 225534)
+++ gcc/stor-layout.h	(working copy)
@@ -118,4 +118,7 @@ extern tree variable_size (tree);
 /* Vector types need to check target flags to determine type.  */
 extern machine_mode vector_type_mode (const_tree);
 
+/* Reset maximum_field_alignment to its default.  */
+extern void reset_maximum_field_alignment (void);
+
 #endif  // GCC_STOR_LAYOUT_H
Index: gcc/toplev.c
===================================================================
--- gcc/toplev.c	(revision 225534)
+++ gcc/toplev.c	(working copy)
@@ -90,6 +90,7 @@ along with GCC; see the file COPYING3.
 #include "optabs.h"
 #include "tree-chkp.h"
 #include "omp-low.h"
+#include "stor-layout.h"
 
 #if defined(DBX_DEBUGGING_INFO) || defined(XCOFF_DEBUGGING_INFO)
 #include "dbxout.h"
@@ -553,6 +554,11 @@ compile_file (void)
 
   if (flag_syntax_only || flag_wpa)
     return;
+ 
+  /* Reset maximum_field_alignment, it can be adjusted by #pragma pack
+     and this shouldn't influence any types built by the middle-end
+     from now on (like gcov_info_type).  */
+  reset_maximum_field_alignment ();
 
   ggc_protect_identifiers = false;
 

Re: [PATCH] Fix PR66805 - #pragma pack affecting gcov_info_type layout

[Richard Biener]

On Wed, 8 Jul 2015, Richard Biener wrote:

> 
> The following fixes #pragma pack effect leaking to all types built
> from the middle-end (so possibly even vector types built by the
> vectorizer?).  The PR in question is about gcov_info_type where
> layout is affected and inconsistency between that and the libgcov.a
> copy causes libgcov to crash.
> 
> As the way to communicate #pragma pack to stor-layout.c is already
> a hack I couldn't think of a better solution like that below.
> 
> Bootstrap and regtest running on x86_64-unknown-linux-gnu.
> 
> Ok?

Ok, so I found initial_max_fld_align and instead propose the following
simpler.

2015-07-08  Richard Biener  <rguenther@suse.de>

	* toplev.c (compile_file): Reset maximum_field_alignment after parsing.

Index: gcc/toplev.c
===================================================================
--- gcc/toplev.c	(revision 225534)
+++ gcc/toplev.c	(working copy)
@@ -553,6 +553,11 @@ compile_file (void)
 
   if (flag_syntax_only || flag_wpa)
     return;
+ 
+  /* Reset maximum_field_alignment, it can be adjusted by #pragma pack
+     and this shouldn't influence any types built by the middle-end
+     from now on (like gcov_info_type).  */
+  maximum_field_alignment = initial_max_fld_align * BITS_PER_UNIT;
 
   ggc_protect_identifiers = false;
 

Re: [PATCH] Fix PR66805 - #pragma pack affecting gcov_info_type layout

[Jakub Jelinek]

On Wed, Jul 08, 2015 at 01:58:38PM +0200, Richard Biener wrote:
> 
> The following fixes #pragma pack effect leaking to all types built
> from the middle-end (so possibly even vector types built by the
> vectorizer?).  The PR in question is about gcov_info_type where
> layout is affected and inconsistency between that and the libgcov.a
> copy causes libgcov to crash.
> 
> As the way to communicate #pragma pack to stor-layout.c is already
> a hack I couldn't think of a better solution like that below.
> 
> Bootstrap and regtest running on x86_64-unknown-linux-gnu.
> 
> Ok?
> 
> Thanks,
> Richard.
> 
> 2015-07-08  Richard Biener  <rguenther@suse.de>
> 
> 	* stor-layout.h (reset_maximum_field_alignment): Declare.
> 	* stor-layout.c (reset_maximum_field_alignment): New function.
> 	* toplev.c: Include stor-layout.h.
> 	(compile_file): Reset maximum_field_alignment after parsing.

toplev.c already sets maximum_field_alignment directly, and to a different
value:
maximum_field_alignment = initial_max_fld_align * BITS_PER_UNIT;
so I'm not sure you need a new function.  And, shouldn't you reset to
maximum_field_alignment = initial_max_fld_align * BITS_PER_UNIT;
?  -fpack-struct= is an ABI changing option, so IMHO it should affect
even gcov_info_type etc.

	Jakub

Re: [PATCH] Fix PR66805 - #pragma pack affecting gcov_info_type layout

[Alexander Monakov]

The same bug was earlier reported as PR gcov-profile/43341:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=43341

Alexander

Re: [PATCH] Fix PR66805 - #pragma pack affecting gcov_info_type layout

[Richard Biener]

On Wed, 8 Jul 2015, Jakub Jelinek wrote:

> On Wed, Jul 08, 2015 at 01:58:38PM +0200, Richard Biener wrote:
> > 
> > The following fixes #pragma pack effect leaking to all types built
> > from the middle-end (so possibly even vector types built by the
> > vectorizer?).  The PR in question is about gcov_info_type where
> > layout is affected and inconsistency between that and the libgcov.a
> > copy causes libgcov to crash.
> > 
> > As the way to communicate #pragma pack to stor-layout.c is already
> > a hack I couldn't think of a better solution like that below.
> > 
> > Bootstrap and regtest running on x86_64-unknown-linux-gnu.
> > 
> > Ok?
> > 
> > Thanks,
> > Richard.
> > 
> > 2015-07-08  Richard Biener  <rguenther@suse.de>
> > 
> > 	* stor-layout.h (reset_maximum_field_alignment): Declare.
> > 	* stor-layout.c (reset_maximum_field_alignment): New function.
> > 	* toplev.c: Include stor-layout.h.
> > 	(compile_file): Reset maximum_field_alignment after parsing.
> 
> toplev.c already sets maximum_field_alignment directly, and to a different
> value:
> maximum_field_alignment = initial_max_fld_align * BITS_PER_UNIT;
> so I'm not sure you need a new function.  And, shouldn't you reset to
> maximum_field_alignment = initial_max_fld_align * BITS_PER_UNIT;
> ?  -fpack-struct= is an ABI changing option, so IMHO it should affect
> even gcov_info_type etc.

That would support the 2nd patch variant.  Note that -fpack-struct
can be specified in optimize attributes...

Richard.

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nuernberg)

Re: [PATCH] Fix PR66805 - #pragma pack affecting gcov_info_type layout

[Jakub Jelinek]

On Wed, Jul 08, 2015 at 02:40:33PM +0200, Richard Biener wrote:
> > toplev.c already sets maximum_field_alignment directly, and to a different
> > value:
> > maximum_field_alignment = initial_max_fld_align * BITS_PER_UNIT;
> > so I'm not sure you need a new function.  And, shouldn't you reset to
> > maximum_field_alignment = initial_max_fld_align * BITS_PER_UNIT;
> > ?  -fpack-struct= is an ABI changing option, so IMHO it should affect
> > even gcov_info_type etc.
> 
> That would support the 2nd patch variant.  Note that -fpack-struct
> can be specified in optimize attributes...

Yeah, the second patch LGTM.

	Jakub