[PATCH] Add warning_decl and error_decl attributes

Jakub Jelinek jakub@redhat.com
Mon Sep 10 13:37:00 GMT 2007


Hi!

On Sat, Sep 08, 2007 at 08:29:01AM -0400, Jakub Jelinek wrote:
> The link time warnings resp. errors aren't perfect, linker will
> in these cases show location inside of the inline fn with -g,
> which won't help much finding the bug, without -g it will show
> section plus offset, which is possible to look up.  We have
> the same issue with e.g. memset swapped argument checking in C++,
> I will try to address it later with a separate patch.  But even if
> that is not resolved, this is useful on its own and is generic enough
> that it could be used for many other uses as well.

Here is the promissed patch.

It introduces 2 new attributes, error_decl and warning_decl.

error_decl functions when call to them isn't optimized out result in
compile time error with some user chosen message text in the error.
Currently when projects need similar compile time diagnostics,
they typically use
extern char write_here_the_error_message[condition ? 1 : -1];
or
extern void undefined_the_error_message(void);
...
if (!condition)
  undefined_the_error_message ();
The former is an compile time error, but requires that condition
is constant already during parsing, so can be useful e.g. with
macros, but isn't useful to check inline function arguments.
The latter is a link time error, so it will be detected later than
desirable, without -g isn't able to print any kind of location
where the error occurred and even with -g it will not do the best
job in presence of inline functions - the locus will be in the inline
function, while most often it is also interesting which function inlined
it, as the arguments of the inline function often matter a lot.
E.g. the Linux kernel heavily uses especially the link time errors
and as this attribute fits nicely to such kind of use, it would just
mean adding the attribute to those artificial never defined prototypes.
Similarly, glibc would like to use this for compile time checking
open(2) arguments - when it is implemented as function-like macro,
the first trick is possible, but we want to switch to using
an inline function if __builtin_va_arg_pack_len () is approved.

warning_decl is similar to error_decl, but it prints a warning
instead of error.  It is useful e.g. for glibc swapped memset arguments
warning.  Currently glibc uses
__warndecl (__warn_memset_zero_len,
            "memset used with constant zero length parameter; this could be due to transposed parameters");
#ifdef __cplusplus
__extern_always_inline void *
__NTH (memset (void *__dest, int __ch, size_t __len))
{
  if (__builtin_constant_p (__len) && __len == 0)
    {
      __warn_memset_zero_len ();
      return __dest;
    }
  return __builtin___memset_chk (__dest, __ch, __len, __bos0 (__dest));
}
#else
# define memset(dest, ch, len) \
  (__builtin_constant_p (len) && (len) == 0 \
   ? (__warn_memset_zero_len (), (void) (ch), (void) (len), (void *) (dest)) \
   : ((__bos0 (dest) != (size_t) -1) \
      ? __builtin___memset_chk (dest, ch, len, __bos0 (dest)) \
      : __memset_ichk (dest, ch, len)))
static __always_inline void *
__NTH (__memset_ichk (void *__dest, int __ch, size_t __len))
{
  return __builtin___memset_chk (__dest, __ch, __len, __bos0 (__dest));
}
#endif

where
#define __warndecl(name, msg) extern void name (void)

but libc_nonshared.a includes a special file which has
#define __warndecl(name, msg) \
  extern void name (void) __attribute__ ((alias ("nop"))) attribute_hidden; \
  link_warning (name, msg)
instead.  So, this issues a link time warning where a compile time warning
would be better.  But more importantly, in C gives a useful location
who used it, but in C++ due to the inline fn reports it with the locus
of the inline function, which is generally not very interesting, the problem
is in its caller or caller of the caller etc.

This patch depends on the %K patch I posted earlier today (well,
it could avoid using %K and instead use %H with &EXPR_LOCATION (exp),
but it would be much less useful without it) and also on the
__builtin_va_arg_pack_len () patch (but that dependency is only for
testing, if __builtin_va_arg_pack_len () is NACKed and this one is
ACKed, other testcases can be of course written).

2007-09-10  Jakub Jelinek  <jakub@redhat.com>

	* expr.c (expand_expr_real_1) <case CALL_EXPR>: Use get_callee_fndecl
	instead of checking CALL_EXPR_FN directly to test for builtins.
	If error_decl or warning_decl attributes are present, print
	error resp. warning.
	c-common.c (handle_error_decl_attribute): New function.
	(c_common_attribute_table): Add error_decl and warning_decl
	attributes.
	* doc/extend.texi: Document error_decl and warning_decl attributes.

	* gcc.dg/va-arg-pack-len-1.c: Use error_decl and warning_decl
	attributes.
	* gcc.dg/va-arg-pack-len-2.c: New test.
	* g++.dg/ext/va-arg-pack-len-1.C: Use error_decl and warning_decl
	attributes.
	* g++.dg/ext/va-arg-pack-len-2.C: New test.

--- gcc/expr.c.jj	2007-09-07 10:29:37.000000000 +0200
+++ gcc/expr.c	2007-09-10 14:04:54.000000000 +0200
@@ -7966,22 +7966,33 @@ expand_expr_real_1 (tree exp, rtx target
       /* All valid uses of __builtin_va_arg_pack () are removed during
 	 inlining.  */
       if (CALL_EXPR_VA_ARG_PACK (exp))
-	error ("invalid use of %<__builtin_va_arg_pack ()%>");
-      /* Check for a built-in function.  */
-      if (TREE_CODE (CALL_EXPR_FN (exp)) == ADDR_EXPR
-	  && (TREE_CODE (TREE_OPERAND (CALL_EXPR_FN (exp), 0))
-	      == FUNCTION_DECL)
-	  && DECL_BUILT_IN (TREE_OPERAND (CALL_EXPR_FN (exp), 0)))
-	{
-	  if (DECL_BUILT_IN_CLASS (TREE_OPERAND (CALL_EXPR_FN (exp), 0))
-	      == BUILT_IN_FRONTEND)
-	    return lang_hooks.expand_expr (exp, original_target,
-					   tmode, modifier,
-					   alt_rtl);
-	  else
-	    return expand_builtin (exp, target, subtarget, tmode, ignore);
-	}
+	error ("%Kinvalid use of %<__builtin_va_arg_pack ()%>", exp);
+      {
+	tree fndecl = get_callee_fndecl (exp), attr;
+
+	if (fndecl
+	    && (attr = lookup_attribute ("error_decl",
+					 DECL_ATTRIBUTES (fndecl))) != NULL)
+	  error ("%Kcall to %qs declared with attribute error_decl: %s",
+		 exp, lang_hooks.decl_printable_name (fndecl, 1),
+		 TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr))));
+	if (fndecl
+	    && (attr = lookup_attribute ("warning_decl",
+					 DECL_ATTRIBUTES (fndecl))) != NULL)
+	  warning (0, "%Kcall to %qs declared with attribute warning_decl: %s",
+		   exp, lang_hooks.decl_printable_name (fndecl, 1),
+		   TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr))));
 
+	/* Check for a built-in function.  */
+	if (fndecl && DECL_BUILT_IN (fndecl))
+	  {
+	    if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_FRONTEND)
+	      return lang_hooks.expand_expr (exp, original_target,
+					     tmode, modifier, alt_rtl);
+	    else
+	      return expand_builtin (exp, target, subtarget, tmode, ignore);
+	  }
+      }
       return expand_call (exp, target, ignore);
 
     case NON_LVALUE_EXPR:
--- gcc/c-common.c.jj	2007-09-04 23:09:31.000000000 +0200
+++ gcc/c-common.c	2007-09-10 13:40:10.000000000 +0200
@@ -518,6 +518,8 @@ static tree handle_always_inline_attribu
 static tree handle_gnu_inline_attribute (tree *, tree, tree, int,
 					 bool *);
 static tree handle_flatten_attribute (tree *, tree, tree, int, bool *);
+static tree handle_error_decl_attribute (tree *, tree, tree, int,
+					 bool *);
 static tree handle_used_attribute (tree *, tree, tree, int, bool *);
 static tree handle_unused_attribute (tree *, tree, tree, int, bool *);
 static tree handle_externally_visible_attribute (tree *, tree, tree, int,
@@ -661,6 +663,10 @@ const struct attribute_spec c_common_att
 			      handle_cold_attribute },
   { "hot",                    0, 0, true,  false, false,
 			      handle_hot_attribute },
+  { "warning_decl",           1, 1, true,  false, false,
+			      handle_error_decl_attribute },
+  { "error_decl",	      1, 1, true,  false, false,
+			      handle_error_decl_attribute },
   { NULL,                     0, 0, false, false, false, NULL }
 };
 
@@ -4912,6 +4918,26 @@ handle_flatten_attribute (tree *node, tr
   return NULL_TREE;
 }
 
+/* Handle a "warning_decl" or "error_decl" attribute; arguments as in
+   struct attribute_spec.handler.  */
+
+static tree
+handle_error_decl_attribute (tree *node, tree name, tree args,
+			  int ARG_UNUSED (flags), bool *no_add_attrs)
+{
+  if (TREE_CODE (*node) == FUNCTION_DECL
+      || TREE_CODE (TREE_VALUE (args)) == STRING_CST)
+    /* Do nothing else, just set the attribute.  We'll get at
+       it later with lookup_attribute.  */
+    ;
+  else
+    {
+      warning (OPT_Wattributes, "%qE attribute ignored", name);
+      *no_add_attrs = true;
+    }
+
+  return NULL_TREE;
+}
 
 /* Handle a "used" attribute; arguments as in
    struct attribute_spec.handler.  */
--- gcc/testsuite/gcc.dg/va-arg-pack-len-1.c.jj	2007-09-08 13:10:38.000000000 +0200
+++ gcc/testsuite/gcc.dg/va-arg-pack-len-1.c	2007-09-10 14:15:17.000000000 +0200
@@ -3,8 +3,10 @@
 
 #include <stdarg.h>
 
-extern int warn_open_missing_mode (void);
-extern int warn_open_too_many_arguments (void);
+extern int error_open_missing_mode (void)
+  __attribute__((__error_decl__ ("open with O_CREAT needs 3 arguments, only 2 were given")));
+extern int warn_open_too_many_arguments (void)
+  __attribute__((__warning_decl__ ("open called with more than 3 arguments")));
 extern void abort (void);
 
 char expected_char;
@@ -83,7 +85,7 @@ myopen (const char *path, int oflag, ...
     {
       if ((oflag & 0x40) != 0 && __builtin_va_arg_pack_len () < 1)
 	{
-	  warn_open_missing_mode ();
+	  error_open_missing_mode ();
 	  return myopen2 (path, oflag);
 	}
       return myopenva (path, oflag, __builtin_va_arg_pack ());
--- gcc/testsuite/gcc.dg/va-arg-pack-len-2.c.jj	2007-09-10 14:12:34.000000000 +0200
+++ gcc/testsuite/gcc.dg/va-arg-pack-len-2.c	2007-09-10 14:17:20.000000000 +0200
@@ -0,0 +1,42 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+#include <stdarg.h>
+
+extern int error_open_missing_mode (void)
+  __attribute__((__error_decl__ ("open with O_CREAT needs 3 arguments, only 2 were given")));
+extern int warn_open_too_many_arguments (void)
+  __attribute__((__warning_decl__ ("open called with more than 3 arguments")));
+
+extern int myopen2 (const char *path, int oflag);
+extern int myopenva (const char *path, int oflag, ...);
+
+extern inline __attribute__((always_inline, gnu_inline)) int
+myopen (const char *path, int oflag, ...)
+{
+  if (__builtin_va_arg_pack_len () > 1)
+    warn_open_too_many_arguments ();	/* { dg-warning "called with more than 3" } */
+
+  if (__builtin_constant_p (oflag))
+    {
+      if ((oflag & 0x40) != 0 && __builtin_va_arg_pack_len () < 1)
+	{
+	  error_open_missing_mode ();	/* { dg-error "needs 3 arguments, only 2 were given" } */
+	  return myopen2 (path, oflag);
+	}
+      return myopenva (path, oflag, __builtin_va_arg_pack ());
+    }
+
+  if (__builtin_va_arg_pack_len () < 1)
+    return myopen2 (path, oflag);
+
+  return myopenva (path, oflag, __builtin_va_arg_pack ());
+}
+
+int
+main (void)
+{
+  myopen ("h", 0x43);
+  myopen ("i", 0x43, 0644, 0655);
+  return 0;
+}
--- gcc/testsuite/g++.dg/ext/va-arg-pack-len-1.C.jj	2007-09-10 14:19:06.000000000 +0200
+++ gcc/testsuite/g++.dg/ext/va-arg-pack-len-1.C	2007-09-10 14:21:47.000000000 +0200
@@ -3,8 +3,10 @@
 
 #include <stdarg.h>
 
-extern "C" int warn_open_missing_mode (void);
-extern "C" int warn_open_too_many_arguments (void);
+extern "C" int error_open_missing_mode (void)
+  __attribute__((__error_decl__ ("open with O_CREAT needs 3 arguments, only 2 were given")));
+extern "C" int warn_open_too_many_arguments (void)
+  __attribute__((__warning_decl__ ("open called with more than 3 arguments")));
 extern "C" void abort (void);
 
 char expected_char;
@@ -83,7 +85,7 @@ myopen (const char *path, int oflag, ...
     {
       if ((oflag & 0x40) != 0 && __builtin_va_arg_pack_len () < 1)
 	{
-	  warn_open_missing_mode ();
+	  error_open_missing_mode ();
 	  return myopen2 (path, oflag);
 	}
       return myopenva (path, oflag, __builtin_va_arg_pack ());
--- gcc/testsuite/g++.dg/ext/va-arg-pack-len-2.C.jj	2007-09-10 14:19:17.000000000 +0200
+++ gcc/testsuite/g++.dg/ext/va-arg-pack-len-2.C	2007-09-10 14:19:44.000000000 +0200
@@ -0,0 +1,42 @@
+// { dg-do compile }
+// { dg-options "-O2" }
+
+#include <stdarg.h>
+
+extern int error_open_missing_mode (void)
+  __attribute__((__error_decl__ ("open with O_CREAT needs 3 arguments, only 2 were given")));
+extern int warn_open_too_many_arguments (void)
+  __attribute__((__warning_decl__ ("open called with more than 3 arguments")));
+
+extern int myopen2 (const char *path, int oflag);
+extern int myopenva (const char *path, int oflag, ...);
+
+extern inline __attribute__((always_inline, gnu_inline)) int
+myopen (const char *path, int oflag, ...)
+{
+  if (__builtin_va_arg_pack_len () > 1)
+    warn_open_too_many_arguments ();	// { dg-warning "called with more than 3" }
+
+  if (__builtin_constant_p (oflag))
+    {
+      if ((oflag & 0x40) != 0 && __builtin_va_arg_pack_len () < 1)
+	{
+	  error_open_missing_mode ();	// { dg-error "needs 3 arguments, only 2 were given" }
+	  return myopen2 (path, oflag);
+	}
+      return myopenva (path, oflag, __builtin_va_arg_pack ());
+    }
+
+  if (__builtin_va_arg_pack_len () < 1)
+    return myopen2 (path, oflag);
+
+  return myopenva (path, oflag, __builtin_va_arg_pack ());
+}
+
+int
+main (void)
+{
+  myopen ("h", 0x43);
+  myopen ("i", 0x43, 0644, 0655);
+  return 0;
+}
--- gcc/doc/extend.texi.jj	2007-09-08 13:20:20.000000000 +0200
+++ gcc/doc/extend.texi	2007-09-10 14:39:26.000000000 +0200
@@ -1802,8 +1802,8 @@ attributes are currently defined for fun
 @code{no_instrument_function}, @code{section}, @code{constructor},
 @code{destructor}, @code{used}, @code{unused}, @code{deprecated},
 @code{weak}, @code{malloc}, @code{alias}, @code{warn_unused_result},
-@code{nonnull}, @code{gnu_inline} and @code{externally_visible},
-@code{hot}, @code{cold}.
+@code{nonnull}, @code{gnu_inline}, @code{externally_visible},
+@code{hot}, @code{cold}, @code{error_decl} and @code{warning_decl}.
 Several other attributes are defined for functions on particular
 target systems.  Other attributes, including @code{section} are
 supported for variables declarations (@pxref{Variable Attributes}) and
@@ -1933,6 +1933,30 @@ Whether the function itself is considere
 the current inlining parameters.  The @code{flatten} attribute only works
 reliably in unit-at-a-time mode.
 
+@item error_decl ("@var{message}")
+@cindex @code{error_decl} function attribute
+If this attribute is used on a function declaration and a call to such a function
+is not eliminated through dead code elimination or other optimizations, an error
+which will include @var{message} will be diagnosed.  This is useful
+for compile time checking, especially together with @code{__builtin_constant_p}
+and inline functions where checking the inline function arguments is not
+possible through @code{extern char [(condition) ? 1 : -1];} tricks.
+While it is possible to leave the function undefined and thus invoke
+a link failure, when using this attribute the problem will be diagnosed
+earlier and with exact location of the call even in presence of inline
+functions or when not emitting debugging information.
+
+@item warning_decl ("@var{message}")
+@cindex @code{warning_decl} function attribute
+If this attribute is used on a function declaration and a call to such a function
+is not eliminated through dead code elimination or other optimizations, a warning
+which will include @var{message} will be diagnosed.  This is useful
+for compile time checking, especially together with @code{__builtin_constant_p}
+and inline functions.  While it is possible to define the function with
+a message in @code{.gnu.warning*} section, when using this attribute the problem
+will be diagnosed earlier and with exact location of the call even in presence
+of inline functions or when not emitting debugging information.
+
 @item cdecl
 @cindex functions that do pop the argument stack on the 386
 @opindex mrtd


	Jakub



More information about the Gcc-patches mailing list