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

Message ID 20190114134639.GA21891@lucon.org
State New
Headers show
Series
  • PATCH: Add -Waddress-of-packed-member to GCC 9 porting guide
Related show

Commit Message

H.J. Lu Jan. 14, 2019, 1:46 p.m.
This patch adds -Waddress-of-packed-member to GCC 9 porting guide.

OK to install?

Thanks.

H.J.
---

Comments

Richard Biener Jan. 14, 2019, 1:52 p.m. | #1
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?

Please also mention the new warning in changes.html
(it seems to be enabled by default even?).

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>
H.J. Lu Jan. 14, 2019, 2:14 p.m. | #2
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>




-- 
H.J.
Martin Liška Jan. 15, 2019, 2:07 p.m. | #3
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?
Martin
H.J. Lu Jan. 15, 2019, 2:19 p.m. | #4
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.


-- 
H.J.
Martin Liška Jan. 15, 2019, 3:05 p.m. | #5
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?
H.J. Lu Jan. 15, 2019, 4:25 p.m. | #6
On Tue, Jan 15, 2019 at 7:05 AM Martin Liška <mliska@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 <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?


How about this?

-- 
H.J.
---
Index: changes.html
===================================================================
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-9/changes.html,v
retrieving revision 1.35
diff -u -p -r1.35 changes.html
--- changes.html 15 Jan 2019 13:17:49 -0000 1.35
+++ changes.html 15 Jan 2019 16:24:18 -0000
@@ -84,8 +84,29 @@ a work-in-progress.</p>
   <ul>
       <li><code>-Waddress-of-packed-member</code>, enabled by default,
       warns about an unaligned pointer value from the address of a packed
-      member of a struct or union.
-      </li>
+      member of a struct or union:
+      <pre><code>
+$ cat packed.c
+struct packed
+{
+  char c;
+  int i;
+} __attribute__ ((packed));
+
+int *
+foo (struct packed *p)
+{
+  return &p->i;
+}
+$ gcc -O2 -S packed.c
+packed.c: In function ‘foo’:
+packed.c:10:10: warning: taking address of packed member of ‘struct
packed’ may result in an unaligned pointer value
[-Waddress-of-packed-member]
+   10 |   return &p->i;
+      |          ^~~~~
+$
+      </code></pre>
+      since the pointer returned from function foo may not be an aligned
+      pointer for int.</li>
   </ul></li>
 </ul>
Martin Liška Jan. 16, 2019, 9:43 a.m. | #7
On 1/15/19 5:25 PM, H.J. Lu wrote:
> On Tue, Jan 15, 2019 at 7:05 AM Martin Liška <mliska@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 <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?

> 

> How about this?

> 


I would then at least mention what's the natural alignment of type that's reported
in the warning.

Martin

Patch

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>