Re: [PATCH] Fix issues with -Waddress-of-packed-member

["H.J. Lu" <hjl.tools@gmail.com>]

On Sun, Jan 20, 2019 at 5:29 AM Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
>
> Hi,
>
>
> I tried to build linux yesterday, and became aware that there are a few
> false-positive warnings with -Waddress-of-packed-member:
>
> struct t {
>   char a;
>   int b;
>   int *c;
>   int d[10];
> } __attribute__((packed));
>
> struct t t0;
> struct t *t1;
> struct t **t2;
>
> t2 = &t1;
> i1 = t0.c;
>
> I fixed them quickly with the attached patch, and added a new test case,
> which also revealed a few missing warnings:
>
> struct t t100[10][10];
> struct t (*baz())[10];
>
> t2 = (struct t**) t100;
> t2 = (struct t**) baz();
>
>
> Well I remembered that Joseph wanted me to use min_align_of_type instead
> of TYPE_ALIGN in the -Wcast-align warning, so I changed -Waddress-of-packed-member
> to do that as well.
>
> Since I was not sure if the test coverage is good enough, I added a few more
> test cases, which just make sure the existing warning works properly.

You should also fix

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88928

with something like

diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index 7821cc894a7..4daadc0f6c7 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -2744,9 +2744,14 @@ check_address_or_pointer_of_packed_member (tree type, tre
e rhs)
     rhs = TREE_TYPE (rhs);   /* Function type.  */
   }
       tree rhstype = TREE_TYPE (rhs);
+      tree basetype = type;
+      while (POINTER_TYPE_P (basetype))
+  basetype = TREE_TYPE (basetype);
       if ((POINTER_TYPE_P (rhstype)
      || TREE_CODE (rhstype) == ARRAY_TYPE)
-    && TYPE_PACKED (TREE_TYPE (rhstype)))
+    && TYPE_PACKED (TREE_TYPE (rhstype))
+    && (!RECORD_OR_UNION_TYPE_P (basetype)
+        || TYPE_FIELDS (basetype) != NULL_TREE))
   {
     unsigned int type_align = TYPE_ALIGN_UNIT (type);
     unsigned int rhs_align = TYPE_ALIGN_UNIT (TREE_TYPE (rhstype));
@@ -2759,7 +2764,8 @@ check_address_or_pointer_of_packed_member (tree
type, tree rhs)
           "unaligned pointer value",
           rhstype, rhs_align, type, type_align);
         tree decl = TYPE_STUB_DECL (TREE_TYPE (rhstype));
-        inform (DECL_SOURCE_LOCATION (decl), "defined here");
+        if (decl)
+     inform (DECL_SOURCE_LOCATION (decl), "defined here");
         decl = TYPE_STUB_DECL (type);
         if (decl)
      inform (DECL_SOURCE_LOCATION (decl), "defined here");

> I am however not sure of a warning that is as noisy as this one, should be
> default-enabled, because it might interfere with configure tests.  That might

These codes can lead unaligned access which may be very hard to track
down or fix since  the codes in question may be in a library.  I think it is a
very useful warning.  Besides, these noisy warnings also reveal implementation
issues in GCC, which, otherwise, may not be known for a long time.

> be better to enable this warning, in -Wall or -Wextra, and, well maybe
> enable -Wcast-align=strict also at the same warning level, since it is currently
> not enabled at all, unless explicitly requested, what do you think?
>
>
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
>
>

-- 
H.J.