Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-08-21 Thread H.J. Lu
On Mon, Aug 21, 2017 at 3:59 AM, Szabolcs Nagy  wrote:
> On 17/08/17 15:56, H.J. Lu wrote:
>> On Thu, Aug 17, 2017 at 6:52 AM, Joseph Myers  
>> wrote:
>>> On Sat, 8 Jul 2017, H.J. Lu wrote:
>>>
 +@item -Wpacked-not-aligned @r{(C, C++, Objective-C and Objective-C++ 
 only)}
 +@opindex Wpacked-not-aligned
 +@opindex Wno-packed-not-aligned
 +Warn if a structure field with explicitly specified alignment in a
 +packed struct or union is misaligned.  For example, a warning will
 +be issued on @code{struct S}, like, @code{warning: alignment 1 of
 +'struct S' is less than 8}, in this code:
>>>
>>> Use @samp for warnings quoted in the manual, as previously discussed.
>>>
>>> OK with that change, in the absence of C++ maintainer objections within 48
>>> hours.
>>>
>>
>> Here is the updated patch.  I moved c++ changes to merge_decls, where
>> alignment is merged,  and check_bitfield_type_and_width, where bit-fields
>> are checked.
>>
>> Tested on x86-64 and i686.
>>
>
> i assume packed semantics is same on arm so these
> should warn on arm too ?
>
> on arm i see:
>
> FAIL: gcc.dg/pr53037-2.c  (test for warnings, line 8)
> 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-3.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: g++.dg/pr53037-2.C  -std=gnu++98  (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++11  (test for warnings, line 6)
> 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++14  (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-3.C  -std=gnu++98  (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++11  (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++14  (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)
>

See:

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

-- 
H.J.


Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-08-21 Thread Szabolcs Nagy
On 17/08/17 15:56, H.J. Lu wrote:
> On Thu, Aug 17, 2017 at 6:52 AM, Joseph Myers  wrote:
>> On Sat, 8 Jul 2017, H.J. Lu wrote:
>>
>>> +@item -Wpacked-not-aligned @r{(C, C++, Objective-C and Objective-C++ only)}
>>> +@opindex Wpacked-not-aligned
>>> +@opindex Wno-packed-not-aligned
>>> +Warn if a structure field with explicitly specified alignment in a
>>> +packed struct or union is misaligned.  For example, a warning will
>>> +be issued on @code{struct S}, like, @code{warning: alignment 1 of
>>> +'struct S' is less than 8}, in this code:
>>
>> Use @samp for warnings quoted in the manual, as previously discussed.
>>
>> OK with that change, in the absence of C++ maintainer objections within 48
>> hours.
>>
> 
> Here is the updated patch.  I moved c++ changes to merge_decls, where
> alignment is merged,  and check_bitfield_type_and_width, where bit-fields
> are checked.
> 
> Tested on x86-64 and i686.
> 

i assume packed semantics is same on arm so these
should warn on arm too ?

on arm i see:

FAIL: gcc.dg/pr53037-2.c  (test for warnings, line 8)
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-3.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: g++.dg/pr53037-2.C  -std=gnu++98  (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++11  (test for warnings, line 6)
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++14  (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-3.C  -std=gnu++98  (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++11  (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++14  (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)



Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-08-17 Thread Jason Merrill
On Thu, Aug 17, 2017 at 7:56 AM, H.J. Lu  wrote:
> On Thu, Aug 17, 2017 at 6:52 AM, Joseph Myers  wrote:
>> On Sat, 8 Jul 2017, H.J. Lu wrote:
>>
>>> +@item -Wpacked-not-aligned @r{(C, C++, Objective-C and Objective-C++ only)}
>>> +@opindex Wpacked-not-aligned
>>> +@opindex Wno-packed-not-aligned
>>> +Warn if a structure field with explicitly specified alignment in a
>>> +packed struct or union is misaligned.  For example, a warning will
>>> +be issued on @code{struct S}, like, @code{warning: alignment 1 of
>>> +'struct S' is less than 8}, in this code:
>>
>> Use @samp for warnings quoted in the manual, as previously discussed.
>>
>> OK with that change, in the absence of C++ maintainer objections within 48
>> hours.
>>
>
> Here is the updated patch.  I moved c++ changes to merge_decls, where
> alignment is merged,  and check_bitfield_type_and_width, where bit-fields
> are checked.

OK.

Jason


Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-08-17 Thread H.J. Lu
On Thu, Aug 17, 2017 at 6:52 AM, Joseph Myers  wrote:
> On Sat, 8 Jul 2017, H.J. Lu wrote:
>
>> +@item -Wpacked-not-aligned @r{(C, C++, Objective-C and Objective-C++ only)}
>> +@opindex Wpacked-not-aligned
>> +@opindex Wno-packed-not-aligned
>> +Warn if a structure field with explicitly specified alignment in a
>> +packed struct or union is misaligned.  For example, a warning will
>> +be issued on @code{struct S}, like, @code{warning: alignment 1 of
>> +'struct S' is less than 8}, in this code:
>
> Use @samp for warnings quoted in the manual, as previously discussed.
>
> OK with that change, in the absence of C++ maintainer objections within 48
> hours.
>

Here is the updated patch.  I moved c++ changes to merge_decls, where
alignment is merged,  and check_bitfield_type_and_width, where bit-fields
are checked.

Tested on x86-64 and i686.


-- 
H.J.
From 441bd3479a83093ad46fbaa67d270d69808c05b2 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" 
Date: Fri, 20 Apr 2012 13:49:05 -0700
Subject: [PATCH] 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.
---
 gcc/c-family/c-attribs.c   | 71 +++
 gcc/c-family/c.opt |  8 
 gcc/c/c-decl.c | 11 +
 gcc/cp/decl.c  |  4 ++
 gcc/cp/decl2.c |  7 +++
 gcc/doc/extend.texi| 87 ++
 gcc/doc/invoke.texi| 29 +++-
 gcc/print-tree.c   |  9 ++--
 gcc/s

Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-08-17 Thread Joseph Myers
On Sat, 8 Jul 2017, H.J. Lu wrote:

> +@item -Wpacked-not-aligned @r{(C, C++, Objective-C and Objective-C++ only)}
> +@opindex Wpacked-not-aligned
> +@opindex Wno-packed-not-aligned
> +Warn if a structure field with explicitly specified alignment in a
> +packed struct or union is misaligned.  For example, a warning will
> +be issued on @code{struct S}, like, @code{warning: alignment 1 of
> +'struct S' is less than 8}, in this code:

Use @samp for warnings quoted in the manual, as previously discussed.

OK with that change, in the absence of C++ maintainer objections within 48 
hours.

-- 
Joseph S. Myers
jos...@codesourcery.com


Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-07-08 Thread H.J. Lu
On Thu, Jul 06, 2017 at 03:44:57PM +, Joseph Myers wrote:
> On Fri, 16 Jun 2017, H.J. Lu wrote:
> 
> > +@code{warning: alignment 8 of 'struct foo' is less than 16}.
> 
> I think @samp is better than @code for warnings, throughout, since they 
> aren't pieces of program code.

Done.

> 
> > +This warning can be disabled by @option{-Wno-if-not-aligned}.
> > +The @code{warn_if_not_aligned } attribute can also be used for types
> 
> Stray space before }.

Done.

> 
> > +static void
> > +handle_warn_if_not_align (tree field, unsigned int record_align)
> 
> Missing comment above this function explaining its semantics and those of 
> its arguments.
> 

Done.

> > +  if ((record_align % warn_if_not_align) != 0)
> > +warning (opt_w, "alignment %d of %qT is less than %d",
> > +record_align, context, warn_if_not_align);
> 
> I'd expect %u for unsigned int alignments, instead of %d.

Done.

> 
> > +  unsigned int off
> > += (tree_to_uhwi (DECL_FIELD_OFFSET (field))
> > +   + tree_to_uhwi (DECL_FIELD_BIT_OFFSET (field)) / BITS_PER_UNIT);
> > +  if ((off % warn_if_not_align) != 0)
> > +warning (opt_w, "%q+D offset %d in %qT isn't aligned to %d",
> > +field, off, context, warn_if_not_align);
> 
> And you can have struct offsets that don't fit in unsigned int (i.e. 
> structures over 4 GB), so should be using unsigned HOST_WIDE_INT to store 
> the offset and %wu to print it.  (Whereas various places in GCC restrict 
> alignments to unsigned int.)

Done.

> 
> What happens if you specify the attribute on a bit-field, or on a type 
> used to declare a bit-field?  I don't think either of those particularly 
> makes sense, but I don't see tests for it either.
> 

I fixed it and added some testcases.

Here is the updated patch.  OK for master?

Thanks.


H.J.

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.
(grokfield): Don't allow bit-field with warn_if_not_aligned 

Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-07-06 Thread Joseph Myers
On Fri, 16 Jun 2017, H.J. Lu wrote:

> +@code{warning: alignment 8 of 'struct foo' is less than 16}.

I think @samp is better than @code for warnings, throughout, since they 
aren't pieces of program code.

> +This warning can be disabled by @option{-Wno-if-not-aligned}.
> +The @code{warn_if_not_aligned } attribute can also be used for types

Stray space before }.

> +static void
> +handle_warn_if_not_align (tree field, unsigned int record_align)

Missing comment above this function explaining its semantics and those of 
its arguments.

> +  if ((record_align % warn_if_not_align) != 0)
> +warning (opt_w, "alignment %d of %qT is less than %d",
> +  record_align, context, warn_if_not_align);

I'd expect %u for unsigned int alignments, instead of %d.

> +  unsigned int off
> += (tree_to_uhwi (DECL_FIELD_OFFSET (field))
> +   + tree_to_uhwi (DECL_FIELD_BIT_OFFSET (field)) / BITS_PER_UNIT);
> +  if ((off % warn_if_not_align) != 0)
> +warning (opt_w, "%q+D offset %d in %qT isn't aligned to %d",
> +  field, off, context, warn_if_not_align);

And you can have struct offsets that don't fit in unsigned int (i.e. 
structures over 4 GB), so should be using unsigned HOST_WIDE_INT to store 
the offset and %wu to print it.  (Whereas various places in GCC restrict 
alignments to unsigned int.)

What happens if you specify the attribute on a bit-field, or on a type 
used to declare a bit-field?  I don't think either of those particularly 
makes sense, but I don't see tests for it either.

-- 
Joseph S. Myers
jos...@codesourcery.com


Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-06-16 Thread H.J. Lu
On Thu, Jun 15, 2017 at 05:31:34PM +, Joseph Myers wrote:
> On Thu, 15 Jun 2017, H.J. Lu wrote:
> 
> > On Thu, Jun 15, 2017 at 8:38 AM, Martin Sebor  wrote:
> > >>
> > >> Where do we go from here?
> > >
> > >
> > > Other than the C and C++ maintainers needing to approve the patch
> > > I can't think of anything else.
> > 
> > Hi Joseph, Jason,
> > 
> > The complete patch is at
> > 
> > https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00541.html
> > 
> > Is this OK for trunk?
> 
> I'd expect the warning calls to include OPT_Wif_not_aligned or 
> OPT_Wpacked_not_aligned (as appropriate, depending on what triggered the 
> warning / would disable it), so the warning output includes an option 
> name.
> 

Done.

> As the attribute is specific to fields I'd expect testcases that use of it 

Done.

> on non-fields is diagnosed.  And I think the diagnostic for that should 
> include quotes, %.
> 

Done.

Here is the updated patch.  OK for trunk?

Thanks.


H.J.
---
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.

gcc/cp/

PR c/53037
* decl.c (duplicate_decls): Also merge DECL_WARN_IF_NOT_ALIGN.

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.
---
 gcc/c-family/c-attribs.c   | 68 ++
 gcc/c-family/c.opt |  8 
 gcc/c/c-decl.c |  4 ++
 gcc/cp/decl.c  |  4 ++
 gcc/doc/extend.texi| 87 ++
 gcc/doc/invoke.texi| 29 +++-
 gcc/print-tree.c   |  

Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-06-15 Thread Joseph Myers
On Thu, 15 Jun 2017, H.J. Lu wrote:

> On Thu, Jun 15, 2017 at 8:38 AM, Martin Sebor  wrote:
> >>
> >> Where do we go from here?
> >
> >
> > Other than the C and C++ maintainers needing to approve the patch
> > I can't think of anything else.
> 
> Hi Joseph, Jason,
> 
> The complete patch is at
> 
> https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00541.html
> 
> Is this OK for trunk?

I'd expect the warning calls to include OPT_Wif_not_aligned or 
OPT_Wpacked_not_aligned (as appropriate, depending on what triggered the 
warning / would disable it), so the warning output includes an option 
name.

As the attribute is specific to fields I'd expect testcases that use of it 
on non-fields is diagnosed.  And I think the diagnostic for that should 
include quotes, %.

-- 
Joseph S. Myers
jos...@codesourcery.com


Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-06-15 Thread H.J. Lu
On Thu, Jun 15, 2017 at 8:38 AM, Martin Sebor  wrote:
>>
>> Where do we go from here?
>
>
> Other than the C and C++ maintainers needing to approve the patch
> I can't think of anything else.

Hi Joseph, Jason,

The complete patch is at

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

Is this OK for trunk?

> If you think it's worthwhile to merge the two options into one
> (or perhaps even incorporate them into -Wpacked) that can be done
> afterwards.

I can take a look after my patch is accepted.

Thanks.


H.J.


Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-06-15 Thread Martin Sebor

On 06/09/2017 07:31 AM, H.J. Lu wrote:

On Thu, Jun 8, 2017 at 12:13 PM, Martin Sebor  wrote:

On 06/08/2017 11:00 AM, H.J. Lu wrote:


On Wed, Jun 7, 2017 at 6:30 AM, H.J. Lu  wrote:


On Tue, Jun 6, 2017 at 5:11 PM, Martin Sebor  wrote:


On 06/06/2017 04:57 PM, H.J. Lu wrote:



On Tue, Jun 6, 2017 at 10:34 AM, Martin Sebor  wrote:



On 06/06/2017 10:59 AM, H.J. Lu wrote:




On Tue, Jun 6, 2017 at 9:10 AM, Martin Sebor 
wrote:




On 06/06/2017 10:07 AM, Martin Sebor wrote:





On 06/05/2017 11:45 AM, H.J. Lu wrote:





On Mon, Jun 5, 2017 at 8:11 AM, Joseph Myers

wrote:





The new attribute needs documentation.  Should the test be in
c-c++-common






This feature does support C++.  But C++ compiler issues a slightly
different warning at a different location.


or does this feature not support C++?



Here is the updated patch with documentation and a C++ test.  This
patch caused a few testsuite failures:

FAIL: gcc.dg/compat/struct-align-1 c_compat_x_tst.o compile





/export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.dg/compat//struct-align-1.h:169:1:

warning: alignment 1 of 'struct B2_m_inner_p_outer' is less than
16

FAIL: g++.dg/torture/pr80334.C   -O0  (test for excess errors)





/export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/torture/pr80334.C:4:8:

warning: alignment 1 of 'B' is less than 16



Users often want the ability to control a warning, even when it
certainly indicates a bug.  I would suggest to add an option to
make it possible for this warning as well.

Btw., a bug related to some of those this warning is meant to
detect is assigning the address of an underaligned object to
a pointer of a natively aligned type.  Clang has an option
to detect this problem: -Waddress-of-packed-member.  It might
make a nice follow-on enhancement to add support for the same
thing.  I mention this because I think it would make sense to
consider this when choosing the name of the GCC option (i.e.,
rather than having two distinct but closely related warnings,
have one that detects both of these alignment type of bugs.






A bug that has some additional context on this is pr 51628.
A possible name for the new option suggested there is -Wpacked.

Martin





Isn't -Waddress-of-packed-member a subset of or the same as
-Wpacked?





In Clang it's neither.  -Waddress-of-packed-member only triggers
when the address of a packed member is taken but not for the cases
in bug 53037 (i.e., reducing the alignment of a member).  It's
also enabled by default, while -Wpacked needs to be specified
explicitly (i.e., it's in neither -Wall or -Wextra).

FWIW, I don't really have a strong opinion about the names of
the options.  My input is that the proliferation of fine-grained
warning options for closely related problems tends to make it
tricky to get their interactions right (both in the compiler
and for users).  Enabling both/all such options can lead to
multiple warnings for what boils down to essentially the same
bug in the same expression, overwhelming the user in repetitive
diagnostics.



There is already -Wpacked.  Should I overload it for this?




I'd say yes if -Wpacked were at least in -Wall.  But it's
an opt-in kind of warning that's not even in -Wextra, and
relaxing an explicitly specified alignment seems more like
a bug than just something that might be nice to know about.
I would expect warn_if_not_aligned to trigger a warning even
without -Wall (i.e., as you have it in your patch, but with
an option to control it).  That would suggest three levels
of warnings:

1) warn by default (warn_if_not_aligned violation)
2) warn with -Wall (using a type with attribute aligned to
   define a member of a packed struct)
3) warn if requested (current -Wpacked)

So one way to deal with it would be to change -Wpacked to
take an argument between 0 and 3, set the default to
correspond to the (1) above, and have -Wall bump it up to
(2).

If the equivalent of -Waddress-of-packed-member were to be
implemented in GCC it would probably be a candidate to add
to the (2) above.(*)

This might be more involved than you envisioned.  A slightly
simpler alternative would be to add a different option, say
something like -Walign=N, and have it handle just (1) and
(2) above, leaving -Wpacked unchanged.



Since there is no agreement on -W options and changes
may touch many places, I will do

1. Add -Wwarn-if-not-aligned and enable it by default.
2. Add -Wpacked-not-aligned and disable it by default.

Once there is an agreement, I replace -Wpacked-not-aligned
with the new option.



Okay.  I can't approve the patch but thank you for enhancing your
patch to handle the additional case I brought up!

Martin


Where do we go from here?


Other than the C and C++ maintainers needing to approve the patch
I can't think of anything else.

If you think it's worthwhile to merge the two options into one
(or perhaps even incorporate them into -Wpacked) that can be done
afterwards.

Martin






Here is the updated patch.

Add war

Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-06-09 Thread H.J. Lu
On Thu, Jun 8, 2017 at 12:13 PM, Martin Sebor  wrote:
> On 06/08/2017 11:00 AM, H.J. Lu wrote:
>>
>> On Wed, Jun 7, 2017 at 6:30 AM, H.J. Lu  wrote:
>>>
>>> On Tue, Jun 6, 2017 at 5:11 PM, Martin Sebor  wrote:

 On 06/06/2017 04:57 PM, H.J. Lu wrote:
>
>
> On Tue, Jun 6, 2017 at 10:34 AM, Martin Sebor  wrote:
>>
>>
>> On 06/06/2017 10:59 AM, H.J. Lu wrote:
>>>
>>>
>>>
>>> On Tue, Jun 6, 2017 at 9:10 AM, Martin Sebor 
>>> wrote:



 On 06/06/2017 10:07 AM, Martin Sebor wrote:
>
>
>
>
> On 06/05/2017 11:45 AM, H.J. Lu wrote:
>>
>>
>>
>>
>> On Mon, Jun 5, 2017 at 8:11 AM, Joseph Myers
>> 
>> wrote:
>>>
>>>
>>>
>>>
>>> The new attribute needs documentation.  Should the test be in
>>> c-c++-common
>>
>>
>>
>>
>>
>> This feature does support C++.  But C++ compiler issues a slightly
>> different warning at a different location.
>>
>>> or does this feature not support C++?
>>>
>>
>> Here is the updated patch with documentation and a C++ test.  This
>> patch caused a few testsuite failures:
>>
>> FAIL: gcc.dg/compat/struct-align-1 c_compat_x_tst.o compile
>>
>>
>>
>>
>>
>> /export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.dg/compat//struct-align-1.h:169:1:
>>
>> warning: alignment 1 of 'struct B2_m_inner_p_outer' is less than
>> 16
>>
>> FAIL: g++.dg/torture/pr80334.C   -O0  (test for excess errors)
>>
>>
>>
>>
>>
>> /export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/torture/pr80334.C:4:8:
>>
>> warning: alignment 1 of 'B' is less than 16
>>
>
> Users often want the ability to control a warning, even when it
> certainly indicates a bug.  I would suggest to add an option to
> make it possible for this warning as well.
>
> Btw., a bug related to some of those this warning is meant to
> detect is assigning the address of an underaligned object to
> a pointer of a natively aligned type.  Clang has an option
> to detect this problem: -Waddress-of-packed-member.  It might
> make a nice follow-on enhancement to add support for the same
> thing.  I mention this because I think it would make sense to
> consider this when choosing the name of the GCC option (i.e.,
> rather than having two distinct but closely related warnings,
> have one that detects both of these alignment type of bugs.





 A bug that has some additional context on this is pr 51628.
 A possible name for the new option suggested there is -Wpacked.

 Martin
>>>
>>>
>>>
>>>
>>> Isn't -Waddress-of-packed-member a subset of or the same as
>>> -Wpacked?
>>
>>
>>
>>
>> In Clang it's neither.  -Waddress-of-packed-member only triggers
>> when the address of a packed member is taken but not for the cases
>> in bug 53037 (i.e., reducing the alignment of a member).  It's
>> also enabled by default, while -Wpacked needs to be specified
>> explicitly (i.e., it's in neither -Wall or -Wextra).
>>
>> FWIW, I don't really have a strong opinion about the names of
>> the options.  My input is that the proliferation of fine-grained
>> warning options for closely related problems tends to make it
>> tricky to get their interactions right (both in the compiler
>> and for users).  Enabling both/all such options can lead to
>> multiple warnings for what boils down to essentially the same
>> bug in the same expression, overwhelming the user in repetitive
>> diagnostics.
>>
>
> There is already -Wpacked.  Should I overload it for this?



 I'd say yes if -Wpacked were at least in -Wall.  But it's
 an opt-in kind of warning that's not even in -Wextra, and
 relaxing an explicitly specified alignment seems more like
 a bug than just something that might be nice to know about.
 I would expect warn_if_not_aligned to trigger a warning even
 without -Wall (i.e., as you have it in your patch, but with
 an option to control it).  That would suggest three levels
 of warnings:

 1) warn by default (warn_if_not_aligned violation)
 2) warn with -Wall (using a type with attribute aligned to
define a member of a packed struct)
 3) warn if requested (current -Wpacked)

 So one way to deal with it would be to change -Wpacked to
 take an argument between 0 and 3, set the default to
 correspond to the (1) above, and have -Wal

Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-06-08 Thread Martin Sebor

On 06/08/2017 11:00 AM, H.J. Lu wrote:

On Wed, Jun 7, 2017 at 6:30 AM, H.J. Lu  wrote:

On Tue, Jun 6, 2017 at 5:11 PM, Martin Sebor  wrote:

On 06/06/2017 04:57 PM, H.J. Lu wrote:


On Tue, Jun 6, 2017 at 10:34 AM, Martin Sebor  wrote:


On 06/06/2017 10:59 AM, H.J. Lu wrote:



On Tue, Jun 6, 2017 at 9:10 AM, Martin Sebor  wrote:



On 06/06/2017 10:07 AM, Martin Sebor wrote:




On 06/05/2017 11:45 AM, H.J. Lu wrote:




On Mon, Jun 5, 2017 at 8:11 AM, Joseph Myers

wrote:




The new attribute needs documentation.  Should the test be in
c-c++-common





This feature does support C++.  But C++ compiler issues a slightly
different warning at a different location.


or does this feature not support C++?



Here is the updated patch with documentation and a C++ test.  This
patch caused a few testsuite failures:

FAIL: gcc.dg/compat/struct-align-1 c_compat_x_tst.o compile




/export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.dg/compat//struct-align-1.h:169:1:

warning: alignment 1 of 'struct B2_m_inner_p_outer' is less than 16

FAIL: g++.dg/torture/pr80334.C   -O0  (test for excess errors)




/export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/torture/pr80334.C:4:8:

warning: alignment 1 of 'B' is less than 16



Users often want the ability to control a warning, even when it
certainly indicates a bug.  I would suggest to add an option to
make it possible for this warning as well.

Btw., a bug related to some of those this warning is meant to
detect is assigning the address of an underaligned object to
a pointer of a natively aligned type.  Clang has an option
to detect this problem: -Waddress-of-packed-member.  It might
make a nice follow-on enhancement to add support for the same
thing.  I mention this because I think it would make sense to
consider this when choosing the name of the GCC option (i.e.,
rather than having two distinct but closely related warnings,
have one that detects both of these alignment type of bugs.





A bug that has some additional context on this is pr 51628.
A possible name for the new option suggested there is -Wpacked.

Martin




Isn't -Waddress-of-packed-member a subset of or the same as
-Wpacked?




In Clang it's neither.  -Waddress-of-packed-member only triggers
when the address of a packed member is taken but not for the cases
in bug 53037 (i.e., reducing the alignment of a member).  It's
also enabled by default, while -Wpacked needs to be specified
explicitly (i.e., it's in neither -Wall or -Wextra).

FWIW, I don't really have a strong opinion about the names of
the options.  My input is that the proliferation of fine-grained
warning options for closely related problems tends to make it
tricky to get their interactions right (both in the compiler
and for users).  Enabling both/all such options can lead to
multiple warnings for what boils down to essentially the same
bug in the same expression, overwhelming the user in repetitive
diagnostics.



There is already -Wpacked.  Should I overload it for this?



I'd say yes if -Wpacked were at least in -Wall.  But it's
an opt-in kind of warning that's not even in -Wextra, and
relaxing an explicitly specified alignment seems more like
a bug than just something that might be nice to know about.
I would expect warn_if_not_aligned to trigger a warning even
without -Wall (i.e., as you have it in your patch, but with
an option to control it).  That would suggest three levels
of warnings:

1) warn by default (warn_if_not_aligned violation)
2) warn with -Wall (using a type with attribute aligned to
   define a member of a packed struct)
3) warn if requested (current -Wpacked)

So one way to deal with it would be to change -Wpacked to
take an argument between 0 and 3, set the default to
correspond to the (1) above, and have -Wall bump it up to
(2).

If the equivalent of -Waddress-of-packed-member were to be
implemented in GCC it would probably be a candidate to add
to the (2) above.(*)

This might be more involved than you envisioned.  A slightly
simpler alternative would be to add a different option, say
something like -Walign=N, and have it handle just (1) and
(2) above, leaving -Wpacked unchanged.



Since there is no agreement on -W options and changes
may touch many places, I will do

1. Add -Wwarn-if-not-aligned and enable it by default.
2. Add -Wpacked-not-aligned and disable it by default.

Once there is an agreement, I replace -Wpacked-not-aligned
with the new option.


Okay.  I can't approve the patch but thank you for enhancing your
patch to handle the additional case I brought up!

Martin


Here is the updated patch.

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 b

Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-06-08 Thread H.J. Lu
On Wed, Jun 7, 2017 at 6:30 AM, H.J. Lu  wrote:
> On Tue, Jun 6, 2017 at 5:11 PM, Martin Sebor  wrote:
>> On 06/06/2017 04:57 PM, H.J. Lu wrote:
>>>
>>> On Tue, Jun 6, 2017 at 10:34 AM, Martin Sebor  wrote:

 On 06/06/2017 10:59 AM, H.J. Lu wrote:
>
>
> On Tue, Jun 6, 2017 at 9:10 AM, Martin Sebor  wrote:
>>
>>
>> On 06/06/2017 10:07 AM, Martin Sebor wrote:
>>>
>>>
>>>
>>> On 06/05/2017 11:45 AM, H.J. Lu wrote:



 On Mon, Jun 5, 2017 at 8:11 AM, Joseph Myers
 
 wrote:
>
>
>
> The new attribute needs documentation.  Should the test be in
> c-c++-common




 This feature does support C++.  But C++ compiler issues a slightly
 different warning at a different location.

> or does this feature not support C++?
>

 Here is the updated patch with documentation and a C++ test.  This
 patch caused a few testsuite failures:

 FAIL: gcc.dg/compat/struct-align-1 c_compat_x_tst.o compile




 /export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.dg/compat//struct-align-1.h:169:1:

 warning: alignment 1 of 'struct B2_m_inner_p_outer' is less than 16

 FAIL: g++.dg/torture/pr80334.C   -O0  (test for excess errors)




 /export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/torture/pr80334.C:4:8:

 warning: alignment 1 of 'B' is less than 16

>>>
>>> Users often want the ability to control a warning, even when it
>>> certainly indicates a bug.  I would suggest to add an option to
>>> make it possible for this warning as well.
>>>
>>> Btw., a bug related to some of those this warning is meant to
>>> detect is assigning the address of an underaligned object to
>>> a pointer of a natively aligned type.  Clang has an option
>>> to detect this problem: -Waddress-of-packed-member.  It might
>>> make a nice follow-on enhancement to add support for the same
>>> thing.  I mention this because I think it would make sense to
>>> consider this when choosing the name of the GCC option (i.e.,
>>> rather than having two distinct but closely related warnings,
>>> have one that detects both of these alignment type of bugs.
>>
>>
>>
>>
>> A bug that has some additional context on this is pr 51628.
>> A possible name for the new option suggested there is -Wpacked.
>>
>> Martin
>
>
>
> Isn't -Waddress-of-packed-member a subset of or the same as
> -Wpacked?



 In Clang it's neither.  -Waddress-of-packed-member only triggers
 when the address of a packed member is taken but not for the cases
 in bug 53037 (i.e., reducing the alignment of a member).  It's
 also enabled by default, while -Wpacked needs to be specified
 explicitly (i.e., it's in neither -Wall or -Wextra).

 FWIW, I don't really have a strong opinion about the names of
 the options.  My input is that the proliferation of fine-grained
 warning options for closely related problems tends to make it
 tricky to get their interactions right (both in the compiler
 and for users).  Enabling both/all such options can lead to
 multiple warnings for what boils down to essentially the same
 bug in the same expression, overwhelming the user in repetitive
 diagnostics.

>>>
>>> There is already -Wpacked.  Should I overload it for this?
>>
>>
>> I'd say yes if -Wpacked were at least in -Wall.  But it's
>> an opt-in kind of warning that's not even in -Wextra, and
>> relaxing an explicitly specified alignment seems more like
>> a bug than just something that might be nice to know about.
>> I would expect warn_if_not_aligned to trigger a warning even
>> without -Wall (i.e., as you have it in your patch, but with
>> an option to control it).  That would suggest three levels
>> of warnings:
>>
>> 1) warn by default (warn_if_not_aligned violation)
>> 2) warn with -Wall (using a type with attribute aligned to
>>define a member of a packed struct)
>> 3) warn if requested (current -Wpacked)
>>
>> So one way to deal with it would be to change -Wpacked to
>> take an argument between 0 and 3, set the default to
>> correspond to the (1) above, and have -Wall bump it up to
>> (2).
>>
>> If the equivalent of -Waddress-of-packed-member were to be
>> implemented in GCC it would probably be a candidate to add
>> to the (2) above.(*)
>>
>> This might be more involved than you envisioned.  A slightly
>> simpler alternative would be to add a different option, say
>> something like -Walign=N, and have it handle just (1) and
>> (2) above, leaving -Wpacked unchanged.
>>
>
> Since there is no agreement on -W options and changes
> may touch many places, I will d

Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-06-07 Thread H.J. Lu
On Tue, Jun 6, 2017 at 5:11 PM, Martin Sebor  wrote:
> On 06/06/2017 04:57 PM, H.J. Lu wrote:
>>
>> On Tue, Jun 6, 2017 at 10:34 AM, Martin Sebor  wrote:
>>>
>>> On 06/06/2017 10:59 AM, H.J. Lu wrote:


 On Tue, Jun 6, 2017 at 9:10 AM, Martin Sebor  wrote:
>
>
> On 06/06/2017 10:07 AM, Martin Sebor wrote:
>>
>>
>>
>> On 06/05/2017 11:45 AM, H.J. Lu wrote:
>>>
>>>
>>>
>>> On Mon, Jun 5, 2017 at 8:11 AM, Joseph Myers
>>> 
>>> wrote:



 The new attribute needs documentation.  Should the test be in
 c-c++-common
>>>
>>>
>>>
>>>
>>> This feature does support C++.  But C++ compiler issues a slightly
>>> different warning at a different location.
>>>
 or does this feature not support C++?

>>>
>>> Here is the updated patch with documentation and a C++ test.  This
>>> patch caused a few testsuite failures:
>>>
>>> FAIL: gcc.dg/compat/struct-align-1 c_compat_x_tst.o compile
>>>
>>>
>>>
>>>
>>> /export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.dg/compat//struct-align-1.h:169:1:
>>>
>>> warning: alignment 1 of 'struct B2_m_inner_p_outer' is less than 16
>>>
>>> FAIL: g++.dg/torture/pr80334.C   -O0  (test for excess errors)
>>>
>>>
>>>
>>>
>>> /export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/torture/pr80334.C:4:8:
>>>
>>> warning: alignment 1 of 'B' is less than 16
>>>
>>
>> Users often want the ability to control a warning, even when it
>> certainly indicates a bug.  I would suggest to add an option to
>> make it possible for this warning as well.
>>
>> Btw., a bug related to some of those this warning is meant to
>> detect is assigning the address of an underaligned object to
>> a pointer of a natively aligned type.  Clang has an option
>> to detect this problem: -Waddress-of-packed-member.  It might
>> make a nice follow-on enhancement to add support for the same
>> thing.  I mention this because I think it would make sense to
>> consider this when choosing the name of the GCC option (i.e.,
>> rather than having two distinct but closely related warnings,
>> have one that detects both of these alignment type of bugs.
>
>
>
>
> A bug that has some additional context on this is pr 51628.
> A possible name for the new option suggested there is -Wpacked.
>
> Martin



 Isn't -Waddress-of-packed-member a subset of or the same as
 -Wpacked?
>>>
>>>
>>>
>>> In Clang it's neither.  -Waddress-of-packed-member only triggers
>>> when the address of a packed member is taken but not for the cases
>>> in bug 53037 (i.e., reducing the alignment of a member).  It's
>>> also enabled by default, while -Wpacked needs to be specified
>>> explicitly (i.e., it's in neither -Wall or -Wextra).
>>>
>>> FWIW, I don't really have a strong opinion about the names of
>>> the options.  My input is that the proliferation of fine-grained
>>> warning options for closely related problems tends to make it
>>> tricky to get their interactions right (both in the compiler
>>> and for users).  Enabling both/all such options can lead to
>>> multiple warnings for what boils down to essentially the same
>>> bug in the same expression, overwhelming the user in repetitive
>>> diagnostics.
>>>
>>
>> There is already -Wpacked.  Should I overload it for this?
>
>
> I'd say yes if -Wpacked were at least in -Wall.  But it's
> an opt-in kind of warning that's not even in -Wextra, and
> relaxing an explicitly specified alignment seems more like
> a bug than just something that might be nice to know about.
> I would expect warn_if_not_aligned to trigger a warning even
> without -Wall (i.e., as you have it in your patch, but with
> an option to control it).  That would suggest three levels
> of warnings:
>
> 1) warn by default (warn_if_not_aligned violation)
> 2) warn with -Wall (using a type with attribute aligned to
>define a member of a packed struct)
> 3) warn if requested (current -Wpacked)
>
> So one way to deal with it would be to change -Wpacked to
> take an argument between 0 and 3, set the default to
> correspond to the (1) above, and have -Wall bump it up to
> (2).
>
> If the equivalent of -Waddress-of-packed-member were to be
> implemented in GCC it would probably be a candidate to add
> to the (2) above.(*)
>
> This might be more involved than you envisioned.  A slightly
> simpler alternative would be to add a different option, say
> something like -Walign=N, and have it handle just (1) and
> (2) above, leaving -Wpacked unchanged.
>

Since there is no agreement on -W options and changes
may touch many places, I will do

1. Add -Wwarn-if-not-aligned and enable it by default.
2. Add -Wpacked-not-aligned and disable it by default.

Once there is an agreement, I replace -Wpacked-not-aligned
with the new opt

Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-06-06 Thread Martin Sebor

On 06/06/2017 04:57 PM, H.J. Lu wrote:

On Tue, Jun 6, 2017 at 10:34 AM, Martin Sebor  wrote:

On 06/06/2017 10:59 AM, H.J. Lu wrote:


On Tue, Jun 6, 2017 at 9:10 AM, Martin Sebor  wrote:


On 06/06/2017 10:07 AM, Martin Sebor wrote:



On 06/05/2017 11:45 AM, H.J. Lu wrote:



On Mon, Jun 5, 2017 at 8:11 AM, Joseph Myers 
wrote:



The new attribute needs documentation.  Should the test be in
c-c++-common




This feature does support C++.  But C++ compiler issues a slightly
different warning at a different location.


or does this feature not support C++?



Here is the updated patch with documentation and a C++ test.  This
patch caused a few testsuite failures:

FAIL: gcc.dg/compat/struct-align-1 c_compat_x_tst.o compile



/export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.dg/compat//struct-align-1.h:169:1:

warning: alignment 1 of 'struct B2_m_inner_p_outer' is less than 16

FAIL: g++.dg/torture/pr80334.C   -O0  (test for excess errors)



/export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/torture/pr80334.C:4:8:

warning: alignment 1 of 'B' is less than 16



Users often want the ability to control a warning, even when it
certainly indicates a bug.  I would suggest to add an option to
make it possible for this warning as well.

Btw., a bug related to some of those this warning is meant to
detect is assigning the address of an underaligned object to
a pointer of a natively aligned type.  Clang has an option
to detect this problem: -Waddress-of-packed-member.  It might
make a nice follow-on enhancement to add support for the same
thing.  I mention this because I think it would make sense to
consider this when choosing the name of the GCC option (i.e.,
rather than having two distinct but closely related warnings,
have one that detects both of these alignment type of bugs.




A bug that has some additional context on this is pr 51628.
A possible name for the new option suggested there is -Wpacked.

Martin



Isn't -Waddress-of-packed-member a subset of or the same as
-Wpacked?



In Clang it's neither.  -Waddress-of-packed-member only triggers
when the address of a packed member is taken but not for the cases
in bug 53037 (i.e., reducing the alignment of a member).  It's
also enabled by default, while -Wpacked needs to be specified
explicitly (i.e., it's in neither -Wall or -Wextra).

FWIW, I don't really have a strong opinion about the names of
the options.  My input is that the proliferation of fine-grained
warning options for closely related problems tends to make it
tricky to get their interactions right (both in the compiler
and for users).  Enabling both/all such options can lead to
multiple warnings for what boils down to essentially the same
bug in the same expression, overwhelming the user in repetitive
diagnostics.



There is already -Wpacked.  Should I overload it for this?


I'd say yes if -Wpacked were at least in -Wall.  But it's
an opt-in kind of warning that's not even in -Wextra, and
relaxing an explicitly specified alignment seems more like
a bug than just something that might be nice to know about.
I would expect warn_if_not_aligned to trigger a warning even
without -Wall (i.e., as you have it in your patch, but with
an option to control it).  That would suggest three levels
of warnings:

1) warn by default (warn_if_not_aligned violation)
2) warn with -Wall (using a type with attribute aligned to
   define a member of a packed struct)
3) warn if requested (current -Wpacked)

So one way to deal with it would be to change -Wpacked to
take an argument between 0 and 3, set the default to
correspond to the (1) above, and have -Wall bump it up to
(2).

If the equivalent of -Waddress-of-packed-member were to be
implemented in GCC it would probably be a candidate to add
to the (2) above.(*)

This might be more involved than you envisioned.  A slightly
simpler alternative would be to add a different option, say
something like -Walign=N, and have it handle just (1) and
(2) above, leaving -Wpacked unchanged.

Martin

PS [*] On a related note, in the Clang discussion of
-Waddress-of-packed-member they briefly considered reusing
-Wcast-align for the same purpose, but decided against it
because it apparently involves an explicit cast.  That
doesn't seem to me like a string enough argument not to
change -Wcast-align to trigger on implicit conversions that
increase alignment.  (The option is essentially useless on
most targets so this extension would make it generally
useful.)


Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-06-06 Thread H.J. Lu
On Tue, Jun 6, 2017 at 10:34 AM, Martin Sebor  wrote:
> On 06/06/2017 10:59 AM, H.J. Lu wrote:
>>
>> On Tue, Jun 6, 2017 at 9:10 AM, Martin Sebor  wrote:
>>>
>>> On 06/06/2017 10:07 AM, Martin Sebor wrote:


 On 06/05/2017 11:45 AM, H.J. Lu wrote:
>
>
> On Mon, Jun 5, 2017 at 8:11 AM, Joseph Myers 
> wrote:
>>
>>
>> The new attribute needs documentation.  Should the test be in
>> c-c++-common
>
>
>
> This feature does support C++.  But C++ compiler issues a slightly
> different warning at a different location.
>
>> or does this feature not support C++?
>>
>
> Here is the updated patch with documentation and a C++ test.  This
> patch caused a few testsuite failures:
>
> FAIL: gcc.dg/compat/struct-align-1 c_compat_x_tst.o compile
>
>
>
> /export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.dg/compat//struct-align-1.h:169:1:
>
> warning: alignment 1 of 'struct B2_m_inner_p_outer' is less than 16
>
> FAIL: g++.dg/torture/pr80334.C   -O0  (test for excess errors)
>
>
>
> /export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/torture/pr80334.C:4:8:
>
> warning: alignment 1 of 'B' is less than 16
>

 Users often want the ability to control a warning, even when it
 certainly indicates a bug.  I would suggest to add an option to
 make it possible for this warning as well.

 Btw., a bug related to some of those this warning is meant to
 detect is assigning the address of an underaligned object to
 a pointer of a natively aligned type.  Clang has an option
 to detect this problem: -Waddress-of-packed-member.  It might
 make a nice follow-on enhancement to add support for the same
 thing.  I mention this because I think it would make sense to
 consider this when choosing the name of the GCC option (i.e.,
 rather than having two distinct but closely related warnings,
 have one that detects both of these alignment type of bugs.
>>>
>>>
>>>
>>> A bug that has some additional context on this is pr 51628.
>>> A possible name for the new option suggested there is -Wpacked.
>>>
>>> Martin
>>
>>
>> Isn't -Waddress-of-packed-member a subset of or the same as
>> -Wpacked?
>
>
> In Clang it's neither.  -Waddress-of-packed-member only triggers
> when the address of a packed member is taken but not for the cases
> in bug 53037 (i.e., reducing the alignment of a member).  It's
> also enabled by default, while -Wpacked needs to be specified
> explicitly (i.e., it's in neither -Wall or -Wextra).
>
> FWIW, I don't really have a strong opinion about the names of
> the options.  My input is that the proliferation of fine-grained
> warning options for closely related problems tends to make it
> tricky to get their interactions right (both in the compiler
> and for users).  Enabling both/all such options can lead to
> multiple warnings for what boils down to essentially the same
> bug in the same expression, overwhelming the user in repetitive
> diagnostics.
>

There is already -Wpacked.  Should I overload it for this?


-- 
H.J.


Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-06-06 Thread Martin Sebor

On 06/06/2017 10:59 AM, H.J. Lu wrote:

On Tue, Jun 6, 2017 at 9:10 AM, Martin Sebor  wrote:

On 06/06/2017 10:07 AM, Martin Sebor wrote:


On 06/05/2017 11:45 AM, H.J. Lu wrote:


On Mon, Jun 5, 2017 at 8:11 AM, Joseph Myers 
wrote:


The new attribute needs documentation.  Should the test be in
c-c++-common



This feature does support C++.  But C++ compiler issues a slightly
different warning at a different location.


or does this feature not support C++?



Here is the updated patch with documentation and a C++ test.  This
patch caused a few testsuite failures:

FAIL: gcc.dg/compat/struct-align-1 c_compat_x_tst.o compile


/export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.dg/compat//struct-align-1.h:169:1:

warning: alignment 1 of 'struct B2_m_inner_p_outer' is less than 16

FAIL: g++.dg/torture/pr80334.C   -O0  (test for excess errors)


/export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/torture/pr80334.C:4:8:

warning: alignment 1 of 'B' is less than 16



Users often want the ability to control a warning, even when it
certainly indicates a bug.  I would suggest to add an option to
make it possible for this warning as well.

Btw., a bug related to some of those this warning is meant to
detect is assigning the address of an underaligned object to
a pointer of a natively aligned type.  Clang has an option
to detect this problem: -Waddress-of-packed-member.  It might
make a nice follow-on enhancement to add support for the same
thing.  I mention this because I think it would make sense to
consider this when choosing the name of the GCC option (i.e.,
rather than having two distinct but closely related warnings,
have one that detects both of these alignment type of bugs.



A bug that has some additional context on this is pr 51628.
A possible name for the new option suggested there is -Wpacked.

Martin


Isn't -Waddress-of-packed-member a subset of or the same as
-Wpacked?


In Clang it's neither.  -Waddress-of-packed-member only triggers
when the address of a packed member is taken but not for the cases
in bug 53037 (i.e., reducing the alignment of a member).  It's
also enabled by default, while -Wpacked needs to be specified
explicitly (i.e., it's in neither -Wall or -Wextra).

FWIW, I don't really have a strong opinion about the names of
the options.  My input is that the proliferation of fine-grained
warning options for closely related problems tends to make it
tricky to get their interactions right (both in the compiler
and for users).  Enabling both/all such options can lead to
multiple warnings for what boils down to essentially the same
bug in the same expression, overwhelming the user in repetitive
diagnostics.

Martin


Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-06-06 Thread H.J. Lu
On Tue, Jun 6, 2017 at 9:10 AM, Martin Sebor  wrote:
> On 06/06/2017 10:07 AM, Martin Sebor wrote:
>>
>> On 06/05/2017 11:45 AM, H.J. Lu wrote:
>>>
>>> On Mon, Jun 5, 2017 at 8:11 AM, Joseph Myers 
>>> wrote:

 The new attribute needs documentation.  Should the test be in
 c-c++-common
>>>
>>>
>>> This feature does support C++.  But C++ compiler issues a slightly
>>> different warning at a different location.
>>>
 or does this feature not support C++?

>>>
>>> Here is the updated patch with documentation and a C++ test.  This
>>> patch caused a few testsuite failures:
>>>
>>> FAIL: gcc.dg/compat/struct-align-1 c_compat_x_tst.o compile
>>>
>>>
>>> /export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.dg/compat//struct-align-1.h:169:1:
>>>
>>> warning: alignment 1 of 'struct B2_m_inner_p_outer' is less than 16
>>>
>>> FAIL: g++.dg/torture/pr80334.C   -O0  (test for excess errors)
>>>
>>>
>>> /export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/torture/pr80334.C:4:8:
>>>
>>> warning: alignment 1 of 'B' is less than 16
>>>
>>
>> Users often want the ability to control a warning, even when it
>> certainly indicates a bug.  I would suggest to add an option to
>> make it possible for this warning as well.
>>
>> Btw., a bug related to some of those this warning is meant to
>> detect is assigning the address of an underaligned object to
>> a pointer of a natively aligned type.  Clang has an option
>> to detect this problem: -Waddress-of-packed-member.  It might
>> make a nice follow-on enhancement to add support for the same
>> thing.  I mention this because I think it would make sense to
>> consider this when choosing the name of the GCC option (i.e.,
>> rather than having two distinct but closely related warnings,
>> have one that detects both of these alignment type of bugs.
>
>
> A bug that has some additional context on this is pr 51628.
> A possible name for the new option suggested there is -Wpacked.
>
> Martin

Isn't -Waddress-of-packed-member a subset of or the same as
-Wpacked?

-- 
H.J.


Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-06-06 Thread Martin Sebor

On 06/06/2017 10:07 AM, Martin Sebor wrote:

On 06/05/2017 11:45 AM, H.J. Lu wrote:

On Mon, Jun 5, 2017 at 8:11 AM, Joseph Myers 
wrote:

The new attribute needs documentation.  Should the test be in
c-c++-common


This feature does support C++.  But C++ compiler issues a slightly
different warning at a different location.


or does this feature not support C++?



Here is the updated patch with documentation and a C++ test.  This
patch caused a few testsuite failures:

FAIL: gcc.dg/compat/struct-align-1 c_compat_x_tst.o compile

/export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.dg/compat//struct-align-1.h:169:1:

warning: alignment 1 of 'struct B2_m_inner_p_outer' is less than 16

FAIL: g++.dg/torture/pr80334.C   -O0  (test for excess errors)

/export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/torture/pr80334.C:4:8:

warning: alignment 1 of 'B' is less than 16



Users often want the ability to control a warning, even when it
certainly indicates a bug.  I would suggest to add an option to
make it possible for this warning as well.

Btw., a bug related to some of those this warning is meant to
detect is assigning the address of an underaligned object to
a pointer of a natively aligned type.  Clang has an option
to detect this problem: -Waddress-of-packed-member.  It might
make a nice follow-on enhancement to add support for the same
thing.  I mention this because I think it would make sense to
consider this when choosing the name of the GCC option (i.e.,
rather than having two distinct but closely related warnings,
have one that detects both of these alignment type of bugs.


A bug that has some additional context on this is pr 51628.
A possible name for the new option suggested there is -Wpacked.

Martin


Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-06-06 Thread Martin Sebor

On 06/05/2017 11:45 AM, H.J. Lu wrote:

On Mon, Jun 5, 2017 at 8:11 AM, Joseph Myers  wrote:

The new attribute needs documentation.  Should the test be in c-c++-common


This feature does support C++.  But C++ compiler issues a slightly
different warning at a different location.


or does this feature not support C++?



Here is the updated patch with documentation and a C++ test.  This
patch caused a few testsuite failures:

FAIL: gcc.dg/compat/struct-align-1 c_compat_x_tst.o compile

/export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.dg/compat//struct-align-1.h:169:1:
warning: alignment 1 of 'struct B2_m_inner_p_outer' is less than 16

FAIL: g++.dg/torture/pr80334.C   -O0  (test for excess errors)

/export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/torture/pr80334.C:4:8:
warning: alignment 1 of 'B' is less than 16



Users often want the ability to control a warning, even when it
certainly indicates a bug.  I would suggest to add an option to
make it possible for this warning as well.

Btw., a bug related to some of those this warning is meant to
detect is assigning the address of an underaligned object to
a pointer of a natively aligned type.  Clang has an option
to detect this problem: -Waddress-of-packed-member.  It might
make a nice follow-on enhancement to add support for the same
thing.  I mention this because I think it would make sense to
consider this when choosing the name of the GCC option (i.e.,
rather than having two distinct but closely related warnings,
have one that detects both of these alignment type of bugs.

Martin


Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-06-05 Thread H.J. Lu
On Mon, Jun 5, 2017 at 8:11 AM, Joseph Myers  wrote:
> The new attribute needs documentation.  Should the test be in c-c++-common

This feature does support C++.  But C++ compiler issues a slightly
different warning at a different location.

> or does this feature not support C++?
>

Here is the updated patch with documentation and a C++ test.  This
patch caused a few testsuite failures:

FAIL: gcc.dg/compat/struct-align-1 c_compat_x_tst.o compile

/export/gnu/import/git/sources/gcc/gcc/testsuite/gcc.dg/compat//struct-align-1.h:169:1:
warning: alignment 1 of 'struct B2_m_inner_p_outer' is less than 16

FAIL: g++.dg/torture/pr80334.C   -O0  (test for excess errors)

/export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/torture/pr80334.C:4:8:
warning: alignment 1 of 'B' is less than 16

-- 
H.J.
From b427b82bf9e7e510a1c1d5a91aa8198dd8036232 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" 
Date: Fri, 20 Apr 2012 13:49:05 -0700
Subject: [PATCH] Add warn_if_not_aligned attribute
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

__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

gcc/

	PR c/53037
	* c-decl.c (finish_enum): Also copy TYPE_WARN_IF_NOT_ALIGN.
	* print-tree.c (print_node): Support TYPE_WARN_IF_NOT_ALIGN.
	* stor-layout.c (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.c (build_range_type_1): Likewise.
	* tree-core.h (tree_type_common): Add warn_if_not_align.  Set
	spare to 18.
	* tree.h (TYPE_WARN_IF_NOT_ALIGN): New.
	(SET_TYPE_WARN_IF_NOT_ALIGN): Likewise.
	* c-family/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/c-decl.c (finish_enum): Copy TYPE_WARN_IF_NOT_ALIGN.
	* doc/extend.texi: Document warn_if_not_aligned attribute.

gcc/testsuite/

	PR c/53037
	* g++.dg/pr53037-1.C: New test.
	* gcc.dg/pr53037-1.c: Likewise.
---
 gcc/c-family/c-attribs.c | 55 +++---
 gcc/c/c-decl.c   |  2 +
 gcc/doc/extend.texi  | 50 
 gcc/print-tree.c |  6 ++-
 gcc/stor-layout.c| 45 +
 gcc/testsuite/g++.dg/pr53037-1.C | 84 
 gcc/testsuite/gcc.dg/pr53037-1.c | 84 
 gcc/tree-core.h  |  3 +-
 gcc/tree.c   |  1 +
 gcc/tree.h   | 10 +
 10 files changed, 331 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/pr53037-1.C
 create mode 100644 gcc/testsuite/gcc.dg/pr53037-1.c

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 695c58c..a76f9f7 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -85,6 +85,8 @@ static tree handle_destructor_attribute (tree *, tree, tree, int, bool *);
 static tree handle_mode_attribute (tree *, tree, tree, int, bool *);
 static tree handle_section_attribute (tree *, tree, tree, int, bool *);
 static tree handle_aligned_attribute (tree *, tree, tree, int, bool *);
+static tree handle_warn_if_not_aligned_attribute (tree *, tree, tree,
+		  int, bool *);
 static tree handle_weak_attribute (tree *, tree, tree, int, bool *) ;
 static tree handle_noplt_attribute (tree *, tree, tree, int, bool *) ;
 static tree handle_alias_ifunc_attribute (bool, tree *, tree, tree, bool *);
@@ -

Re: RFC: [PATCH] Add warn_if_not_aligned attribute

2017-06-05 Thread Joseph Myers
The new attribute needs documentation.  Should the test be in c-c++-common 
or does this feature not support C++?

-- 
Joseph S. Myers
jos...@codesourcery.com


RFC: [PATCH] Add warn_if_not_aligned attribute

2017-06-04 Thread H.J. Lu
__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

gcc/

PR c/53037
* c-decl.c (finish_enum): Also copy TYPE_WARN_IF_NOT_ALIGN.
* print-tree.c (print_node): Support TYPE_WARN_IF_NOT_ALIGN.
* stor-layout.c (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.c (build_range_type_1): Likewise.
* tree-core.h (tree_type_common): Add warn_if_not_align.  Set
spare to 18.
* tree.h (TYPE_WARN_IF_NOT_ALIGN): New.
(SET_TYPE_WARN_IF_NOT_ALIGN): Likewise.
* c-family/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/c-decl.c (finish_enum): Copy TYPE_WARN_IF_NOT_ALIGN.

gcc/testsuite/

PR c/53037
* gcc.dg/pr53037-1.c: New test.
---
 gcc/c-family/c-attribs.c | 55 +
 gcc/c/c-decl.c   |  2 ++
 gcc/print-tree.c |  6 ++--
 gcc/stor-layout.c| 45 ++
 gcc/testsuite/gcc.dg/pr53037-1.c | 59 
 gcc/tree-core.h  |  3 +-
 gcc/tree.c   |  1 +
 gcc/tree.h   | 10 +++
 8 files changed, 172 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr53037-1.c

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 695c58c..a76f9f7 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -85,6 +85,8 @@ static tree handle_destructor_attribute (tree *, tree, tree, 
int, bool *);
 static tree handle_mode_attribute (tree *, tree, tree, int, bool *);
 static tree handle_section_attribute (tree *, tree, tree, int, bool *);
 static tree handle_aligned_attribute (tree *, tree, tree, int, bool *);
+static tree handle_warn_if_not_aligned_attribute (tree *, tree, tree,
+ int, bool *);
 static tree handle_weak_attribute (tree *, tree, tree, int, bool *) ;
 static tree handle_noplt_attribute (tree *, tree, tree, int, bool *) ;
 static tree handle_alias_ifunc_attribute (bool, tree *, tree, tree, bool *);
@@ -208,6 +210,9 @@ const struct attribute_spec c_common_attribute_table[] =
  handle_section_attribute, false },
   { "aligned",0, 1, false, false, false,
  handle_aligned_attribute, false },
+  { "warn_if_not_aligned",0, 1, false, false, false,
+ handle_warn_if_not_aligned_attribute,
+ false },
   { "weak",   0, 0, true,  false, false,
  handle_weak_attribute, false },
   { "noplt",   0, 0, true,  false, false,
@@ -1558,12 +1563,13 @@ check_cxx_fundamental_alignment_constraints (tree node,
   return !alignment_too_large_p;
 }
 
-/* Handle a "aligned" attribute; arguments as in
-   struct attribute_spec.handler.  */
+/* Common codes shared by handle_warn_if_not_aligned_attribute and
+   handle_aligned_attribute.  */
 
 static tree
-handle_aligned_attribute (tree *node, tree ARG_UNUSED (name), tree args,
- int flags, bool *no_add_attrs)
+common_handle_aligned_attribute (tree *node, tree args, int flags,
+