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]

Re: [PATCH] add missing attribute nonnull to stdio functions (PR 78673 and 17308)


On 12/05/2016 03:56 PM, Martin Sebor wrote:
The new -Wformat-length warning pass detects and diagnoses non-
constant null format strings.  This is in addition to (but not
in conflict with) -Wformat which is limited to detecting only
null constants.

A recently posted patch of mine also adds the detection of null
pointer arguments to the %s directive, and null destination
pointers:

  https://gcc.gnu.org/ml/gcc-patches/2016-12/msg00304.html

After I put that patch together and tested it I realized that
detecting null pointers is the business of the -Wnonnull warning
and would be better handled more generally for all functions
decorated with attribute nonnull.  I set out to look for a better
place to issue the non-constant null pointer format string (and
other sprintf-related) warnings.  In the process I discovered that
some of the sprintf functions (quite a few in fact) are missing
this attribute on some of their arguments.

The attached patch adds the nonnull attribute missing from some
of the stdio functions.  To achieve parity with -Wformat-length,
and to also resolve the ancient request in bug 17308 for more
useful nonnull warnings, the patch also adds the attribute nonnull
handling to the middle end so that all known null pointers can be
detected and diagnosed, not just constants.

To avoid duplicating warnings issued by the front and middle ends
I made the warning conditional on optimization in both (the front
end issues its diagnostics when not optimizing, the middle end
when optimizing).  I did this so as not to suppress warnings that
GCC issues only without optimization (e.g., for memcpy(0, 0, 0)).
Since these calls are eliminated I don't think thee warnings for
them are terribly important so if there's a preference for issuing
the warning just in the middle end I'm open to removing the front
end code.

Martin

gcc-78673.diff


PR c/78673 - sprintf missing attribute nonnull on destination argument
PR c/17308 - nonnull attribute not as useful as it could be

gcc/ChangeLog:

	PR c/78673
	PR c/17308
	* builtin-attrs.def (ATTR_NONNULL_1_1, ATTR_NONNULL_1_2): Defined.
	(ATTR_NONNULL_1_3, ATTR_NONNULL_1_4, ATTR_NONNULL_1_5): Same.
	(ATTR_NOTHROW_NONNULL_1_1, ATTR_NOTHROW_NONNULL_1_2): Same.
	(ATTR_NOTHROW_NONNULL_1_3, ATTR_NOTHROW_NONNULL_1_4): Same.
	(ATTR_NOTHROW_NONNULL_1_5): Same.
	(ATTR_NONNULL_1_FORMAT_PRINTF_1_2): Same.
	(ATTR_NONNULL_1_FORMAT_PRINTF_2_0): Same.
	(ATTR_NONNULL_1_FORMAT_PRINTF_2_3): Same.
	(ATTR_NONNULL_1_FORMAT_PRINTF_3_0): Same.
	(ATTR_NONNULL_1_FORMAT_PRINTF_3_4): Same.
	(ATTR_NONNULL_1_FORMAT_PRINTF_4_0): Same.
	(ATTR_NONNULL_1_FORMAT_PRINTF_4_5): Same.
	* builtins.c (validate_arg): Add argument.  Treat null pointers
	passed to nonnull arguments as invalid.
	(validate_arglist): Same.
	* builtins.def (fprintf, fprintf_unlocked): Add nonnull attribute.
	(printf, printf_unlocked, sprintf. vfprintf, vsprintf): Same.
	(__sprintf_chk, __vsprintf_chk, __fprintf_chk, __vfprintf_chk): Same.
	* calls.c (get_nonnull_ags, maybe_warn_null_arg): New functions.
	(initialize_argument_information): Diagnose null pointers passed to
	arguments declared nonnull.
	* calls.h (get_nonnull_args): Declared.

gcc/c-family/ChangeLog:

	PR c/78673
	PR c/17308
	* c-common.c (check_nonnull_arg): Disable when optimization
	is enabled.

gcc/testsuite/ChangeLog:

	PR c/78673
	PR c/17308
	* gcc.dg/builtins-nonnull.c: New test.


 /* Profiling hooks.  */
diff --git a/gcc/calls.c b/gcc/calls.c
index 21385ce..235dec0 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -1194,6 +1194,76 @@ maybe_complain_about_tail_call (tree call_expr, const char *reason)
   error_at (EXPR_LOCATION (call_expr), "cannot tail-call: %s", reason);
 }

+/* Return a bitmap with a bit set corresponding to each argument in
+   a function call expression CALLEXPR declared with attribute nonnull,
+   or null if none of the function's argument are nonnull.  */
Please note that the caller is responsible for releasing the bitmap in this comment.


+
+bitmap
+get_nonnull_args (const_tree callexpr)
+{
+  tree fn = CALL_EXPR_FN (callexpr);
+  if (!fn || TREE_CODE (fn) != ADDR_EXPR)
+    return NULL;
+
+  tree fndecl = TREE_OPERAND (fn, 0);
+  tree fntype = TREE_TYPE (fndecl);
+  tree attrs = TYPE_ATTRIBUTES (fntype);
+  if (!attrs)
+    return NULL;
+
+  attrs = lookup_attribute ("nonnull", attrs);
+  if (!attrs)
+    return NULL;
+
+  /* Return an empty but non-null bitmap as an indication that all
+     of the function's arguments must be non-null.  */
+  bitmap argmap = BITMAP_ALLOC (NULL);
+  if (!TREE_VALUE (attrs))
+    return argmap;
+
+  /* Iterate over the indices of the format arguments declared nonnull
+     and set a bit for each.  */
+  for (tree idx = TREE_VALUE (attrs); idx; idx = TREE_CHAIN (idx))
+    {
+      unsigned int val = TREE_INT_CST_LOW (TREE_VALUE (idx)) - 1;
+      bitmap_set_bit (argmap, val);
+    }
+
+  return argmap;
+}
I'm not entirely sure you're using lookup_attribute properly here. The docs for that function make me believe you need to call back into lookup_attribute with the TREE_CHAIN of the previous return value to find the next occurrence.


Jeff


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