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
