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?
--
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>
