Re: RFC: [PATCH] Add warn_if_not_aligned attribute

[Martin Sebor <msebor@gmail.com>]

On 06/06/2017 10:59 AM, H.J. Lu wrote:
> On Tue, Jun 6, 2017 at 9:10 AM, Martin Sebor <msebor@gmail.com> 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 <joseph@codesourcery.com>
>>>> 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