PATCH: Add -Waddress-of-packed-member to GCC 9 porting guide

[Martin Liška]

On 1/15/19 3:19 PM, H.J. Lu wrote:
> On Tue, Jan 15, 2019 at 6:07 AM Martin Liška <mliska@suse.cz> wrote:
>>
>> On 1/14/19 3:14 PM, H.J. Lu wrote:
>>> On Mon, Jan 14, 2019 at 5:53 AM Richard Biener
>>> <richard.guenther@gmail.com> wrote:
>>>>
>>>> On Mon, Jan 14, 2019 at 2:46 PM H.J. Lu <hongjiu.lu@intel.com> wrote:
>>>>>
>>>>> This patch adds -Waddress-of-packed-member to GCC 9 porting guide.
>>>>>
>>>>> OK to install?
>>>>
>>>> The docs fail to mention what to do when the unaligned pointer is _not_
>>>> safe to use.  That is, how do I fix
>>>>
>>>> struct { char c; int i[4]; } s __attribute__((packed));
>>>> int foo()
>>>> {
>>>>   int *p = s.i;
>>>>   return bar (p);
>>>> }
>>>> int bar (int *q)
>>>> {
>>>>   return *q;
>>>> }
>>>>
>>>> for the cases where eliding the pointer isn't easily possible?
>>>
>>> You can't have both packed struct and aligned array at the same time.
>>> The only thing I can say is "don't do it".
>>>
>>>> Please also mention the new warning in changes.html
>>>> (it seems to be enabled by default even?).
>>>
>>> I will add a paragraph.
>>>
>>> H.J.
>>>> IIRC the frontends themselves build "bogus" pointer types
>>>> to aligned data from a simple &s.i[1] because the FIELD_DECLs
>>>> types are naturally aligned.
>>>>
>>>> Richard.
>>>>
>>>>> Thanks.
>>>>>
>>>>> H.J.
>>>>> ---
>>>>> Index: gcc-9/porting_to.html
>>>>> ===================================================================
>>>>> RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-9/porting_to.html,v
>>>>> retrieving revision 1.1
>>>>> diff -u -r1.1 porting_to.html
>>>>> --- gcc-9/porting_to.html       11 Jan 2019 18:21:45 -0000      1.1
>>>>> +++ gcc-9/porting_to.html       14 Jan 2019 13:46:07 -0000
>>>>> @@ -56,13 +56,36 @@
>>>>>        }
>>>>>    </code></pre>
>>>>>
>>>>> +<h2 id="c/cxx">C/C++ language issues</h2>
>>>>> +
>>>>> +<h3 id="Waddress-of-packed-member"><code>-Waddress-of-packed-member</code>
>>>>> +is enabled by default</h3>
>>>>> +
>>>>> +<p>
>>>>> +  When address of packed member of struct or union is taken, it may result
>>>>> +  in an unaligned pointer value.  A new warning
>>>>> +  <code>-Waddress-of-packed-member</code> was added to check alignment at
>>>>> +  pointer assignment.  It warns both unaligned address and unaligned
>>>>> +  pointer.
>>>>> +</p>
>>>>> +
>>>>> +<p>
>>>>> +  If the pointer value is safe to use, you can suppress
>>>>> +  <code>-Waddress-of-packed-member</code> warnings by using pragmas:
>>>>> +</p>
>>>>> +  <pre><code>
>>>>> +    #pragma GCC diagnostic push
>>>>> +    #pragma GCC diagnostic ignored "-Waddress-of-packed-member"
>>>>> +    /* (code for which the warning is to be disabled)  */
>>>>> +    #pragma GCC diagnostic pop
>>>>> +  </code></pre>
>>>>> +
>>>>>  <!--
>>>>>  <h2 id="cxx">C++ language issues</h2>
>>>>>  -->
>>>>>
>>>>>  <!--
>>>>>  <h2 id="fortran">Fortran language issues</h2>
>>>>> --->
>>>>>
>>>>>  <!--
>>>>>  <h2 id="links">Links</h2>
>>>
>>>
>>>
>>
>> Thanks for working on that.
>> Can we please mention a small demonstration of the problem in porting_to?
>>
>> What about this:
>> $ cat pack.c
>> #include <stddef.h>
>>
>> int main(void) {
>>   struct foo {
>>     char c;
>>     int x;
>>   } __attribute__((packed));
>>   struct foo arr[2] = {{'a', 10}, {'b', 20}};
>>   int *p0 = &arr[0].x;
>>   int *p1 = &arr[1].x;
>>   __builtin_printf("sizeof(struct foo)      = %d\n", (int)sizeof(struct foo));
>>   __builtin_printf("offsetof(struct foo, c) = %d\n", (int)offsetof(struct foo, c));
>>   __builtin_printf("offsetof(struct foo, x) = %d\n", (int)offsetof(struct foo, x));
>>   __builtin_printf("arr[0].x = %d\n", arr[0].x);
>>   __builtin_printf("arr[1].x = %d\n", arr[1].x);
>>   __builtin_printf("&arr = %p\n", arr);
>>   __builtin_printf("p0 = %p\n", (void *)p0);
>>   __builtin_printf("p1 = %p\n", (void *)p1);
>>   __builtin_printf("*p0 = %d\n", *p0);
>>   __builtin_printf("*p1 = %d\n", *p1);
>>   return 0;
>> }
>>
>> $ gcc pack.c -fsanitize=undefined && ./a.out
>> pack.c: In function ‘main’:
>> pack.c:9:13: warning: taking address of packed member of ‘struct foo’ may result in an unaligned pointer value [-Waddress-of-packed-member]
>>     9 |   int *p0 = &arr[0].x;
>>       |             ^~~~~~~~~
>> pack.c:10:13: warning: taking address of packed member of ‘struct foo’ may result in an unaligned pointer value [-Waddress-of-packed-member]
>>    10 |   int *p1 = &arr[1].x;
>>       |             ^~~~~~~~~
>> sizeof(struct foo)      = 5
>> offsetof(struct foo, c) = 0
>> offsetof(struct foo, x) = 1
>> arr[0].x = 10
>> arr[1].x = 20
>> &arr = 0x7fffffffdc26
>> p0 = 0x7fffffffdc27
>> p1 = 0x7fffffffdc2c
>> pack.c:19:3: runtime error: load of misaligned address 0x7fffffffdc27 for type 'int', which requires 4 byte alignment
>> 0x7fffffffdc27: note: pointer points here
>>  00 00 00 61 0a  00 00 00 62 14 00 00 00  2c dc ff ff ff 7f 00 00  27 dc ff ff ff 7f 00 00  80 12 40
>>              ^
>> *p0 = 10
>> *p1 = 20
>>
>> ---end---
>>
>> It presents the new warning, run-time memory layout dump and also sanitizer error.
>>
>> Thoughts?
> 
> This doesn't help port to GCC 9.
> 
> 

But it can still go into changes as example of a code
for which the warning is triggered.

Thoughts?