[Bug c/53037] warn_if_not_aligned(X)

[vries at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

Tom de Vries  changed:

   What|Removed |Added

 CC||vries at gcc dot gnu.org

--- Comment #43 from Tom de Vries  ---
*** Bug 81909 has been marked as a duplicate of this bug. ***

[Bug c/53037] warn_if_not_aligned(X)

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #42 from Eric Botcazou  ---
Author: ebotcazou
Date: Tue Nov  7 17:37:29 2017
New Revision: 254503

URL: https://gcc.gnu.org/viewcvs?rev=254503=gcc=rev
Log:
PR c/53037
* stor-layout.c: Include attribs.h.
(handle_warn_if_not_align): Replace test on TYPE_USER_ALIGN with
explicit lookup of "aligned" attribute.

Modified:
trunk/gcc/ChangeLog
trunk/gcc/stor-layout.c

[Bug c/53037] warn_if_not_aligned(X)

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #41 from Aldy Hernandez  ---
Author: aldyh
Date: Wed Sep 13 17:08:36 2017
New Revision: 252478

URL: https://gcc.gnu.org/viewcvs?rev=252478=gcc=rev
Log:
Add warn_if_not_aligned attribute

Add warn_if_not_aligned attribute as well as  command line options:
-Wif-not-aligned and -Wpacked-not-aligned.

__attribute__((warn_if_not_aligned(N))) causes compiler to issue a
warning if the field in a struct or union is not aligned to N:

typedef unsigned long long __u64
  __attribute__((aligned(4),warn_if_not_aligned(8)));

struct foo
{
  int i1;
  int i2;
  __u64 x;
};

__u64 is aligned to 4 bytes.  But inside struct foo, __u64 should be
aligned at 8 bytes.  It is used to define struct foo in such a way that
struct foo has the same layout and x has the same alignment when __u64
is aligned at either 4 or 8 bytes.

Since struct foo is normally aligned to 4 bytes, a warning will be issued:

warning: alignment 4 of 'struct foo' is less than 8

Align struct foo to 8 bytes:

struct foo
{
  int i1;
  int i2;
  __u64 x;
} __attribute__((aligned(8)));

silences the warning.  It also warns the field with misaligned offset:

struct foo
{
  int i1;
  int i2;
  int i3;
  __u64 x;
} __attribute__((aligned(8)));

warning: 'x' offset 12 in 'struct foo' isn't aligned to 8

This warning is controlled by -Wif-not-aligned and is enabled by default.

When -Wpacked-not-aligned is used, the same warning is also issued for
the field with explicitly specified alignment in a packed struct or union:

struct __attribute__ ((aligned (8))) S8 { char a[8]; };
struct __attribute__ ((packed)) S {
  struct S8 s8;
};

warning: alignment 1 of 'struct S' is less than 8

This warning is disabled by default and enabled by -Wall.

gcc/

PR c/53037
* print-tree.c (print_node): Support DECL_WARN_IF_NOT_ALIGN
and TYPE_WARN_IF_NOT_ALIGN.
* stor-layout.c (do_type_align): Merge DECL_WARN_IF_NOT_ALIGN.
(handle_warn_if_not_align): New.
(place_union_field): Call handle_warn_if_not_align.
(place_field): Call handle_warn_if_not_align.  Copy
TYPE_WARN_IF_NOT_ALIGN.
(finish_builtin_struct): Copy TYPE_WARN_IF_NOT_ALIGN.
(layout_type): Likewise.
* tree-core.h (tree_type_common): Add warn_if_not_align.  Set
spare to 18.
(tree_decl_common): Add warn_if_not_align.
* tree.c (build_range_type_1): Copy TYPE_WARN_IF_NOT_ALIGN.
* tree.h (TYPE_WARN_IF_NOT_ALIGN): New.
(SET_TYPE_WARN_IF_NOT_ALIGN): Likewise.
(DECL_WARN_IF_NOT_ALIGN): Likewise.
(SET_DECL_WARN_IF_NOT_ALIGN): Likewise.
* doc/extend.texi: Document warn_if_not_aligned attribute.
* doc/invoke.texi: Document -Wif-not-aligned and
-Wpacked-not-aligned.

gcc/c-family/

PR c/53037
* c-attribs.c (handle_warn_if_not_aligned_attribute): New.
(c_common_attribute_table): Add warn_if_not_aligned.
(handle_aligned_attribute): Renamed to ...
(common_handle_aligned_attribute): Remove argument, name, and add
argument, warn_if_not_aligned.  Handle warn_if_not_aligned.
(handle_aligned_attribute): New.
* c.opt: Add -Wif-not-aligned and -Wpacked-not-aligned.

gcc/c/

PR c/53037
* c-decl.c (merge_decls): Also merge DECL_WARN_IF_NOT_ALIGN.
(check_bitfield_type_and_width): Don't allow bit-field with
warn_if_not_aligned type.

gcc/cp/

PR c/53037
* decl.c (duplicate_decls): Also merge DECL_WARN_IF_NOT_ALIGN.
* decl2.c (grokbitfield): Don't allow bit-field with
warn_if_not_aligned type.

gcc/testsuite/

PR c/53037
* c-c++-common/pr53037-5.c: New test.
* g++.dg/pr53037-1.C: Likewise.
* g++.dg/pr53037-2.C: Likewise.
* g++.dg/pr53037-3.C: Likewise.
* g++.dg/pr53037-4.C: Likewise.
* gcc.dg/pr53037-1.c: Likewise.
* gcc.dg/pr53037-2.c: Likewise.
* gcc.dg/pr53037-3.c: Likewise.
* gcc.dg/pr53037-4.c: Likewise.

Added:
branches/range-gen2/gcc/testsuite/c-c++-common/pr53037-5.c
branches/range-gen2/gcc/testsuite/g++.dg/pr53037-1.C
branches/range-gen2/gcc/testsuite/g++.dg/pr53037-2.C
branches/range-gen2/gcc/testsuite/g++.dg/pr53037-3.C
branches/range-gen2/gcc/testsuite/g++.dg/pr53037-4.C
branches/range-gen2/gcc/testsuite/gcc.dg/pr53037-1.c
branches/range-gen2/gcc/testsuite/gcc.dg/pr53037-2.c
branches/range-gen2/gcc/testsuite/gcc.dg/pr53037-3.c
branches/range-gen2/gcc/testsuite/gcc.dg/pr53037-4.c
Modified:
branches/range-gen2/gcc/ChangeLog
branches/range-gen2/gcc/c-family/ChangeLog
branches/range-gen2/gcc/c-family/c-attribs.c
branches/range-gen2/gcc/c-family/c.opt
branches/range-gen2/gcc/c/ChangeLog
branches/range-gen2/gcc/c/c-decl.c
branches/range-gen2/gcc/cp/ChangeLog
branches/range-gen2/gcc/cp/decl.c
branches/range-gen2/gcc/cp/decl2.c

[Bug c/53037] warn_if_not_aligned(X)

[danglin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

John David Anglin  changed:

   What|Removed |Added

 CC||danglin at gcc dot gnu.org

--- Comment #40 from John David Anglin  ---
(In reply to Rainer Orth from comment #26)
> Several of the new tests FAIL on Solaris/SPARC (both 32 and 64-bit):
> 
> +FAIL: g++.dg/pr53037-2.C  -std=gnu++11  (test for warnings, line 16)
> +FAIL: g++.dg/pr53037-2.C  -std=gnu++11  (test for warnings, line 29)
> +FAIL: g++.dg/pr53037-2.C  -std=gnu++11  (test for warnings, line 6)
> +FAIL: g++.dg/pr53037-2.C  -std=gnu++14  (test for warnings, line 16)
> +FAIL: g++.dg/pr53037-2.C  -std=gnu++14  (test for warnings, line 29)
> +FAIL: g++.dg/pr53037-2.C  -std=gnu++14  (test for warnings, line 6)
> +FAIL: g++.dg/pr53037-2.C  -std=gnu++98  (test for warnings, line 16)
> +FAIL: g++.dg/pr53037-2.C  -std=gnu++98  (test for warnings, line 29)
> +FAIL: g++.dg/pr53037-2.C  -std=gnu++98  (test for warnings, line 6)
> +FAIL: g++.dg/pr53037-3.C  -std=gnu++11  (test for warnings, line 16)
> +FAIL: g++.dg/pr53037-3.C  -std=gnu++11  (test for warnings, line 29)
> +FAIL: g++.dg/pr53037-3.C  -std=gnu++11  (test for warnings, line 6)
> +FAIL: g++.dg/pr53037-3.C  -std=gnu++14  (test for warnings, line 16)
> +FAIL: g++.dg/pr53037-3.C  -std=gnu++14  (test for warnings, line 29)
> +FAIL: g++.dg/pr53037-3.C  -std=gnu++14  (test for warnings, line 6)
> +FAIL: g++.dg/pr53037-3.C  -std=gnu++98  (test for warnings, line 16)
> +FAIL: g++.dg/pr53037-3.C  -std=gnu++98  (test for warnings, line 29)
> +FAIL: g++.dg/pr53037-3.C  -std=gnu++98  (test for warnings, line 6)
> 
> +FAIL: gcc.dg/pr53037-2.c  (test for warnings, line 16)
> +FAIL: gcc.dg/pr53037-2.c  (test for warnings, line 32)
> +FAIL: gcc.dg/pr53037-2.c  (test for warnings, line 8)
> +FAIL: gcc.dg/pr53037-3.c  (test for warnings, line 16)
> +FAIL: gcc.dg/pr53037-3.c  (test for warnings, line 32)
> +FAIL: gcc.dg/pr53037-3.c  (test for warnings, line 8)
> 
>   Rainer

Similar fails occur on hppa.

[Bug c/53037] warn_if_not_aligned(X)

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #39 from Eric Botcazou  ---
> i don't quite understand this reasoning, i would
> not expect the internal STRICT_ALIGNMENT setting
> to change how types behave (e.g. it might mean
> some code errors out, but the semantics of packed
> and aligned attributes are not changed).

That's not what I said.  I only described how modes work on strict-alignment
targets: modes have alignment and you cannot have a type with a mode and the
type's alignment be lower than the mode's alignment.

> i don't see this on arm/sparc, instead i see wrong
> alignment given:
> 
> struct __attribute__ ((aligned (8))) S8 { char a[8]; };
> struct __attribute__ ((packed)) S1 { struct S8 s8; };
> 
> char a[_Alignof(struct S8)] = {0};
> char b[_Alignof(struct S1)] = {0};
> 
> compiles into a size 8 array for a and size 1 array for b,
> so _Alignof is inconsistent, which i'd expect to be an
> error (or warning at least).

I don't see any inconsistency here, __attribute__ ((packed)) is expected to
lower the alignment and takes precedence.  Whether or not this is the best
possible implementation is debatable, but it's the historical one.  We might
indeed want to warn about that, but the proposed patch is IMO not correct.

Would this patch be sufficient for:

struct __attribute__ ((aligned (4))) S8 { char a[8]; };
struct __attribute__ ((packed)) S1 { struct S8 s8; };

for example?

[Bug c/53037] warn_if_not_aligned(X)

[nsz at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

nsz at gcc dot gnu.org changed:

   What|Removed |Added

 CC||nsz at gcc dot gnu.org

--- Comment #38 from nsz at gcc dot gnu.org ---
(In reply to Eric Botcazou from comment #32)
> > Sparc defines STRICT_ALIGNMENT which leads to
> > 
> >   unsigned mode_align = GET_MODE_ALIGNMENT (TYPE_MODE (type));
> > 
> >   /* Don't override a larger alignment requirement coming from a user
> >  alignment of one of the fields.  */
> >   if (mode_align >= TYPE_ALIGN (type))
> > {
> >   SET_TYPE_ALIGN (type, mode_align);
> >   TYPE_USER_ALIGN (type) = 0; 
> > }
> > 
> > so __attribute__ ((packed)) is basically ignored on Sparc.
> 
> I don't think that's correct.  Simply, on strict-alignment targets, you
> cannot have an aggregate type less aligned than its scalar mode, if any; for
> other targets, that's only true for scalar types.  But you can have an
> aggregate type with alignment 1 if it has BLKmode.
> 

i don't quite understand this reasoning, i would
not expect the internal STRICT_ALIGNMENT setting
to change how types behave (e.g. it might mean
some code errors out, but the semantics of packed
and aligned attributes are not changed).

> > diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c
> > index 3028d55773a..6dd605810ac 100644
> > --- a/gcc/stor-layout.c
> > +++ b/gcc/stor-layout.c
> > @@ -1784,7 +1784,7 @@ finalize_type_size (tree type)
> >  
> >/* Don't override a larger alignment requirement coming from a user
> > alignment of one of the fields.  */
> > -  if (mode_align >= TYPE_ALIGN (type))
> > +  if (mode_align > TYPE_ALIGN (type))
> >{
> >  SET_TYPE_ALIGN (type, mode_align);
> >  TYPE_USER_ALIGN (type) = 0;
> > 
> > works with cross compiler.
> 
> The existing code works as intended: if the alignment given by the mode is
> larger than or equal to the type's alignment, then this alignment given by
> the mode becomes the natural alignment and TYPE_USER_ALIGN becomes
> obsolete/wrong.
> 

i don't see this on arm/sparc, instead i see wrong
alignment given:

struct __attribute__ ((aligned (8))) S8 { char a[8]; };
struct __attribute__ ((packed)) S1 { struct S8 s8; };

char a[_Alignof(struct S8)] = {0};
char b[_Alignof(struct S1)] = {0};

compiles into a size 8 array for a and size 1 array for b,
so _Alignof is inconsistent, which i'd expect to be an
error (or warning at least).

> So I think that the absence of warning is correct on strict-alignment
> platforms and that, if you want to make the tests portable, then you must
> use structures whose rounded size is not a power of 2 or is larger than 128
> bits, so that they don't get a scalar mode.

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #37 from H.J. Lu  ---
(In reply to Andreas Schwab from comment #35)
> On m68k:
> 
> FAIL: gcc.dg/pr53037-1.c  (test for warnings, line 42)
> FAIL: gcc.dg/pr53037-1.c  (test for warnings, line 75)
> FAIL: gcc.dg/pr53037-1.c (test for excess errors)
> Excess errors:
> /daten/aranym/gcc/gcc-20170821/gcc/testsuite/gcc.dg/pr53037-1.c:42:1:
> warning: alignment 2 of 'struct foo5' is less than 16 [-Wif-not-aligned]
> /daten/aranym/gcc/gcc-20170821/gcc/testsuite/gcc.dg/pr53037-1.c:75:1:
> warning: alignment 2 of 'union bar3' is less than 16 [-Wif-not-aligned]
> 
> FAIL: g++.dg/pr53037-1.C  -std=gnu++98  (test for warnings, line 38)
> FAIL: g++.dg/pr53037-1.C  -std=gnu++98  (test for warnings, line 71)
> FAIL: g++.dg/pr53037-1.C  -std=gnu++98 (test for excess errors)
> Excess errors:
> /daten/aranym/gcc/gcc-20170821/gcc/testsuite/g++.dg/pr53037-1.C:38:8:
> warning: alignment 2 of 'foo5' is less than 16 [-Wif-not-aligned]
> /daten/aranym/gcc/gcc-20170821/gcc/testsuite/g++.dg/pr53037-1.C:71:7:
> warning: alignment 2 of 'bar3' is less than 16 [-Wif-not-aligned]

A patch is posted at

https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01193.html

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #36 from H.J. Lu  ---
(In reply to Andreas Schwab from comment #34)
> On ia64:
> 
> FAIL: g++.dg/pr53037-4.C  -std=gnu++11 (test for excess errors)
> Excess errors:
> /usr/local/gcc/gcc-20170821/gcc/testsuite/g++.dg/pr53037-4.C:9:1: error:
> alignment for 'void foo2()' must be at least 16

A patch is posted at

https://gcc.gnu.org/ml/gcc-patches/2017-08/msg01192.html

[Bug c/53037] warn_if_not_aligned(X)

[sch...@linux-m68k.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #35 from Andreas Schwab  ---
On m68k:

FAIL: gcc.dg/pr53037-1.c  (test for warnings, line 42)
FAIL: gcc.dg/pr53037-1.c  (test for warnings, line 75)
FAIL: gcc.dg/pr53037-1.c (test for excess errors)
Excess errors:
/daten/aranym/gcc/gcc-20170821/gcc/testsuite/gcc.dg/pr53037-1.c:42:1: warning:
alignment 2 of 'struct foo5' is less than 16 [-Wif-not-aligned]
/daten/aranym/gcc/gcc-20170821/gcc/testsuite/gcc.dg/pr53037-1.c:75:1: warning:
alignment 2 of 'union bar3' is less than 16 [-Wif-not-aligned]

FAIL: g++.dg/pr53037-1.C  -std=gnu++98  (test for warnings, line 38)
FAIL: g++.dg/pr53037-1.C  -std=gnu++98  (test for warnings, line 71)
FAIL: g++.dg/pr53037-1.C  -std=gnu++98 (test for excess errors)
Excess errors:
/daten/aranym/gcc/gcc-20170821/gcc/testsuite/g++.dg/pr53037-1.C:38:8: warning:
alignment 2 of 'foo5' is less than 16 [-Wif-not-aligned]
/daten/aranym/gcc/gcc-20170821/gcc/testsuite/g++.dg/pr53037-1.C:71:7: warning:
alignment 2 of 'bar3' is less than 16 [-Wif-not-aligned]

[Bug c/53037] warn_if_not_aligned(X)

[sch...@linux-m68k.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #34 from Andreas Schwab  ---
On ia64:

FAIL: g++.dg/pr53037-4.C  -std=gnu++11 (test for excess errors)
Excess errors:
/usr/local/gcc/gcc-20170821/gcc/testsuite/g++.dg/pr53037-4.C:9:1: error:
alignment for 'void foo2()' must be at least 16

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #33 from H.J. Lu  ---
(In reply to Eric Botcazou from comment #32)
> > Sparc defines STRICT_ALIGNMENT which leads to
> > 
> >   unsigned mode_align = GET_MODE_ALIGNMENT (TYPE_MODE (type));
> > 
> >   /* Don't override a larger alignment requirement coming from a user
> >  alignment of one of the fields.  */
> >   if (mode_align >= TYPE_ALIGN (type))
> > {
> >   SET_TYPE_ALIGN (type, mode_align);
> >   TYPE_USER_ALIGN (type) = 0; 
> > }
> > 
> > so __attribute__ ((packed)) is basically ignored on Sparc.
> 
> I don't think that's correct.  Simply, on strict-alignment targets, you
> cannot have an aggregate type less aligned than its scalar mode, if any; for
> other targets, that's only true for scalar types.  But you can have an
> aggregate type with alignment 1 if it has BLKmode.
> 
> > diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c
> > index 3028d55773a..6dd605810ac 100644
> > --- a/gcc/stor-layout.c
> > +++ b/gcc/stor-layout.c
> > @@ -1784,7 +1784,7 @@ finalize_type_size (tree type)
> >  
> >/* Don't override a larger alignment requirement coming from a user
> > alignment of one of the fields.  */
> > -  if (mode_align >= TYPE_ALIGN (type))
> > +  if (mode_align > TYPE_ALIGN (type))
> >{
> >  SET_TYPE_ALIGN (type, mode_align);
> >  TYPE_USER_ALIGN (type) = 0;
> > 
> > works with cross compiler.
> 
> The existing code works as intended: if the alignment given by the mode is
> larger than or equal to the type's alignment, then this alignment given by
> the mode becomes the natural alignment and TYPE_USER_ALIGN becomes
> obsolete/wrong.

We should add a testcase to show there is a difference in generated
code on Sparc.

[Bug c/53037] warn_if_not_aligned(X)

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #32 from Eric Botcazou  ---
> Sparc defines STRICT_ALIGNMENT which leads to
> 
>   unsigned mode_align = GET_MODE_ALIGNMENT (TYPE_MODE (type));
> 
>   /* Don't override a larger alignment requirement coming from a user
>  alignment of one of the fields.  */
>   if (mode_align >= TYPE_ALIGN (type))
> {
>   SET_TYPE_ALIGN (type, mode_align);
>   TYPE_USER_ALIGN (type) = 0; 
> }
> 
> so __attribute__ ((packed)) is basically ignored on Sparc.

I don't think that's correct.  Simply, on strict-alignment targets, you cannot
have an aggregate type less aligned than its scalar mode, if any; for other
targets, that's only true for scalar types.  But you can have an aggregate type
with alignment 1 if it has BLKmode.

> diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c
> index 3028d55773a..6dd605810ac 100644
> --- a/gcc/stor-layout.c
> +++ b/gcc/stor-layout.c
> @@ -1784,7 +1784,7 @@ finalize_type_size (tree type)
>  
>/* Don't override a larger alignment requirement coming from a user
> alignment of one of the fields.  */
> -  if (mode_align >= TYPE_ALIGN (type))
> +  if (mode_align > TYPE_ALIGN (type))
>{
>  SET_TYPE_ALIGN (type, mode_align);
>  TYPE_USER_ALIGN (type) = 0;
> 
> works with cross compiler.

The existing code works as intended: if the alignment given by the mode is
larger than or equal to the type's alignment, then this alignment given by the
mode becomes the natural alignment and TYPE_USER_ALIGN becomes obsolete/wrong.

So I think that the absence of warning is correct on strict-alignment platforms
and that, if you want to make the tests portable, then you must use structures
whose rounded size is not a power of 2 or is larger than 128 bits, so that they
don't get a scalar mode.

[Bug c/53037] warn_if_not_aligned(X)

[ro at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

Rainer Orth  changed:

   What|Removed |Added

 CC||ebotcazou at gcc dot gnu.org

--- Comment #31 from Rainer Orth  ---
(In reply to H.J. Lu from comment #29)
> This patch
> 
> diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c
> index 3028d55773a..6dd605810ac 100644
> --- a/gcc/stor-layout.c
> +++ b/gcc/stor-layout.c
> @@ -1784,7 +1784,7 @@ finalize_type_size (tree type)
>  
>/* Don't override a larger alignment requirement coming from a user
> alignment of one of the fields.  */
> -  if (mode_align >= TYPE_ALIGN (type))
> +  if (mode_align > TYPE_ALIGN (type))
>{
>  SET_TYPE_ALIGN (type, mode_align);
>  TYPE_USER_ALIGN (type) = 0;
> 
> works with cross compiler.  But I have no idea if it is correct.

I've included it in a sparc bootstrap and the only effect on testresults was
to fix the set of failures I've reported.

Let's ask Eric for his opinion.

  Rainer

[Bug c/53037] warn_if_not_aligned(X)

[hpa at zytor dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #30 from H. Peter Anvin  ---
On August 18, 2017 3:52:12 PM CDT, "hjl.tools at gmail dot com"
 wrote:
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037
>
>--- Comment #29 from H.J. Lu  ---
>(In reply to r...@cebitec.uni-bielefeld.de from comment #28)
>> > --- Comment #27 from H.J. Lu  ---
>> 
>> > What are error messages?
>> 
>> None, the warnings are simply missing.
>> 
>>  Rainer
>
>Sparc defines STRICT_ALIGNMENT which leads to
>
>  unsigned mode_align = GET_MODE_ALIGNMENT (TYPE_MODE (type));
>
>/* Don't override a larger alignment requirement coming from a user
> alignment of one of the fields.  */
>  if (mode_align >= TYPE_ALIGN (type))
>{
>  SET_TYPE_ALIGN (type, mode_align);
>  TYPE_USER_ALIGN (type) = 0; 
>}
>
>so __attribute__ ((packed)) is basically ignored on Sparc.  This patch
>
>diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c
>index 3028d55773a..6dd605810ac 100644
>--- a/gcc/stor-layout.c
>+++ b/gcc/stor-layout.c
>@@ -1784,7 +1784,7 @@ finalize_type_size (tree type)
>
>/* Don't override a larger alignment requirement coming from a user
>alignment of one of the fields.  */
>-  if (mode_align >= TYPE_ALIGN (type))
>+  if (mode_align > TYPE_ALIGN (type))
>   {
> SET_TYPE_ALIGN (type, mode_align);
> TYPE_USER_ALIGN (type) = 0;
>
>works with cross compiler.  But I have no idea if it is correct.

Ignoring __attribute__((packed)) I would consider to be a super serious error.

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #29 from H.J. Lu  ---
(In reply to r...@cebitec.uni-bielefeld.de from comment #28)
> > --- Comment #27 from H.J. Lu  ---
> 
> > What are error messages?
> 
> None, the warnings are simply missing.
> 
>   Rainer

Sparc defines STRICT_ALIGNMENT which leads to

  unsigned mode_align = GET_MODE_ALIGNMENT (TYPE_MODE (type));

  /* Don't override a larger alignment requirement coming from a user
 alignment of one of the fields.  */
  if (mode_align >= TYPE_ALIGN (type))
{
  SET_TYPE_ALIGN (type, mode_align);
  TYPE_USER_ALIGN (type) = 0; 
}

so __attribute__ ((packed)) is basically ignored on Sparc.  This patch

diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c
index 3028d55773a..6dd605810ac 100644
--- a/gcc/stor-layout.c
+++ b/gcc/stor-layout.c
@@ -1784,7 +1784,7 @@ finalize_type_size (tree type)

   /* Don't override a larger alignment requirement coming from a user
alignment of one of the fields.  */
-  if (mode_align >= TYPE_ALIGN (type))
+  if (mode_align > TYPE_ALIGN (type))
   {
 SET_TYPE_ALIGN (type, mode_align);
 TYPE_USER_ALIGN (type) = 0;

works with cross compiler.  But I have no idea if it is correct.

[Bug c/53037] warn_if_not_aligned(X)

[ro at CeBiTec dot Uni-Bielefeld.DE]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #28 from ro at CeBiTec dot Uni-Bielefeld.DE  ---
> --- Comment #27 from H.J. Lu  ---

> What are error messages?

None, the warnings are simply missing.

Rainer

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #27 from H.J. Lu  ---
(In reply to Rainer Orth from comment #26)
> Several of the new tests FAIL on Solaris/SPARC (both 32 and 64-bit):
> 
> +FAIL: g++.dg/pr53037-2.C  -std=gnu++11  (test for warnings, line 16)
> +FAIL: g++.dg/pr53037-2.C  -std=gnu++11  (test for warnings, line 29)
> +FAIL: g++.dg/pr53037-2.C  -std=gnu++11  (test for warnings, line 6)
> +FAIL: g++.dg/pr53037-2.C  -std=gnu++14  (test for warnings, line 16)
> +FAIL: g++.dg/pr53037-2.C  -std=gnu++14  (test for warnings, line 29)
> +FAIL: g++.dg/pr53037-2.C  -std=gnu++14  (test for warnings, line 6)
> +FAIL: g++.dg/pr53037-2.C  -std=gnu++98  (test for warnings, line 16)
> +FAIL: g++.dg/pr53037-2.C  -std=gnu++98  (test for warnings, line 29)
> +FAIL: g++.dg/pr53037-2.C  -std=gnu++98  (test for warnings, line 6)
> +FAIL: g++.dg/pr53037-3.C  -std=gnu++11  (test for warnings, line 16)
> +FAIL: g++.dg/pr53037-3.C  -std=gnu++11  (test for warnings, line 29)
> +FAIL: g++.dg/pr53037-3.C  -std=gnu++11  (test for warnings, line 6)
> +FAIL: g++.dg/pr53037-3.C  -std=gnu++14  (test for warnings, line 16)
> +FAIL: g++.dg/pr53037-3.C  -std=gnu++14  (test for warnings, line 29)
> +FAIL: g++.dg/pr53037-3.C  -std=gnu++14  (test for warnings, line 6)
> +FAIL: g++.dg/pr53037-3.C  -std=gnu++98  (test for warnings, line 16)
> +FAIL: g++.dg/pr53037-3.C  -std=gnu++98  (test for warnings, line 29)
> +FAIL: g++.dg/pr53037-3.C  -std=gnu++98  (test for warnings, line 6)
> 
> +FAIL: gcc.dg/pr53037-2.c  (test for warnings, line 16)
> +FAIL: gcc.dg/pr53037-2.c  (test for warnings, line 32)
> +FAIL: gcc.dg/pr53037-2.c  (test for warnings, line 8)
> +FAIL: gcc.dg/pr53037-3.c  (test for warnings, line 16)
> +FAIL: gcc.dg/pr53037-3.c  (test for warnings, line 32)
> +FAIL: gcc.dg/pr53037-3.c  (test for warnings, line 8)
> 

What are error messages?

[Bug c/53037] warn_if_not_aligned(X)

[ro at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

Rainer Orth  changed:

   What|Removed |Added

 CC||ro at gcc dot gnu.org

--- Comment #26 from Rainer Orth  ---
Several of the new tests FAIL on Solaris/SPARC (both 32 and 64-bit):

+FAIL: g++.dg/pr53037-2.C  -std=gnu++11  (test for warnings, line 16)
+FAIL: g++.dg/pr53037-2.C  -std=gnu++11  (test for warnings, line 29)
+FAIL: g++.dg/pr53037-2.C  -std=gnu++11  (test for warnings, line 6)
+FAIL: g++.dg/pr53037-2.C  -std=gnu++14  (test for warnings, line 16)
+FAIL: g++.dg/pr53037-2.C  -std=gnu++14  (test for warnings, line 29)
+FAIL: g++.dg/pr53037-2.C  -std=gnu++14  (test for warnings, line 6)
+FAIL: g++.dg/pr53037-2.C  -std=gnu++98  (test for warnings, line 16)
+FAIL: g++.dg/pr53037-2.C  -std=gnu++98  (test for warnings, line 29)
+FAIL: g++.dg/pr53037-2.C  -std=gnu++98  (test for warnings, line 6)
+FAIL: g++.dg/pr53037-3.C  -std=gnu++11  (test for warnings, line 16)
+FAIL: g++.dg/pr53037-3.C  -std=gnu++11  (test for warnings, line 29)
+FAIL: g++.dg/pr53037-3.C  -std=gnu++11  (test for warnings, line 6)
+FAIL: g++.dg/pr53037-3.C  -std=gnu++14  (test for warnings, line 16)
+FAIL: g++.dg/pr53037-3.C  -std=gnu++14  (test for warnings, line 29)
+FAIL: g++.dg/pr53037-3.C  -std=gnu++14  (test for warnings, line 6)
+FAIL: g++.dg/pr53037-3.C  -std=gnu++98  (test for warnings, line 16)
+FAIL: g++.dg/pr53037-3.C  -std=gnu++98  (test for warnings, line 29)
+FAIL: g++.dg/pr53037-3.C  -std=gnu++98  (test for warnings, line 6)

+FAIL: gcc.dg/pr53037-2.c  (test for warnings, line 16)
+FAIL: gcc.dg/pr53037-2.c  (test for warnings, line 32)
+FAIL: gcc.dg/pr53037-2.c  (test for warnings, line 8)
+FAIL: gcc.dg/pr53037-3.c  (test for warnings, line 16)
+FAIL: gcc.dg/pr53037-3.c  (test for warnings, line 32)
+FAIL: gcc.dg/pr53037-3.c  (test for warnings, line 8)

  Rainer

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

H.J. Lu  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED
   Target Milestone|--- |8.0

--- Comment #25 from H.J. Lu  ---
Fixed.

[Bug c/53037] warn_if_not_aligned(X)

[hjl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #24 from hjl at gcc dot gnu.org  ---
Author: hjl
Date: Fri Aug 18 09:38:38 2017
New Revision: 251180

URL: https://gcc.gnu.org/viewcvs?rev=251180=gcc=rev
Log:
Add warn_if_not_aligned attribute

Add warn_if_not_aligned attribute as well as  command line options:
-Wif-not-aligned and -Wpacked-not-aligned.

__attribute__((warn_if_not_aligned(N))) causes compiler to issue a
warning if the field in a struct or union is not aligned to N:

typedef unsigned long long __u64
  __attribute__((aligned(4),warn_if_not_aligned(8)));

struct foo
{
  int i1;
  int i2;
  __u64 x;
};

__u64 is aligned to 4 bytes.  But inside struct foo, __u64 should be
aligned at 8 bytes.  It is used to define struct foo in such a way that
struct foo has the same layout and x has the same alignment when __u64
is aligned at either 4 or 8 bytes.

Since struct foo is normally aligned to 4 bytes, a warning will be issued:

warning: alignment 4 of 'struct foo' is less than 8

Align struct foo to 8 bytes:

struct foo
{
  int i1;
  int i2;
  __u64 x;
} __attribute__((aligned(8)));

silences the warning.  It also warns the field with misaligned offset:

struct foo
{
  int i1;
  int i2;
  int i3;
  __u64 x;
} __attribute__((aligned(8)));

warning: 'x' offset 12 in 'struct foo' isn't aligned to 8

This warning is controlled by -Wif-not-aligned and is enabled by default.

When -Wpacked-not-aligned is used, the same warning is also issued for
the field with explicitly specified alignment in a packed struct or union:

struct __attribute__ ((aligned (8))) S8 { char a[8]; };
struct __attribute__ ((packed)) S {
  struct S8 s8;
};

warning: alignment 1 of 'struct S' is less than 8

This warning is disabled by default and enabled by -Wall.

gcc/

PR c/53037
* print-tree.c (print_node): Support DECL_WARN_IF_NOT_ALIGN
and TYPE_WARN_IF_NOT_ALIGN.
* stor-layout.c (do_type_align): Merge DECL_WARN_IF_NOT_ALIGN.
(handle_warn_if_not_align): New.
(place_union_field): Call handle_warn_if_not_align.
(place_field): Call handle_warn_if_not_align.  Copy
TYPE_WARN_IF_NOT_ALIGN.
(finish_builtin_struct): Copy TYPE_WARN_IF_NOT_ALIGN.
(layout_type): Likewise.
* tree-core.h (tree_type_common): Add warn_if_not_align.  Set
spare to 18.
(tree_decl_common): Add warn_if_not_align.
* tree.c (build_range_type_1): Copy TYPE_WARN_IF_NOT_ALIGN.
* tree.h (TYPE_WARN_IF_NOT_ALIGN): New.
(SET_TYPE_WARN_IF_NOT_ALIGN): Likewise.
(DECL_WARN_IF_NOT_ALIGN): Likewise.
(SET_DECL_WARN_IF_NOT_ALIGN): Likewise.
* doc/extend.texi: Document warn_if_not_aligned attribute.
* doc/invoke.texi: Document -Wif-not-aligned and
-Wpacked-not-aligned.

gcc/c-family/

PR c/53037
* c-attribs.c (handle_warn_if_not_aligned_attribute): New.
(c_common_attribute_table): Add warn_if_not_aligned.
(handle_aligned_attribute): Renamed to ...
(common_handle_aligned_attribute): Remove argument, name, and add
argument, warn_if_not_aligned.  Handle warn_if_not_aligned.
(handle_aligned_attribute): New.
* c.opt: Add -Wif-not-aligned and -Wpacked-not-aligned.

gcc/c/

PR c/53037
* c-decl.c (merge_decls): Also merge DECL_WARN_IF_NOT_ALIGN.
(check_bitfield_type_and_width): Don't allow bit-field with
warn_if_not_aligned type.

gcc/cp/

PR c/53037
* decl.c (duplicate_decls): Also merge DECL_WARN_IF_NOT_ALIGN.
* decl2.c (grokbitfield): Don't allow bit-field with
warn_if_not_aligned type.

gcc/testsuite/

PR c/53037
* c-c++-common/pr53037-5.c: New test.
* g++.dg/pr53037-1.C: Likewise.
* g++.dg/pr53037-2.C: Likewise.
* g++.dg/pr53037-3.C: Likewise.
* g++.dg/pr53037-4.C: Likewise.
* gcc.dg/pr53037-1.c: Likewise.
* gcc.dg/pr53037-2.c: Likewise.
* gcc.dg/pr53037-3.c: Likewise.
* gcc.dg/pr53037-4.c: Likewise.

Added:
trunk/gcc/testsuite/c-c++-common/pr53037-5.c
trunk/gcc/testsuite/g++.dg/pr53037-1.C
trunk/gcc/testsuite/g++.dg/pr53037-2.C
trunk/gcc/testsuite/g++.dg/pr53037-3.C
trunk/gcc/testsuite/g++.dg/pr53037-4.C
trunk/gcc/testsuite/gcc.dg/pr53037-1.c
trunk/gcc/testsuite/gcc.dg/pr53037-2.c
trunk/gcc/testsuite/gcc.dg/pr53037-3.c
trunk/gcc/testsuite/gcc.dg/pr53037-4.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/c-family/ChangeLog
trunk/gcc/c-family/c-attribs.c
trunk/gcc/c-family/c.opt
trunk/gcc/c/ChangeLog
trunk/gcc/c/c-decl.c
trunk/gcc/cp/ChangeLog
trunk/gcc/cp/decl.c
trunk/gcc/cp/decl2.c
trunk/gcc/doc/extend.texi
trunk/gcc/doc/invoke.texi
trunk/gcc/print-tree.c
trunk/gcc/stor-layout.c
trunk/gcc/testsuite/ChangeLog
trunk/gcc/tree-core.h
trunk/gcc/tree.c
trunk/gcc/tree.h

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #23 from H.J. Lu  ---
(In reply to Martin Sebor from comment #22)
> The warning on the test case in comment #21 looks good to me.  Thanks! 
> 
> From reading comment #1 I'm not sure your patch does quite what you
> described there.  It doesn't warn on the declaration of the global object x
> below:
> 
> typedef unsigned long long __u64 __attribute__((aligned(4),
> warn_if_not_aligned(8)));
> 
> static char c;
> static __u64 x;
> 
> even though x is 4-byte aligned.  (It does warn when a __u64 member is
> declared in a packed struct.)

This is by design.  The current description is at:

https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00180.html

__attribute__((warn_if_not_aligned(N))) issues a warning if the field
in a struct or union is not aligned to N:

typedef unsigned long long __u64
  __attribute__((aligned(4),warn_if_not_aligned(8)));

struct foo
{
  int i1;
  int i2;
  __u64 x;
};

__u64 is aligned to 4 bytes.  But inside struct foo, __u64 should be
aligned at 8 bytes.  It is used to define struct foo in such a way that
struct foo has the same layout and x has the same alignment when __u64
is aligned at either 4 or 8 bytes.

Since struct foo is normally aligned to 4 bytes, a warning will be issued:

warning: alignment 4 of ‘struct foo’ is less than 8

Align struct foo to 8 bytes:

struct foo
{
  int i1;
  int i2;
  __u64 x;
} __attribute__((aligned(8)));

silences the warning.  It also warns the field with misaligned offset:

struct foo
{
  int i1;
  int i2;
  int i3;
  __u64 x;
} __attribute__((aligned(8)));

warning: ‘x’ offset 12 in ‘struct foo’ isn't aligned to 8

The same warning is also issued without the warn_if_not_aligned attribute
for the field with explicitly specified alignment in a packed struct or
union:

struct __attribute__ ((aligned (8))) S8 { char a[8]; };
struct __attribute__ ((packed)) S {
  struct S8 s8;
};

warning: alignment 1 of ‘struct S’ is less than 8

[Bug c/53037] warn_if_not_aligned(X)

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #22 from Martin Sebor  ---
The warning on the test case in comment #21 looks good to me.  Thanks! 

>From reading comment #1 I'm not sure your patch does quite what you described
there.  It doesn't warn on the declaration of the global object x below:

typedef unsigned long long __u64 __attribute__((aligned(4),
warn_if_not_aligned(8)));

static char c;
static __u64 x;

even though x is 4-byte aligned.  (It does warn when a __u64 member is declared
in a packed struct.)

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

H.J. Lu  changed:

   What|Removed |Added

  Attachment #27198|0   |1
is obsolete||
  Attachment #27199|0   |1
is obsolete||

--- Comment #21 from H.J. Lu  ---
Created attachment 41460
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=41460=edit
An updated patch

Please take a look at this updated patch against r248831. I got

[hjl@gnu-6 pr53037]$ cat z.i 
struct __attribute__ ((aligned (8))) S8 { char a[8]; };
struct __attribute__ ((packed)) S {
  struct S8 s8;
};
[hjl@gnu-6 pr53037]$ make z.s
/export/build/gnu/gcc-test/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/gcc-test/build-x86_64-linux/gcc/ -mx32 -Wall -O2 -S z.i
z.i:4:1: warning: alignment 1 of ‘struct S’ is less than 8
 };
 ^
[hjl@gnu-6 pr53037]$

[Bug c/53037] warn_if_not_aligned(X)

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #20 from Martin Sebor  ---
It would be useful to have the ability to trigger a warning when an object of a
type with an explicitly specified alignment (even if the alignment matches its
own native one) is defined in a way that would cause its alignment to be
reduced.  The specific use case where it came up is defining a packed struct
that contains a member of type std::mutex (or some type that contains it.)

A test case for the warning I was asking about (with no warn_if_not_aligned
attribute) is simply:

  struct __attribute___ ((aligned (8))) S8 { char a[8]; };   // explicitly
overaligned

  struct __attribute__ ((packed)) S {
struct S8 s8;   // warn here: alignment of 'struct A' reduced from 8 to 1
  };

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #19 from H.J. Lu  ---
(In reply to Martin Sebor from comment #18)
> This seems like a useful enhancement.  H.J., what is the status of the
> patch?  Do you have plans to submit it?

I was not sure if there were enough interests.  I need to rebase my code
to see if it still applies.

> On a related note, would warning on the use of a type explicitly decorated
> with a restrictive alignment attribute in an unaligned context achieve the
> same thing as the new attribute without false positives?  (That way existing
> code would automatically benefit from the new warning, without having to add
> a new attribute.)

A testcase?

[Bug c/53037] warn_if_not_aligned(X)

[msebor at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

Martin Sebor  changed:

   What|Removed |Added

   Keywords||diagnostic
 Status|UNCONFIRMED |NEW
   Last reconfirmed||2017-06-01
 CC||msebor at gcc dot gnu.org
 Ever confirmed|0   |1

--- Comment #18 from Martin Sebor  ---
This seems like a useful enhancement.  H.J., what is the status of the patch? 
Do you have plans to submit it?

On a related note, would warning on the use of a type explicitly decorated with
a restrictive alignment attribute in an unaligned context achieve the same
thing as the new attribute without false positives?  (That way existing code
would automatically benefit from the new warning, without having to add a new
attribute.)

[Bug c/53037] warn_if_not_aligned(X)

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037



Paolo Carlini paolo.carlini at oracle dot com changed:



   What|Removed |Added



 CC||crillion at tiscali dot it



--- Comment #17 from Paolo Carlini paolo.carlini at oracle dot com 2013-02-13 
10:38:44 UTC ---

*** Bug 56304 has been marked as a duplicate of this bug. ***

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #2 from H.J. Lu hjl.tools at gmail dot com 2012-04-19 15:47:14 
UTC ---
Given

typedef unsigned long long __u64 __attribute__((aligned(4)));

all most all __u64 will be aligned at 4.   The only case we may
do something about is

typedef unsigned long long __u64
__attribute__((aligned(4),warn_if_not_aligned(8)));

struct foo
{
  int i1;
  int i2;
  int i3;

  __u64 x;
};

since alignment of the x field also depends on alignment of
struct foo.

[Bug c/53037] warn_if_not_aligned(X)

[hpa at zytor dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #3 from H. Peter Anvin hpa at zytor dot com 2012-04-19 15:51:35 
UTC ---
Logically, about half of u64's will be properly aligned at the moment... Linus'
request is that we flag the currently misaligned __[su]64's as __compat_[su]64
and make __[su]64 aligned, so at least *new* interfaces will be properly
aligned.

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #4 from H.J. Lu hjl.tools at gmail dot com 2012-04-19 16:00:41 
UTC ---
(In reply to comment #3)
 Logically, about half of u64's will be properly aligned at the moment... 
 Linus'

No necessarily. For

u64 x;
int y;
u64 z;

both x and z may be 4 byte aligned.

 request is that we flag the currently misaligned __[su]64's as __compat_[su]64
 and make __[su]64 aligned, so at least *new* interfaces will be properly
 aligned.

Is this feature only used for function parameters?

[Bug c/53037] warn_if_not_aligned(X)

[hpa at zytor dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #5 from H. Peter Anvin hpa at zytor dot com 2012-04-19 16:05:29 
UTC ---
On 04/19/2012 09:00 AM, hjl.tools at gmail dot com wrote:
 
 request is that we flag the currently misaligned __[su]64's as 
 __compat_[su]64
 and make __[su]64 aligned, so at least *new* interfaces will be properly
 aligned.
 
 Is this feature only used for function parameters?
 

No.

-hpa

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #6 from H.J. Lu hjl.tools at gmail dot com 2012-04-19 16:53:18 
UTC ---
For a global or local 64bit variable, x, inside kernel,
why should it be 4 byte aligned if it isn't part of system
call interface?

[Bug c/53037] warn_if_not_aligned(X)

[hpa at zytor dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #7 from H. Peter Anvin hpa at zytor dot com 2012-04-19 16:57:14 
UTC ---
The __u64/__s64 types are used for interfaces only.  The kernel itself is
x86-64 and uses aligned types for internal uses.

The mismatch between i386 and x86-64 alignment has a tendency to cause
unexpected bugs, and Linus would like to avoid those by having new interfaces
use aligned types consistently.

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #8 from H.J. Lu hjl.tools at gmail dot com 2012-04-19 17:07:20 
UTC ---
Shouldn't

typedef unsigned long long __u64 __attribute__((aligned(4)));

only be used in system call interface?

[Bug c/53037] warn_if_not_aligned(X)

[hpa at zytor dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #9 from H. Peter Anvin hpa at zytor dot com 2012-04-19 17:11:00 
UTC ---
Yes.

The point is: WE WANT TO MIGRATE THE SYSTEM CALL INTERFACE TO AN ALIGNED
__[us]64 INTERFACE, mostly so that new interfaces are properly aligned from the
start.

In order to do that, we need to flag the existing legacy interfaces which will
need to be flagged as __compat_[us]64 in order to not break.

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #10 from H.J. Lu hjl.tools at gmail dot com 2012-04-19 17:20:42 
UTC ---
Isn't checking alignment of x in:

typedef unsigned long long __u64
__attribute__((aligned(4),warn_if_not_aligned(8)));

struct foo
{
  int i1;
  int i2;
  int i3;

  __u64 x;
};

sufficient for kernel interface purpose?  Is there another case
we need to check?

[Bug c/53037] warn_if_not_aligned(X)

[hpa at zytor dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #11 from H. Peter Anvin hpa at zytor dot com 2012-04-19 17:42:28 
UTC ---
Sorry, that should be sufficient.  I'm not awake today, it seems.

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #12 from H.J. Lu hjl.tools at gmail dot com 2012-04-19 20:15:55 
UTC ---
Created attachment 27197
  -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=27197
A patch

I got

[hjl@gnu-6 pr53037]$ cat x.i
typedef unsigned long long __u64
__attribute__((aligned(4),warn_if_not_aligned(8)));

struct foo1
{
  int i1;
  int i2;
  int i3;
  __u64 x;
};

struct foo2
{
  int i1;
  int i2;
  int i3;
  __u64 x;
} __attribute__((aligned(8)));

struct foo3
{
  int i1;
  int i2;
  __u64 x;
} __attribute__((aligned(8)));

struct foo4
{
  int i1;
  int i3;
  __u64 x;
};

union bar1
{
  int i1;
  int i3;
  __u64 x;
};

union bar2
{
  int i1;
  int i3;
  __u64 x;
} __attribute__((aligned(8)));
[hjl@gnu-6 pr53037]$ make x.s
/export/build/gnu/gcc/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/gcc/build-x86_64-linux/gcc/ -mx32 -Wall -S x.i
x.i:10:1: warning: alignment 32 of ‘struct foo1’ is less than 64 [enabled by
default]
 };
 ^
x.i:10:1: warning: ‘x’ offset 12 in ‘struct foo1’ isn't aligned to 8 [enabled
by default]
 };
 ^
x.i:18:1: warning: ‘x’ offset 12 in ‘struct foo2’ isn't aligned to 8 [enabled
by default]
 } __attribute__((aligned(8)));
 ^
x.i:32:1: warning: alignment 32 of ‘struct foo4’ is less than 64 [enabled by
default]
 };
 ^
x.i:39:1: warning: alignment 32 of ‘union bar1’ is less than 64 [enabled by
default]
 };
 ^
[hjl@gnu-6 pr53037]$

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

H.J. Lu hjl.tools at gmail dot com changed:

   What|Removed |Added

  Attachment #27197|0   |1
is obsolete||

--- Comment #13 from H.J. Lu hjl.tools at gmail dot com 2012-04-19 20:18:56 
UTC ---
Created attachment 27198
  -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=27198
A better patch

[Bug c/53037] warn_if_not_aligned(X)

[hpa at zytor dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #14 from H. Peter Anvin hpa at zytor dot com 2012-04-19 20:24:25 
UTC ---
Are the last two warnings in bits (as opposed to bytes)?  It looks a little
confusing...

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #15 from H.J. Lu hjl.tools at gmail dot com 2012-04-19 20:41:47 
UTC ---
(In reply to comment #14)
 Are the last two warnings in bits (as opposed to bytes)?  It looks a little
 confusing...

It is fixed by the updated patch.

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #16 from H.J. Lu hjl.tools at gmail dot com 2012-04-19 21:06:49 
UTC ---
Created attachment 27199
  -- http://gcc.gnu.org/bugzilla/attachment.cgi?id=27199
A smaller patch

There is no point to support

struct foo
{
  int i1;
  long long i2  __attribute__((aligned(4), warn_if_not_aligned(8)));
};

since GCC won't decrease alignment in field.  The smaller patch gave

[hjl@gnu-6 pr53037]$ cat x.i
typedef unsigned long long __u64
__attribute__((aligned(4),warn_if_not_aligned(8)));

struct foo1
{
  int i1;
  int i2;
  int i3;
  __u64 x;
};

struct foo2
{
  int i1;
  int i2;
  int i3;
  __u64 x;
} __attribute__((aligned(8)));

struct foo3
{
  int i1;
  int i2;
  __u64 x;
} __attribute__((aligned(8)));

struct foo4
{
  int i1;
  int i3;
  __u64 x;
};

union bar1
{
  int i1;
  int i3;
  __u64 x;
};

union bar2
{
  int i1;
  int i3;
  __u64 x;
} __attribute__((aligned(8)));
[hjl@gnu-6 pr53037]$ make x.s
/export/build/gnu/gcc/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/gcc/build-x86_64-linux/gcc/ -mx32 -Wall -O2 -S x.i
x.i:10:1: warning: alignment 4 of ‘struct foo1’ is less than 8 [enabled by
default]
 };
 ^
x.i:9:9: warning: ‘x’ offset 12 in ‘struct foo1’ isn't aligned to 8 [enabled by
default]
   __u64 x;
 ^
x.i:17:9: warning: ‘x’ offset 12 in ‘struct foo2’ isn't aligned to 8 [enabled
by default]
   __u64 x;
 ^
x.i:32:1: warning: alignment 4 of ‘struct foo4’ is less than 8 [enabled by
default]
 };
 ^
x.i:39:1: warning: alignment 4 of ‘union bar1’ is less than 8 [enabled by
default]
 };
 ^
[hjl@gnu-6 pr53037]$

[Bug c/53037] warn_if_not_aligned(X)

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037

--- Comment #1 from H.J. Lu hjl.tools at gmail dot com 2012-04-19 00:33:55 
UTC ---
We need to add another field to tree_type_common and tree_decl_common to
store the warn_if_not_aligned value.