public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 1/2] c-family: -Waddress-of-packed-member and casts
@ 2023-11-22 21:12 Jason Merrill
  2023-11-22 21:12 ` [PATCH 2/2] c-family: rename warn_for_address_or_pointer_of_packed_member Jason Merrill
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Jason Merrill @ 2023-11-22 21:12 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jeff Law, Alexandre Oliva, H . J . Lu

Tested x86_64-pc-linux-gnu, OK for trunk?

-- 8< --

-Waddress-of-packed-member, in addition to the documented warning about
taking the address of a packed member, also warns about casting from
a pointer to a TYPE_PACKED type to a pointer to a type with greater
alignment.

This wrongly warns if the source is a pointer to enum when -fshort-enums
is on, since that is also represented by TYPE_PACKED.

And there's already -Wcast-align to catch casting from pointer to less
aligned type (packed or otherwise) to pointer to more aligned type; even
apart from the enum problem, this seems like a somewhat arbitrary subset of
that warning.  Though that isn't currently on by default.

So, this patch removes the undocumented type-based warning from
-Waddress-of-packed-member.  Some of the tests where the warning is
desirable I changed to use -Wcast-align=strict instead.  The ones that
require -Wno-incompatible-pointer-types, I just removed.

gcc/c-family/ChangeLog:

	* c-warn.cc (check_address_or_pointer_of_packed_member):
	Remove warning based on TYPE_PACKED.

gcc/testsuite/ChangeLog:

	* c-c++-common/Waddress-of-packed-member-1.c: Don't expect
	a warning on the cast cases.
	* c-c++-common/pr51628-35.c: Use -Wcast-align=strict.
	* g++.dg/warn/Waddress-of-packed-member3.C: Likewise.
	* gcc.dg/pr88928.c: Likewise.
	* gcc.dg/pr51628-20.c: Removed.
	* gcc.dg/pr51628-21.c: Removed.
	* gcc.dg/pr51628-25.c: Removed.
---
 gcc/c-family/c-warn.cc                        | 58 +------------------
 .../Waddress-of-packed-member-1.c             | 12 ++--
 gcc/testsuite/c-c++-common/pr51628-35.c       |  6 +-
 .../g++.dg/warn/Waddress-of-packed-member3.C  |  8 +--
 gcc/testsuite/gcc.dg/pr51628-20.c             | 11 ----
 gcc/testsuite/gcc.dg/pr51628-21.c             | 11 ----
 gcc/testsuite/gcc.dg/pr51628-25.c             |  9 ---
 gcc/testsuite/gcc.dg/pr88928.c                |  6 +-
 8 files changed, 19 insertions(+), 102 deletions(-)
 delete mode 100644 gcc/testsuite/gcc.dg/pr51628-20.c
 delete mode 100644 gcc/testsuite/gcc.dg/pr51628-21.c
 delete mode 100644 gcc/testsuite/gcc.dg/pr51628-25.c

diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc
index d2938b91043..2a399ba6d14 100644
--- a/gcc/c-family/c-warn.cc
+++ b/gcc/c-family/c-warn.cc
@@ -2991,10 +2991,9 @@ check_alignment_of_packed_member (tree type, tree field, bool rvalue)
   return NULL_TREE;
 }
 
-/* Return struct or union type if the right hand value, RHS:
-   1. Is a pointer value which isn't aligned to a pointer type TYPE.
-   2. Is an address which takes the unaligned address of packed member
-      of struct or union when assigning to TYPE.
+/* Return struct or union type if the right hand value, RHS
+   is an address which takes the unaligned address of packed member
+   of struct or union when assigning to TYPE.
    Otherwise, return NULL_TREE.  */
 
 static tree
@@ -3021,57 +3020,6 @@ check_address_or_pointer_of_packed_member (tree type, tree rhs)
 
   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.  */
-	  if (rhs == NULL_TREE)
-	    return NULL_TREE;
-	  rhs = TREE_TYPE (rhs);	/* Pointer type.  */
-	  /* We could be called while processing a template and RHS could be
-	     a functor.  In that case it's a class, not a pointer.  */
-	  if (!rhs || !POINTER_TYPE_P (rhs))
-	    return NULL_TREE;
-	  rhs = TREE_TYPE (rhs);	/* Function type.  */
-	  rhstype = TREE_TYPE (rhs);
-	  if (!rhstype || !POINTER_TYPE_P (rhstype))
-	    return NULL_TREE;
-	  rvalue = true;
-	}
-      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 = min_align_of_type (type);
-	  unsigned int rhs_align = min_align_of_type (rhstype);
-	  if (rhs_align < type_align)
-	    {
-	      auto_diagnostic_group d;
-	      location_t location = EXPR_LOC_OR_LOC (rhs, input_location);
-	      if (warning_at (location, OPT_Waddress_of_packed_member,
-			      "converting a packed %qT pointer (alignment %d) "
-			      "to a %qT pointer (alignment %d) may result in "
-			      "an unaligned pointer value",
-			      rhstype, rhs_align, type, type_align))
-		{
-		  tree decl = TYPE_STUB_DECL (rhstype);
-		  if (decl)
-		    inform (DECL_SOURCE_LOCATION (decl), "defined here");
-		  decl = TYPE_STUB_DECL (type);
-		  if (decl)
-		    inform (DECL_SOURCE_LOCATION (decl), "defined here");
-		}
-	    }
-	}
-      return NULL_TREE;
-    }
-
   tree context = NULL_TREE;
 
   /* Check alignment of the object.  */
diff --git a/gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c b/gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c
index 95a376664da..0f5188df70a 100644
--- a/gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c
+++ b/gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c
@@ -52,12 +52,12 @@ void foo (void)
   f0 = *&__real__ t0.f;        /* { dg-bogus "may result in an unaligned pointer value" } */
   f0 = *&__imag__ t0.f;        /* { dg-bogus "may result in an unaligned pointer value" } */
   i1 = (&t0.c, (int*) 0);      /* { dg-bogus "may result in an unaligned pointer value" } */
-  t2 = (struct t**) t10;     /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
-  t2 = (struct t**) t100;    /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
-  t2 = (struct t**) t1;      /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
-  t2 = (struct t**) bar();   /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
-  t2 = (struct t**) baz();   /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
-  t2 = (struct t**) bazz();  /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
+  t2 = (struct t**) t10;       /* { dg-bogus "may result in an unaligned pointer value" } */
+  t2 = (struct t**) t100;      /* { dg-bogus "may result in an unaligned pointer value" } */
+  t2 = (struct t**) t1;        /* { dg-bogus "may result in an unaligned pointer value" } */
+  t2 = (struct t**) bar();     /* { dg-bogus "may result in an unaligned pointer value" } */
+  t2 = (struct t**) baz();     /* { dg-bogus "may result in an unaligned pointer value" } */
+  t2 = (struct t**) bazz();    /* { dg-bogus "may result in an unaligned pointer value" } */
   i1 = &t0.b;                /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
   i1 = &t1->b;               /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
   i1 = &t10[0].b;            /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
diff --git a/gcc/testsuite/c-c++-common/pr51628-35.c b/gcc/testsuite/c-c++-common/pr51628-35.c
index fa37d99beb7..a88c19ea0df 100644
--- a/gcc/testsuite/c-c++-common/pr51628-35.c
+++ b/gcc/testsuite/c-c++-common/pr51628-35.c
@@ -1,6 +1,6 @@
 /* PR c/51628.  */
 /* { dg-do compile } */
-/* { dg-options "-O" } */
+/* { dg-options "-O -Wcast-align=strict" } */
 
 struct B { int i; };
 struct C { struct B b; } __attribute__ ((packed));
@@ -12,12 +12,12 @@ long *
 foo1 (void)
 {
   return (long *) p;
-/* { dg-warning "may result in an unaligned pointer value" "" { target { ! default_packed } } .-1 } */
+/* { dg-warning "increases required alignment" "" { target { ! default_packed } } .-1 } */
 }
 
 long *
 foo2 (void)
 {
   return (long *) bar ();
-/* { dg-warning "may result in an unaligned pointer value" "" { target { ! default_packed } } .-1 } */
+/* { dg-warning "increases required alignment" "" { target { ! default_packed } } .-1 } */
 }
diff --git a/gcc/testsuite/g++.dg/warn/Waddress-of-packed-member3.C b/gcc/testsuite/g++.dg/warn/Waddress-of-packed-member3.C
index aeffb969c01..28dd05d366c 100644
--- a/gcc/testsuite/g++.dg/warn/Waddress-of-packed-member3.C
+++ b/gcc/testsuite/g++.dg/warn/Waddress-of-packed-member3.C
@@ -1,5 +1,5 @@
 // { dg-do compile { target { ! default_packed } } }
-// Test that -Waddress-of-packed-member works with member functions.
+// { dg-additional-options -Wcast-align=strict }
 
 struct S {
   char c;
@@ -16,8 +16,8 @@ S**
 f ()
 {
   S **s;
-  s = reinterpret_cast<S**>(foo ()); // { dg-warning "converting a packed" }
-  s = reinterpret_cast<S**>(x.memfn ()); // { dg-warning "converting a packed" }
-  s = reinterpret_cast<S**>(X::smemfn ()); // { dg-warning "converting a packed" }
+  s = reinterpret_cast<S**>(foo ()); // { dg-warning "increases required alignment" }
+  s = reinterpret_cast<S**>(x.memfn ()); // { dg-warning "increases required alignment" }
+  s = reinterpret_cast<S**>(X::smemfn ()); // { dg-warning "increases required alignment" }
   return s;
 }
diff --git a/gcc/testsuite/gcc.dg/pr51628-20.c b/gcc/testsuite/gcc.dg/pr51628-20.c
deleted file mode 100644
index 2249d85098b..00000000000
--- a/gcc/testsuite/gcc.dg/pr51628-20.c
+++ /dev/null
@@ -1,11 +0,0 @@
-/* PR c/51628.  */
-/* { dg-do compile } */
-/* { dg-options "-O -Wno-incompatible-pointer-types" } */
-
-struct B { int i; };
-struct C { struct B b; } __attribute__ ((packed));
-
-extern struct C *p;
-
-long* g8 (void) { return p; }
-/* { dg-warning "may result in an unaligned pointer value" "" { target { ! default_packed } } .-1 } */
diff --git a/gcc/testsuite/gcc.dg/pr51628-21.c b/gcc/testsuite/gcc.dg/pr51628-21.c
deleted file mode 100644
index f1adbe64002..00000000000
--- a/gcc/testsuite/gcc.dg/pr51628-21.c
+++ /dev/null
@@ -1,11 +0,0 @@
-/* PR c/51628.  */
-/* { dg-do compile } */
-/* { dg-options "-O -Wno-incompatible-pointer-types" } */
-
-struct B { int i; };
-struct C { struct B b; } __attribute__ ((packed));
-
-extern struct C p[];
-
-long* g8 (void) { return p; }
-/* { dg-warning "may result in an unaligned pointer value" "" { target { ! default_packed } } .-1 } */
diff --git a/gcc/testsuite/gcc.dg/pr51628-25.c b/gcc/testsuite/gcc.dg/pr51628-25.c
deleted file mode 100644
index f00d9b1bcac..00000000000
--- a/gcc/testsuite/gcc.dg/pr51628-25.c
+++ /dev/null
@@ -1,9 +0,0 @@
-/* PR c/51628.  */
-/* { dg-do compile } */
-/* { dg-options "-O -Wno-incompatible-pointer-types" } */
-
-struct B { int i; };
-struct C { struct B b; } __attribute__ ((packed));
-
-long* g8 (struct C *p) { return p; }
-/* { dg-warning "may result in an unaligned pointer value" "" { target { ! default_packed } } .-1 } */
diff --git a/gcc/testsuite/gcc.dg/pr88928.c b/gcc/testsuite/gcc.dg/pr88928.c
index 0b6c1d70f05..1d176d6d51d 100644
--- a/gcc/testsuite/gcc.dg/pr88928.c
+++ b/gcc/testsuite/gcc.dg/pr88928.c
@@ -1,6 +1,6 @@
-/* { dg-do compile } */
-/* { dg-options "-Wno-pedantic -Waddress-of-packed-member" } */
+/* { dg-do compile { target { ! default_packed } } } */
+/* { dg-options "-Wno-pedantic -Waddress-of-packed-member -Wcast-align=strict" } */
 struct a { } __attribute__((__packed__));
 void c (struct a **);
 void d (const struct a *b) { c ((struct a **) b); }
-/* { dg-warning "may result in an unaligned pointer value" "" { target { ! default_packed } } .-1 } */
+/* { dg-warning "increases required alignment" "" { target *-*-* } .-1 } */

base-commit: 65bd6de0de57abc86931a5e0b9a8d453ad530004
-- 
2.39.3


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 2/2] c-family: rename warn_for_address_or_pointer_of_packed_member
  2023-11-22 21:12 [PATCH 1/2] c-family: -Waddress-of-packed-member and casts Jason Merrill
@ 2023-11-22 21:12 ` Jason Merrill
  2023-12-09  2:22 ` [PATCH 1/2] c-family: -Waddress-of-packed-member and casts Alexandre Oliva
  2023-12-11  7:28 ` Richard Biener
  2 siblings, 0 replies; 14+ messages in thread
From: Jason Merrill @ 2023-11-22 21:12 UTC (permalink / raw)
  To: gcc-patches

Following the last patch, let's rename the functions to reflect the change
in behavior.

gcc/c-family/ChangeLog:

	* c-warn.cc (check_address_or_pointer_of_packed_member):
	Rename to check_address_of_packed_member.
	(check_and_warn_address_or_pointer_of_packed_member):
	Rename to check_and_warn_address_of_packed_member.
	(warn_for_address_or_pointer_of_packed_member):
	Rename to warn_for_address_of_packed_member.
	* c-common.h: Adjust.

gcc/c/ChangeLog:

	* c-typeck.cc (convert_for_assignment): Adjust call to
	warn_for_address_of_packed_member.

gcc/cp/ChangeLog:

	* call.cc (convert_for_arg_passing)
	* typeck.cc (convert_for_assignment): Adjust call to
	warn_for_address_of_packed_member.
---
 gcc/c-family/c-common.h |  2 +-
 gcc/c-family/c-warn.cc  | 32 ++++++++++++++------------------
 gcc/c/c-typeck.cc       |  4 ++--
 gcc/cp/call.cc          |  2 +-
 gcc/cp/typeck.cc        |  2 +-
 5 files changed, 19 insertions(+), 23 deletions(-)

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index b57e83d7c5d..9380452a93b 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1482,7 +1482,7 @@ extern void warnings_for_convert_and_check (location_t, tree, tree, tree);
 extern void c_do_switch_warnings (splay_tree, location_t, tree, tree, bool);
 extern void warn_for_omitted_condop (location_t, tree);
 extern bool warn_for_restrict (unsigned, tree *, unsigned);
-extern void warn_for_address_or_pointer_of_packed_member (tree, tree);
+extern void warn_for_address_of_packed_member (tree, tree);
 extern void warn_parm_array_mismatch (location_t, tree, tree);
 extern void maybe_warn_sizeof_array_div (location_t, tree, tree, tree, tree);
 extern void do_warn_array_compare (location_t, tree_code, tree, tree);
diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc
index 2a399ba6d14..abe66dd3030 100644
--- a/gcc/c-family/c-warn.cc
+++ b/gcc/c-family/c-warn.cc
@@ -2991,13 +2991,13 @@ check_alignment_of_packed_member (tree type, tree field, bool rvalue)
   return NULL_TREE;
 }
 
-/* Return struct or union type if the right hand value, RHS
+/* Return struct or union type if the right hand value, RHS,
    is an address which takes the unaligned address of packed member
    of struct or union when assigning to TYPE.
    Otherwise, return NULL_TREE.  */
 
 static tree
-check_address_or_pointer_of_packed_member (tree type, tree rhs)
+check_address_of_packed_member (tree type, tree rhs)
 {
   bool rvalue = true;
   bool indirect = false;
@@ -3042,14 +3042,12 @@ check_address_or_pointer_of_packed_member (tree type, tree rhs)
   return context;
 }
 
-/* Check and warn if the right hand value, RHS:
-   1. Is a pointer value which isn't aligned to a pointer type TYPE.
-   2. Is an address which takes the unaligned address of packed member
-      of struct or union when assigning to TYPE.
- */
+/* Check and warn if the right hand value, RHS,
+   is an address which takes the unaligned address of packed member
+   of struct or union when assigning to TYPE.  */
 
 static void
-check_and_warn_address_or_pointer_of_packed_member (tree type, tree rhs)
+check_and_warn_address_of_packed_member (tree type, tree rhs)
 {
   bool nop_p = false;
   tree orig_rhs;
@@ -3067,11 +3065,11 @@ check_and_warn_address_or_pointer_of_packed_member (tree type, tree rhs)
   if (TREE_CODE (rhs) == COND_EXPR)
     {
       /* Check the THEN path.  */
-      check_and_warn_address_or_pointer_of_packed_member
+      check_and_warn_address_of_packed_member
 	(type, TREE_OPERAND (rhs, 1));
 
       /* Check the ELSE path.  */
-      check_and_warn_address_or_pointer_of_packed_member
+      check_and_warn_address_of_packed_member
 	(type, TREE_OPERAND (rhs, 2));
     }
   else
@@ -3095,7 +3093,7 @@ check_and_warn_address_or_pointer_of_packed_member (tree type, tree rhs)
 	}
 
       tree context
-	= check_address_or_pointer_of_packed_member (type, rhs);
+	= check_address_of_packed_member (type, rhs);
       if (context)
 	{
 	  location_t loc = EXPR_LOC_OR_LOC (rhs, input_location);
@@ -3107,14 +3105,12 @@ check_and_warn_address_or_pointer_of_packed_member (tree type, tree rhs)
     }
 }
 
-/* Warn if the right hand value, RHS:
-   1. Is a pointer value which isn't aligned to a pointer type TYPE.
-   2. Is an address which takes the unaligned address of packed member
-      of struct or union when assigning to TYPE.
-*/
+/* Warn if the right hand value, RHS,
+   is an address which takes the unaligned address of packed member
+   of struct or union when assigning to TYPE.  */
 
 void
-warn_for_address_or_pointer_of_packed_member (tree type, tree rhs)
+warn_for_address_of_packed_member (tree type, tree rhs)
 {
   if (!warn_address_of_packed_member)
     return;
@@ -3123,7 +3119,7 @@ warn_for_address_or_pointer_of_packed_member (tree type, tree rhs)
   if (!POINTER_TYPE_P (type))
     return;
 
-  check_and_warn_address_or_pointer_of_packed_member (type, rhs);
+  check_and_warn_address_of_packed_member (type, rhs);
 }
 
 /* Return EXPR + 1.  Convenience helper used below.  */
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index 1dbb4471a88..3960c407ca7 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -6942,7 +6942,7 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
 
   if (TYPE_MAIN_VARIANT (type) == TYPE_MAIN_VARIANT (rhstype))
     {
-      warn_for_address_or_pointer_of_packed_member (type, orig_rhs);
+      warn_for_address_of_packed_member (type, orig_rhs);
       return rhs;
     }
 
@@ -7599,7 +7599,7 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
 
       /* If RHS isn't an address, check pointer or array of packed
 	 struct or union.  */
-      warn_for_address_or_pointer_of_packed_member (type, orig_rhs);
+      warn_for_address_of_packed_member (type, orig_rhs);
 
       return convert (type, rhs);
     }
diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 81b104f4b40..516e02ceb32 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -9306,7 +9306,7 @@ convert_for_arg_passing (tree type, tree val, tsubst_flags_t complain)
     }
 
   if (complain & tf_warning)
-    warn_for_address_or_pointer_of_packed_member (type, val);
+    warn_for_address_of_packed_member (type, val);
 
   return val;
 }
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index e995fb6ddd7..f2e90a86148 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -10378,7 +10378,7 @@ convert_for_assignment (tree type, tree rhs,
     maybe_warn_unparenthesized_assignment (rhs, complain);
 
   if (complain & tf_warning)
-    warn_for_address_or_pointer_of_packed_member (type, rhs);
+    warn_for_address_of_packed_member (type, rhs);
 
   return perform_implicit_conversion_flags (strip_top_quals (type), rhs,
 					    complain, flags);
-- 
2.39.3


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] c-family: -Waddress-of-packed-member and casts
  2023-11-22 21:12 [PATCH 1/2] c-family: -Waddress-of-packed-member and casts Jason Merrill
  2023-11-22 21:12 ` [PATCH 2/2] c-family: rename warn_for_address_or_pointer_of_packed_member Jason Merrill
@ 2023-12-09  2:22 ` Alexandre Oliva
  2023-12-11  7:28 ` Richard Biener
  2 siblings, 0 replies; 14+ messages in thread
From: Alexandre Oliva @ 2023-12-09  2:22 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches, Jeff Law, H . J . Lu

On Nov 22, 2023, Jason Merrill <jason@redhat.com> wrote:

> Tested x86_64-pc-linux-gnu, OK for trunk?

FYI, Linaro's regression tester let me know that my patch reversal, that
expected this patch to go in instead, caused two "regressions".
https://linaro.atlassian.net/browse/GNU-1067

-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] c-family: -Waddress-of-packed-member and casts
  2023-11-22 21:12 [PATCH 1/2] c-family: -Waddress-of-packed-member and casts Jason Merrill
  2023-11-22 21:12 ` [PATCH 2/2] c-family: rename warn_for_address_or_pointer_of_packed_member Jason Merrill
  2023-12-09  2:22 ` [PATCH 1/2] c-family: -Waddress-of-packed-member and casts Alexandre Oliva
@ 2023-12-11  7:28 ` Richard Biener
  2024-02-07 16:19   ` Torbjorn SVENSSON
  2 siblings, 1 reply; 14+ messages in thread
From: Richard Biener @ 2023-12-11  7:28 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches, Jeff Law, Alexandre Oliva, H . J . Lu

On Wed, Nov 22, 2023 at 11:45 PM Jason Merrill <jason@redhat.com> wrote:
>
> Tested x86_64-pc-linux-gnu, OK for trunk?

OK

> -- 8< --
>
> -Waddress-of-packed-member, in addition to the documented warning about
> taking the address of a packed member, also warns about casting from
> a pointer to a TYPE_PACKED type to a pointer to a type with greater
> alignment.
>
> This wrongly warns if the source is a pointer to enum when -fshort-enums
> is on, since that is also represented by TYPE_PACKED.
>
> And there's already -Wcast-align to catch casting from pointer to less
> aligned type (packed or otherwise) to pointer to more aligned type; even
> apart from the enum problem, this seems like a somewhat arbitrary subset of
> that warning.  Though that isn't currently on by default.
>
> So, this patch removes the undocumented type-based warning from
> -Waddress-of-packed-member.  Some of the tests where the warning is
> desirable I changed to use -Wcast-align=strict instead.  The ones that
> require -Wno-incompatible-pointer-types, I just removed.
>
> gcc/c-family/ChangeLog:
>
>         * c-warn.cc (check_address_or_pointer_of_packed_member):
>         Remove warning based on TYPE_PACKED.
>
> gcc/testsuite/ChangeLog:
>
>         * c-c++-common/Waddress-of-packed-member-1.c: Don't expect
>         a warning on the cast cases.
>         * c-c++-common/pr51628-35.c: Use -Wcast-align=strict.
>         * g++.dg/warn/Waddress-of-packed-member3.C: Likewise.
>         * gcc.dg/pr88928.c: Likewise.
>         * gcc.dg/pr51628-20.c: Removed.
>         * gcc.dg/pr51628-21.c: Removed.
>         * gcc.dg/pr51628-25.c: Removed.
> ---
>  gcc/c-family/c-warn.cc                        | 58 +------------------
>  .../Waddress-of-packed-member-1.c             | 12 ++--
>  gcc/testsuite/c-c++-common/pr51628-35.c       |  6 +-
>  .../g++.dg/warn/Waddress-of-packed-member3.C  |  8 +--
>  gcc/testsuite/gcc.dg/pr51628-20.c             | 11 ----
>  gcc/testsuite/gcc.dg/pr51628-21.c             | 11 ----
>  gcc/testsuite/gcc.dg/pr51628-25.c             |  9 ---
>  gcc/testsuite/gcc.dg/pr88928.c                |  6 +-
>  8 files changed, 19 insertions(+), 102 deletions(-)
>  delete mode 100644 gcc/testsuite/gcc.dg/pr51628-20.c
>  delete mode 100644 gcc/testsuite/gcc.dg/pr51628-21.c
>  delete mode 100644 gcc/testsuite/gcc.dg/pr51628-25.c
>
> diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc
> index d2938b91043..2a399ba6d14 100644
> --- a/gcc/c-family/c-warn.cc
> +++ b/gcc/c-family/c-warn.cc
> @@ -2991,10 +2991,9 @@ check_alignment_of_packed_member (tree type, tree field, bool rvalue)
>    return NULL_TREE;
>  }
>
> -/* Return struct or union type if the right hand value, RHS:
> -   1. Is a pointer value which isn't aligned to a pointer type TYPE.
> -   2. Is an address which takes the unaligned address of packed member
> -      of struct or union when assigning to TYPE.
> +/* Return struct or union type if the right hand value, RHS
> +   is an address which takes the unaligned address of packed member
> +   of struct or union when assigning to TYPE.
>     Otherwise, return NULL_TREE.  */
>
>  static tree
> @@ -3021,57 +3020,6 @@ check_address_or_pointer_of_packed_member (tree type, tree rhs)
>
>    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.  */
> -         if (rhs == NULL_TREE)
> -           return NULL_TREE;
> -         rhs = TREE_TYPE (rhs);        /* Pointer type.  */
> -         /* We could be called while processing a template and RHS could be
> -            a functor.  In that case it's a class, not a pointer.  */
> -         if (!rhs || !POINTER_TYPE_P (rhs))
> -           return NULL_TREE;
> -         rhs = TREE_TYPE (rhs);        /* Function type.  */
> -         rhstype = TREE_TYPE (rhs);
> -         if (!rhstype || !POINTER_TYPE_P (rhstype))
> -           return NULL_TREE;
> -         rvalue = true;
> -       }
> -      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 = min_align_of_type (type);
> -         unsigned int rhs_align = min_align_of_type (rhstype);
> -         if (rhs_align < type_align)
> -           {
> -             auto_diagnostic_group d;
> -             location_t location = EXPR_LOC_OR_LOC (rhs, input_location);
> -             if (warning_at (location, OPT_Waddress_of_packed_member,
> -                             "converting a packed %qT pointer (alignment %d) "
> -                             "to a %qT pointer (alignment %d) may result in "
> -                             "an unaligned pointer value",
> -                             rhstype, rhs_align, type, type_align))
> -               {
> -                 tree decl = TYPE_STUB_DECL (rhstype);
> -                 if (decl)
> -                   inform (DECL_SOURCE_LOCATION (decl), "defined here");
> -                 decl = TYPE_STUB_DECL (type);
> -                 if (decl)
> -                   inform (DECL_SOURCE_LOCATION (decl), "defined here");
> -               }
> -           }
> -       }
> -      return NULL_TREE;
> -    }
> -
>    tree context = NULL_TREE;
>
>    /* Check alignment of the object.  */
> diff --git a/gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c b/gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c
> index 95a376664da..0f5188df70a 100644
> --- a/gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c
> +++ b/gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c
> @@ -52,12 +52,12 @@ void foo (void)
>    f0 = *&__real__ t0.f;        /* { dg-bogus "may result in an unaligned pointer value" } */
>    f0 = *&__imag__ t0.f;        /* { dg-bogus "may result in an unaligned pointer value" } */
>    i1 = (&t0.c, (int*) 0);      /* { dg-bogus "may result in an unaligned pointer value" } */
> -  t2 = (struct t**) t10;     /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
> -  t2 = (struct t**) t100;    /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
> -  t2 = (struct t**) t1;      /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
> -  t2 = (struct t**) bar();   /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
> -  t2 = (struct t**) baz();   /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
> -  t2 = (struct t**) bazz();  /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
> +  t2 = (struct t**) t10;       /* { dg-bogus "may result in an unaligned pointer value" } */
> +  t2 = (struct t**) t100;      /* { dg-bogus "may result in an unaligned pointer value" } */
> +  t2 = (struct t**) t1;        /* { dg-bogus "may result in an unaligned pointer value" } */
> +  t2 = (struct t**) bar();     /* { dg-bogus "may result in an unaligned pointer value" } */
> +  t2 = (struct t**) baz();     /* { dg-bogus "may result in an unaligned pointer value" } */
> +  t2 = (struct t**) bazz();    /* { dg-bogus "may result in an unaligned pointer value" } */
>    i1 = &t0.b;                /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
>    i1 = &t1->b;               /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
>    i1 = &t10[0].b;            /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
> diff --git a/gcc/testsuite/c-c++-common/pr51628-35.c b/gcc/testsuite/c-c++-common/pr51628-35.c
> index fa37d99beb7..a88c19ea0df 100644
> --- a/gcc/testsuite/c-c++-common/pr51628-35.c
> +++ b/gcc/testsuite/c-c++-common/pr51628-35.c
> @@ -1,6 +1,6 @@
>  /* PR c/51628.  */
>  /* { dg-do compile } */
> -/* { dg-options "-O" } */
> +/* { dg-options "-O -Wcast-align=strict" } */
>
>  struct B { int i; };
>  struct C { struct B b; } __attribute__ ((packed));
> @@ -12,12 +12,12 @@ long *
>  foo1 (void)
>  {
>    return (long *) p;
> -/* { dg-warning "may result in an unaligned pointer value" "" { target { ! default_packed } } .-1 } */
> +/* { dg-warning "increases required alignment" "" { target { ! default_packed } } .-1 } */
>  }
>
>  long *
>  foo2 (void)
>  {
>    return (long *) bar ();
> -/* { dg-warning "may result in an unaligned pointer value" "" { target { ! default_packed } } .-1 } */
> +/* { dg-warning "increases required alignment" "" { target { ! default_packed } } .-1 } */
>  }
> diff --git a/gcc/testsuite/g++.dg/warn/Waddress-of-packed-member3.C b/gcc/testsuite/g++.dg/warn/Waddress-of-packed-member3.C
> index aeffb969c01..28dd05d366c 100644
> --- a/gcc/testsuite/g++.dg/warn/Waddress-of-packed-member3.C
> +++ b/gcc/testsuite/g++.dg/warn/Waddress-of-packed-member3.C
> @@ -1,5 +1,5 @@
>  // { dg-do compile { target { ! default_packed } } }
> -// Test that -Waddress-of-packed-member works with member functions.
> +// { dg-additional-options -Wcast-align=strict }
>
>  struct S {
>    char c;
> @@ -16,8 +16,8 @@ S**
>  f ()
>  {
>    S **s;
> -  s = reinterpret_cast<S**>(foo ()); // { dg-warning "converting a packed" }
> -  s = reinterpret_cast<S**>(x.memfn ()); // { dg-warning "converting a packed" }
> -  s = reinterpret_cast<S**>(X::smemfn ()); // { dg-warning "converting a packed" }
> +  s = reinterpret_cast<S**>(foo ()); // { dg-warning "increases required alignment" }
> +  s = reinterpret_cast<S**>(x.memfn ()); // { dg-warning "increases required alignment" }
> +  s = reinterpret_cast<S**>(X::smemfn ()); // { dg-warning "increases required alignment" }
>    return s;
>  }
> diff --git a/gcc/testsuite/gcc.dg/pr51628-20.c b/gcc/testsuite/gcc.dg/pr51628-20.c
> deleted file mode 100644
> index 2249d85098b..00000000000
> --- a/gcc/testsuite/gcc.dg/pr51628-20.c
> +++ /dev/null
> @@ -1,11 +0,0 @@
> -/* PR c/51628.  */
> -/* { dg-do compile } */
> -/* { dg-options "-O -Wno-incompatible-pointer-types" } */
> -
> -struct B { int i; };
> -struct C { struct B b; } __attribute__ ((packed));
> -
> -extern struct C *p;
> -
> -long* g8 (void) { return p; }
> -/* { dg-warning "may result in an unaligned pointer value" "" { target { ! default_packed } } .-1 } */
> diff --git a/gcc/testsuite/gcc.dg/pr51628-21.c b/gcc/testsuite/gcc.dg/pr51628-21.c
> deleted file mode 100644
> index f1adbe64002..00000000000
> --- a/gcc/testsuite/gcc.dg/pr51628-21.c
> +++ /dev/null
> @@ -1,11 +0,0 @@
> -/* PR c/51628.  */
> -/* { dg-do compile } */
> -/* { dg-options "-O -Wno-incompatible-pointer-types" } */
> -
> -struct B { int i; };
> -struct C { struct B b; } __attribute__ ((packed));
> -
> -extern struct C p[];
> -
> -long* g8 (void) { return p; }
> -/* { dg-warning "may result in an unaligned pointer value" "" { target { ! default_packed } } .-1 } */
> diff --git a/gcc/testsuite/gcc.dg/pr51628-25.c b/gcc/testsuite/gcc.dg/pr51628-25.c
> deleted file mode 100644
> index f00d9b1bcac..00000000000
> --- a/gcc/testsuite/gcc.dg/pr51628-25.c
> +++ /dev/null
> @@ -1,9 +0,0 @@
> -/* PR c/51628.  */
> -/* { dg-do compile } */
> -/* { dg-options "-O -Wno-incompatible-pointer-types" } */
> -
> -struct B { int i; };
> -struct C { struct B b; } __attribute__ ((packed));
> -
> -long* g8 (struct C *p) { return p; }
> -/* { dg-warning "may result in an unaligned pointer value" "" { target { ! default_packed } } .-1 } */
> diff --git a/gcc/testsuite/gcc.dg/pr88928.c b/gcc/testsuite/gcc.dg/pr88928.c
> index 0b6c1d70f05..1d176d6d51d 100644
> --- a/gcc/testsuite/gcc.dg/pr88928.c
> +++ b/gcc/testsuite/gcc.dg/pr88928.c
> @@ -1,6 +1,6 @@
> -/* { dg-do compile } */
> -/* { dg-options "-Wno-pedantic -Waddress-of-packed-member" } */
> +/* { dg-do compile { target { ! default_packed } } } */
> +/* { dg-options "-Wno-pedantic -Waddress-of-packed-member -Wcast-align=strict" } */
>  struct a { } __attribute__((__packed__));
>  void c (struct a **);
>  void d (const struct a *b) { c ((struct a **) b); }
> -/* { dg-warning "may result in an unaligned pointer value" "" { target { ! default_packed } } .-1 } */
> +/* { dg-warning "increases required alignment" "" { target *-*-* } .-1 } */
>
> base-commit: 65bd6de0de57abc86931a5e0b9a8d453ad530004
> --
> 2.39.3
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/2] c-family: -Waddress-of-packed-member and casts
  2023-12-11  7:28 ` Richard Biener
@ 2024-02-07 16:19   ` Torbjorn SVENSSON
  2024-02-22  8:51     ` [PING] " Torbjorn SVENSSON
  0 siblings, 1 reply; 14+ messages in thread
From: Torbjorn SVENSSON @ 2024-02-07 16:19 UTC (permalink / raw)
  To: Richard Biener, Jason Merrill
  Cc: gcc-patches, Jeff Law, Alexandre Oliva, H . J . Lu, Yvan Roux

Hi,

Is it okay to backport b7e4a4c626eeeb32c291d5bbbaa148c5081b6bfd to 
releases/gcc-13?

Without this backport, I see these failures on arm-none-eabi:

FAIL: 
gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c 
(test for excess errors)
FAIL: gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early.c 
(test for excess errors)

Kind regards,
Torbjörn

On 2023-12-11 08:28, Richard Biener wrote:
> On Wed, Nov 22, 2023 at 11:45 PM Jason Merrill <jason@redhat.com> wrote:
>>
>> Tested x86_64-pc-linux-gnu, OK for trunk?
> 
> OK
> 
>> -- 8< --
>>
>> -Waddress-of-packed-member, in addition to the documented warning about
>> taking the address of a packed member, also warns about casting from
>> a pointer to a TYPE_PACKED type to a pointer to a type with greater
>> alignment.
>>
>> This wrongly warns if the source is a pointer to enum when -fshort-enums
>> is on, since that is also represented by TYPE_PACKED.
>>
>> And there's already -Wcast-align to catch casting from pointer to less
>> aligned type (packed or otherwise) to pointer to more aligned type; even
>> apart from the enum problem, this seems like a somewhat arbitrary subset of
>> that warning.  Though that isn't currently on by default.
>>
>> So, this patch removes the undocumented type-based warning from
>> -Waddress-of-packed-member.  Some of the tests where the warning is
>> desirable I changed to use -Wcast-align=strict instead.  The ones that
>> require -Wno-incompatible-pointer-types, I just removed.
>>
>> gcc/c-family/ChangeLog:
>>
>>          * c-warn.cc (check_address_or_pointer_of_packed_member):
>>          Remove warning based on TYPE_PACKED.
>>
>> gcc/testsuite/ChangeLog:
>>
>>          * c-c++-common/Waddress-of-packed-member-1.c: Don't expect
>>          a warning on the cast cases.
>>          * c-c++-common/pr51628-35.c: Use -Wcast-align=strict.
>>          * g++.dg/warn/Waddress-of-packed-member3.C: Likewise.
>>          * gcc.dg/pr88928.c: Likewise.
>>          * gcc.dg/pr51628-20.c: Removed.
>>          * gcc.dg/pr51628-21.c: Removed.
>>          * gcc.dg/pr51628-25.c: Removed.
>> ---
>>   gcc/c-family/c-warn.cc                        | 58 +------------------
>>   .../Waddress-of-packed-member-1.c             | 12 ++--
>>   gcc/testsuite/c-c++-common/pr51628-35.c       |  6 +-
>>   .../g++.dg/warn/Waddress-of-packed-member3.C  |  8 +--
>>   gcc/testsuite/gcc.dg/pr51628-20.c             | 11 ----
>>   gcc/testsuite/gcc.dg/pr51628-21.c             | 11 ----
>>   gcc/testsuite/gcc.dg/pr51628-25.c             |  9 ---
>>   gcc/testsuite/gcc.dg/pr88928.c                |  6 +-
>>   8 files changed, 19 insertions(+), 102 deletions(-)
>>   delete mode 100644 gcc/testsuite/gcc.dg/pr51628-20.c
>>   delete mode 100644 gcc/testsuite/gcc.dg/pr51628-21.c
>>   delete mode 100644 gcc/testsuite/gcc.dg/pr51628-25.c
>>
>> diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc
>> index d2938b91043..2a399ba6d14 100644
>> --- a/gcc/c-family/c-warn.cc
>> +++ b/gcc/c-family/c-warn.cc
>> @@ -2991,10 +2991,9 @@ check_alignment_of_packed_member (tree type, tree field, bool rvalue)
>>     return NULL_TREE;
>>   }
>>
>> -/* Return struct or union type if the right hand value, RHS:
>> -   1. Is a pointer value which isn't aligned to a pointer type TYPE.
>> -   2. Is an address which takes the unaligned address of packed member
>> -      of struct or union when assigning to TYPE.
>> +/* Return struct or union type if the right hand value, RHS
>> +   is an address which takes the unaligned address of packed member
>> +   of struct or union when assigning to TYPE.
>>      Otherwise, return NULL_TREE.  */
>>
>>   static tree
>> @@ -3021,57 +3020,6 @@ check_address_or_pointer_of_packed_member (tree type, tree rhs)
>>
>>     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.  */
>> -         if (rhs == NULL_TREE)
>> -           return NULL_TREE;
>> -         rhs = TREE_TYPE (rhs);        /* Pointer type.  */
>> -         /* We could be called while processing a template and RHS could be
>> -            a functor.  In that case it's a class, not a pointer.  */
>> -         if (!rhs || !POINTER_TYPE_P (rhs))
>> -           return NULL_TREE;
>> -         rhs = TREE_TYPE (rhs);        /* Function type.  */
>> -         rhstype = TREE_TYPE (rhs);
>> -         if (!rhstype || !POINTER_TYPE_P (rhstype))
>> -           return NULL_TREE;
>> -         rvalue = true;
>> -       }
>> -      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 = min_align_of_type (type);
>> -         unsigned int rhs_align = min_align_of_type (rhstype);
>> -         if (rhs_align < type_align)
>> -           {
>> -             auto_diagnostic_group d;
>> -             location_t location = EXPR_LOC_OR_LOC (rhs, input_location);
>> -             if (warning_at (location, OPT_Waddress_of_packed_member,
>> -                             "converting a packed %qT pointer (alignment %d) "
>> -                             "to a %qT pointer (alignment %d) may result in "
>> -                             "an unaligned pointer value",
>> -                             rhstype, rhs_align, type, type_align))
>> -               {
>> -                 tree decl = TYPE_STUB_DECL (rhstype);
>> -                 if (decl)
>> -                   inform (DECL_SOURCE_LOCATION (decl), "defined here");
>> -                 decl = TYPE_STUB_DECL (type);
>> -                 if (decl)
>> -                   inform (DECL_SOURCE_LOCATION (decl), "defined here");
>> -               }
>> -           }
>> -       }
>> -      return NULL_TREE;
>> -    }
>> -
>>     tree context = NULL_TREE;
>>
>>     /* Check alignment of the object.  */
>> diff --git a/gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c b/gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c
>> index 95a376664da..0f5188df70a 100644
>> --- a/gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c
>> +++ b/gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c
>> @@ -52,12 +52,12 @@ void foo (void)
>>     f0 = *&__real__ t0.f;        /* { dg-bogus "may result in an unaligned pointer value" } */
>>     f0 = *&__imag__ t0.f;        /* { dg-bogus "may result in an unaligned pointer value" } */
>>     i1 = (&t0.c, (int*) 0);      /* { dg-bogus "may result in an unaligned pointer value" } */
>> -  t2 = (struct t**) t10;     /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
>> -  t2 = (struct t**) t100;    /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
>> -  t2 = (struct t**) t1;      /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
>> -  t2 = (struct t**) bar();   /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
>> -  t2 = (struct t**) baz();   /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
>> -  t2 = (struct t**) bazz();  /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
>> +  t2 = (struct t**) t10;       /* { dg-bogus "may result in an unaligned pointer value" } */
>> +  t2 = (struct t**) t100;      /* { dg-bogus "may result in an unaligned pointer value" } */
>> +  t2 = (struct t**) t1;        /* { dg-bogus "may result in an unaligned pointer value" } */
>> +  t2 = (struct t**) bar();     /* { dg-bogus "may result in an unaligned pointer value" } */
>> +  t2 = (struct t**) baz();     /* { dg-bogus "may result in an unaligned pointer value" } */
>> +  t2 = (struct t**) bazz();    /* { dg-bogus "may result in an unaligned pointer value" } */
>>     i1 = &t0.b;                /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
>>     i1 = &t1->b;               /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
>>     i1 = &t10[0].b;            /* { dg-warning "may result in an unaligned pointer value" ""  { target { ! default_packed } } } */
>> diff --git a/gcc/testsuite/c-c++-common/pr51628-35.c b/gcc/testsuite/c-c++-common/pr51628-35.c
>> index fa37d99beb7..a88c19ea0df 100644
>> --- a/gcc/testsuite/c-c++-common/pr51628-35.c
>> +++ b/gcc/testsuite/c-c++-common/pr51628-35.c
>> @@ -1,6 +1,6 @@
>>   /* PR c/51628.  */
>>   /* { dg-do compile } */
>> -/* { dg-options "-O" } */
>> +/* { dg-options "-O -Wcast-align=strict" } */
>>
>>   struct B { int i; };
>>   struct C { struct B b; } __attribute__ ((packed));
>> @@ -12,12 +12,12 @@ long *
>>   foo1 (void)
>>   {
>>     return (long *) p;
>> -/* { dg-warning "may result in an unaligned pointer value" "" { target { ! default_packed } } .-1 } */
>> +/* { dg-warning "increases required alignment" "" { target { ! default_packed } } .-1 } */
>>   }
>>
>>   long *
>>   foo2 (void)
>>   {
>>     return (long *) bar ();
>> -/* { dg-warning "may result in an unaligned pointer value" "" { target { ! default_packed } } .-1 } */
>> +/* { dg-warning "increases required alignment" "" { target { ! default_packed } } .-1 } */
>>   }
>> diff --git a/gcc/testsuite/g++.dg/warn/Waddress-of-packed-member3.C b/gcc/testsuite/g++.dg/warn/Waddress-of-packed-member3.C
>> index aeffb969c01..28dd05d366c 100644
>> --- a/gcc/testsuite/g++.dg/warn/Waddress-of-packed-member3.C
>> +++ b/gcc/testsuite/g++.dg/warn/Waddress-of-packed-member3.C
>> @@ -1,5 +1,5 @@
>>   // { dg-do compile { target { ! default_packed } } }
>> -// Test that -Waddress-of-packed-member works with member functions.
>> +// { dg-additional-options -Wcast-align=strict }
>>
>>   struct S {
>>     char c;
>> @@ -16,8 +16,8 @@ S**
>>   f ()
>>   {
>>     S **s;
>> -  s = reinterpret_cast<S**>(foo ()); // { dg-warning "converting a packed" }
>> -  s = reinterpret_cast<S**>(x.memfn ()); // { dg-warning "converting a packed" }
>> -  s = reinterpret_cast<S**>(X::smemfn ()); // { dg-warning "converting a packed" }
>> +  s = reinterpret_cast<S**>(foo ()); // { dg-warning "increases required alignment" }
>> +  s = reinterpret_cast<S**>(x.memfn ()); // { dg-warning "increases required alignment" }
>> +  s = reinterpret_cast<S**>(X::smemfn ()); // { dg-warning "increases required alignment" }
>>     return s;
>>   }
>> diff --git a/gcc/testsuite/gcc.dg/pr51628-20.c b/gcc/testsuite/gcc.dg/pr51628-20.c
>> deleted file mode 100644
>> index 2249d85098b..00000000000
>> --- a/gcc/testsuite/gcc.dg/pr51628-20.c
>> +++ /dev/null
>> @@ -1,11 +0,0 @@
>> -/* PR c/51628.  */
>> -/* { dg-do compile } */
>> -/* { dg-options "-O -Wno-incompatible-pointer-types" } */
>> -
>> -struct B { int i; };
>> -struct C { struct B b; } __attribute__ ((packed));
>> -
>> -extern struct C *p;
>> -
>> -long* g8 (void) { return p; }
>> -/* { dg-warning "may result in an unaligned pointer value" "" { target { ! default_packed } } .-1 } */
>> diff --git a/gcc/testsuite/gcc.dg/pr51628-21.c b/gcc/testsuite/gcc.dg/pr51628-21.c
>> deleted file mode 100644
>> index f1adbe64002..00000000000
>> --- a/gcc/testsuite/gcc.dg/pr51628-21.c
>> +++ /dev/null
>> @@ -1,11 +0,0 @@
>> -/* PR c/51628.  */
>> -/* { dg-do compile } */
>> -/* { dg-options "-O -Wno-incompatible-pointer-types" } */
>> -
>> -struct B { int i; };
>> -struct C { struct B b; } __attribute__ ((packed));
>> -
>> -extern struct C p[];
>> -
>> -long* g8 (void) { return p; }
>> -/* { dg-warning "may result in an unaligned pointer value" "" { target { ! default_packed } } .-1 } */
>> diff --git a/gcc/testsuite/gcc.dg/pr51628-25.c b/gcc/testsuite/gcc.dg/pr51628-25.c
>> deleted file mode 100644
>> index f00d9b1bcac..00000000000
>> --- a/gcc/testsuite/gcc.dg/pr51628-25.c
>> +++ /dev/null
>> @@ -1,9 +0,0 @@
>> -/* PR c/51628.  */
>> -/* { dg-do compile } */
>> -/* { dg-options "-O -Wno-incompatible-pointer-types" } */
>> -
>> -struct B { int i; };
>> -struct C { struct B b; } __attribute__ ((packed));
>> -
>> -long* g8 (struct C *p) { return p; }
>> -/* { dg-warning "may result in an unaligned pointer value" "" { target { ! default_packed } } .-1 } */
>> diff --git a/gcc/testsuite/gcc.dg/pr88928.c b/gcc/testsuite/gcc.dg/pr88928.c
>> index 0b6c1d70f05..1d176d6d51d 100644
>> --- a/gcc/testsuite/gcc.dg/pr88928.c
>> +++ b/gcc/testsuite/gcc.dg/pr88928.c
>> @@ -1,6 +1,6 @@
>> -/* { dg-do compile } */
>> -/* { dg-options "-Wno-pedantic -Waddress-of-packed-member" } */
>> +/* { dg-do compile { target { ! default_packed } } } */
>> +/* { dg-options "-Wno-pedantic -Waddress-of-packed-member -Wcast-align=strict" } */
>>   struct a { } __attribute__((__packed__));
>>   void c (struct a **);
>>   void d (const struct a *b) { c ((struct a **) b); }
>> -/* { dg-warning "may result in an unaligned pointer value" "" { target { ! default_packed } } .-1 } */
>> +/* { dg-warning "increases required alignment" "" { target *-*-* } .-1 } */
>>
>> base-commit: 65bd6de0de57abc86931a5e0b9a8d453ad530004
>> --
>> 2.39.3
>>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PING] Re: [PATCH 1/2] c-family: -Waddress-of-packed-member and casts
  2024-02-07 16:19   ` Torbjorn SVENSSON
@ 2024-02-22  8:51     ` Torbjorn SVENSSON
  2024-02-28 23:07       ` Jason Merrill
  0 siblings, 1 reply; 14+ messages in thread
From: Torbjorn SVENSSON @ 2024-02-22  8:51 UTC (permalink / raw)
  To: Richard Biener, Jason Merrill
  Cc: gcc-patches, Jeff Law, Alexandre Oliva, H . J . Lu, Yvan Roux

Ping!

Kind regards,
Torbjörn


On 2024-02-07 17:19, Torbjorn SVENSSON wrote:
> Hi,
> 
> Is it okay to backport b7e4a4c626eeeb32c291d5bbbaa148c5081b6bfd to 
> releases/gcc-13?
> 
> Without this backport, I see these failures on arm-none-eabi:
> 
> FAIL: 
> gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c 
> (test for excess errors)
> FAIL: gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early.c 
> (test for excess errors)
> 
> Kind regards,
> Torbjörn
> 
> On 2023-12-11 08:28, Richard Biener wrote:
>> On Wed, Nov 22, 2023 at 11:45 PM Jason Merrill <jason@redhat.com> wrote:
>>>
>>> Tested x86_64-pc-linux-gnu, OK for trunk?
>>
>> OK
>>
>>> -- 8< --
>>>
>>> -Waddress-of-packed-member, in addition to the documented warning about
>>> taking the address of a packed member, also warns about casting from
>>> a pointer to a TYPE_PACKED type to a pointer to a type with greater
>>> alignment.
>>>
>>> This wrongly warns if the source is a pointer to enum when -fshort-enums
>>> is on, since that is also represented by TYPE_PACKED.
>>>
>>> And there's already -Wcast-align to catch casting from pointer to less
>>> aligned type (packed or otherwise) to pointer to more aligned type; even
>>> apart from the enum problem, this seems like a somewhat arbitrary 
>>> subset of
>>> that warning.  Though that isn't currently on by default.
>>>
>>> So, this patch removes the undocumented type-based warning from
>>> -Waddress-of-packed-member.  Some of the tests where the warning is
>>> desirable I changed to use -Wcast-align=strict instead.  The ones that
>>> require -Wno-incompatible-pointer-types, I just removed.
>>>
>>> gcc/c-family/ChangeLog:
>>>
>>>          * c-warn.cc (check_address_or_pointer_of_packed_member):
>>>          Remove warning based on TYPE_PACKED.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>>          * c-c++-common/Waddress-of-packed-member-1.c: Don't expect
>>>          a warning on the cast cases.
>>>          * c-c++-common/pr51628-35.c: Use -Wcast-align=strict.
>>>          * g++.dg/warn/Waddress-of-packed-member3.C: Likewise.
>>>          * gcc.dg/pr88928.c: Likewise.
>>>          * gcc.dg/pr51628-20.c: Removed.
>>>          * gcc.dg/pr51628-21.c: Removed.
>>>          * gcc.dg/pr51628-25.c: Removed.
>>> ---
>>>   gcc/c-family/c-warn.cc                        | 58 +------------------
>>>   .../Waddress-of-packed-member-1.c             | 12 ++--
>>>   gcc/testsuite/c-c++-common/pr51628-35.c       |  6 +-
>>>   .../g++.dg/warn/Waddress-of-packed-member3.C  |  8 +--
>>>   gcc/testsuite/gcc.dg/pr51628-20.c             | 11 ----
>>>   gcc/testsuite/gcc.dg/pr51628-21.c             | 11 ----
>>>   gcc/testsuite/gcc.dg/pr51628-25.c             |  9 ---
>>>   gcc/testsuite/gcc.dg/pr88928.c                |  6 +-
>>>   8 files changed, 19 insertions(+), 102 deletions(-)
>>>   delete mode 100644 gcc/testsuite/gcc.dg/pr51628-20.c
>>>   delete mode 100644 gcc/testsuite/gcc.dg/pr51628-21.c
>>>   delete mode 100644 gcc/testsuite/gcc.dg/pr51628-25.c
>>>
>>> diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc
>>> index d2938b91043..2a399ba6d14 100644
>>> --- a/gcc/c-family/c-warn.cc
>>> +++ b/gcc/c-family/c-warn.cc
>>> @@ -2991,10 +2991,9 @@ check_alignment_of_packed_member (tree type, 
>>> tree field, bool rvalue)
>>>     return NULL_TREE;
>>>   }
>>>
>>> -/* Return struct or union type if the right hand value, RHS:
>>> -   1. Is a pointer value which isn't aligned to a pointer type TYPE.
>>> -   2. Is an address which takes the unaligned address of packed member
>>> -      of struct or union when assigning to TYPE.
>>> +/* Return struct or union type if the right hand value, RHS
>>> +   is an address which takes the unaligned address of packed member
>>> +   of struct or union when assigning to TYPE.
>>>      Otherwise, return NULL_TREE.  */
>>>
>>>   static tree
>>> @@ -3021,57 +3020,6 @@ check_address_or_pointer_of_packed_member 
>>> (tree type, tree rhs)
>>>
>>>     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.  */
>>> -         if (rhs == NULL_TREE)
>>> -           return NULL_TREE;
>>> -         rhs = TREE_TYPE (rhs);        /* Pointer type.  */
>>> -         /* We could be called while processing a template and RHS 
>>> could be
>>> -            a functor.  In that case it's a class, not a pointer.  */
>>> -         if (!rhs || !POINTER_TYPE_P (rhs))
>>> -           return NULL_TREE;
>>> -         rhs = TREE_TYPE (rhs);        /* Function type.  */
>>> -         rhstype = TREE_TYPE (rhs);
>>> -         if (!rhstype || !POINTER_TYPE_P (rhstype))
>>> -           return NULL_TREE;
>>> -         rvalue = true;
>>> -       }
>>> -      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 = min_align_of_type (type);
>>> -         unsigned int rhs_align = min_align_of_type (rhstype);
>>> -         if (rhs_align < type_align)
>>> -           {
>>> -             auto_diagnostic_group d;
>>> -             location_t location = EXPR_LOC_OR_LOC (rhs, 
>>> input_location);
>>> -             if (warning_at (location, OPT_Waddress_of_packed_member,
>>> -                             "converting a packed %qT pointer 
>>> (alignment %d) "
>>> -                             "to a %qT pointer (alignment %d) may 
>>> result in "
>>> -                             "an unaligned pointer value",
>>> -                             rhstype, rhs_align, type, type_align))
>>> -               {
>>> -                 tree decl = TYPE_STUB_DECL (rhstype);
>>> -                 if (decl)
>>> -                   inform (DECL_SOURCE_LOCATION (decl), "defined 
>>> here");
>>> -                 decl = TYPE_STUB_DECL (type);
>>> -                 if (decl)
>>> -                   inform (DECL_SOURCE_LOCATION (decl), "defined 
>>> here");
>>> -               }
>>> -           }
>>> -       }
>>> -      return NULL_TREE;
>>> -    }
>>> -
>>>     tree context = NULL_TREE;
>>>
>>>     /* Check alignment of the object.  */
>>> diff --git a/gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c 
>>> b/gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c
>>> index 95a376664da..0f5188df70a 100644
>>> --- a/gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c
>>> +++ b/gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c
>>> @@ -52,12 +52,12 @@ void foo (void)
>>>     f0 = *&__real__ t0.f;        /* { dg-bogus "may result in an 
>>> unaligned pointer value" } */
>>>     f0 = *&__imag__ t0.f;        /* { dg-bogus "may result in an 
>>> unaligned pointer value" } */
>>>     i1 = (&t0.c, (int*) 0);      /* { dg-bogus "may result in an 
>>> unaligned pointer value" } */
>>> -  t2 = (struct t**) t10;     /* { dg-warning "may result in an 
>>> unaligned pointer value" ""  { target { ! default_packed } } } */
>>> -  t2 = (struct t**) t100;    /* { dg-warning "may result in an 
>>> unaligned pointer value" ""  { target { ! default_packed } } } */
>>> -  t2 = (struct t**) t1;      /* { dg-warning "may result in an 
>>> unaligned pointer value" ""  { target { ! default_packed } } } */
>>> -  t2 = (struct t**) bar();   /* { dg-warning "may result in an 
>>> unaligned pointer value" ""  { target { ! default_packed } } } */
>>> -  t2 = (struct t**) baz();   /* { dg-warning "may result in an 
>>> unaligned pointer value" ""  { target { ! default_packed } } } */
>>> -  t2 = (struct t**) bazz();  /* { dg-warning "may result in an 
>>> unaligned pointer value" ""  { target { ! default_packed } } } */
>>> +  t2 = (struct t**) t10;       /* { dg-bogus "may result in an 
>>> unaligned pointer value" } */
>>> +  t2 = (struct t**) t100;      /* { dg-bogus "may result in an 
>>> unaligned pointer value" } */
>>> +  t2 = (struct t**) t1;        /* { dg-bogus "may result in an 
>>> unaligned pointer value" } */
>>> +  t2 = (struct t**) bar();     /* { dg-bogus "may result in an 
>>> unaligned pointer value" } */
>>> +  t2 = (struct t**) baz();     /* { dg-bogus "may result in an 
>>> unaligned pointer value" } */
>>> +  t2 = (struct t**) bazz();    /* { dg-bogus "may result in an 
>>> unaligned pointer value" } */
>>>     i1 = &t0.b;                /* { dg-warning "may result in an 
>>> unaligned pointer value" ""  { target { ! default_packed } } } */
>>>     i1 = &t1->b;               /* { dg-warning "may result in an 
>>> unaligned pointer value" ""  { target { ! default_packed } } } */
>>>     i1 = &t10[0].b;            /* { dg-warning "may result in an 
>>> unaligned pointer value" ""  { target { ! default_packed } } } */
>>> diff --git a/gcc/testsuite/c-c++-common/pr51628-35.c 
>>> b/gcc/testsuite/c-c++-common/pr51628-35.c
>>> index fa37d99beb7..a88c19ea0df 100644
>>> --- a/gcc/testsuite/c-c++-common/pr51628-35.c
>>> +++ b/gcc/testsuite/c-c++-common/pr51628-35.c
>>> @@ -1,6 +1,6 @@
>>>   /* PR c/51628.  */
>>>   /* { dg-do compile } */
>>> -/* { dg-options "-O" } */
>>> +/* { dg-options "-O -Wcast-align=strict" } */
>>>
>>>   struct B { int i; };
>>>   struct C { struct B b; } __attribute__ ((packed));
>>> @@ -12,12 +12,12 @@ long *
>>>   foo1 (void)
>>>   {
>>>     return (long *) p;
>>> -/* { dg-warning "may result in an unaligned pointer value" "" { 
>>> target { ! default_packed } } .-1 } */
>>> +/* { dg-warning "increases required alignment" "" { target { ! 
>>> default_packed } } .-1 } */
>>>   }
>>>
>>>   long *
>>>   foo2 (void)
>>>   {
>>>     return (long *) bar ();
>>> -/* { dg-warning "may result in an unaligned pointer value" "" { 
>>> target { ! default_packed } } .-1 } */
>>> +/* { dg-warning "increases required alignment" "" { target { ! 
>>> default_packed } } .-1 } */
>>>   }
>>> diff --git a/gcc/testsuite/g++.dg/warn/Waddress-of-packed-member3.C 
>>> b/gcc/testsuite/g++.dg/warn/Waddress-of-packed-member3.C
>>> index aeffb969c01..28dd05d366c 100644
>>> --- a/gcc/testsuite/g++.dg/warn/Waddress-of-packed-member3.C
>>> +++ b/gcc/testsuite/g++.dg/warn/Waddress-of-packed-member3.C
>>> @@ -1,5 +1,5 @@
>>>   // { dg-do compile { target { ! default_packed } } }
>>> -// Test that -Waddress-of-packed-member works with member functions.
>>> +// { dg-additional-options -Wcast-align=strict }
>>>
>>>   struct S {
>>>     char c;
>>> @@ -16,8 +16,8 @@ S**
>>>   f ()
>>>   {
>>>     S **s;
>>> -  s = reinterpret_cast<S**>(foo ()); // { dg-warning "converting a 
>>> packed" }
>>> -  s = reinterpret_cast<S**>(x.memfn ()); // { dg-warning "converting 
>>> a packed" }
>>> -  s = reinterpret_cast<S**>(X::smemfn ()); // { dg-warning 
>>> "converting a packed" }
>>> +  s = reinterpret_cast<S**>(foo ()); // { dg-warning "increases 
>>> required alignment" }
>>> +  s = reinterpret_cast<S**>(x.memfn ()); // { dg-warning "increases 
>>> required alignment" }
>>> +  s = reinterpret_cast<S**>(X::smemfn ()); // { dg-warning 
>>> "increases required alignment" }
>>>     return s;
>>>   }
>>> diff --git a/gcc/testsuite/gcc.dg/pr51628-20.c 
>>> b/gcc/testsuite/gcc.dg/pr51628-20.c
>>> deleted file mode 100644
>>> index 2249d85098b..00000000000
>>> --- a/gcc/testsuite/gcc.dg/pr51628-20.c
>>> +++ /dev/null
>>> @@ -1,11 +0,0 @@
>>> -/* PR c/51628.  */
>>> -/* { dg-do compile } */
>>> -/* { dg-options "-O -Wno-incompatible-pointer-types" } */
>>> -
>>> -struct B { int i; };
>>> -struct C { struct B b; } __attribute__ ((packed));
>>> -
>>> -extern struct C *p;
>>> -
>>> -long* g8 (void) { return p; }
>>> -/* { dg-warning "may result in an unaligned pointer value" "" { 
>>> target { ! default_packed } } .-1 } */
>>> diff --git a/gcc/testsuite/gcc.dg/pr51628-21.c 
>>> b/gcc/testsuite/gcc.dg/pr51628-21.c
>>> deleted file mode 100644
>>> index f1adbe64002..00000000000
>>> --- a/gcc/testsuite/gcc.dg/pr51628-21.c
>>> +++ /dev/null
>>> @@ -1,11 +0,0 @@
>>> -/* PR c/51628.  */
>>> -/* { dg-do compile } */
>>> -/* { dg-options "-O -Wno-incompatible-pointer-types" } */
>>> -
>>> -struct B { int i; };
>>> -struct C { struct B b; } __attribute__ ((packed));
>>> -
>>> -extern struct C p[];
>>> -
>>> -long* g8 (void) { return p; }
>>> -/* { dg-warning "may result in an unaligned pointer value" "" { 
>>> target { ! default_packed } } .-1 } */
>>> diff --git a/gcc/testsuite/gcc.dg/pr51628-25.c 
>>> b/gcc/testsuite/gcc.dg/pr51628-25.c
>>> deleted file mode 100644
>>> index f00d9b1bcac..00000000000
>>> --- a/gcc/testsuite/gcc.dg/pr51628-25.c
>>> +++ /dev/null
>>> @@ -1,9 +0,0 @@
>>> -/* PR c/51628.  */
>>> -/* { dg-do compile } */
>>> -/* { dg-options "-O -Wno-incompatible-pointer-types" } */
>>> -
>>> -struct B { int i; };
>>> -struct C { struct B b; } __attribute__ ((packed));
>>> -
>>> -long* g8 (struct C *p) { return p; }
>>> -/* { dg-warning "may result in an unaligned pointer value" "" { 
>>> target { ! default_packed } } .-1 } */
>>> diff --git a/gcc/testsuite/gcc.dg/pr88928.c 
>>> b/gcc/testsuite/gcc.dg/pr88928.c
>>> index 0b6c1d70f05..1d176d6d51d 100644
>>> --- a/gcc/testsuite/gcc.dg/pr88928.c
>>> +++ b/gcc/testsuite/gcc.dg/pr88928.c
>>> @@ -1,6 +1,6 @@
>>> -/* { dg-do compile } */
>>> -/* { dg-options "-Wno-pedantic -Waddress-of-packed-member" } */
>>> +/* { dg-do compile { target { ! default_packed } } } */
>>> +/* { dg-options "-Wno-pedantic -Waddress-of-packed-member 
>>> -Wcast-align=strict" } */
>>>   struct a { } __attribute__((__packed__));
>>>   void c (struct a **);
>>>   void d (const struct a *b) { c ((struct a **) b); }
>>> -/* { dg-warning "may result in an unaligned pointer value" "" { 
>>> target { ! default_packed } } .-1 } */
>>> +/* { dg-warning "increases required alignment" "" { target *-*-* } 
>>> .-1 } */
>>>
>>> base-commit: 65bd6de0de57abc86931a5e0b9a8d453ad530004
>>> -- 
>>> 2.39.3
>>>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PING] Re: [PATCH 1/2] c-family: -Waddress-of-packed-member and casts
  2024-02-22  8:51     ` [PING] " Torbjorn SVENSSON
@ 2024-02-28 23:07       ` Jason Merrill
  2024-03-09  9:02         ` [PATCH] testsuite: xfail test for arm Torbjörn SVENSSON
  0 siblings, 1 reply; 14+ messages in thread
From: Jason Merrill @ 2024-02-28 23:07 UTC (permalink / raw)
  To: Torbjorn SVENSSON, Richard Biener
  Cc: gcc-patches, Jeff Law, Alexandre Oliva, H . J . Lu, Yvan Roux

On 2/22/24 03:51, Torbjorn SVENSSON wrote:
> Ping!

Hmm, I'm somewhat reluctant to backport a significant behavior change 
like this just to silence a testsuite failure, even though I think the 
change is correct.  Maybe just xfail the tests for GCC 13?

Jason

> On 2024-02-07 17:19, Torbjorn SVENSSON wrote:
>> Hi,
>>
>> Is it okay to backport b7e4a4c626eeeb32c291d5bbbaa148c5081b6bfd to 
>> releases/gcc-13?
>>
>> Without this backport, I see these failures on arm-none-eabi:
>>
>> FAIL: 
>> gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c 
>> (test for excess errors)
>> FAIL: gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early.c 
>> (test for excess errors)
>>
>> Kind regards,
>> Torbjörn
>>
>> On 2023-12-11 08:28, Richard Biener wrote:
>>> On Wed, Nov 22, 2023 at 11:45 PM Jason Merrill <jason@redhat.com> wrote:
>>>>
>>>> Tested x86_64-pc-linux-gnu, OK for trunk?
>>>
>>> OK
>>>
>>>> -- 8< --
>>>>
>>>> -Waddress-of-packed-member, in addition to the documented warning about
>>>> taking the address of a packed member, also warns about casting from
>>>> a pointer to a TYPE_PACKED type to a pointer to a type with greater
>>>> alignment.
>>>>
>>>> This wrongly warns if the source is a pointer to enum when 
>>>> -fshort-enums
>>>> is on, since that is also represented by TYPE_PACKED.
>>>>
>>>> And there's already -Wcast-align to catch casting from pointer to less
>>>> aligned type (packed or otherwise) to pointer to more aligned type; 
>>>> even
>>>> apart from the enum problem, this seems like a somewhat arbitrary 
>>>> subset of
>>>> that warning.  Though that isn't currently on by default.
>>>>
>>>> So, this patch removes the undocumented type-based warning from
>>>> -Waddress-of-packed-member.  Some of the tests where the warning is
>>>> desirable I changed to use -Wcast-align=strict instead.  The ones that
>>>> require -Wno-incompatible-pointer-types, I just removed.
>>>>
>>>> gcc/c-family/ChangeLog:
>>>>
>>>>          * c-warn.cc (check_address_or_pointer_of_packed_member):
>>>>          Remove warning based on TYPE_PACKED.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>>          * c-c++-common/Waddress-of-packed-member-1.c: Don't expect
>>>>          a warning on the cast cases.
>>>>          * c-c++-common/pr51628-35.c: Use -Wcast-align=strict.
>>>>          * g++.dg/warn/Waddress-of-packed-member3.C: Likewise.
>>>>          * gcc.dg/pr88928.c: Likewise.
>>>>          * gcc.dg/pr51628-20.c: Removed.
>>>>          * gcc.dg/pr51628-21.c: Removed.
>>>>          * gcc.dg/pr51628-25.c: Removed.
>>>> ---
>>>>   gcc/c-family/c-warn.cc                        | 58 
>>>> +------------------
>>>>   .../Waddress-of-packed-member-1.c             | 12 ++--
>>>>   gcc/testsuite/c-c++-common/pr51628-35.c       |  6 +-
>>>>   .../g++.dg/warn/Waddress-of-packed-member3.C  |  8 +--
>>>>   gcc/testsuite/gcc.dg/pr51628-20.c             | 11 ----
>>>>   gcc/testsuite/gcc.dg/pr51628-21.c             | 11 ----
>>>>   gcc/testsuite/gcc.dg/pr51628-25.c             |  9 ---
>>>>   gcc/testsuite/gcc.dg/pr88928.c                |  6 +-
>>>>   8 files changed, 19 insertions(+), 102 deletions(-)
>>>>   delete mode 100644 gcc/testsuite/gcc.dg/pr51628-20.c
>>>>   delete mode 100644 gcc/testsuite/gcc.dg/pr51628-21.c
>>>>   delete mode 100644 gcc/testsuite/gcc.dg/pr51628-25.c
>>>>
>>>> diff --git a/gcc/c-family/c-warn.cc b/gcc/c-family/c-warn.cc
>>>> index d2938b91043..2a399ba6d14 100644
>>>> --- a/gcc/c-family/c-warn.cc
>>>> +++ b/gcc/c-family/c-warn.cc
>>>> @@ -2991,10 +2991,9 @@ check_alignment_of_packed_member (tree type, 
>>>> tree field, bool rvalue)
>>>>     return NULL_TREE;
>>>>   }
>>>>
>>>> -/* Return struct or union type if the right hand value, RHS:
>>>> -   1. Is a pointer value which isn't aligned to a pointer type TYPE.
>>>> -   2. Is an address which takes the unaligned address of packed member
>>>> -      of struct or union when assigning to TYPE.
>>>> +/* Return struct or union type if the right hand value, RHS
>>>> +   is an address which takes the unaligned address of packed member
>>>> +   of struct or union when assigning to TYPE.
>>>>      Otherwise, return NULL_TREE.  */
>>>>
>>>>   static tree
>>>> @@ -3021,57 +3020,6 @@ check_address_or_pointer_of_packed_member 
>>>> (tree type, tree rhs)
>>>>
>>>>     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.  */
>>>> -         if (rhs == NULL_TREE)
>>>> -           return NULL_TREE;
>>>> -         rhs = TREE_TYPE (rhs);        /* Pointer type.  */
>>>> -         /* We could be called while processing a template and RHS 
>>>> could be
>>>> -            a functor.  In that case it's a class, not a pointer.  */
>>>> -         if (!rhs || !POINTER_TYPE_P (rhs))
>>>> -           return NULL_TREE;
>>>> -         rhs = TREE_TYPE (rhs);        /* Function type.  */
>>>> -         rhstype = TREE_TYPE (rhs);
>>>> -         if (!rhstype || !POINTER_TYPE_P (rhstype))
>>>> -           return NULL_TREE;
>>>> -         rvalue = true;
>>>> -       }
>>>> -      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 = min_align_of_type (type);
>>>> -         unsigned int rhs_align = min_align_of_type (rhstype);
>>>> -         if (rhs_align < type_align)
>>>> -           {
>>>> -             auto_diagnostic_group d;
>>>> -             location_t location = EXPR_LOC_OR_LOC (rhs, 
>>>> input_location);
>>>> -             if (warning_at (location, OPT_Waddress_of_packed_member,
>>>> -                             "converting a packed %qT pointer 
>>>> (alignment %d) "
>>>> -                             "to a %qT pointer (alignment %d) may 
>>>> result in "
>>>> -                             "an unaligned pointer value",
>>>> -                             rhstype, rhs_align, type, type_align))
>>>> -               {
>>>> -                 tree decl = TYPE_STUB_DECL (rhstype);
>>>> -                 if (decl)
>>>> -                   inform (DECL_SOURCE_LOCATION (decl), "defined 
>>>> here");
>>>> -                 decl = TYPE_STUB_DECL (type);
>>>> -                 if (decl)
>>>> -                   inform (DECL_SOURCE_LOCATION (decl), "defined 
>>>> here");
>>>> -               }
>>>> -           }
>>>> -       }
>>>> -      return NULL_TREE;
>>>> -    }
>>>> -
>>>>     tree context = NULL_TREE;
>>>>
>>>>     /* Check alignment of the object.  */
>>>> diff --git 
>>>> a/gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c 
>>>> b/gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c
>>>> index 95a376664da..0f5188df70a 100644
>>>> --- a/gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c
>>>> +++ b/gcc/testsuite/c-c++-common/Waddress-of-packed-member-1.c
>>>> @@ -52,12 +52,12 @@ void foo (void)
>>>>     f0 = *&__real__ t0.f;        /* { dg-bogus "may result in an 
>>>> unaligned pointer value" } */
>>>>     f0 = *&__imag__ t0.f;        /* { dg-bogus "may result in an 
>>>> unaligned pointer value" } */
>>>>     i1 = (&t0.c, (int*) 0);      /* { dg-bogus "may result in an 
>>>> unaligned pointer value" } */
>>>> -  t2 = (struct t**) t10;     /* { dg-warning "may result in an 
>>>> unaligned pointer value" ""  { target { ! default_packed } } } */
>>>> -  t2 = (struct t**) t100;    /* { dg-warning "may result in an 
>>>> unaligned pointer value" ""  { target { ! default_packed } } } */
>>>> -  t2 = (struct t**) t1;      /* { dg-warning "may result in an 
>>>> unaligned pointer value" ""  { target { ! default_packed } } } */
>>>> -  t2 = (struct t**) bar();   /* { dg-warning "may result in an 
>>>> unaligned pointer value" ""  { target { ! default_packed } } } */
>>>> -  t2 = (struct t**) baz();   /* { dg-warning "may result in an 
>>>> unaligned pointer value" ""  { target { ! default_packed } } } */
>>>> -  t2 = (struct t**) bazz();  /* { dg-warning "may result in an 
>>>> unaligned pointer value" ""  { target { ! default_packed } } } */
>>>> +  t2 = (struct t**) t10;       /* { dg-bogus "may result in an 
>>>> unaligned pointer value" } */
>>>> +  t2 = (struct t**) t100;      /* { dg-bogus "may result in an 
>>>> unaligned pointer value" } */
>>>> +  t2 = (struct t**) t1;        /* { dg-bogus "may result in an 
>>>> unaligned pointer value" } */
>>>> +  t2 = (struct t**) bar();     /* { dg-bogus "may result in an 
>>>> unaligned pointer value" } */
>>>> +  t2 = (struct t**) baz();     /* { dg-bogus "may result in an 
>>>> unaligned pointer value" } */
>>>> +  t2 = (struct t**) bazz();    /* { dg-bogus "may result in an 
>>>> unaligned pointer value" } */
>>>>     i1 = &t0.b;                /* { dg-warning "may result in an 
>>>> unaligned pointer value" ""  { target { ! default_packed } } } */
>>>>     i1 = &t1->b;               /* { dg-warning "may result in an 
>>>> unaligned pointer value" ""  { target { ! default_packed } } } */
>>>>     i1 = &t10[0].b;            /* { dg-warning "may result in an 
>>>> unaligned pointer value" ""  { target { ! default_packed } } } */
>>>> diff --git a/gcc/testsuite/c-c++-common/pr51628-35.c 
>>>> b/gcc/testsuite/c-c++-common/pr51628-35.c
>>>> index fa37d99beb7..a88c19ea0df 100644
>>>> --- a/gcc/testsuite/c-c++-common/pr51628-35.c
>>>> +++ b/gcc/testsuite/c-c++-common/pr51628-35.c
>>>> @@ -1,6 +1,6 @@
>>>>   /* PR c/51628.  */
>>>>   /* { dg-do compile } */
>>>> -/* { dg-options "-O" } */
>>>> +/* { dg-options "-O -Wcast-align=strict" } */
>>>>
>>>>   struct B { int i; };
>>>>   struct C { struct B b; } __attribute__ ((packed));
>>>> @@ -12,12 +12,12 @@ long *
>>>>   foo1 (void)
>>>>   {
>>>>     return (long *) p;
>>>> -/* { dg-warning "may result in an unaligned pointer value" "" { 
>>>> target { ! default_packed } } .-1 } */
>>>> +/* { dg-warning "increases required alignment" "" { target { ! 
>>>> default_packed } } .-1 } */
>>>>   }
>>>>
>>>>   long *
>>>>   foo2 (void)
>>>>   {
>>>>     return (long *) bar ();
>>>> -/* { dg-warning "may result in an unaligned pointer value" "" { 
>>>> target { ! default_packed } } .-1 } */
>>>> +/* { dg-warning "increases required alignment" "" { target { ! 
>>>> default_packed } } .-1 } */
>>>>   }
>>>> diff --git a/gcc/testsuite/g++.dg/warn/Waddress-of-packed-member3.C 
>>>> b/gcc/testsuite/g++.dg/warn/Waddress-of-packed-member3.C
>>>> index aeffb969c01..28dd05d366c 100644
>>>> --- a/gcc/testsuite/g++.dg/warn/Waddress-of-packed-member3.C
>>>> +++ b/gcc/testsuite/g++.dg/warn/Waddress-of-packed-member3.C
>>>> @@ -1,5 +1,5 @@
>>>>   // { dg-do compile { target { ! default_packed } } }
>>>> -// Test that -Waddress-of-packed-member works with member functions.
>>>> +// { dg-additional-options -Wcast-align=strict }
>>>>
>>>>   struct S {
>>>>     char c;
>>>> @@ -16,8 +16,8 @@ S**
>>>>   f ()
>>>>   {
>>>>     S **s;
>>>> -  s = reinterpret_cast<S**>(foo ()); // { dg-warning "converting a 
>>>> packed" }
>>>> -  s = reinterpret_cast<S**>(x.memfn ()); // { dg-warning 
>>>> "converting a packed" }
>>>> -  s = reinterpret_cast<S**>(X::smemfn ()); // { dg-warning 
>>>> "converting a packed" }
>>>> +  s = reinterpret_cast<S**>(foo ()); // { dg-warning "increases 
>>>> required alignment" }
>>>> +  s = reinterpret_cast<S**>(x.memfn ()); // { dg-warning "increases 
>>>> required alignment" }
>>>> +  s = reinterpret_cast<S**>(X::smemfn ()); // { dg-warning 
>>>> "increases required alignment" }
>>>>     return s;
>>>>   }
>>>> diff --git a/gcc/testsuite/gcc.dg/pr51628-20.c 
>>>> b/gcc/testsuite/gcc.dg/pr51628-20.c
>>>> deleted file mode 100644
>>>> index 2249d85098b..00000000000
>>>> --- a/gcc/testsuite/gcc.dg/pr51628-20.c
>>>> +++ /dev/null
>>>> @@ -1,11 +0,0 @@
>>>> -/* PR c/51628.  */
>>>> -/* { dg-do compile } */
>>>> -/* { dg-options "-O -Wno-incompatible-pointer-types" } */
>>>> -
>>>> -struct B { int i; };
>>>> -struct C { struct B b; } __attribute__ ((packed));
>>>> -
>>>> -extern struct C *p;
>>>> -
>>>> -long* g8 (void) { return p; }
>>>> -/* { dg-warning "may result in an unaligned pointer value" "" { 
>>>> target { ! default_packed } } .-1 } */
>>>> diff --git a/gcc/testsuite/gcc.dg/pr51628-21.c 
>>>> b/gcc/testsuite/gcc.dg/pr51628-21.c
>>>> deleted file mode 100644
>>>> index f1adbe64002..00000000000
>>>> --- a/gcc/testsuite/gcc.dg/pr51628-21.c
>>>> +++ /dev/null
>>>> @@ -1,11 +0,0 @@
>>>> -/* PR c/51628.  */
>>>> -/* { dg-do compile } */
>>>> -/* { dg-options "-O -Wno-incompatible-pointer-types" } */
>>>> -
>>>> -struct B { int i; };
>>>> -struct C { struct B b; } __attribute__ ((packed));
>>>> -
>>>> -extern struct C p[];
>>>> -
>>>> -long* g8 (void) { return p; }
>>>> -/* { dg-warning "may result in an unaligned pointer value" "" { 
>>>> target { ! default_packed } } .-1 } */
>>>> diff --git a/gcc/testsuite/gcc.dg/pr51628-25.c 
>>>> b/gcc/testsuite/gcc.dg/pr51628-25.c
>>>> deleted file mode 100644
>>>> index f00d9b1bcac..00000000000
>>>> --- a/gcc/testsuite/gcc.dg/pr51628-25.c
>>>> +++ /dev/null
>>>> @@ -1,9 +0,0 @@
>>>> -/* PR c/51628.  */
>>>> -/* { dg-do compile } */
>>>> -/* { dg-options "-O -Wno-incompatible-pointer-types" } */
>>>> -
>>>> -struct B { int i; };
>>>> -struct C { struct B b; } __attribute__ ((packed));
>>>> -
>>>> -long* g8 (struct C *p) { return p; }
>>>> -/* { dg-warning "may result in an unaligned pointer value" "" { 
>>>> target { ! default_packed } } .-1 } */
>>>> diff --git a/gcc/testsuite/gcc.dg/pr88928.c 
>>>> b/gcc/testsuite/gcc.dg/pr88928.c
>>>> index 0b6c1d70f05..1d176d6d51d 100644
>>>> --- a/gcc/testsuite/gcc.dg/pr88928.c
>>>> +++ b/gcc/testsuite/gcc.dg/pr88928.c
>>>> @@ -1,6 +1,6 @@
>>>> -/* { dg-do compile } */
>>>> -/* { dg-options "-Wno-pedantic -Waddress-of-packed-member" } */
>>>> +/* { dg-do compile { target { ! default_packed } } } */
>>>> +/* { dg-options "-Wno-pedantic -Waddress-of-packed-member 
>>>> -Wcast-align=strict" } */
>>>>   struct a { } __attribute__((__packed__));
>>>>   void c (struct a **);
>>>>   void d (const struct a *b) { c ((struct a **) b); }
>>>> -/* { dg-warning "may result in an unaligned pointer value" "" { 
>>>> target { ! default_packed } } .-1 } */
>>>> +/* { dg-warning "increases required alignment" "" { target *-*-* } 
>>>> .-1 } */
>>>>
>>>> base-commit: 65bd6de0de57abc86931a5e0b9a8d453ad530004
>>>> -- 
>>>> 2.39.3
>>>>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] testsuite: xfail test for arm
  2024-02-28 23:07       ` Jason Merrill
@ 2024-03-09  9:02         ` Torbjörn SVENSSON
  2024-03-09  9:23           ` Andrew Pinski
  0 siblings, 1 reply; 14+ messages in thread
From: Torbjörn SVENSSON @ 2024-03-09  9:02 UTC (permalink / raw)
  To: gcc-patches, jason, richard.guenther
  Cc: jlaw, hjl.tools, oliva, yvan.roux, Torbjörn SVENSSON

I don't know if this affects other targets than arm-none-eabi, so I
used arm-*-*. If you think it should be *-*-* or some other target
selector, please let me know what to use instead.

Ok for releases/gcc-13?

--

On arm-none-eabi, the test case fails with
.../null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c:63:65: warning: converting a packed 'enum obj_type' pointer (alignment 1) to a 'struct connection' pointer (alignment 4) may result in an unaligned pointer value [-Waddress-of-packed-member]

The error was fixed in basepoints/gcc-14-6517-gb7e4a4c626e, but it
was considered to be a too big change to be backported and thus, the
failing test is marked xfail in GCC13.

gcc/testsuite/ChangeLog:
	* gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c:
	Added dg-bogus with xfail on offending line for arm-*-*.

Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
---
 .../null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c         | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c b/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
index 2a9c715c32c..461d5f1199c 100644
--- a/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
+++ b/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
@@ -60,7 +60,7 @@ static inline enum obj_type obj_type(const enum obj_type *t)
 }
 static inline struct connection *__objt_conn(enum obj_type *t)
 {
- return ((struct connection *)(((void *)(t)) - ((long)&((struct connection *)0)->obj_type)));
+ return ((struct connection *)(((void *)(t)) - ((long)&((struct connection *)0)->obj_type))); /* { dg-bogus "may result in an unaligned pointer value" "" { xfail arm-*-* } } */
 }
 static inline struct connection *objt_conn(enum obj_type *t)
 {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] testsuite: xfail test for arm
  2024-03-09  9:02         ` [PATCH] testsuite: xfail test for arm Torbjörn SVENSSON
@ 2024-03-09  9:23           ` Andrew Pinski
  2024-03-11 10:23             ` [PATCH v2] testsuite: xfail test for short_enums Torbjörn SVENSSON
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Pinski @ 2024-03-09  9:23 UTC (permalink / raw)
  To: Torbjörn SVENSSON
  Cc: gcc-patches, jason, richard.guenther, jlaw, hjl.tools, oliva, yvan.roux

On Sat, Mar 9, 2024 at 1:07 AM Torbjörn SVENSSON
<torbjorn.svensson@foss.st.com> wrote:
>
> I don't know if this affects other targets than arm-none-eabi, so I
> used arm-*-*. If you think it should be *-*-* or some other target
> selector, please let me know what to use instead.
>
> Ok for releases/gcc-13?

Most likely should be short_enums instead of arm*-*-* (I think the old
arm non-eabi didn't use short enums) due to the fix
r14-6517-gb7e4a4c626e applies when -fshort-enums is used.
Also if you are adding a dg-bogus to the branch, it might makes sense
to the same to the trunk (obviously without the xfail part).
Also makes sense to add a reference to r14-6517-gb7e4a4c626e to the
dg-bogus in the source too.

Thanks,
Andrew Pinski

>
> --
>
> On arm-none-eabi, the test case fails with
> .../null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c:63:65: warning: converting a packed 'enum obj_type' pointer (alignment 1) to a 'struct connection' pointer (alignment 4) may result in an unaligned pointer value [-Waddress-of-packed-member]
>
> The error was fixed in basepoints/gcc-14-6517-gb7e4a4c626e, but it
> was considered to be a too big change to be backported and thus, the
> failing test is marked xfail in GCC13.
>
> gcc/testsuite/ChangeLog:
>         * gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c:
>         Added dg-bogus with xfail on offending line for arm-*-*.
>
> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
> ---
>  .../null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c         | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c b/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
> index 2a9c715c32c..461d5f1199c 100644
> --- a/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
> +++ b/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
> @@ -60,7 +60,7 @@ static inline enum obj_type obj_type(const enum obj_type *t)
>  }
>  static inline struct connection *__objt_conn(enum obj_type *t)
>  {
> - return ((struct connection *)(((void *)(t)) - ((long)&((struct connection *)0)->obj_type)));
> + return ((struct connection *)(((void *)(t)) - ((long)&((struct connection *)0)->obj_type))); /* { dg-bogus "may result in an unaligned pointer value" "" { xfail arm-*-* } } */
>  }
>  static inline struct connection *objt_conn(enum obj_type *t)
>  {
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2] testsuite: xfail test for short_enums
  2024-03-09  9:23           ` Andrew Pinski
@ 2024-03-11 10:23             ` Torbjörn SVENSSON
  2024-03-12 13:21               ` Jason Merrill
  0 siblings, 1 reply; 14+ messages in thread
From: Torbjörn SVENSSON @ 2024-03-11 10:23 UTC (permalink / raw)
  To: gcc-patches, pinskia
  Cc: jlaw, hjl.tools, oliva, yvan.roux, jason, richard.guenther,
	Torbjörn SVENSSON

Changes compared to v1:
- Added reference to r14-6517-gb7e4a4c626e in dg-bogus comment
- Changed arm-*-* to short_enums in target selector
- Updated commit message to align with above changes


As the entire block generating the warning was removed in
r14-6517-gb7e4a4c626e, does it still make sense to add something to
trunk for the same line?
Do you want me to add the dg-bogus, but change "xfail" to "target" for
trunk?

Is this patch ok for releases/gcc-13?

--

On arm-none-eabi, the test case fails with
.../null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c:63:65: warning: converting a packed 'enum obj_type' pointer (alignment 1) to a 'struct connection' pointer (alignment 4) may result in an unaligned pointer value [-Waddress-of-packed-member]

The error was fixed in basepoints/gcc-14-6517-gb7e4a4c626e, but it
was considered to be a too big change to be backported and thus, the
failing test is marked xfail in GCC13.

gcc/testsuite/ChangeLog:

	* gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c:
	Added dg-bogus with xfail on offending line for short_enums.

Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
---
 .../null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c         | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c b/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
index 2a9c715c32c..e8cde7338a0 100644
--- a/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
+++ b/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
@@ -60,7 +60,7 @@ static inline enum obj_type obj_type(const enum obj_type *t)
 }
 static inline struct connection *__objt_conn(enum obj_type *t)
 {
- return ((struct connection *)(((void *)(t)) - ((long)&((struct connection *)0)->obj_type)));
+ return ((struct connection *)(((void *)(t)) - ((long)&((struct connection *)0)->obj_type))); /* { dg-bogus "may result in an unaligned pointer value" "Fixed in r14-6517-gb7e4a4c626e" { xfail short_enums } */
 }
 static inline struct connection *objt_conn(enum obj_type *t)
 {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] testsuite: xfail test for short_enums
  2024-03-11 10:23             ` [PATCH v2] testsuite: xfail test for short_enums Torbjörn SVENSSON
@ 2024-03-12 13:21               ` Jason Merrill
  2024-03-13 14:18                 ` [comitted] testsuite: target " Torbjörn SVENSSON
  2024-03-13 14:22                 ` [PATCH v2] testsuite: xfail " Torbjorn SVENSSON
  0 siblings, 2 replies; 14+ messages in thread
From: Jason Merrill @ 2024-03-12 13:21 UTC (permalink / raw)
  To: Torbjörn SVENSSON, gcc-patches, pinskia
  Cc: jlaw, hjl.tools, oliva, yvan.roux, richard.guenther

On 3/11/24 06:23, Torbjörn SVENSSON wrote:
> Changes compared to v1:
> - Added reference to r14-6517-gb7e4a4c626e in dg-bogus comment
> - Changed arm-*-* to short_enums in target selector
> - Updated commit message to align with above changes
> 
> 
> As the entire block generating the warning was removed in
> r14-6517-gb7e4a4c626e, does it still make sense to add something to
> trunk for the same line?
> Do you want me to add the dg-bogus, but change "xfail" to "target" for
> trunk?

Sounds good.

> Is this patch ok for releases/gcc-13?

OK.

> --
> 
> On arm-none-eabi, the test case fails with
> .../null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c:63:65: warning: converting a packed 'enum obj_type' pointer (alignment 1) to a 'struct connection' pointer (alignment 4) may result in an unaligned pointer value [-Waddress-of-packed-member]
> 
> The error was fixed in basepoints/gcc-14-6517-gb7e4a4c626e, but it
> was considered to be a too big change to be backported and thus, the
> failing test is marked xfail in GCC13.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c:
> 	Added dg-bogus with xfail on offending line for short_enums.
> 
> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
> ---
>   .../null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c         | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c b/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
> index 2a9c715c32c..e8cde7338a0 100644
> --- a/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
> +++ b/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
> @@ -60,7 +60,7 @@ static inline enum obj_type obj_type(const enum obj_type *t)
>   }
>   static inline struct connection *__objt_conn(enum obj_type *t)
>   {
> - return ((struct connection *)(((void *)(t)) - ((long)&((struct connection *)0)->obj_type)));
> + return ((struct connection *)(((void *)(t)) - ((long)&((struct connection *)0)->obj_type))); /* { dg-bogus "may result in an unaligned pointer value" "Fixed in r14-6517-gb7e4a4c626e" { xfail short_enums } */
>   }
>   static inline struct connection *objt_conn(enum obj_type *t)
>   {


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [comitted] testsuite: target test for short_enums
  2024-03-12 13:21               ` Jason Merrill
@ 2024-03-13 14:18                 ` Torbjörn SVENSSON
  2024-03-13 14:22                 ` [PATCH v2] testsuite: xfail " Torbjorn SVENSSON
  1 sibling, 0 replies; 14+ messages in thread
From: Torbjörn SVENSSON @ 2024-03-13 14:18 UTC (permalink / raw)
  To: gcc-patches, jason
  Cc: jlaw, hjl.tools, oliva, yvan.roux, richard.guenther, pinskia,
	Torbjörn SVENSSON

Committed the blow as requested by Jason in the patch for releases/gcc-13.

--

On arm-none-eabi, the test case fails with below warning on GCC13
.../null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c:63:65: warning: converting a packed 'enum obj_type' pointer (alignment 1) to a 'struct connection' pointer (alignment 4) may result in an unaligned pointer value [-Waddress-of-packed-member]

Add a dg-bogus to ensure that the warning is not reintroduced.

gcc/testsuite/ChangeLog:

	* c-c++-common/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c:
	Added dg-bogus with target on offending line for short_enums.

Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
---
 .../null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c         | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/c-c++-common/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c b/gcc/testsuite/c-c++-common/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
index c1c8e6f6a39..a37962f1d85 100644
--- a/gcc/testsuite/c-c++-common/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
+++ b/gcc/testsuite/c-c++-common/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
@@ -61,7 +61,7 @@ static inline enum obj_type obj_type(const enum obj_type *t)
 }
 static inline struct connection *__objt_conn(enum obj_type *t)
 {
- return ((struct connection *)(((char *)(t)) - ((long)&((struct connection *)0)->obj_type)));
+ return ((struct connection *)(((char *)(t)) - ((long)&((struct connection *)0)->obj_type))); /* { dg-bogus "may result in an unaligned pointer value" "Fixed in r14-6517-gb7e4a4c626e" { target short_enums } } */
 }
 static inline struct connection *objt_conn(enum obj_type *t)
 {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] testsuite: xfail test for short_enums
  2024-03-12 13:21               ` Jason Merrill
  2024-03-13 14:18                 ` [comitted] testsuite: target " Torbjörn SVENSSON
@ 2024-03-13 14:22                 ` Torbjorn SVENSSON
  2024-03-15  8:33                   ` [committed] testsuite: Added missing } in the dg-bogus comment [PR114343] Torbjörn SVENSSON
  1 sibling, 1 reply; 14+ messages in thread
From: Torbjorn SVENSSON @ 2024-03-13 14:22 UTC (permalink / raw)
  To: Jason Merrill, gcc-patches, pinskia
  Cc: jlaw, hjl.tools, oliva, yvan.roux, richard.guenther



On 2024-03-12 14:21, Jason Merrill wrote:
> On 3/11/24 06:23, Torbjörn SVENSSON wrote:
>> Changes compared to v1:
>> - Added reference to r14-6517-gb7e4a4c626e in dg-bogus comment
>> - Changed arm-*-* to short_enums in target selector
>> - Updated commit message to align with above changes
>>
>>
>> As the entire block generating the warning was removed in
>> r14-6517-gb7e4a4c626e, does it still make sense to add something to
>> trunk for the same line?
>> Do you want me to add the dg-bogus, but change "xfail" to "target" for
>> trunk?
> 
> Sounds good.

Pushed as basepoints/gcc-14-9452-g5a44e14eb4f

> 
>> Is this patch ok for releases/gcc-13?
> 
> OK.

Pushed as releases/gcc-13.2.0-824-g1277f69b9b0

Kind regards,
Torbjörn

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [committed] testsuite: Added missing } in the dg-bogus comment [PR114343]
  2024-03-13 14:22                 ` [PATCH v2] testsuite: xfail " Torbjorn SVENSSON
@ 2024-03-15  8:33                   ` Torbjörn SVENSSON
  0 siblings, 0 replies; 14+ messages in thread
From: Torbjörn SVENSSON @ 2024-03-15  8:33 UTC (permalink / raw)
  To: gcc-patches; +Cc: yvan.roux, pinskia, Torbjörn SVENSSON

Committed below as obvious. The } got lost in my copy from the internal system.
Sorry for the inconvinience.

--

gcc/testsuite/ChangeLog:

	PR testsuite/114343
	* gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c:
	Added missing } in the dg-bogus comment.

Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
---
 .../null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c         | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c b/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
index e8cde7338a0..33cf10c1e29 100644
--- a/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
+++ b/gcc/testsuite/gcc.dg/analyzer/null-deref-pr108251-smp_fetch_ssl_fc_has_early-O2.c
@@ -60,7 +60,7 @@ static inline enum obj_type obj_type(const enum obj_type *t)
 }
 static inline struct connection *__objt_conn(enum obj_type *t)
 {
- return ((struct connection *)(((void *)(t)) - ((long)&((struct connection *)0)->obj_type))); /* { dg-bogus "may result in an unaligned pointer value" "Fixed in r14-6517-gb7e4a4c626e" { xfail short_enums } */
+ return ((struct connection *)(((void *)(t)) - ((long)&((struct connection *)0)->obj_type))); /* { dg-bogus "may result in an unaligned pointer value" "Fixed in r14-6517-gb7e4a4c626e" { xfail short_enums } } */
 }
 static inline struct connection *objt_conn(enum obj_type *t)
 {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2024-03-15  8:35 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-22 21:12 [PATCH 1/2] c-family: -Waddress-of-packed-member and casts Jason Merrill
2023-11-22 21:12 ` [PATCH 2/2] c-family: rename warn_for_address_or_pointer_of_packed_member Jason Merrill
2023-12-09  2:22 ` [PATCH 1/2] c-family: -Waddress-of-packed-member and casts Alexandre Oliva
2023-12-11  7:28 ` Richard Biener
2024-02-07 16:19   ` Torbjorn SVENSSON
2024-02-22  8:51     ` [PING] " Torbjorn SVENSSON
2024-02-28 23:07       ` Jason Merrill
2024-03-09  9:02         ` [PATCH] testsuite: xfail test for arm Torbjörn SVENSSON
2024-03-09  9:23           ` Andrew Pinski
2024-03-11 10:23             ` [PATCH v2] testsuite: xfail test for short_enums Torbjörn SVENSSON
2024-03-12 13:21               ` Jason Merrill
2024-03-13 14:18                 ` [comitted] testsuite: target " Torbjörn SVENSSON
2024-03-13 14:22                 ` [PATCH v2] testsuite: xfail " Torbjorn SVENSSON
2024-03-15  8:33                   ` [committed] testsuite: Added missing } in the dg-bogus comment [PR114343] Torbjörn SVENSSON

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).