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.
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
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?
Thanks
Bernd.
2019-01-20 Bernd Edlinger <bernd.edlin...@hotmail.de>
* c-warn.c (check_alignment_of_packed_member): Add a boolean parameter
for rvalue context. Handle rvalues correctly. Use min_align_of_type
instead of TYPE_ALIGN.
(check_address_or_pointer_of_packed_member): Handle rvalues coorrectly.
Use min_align_of_type instead of TYPE_ALIGN_UNIT.
testsuite:
2019-01-20 Bernd Edlinger <bernd.edlin...@hotmail.de>
* c-c++-common/Waddress-of-packed-member-1.c: New test case.
Index: gcc/c-family/c-warn.c
===================================================================
--- gcc/c-family/c-warn.c (revision 268084)
+++ gcc/c-family/c-warn.c (working copy)
@@ -35,6 +35,7 @@ along with GCC; see the file COPYING3. If not see
#include "gimplify.h"
#include "c-family/c-indentation.h"
#include "calls.h"
+#include "stor-layout.h"
/* Print a warning if a constant expression had overflow in folding.
Invoke this function on every expression that the language
@@ -2688,25 +2689,26 @@ warn_for_multistatement_macros (location_t body_lo
}
/* Return struct or union type if the alignment of data memeber, FIELD,
- is less than the alignment of TYPE. Otherwise, return NULL_TREE. */
+ is less than the alignment of TYPE. Otherwise, return NULL_TREE.
+ If RVALUE is true, only arrays evaluate to pointers. */
static tree
-check_alignment_of_packed_member (tree type, tree field)
+check_alignment_of_packed_member (tree type, tree field, bool rvalue)
{
/* Check alignment of the data member. */
if (TREE_CODE (field) == FIELD_DECL
- && (DECL_PACKED (field)
- || TYPE_PACKED (TREE_TYPE (field))))
+ && (DECL_PACKED (field) || TYPE_PACKED (TREE_TYPE (field)))
+ && (!rvalue || TREE_CODE (TREE_TYPE (field)) == ARRAY_TYPE))
{
/* Check the expected alignment against the field alignment. */
- unsigned int type_align = TYPE_ALIGN (type);
+ unsigned int type_align = min_align_of_type (type);
tree context = DECL_CONTEXT (field);
- unsigned int record_align = TYPE_ALIGN (context);
- if ((record_align % type_align) != 0)
+ unsigned int record_align = min_align_of_type (context);
+ if (record_align < type_align)
return context;
tree field_off = byte_position (field);
if (!multiple_of_p (TREE_TYPE (field_off), field_off,
- size_int (type_align / BITS_PER_UNIT)))
+ size_int (type_align)))
return context;
}
@@ -2722,19 +2724,27 @@ static tree
static tree
check_address_or_pointer_of_packed_member (tree type, tree rhs)
{
+ bool rvalue = true;
+
if (INDIRECT_REF_P (rhs))
rhs = TREE_OPERAND (rhs, 0);
if (TREE_CODE (rhs) == ADDR_EXPR)
- rhs = TREE_OPERAND (rhs, 0);
+ {
+ rhs = TREE_OPERAND (rhs, 0);
+ rvalue = false;
+ }
- if (POINTER_TYPE_P (type))
- type = TREE_TYPE (type);
+ if (!POINTER_TYPE_P (type))
+ return NULL_TREE;
+ type = TREE_TYPE (type);
+
if (TREE_CODE (rhs) == PARM_DECL
|| VAR_P (rhs)
|| TREE_CODE (rhs) == CALL_EXPR)
{
+ tree rhstype = TREE_TYPE (rhs);
if (TREE_CODE (rhs) == CALL_EXPR)
{
rhs = CALL_EXPR_FN (rhs); /* Pointer expression. */
@@ -2742,23 +2752,28 @@ check_address_or_pointer_of_packed_member (tree ty
return NULL_TREE;
rhs = TREE_TYPE (rhs); /* Pointer type. */
rhs = TREE_TYPE (rhs); /* Function type. */
+ rhstype = TREE_TYPE (rhs);
+ if (!POINTER_TYPE_P (rhstype))
+ return NULL_TREE;
+ rvalue = true;
}
- tree rhstype = TREE_TYPE (rhs);
- if ((POINTER_TYPE_P (rhstype)
- || TREE_CODE (rhstype) == ARRAY_TYPE)
- && TYPE_PACKED (TREE_TYPE (rhstype)))
+ if (rvalue && POINTER_TYPE_P (rhstype))
+ rhstype = TREE_TYPE (rhstype);
+ while (TREE_CODE (rhstype) == ARRAY_TYPE)
+ rhstype = TREE_TYPE (rhstype);
+ if (TYPE_PACKED (rhstype))
{
- unsigned int type_align = TYPE_ALIGN_UNIT (type);
- unsigned int rhs_align = TYPE_ALIGN_UNIT (TREE_TYPE (rhstype));
- if ((rhs_align % type_align) != 0)
+ unsigned int type_align = min_align_of_type (type);
+ unsigned int rhs_align = min_align_of_type (rhstype);
+ if (rhs_align < type_align)
{
location_t location = EXPR_LOC_OR_LOC (rhs, input_location);
warning_at (location, OPT_Waddress_of_packed_member,
"converting a packed %qT pointer (alignment %d) "
- "to %qT (alignment %d) may result in an "
+ "to a %qT pointer (alignment %d) may result in an "
"unaligned pointer value",
rhstype, rhs_align, type, type_align);
- tree decl = TYPE_STUB_DECL (TREE_TYPE (rhstype));
+ tree decl = TYPE_STUB_DECL (rhstype);
inform (DECL_SOURCE_LOCATION (decl), "defined here");
decl = TYPE_STUB_DECL (type);
if (decl)
@@ -2776,7 +2791,7 @@ check_address_or_pointer_of_packed_member (tree ty
if (TREE_CODE (rhs) == COMPONENT_REF)
{
tree field = TREE_OPERAND (rhs, 1);
- context = check_alignment_of_packed_member (type, field);
+ context = check_alignment_of_packed_member (type, field, rvalue);
if (context)
break;
}
Index: gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c
===================================================================
--- gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c (revision 0)
+++ gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c (working copy)
@@ -0,0 +1,55 @@
+/* { dg-do compile } */
+/* { dg-options "-Waddress-of-packed-member" } */
+
+struct t {
+ char a;
+ int b;
+ int *c;
+ int d[10];
+} __attribute__((packed));
+
+struct t t0;
+struct t t10[10];
+struct t t100[10][10];
+struct t *t1;
+struct t **t2;
+struct t *bar();
+struct t (*baz())[10];
+struct t (*bazz())[10][10];
+int *i1;
+__UINTPTR_TYPE__ u1;
+__UINTPTR_TYPE__ baa();
+
+void foo (void)
+{
+ t1 = &t0; /* { dg-bogus "may result in an unaligned pointer value" } */
+ t1 = t10; /* { dg-bogus "may result in an unaligned pointer value" } */
+ t2 = &t1; /* { dg-bogus "may result in an unaligned pointer value" } */
+ t2 = t2; /* { dg-bogus "may result in an unaligned pointer value" } */
+ t2 = (struct t**)t2; /* { dg-bogus "may result in an unaligned pointer value" } */
+ t1 = (struct t*)t2; /* { dg-bogus "may result in an unaligned pointer value" } */
+ t1 = bar(); /* { dg-bogus "may result in an unaligned pointer value" } */
+ t1 = (struct t*) baz(); /* { dg-bogus "may result in an unaligned pointer value" } */
+ t1 = (struct t*) bazz(); /* { dg-bogus "may result in an unaligned pointer value" } */
+ t1 = *baz(); /* { dg-bogus "may result in an unaligned pointer value" } */
+ t1 = **bazz(); /* { dg-bogus "may result in an unaligned pointer value" } */
+ t1 = (struct t*) baa(); /* { dg-bogus "may result in an unaligned pointer value" } */
+ t2 = (struct t**) baa(); /* { dg-bogus "may result in an unaligned pointer value" } */
+ i1 = t0.c; /* { dg-bogus "may result in an unaligned pointer value" } */
+ i1 = t1->c; /* { dg-bogus "may result in an unaligned pointer value" } */
+ i1 = t10[0].c; /* { dg-bogus "may result in an unaligned pointer value" } */
+ u1 = (__UINTPTR_TYPE__) &t0; /* { dg-bogus "may result in an unaligned pointer value" } */
+ u1 = (__UINTPTR_TYPE__) t1; /* { dg-bogus "may result in an unaligned pointer value" } */
+ t2 = (struct t**) t10; /* { dg-warning "may result in an unaligned pointer value" } */
+ t2 = (struct t**) t100; /* { dg-warning "may result in an unaligned pointer value" } */
+ t2 = (struct t**) t1; /* { dg-warning "may result in an unaligned pointer value" } */
+ t2 = (struct t**) bar(); /* { dg-warning "may result in an unaligned pointer value" } */
+ t2 = (struct t**) baz(); /* { dg-warning "may result in an unaligned pointer value" } */
+ t2 = (struct t**) bazz(); /* { dg-warning "may result in an unaligned pointer value" } */
+ i1 = &t0.b; /* { dg-warning "may result in an unaligned pointer value" } */
+ i1 = &t1->b; /* { dg-warning "may result in an unaligned pointer value" } */
+ i1 = &t10[0].b; /* { dg-warning "may result in an unaligned pointer value" } */
+ i1 = t0.d; /* { dg-warning "may result in an unaligned pointer value" } */
+ i1 = t1->d; /* { dg-warning "may result in an unaligned pointer value" } */
+ i1 = t10[0].d; /* { dg-warning "may result in an unaligned pointer value" } */
+}
