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

[Martin Uecker <muecker@gwdg.de>]

Am Freitag, dem 05.07.2024 um 16:37 +0200 schrieb Alejandro Colomar via Gcc:
> [CC += linux-man@, since we're discussing an API documented there, and
>  the manual page would also need to be updated]
> 
> Hi Xi,  Jakub,
> 
> On Fri, Jul 05, 2024 at 09:38:21PM GMT, Xi Ruoyao wrote:
> > On Fri, 2024-07-05 at 15:03 +0200, Alejandro Colomar wrote:
> > > ISO C specifies these APIs as accepting a restricted pointer in their
> > > first parameter:
> > > 
> > > $ stdc c99 strtol
> > > long int strtol(const char *restrict nptr, char **restrict endptr, int base);
> > > $ stdc c11 strtol
> > > long int strtol(const char *restrict nptr, char **restrict endptr, int base);
> > > 
> > > However, it should be considered a defect in ISO C.  It's common to see
> > > code that aliases it:
> > > 
> > > 	char str[] = "10 20";
> > > 
> > > 	p = str;
> > > 	a = strtol(p, &p, 0);  // Let's ignore error handling for
> > > 	b = strtol(p, &p, 0);  // simplicity.
> > 
> > Why this is wrong?
> > 
> > During the execution of strtol() the only expression accessing the
> > object "p" is *endptr.  When the body of strtol() refers "nptr" it
> > accesses a different object, not "p".
> 
> <http://port70.net/~nsz/c/c11/n1570.html#6.7.3p8>
> 
> Theoretically, 'restrict' is defined in terms of accesses, not just
> references, so it's fine for strtol(3) to hold two references of p in
> restrict pointers.  That is, the following code is valid:
> 
> 	int
> 	dumb(int *restrict a, int *restrict also_a)
> 	{
> 		// We don't access the objects
> 		return a == also_a;
> 	}
> 
> 	int
> 	main(void)
> 	{
> 		int x = 3;
> 
> 		return dumb(&x, &x);
> 	}
> 
> However, in practice that's dumb.  The caller cannot know that the
> function doesn't access the object, so it must be cautious and enable
> -Wrestrict, which should be paranoid and do not allow passing references
> to the same object in different arguments, just in case the function
> decides to access to objects.  Of course, GCC reports a diagnostic for
> the previous code:
> 
> 	$ cc -Wall -Wextra dumb.c 
> 	dumb.c: In function ‘main’:
> 	dumb.c:13:21: warning: passing argument 1 to ‘restrict’-qualified parameter aliases with argument 2 [-Wrestrict]
> 	   13 |         return dumb(&x, &x);
> 	      |                     ^~  ~~
> 
> ... even when there's no UB, since the object is not being accessed.
> 
> But when the thing gets non-trivial, as in strtol(3), GCC misses the
> -Wrestrict diagnostic, as reported in
> <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112833>.
> 
> Let's write a reproducer by altering the dumb.c program from above, with
> just another reference:
> 
> 	int
> 	dumb2(int *restrict a, int *restrict *restrict ap)
> 	{
> 		// We don't access the objects
> 		return a == *ap;
> 	}
> 
> 	int
> 	main(void)
> 	{
> 		int x = 3;
> 		int *xp = &x;
> 
> 		return dumb2(&x, &xp);
> 	}
> 
> GCC doesn't report anything bad here, even though it's basically the
> same as the program from above:
> 
> 	$ cc -Wall -Wextra dumb2.c
> 	$

strtol does have  a "char * restrict * restrict" though, so the
situation is different.   A "char **" and a "const char *"
shouldn't alias anyway. 


> 
> Again, there's no UB, but we really want to be cautious and get a
> diagnostic as callers, just in case the callee decides to access the
> object; we never know.
> 
> So, GCC should be patched to report a warning in the program above.
> That will also cause strtol(3) to start issuing warnings in use cases
> like the one I showed.
> 
> Even further, let's try something really weird: inequality comparison,
> which is only defined for pointers to the same array object:
> 
> 	int
> 	dumb3(int *restrict a, int *restrict *restrict ap)
> 	{
> 		// We don't access the objects
> 		return a > *ap;
> 	}
> 
> 	int
> 	main(void)
> 	{
> 		int x = 3;
> 		int *xp = &x;
> 
> 		return dumb3(&x, &xp);
> 	}
> 
> The behavior is still defined, since the obnjects are not accessed, but
> the compiler should really warn, on both sides:
> 
> -  The caller is passing references to the same object in restricted
>    parameters, which is a red flag.
> 
> -  The callee is comparing for inequality pointers that should, under
>    normal circumstances, cause Undefined Behavior.
> 
> 
> > And if this is really wrong you should report it to WG14 before changing
> > glibc.
> 
> Well, I don't know how to report that defect to WG14.  If you help me,
> I'll be pleased to do so.  Do they have a public mailing list or
> anything like that?

One can submit clarification or change requests:

https://www.open-std.org/jtc1/sc22/wg14/www/contributing.html

Martin