public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] detect references to statics in inline function signatures (PR 88718)
@ 2019-01-11 20:10 Martin Sebor
  2019-01-11 21:27 ` Joseph Myers
  2019-05-31 20:08 ` Jeff Law
  0 siblings, 2 replies; 8+ messages in thread
From: Martin Sebor @ 2019-01-11 20:10 UTC (permalink / raw)
  To: gcc-patches, Joseph S. Myers

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

The attached patch extends the detection of references to static
variables in inline functions to the function signatures, including
their return type.  Since the declaration of a function need not be
yet available at the time the static is referenced (e.g., when it's
referenced in the functions return type), the patch introduces
the concept of "tentative records" of such references and either
completes the records when the full declaration of the function,
including its definition, is known to be inline, or removes it
when it isn't.

Martin

[-- Attachment #2: gcc-88718.diff --]
[-- Type: text/x-patch, Size: 15121 bytes --]

PR c/88718 - Strange inconsistency between old style and new style definitions of inline functions.

gcc/c/ChangeLog:

	PR c/88718
	* c-decl.c (reset_inline_statics): New function.
	(record_inline_static): Optimize.
	(check_inline_statics): Handle tentative records for inline
	declarations without definitions.
	Print static declaration location.
	(push_file_scope): Clear records of references to statics.
	(finish_decl): Add tentative records of references to statics.
	(finish_function): Same.
	* c-typeck.c (build_external_ref): Handle all references to statics.

gcc/testsuite/ChangeLog:

	PR c/88718
	* gcc.dg/inline-40.c: New test.
	* gcc.dg/inline-41.c: New test.

Index: gcc/c/c-decl.c
===================================================================
--- gcc/c/c-decl.c	(revision 267616)
+++ gcc/c/c-decl.c	(working copy)
@@ -826,14 +826,47 @@ c_finish_incomplete_decl (tree decl)
     }
 }
 \f
-/* Record that inline function FUNC contains a reference (location
-   LOC) to static DECL (file-scope or function-local according to
-   TYPE).  */
+/* Free records of references to static variables gathered so far.  */
 
+static void
+reset_inline_statics (void)
+{
+  if (!c_inline_statics)
+    return;
+
+  for (c_inline_static *csi = c_inline_statics, *next = csi->next;
+       csi; csi = next)
+    ggc_free (csi);
+
+  c_inline_statics = NULL;
+}
+
+/* Record that inline function FUNC either does contain or may contain
+   a reference (location LOC) to static DECL (file-scope or function-local
+   according to TYPE).  For a null FUNC, a tentative record is created that
+   reflects a reference in the function signature and that is either updated
+   or removed when the function declaration is complete.  */
+
 void
 record_inline_static (location_t loc, tree func, tree decl,
 		      enum c_inline_static_type type)
 {
+  gcc_assert (decl);
+
+  if (c_inline_statics)
+    {
+      /* Avoid adding another tentative record for this DECL if one
+	 already exists.  */
+      for (c_inline_static *csi = c_inline_statics; csi; csi = csi->next)
+	{
+	  if (c_inline_statics->function)
+	    break;
+
+	  if (decl == c_inline_statics->static_decl)
+	    return;
+	}
+    }
+
   c_inline_static *csi = ggc_alloc<c_inline_static> ();
   csi->location = loc;
   csi->function = func;
@@ -843,6 +876,50 @@ record_inline_static (location_t loc, tree func, t
   c_inline_statics = csi;
 }
 
+/* Update tentative records of references to static declarations in
+   an inline declaration of function FUNC, or remove them if FUNC
+   isn't declared inline.  A tentative record is one whose FUNCTION
+   is null.  */
+
+static void
+update_tentative_inline_static (tree func)
+{
+  gcc_assert (func);
+
+  c_inline_static *csi = c_inline_statics;
+  if (!csi)
+      return;
+
+  /* True to associate FUNC with the tentative records, false to remove
+     them.  */
+  bool add = (DECL_DECLARED_INLINE_P (func)
+	      && DECL_EXTERNAL (func)
+	      && DECL_INITIAL (func));
+
+  for (c_inline_static *csi = c_inline_statics, *last = csi;
+       csi; csi = csi->next)
+    {
+      if (add)
+	{
+	  if (csi->function == func
+	      || csi->function)
+	    continue;
+
+	  csi->function = func;
+	}
+      else
+	{
+	  if (csi->function)
+	    break;
+
+	  if (last == c_inline_statics)
+	    c_inline_statics = last = csi->next;
+	  else
+	    last->next = csi->next;
+	}
+    }
+}
+
 /* Check for references to static declarations in inline functions at
    the end of the translation unit and diagnose them if the functions
    are still inline definitions.  */
@@ -853,22 +930,36 @@ check_inline_statics (void)
   struct c_inline_static *csi;
   for (csi = c_inline_statics; csi; csi = csi->next)
     {
-      if (DECL_EXTERNAL (csi->function))
-	switch (csi->type)
-	  {
-	  case csi_internal:
-	    pedwarn (csi->location, 0,
-		     "%qD is static but used in inline function %qD "
-		     "which is not static", csi->static_decl, csi->function);
-	    break;
-	  case csi_modifiable:
-	    pedwarn (csi->location, 0,
-		     "%q+D is static but declared in inline function %qD "
-		     "which is not static", csi->static_decl, csi->function);
-	    break;
-	  default:
-	    gcc_unreachable ();
-	  }
+      /* FUNCTION is unset for a declaration whose signature references
+	 a static variable and for which a definition wasn't provided.  */
+      if (!csi->function)
+	continue;
+
+      if (!DECL_EXTERNAL (csi->function))
+	continue;
+
+      bool warned;
+      switch (csi->type)
+	{
+	case csi_internal:
+	  warned = pedwarn (csi->location, 0,
+			    "%qD is static but used in inline function %qD "
+			    "which is not static",
+			    csi->static_decl, csi->function);
+	break;
+	case csi_modifiable:
+	  warned = pedwarn (csi->location, 0,
+			    "%q+D is static but declared in inline function %qD "
+			    "which is not static",
+			    csi->static_decl, csi->function);
+	  break;
+	default:
+	  gcc_unreachable ();
+	}
+
+      if (warned)
+	inform (DECL_SOURCE_LOCATION (csi->static_decl),
+		"%qD declared here", csi->static_decl);
     }
   c_inline_statics = NULL;
 }
@@ -1412,6 +1503,9 @@ push_file_scope (void)
 
   start_fname_decls ();
 
+  /* Free references to static declarations in inline functions.  */
+  reset_inline_statics ();
+
   for (decl = visible_builtins; decl; decl = DECL_CHAIN (decl))
     bind (DECL_NAME (decl), decl, file_scope,
 	  /*invisible=*/false, /*nested=*/true, DECL_SOURCE_LOCATION (decl));
@@ -5165,6 +5259,11 @@ finish_decl (tree decl, location_t init_loc, tree
 	    set_user_assembler_name (decl, asmspec);
 	}
 
+      /* Remove any tentative record of a non-static inline function
+	 referencing a static decl made a DECL that is not a definition.  */
+      if (TREE_CODE (decl) == FUNCTION_DECL)
+	update_tentative_inline_static (decl);
+	
       if (DECL_FILE_SCOPE_P (decl))
 	{
 	  if (DECL_INITIAL (decl) == NULL_TREE
@@ -9636,6 +9735,11 @@ finish_function (void)
 	}
     }
 
+  /* Associate any tentative records of references to static variables
+     with this function declaration if it is inline and not static, or
+     remove them otherwise.  */
+  update_tentative_inline_static (fndecl);
+
   if (!decl_function_context (fndecl))
     undef_nested_function = false;
 
Index: gcc/c/c-typeck.c
===================================================================
--- gcc/c/c-typeck.c	(revision 267616)
+++ gcc/c/c-typeck.c	(working copy)
@@ -52,6 +52,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "stringpool.h"
 #include "attribs.h"
 #include "asan.h"
+#include "c-family/c-pragma.h"
+#include "c-parser.h"
 
 /* Possible cases of implicit bad conversions.  Used to select
    diagnostic messages in convert_for_assignment.  */
@@ -2818,19 +2820,26 @@ build_external_ref (location_t loc, tree id, bool
       if (context != NULL_TREE && context != current_function_decl)
 	DECL_NONLOCAL (ref) = 1;
     }
-  /* C99 6.7.4p3: An inline definition of a function with external
-     linkage ... shall not contain a reference to an identifier with
-     internal linkage.  */
-  else if (current_function_decl != NULL_TREE
+  else if (VAR_OR_FUNCTION_DECL_P (ref)
+	   && (!VAR_P (ref) || TREE_STATIC (ref))
+	   && ! TREE_PUBLIC (ref))
+    {
+      /* C99 6.7.4p3: An inline definition of a function with external
+	 linkage ... shall not contain a reference to an identifier with
+	 internal linkage.  */
+      if ((current_function_decl
 	   && DECL_DECLARED_INLINE_P (current_function_decl)
 	   && DECL_EXTERNAL (current_function_decl)
-	   && VAR_OR_FUNCTION_DECL_P (ref)
-	   && (!VAR_P (ref) || TREE_STATIC (ref))
-	   && ! TREE_PUBLIC (ref)
 	   && DECL_CONTEXT (ref) != current_function_decl)
-    record_inline_static (loc, current_function_decl, ref,
-			  csi_internal);
-
+	  /* If the function declaration is not available yet at this
+	     point make a record of the reference if it isn't associated
+	     with any context yet and either fill in the details or
+	     discard the record when the declaration has been completed.  */
+	  || (!current_function_decl
+	      && !DECL_CONTEXT (ref)))
+	record_inline_static (loc, current_function_decl, ref,
+			      csi_internal);
+    }
   return ref;
 }
 
Index: gcc/testsuite/gcc.dg/inline-40.c
===================================================================
--- gcc/testsuite/gcc.dg/inline-40.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/inline-40.c	(working copy)
@@ -0,0 +1,130 @@
+/* PR c/88718 - Strange inconsistency between old style and new style
+   definitions of inline functions
+   { dg-do compile }
+   { dg-options "-Wall -Wno-unused" } */
+
+extern int global;
+extern const int cst_global;
+
+static int local;
+static const int cst_local;
+
+/* Verify that inline declarations that aren't definitions are accepted
+   without a warning even if their argument lists reference static
+   variables.  */
+
+inline void global_decl_arg_ref_global (int (*)[sizeof global]);
+static inline void local_decl_arg_ref_global (int (*)[sizeof global]);
+
+inline void global_decl_arg_ref_cst_global (int (*)[sizeof cst_global]);
+static inline void local_decl_arg_ref_cst_global (int (*)[sizeof cst_global]);
+
+inline void global_decl_arg_ref_local (int (*)[sizeof local]);
+static inline void local_decl_arg_ref_local (int (*)[sizeof local]);
+
+inline void global_decl_arg_ref_cst_local (int (*)[sizeof cst_local]);
+static inline void local_decl_arg_ref_cst_local (int (*)[sizeof cst_local]);
+
+/* Verify that implicitly extern inline definitions trigger a warning
+   when their argument lists reference static (but not extern) variables.  */
+
+inline void
+global_decl_arg_ref_global (int (*p)[sizeof global]) { (void)&p; }
+
+inline void
+global_decl_arg_ref_cst_global (int (*p)[sizeof cst_global]) { (void)&p; }
+
+inline void
+global_decl_arg_ref_local (int (*p)[sizeof local])
+/* { dg-warning ".local. is static but used in inline function .global_decl_arg_ref_local." "" { target *-*-* } .-1 } */
+{
+  (void)&p;
+}
+
+inline void
+global_decl_arg_ref_cst_local (int (*p)[sizeof cst_local])
+/* { dg-warning ".cst_local. is static but used in inline function .global_decl_arg_ref_cst_local." "" { target *-*-* } .-1 } */
+{
+  (void)&p;
+}
+
+/* Verify that static inline definitions don't trigger a warning
+   when their argument lists reference static or extern variables.  */
+
+static inline void
+local_decl_arg_ref_global (int (*p)[sizeof global]) { (void)&p; }
+static inline void
+local_decl_arg_ref_cst_global (int (*p)[sizeof cst_global]) { (void)&p; }
+static inline void
+local_decl_arg_ref_local (int (*p)[sizeof local]) { (void)&p; }
+static inline void
+local_decl_arg_ref_cst_local (int (*p)[sizeof cst_local]) { (void)&p; }
+
+
+/* Same as above but with return types.  */
+
+extern void abort (void);
+
+/* Verify that inline declarations that aren't definitions are accepted
+   without a warning even if their return types reference static
+   variables.  */
+
+inline struct { int a[sizeof global]; }
+  global_decl_ret_ref_global (void);
+
+static inline struct { int a[sizeof global]; }
+  local_decl_ret_ref_global (void);
+
+inline struct { int a[sizeof cst_global]; }
+  global_decl_ret_ref_cst_global (void);
+
+static inline struct { int a[sizeof cst_global]; }
+  local_decl_ret_ref_cst_global (void);
+
+inline struct { int a[sizeof local]; }
+  global_decl_ret_ref_local (void);
+static inline struct { int a[sizeof local]; }
+  local_decl_ret_ref_local (void);
+
+inline struct { int a[sizeof cst_local]; }
+  global_decl_ret_ref_cst_local (void);
+
+static inline struct { int a[sizeof global]; }
+  local_decl_ret_ref_cst_local (void);
+
+/* Verify that implicitly extern inline definitions trigger a warning
+   when their return types reference static (but not extern) variables.  */
+
+inline struct { int a[sizeof global]; }
+  global_def_ret_ref_global (void) { abort (); }
+
+inline struct { int a[sizeof cst_global]; }
+  global_def_ret_ref_cst_global (void) { abort (); }
+
+inline struct { int a[sizeof local]; }
+  global_def_ret_ref_local (void)
+  /* { dg-warning ".local. is static but used in inline function .global_def_ret_ref_local." "" { target *-*-* } .-2 } */
+  {
+    abort ();
+  }
+
+inline struct { int a[sizeof cst_local]; }
+  global_def_ret_ref_cst_local (void)
+  /* { dg-warning ".cst_local. is static but used in inline function .global_def_ret_ref_cst_local." "" { target *-*-* } .-2 } */
+  {
+    abort ();
+  }
+
+/* Verify that static inline definitions don't trigger a warning
+   when their return types reference static or extern variables.  */
+
+static inline struct { int a[sizeof global]; }
+  local_def_ret_ref_global (void) { abort (); }
+static inline struct { int a[sizeof cst_global]; }
+  local_def_ret_ref_cst_global (void) { abort (); }
+static inline struct { int a[sizeof local]; }
+  local_def_ret_ref_local (void) { abort (); }
+static inline struct { int a[sizeof cst_local]; }
+  local_def_ret_ref_cst_local (void) { abort (); }
+
+/* { dg-prune-output "declared but never defined" } */
Index: gcc/testsuite/gcc.dg/inline-41.c
===================================================================
--- gcc/testsuite/gcc.dg/inline-41.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/inline-41.c	(working copy)
@@ -0,0 +1,53 @@
+/* PR c/88718 - Strange inconsistency between old style and new style
+   definitions of inline functions
+   Verify that warnings for references to multiple static variables in
+   inline function signatures are all diagnosed and that the locations
+   of the diagnostics, including the notes, are correct.
+   { dg-do compile }
+   { dg-options "-Wall -Wno-unused" } */
+
+extern const int e1;
+extern const int e2;
+
+static const int s1 = 1;   /* { dg-message ".s1. declared here" } */
+static const int s2 = 2;   /* { dg-message ".s2. declared here" } */
+
+
+inline void fes (int (*)[sizeof e1][sizeof s2]);
+
+inline void
+fes (int (*p)
+     [sizeof e1]
+     [sizeof s2])   /* { dg-warning ".s2. is static but used in inline function .fes. which is not static" } */
+{ }
+
+
+inline void fse (int (*)[sizeof s1][sizeof e2]);
+
+inline void
+fse (int (*p)
+     [sizeof s1]    /* { dg-warning ".s1. is static but used in inline function .fse. which is not static" } */
+     [sizeof e2])
+{ }
+
+
+static int s3 = 3;   /* { dg-message ".s3. declared here" } */
+static int s4 = 4;   /* { dg-message ".s4. declared here" } */
+
+/* Use s1 and s2 here and verify that the warnings mention s3 and s4
+   referenced in the definition.  */
+inline void fss (int (*)[sizeof s1][sizeof s2]);
+
+inline void
+fss (int (*p)
+     [sizeof s3]    /* { dg-warning ".s3. is static but used in inline function .fss. which is not static" } */
+     [sizeof s4])   /* { dg-warning ".s4. is static but used in inline function .fss. which is not static" } */
+{ }
+
+
+/* Use s1 and s2 in the declaration and verify that no warnings are
+   emitted for the definition that doesn't reference any statics.  */
+inline void fee (int (*)[sizeof s1][sizeof s2]);
+
+inline void
+fee (int (*p)[sizeof e1][sizeof e2]) { }

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

end of thread, other threads:[~2019-06-03 16:30 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-11 20:10 [PATCH] detect references to statics in inline function signatures (PR 88718) Martin Sebor
2019-01-11 21:27 ` Joseph Myers
2019-01-16 21:09   ` Martin Sebor
2019-01-17  1:10     ` Joseph Myers
2019-05-31 20:08 ` Jeff Law
2019-06-03  9:03   ` Richard Biener
2019-06-03 15:20   ` Joseph Myers
2019-06-03 16:30     ` Martin Sebor

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).