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