On Fri, 30 Jul 2021, Tom de Vries wrote: > On 7/30/21 6:17 PM, Martin Sebor wrote: > > On 7/28/21 9:20 AM, Tom de Vries wrote: > >> Hi, > >> > >> Improve nonnull attribute documentation in a number of ways: > >> > >> Reorganize discussion of effects into: > >> - effects for calls to functions with nonnull-marked parameters, and > >> - effects for function definitions with nonnull-marked parameters. > >> This makes it clear that -fno-delete-null-pointer-checks has no effect > >> for > >> optimizations based on nonnull-marked parameters in function definitions > >> (see PR100404). > > > > This resolves half of PR 101665 that I raised the other day (i.e., > > updates the docs). Thank you! > > You're welcome :) > > > Since PR 100404 was resolved as > > invalid, > > Yeah, I can also live with reopening that one as documentation PR. > > > can you please reference the other PR in the changelog? > > Done. > > > The other half (warning when attribute nonnull is specified along > > with attribute optimize "-fno-delete-null-pointer-checks") remains. > > I plan to look into it unless someone beats me to it or unless some > > other solution emerges. > > > > FWIW, In my reply to Richi here ( > https://gcc.gnu.org/pipermail/gcc-patches/2021-July/576415.html ) I > proposed to split the existing nonnull attribute functionality into > assume_nonnull/verify_nonnull attributes. I'm curious what you think of > that proposal. > > > A few comments on the documentation changes below. > > > >> > >> Mention -Wnonnull-compare. > >> > >> Mention workaround from PR100404 comment 7. > >> > >> The workaround can be used for this scenario. Say we have a test.c: > >> ... > >> #include <stdlib.h> > >> > >> extern int isnull (char *ptr) __attribute__ ((nonnull)); > >> int isnull (char *ptr) > >> { > >> if (ptr == 0) > >> return 1; > >> return 0; > >> } > >> > >> int > >> main (void) > >> { > >> char *ptr = NULL; > >> if (isnull (ptr)) __builtin_abort (); > >> return 0; > >> } > >> ... > >> > >> The test-case contains a mistake: ptr == NULL, and we want to detect the > >> mistake using an abort: > >> ... > >> $ gcc test.c > >> $ ./a.out > >> Aborted (core dumped) > >> ... > >> > >> At -O2 however, the mistake is not detected: > >> ... > >> $ gcc test.c -O2 > >> $ ./a.out > >> ... > >> which is what -Wnonnull-compare (not show here) warns about. > >> > >> The easiest way to fix this is by dropping the nonnull attribute. But > >> that > >> also disables -Wnonnull, which would detect something like: > >> ... > >> if (isnull (NULL)) __builtin_abort (); > >> ... > >> at compile time. > >> > >> Using this workaround: > >> ... > >> int isnull (char *ptr) > >> { > >> + asm ("" : "+r"(ptr)); > >> if (ptr == 0) > >> return 1; > >> return 0; > >> } > >> ... > >> we still manage to detect the problem at runtime with -O2: > >> ... > >> $ ~/gcc_versions/devel/install/bin/gcc test.c -O2 > >> $ ./a.out > >> Aborted (core dumped) > >> ... > >> while keeping the possibility to detect "isnull (NULL)" at compile time. > >> > >> OK for trunk? > >> > >> Thanks, > >> - Tom > >> > >> [gcc/doc] Improve nonnull attribute documentation > >> > >> gcc/ChangeLog: > >> > >> 2021-07-28 Tom de Vries <tdevr...@suse.de> > >> > >> * doc/extend.texi (nonnull attribute): Improve documentation. > >> > >> --- > >> gcc/doc/extend.texi | 51 > >> ++++++++++++++++++++++++++++++++++++++++----------- > >> 1 file changed, 40 insertions(+), 11 deletions(-) > >> > >> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi > >> index b83cd4919bb..3389effd70c 100644 > >> --- a/gcc/doc/extend.texi > >> +++ b/gcc/doc/extend.texi > >> @@ -3488,17 +3488,46 @@ my_memcpy (void *dest, const void *src, size_t > >> len) > >> @end smallexample > >> @noindent > >> -causes the compiler to check that, in calls to @code{my_memcpy}, > >> -arguments @var{dest} and @var{src} are non-null. If the compiler > >> -determines that a null pointer is passed in an argument slot marked > >> -as non-null, and the @option{-Wnonnull} option is enabled, a warning > >> -is issued. @xref{Warning Options}. Unless disabled by > >> -the @option{-fno-delete-null-pointer-checks} option the compiler may > >> -also perform optimizations based on the knowledge that certain function > >> -arguments cannot be null. In addition, > >> -the @option{-fisolate-erroneous-paths-attribute} option can be specified > >> -to have GCC transform calls with null arguments to non-null functions > >> -into traps. @xref{Optimize Options}. > >> +informs the compiler that, in calls to @code{my_memcpy}, arguments > >> +@var{dest} and @var{src} must be non-null. > >> + > >> +The attribute has effect both for functions calls and function > >> definitions. > > > > Missing article: has <ins>an </ins> effect. Also, an effect on > > (rather than for) might be more appropriate. > > > > Done. > > >> + > >> +For function calls: > >> +@itemize @bullet > >> +@item If the compiler determines that a null pointer is > >> +passed in an argument slot marked as non-null, and the > >> +@option{-Wnonnull} option is enabled, a warning is issued. > >> +@xref{Warning Options}. > >> +@item The @option{-fisolate-erroneous-paths-attribute} option can be > >> +specified to have GCC transform calls with null arguments to non-null > >> +functions into traps. @xref{Optimize Options}. > >> +@item The compiler may also perform optimizations based on the > >> +knowledge that certain function arguments cannot be null. These > >> +optimizations can be disabled by the > >> +@option{-fno-delete-null-pointer-checks} option. @xref{Optimize > >> Options}. > >> +@end itemize > >> + > >> +For function definitions: > >> +@itemize @bullet > >> +@item If the compiler determines that a function parameter that is > >> +marked with non-null > > > > Either no "with" or no hyphen in nonnull (when it names the attribute). > > > > Done. > > >> is compared with null, and > >> +@option{-Wnonnull-compare} option is enabled, a warning is issued. > >> +@xref{Warning Options}. > >> +@item The compiler may also perform optimizations based on the > >> +knowledge that certain function parameters cannot be null. > > > > "certain function parameters" makes me think it might include others > > besides those marked nonnull. Unless that's the case, to avoid any > > doubt I'd suggest to rephrase that: say "based on the knowledge that > > @code{nonnul} parameters cannot be null." > > > > Done. > > >> This can > >> +be disabled by hiding the nonnullness using an inline assembly > >> statement: > > > > This is a clever trick but it feels more like a workaround than > > a solution to recommend. I think I would find it preferable to > > accept and gracefully handle the combination of attribute nonnull > > and optimize ("-fno-delete-null-pointer-checks"). But that has > > the downside of disabling the removal of all such checks, not > > just those of the argument, and so doesn't seem like an optimal > > solution either. So it seems like some other mechanism might be > > needed here. > > Yeah, I hope the assume_nonnull / verify_nonnull is an acceptable way > forward. > > Anyway, dropped the workaround from this version.
This version is OK. Thanks, Richard.
