From: David Malcolm <dmalcolm@redhat.com>
To: gcc-patches@gcc.gnu.org, gcc@gcc.gnu.org
Subject: [PATCH v2] Document that the 'access' and 'nonnull' attributes are independent
Date: Wed, 23 Mar 2022 09:01:17 -0400 [thread overview]
Message-ID: <20220323130117.525495-1-dmalcolm@redhat.com> (raw)
In-Reply-To: <bfcd0ef2-c2bb-fc9a-5e06-487e85a8db6c@gmail.com>
On Mon, 2022-03-14 at 16:18 -0600, Martin Sebor wrote:
> On 3/9/22 14:57, David Malcolm via Gcc wrote:
> > On Wed, 2022-03-09 at 13:30 -0800, Andrew Pinski wrote:
> > > On Wed, Mar 9, 2022 at 1:25 PM David Malcolm via Gcc
> > > <gcc@gcc.gnu.org> wrote:
> > > >
> > > > We gained __attribute__ ((access, ...)) in GCC 10:
> > > >
> > > > https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html
> > > > which identifies one of the pointer/reference arguments of a
> > > > function
> > > > as being accessed according to an access-mode: read_only,
> > > > read_write,
> > > > write_only, or none.
> > > >
> > > > We also have __attribute__ ((nonnull)) to indicate that a
> > > > function
> > > > argument (or all of them) must be non-NULL.
> > > >
> > > > There doesn't seem to be a relationship between these in the
> > > > implementation, but it strikes me that almost anywhere that a
> > > > user
> > > > might use the "access" attribute, that parameter is probably
> > > > going
> > > > to
> > > > be required to be nonnull - though perhaps there are cases
> > > > where
> > > > APIs
> > > > check for NULL and reject them gracefully?
> > >
> > > No, I think they are separate. The access just says these access
> > > attributes are read only, write only, read-write or don't access
> > > what
> > > the pointer points to; it does not say they have to be read or
> > > written
> > > to.
> > > I think it is a bad idea to connect the two ideas because you
> > > could
> > > have some cases where an argument is optional but is only read
> > > from;
> > > or is only written to (there are many in GCC sources even).
> >
> > Thanks for the clarification...
> >
> > >
> > > Thanks,
> > > Andrew Pinski
> > >
> > > >
> > > > Might we want to somehow make __attribute__ ((access, ...))
> > > > imply
> > > > __attribute__ ((nonnull))? (for non "none" access modes,
> > > > perhaps?)
> > > >
> > > > If so, one place to implement this might be in tree.cc's
> > > > get_nonnull_args, and have it add to the bitmap any arguments
> > > > that
> > > > have an appropriate access attribute.
> > > >
> > > > get_nonnull_args is used in various places:
> > > > - validating builtins
> > > > - in ranger_cache::block_apply_nonnull
> > > > - by -Wnonnull (in pass_post_ipa_warn::execute)
> > > > - by -Wanalyzer-possible-null-argument and -Wanalyzer-null-
> > > > argument;
> > > > I'm tracking the failure of these last two to make use of
> > > > __attribute__
> > > > ((access)) in PR analyzer/104860.
> > > >
> > > > So do we:
> > > >
> > > > (a) leave it up to the user, requiring them to specify
> > > > __attribute__
> > > > ((nonnull)) in addition to __attribute__ ((access, ...))
> >
> > ...so that's (a) then.
> >
> > I think it might be more user-friendly to be explicit about this in
> > the
> > documentation, maybe something like the attached?
>
> I agree it's worth clarifying the manual.
>
> But I don't think there's a way to annotate a function to indicate
> that it will definitely access an object (or dereference a pointer).
> Attribute access just implies that it might dereference it (unless
> the size is zero), and attribute nonnull that the pointer must not
> be null, not that it will be dereferenced (or even that it must be
> valid, although that's implied by the language and should probably
> be enforced in all contexts by some other warning).
>
> The combination of access with nonzero size and nonnull only means
> that the pointer must be nonnull and point to an object with at least
> size elements.
Here's an updated version of the patch which expresses that also.
OK for trunk?
Dave
>
> Martin
>
> >
> > (not yet fully tested, but seems to build)
> >
> > Dave
> >
> >
> >
> >
> > > >
> > > > (b) leave it up to the individual sites in GCC that currently
> > > > make
> > > > use
> > > > of get_nonnull_args to add logic for handling __attribute__
> > > > ((access,
> > > > ...))
> > > >
> > > > (c) extend get_nonnull_args
> > > >
> > > > ?
> > > >
> > > > Thoughts?
> > > > Dave
> > > >
> > >
> >
>
gcc/ChangeLog:
* doc/extend.texi (Common Function Attributes): Document that
'access' does not imply 'nonnull'.
Signed-off-by: David Malcolm <dmalcolm@redhat.com>
---
gcc/doc/extend.texi | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index a4a25e86928..790014f3da5 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -2652,6 +2652,15 @@ The mode is intended to be used as a means to help validate the expected
object size, for example in functions that call @code{__builtin_object_size}.
@xref{Object Size Checking}.
+Note that the @code{access} attribute merely specifies how an object
+referenced by the pointer argument can be accessed; it does not imply that
+an access @strong{will} happen. If the pointer will definitely be
+dereferenced, you may want to also add
+@code{__attribute__((nonnull (@var{arg-index})))} to the function for the
+pointer parameter in question (though the presence of the @code{nonnull}
+attribute does not imply that the parameter will be dereferenced, merely
+that it must be non-null).
+
@item alias ("@var{target}")
@cindex @code{alias} function attribute
The @code{alias} attribute causes the declaration to be emitted as an alias
--
2.26.3
next prev parent reply other threads:[~2022-03-23 13:01 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-09 21:23 __attribute__ ((access, ...)) vs __attribute__ ((nonnull)) David Malcolm
2022-03-09 21:30 ` Andrew Pinski
2022-03-09 21:57 ` [PATCH] Document that the 'access' and 'nonnull' attributes are independent David Malcolm
2022-03-14 22:18 ` Martin Sebor
2022-03-23 13:01 ` David Malcolm [this message]
2022-03-23 16:31 ` [PATCH v2] " Martin Sebor
2022-03-23 16:52 ` Sebastian Huber
2022-03-25 18:45 ` [PATCH v3] " David Malcolm
2022-03-25 20:38 ` Martin Sebor
2022-04-05 20:46 ` David Malcolm
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220323130117.525495-1-dmalcolm@redhat.com \
--to=dmalcolm@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=gcc@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).