[PATCH][gcc/doc] Improve nonnull attribute documentation

[PATCH][gcc/doc] Improve nonnull attribute documentation

[Tom de Vries]

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).

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  <tdevries@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.
+
+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 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.  This can
+be disabled by hiding the nonnullness using an inline assembly statement:
+
+@smallexample
+extern int isnull (char *ptr) __attribute__((nonnull));
+int isnull (char *ptr) @{
+  asm ("" : "+r"(ptr));
+  if (ptr == 0)
+    return 1;
+  return 0;
+@}
+@end smallexample
+@end itemize
 
 If no @var{arg-index} is given to the @code{nonnull} attribute,
 all pointer arguments are marked as non-null.  To illustrate, the

Re: [PATCH][gcc/doc] Improve nonnull attribute documentation

[Richard Biener]

On Wed, 28 Jul 2021, 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).
> 
> 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?

I think it's an improvement over the current situation but the
inline-assembler suggestion to "fix" definition side optimizations
are IMHO a hint at that we need a better solution here.  Splitting
the attribute into a caller and a calle side one for example,
or making -fno-delete-null-pointer-checks do what it suggests.

And as suggested elsewhere the effect of -fno-delete-null-pointer-checks
making objects at NULL address valid should be a target hook based
on the address-space with the default implementation considering
only the default address-space having no objects at NULL.

Richard.

> Thanks,
> - Tom
> 
> [gcc/doc] Improve nonnull attribute documentation
> 
> gcc/ChangeLog:
> 
> 2021-07-28  Tom de Vries  <tdevries@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.
> +
> +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 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.  This can
> +be disabled by hiding the nonnullness using an inline assembly statement:
> +
> +@smallexample
> +extern int isnull (char *ptr) __attribute__((nonnull));
> +int isnull (char *ptr) @{
> +  asm ("" : "+r"(ptr));
> +  if (ptr == 0)
> +    return 1;
> +  return 0;
> +@}
> +@end smallexample
> +@end itemize
>  
>  If no @var{arg-index} is given to the @code{nonnull} attribute,
>  all pointer arguments are marked as non-null.  To illustrate, the
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Re: [PATCH][gcc/doc] Improve nonnull attribute documentation

[Tom de Vries]

On 7/30/21 9:25 AM, Richard Biener wrote:
> On Wed, 28 Jul 2021, 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).
>>
>> 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?
> 
> I think it's an improvement over the current situation but the
> inline-assembler suggestion to "fix" definition side optimizations
> are IMHO a hint at that we need a better solution here.

Agreed.

[ If you want I can resubmit without that bit, if it's not acceptable. I
merely included it because it's the only solution I found advertised (in
bugzilla). ]

> Splitting
> the attribute into a caller and a calle side one for example,
> or making -fno-delete-null-pointer-checks do what it suggests.
> 

I think the problem is that in fact there are two attributes:
- assume_nonnull
- verify_nonnull (or, want_nonnull or some such)
which got conflated in the nonnull attribute.  [ Which still could be
fine if you got an -fnonnull=assume/verify to switch between the two
interpretations. ]

Anyway, so more concretely my idea is that this should generate a -Wnonnull:
...
extern void foo (void *ptr) __attribute__((verify_nonnull (1)));
void bar (void) {
  foo (nullptr);
}
...
and the same with assume_nonnull (after all, the assumption is violated).

And this assert could be optimized away (with -Wnonnull-compare):
...
extern void foo (void *ptr) __attribute__((assume_nonnull (1)));
void foo (void *ptr) {
  assert (ptr != nullptr);
}
...
while the assert shouldn't be optimized away with verify_nonnull.

And likewise, this if could be optimized away by
fdelete-null-pointer-checks:
...	
extern void foo (void *ptr) __attribute__((assume_nonnull (1)));
void bar (void *ptr) {
  foo (ptr);
  if (ptr != nullptr)
    { ... }
}
...
while the if shouldn't be optimized away with verify_nonnull.

I think this way of splitting up the functionality conforms to the
principle of least surprise and does not require thinking about caller /
callee distinction, nor does it require extension of
-fno-delete-null-pointer-checks.

Thanks,
- Tom

> And as suggested elsewhere the effect of -fno-delete-null-pointer-checks
> making objects at NULL address valid should be a target hook based
> on the address-space with the default implementation considering
> only the default address-space having no objects at NULL.
> 

Re: [PATCH][gcc/doc] Improve nonnull attribute documentation

[Martin Sebor]

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!  Since PR 100404 was resolved as
invalid, can you please reference the other PR in the changelog?
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.

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  <tdevries@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.

> +
> +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).

> 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."

> 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.

Martin

> +
> +@smallexample
> +extern int isnull (char *ptr) __attribute__((nonnull));
> +int isnull (char *ptr) @{
> +  asm ("" : "+r"(ptr));
> +  if (ptr == 0)
> +    return 1;
> +  return 0;
> +@}
> +@end smallexample
> +@end itemize
>   
>   If no @var{arg-index} is given to the @code{nonnull} attribute,
>   all pointer arguments are marked as non-null.  To illustrate, the
> 

Re: [PATCH][gcc/doc] Improve nonnull attribute documentation

[Tom de Vries]

[-- Attachment #1: Type: text/plain, Size: 6876 bytes --]

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  <tdevries@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.

Thanks,
- Tom



[-- Attachment #2: 0001-gcc-doc-Improve-nonnull-attribute-documentation.patch --]
[-- Type: text/x-patch, Size: 3183 bytes --]

[gcc/doc] Improve nonnull attribute documentation

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).

Mention -Wnonnull-compare.

gcc/ChangeLog:

2021-07-28  Tom de Vries  <tdevries@suse.de>

	PR doc/101665
	* doc/extend.texi (nonnull attribute): Improve documentation.

---
 gcc/doc/extend.texi | 42 +++++++++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index b83cd4919bb..49df8e6dc38 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -3488,17 +3488,37 @@ 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 an effect both on functions calls and function definitions.
+
+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 nonnull 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 @code{nonnul} parameters cannot be null.  This can
+currently not be disabled other than by removing the nonnull
+attribute.
+@end itemize
 
 If no @var{arg-index} is given to the @code{nonnull} attribute,
 all pointer arguments are marked as non-null.  To illustrate, the

Re: [PATCH][gcc/doc] Improve nonnull attribute documentation

[Martin Sebor]

On 7/30/21 2:21 PM, 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.

I think it may have been worth considering at the time nonnull was
being proposed but it's too late now.  Users are familiar with it
under its current name, it's embedded in lots of code, and it's
supported by GCC-compatible compilers.  Introducing a pair of new
attributes with slightly different semantics would make it difficult
to write code that's meant to be portable across these implementations
(including prior versions of GCC).  Not to mention that it wouldn't
help users of nonnull.

But since the problem is limited to function definitions, we don't
need a solution for declarations.  What's missing is an intuitive
way to tell GCC that an attribute on the declaration should not
have an effect in its definition.  A nicer name for Andrew's asm
trick.  Coming up with a generic enough name would make it usable
for arguments with other attributes with similar effect (the only
one that comes to mind at the moment is attribute access which
also triggers warnings in function bodies, although it's not yet
used for optimization; others might pop up in the future).

Martin

> 
>> 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  <tdevries@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.
> 
> Thanks,
> - Tom
> 
> 

Re: [PATCH][gcc/doc] Improve nonnull attribute documentation

[Richard Biener]

On Fri, 30 Jul 2021, Martin Sebor wrote:

> On 7/30/21 2:21 PM, 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.
> 
> I think it may have been worth considering at the time nonnull was
> being proposed but it's too late now.  Users are familiar with it
> under its current name, it's embedded in lots of code, and it's
> supported by GCC-compatible compilers.  Introducing a pair of new
> attributes with slightly different semantics would make it difficult
> to write code that's meant to be portable across these implementations
> (including prior versions of GCC).  Not to mention that it wouldn't
> help users of nonnull.

True.

> But since the problem is limited to function definitions, we don't
> need a solution for declarations.  What's missing is an intuitive
> way to tell GCC that an attribute on the declaration should not
> have an effect in its definition.  A nicer name for Andrew's asm
> trick.  Coming up with a generic enough name would make it usable
> for arguments with other attributes with similar effect (the only
> one that comes to mind at the moment is attribute access which
> also triggers warnings in function bodies, although it's not yet
> used for optimization; others might pop up in the future).

I think -fno-delete-null-pointer-checks covering exactly this case
would be OK.  I think we just want to make sure we're not losing
the warnings when functions are called with explicit NULL in that
case.

Richard.

> Martin
> 
> > 
> >> 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  <tdevries@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.
> > 
> > Thanks,
> > - Tom
> > 
> > 
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Re: [PATCH][gcc/doc] Improve nonnull attribute documentation

[Richard Biener]

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  <tdevries@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.