This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[POC PATCH] rough prototype of __builtin_warning


Attached is a very rough and only superficially barely tested
prototype of the __builtin_warning intrinsic we talked about
the other day.  The built-in is declared like so:

  int __builtin_warning (int loc,
                         const char *option,
                         const char *txt, ...);

If it's still present during RTL expansion the built-in calls

  bool ret = warning_at (LOC, find_opt (option), txt, ...);

and expands to the constant value of RET (which could be used
to issue __builtin_note but there may be better ways to deal
with those than that).

LOC is the location of the warning or zero for the location of
the built-in itself (when called by user code), OPTION is either
the name of the warning option (e.g., "-Wnonnull", when called
by user code) or the index of the option (e.g., OPT_Wnonnull,
when emitted by GCC into the IL), and TXT is the format string
to format the warning text.  The rest of the arguments are not
used but I expect to extract and pass them to the diagnostic
pretty printer to format the text of the warning.

Using the built-in in user code should be obvious.  To show
how it might be put to use within GCC, I added code to
gimple-ssa-isolate-paths.c to emit -Wnonnull in response to
invalid null pointer accesses.  For this demo, when compiled
with the patch applied and with -Wnull-dereference (which is
not in -Wall or -Wextra), GCC issues three warnings: two
instances of -Wnull-dereference one of which is a false positive,
and one -Wnonnull (the one I added, which is included in -Wall),
which is a true positive:

  int f (void)
  {
    char a[4] = "12";
    char *p = __builtin_strlen (a) < 3 ? a : 0;
    return *p;
  }

  int g (void)
  {
    char a[4] = "12";
    char *p = __builtin_strlen (a) > 3 ? a : 0;
    return *p;
  }

  In function ‘f’:
  warning: potential null pointer dereference [-Wnull-dereference]
    7 |   return *p;
      |          ^~
  In function ‘g’:
  warning: null pointer dereference [-Wnull-dereference]
   14 |   return *p;
      |          ^~
  warning: invalid use of a null pointer [-Wnonnull]

The -Wnull-dereference instance in f is a false positive because
the strlen result is clearly less than two.  The strlen pass folds
the strlen result to a constant but it runs after path isolation
which will have already issued the bogus warning.

Martin

PS I tried compiling GCC with the patch.  It fails in libgomp
with:

  libgomp/oacc-mem.c: In function ‘gomp_acc_remove_pointer’:
  cc1: warning: invalid use of a null pointer [-Wnonnull]

so clearly it's missing location information.  With
-Wnull-dereference enabled we get more detail:

  libgomp/oacc-mem.c: In function ‘gomp_acc_remove_pointer’:
libgomp/oacc-mem.c:1013:31: warning: potential null pointer dereference [-Wnull-dereference]
   1013 |       for (size_t i = 0; i < t->list_count; i++)
        |                              ~^~~~~~~~~~~~
libgomp/oacc-mem.c:1012:19: warning: potential null pointer dereference [-Wnull-dereference]
   1012 |       t->refcount = minrefs;
        |       ~~~~~~~~~~~~^~~~~~~~~
libgomp/oacc-mem.c:1013:31: warning: potential null pointer dereference [-Wnull-dereference]
   1013 |       for (size_t i = 0; i < t->list_count; i++)
        |                              ~^~~~~~~~~~~~
libgomp/oacc-mem.c:1012:19: warning: potential null pointer dereference [-Wnull-dereference]
   1012 |       t->refcount = minrefs;
        |       ~~~~~~~~~~~~^~~~~~~~~
  cc1: warning: invalid use of a null pointer [-Wnonnull]

I didn't spend too long examining the code but it seems like
the warnings might actually be justified.  When the first loop
terminates with t being null the subsequent dereferences are
invalid:

      if (t->refcount == minrefs)
	{
	  /* This is the last reference, so pull the descriptor off the
	     chain. This prevents gomp_unmap_vars via gomp_unmap_tgt from
	     freeing the device memory. */
	  struct target_mem_desc *tp;
	  for (tp = NULL, t = acc_dev->openacc.data_environ; t != NULL;
	       tp = t, t = t->prev)
	    {
	      if (n->tgt == t)
		{
		  if (tp)
		    tp->prev = t->prev;
		  else
		    acc_dev->openacc.data_environ = t->prev;
		  break;
		}
	    }
	}

      /* Set refcount to 1 to allow gomp_unmap_vars to unmap it.  */
      n->refcount = 1;
      t->refcount = minrefs;
      for (size_t i = 0; i < t->list_count; i++)

gcc/ChangeLog:

	* builtin-types.def (BT_FN_INT_INT_CONST_STRING_CONST_STRING): New.
	* builtins.c (expand_builtin): Handle BUILT_IN_WARNING.
	(expand_builtin_memory_chk): Same.
	(is_simple_builtin): Same.
	* builtins.def (BUILT_IN_WARNING): New.
	* gimple-ssa-isolate-paths.c (insert_warning): New function.
	(isolate_path): Call insert_warning.
	(stmt_uses_name_in_undefined_way): Make static.
	(find_explicit_erroneous_behavior): Call insert_warning.

Index: gcc/builtin-types.def
===================================================================
--- gcc/builtin-types.def	(revision 274238)
+++ gcc/builtin-types.def	(working copy)
@@ -811,6 +811,7 @@ DEF_FUNCTION_TYPE_VAR_3 (BT_FN_SSIZE_STRING_SIZE_C
 			 BT_SSIZE, BT_STRING, BT_SIZE, BT_CONST_STRING)
 DEF_FUNCTION_TYPE_VAR_3 (BT_FN_INT_FILEPTR_INT_CONST_STRING_VAR,
 			 BT_INT, BT_FILEPTR, BT_INT, BT_CONST_STRING)
+DEF_FUNCTION_TYPE_VAR_3 (BT_FN_INT_INT_CONST_STRING_CONST_STRING, BT_INT, BT_INT, BT_CONST_STRING, BT_CONST_STRING)
 
 DEF_FUNCTION_TYPE_VAR_4 (BT_FN_INT_STRING_INT_SIZE_CONST_STRING_VAR,
 			 BT_INT, BT_STRING, BT_INT, BT_SIZE, BT_CONST_STRING)
Index: gcc/builtins.c
===================================================================
--- gcc/builtins.c	(revision 274238)
+++ gcc/builtins.c	(working copy)
@@ -72,6 +72,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "file-prefix-map.h" /* remap_macro_filename()  */
 #include "gomp-constants.h"
 #include "omp-general.h"
+#include "opts.h"
 
 struct target_builtins default_target_builtins;
 #if SWITCHABLE_TARGET
@@ -178,6 +179,7 @@ static tree fold_builtin_strcspn (location_t, tree
 static rtx expand_builtin_object_size (tree);
 static rtx expand_builtin_memory_chk (tree, rtx, machine_mode,
 				      enum built_in_function);
+static rtx expand_builtin_warning (tree);
 static void maybe_emit_chk_warning (tree, enum built_in_function);
 static void maybe_emit_sprintf_chk_warning (tree, enum built_in_function);
 static void maybe_emit_free_warning (tree);
@@ -8349,6 +8351,9 @@ expand_builtin (tree exp, rtx target, rtx subtarge
       mode = get_builtin_sync_mode (fcode - BUILT_IN_SPECULATION_SAFE_VALUE_1);
       return expand_speculation_safe_value (mode, exp, target, ignore);
 
+    case BUILT_IN_WARNING:
+      return expand_builtin_warning (exp);
+
     default:	/* just do library call, if unknown builtin */
       break;
     }
@@ -10486,6 +10491,46 @@ expand_builtin_memory_chk (tree exp, rtx target, m
     }
 }
 
+/* Expand a call to __builtin_warning into its constant result.  */
+
+static rtx
+expand_builtin_warning (tree exp)
+{
+  tree loc = CALL_EXPR_ARG (exp, 0);
+  tree opt = CALL_EXPR_ARG (exp, 1);
+  tree str = CALL_EXPR_ARG (exp, 2);
+
+  tree byteoff, memsize, decl;
+
+  /* The option index.  GCC internal calls pass in an an integer
+     but user calls pass in the option name as a string.  */
+  int iopt;
+  if (TREE_CODE (opt) == INTEGER_CST)
+    iopt = tree_to_shwi (opt);
+  else
+    {
+      /* Look up the option name.  */
+      tree byteoff, memsize, decl;
+      tree optstr = string_constant (opt, &byteoff, &memsize, &decl);
+      gcc_assert (optstr && decl == NULL);
+      const char *opt = TREE_STRING_POINTER (optstr);
+      size_t idx = find_opt (opt + 1, -1);
+      gcc_assert (idx != OPT_SPECIAL_unknown);
+      iopt = idx;
+    }
+
+  str = string_constant (str, &byteoff, &memsize, &decl);
+
+  /* FIXME: handle invalid arguments.  */
+  gcc_assert (str != 0 && decl == NULL);
+
+  location_t iloc = tree_to_shwi (loc);
+  if (iloc == 0)
+    iloc = EXPR_LOCATION (exp);
+  bool res = warning_at (iloc, iopt, "%s", TREE_STRING_POINTER (str));
+  return res ? const1_rtx : const0_rtx;
+}
+
 /* Emit warning if a buffer overflow is detected at compile time.  */
 
 static void
@@ -11137,6 +11182,8 @@ is_simple_builtin (tree decl)
       case BUILT_IN_EH_FILTER:
       case BUILT_IN_EH_POINTER:
       case BUILT_IN_EH_COPY_VALUES:
+	/* Trivially constant.  */
+      case BUILT_IN_WARNING:
 	return true;
 
       default:
Index: gcc/builtins.def
===================================================================
--- gcc/builtins.def	(revision 274238)
+++ gcc/builtins.def	(working copy)
@@ -1038,6 +1038,11 @@ DEF_GCC_BUILTIN (BUILT_IN_FILE, "FILE", BT_FN_CONS
 DEF_GCC_BUILTIN (BUILT_IN_FUNCTION, "FUNCTION", BT_FN_CONST_STRING, ATTR_NOTHROW_LEAF_LIST)
 DEF_GCC_BUILTIN (BUILT_IN_LINE, "LINE", BT_FN_INT, ATTR_NOTHROW_LEAF_LIST)
 
+/* __builtin_warning */
+DEF_GCC_BUILTIN (BUILT_IN_WARNING, "warning",
+		BT_FN_INT_INT_CONST_STRING_CONST_STRING,
+		ATTR_NOTHROW_NONNULL_LEAF)	
+
 /* Synchronization Primitives.  */
 #include "sync-builtins.def"
 
Index: gcc/gimple-ssa-isolate-paths.c
===================================================================
--- gcc/gimple-ssa-isolate-paths.c	(revision 274238)
+++ gcc/gimple-ssa-isolate-paths.c	(working copy)
@@ -115,6 +115,26 @@ insert_trap (gimple_stmt_iterator *si_p, tree op)
   *si_p = gsi_for_stmt (stmt);
 }
 
+/* Insert a call to __builtin_warning (LOC, OPT, STR) at IT, with LOC
+   set to the location of the statement at *IT.  */
+
+static void
+insert_warning (gimple_stmt_iterator *it, int opt, const char *str)
+{
+  gimple *stmt = gsi_stmt (*it);
+
+  location_t loc = gimple_location (stmt);
+  tree warnfn = builtin_decl_explicit (BUILT_IN_WARNING);
+  tree locarg = build_int_cst (integer_type_node, loc);
+  tree optarg = build_int_cst (integer_type_node, opt);
+  tree strarg = build_string_literal (strlen (str), str);
+  gcall *warn_stmt = gimple_build_call (warnfn, 3, locarg, optarg, strarg);
+  gimple_seq seq = NULL;
+  gimple_seq_add_stmt (&seq, warn_stmt);
+  gsi_insert_before (it, seq, GSI_NEW_STMT);
+  *it = gsi_for_stmt (stmt);
+}
+
 /* BB when reached via incoming edge E will exhibit undefined behavior
    at STMT.  Isolate and optimize the path which exhibits undefined
    behavior.
@@ -217,7 +237,11 @@ isolate_path (basic_block bb, basic_block duplicat
 	  update_stmt (ret);
 	}
       else
-	insert_trap (&si2, op);
+	{
+	  insert_warning (&si2, OPT_Wnonnull,
+			  "invalid use of a null pointer");
+	  insert_trap (&si2, op);
+	}
     }
 
   return duplicate;
@@ -262,7 +286,7 @@ is_divmod_with_given_divisor (gimple *stmt, tree d
    undefined behavior exposed by path splitting and that's reflected in
    the diagnostic.  */
 
-bool
+static bool
 stmt_uses_name_in_undefined_way (gimple *use_stmt, tree name, location_t loc)
 {
   /* If we are working with a non pointer type, then see
@@ -860,6 +884,8 @@ find_explicit_erroneous_behavior (void)
 
 	  if (stmt_uses_0_or_null_in_undefined_way (stmt))
 	    {
+	      insert_warning (&si, OPT_Wnonnull,
+			      "invalid use of a null pointer");
 	      insert_trap (&si, null_pointer_node);
 	      bb = gimple_bb (gsi_stmt (si));
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]