On 1/15/19 5:25 PM, H.J. Lu wrote:
> On Tue, Jan 15, 2019 at 7:05 AM Martin Liška <mli...@suse.cz> wrote:
>>
>> On 1/15/19 3:19 PM, H.J. Lu wrote:
>>> On Tue, Jan 15, 2019 at 6:07 AM Martin Liška <mli...@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.guent...@gmail.com> wrote:
>>>>>>
>>>>>> On Mon, Jan 14, 2019 at 2:46 PM H.J. Lu <hongjiu...@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?
>
> How about this?
>
I would then at least mention what's the natural alignment of type that's
reported
in the warning.
Martin
