[PR c/84293] Unexpected strict-alias warning

[PR c/84293] Unexpected strict-alias warning

[Nathan Sidwell]

[-- Attachment #1: Type: text/plain, Size: 1419 bytes --]

This bug comes from python sources.  It has some type-punning macros to 
switch between representation, but these can cause unexpected 
strict-alias warnings, even though the punning's context is a system 
header file.  (I guess that without macro expansion location information 
we'd've thought the context was user code.)

As shown in the bug report, if the pun is spelt '(struct X *)&obj' we do 
not warn, but if it is '(X_t *)&obj' we do.  We also don't warn in 
either case if we go via -save-temps.  The patch changes things to not 
warn in either spelling.

The problem is that strict_aliasing_warning uses input_location to 
identifier the current code point.  But that's not necessarily the 
location of the above pun -- it can be the point where we use the pun.

We cannot just use the incoming EXPR's location, because we may well 
have peeled some pieces off it, and we end up with the '&obj' fragment, 
which in this case is located in the user source.  It also turns out 
that strict_aliasing_warning no longer uses the incoming 'otype' 
argument, immediately doing:
    STRIP_NOPS (expr);
    otype = TREE_TYPE (expr);

So we may as well drop that arg, substituting a location_t from the caller.

That's what this patch does.  The callers in the C & C++ FEs pass the 
EXPR_LOCATION of the pointer being dereferenced, or converted, as 
appropriate.

Joseph, are the C bits ok?

nathan

-- 
Nathan Sidwell

[-- Attachment #2: 84293.diff --]
[-- Type: text/x-patch, Size: 7992 bytes --]

2018-02-09  Nathan Sidwell  <nathan@acm.org>

	PR c/84293
	gcc/c/
	* c-typeck.c (build_indirect_ref, build_c_cast): Pass expr location
	to strict_aliasing_warning.

	gcc/c-family/
	* c-common.h (strict_aliasing_warning): Drop OTYPE arg, insert LOC
	arg.
	* c-warn.c (strict_aliasing_warning): Drop OTYPE arg, require LOC
	arg.  Adjust.

	gcc/cp/
	* typeck.c (cp_build_indirect_ref_1, build_reinterpret_cast_1):
	Pass expr location to strict_aliasing_warning.

	gcc/testsuite/
	* c-c++-common/pr84293.h: New.
	* c-c++-common/pr84293.c: New.

Index: gcc/c/c-typeck.c
===================================================================
--- gcc/c/c-typeck.c	(revision 257480)
+++ gcc/c/c-typeck.c	(working copy)
@@ -2524,7 +2524,7 @@ build_indirect_ref (location_t loc, tree
 	     the backend.  This only needs to be done at
 	     warn_strict_aliasing > 2.  */
 	  if (warn_strict_aliasing > 2)
-	    if (strict_aliasing_warning (TREE_TYPE (TREE_OPERAND (pointer, 0)),
+	    if (strict_aliasing_warning (EXPR_LOCATION (pointer),
 					 type, TREE_OPERAND (pointer, 0)))
 	      TREE_NO_WARNING (pointer) = 1;
 	}
@@ -5696,7 +5696,7 @@ build_c_cast (location_t loc, tree type,
 		    "of different size");
 
       if (warn_strict_aliasing <= 2)
-        strict_aliasing_warning (otype, type, expr);
+        strict_aliasing_warning (EXPR_LOCATION (value), type, expr);
 
       /* If pedantic, warn for conversions between function and object
 	 pointer types, except for converting a null pointer constant
Index: gcc/c-family/c-common.h
===================================================================
--- gcc/c-family/c-common.h	(revision 257480)
+++ gcc/c-family/c-common.h	(working copy)
@@ -1260,7 +1260,7 @@ extern void warn_tautological_cmp (locat
 extern void warn_logical_not_parentheses (location_t, enum tree_code, tree,
 					  tree);
 extern bool warn_if_unused_value (const_tree, location_t);
-extern bool strict_aliasing_warning (tree, tree, tree);
+extern bool strict_aliasing_warning (location_t, tree, tree);
 extern void sizeof_pointer_memaccess_warning (location_t *, tree,
 					      vec<tree, va_gc> *, tree *,
 					      bool (*) (tree, tree));
Index: gcc/c-family/c-warn.c
===================================================================
--- gcc/c-family/c-warn.c	(revision 257480)
+++ gcc/c-family/c-warn.c	(working copy)
@@ -599,17 +599,21 @@ warn_if_unused_value (const_tree exp, lo
     }
 }
 
-/* Print a warning about casts that might indicate violation
-   of strict aliasing rules if -Wstrict-aliasing is used and
-   strict aliasing mode is in effect. OTYPE is the original
-   TREE_TYPE of EXPR, and TYPE the type we're casting to. */
+/* Print a warning about casts that might indicate violation of strict
+   aliasing rules if -Wstrict-aliasing is used and strict aliasing
+   mode is in effect.  LOC is the location of the expression being
+   cast, EXPR might be from inside it.  TYPE is the type we're casting
+   to.  */
 
 bool
-strict_aliasing_warning (tree otype, tree type, tree expr)
+strict_aliasing_warning (location_t loc, tree type, tree expr)
 {
+  if (loc == UNKNOWN_LOCATION)
+    loc = input_location;
+
   /* Strip pointer conversion chains and get to the correct original type.  */
   STRIP_NOPS (expr);
-  otype = TREE_TYPE (expr);
+  tree otype = TREE_TYPE (expr);
 
   if (!(flag_strict_aliasing
 	&& POINTER_TYPE_P (type)
@@ -628,8 +632,9 @@ strict_aliasing_warning (tree otype, tre
 	 if the cast breaks type based aliasing.  */
       if (!COMPLETE_TYPE_P (TREE_TYPE (type)) && warn_strict_aliasing == 2)
 	{
-	  warning (OPT_Wstrict_aliasing, "type-punning to incomplete type "
-		   "might break strict-aliasing rules");
+	  warning_at (loc, OPT_Wstrict_aliasing,
+		      "type-punning to incomplete type "
+		      "might break strict-aliasing rules");
 	  return true;
 	}
       else
@@ -645,15 +650,17 @@ strict_aliasing_warning (tree otype, tre
 	      && !alias_set_subset_of (set2, set1)
 	      && !alias_sets_conflict_p (set1, set2))
 	    {
-	      warning (OPT_Wstrict_aliasing, "dereferencing type-punned "
-		       "pointer will break strict-aliasing rules");
+	      warning_at (loc, OPT_Wstrict_aliasing,
+			  "dereferencing type-punned "
+			  "pointer will break strict-aliasing rules");
 	      return true;
 	    }
 	  else if (warn_strict_aliasing == 2
 		   && !alias_sets_must_conflict_p (set1, set2))
 	    {
-	      warning (OPT_Wstrict_aliasing, "dereferencing type-punned "
-		       "pointer might break strict-aliasing rules");
+	      warning_at (loc, OPT_Wstrict_aliasing,
+			  "dereferencing type-punned "
+			  "pointer might break strict-aliasing rules");
 	      return true;
 	    }
 	}
@@ -669,8 +676,9 @@ strict_aliasing_warning (tree otype, tre
       if (!COMPLETE_TYPE_P (type)
 	  || !alias_sets_must_conflict_p (set1, set2))
 	{
-	  warning (OPT_Wstrict_aliasing, "dereferencing type-punned "
-		   "pointer might break strict-aliasing rules");
+	  warning_at (loc, OPT_Wstrict_aliasing,
+		      "dereferencing type-punned "
+		      "pointer might break strict-aliasing rules");
 	  return true;
 	}
     }
Index: gcc/cp/typeck.c
===================================================================
--- gcc/cp/typeck.c	(revision 257480)
+++ gcc/cp/typeck.c	(working copy)
@@ -3133,7 +3133,7 @@ cp_build_indirect_ref_1 (tree ptr, ref_o
 	     the backend.  This only needs to be done at
 	     warn_strict_aliasing > 2.  */
 	  if (warn_strict_aliasing > 2)
-	    if (strict_aliasing_warning (TREE_TYPE (TREE_OPERAND (ptr, 0)),
+	    if (strict_aliasing_warning (EXPR_LOCATION (ptr),
 					 type, TREE_OPERAND (ptr, 0)))
 	      TREE_NO_WARNING (ptr) = 1;
 	}
@@ -7331,7 +7331,7 @@ build_reinterpret_cast_1 (tree type, tre
       expr = cp_build_addr_expr (expr, complain);
 
       if (warn_strict_aliasing > 2)
-	strict_aliasing_warning (TREE_TYPE (expr), type, expr);
+	strict_aliasing_warning (EXPR_LOCATION (expr), type, expr);
 
       if (expr != error_mark_node)
 	expr = build_reinterpret_cast_1
@@ -7425,8 +7425,6 @@ build_reinterpret_cast_1 (tree type, tre
   else if ((TYPE_PTRDATAMEM_P (type) && TYPE_PTRDATAMEM_P (intype))
 	   || (TYPE_PTROBV_P (type) && TYPE_PTROBV_P (intype)))
     {
-      tree sexpr = expr;
-
       if (!c_cast_p
 	  && check_for_casting_away_constness (intype, type,
 					       REINTERPRET_CAST_EXPR,
@@ -7444,11 +7442,9 @@ build_reinterpret_cast_1 (tree type, tre
 	warning (OPT_Wcast_align, "cast from %qH to %qI "
 		 "increases required alignment of target type", intype, type);
 
-      /* We need to strip nops here, because the front end likes to
-	 create (int *)&a for array-to-pointer decay, instead of &a[0].  */
-      STRIP_NOPS (sexpr);
       if (warn_strict_aliasing <= 2)
-	strict_aliasing_warning (intype, type, sexpr);
+	/* strict_aliasing_warning STRIP_NOPs its expr.  */
+	strict_aliasing_warning (EXPR_LOCATION (expr), type, expr);
 
       return build_nop (type, expr);
     }
Index: gcc/testsuite/c-c++-common/pr84293.c
===================================================================
--- gcc/testsuite/c-c++-common/pr84293.c	(revision 0)
+++ gcc/testsuite/c-c++-common/pr84293.c	(working copy)
@@ -0,0 +1,10 @@
+/* PR c/84293 unexpected warning from system header.  */
+#include "./pr84293.h"
+struct typeobject thing;
+
+#pragma GCC diagnostic warning "-Wstrict-aliasing"
+void __attribute__ ((optimize (2))) init ()
+{
+  INCREF_TDEF (&thing);
+  INCREF_STAG (&thing);
+}
Index: gcc/testsuite/c-c++-common/pr84293.h
===================================================================
--- gcc/testsuite/c-c++-common/pr84293.h	(revision 0)
+++ gcc/testsuite/c-c++-common/pr84293.h	(working copy)
@@ -0,0 +1,7 @@
+/* PR c/84293 unexpected warning from system header expansion.  */
+#pragma GCC system_header
+struct typeobject { unsigned refs; };
+typedef struct object { unsigned refs; } Object;
+
+#define INCREF_TDEF(op) (((Object*)(op))->refs++)
+#define INCREF_STAG(op) (((struct object*)(op))->refs++)

Re: [PR c/84293] Unexpected strict-alias warning

[Joseph Myers]

On Fri, 9 Feb 2018, Nathan Sidwell wrote:

> Joseph, are the C bits ok?

Yes.

-- 
Joseph S. Myers
joseph@codesourcery.com