[PATCH v1] Remove 'restrict' from 'nptr' in strtol(3)-like functions

[Alejandro Colomar]

On Sat, Jul 06, 2024 at 12:02:06AM GMT, Alejandro Colomar wrote:
> Hi Jonathan,
> 
> On Fri, Jul 05, 2024 at 10:39:52PM GMT, Jonathan Wakely wrote:
> > On Fri, 5 Jul 2024 at 21:55, Alejandro Colomar <alx@kernel.org> wrote:
> > >
> > > On Fri, Jul 05, 2024 at 09:28:46PM GMT, Jonathan Wakely wrote:
> > > > > If we marked endptr as "write_only" (which it might already
> > > > > be) then a future warning mechanism for -Wrestrict could
> > > > > ignore the content of *endptr.
> > > >
> > > >
> > > > That seems more useful. Add semantic information instead of taking it
> > > > away. If the concern is a hypothetical future compiler warning that
> > > > would give false positives for perfectly valid uses of strtol, then
> > > > the problem is the compiler warning, not strtol. If additional
> > > > information can be added to avoid the false positives (and also
> > > > possibly optimize the code better), then that wouldn't require a
> > > > change to the standard motivated by a hypothetical compiler warning.
> > >
> > > Let me be a little bit sarcastic.
> > >
> > > If so, let's take down -Wrestrict at all, because it triggers false
> > > positives at the same rate.  How is it even in -Wall and not just
> > > -Wextra?
> > >
> > > Here's a false positive:
> > >
> > >         $ cat d.c
> > >         int is_same_pointer(const char *restrict ca, char *restrict a)
> > >         {
> > >                 return ca == a;
> > 
> > This is a strawman argument, because all your example functions have
> > been pretty useless and/or not good uses of restrict.
> > 
> > Yes, if you put restrict on functions where you don't ever access any
> > objects through the pointers, the restrict qualifiers are misleading
> 
> That's precisely the case with strtol(3): it doesn't access any objects
> through *endptr, and so that pointer need not be restrict.
> 
> Then, nptr is a read-only pointer, so is doesn't matter either if it's
> accessed or not.
> 
> Let's say we add as many attributes as possible to strtol(3):
> 
> [[gnu::access(read_only, 1)]]
> [[gnu::access(write_only, 1)]]

s/1/2/

> [[gnu::leaf]]
> [[gnu::nothrow]]
> [[gnu::null_terminated_string_arg(1)]]
>  // [[gnu::access(none, *1)]]

s/*1/*2/

> long
> alx_strtol(const char *nptr, char **_Nullable restrict endp, int base);
> 
> Let's say we could mark *endptr as a 'access(none)' pointer, since it's
> not accessed.  Let's say we do that with [[gnu::access(none, *1)]].

s/*1/*2/

> Then, do you think the information of that prototype is any different
> than a prototype with restrict on the remaining pointers?
> 
> [[gnu::access(read_only, 1)]]
> [[gnu::access(write_only, 1)]]

s/1/2/

> [[gnu::leaf]]
> [[gnu::nothrow]]
> [[gnu::null_terminated_string_arg(1)]]
>  // [[gnu::access(none, *1)]]

s/*1/*2/

> long
> alx_strtol(const char *restrict nptr,
>            char *restrict *_Nullable restrict endp, int base);
> 
> I don't think so.  Since *endptr is access(none), it certainly cannot
> access nptr, and thus the qualifier on nptr is superfluous.
> 
> And even without the hypothetical [[gnu::access(none, *1)]]:

s/*1/*2/

> 
> -  The callee doesn't care about restrict, because it doesn't access
>    any objects via *endptr, so it certainly knows that nptr can be read
>    without any concerns about optimization.
> 
> -  The caller can't know if strtol(3) accesses *endptr, or nptr, and so
>    it can't optimize.  Unless it passes an uninitialized value in
>    *endptr, which means the caller knows for sure that nptr won't be
>    written, regardless of restrict on it or not.
> 
> Please, describe what's the information you think is being added by
> having restrict on nptr, on how it would be lost if we remove it.
> 
> Cheers,
> Alex
> 
> > and so compilers might give bad warnings for your bad API.
> 
> -- 
> <https://www.alejandro-colomar.es/>



-- 
<https://www.alejandro-colomar.es/>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc/attachments/20240706/9b913ca5/attachment.sig>