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

[Jeff Law]

On 12/07/2016 08:24 PM, Martin Sebor wrote:

>
> You're right!  Good chatch! I missed that there are two ways to
> represent the same thing.  For example, these two declarations
>
>   void __attribute ((nonnull (1, 2)))
>   f (void);
>
>   void __attribute ((nonnull (1))) __attribute ((nonnull (2)))
>   f (void);
>
> apply to the same arguments but each is represented differently,
> as is this one:
>
>   void __attribute ((nonnull))
>   f (void);
>
> The builtins use the first form and I didn't have tests for user-
> defined attributes in the second form.  I've fixed that in the
> attached updated patch (and added more tests).  It does seem,
> though, that it would be better to represent these declarations
> canonically.  It would simplify the processing and avoid bugs.
I'm not sure what the historical context is for having two forms of the 
same thing.  Though I would generally agree that there should be a 
canonical form.

One more high level note.  While I am in favor of adding these 
attributes, they can cause interesting codegen issues that we need to be 
on the lookout for.

Specifically when we see an SSA_NAME as an argument when the argument is 
marked as NONNULL, the optimizers will try to use that information to 
eliminate unnecessary NULL pointer checks.

The canonical example has been

memcpy (src, dst, 0)
[ stuff ]
if (src == 0)
   whatever

GCC can sometimes follow things well enough to realize that the test, in 
a conforming program, will always be false and remove the test. Some 
have claimed this is undesirable behavior on GCC's part (and they may be 
right).

Anyway, something to keep in mind.  These changes may make GCC ever so 
slightly more aggressive in its NULL pointer eliminations.


>
> Thanks
> 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.
> 	* gcc.dg/nonnull-4.c: New test.
OK.
jeff