public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* __attribute__ ((access, ...)) vs __attribute__ ((nonnull))
@ 2022-03-09 21:23 David Malcolm
  2022-03-09 21:30 ` Andrew Pinski
  0 siblings, 1 reply; 10+ messages in thread
From: David Malcolm @ 2022-03-09 21:23 UTC (permalink / raw)
  To: gcc

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?

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

(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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: __attribute__ ((access, ...)) vs __attribute__ ((nonnull))
  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
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Pinski @ 2022-03-09 21:30 UTC (permalink / raw)
  To: David Malcolm; +Cc: GCC Mailing List

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] Document that the 'access' and 'nonnull' attributes are independent
  2022-03-09 21:30 ` Andrew Pinski
@ 2022-03-09 21:57   ` David Malcolm
  2022-03-14 22:18     ` Martin Sebor
  0 siblings, 1 reply; 10+ messages in thread
From: David Malcolm @ 2022-03-09 21:57 UTC (permalink / raw)
  To: Andrew Pinski; +Cc: GCC Mailing List

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

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?

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


[-- Attachment #2: 0001-Document-that-the-access-and-nonnull-attributes-are-.patch --]
[-- Type: text/x-patch, Size: 1349 bytes --]

From 9dc75031963f01b1711a769b7afcc5c590ce07fd Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Wed, 9 Mar 2022 16:52:49 -0500
Subject: [PATCH] Document that the 'access' and 'nonnull' attributes are
 independent

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 | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 0dc752e8aad..bfa09c0f5bc 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -2652,6 +2652,13 @@ 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.
+
 @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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Document that the 'access' and 'nonnull' attributes are independent
  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       ` [PATCH v2] " David Malcolm
  0 siblings, 1 reply; 10+ messages in thread
From: Martin Sebor @ 2022-03-14 22:18 UTC (permalink / raw)
  To: David Malcolm, Andrew Pinski; +Cc: GCC Mailing List

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.

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] Document that the 'access' and 'nonnull' attributes are independent
  2022-03-14 22:18     ` Martin Sebor
@ 2022-03-23 13:01       ` David Malcolm
  2022-03-23 16:31         ` Martin Sebor
  0 siblings, 1 reply; 10+ messages in thread
From: David Malcolm @ 2022-03-23 13:01 UTC (permalink / raw)
  To: gcc-patches, gcc

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Document that the 'access' and 'nonnull' attributes are independent
  2022-03-23 13:01       ` [PATCH v2] " David Malcolm
@ 2022-03-23 16:31         ` Martin Sebor
  2022-03-23 16:52           ` Sebastian Huber
  0 siblings, 1 reply; 10+ messages in thread
From: Martin Sebor @ 2022-03-23 16:31 UTC (permalink / raw)
  To: David Malcolm, gcc-patches, gcc

On 3/23/22 07:01, David Malcolm wrote:
> 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

I'm sorry, I probably wasn't clear.  This text sounds contradictory:

   If the pointer will definitely be dereferenced ... does not imply
   that the parameter will be dereferenced...

The concern is that the constraints implied by atttributes access and
nonnull are independent of each other.  I would suggest to document
that without talking about dereferencing because that's not implied
by either of them.  E.g., something like this (feel free to tweak it
as you see fit):

   Note that the @code{access} attribute doesn't imply the same
   constraint as attribute @code{nonnull} (@pxref{Attribute nonnull}).
   The latter attribute should be used to annotate arguments that must
   never be null, regardless of the value of the size argument.

A couple of tests that show the difference are
gcc.dg/Wstringop-overflow-{23,24}.c.  GCC issues -Wnonnull for just
a subset of calls to functions with attribute access with a null
pointer (those with a nonzero size argument).  Adding attribute
nonnull would make it issue -Wnonnull in all of them.

Martin

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] Document that the 'access' and 'nonnull' attributes are independent
  2022-03-23 16:31         ` Martin Sebor
@ 2022-03-23 16:52           ` Sebastian Huber
  2022-03-25 18:45             ` [PATCH v3] " David Malcolm
  0 siblings, 1 reply; 10+ messages in thread
From: Sebastian Huber @ 2022-03-23 16:52 UTC (permalink / raw)
  To: Martin Sebor, David Malcolm, gcc-patches, gcc

On 23/03/2022 17:31, Martin Sebor via Gcc-patches wrote:
> 
> The concern is that the constraints implied by atttributes access and
> nonnull are independent of each other.  I would suggest to document
> that without talking about dereferencing because that's not implied
> by either of them.  E.g., something like this (feel free to tweak it
> as you see fit):
> 
>    Note that the @code{access} attribute doesn't imply the same
>    constraint as attribute @code{nonnull} (@pxref{Attribute nonnull}).
>    The latter attribute should be used to annotate arguments that must
>    never be null, regardless of the value of the size argument.

I would not give an advice on using the nonnull attribute here. This 
attribute could have pretty dangerous effects in the function definition 
(removal of null pointer checks).

-- 
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3] Document that the 'access' and 'nonnull' attributes are independent
  2022-03-23 16:52           ` Sebastian Huber
@ 2022-03-25 18:45             ` David Malcolm
  2022-03-25 20:38               ` Martin Sebor
  0 siblings, 1 reply; 10+ messages in thread
From: David Malcolm @ 2022-03-25 18:45 UTC (permalink / raw)
  To: Sebastian Huber, Martin Sebor, gcc-patches, gcc

On Wed, 2022-03-23 at 17:52 +0100, Sebastian Huber wrote:
> On 23/03/2022 17:31, Martin Sebor via Gcc-patches wrote:
> > 
> > The concern is that the constraints implied by atttributes access
> > and
> > nonnull are independent of each other.  I would suggest to document
> > that without talking about dereferencing because that's not implied
> > by either of them.  E.g., something like this (feel free to tweak
> > it
> > as you see fit):
> > 
> >    Note that the @code{access} attribute doesn't imply the same
> >    constraint as attribute @code{nonnull} (@pxref{Attribute
> > nonnull}).
> >    The latter attribute should be used to annotate arguments that
> > must
> >    never be null, regardless of the value of the size argument.
> 
> I would not give an advice on using the nonnull attribute here. This 
> attribute could have pretty dangerous effects in the function
> definition 
> (removal of null pointer checks).
> 

That's a fair point.

Here's a v3 of the patch, which tones down the advice, and mentions that
there are caveats when directing the reader to the "nonnull" attribute.

How does this look?

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 | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index a4a25e86928..539dad7001d 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -2652,6 +2652,14 @@ 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.  Also, the @code{access} attribute does not
+imply the attribute @code{nonnull}; it may be appropriate to add both attributes
+at the declaration of a function that unconditionally manipulates a buffer via
+a pointer argument.  See the @code{nonnull} attribute for more information and
+caveats.
+
 @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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] Document that the 'access' and 'nonnull' attributes are independent
  2022-03-25 18:45             ` [PATCH v3] " David Malcolm
@ 2022-03-25 20:38               ` Martin Sebor
  2022-04-05 20:46                 ` David Malcolm
  0 siblings, 1 reply; 10+ messages in thread
From: Martin Sebor @ 2022-03-25 20:38 UTC (permalink / raw)
  To: David Malcolm, Sebastian Huber, gcc-patches, gcc

On 3/25/22 12:45, David Malcolm wrote:
> On Wed, 2022-03-23 at 17:52 +0100, Sebastian Huber wrote:
>> On 23/03/2022 17:31, Martin Sebor via Gcc-patches wrote:
>>>
>>> The concern is that the constraints implied by atttributes access
>>> and
>>> nonnull are independent of each other.  I would suggest to document
>>> that without talking about dereferencing because that's not implied
>>> by either of them.  E.g., something like this (feel free to tweak
>>> it
>>> as you see fit):
>>>
>>>     Note that the @code{access} attribute doesn't imply the same
>>>     constraint as attribute @code{nonnull} (@pxref{Attribute
>>> nonnull}).
>>>     The latter attribute should be used to annotate arguments that
>>> must
>>>     never be null, regardless of the value of the size argument.
>>
>> I would not give an advice on using the nonnull attribute here. This
>> attribute could have pretty dangerous effects in the function
>> definition
>> (removal of null pointer checks).
>>
> 
> That's a fair point.
> 
> Here's a v3 of the patch, which tones down the advice, and mentions that
> there are caveats when directing the reader to the "nonnull" attribute.
> 
> How does this look?

This version looks good to me.

Thanks
Martin

> 
> 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 | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> index a4a25e86928..539dad7001d 100644
> --- a/gcc/doc/extend.texi
> +++ b/gcc/doc/extend.texi
> @@ -2652,6 +2652,14 @@ 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.  Also, the @code{access} attribute does not
> +imply the attribute @code{nonnull}; it may be appropriate to add both attributes
> +at the declaration of a function that unconditionally manipulates a buffer via
> +a pointer argument.  See the @code{nonnull} attribute for more information and
> +caveats.
> +
>   @item alias ("@var{target}")
>   @cindex @code{alias} function attribute
>   The @code{alias} attribute causes the declaration to be emitted as an alias


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] Document that the 'access' and 'nonnull' attributes are independent
  2022-03-25 20:38               ` Martin Sebor
@ 2022-04-05 20:46                 ` David Malcolm
  0 siblings, 0 replies; 10+ messages in thread
From: David Malcolm @ 2022-04-05 20:46 UTC (permalink / raw)
  To: Martin Sebor, Sebastian Huber, gcc-patches, gcc

On Fri, 2022-03-25 at 14:38 -0600, Martin Sebor wrote:
> On 3/25/22 12:45, David Malcolm wrote:
> > On Wed, 2022-03-23 at 17:52 +0100, Sebastian Huber wrote:
> > > On 23/03/2022 17:31, Martin Sebor via Gcc-patches wrote:
> > > > 
> > > > The concern is that the constraints implied by atttributes
> > > > access
> > > > and
> > > > nonnull are independent of each other.  I would suggest to
> > > > document
> > > > that without talking about dereferencing because that's not
> > > > implied
> > > > by either of them.  E.g., something like this (feel free to
> > > > tweak
> > > > it
> > > > as you see fit):
> > > > 
> > > >     Note that the @code{access} attribute doesn't imply the
> > > > same
> > > >     constraint as attribute @code{nonnull} (@pxref{Attribute
> > > > nonnull}).
> > > >     The latter attribute should be used to annotate arguments
> > > > that
> > > > must
> > > >     never be null, regardless of the value of the size
> > > > argument.
> > > 
> > > I would not give an advice on using the nonnull attribute here.
> > > This
> > > attribute could have pretty dangerous effects in the function
> > > definition
> > > (removal of null pointer checks).
> > > 
> > 
> > That's a fair point.
> > 
> > Here's a v3 of the patch, which tones down the advice, and mentions
> > that
> > there are caveats when directing the reader to the "nonnull"
> > attribute.
> > 
> > How does this look?
> 
> This version looks good to me.

Thanks.

I tested this, and have gone ahead and pushed this to trunk
(as r12-8007-g0b5723d74f3a73).

Dave


> 
> Thanks
> Martin
> 
> > 
> > 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 | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> > 
> > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> > index a4a25e86928..539dad7001d 100644
> > --- a/gcc/doc/extend.texi
> > +++ b/gcc/doc/extend.texi
> > @@ -2652,6 +2652,14 @@ 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.  Also, the @code{access} attribute
> > does not
> > +imply the attribute @code{nonnull}; it may be appropriate to add
> > both attributes
> > +at the declaration of a function that unconditionally manipulates
> > a buffer via
> > +a pointer argument.  See the @code{nonnull} attribute for more
> > information and
> > +caveats.
> > +
> >   @item alias ("@var{target}")
> >   @cindex @code{alias} function attribute
> >   The @code{alias} attribute causes the declaration to be emitted
> > as an alias
> 



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-04-05 20:46 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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       ` [PATCH v2] " David Malcolm
2022-03-23 16:31         ` 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

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