[POC PATCH] rough prototype of __builtin_warning

[Martin Sebor]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: gcc-builtin_warning.diff
Type: text/x-patch
Size: 6799 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20190809/ea3a612d/attachment.bin>