diff mbox series

libio: Add nonnull attribute for most FILE * arguments in stdio.h

Message ID 20230518172511.2130831-1-xry111@xry111.site
State Superseded
Headers
Series libio: Add nonnull attribute for most FILE * arguments in stdio.h |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Xi Ruoyao May 18, 2023, 5:25 p.m. UTC 
  During the review of a GCC analyzer test case, we found most stdio
functions accepting a FILE * argument expect it to be nonnull and just
segfault when the argument is NULL.  Add nonnull attribute for them.

setbuf is well defined when __stream is NULL so it's not touched.

For fputs, fgets, fread, fwrite, fprintf, vfprintf, and their unlocked
version, if __stream is empty but there is nothing to read or write,
they don't segfault and I'm not sure if the standard allows such a use
so I left them out.
---
 libio/stdio.h | 119 ++++++++++++++++++++++++++------------------------
 1 file changed, 62 insertions(+), 57 deletions(-)

Comments

Alejandro Colomar May 18, 2023, 6:06 p.m. UTC | #1 
Hi Xi!

On 5/18/23 19:25, Xi Ruoyao via Libc-alpha wrote:
> During the review of a GCC analyzer test case, we found most stdio
> functions accepting a FILE * argument expect it to be nonnull and just
> segfault when the argument is NULL.  Add nonnull attribute for them.
> 
> setbuf is well defined when __stream is NULL so it's not touched.
> 
> For fputs, fgets, fread, fwrite, fprintf, vfprintf, and their unlocked
> version, if __stream is empty but there is nothing to read or write,
> they don't segfault and I'm not sure if the standard allows such a use
> so I left them out.
> ---
>   libio/stdio.h | 119 ++++++++++++++++++++++++++------------------------
>   1 file changed, 62 insertions(+), 57 deletions(-)
> 
> diff --git a/libio/stdio.h b/libio/stdio.h
> index 4cf9f1c012..ae3d7295d4 100644
> --- a/libio/stdio.h
> +++ b/libio/stdio.h
> @@ -232,7 +232,7 @@ extern char *tempnam (const char *__dir, const char *__pfx)
>   
>      This function is a possible cancellation point and therefore not
>      marked with __THROW.  */
> -extern int fflush (FILE *__stream);
> +extern int fflush (FILE *__stream) __nonnull ((1));

flush(NULL) is well defined.  It flushes all streams that can be
flushed.  This reminds me that I should document that in the SYNOPSIS
section of the manual page; an oversight on my side.

>   
>   #ifdef __USE_MISC
>   /* Faster versions when locking is not required.
> @@ -241,7 +241,7 @@ extern int fflush (FILE *__stream);
>      cancellation point.  But due to similarity with an POSIX interface
>      or due to the implementation it is a cancellation point and
>      therefore not marked with __THROW.  */
> -extern int fflush_unlocked (FILE *__stream);
> +extern int fflush_unlocked (FILE *__stream) __nonnull ((1));

Without checking, I'll guess that fflush_unlocked(NULL) is also well
defined.

I didn't see any other similar cases, but I may have missed some; I
didn't revise them all thoroughly; please check.

>   #endif
>   
>   #ifdef __USE_GNU
> @@ -278,7 +278,7 @@ extern FILE *__REDIRECT (fopen, (const char *__restrict __filename,
>   extern FILE *__REDIRECT (freopen, (const char *__restrict __filename,
>   				   const char *__restrict __modes,
>   				   FILE *__restrict __stream), freopen64)
> -  __wur;
> +  __wur __nonnull ((3));
>   # else
>   #  define fopen fopen64
>   #  define freopen freopen64
> @@ -335,16 +335,16 @@ extern void setbuf (FILE *__restrict __stream, char *__restrict __buf) __THROW;
>      If BUF is not NULL, use N bytes of it for buffering;
>      else allocate an internal buffer N bytes long.  */
>   extern int setvbuf (FILE *__restrict __stream, char *__restrict __buf,
> -		    int __modes, size_t __n) __THROW;
> +		    int __modes, size_t __n) __THROW __nonnull ((1));
>   
>   #ifdef	__USE_MISC
>   /* If BUF is NULL, make STREAM unbuffered.
>      Else make it use SIZE bytes of BUF for buffering.  */
>   extern void setbuffer (FILE *__restrict __stream, char *__restrict __buf,
> -		       size_t __size) __THROW;
> +		       size_t __size) __THROW __nonnull ((1));
>   
>   /* Make STREAM line-buffered.  */
> -extern void setlinebuf (FILE *__stream) __THROW;
> +extern void setlinebuf (FILE *__stream) __THROW __nonnull ((1));
>   #endif
>   
>   
> @@ -418,7 +418,7 @@ extern int dprintf (int __fd, const char *__restrict __fmt, ...)
>      This function is a possible cancellation point and therefore not
>      marked with __THROW.  */
>   extern int fscanf (FILE *__restrict __stream,
> -		   const char *__restrict __format, ...) __wur;
> +		   const char *__restrict __format, ...) __wur __nonnull ((1));
>   /* Read formatted input from stdin.
>   
>      This function is a possible cancellation point and therefore not
> @@ -439,7 +439,7 @@ extern int sscanf (const char *__restrict __s,
>   #  ifdef __REDIRECT
>   extern int __REDIRECT (fscanf, (FILE *__restrict __stream,
>   				const char *__restrict __format, ...),
> -		       __isoc23_fscanf) __wur;
> +		       __isoc23_fscanf) __wur __nonnull ((1));
>   extern int __REDIRECT (scanf, (const char *__restrict __format, ...),
>   		       __isoc23_scanf) __wur;
>   extern int __REDIRECT_NTH (sscanf, (const char *__restrict __s,
> @@ -447,7 +447,7 @@ extern int __REDIRECT_NTH (sscanf, (const char *__restrict __s,
>   			   __isoc23_sscanf);
>   #  else
>   extern int __isoc23_fscanf (FILE *__restrict __stream,
> -			    const char *__restrict __format, ...) __wur;
> +			    const char *__restrict __format, ...) __wur __nonnull ((1));
>   extern int __isoc23_scanf (const char *__restrict __format, ...) __wur;
>   extern int __isoc23_sscanf (const char *__restrict __s,
>   			    const char *__restrict __format, ...) __THROW;
> @@ -459,7 +459,7 @@ extern int __isoc23_sscanf (const char *__restrict __s,
>   #  ifdef __REDIRECT
>   extern int __REDIRECT (fscanf, (FILE *__restrict __stream,
>   				const char *__restrict __format, ...),
> -		       __isoc99_fscanf) __wur;
> +		       __isoc99_fscanf) __wur __nonnull ((1));
>   extern int __REDIRECT (scanf, (const char *__restrict __format, ...),
>   		       __isoc99_scanf) __wur;
>   extern int __REDIRECT_NTH (sscanf, (const char *__restrict __s,
> @@ -467,7 +467,7 @@ extern int __REDIRECT_NTH (sscanf, (const char *__restrict __s,
>   			   __isoc99_sscanf);
>   #  else
>   extern int __isoc99_fscanf (FILE *__restrict __stream,
> -			    const char *__restrict __format, ...) __wur;
> +			    const char *__restrict __format, ...) __wur __nonnull ((1));
>   extern int __isoc99_scanf (const char *__restrict __format, ...) __wur;
>   extern int __isoc99_sscanf (const char *__restrict __s,
>   			    const char *__restrict __format, ...) __THROW;
> @@ -485,7 +485,7 @@ extern int __isoc99_sscanf (const char *__restrict __s,
>      marked with __THROW.  */
>   extern int vfscanf (FILE *__restrict __s, const char *__restrict __format,
>   		    __gnuc_va_list __arg)
> -     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur;
> +     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur __nonnull ((1));
>   
>   /* Read formatted input from stdin into argument list ARG.
>   
> @@ -508,7 +508,7 @@ extern int __REDIRECT (vfscanf,
>   		       (FILE *__restrict __s,
>   			const char *__restrict __format, __gnuc_va_list __arg),
>   		       __isoc23_vfscanf)
> -     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur;
> +     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur __nonnull ((1));
>   extern int __REDIRECT (vscanf, (const char *__restrict __format,
>   				__gnuc_va_list __arg), __isoc23_vscanf)
>        __attribute__ ((__format__ (__scanf__, 1, 0))) __wur;
> @@ -520,7 +520,7 @@ extern int __REDIRECT_NTH (vsscanf,
>   #   elif !defined __REDIRECT
>   extern int __isoc23_vfscanf (FILE *__restrict __s,
>   			     const char *__restrict __format,
> -			     __gnuc_va_list __arg) __wur;
> +			     __gnuc_va_list __arg) __wur __nonnull ((1));
>   extern int __isoc23_vscanf (const char *__restrict __format,
>   			    __gnuc_va_list __arg) __wur;
>   extern int __isoc23_vsscanf (const char *__restrict __s,
> @@ -537,7 +537,7 @@ extern int __REDIRECT (vfscanf,
>   		       (FILE *__restrict __s,
>   			const char *__restrict __format, __gnuc_va_list __arg),
>   		       __isoc99_vfscanf)
> -     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur;
> +     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur __nonnull ((1));
>   extern int __REDIRECT (vscanf, (const char *__restrict __format,
>   				__gnuc_va_list __arg), __isoc99_vscanf)
>        __attribute__ ((__format__ (__scanf__, 1, 0))) __wur;
> @@ -549,7 +549,7 @@ extern int __REDIRECT_NTH (vsscanf,
>   #   elif !defined __REDIRECT
>   extern int __isoc99_vfscanf (FILE *__restrict __s,
>   			     const char *__restrict __format,
> -			     __gnuc_va_list __arg) __wur;
> +			     __gnuc_va_list __arg) __wur __nonnull ((1));
>   extern int __isoc99_vscanf (const char *__restrict __format,
>   			    __gnuc_va_list __arg) __wur;
>   extern int __isoc99_vsscanf (const char *__restrict __s,
> @@ -568,8 +568,8 @@ extern int __isoc99_vsscanf (const char *__restrict __s,
>   
>      These functions are possible cancellation points and therefore not
>      marked with __THROW.  */
> -extern int fgetc (FILE *__stream);
> -extern int getc (FILE *__stream);
> +extern int fgetc (FILE *__stream) __nonnull ((1));
> +extern int getc (FILE *__stream) __nonnull ((1));
>   
>   /* Read a character from stdin.
>   
> @@ -582,7 +582,7 @@ extern int getchar (void);
>   
>      These functions are possible cancellation points and therefore not
>      marked with __THROW.  */
> -extern int getc_unlocked (FILE *__stream);
> +extern int getc_unlocked (FILE *__stream) __nonnull ((1));
>   extern int getchar_unlocked (void);
>   #endif /* Use POSIX.  */
>   
> @@ -593,7 +593,7 @@ extern int getchar_unlocked (void);
>      cancellation point.  But due to similarity with an POSIX interface
>      or due to the implementation it is a cancellation point and
>      therefore not marked with __THROW.  */
> -extern int fgetc_unlocked (FILE *__stream);
> +extern int fgetc_unlocked (FILE *__stream) __nonnull ((1));
>   #endif /* Use MISC.  */
>   
>   
> @@ -604,8 +604,8 @@ extern int fgetc_unlocked (FILE *__stream);
>   
>      These functions is a possible cancellation point and therefore not

Aside: here's a typo mixing plural and singular in the sentence (not
part of your commit).  I'll prepare a patch for it.

>      marked with __THROW.  */
> -extern int fputc (int __c, FILE *__stream);
> -extern int putc (int __c, FILE *__stream);
> +extern int fputc (int __c, FILE *__stream) __nonnull ((2));
> +extern int putc (int __c, FILE *__stream) __nonnull ((2));
>   
>   /* Write a character to stdout.
>   
> @@ -620,7 +620,7 @@ extern int putchar (int __c);
>      cancellation point.  But due to similarity with an POSIX interface
>      or due to the implementation it is a cancellation point and
>      therefore not marked with __THROW.  */
> -extern int fputc_unlocked (int __c, FILE *__stream);
> +extern int fputc_unlocked (int __c, FILE *__stream) __nonnull ((2));
>   #endif /* Use MISC.  */
>   
>   #ifdef __USE_POSIX199506
> @@ -628,7 +628,7 @@ extern int fputc_unlocked (int __c, FILE *__stream);
>   
>      These functions are possible cancellation points and therefore not
>      marked with __THROW.  */
> -extern int putc_unlocked (int __c, FILE *__stream);
> +extern int putc_unlocked (int __c, FILE *__stream) __nonnull ((2));
>   extern int putchar_unlocked (int __c);
>   #endif /* Use POSIX.  */
>   
> @@ -636,10 +636,10 @@ extern int putchar_unlocked (int __c);
>   #if defined __USE_MISC \
>       || (defined __USE_XOPEN && !defined __USE_XOPEN2K)
>   /* Get a word (int) from STREAM.  */
> -extern int getw (FILE *__stream);
> +extern int getw (FILE *__stream) __nonnull ((1));
>   
>   /* Write a word (int) to STREAM.  */
> -extern int putw (int __w, FILE *__stream);
> +extern int putw (int __w, FILE *__stream) __nonnull ((2));
>   #endif
>   
>   
> @@ -689,10 +689,10 @@ extern char *fgets_unlocked (char *__restrict __s, int __n,
>      therefore not marked with __THROW.  */
>   extern __ssize_t __getdelim (char **__restrict __lineptr,
>                                size_t *__restrict __n, int __delimiter,
> -                             FILE *__restrict __stream) __wur;
> +                             FILE *__restrict __stream) __wur __nonnull ((4));
>   extern __ssize_t getdelim (char **__restrict __lineptr,
>                              size_t *__restrict __n, int __delimiter,
> -                           FILE *__restrict __stream) __wur;
> +                           FILE *__restrict __stream) __wur __nonnull ((4));
>   
>   /* Like `getdelim', but reads up to a newline.
>   
> @@ -702,7 +702,7 @@ extern __ssize_t getdelim (char **__restrict __lineptr,
>      therefore not marked with __THROW.  */
>   extern __ssize_t getline (char **__restrict __lineptr,
>                             size_t *__restrict __n,
> -                          FILE *__restrict __stream) __wur;
> +                          FILE *__restrict __stream) __wur __nonnull ((3));
>   #endif
>   
>   
> @@ -723,7 +723,7 @@ extern int puts (const char *__s);
>   
>      This function is a possible cancellation point and therefore not
>      marked with __THROW.  */
> -extern int ungetc (int __c, FILE *__stream);
> +extern int ungetc (int __c, FILE *__stream) __nonnull ((2));
>   
>   
>   /* Read chunks of generic data from STREAM.
> @@ -768,17 +768,17 @@ extern size_t fwrite_unlocked (const void *__restrict __ptr, size_t __size,
>   
>      This function is a possible cancellation point and therefore not
>      marked with __THROW.  */
> -extern int fseek (FILE *__stream, long int __off, int __whence);
> +extern int fseek (FILE *__stream, long int __off, int __whence) __nonnull ((1));
>   /* Return the current position of STREAM.
>   
>      This function is a possible cancellation point and therefore not
>      marked with __THROW.  */
> -extern long int ftell (FILE *__stream) __wur;
> +extern long int ftell (FILE *__stream) __wur __nonnull ((1));
>   /* Rewind to the beginning of STREAM.
>   
>      This function is a possible cancellation point and therefore not
>      marked with __THROW.  */
> -extern void rewind (FILE *__stream);
> +extern void rewind (FILE *__stream) __nonnull ((1));
>   
>   /* The Single Unix Specification, Version 2, specifies an alternative,
>      more adequate interface for the two functions above which deal with
> @@ -791,18 +791,19 @@ extern void rewind (FILE *__stream);
>   
>      This function is a possible cancellation point and therefore not
>      marked with __THROW.  */
> -extern int fseeko (FILE *__stream, __off_t __off, int __whence);
> +extern int fseeko (FILE *__stream, __off_t __off, int __whence) __nonnull ((1));
>   /* Return the current position of STREAM.
>   
>      This function is a possible cancellation point and therefore not
>      marked with __THROW.  */
> -extern __off_t ftello (FILE *__stream) __wur;
> +extern __off_t ftello (FILE *__stream) __wur __nonnull ((1));
>   # else
>   #  ifdef __REDIRECT
>   extern int __REDIRECT (fseeko,
>   		       (FILE *__stream, __off64_t __off, int __whence),
> -		       fseeko64);
> -extern __off64_t __REDIRECT (ftello, (FILE *__stream), ftello64);
> +		       fseeko64) __nonnull ((1));
> +extern __off64_t __REDIRECT (ftello, (FILE *__stream), ftello64)
> +  __nonnull ((1));
>   #  else
>   #   define fseeko fseeko64
>   #   define ftello ftello64
> @@ -815,18 +816,20 @@ extern __off64_t __REDIRECT (ftello, (FILE *__stream), ftello64);
>   
>      This function is a possible cancellation point and therefore not
>      marked with __THROW.  */
> -extern int fgetpos (FILE *__restrict __stream, fpos_t *__restrict __pos);
> +extern int fgetpos (FILE *__restrict __stream, fpos_t *__restrict __pos)
> +  __nonnull ((1));
>   /* Set STREAM's position.
>   
>      This function is a possible cancellation point and therefore not
>      marked with __THROW.  */
> -extern int fsetpos (FILE *__stream, const fpos_t *__pos);
> +extern int fsetpos (FILE *__stream, const fpos_t *__pos) __nonnull ((1));
>   #else
>   # ifdef __REDIRECT
>   extern int __REDIRECT (fgetpos, (FILE *__restrict __stream,
> -				 fpos_t *__restrict __pos), fgetpos64);
> +				 fpos_t *__restrict __pos), fgetpos64) __nonnull ((1));
>   extern int __REDIRECT (fsetpos,
> -		       (FILE *__stream, const fpos_t *__pos), fsetpos64);
> +		       (FILE *__stream, const fpos_t *__pos), fsetpos64)
> +  __nonnull ((1));
>   # else
>   #  define fgetpos fgetpos64
>   #  define fsetpos fsetpos64
> @@ -834,24 +837,26 @@ extern int __REDIRECT (fsetpos,
>   #endif
>   
>   #ifdef __USE_LARGEFILE64
> -extern int fseeko64 (FILE *__stream, __off64_t __off, int __whence);
> -extern __off64_t ftello64 (FILE *__stream) __wur;
> -extern int fgetpos64 (FILE *__restrict __stream, fpos64_t *__restrict __pos);
> -extern int fsetpos64 (FILE *__stream, const fpos64_t *__pos);
> +extern int fseeko64 (FILE *__stream, __off64_t __off, int __whence)
> +  __nonnull ((1));
> +extern __off64_t ftello64 (FILE *__stream) __wur __nonnull ((1));
> +extern int fgetpos64 (FILE *__restrict __stream, fpos64_t *__restrict __pos)
> +  __nonnull ((1));
> +extern int fsetpos64 (FILE *__stream, const fpos64_t *__pos) __nonnull ((1));
>   #endif
>   
>   /* Clear the error and EOF indicators for STREAM.  */
> -extern void clearerr (FILE *__stream) __THROW;
> +extern void clearerr (FILE *__stream) __THROW __nonnull ((1));
>   /* Return the EOF indicator for STREAM.  */
> -extern int feof (FILE *__stream) __THROW __wur;
> +extern int feof (FILE *__stream) __THROW __wur __nonnull ((1));
>   /* Return the error indicator for STREAM.  */
> -extern int ferror (FILE *__stream) __THROW __wur;
> +extern int ferror (FILE *__stream) __THROW __wur __nonnull ((1));
>   
>   #ifdef __USE_MISC
>   /* Faster versions when locking is not required.  */
> -extern void clearerr_unlocked (FILE *__stream) __THROW;
> -extern int feof_unlocked (FILE *__stream) __THROW __wur;
> -extern int ferror_unlocked (FILE *__stream) __THROW __wur;
> +extern void clearerr_unlocked (FILE *__stream) __THROW __nonnull ((1));
> +extern int feof_unlocked (FILE *__stream) __THROW __wur __nonnull ((1));
> +extern int ferror_unlocked (FILE *__stream) __THROW __wur __nonnull ((1));
>   #endif
>   
>   
> @@ -864,12 +869,12 @@ extern void perror (const char *__s) __COLD;
>   
>   #ifdef	__USE_POSIX
>   /* Return the system file descriptor for STREAM.  */
> -extern int fileno (FILE *__stream) __THROW __wur;
> +extern int fileno (FILE *__stream) __THROW __wur __nonnull ((1));
>   #endif /* Use POSIX.  */
>   
>   #ifdef __USE_MISC
>   /* Faster version when locking is not required.  */
> -extern int fileno_unlocked (FILE *__stream) __THROW __wur;
> +extern int fileno_unlocked (FILE *__stream) __THROW __wur __nonnull ((1));
>   #endif
>   
>   
> @@ -878,7 +883,7 @@ extern int fileno_unlocked (FILE *__stream) __THROW __wur;
>   
>      This function is a possible cancellation point and therefore not
>      marked with __THROW.  */
> -extern int pclose (FILE *__stream);
> +extern int pclose (FILE *__stream) __nonnull ((1));

You didn't patch fclose(3).  Any reason?  I guess it's similarly UB to
call fclose(NULL).

Cheers,
Alex

>   
>   /* Create a new stream connected to a pipe running the given command.
>   
> @@ -922,14 +927,14 @@ extern int obstack_vprintf (struct obstack *__restrict __obstack,
>   /* These are defined in POSIX.1:1996.  */
>   
>   /* Acquire ownership of STREAM.  */
> -extern void flockfile (FILE *__stream) __THROW;
> +extern void flockfile (FILE *__stream) __THROW __nonnull ((1));
>   
>   /* Try to acquire ownership of STREAM but do not block if it is not
>      possible.  */
> -extern int ftrylockfile (FILE *__stream) __THROW __wur;
> +extern int ftrylockfile (FILE *__stream) __THROW __wur __nonnull ((1));
>   
>   /* Relinquish the ownership granted for STREAM.  */
> -extern void funlockfile (FILE *__stream) __THROW;
> +extern void funlockfile (FILE *__stream) __THROW __nonnull ((1));
>   #endif /* POSIX */
>   
>   #if defined __USE_XOPEN && !defined __USE_XOPEN2K && !defined __USE_GNU

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
Alejandro Colomar May 18, 2023, 6:29 p.m. UTC | #2 
On 5/18/23 20:06, Alex Colomar wrote:
> Hi Xi!
> 
> On 5/18/23 19:25, Xi Ruoyao via Libc-alpha wrote:
>> During the review of a GCC analyzer test case, we found most stdio
>> functions accepting a FILE * argument expect it to be nonnull and just
>> segfault when the argument is NULL.  Add nonnull attribute for them.
>>
>> setbuf is well defined when __stream is NULL so it's not touched.

Is this true?  I couldn't find anything about it in either setbuf(3),
setbuf(3POSIX), or info libc.

The buffer can be NULL, but the stream?

>>
>> For fputs, fgets, fread, fwrite, fprintf, vfprintf, and their unlocked
>> version, if __stream is empty but there is nothing to read or write,
>> they don't segfault and I'm not sure if the standard allows such a use
>> so I left them out.
>> ---
>>   libio/stdio.h | 119 ++++++++++++++++++++++++++------------------------
>>   1 file changed, 62 insertions(+), 57 deletions(-)
>>
>> diff --git a/libio/stdio.h b/libio/stdio.h
>> index 4cf9f1c012..ae3d7295d4 100644
>> --- a/libio/stdio.h
>> +++ b/libio/stdio.h
>> @@ -232,7 +232,7 @@ extern char *tempnam (const char *__dir, const 
>> char *__pfx)
>>      This function is a possible cancellation point and therefore not
>>      marked with __THROW.  */
>> -extern int fflush (FILE *__stream);
>> +extern int fflush (FILE *__stream) __nonnull ((1));
> 
> flush(NULL) is well defined.  It flushes all streams that can be
> flushed.  This reminds me that I should document that in the SYNOPSIS
> section of the manual page; an oversight on my side.
> 
>>   #ifdef __USE_MISC
>>   /* Faster versions when locking is not required.
>> @@ -241,7 +241,7 @@ extern int fflush (FILE *__stream);
>>      cancellation point.  But due to similarity with an POSIX interface
>>      or due to the implementation it is a cancellation point and
>>      therefore not marked with __THROW.  */
>> -extern int fflush_unlocked (FILE *__stream);
>> +extern int fflush_unlocked (FILE *__stream) __nonnull ((1));
> 
> Without checking, I'll guess that fflush_unlocked(NULL) is also well
> defined.
> 
> I didn't see any other similar cases, but I may have missed some; I
> didn't revise them all thoroughly; please check.
> 
>>   #endif
>>   #ifdef __USE_GNU
>> @@ -278,7 +278,7 @@ extern FILE *__REDIRECT (fopen, (const char 
>> *__restrict __filename,
>>   extern FILE *__REDIRECT (freopen, (const char *__restrict __filename,
>>                      const char *__restrict __modes,
>>                      FILE *__restrict __stream), freopen64)
>> -  __wur;
>> +  __wur __nonnull ((3));
>>   # else
>>   #  define fopen fopen64
>>   #  define freopen freopen64
>> @@ -335,16 +335,16 @@ extern void setbuf (FILE *__restrict __stream, 
>> char *__restrict __buf) __THROW;
>>      If BUF is not NULL, use N bytes of it for buffering;
>>      else allocate an internal buffer N bytes long.  */
>>   extern int setvbuf (FILE *__restrict __stream, char *__restrict __buf,
>> -            int __modes, size_t __n) __THROW;
>> +            int __modes, size_t __n) __THROW __nonnull ((1));
>>   #ifdef    __USE_MISC
>>   /* If BUF is NULL, make STREAM unbuffered.
>>      Else make it use SIZE bytes of BUF for buffering.  */
>>   extern void setbuffer (FILE *__restrict __stream, char *__restrict 
>> __buf,
>> -               size_t __size) __THROW;
>> +               size_t __size) __THROW __nonnull ((1));
>>   /* Make STREAM line-buffered.  */
>> -extern void setlinebuf (FILE *__stream) __THROW;
>> +extern void setlinebuf (FILE *__stream) __THROW __nonnull ((1));
>>   #endif
>> @@ -418,7 +418,7 @@ extern int dprintf (int __fd, const char 
>> *__restrict __fmt, ...)
>>      This function is a possible cancellation point and therefore not
>>      marked with __THROW.  */
>>   extern int fscanf (FILE *__restrict __stream,
>> -           const char *__restrict __format, ...) __wur;
>> +           const char *__restrict __format, ...) __wur __nonnull ((1));
>>   /* Read formatted input from stdin.
>>      This function is a possible cancellation point and therefore not
>> @@ -439,7 +439,7 @@ extern int sscanf (const char *__restrict __s,
>>   #  ifdef __REDIRECT
>>   extern int __REDIRECT (fscanf, (FILE *__restrict __stream,
>>                   const char *__restrict __format, ...),
>> -               __isoc23_fscanf) __wur;
>> +               __isoc23_fscanf) __wur __nonnull ((1));
>>   extern int __REDIRECT (scanf, (const char *__restrict __format, ...),
>>                  __isoc23_scanf) __wur;
>>   extern int __REDIRECT_NTH (sscanf, (const char *__restrict __s,
>> @@ -447,7 +447,7 @@ extern int __REDIRECT_NTH (sscanf, (const char 
>> *__restrict __s,
>>                  __isoc23_sscanf);
>>   #  else
>>   extern int __isoc23_fscanf (FILE *__restrict __stream,
>> -                const char *__restrict __format, ...) __wur;
>> +                const char *__restrict __format, ...) __wur __nonnull 
>> ((1));
>>   extern int __isoc23_scanf (const char *__restrict __format, ...) __wur;
>>   extern int __isoc23_sscanf (const char *__restrict __s,
>>                   const char *__restrict __format, ...) __THROW;
>> @@ -459,7 +459,7 @@ extern int __isoc23_sscanf (const char *__restrict 
>> __s,
>>   #  ifdef __REDIRECT
>>   extern int __REDIRECT (fscanf, (FILE *__restrict __stream,
>>                   const char *__restrict __format, ...),
>> -               __isoc99_fscanf) __wur;
>> +               __isoc99_fscanf) __wur __nonnull ((1));
>>   extern int __REDIRECT (scanf, (const char *__restrict __format, ...),
>>                  __isoc99_scanf) __wur;
>>   extern int __REDIRECT_NTH (sscanf, (const char *__restrict __s,
>> @@ -467,7 +467,7 @@ extern int __REDIRECT_NTH (sscanf, (const char 
>> *__restrict __s,
>>                  __isoc99_sscanf);
>>   #  else
>>   extern int __isoc99_fscanf (FILE *__restrict __stream,
>> -                const char *__restrict __format, ...) __wur;
>> +                const char *__restrict __format, ...) __wur __nonnull 
>> ((1));
>>   extern int __isoc99_scanf (const char *__restrict __format, ...) __wur;
>>   extern int __isoc99_sscanf (const char *__restrict __s,
>>                   const char *__restrict __format, ...) __THROW;
>> @@ -485,7 +485,7 @@ extern int __isoc99_sscanf (const char *__restrict 
>> __s,
>>      marked with __THROW.  */
>>   extern int vfscanf (FILE *__restrict __s, const char *__restrict 
>> __format,
>>               __gnuc_va_list __arg)
>> -     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur;
>> +     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur __nonnull 
>> ((1));
>>   /* Read formatted input from stdin into argument list ARG.
>> @@ -508,7 +508,7 @@ extern int __REDIRECT (vfscanf,
>>                  (FILE *__restrict __s,
>>               const char *__restrict __format, __gnuc_va_list __arg),
>>                  __isoc23_vfscanf)
>> -     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur;
>> +     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur __nonnull 
>> ((1));
>>   extern int __REDIRECT (vscanf, (const char *__restrict __format,
>>                   __gnuc_va_list __arg), __isoc23_vscanf)
>>        __attribute__ ((__format__ (__scanf__, 1, 0))) __wur;
>> @@ -520,7 +520,7 @@ extern int __REDIRECT_NTH (vsscanf,
>>   #   elif !defined __REDIRECT
>>   extern int __isoc23_vfscanf (FILE *__restrict __s,
>>                    const char *__restrict __format,
>> -                 __gnuc_va_list __arg) __wur;
>> +                 __gnuc_va_list __arg) __wur __nonnull ((1));
>>   extern int __isoc23_vscanf (const char *__restrict __format,
>>                   __gnuc_va_list __arg) __wur;
>>   extern int __isoc23_vsscanf (const char *__restrict __s,
>> @@ -537,7 +537,7 @@ extern int __REDIRECT (vfscanf,
>>                  (FILE *__restrict __s,
>>               const char *__restrict __format, __gnuc_va_list __arg),
>>                  __isoc99_vfscanf)
>> -     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur;
>> +     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur __nonnull 
>> ((1));
>>   extern int __REDIRECT (vscanf, (const char *__restrict __format,
>>                   __gnuc_va_list __arg), __isoc99_vscanf)
>>        __attribute__ ((__format__ (__scanf__, 1, 0))) __wur;
>> @@ -549,7 +549,7 @@ extern int __REDIRECT_NTH (vsscanf,
>>   #   elif !defined __REDIRECT
>>   extern int __isoc99_vfscanf (FILE *__restrict __s,
>>                    const char *__restrict __format,
>> -                 __gnuc_va_list __arg) __wur;
>> +                 __gnuc_va_list __arg) __wur __nonnull ((1));
>>   extern int __isoc99_vscanf (const char *__restrict __format,
>>                   __gnuc_va_list __arg) __wur;
>>   extern int __isoc99_vsscanf (const char *__restrict __s,
>> @@ -568,8 +568,8 @@ extern int __isoc99_vsscanf (const char 
>> *__restrict __s,
>>      These functions are possible cancellation points and therefore not
>>      marked with __THROW.  */
>> -extern int fgetc (FILE *__stream);
>> -extern int getc (FILE *__stream);
>> +extern int fgetc (FILE *__stream) __nonnull ((1));
>> +extern int getc (FILE *__stream) __nonnull ((1));
>>   /* Read a character from stdin.
>> @@ -582,7 +582,7 @@ extern int getchar (void);
>>      These functions are possible cancellation points and therefore not
>>      marked with __THROW.  */
>> -extern int getc_unlocked (FILE *__stream);
>> +extern int getc_unlocked (FILE *__stream) __nonnull ((1));
>>   extern int getchar_unlocked (void);
>>   #endif /* Use POSIX.  */
>> @@ -593,7 +593,7 @@ extern int getchar_unlocked (void);
>>      cancellation point.  But due to similarity with an POSIX interface
>>      or due to the implementation it is a cancellation point and
>>      therefore not marked with __THROW.  */
>> -extern int fgetc_unlocked (FILE *__stream);
>> +extern int fgetc_unlocked (FILE *__stream) __nonnull ((1));
>>   #endif /* Use MISC.  */
>> @@ -604,8 +604,8 @@ extern int fgetc_unlocked (FILE *__stream);
>>      These functions is a possible cancellation point and therefore not
> 
> Aside: here's a typo mixing plural and singular in the sentence (not
> part of your commit).  I'll prepare a patch for it.
> 
>>      marked with __THROW.  */
>> -extern int fputc (int __c, FILE *__stream);
>> -extern int putc (int __c, FILE *__stream);
>> +extern int fputc (int __c, FILE *__stream) __nonnull ((2));
>> +extern int putc (int __c, FILE *__stream) __nonnull ((2));
>>   /* Write a character to stdout.
>> @@ -620,7 +620,7 @@ extern int putchar (int __c);
>>      cancellation point.  But due to similarity with an POSIX interface
>>      or due to the implementation it is a cancellation point and
>>      therefore not marked with __THROW.  */
>> -extern int fputc_unlocked (int __c, FILE *__stream);
>> +extern int fputc_unlocked (int __c, FILE *__stream) __nonnull ((2));
>>   #endif /* Use MISC.  */
>>   #ifdef __USE_POSIX199506
>> @@ -628,7 +628,7 @@ extern int fputc_unlocked (int __c, FILE *__stream);
>>      These functions are possible cancellation points and therefore not
>>      marked with __THROW.  */
>> -extern int putc_unlocked (int __c, FILE *__stream);
>> +extern int putc_unlocked (int __c, FILE *__stream) __nonnull ((2));
>>   extern int putchar_unlocked (int __c);
>>   #endif /* Use POSIX.  */
>> @@ -636,10 +636,10 @@ extern int putchar_unlocked (int __c);
>>   #if defined __USE_MISC \
>>       || (defined __USE_XOPEN && !defined __USE_XOPEN2K)
>>   /* Get a word (int) from STREAM.  */
>> -extern int getw (FILE *__stream);
>> +extern int getw (FILE *__stream) __nonnull ((1));
>>   /* Write a word (int) to STREAM.  */
>> -extern int putw (int __w, FILE *__stream);
>> +extern int putw (int __w, FILE *__stream) __nonnull ((2));
>>   #endif
>> @@ -689,10 +689,10 @@ extern char *fgets_unlocked (char *__restrict 
>> __s, int __n,
>>      therefore not marked with __THROW.  */
>>   extern __ssize_t __getdelim (char **__restrict __lineptr,
>>                                size_t *__restrict __n, int __delimiter,
>> -                             FILE *__restrict __stream) __wur;
>> +                             FILE *__restrict __stream) __wur 
>> __nonnull ((4));
>>   extern __ssize_t getdelim (char **__restrict __lineptr,
>>                              size_t *__restrict __n, int __delimiter,
>> -                           FILE *__restrict __stream) __wur;
>> +                           FILE *__restrict __stream) __wur __nonnull 
>> ((4));
>>   /* Like `getdelim', but reads up to a newline.
>> @@ -702,7 +702,7 @@ extern __ssize_t getdelim (char **__restrict 
>> __lineptr,
>>      therefore not marked with __THROW.  */
>>   extern __ssize_t getline (char **__restrict __lineptr,
>>                             size_t *__restrict __n,
>> -                          FILE *__restrict __stream) __wur;
>> +                          FILE *__restrict __stream) __wur __nonnull 
>> ((3));
>>   #endif
>> @@ -723,7 +723,7 @@ extern int puts (const char *__s);
>>      This function is a possible cancellation point and therefore not
>>      marked with __THROW.  */
>> -extern int ungetc (int __c, FILE *__stream);
>> +extern int ungetc (int __c, FILE *__stream) __nonnull ((2));
>>   /* Read chunks of generic data from STREAM.
>> @@ -768,17 +768,17 @@ extern size_t fwrite_unlocked (const void 
>> *__restrict __ptr, size_t __size,
>>      This function is a possible cancellation point and therefore not
>>      marked with __THROW.  */
>> -extern int fseek (FILE *__stream, long int __off, int __whence);
>> +extern int fseek (FILE *__stream, long int __off, int __whence) 
>> __nonnull ((1));
>>   /* Return the current position of STREAM.
>>      This function is a possible cancellation point and therefore not
>>      marked with __THROW.  */
>> -extern long int ftell (FILE *__stream) __wur;
>> +extern long int ftell (FILE *__stream) __wur __nonnull ((1));
>>   /* Rewind to the beginning of STREAM.
>>      This function is a possible cancellation point and therefore not
>>      marked with __THROW.  */
>> -extern void rewind (FILE *__stream);
>> +extern void rewind (FILE *__stream) __nonnull ((1));
>>   /* The Single Unix Specification, Version 2, specifies an alternative,
>>      more adequate interface for the two functions above which deal with
>> @@ -791,18 +791,19 @@ extern void rewind (FILE *__stream);
>>      This function is a possible cancellation point and therefore not
>>      marked with __THROW.  */
>> -extern int fseeko (FILE *__stream, __off_t __off, int __whence);
>> +extern int fseeko (FILE *__stream, __off_t __off, int __whence) 
>> __nonnull ((1));
>>   /* Return the current position of STREAM.
>>      This function is a possible cancellation point and therefore not
>>      marked with __THROW.  */
>> -extern __off_t ftello (FILE *__stream) __wur;
>> +extern __off_t ftello (FILE *__stream) __wur __nonnull ((1));
>>   # else
>>   #  ifdef __REDIRECT
>>   extern int __REDIRECT (fseeko,
>>                  (FILE *__stream, __off64_t __off, int __whence),
>> -               fseeko64);
>> -extern __off64_t __REDIRECT (ftello, (FILE *__stream), ftello64);
>> +               fseeko64) __nonnull ((1));
>> +extern __off64_t __REDIRECT (ftello, (FILE *__stream), ftello64)
>> +  __nonnull ((1));
>>   #  else
>>   #   define fseeko fseeko64
>>   #   define ftello ftello64
>> @@ -815,18 +816,20 @@ extern __off64_t __REDIRECT (ftello, (FILE 
>> *__stream), ftello64);
>>      This function is a possible cancellation point and therefore not
>>      marked with __THROW.  */
>> -extern int fgetpos (FILE *__restrict __stream, fpos_t *__restrict 
>> __pos);
>> +extern int fgetpos (FILE *__restrict __stream, fpos_t *__restrict __pos)
>> +  __nonnull ((1));
>>   /* Set STREAM's position.
>>      This function is a possible cancellation point and therefore not
>>      marked with __THROW.  */
>> -extern int fsetpos (FILE *__stream, const fpos_t *__pos);
>> +extern int fsetpos (FILE *__stream, const fpos_t *__pos) __nonnull 
>> ((1));
>>   #else
>>   # ifdef __REDIRECT
>>   extern int __REDIRECT (fgetpos, (FILE *__restrict __stream,
>> -                 fpos_t *__restrict __pos), fgetpos64);
>> +                 fpos_t *__restrict __pos), fgetpos64) __nonnull ((1));
>>   extern int __REDIRECT (fsetpos,
>> -               (FILE *__stream, const fpos_t *__pos), fsetpos64);
>> +               (FILE *__stream, const fpos_t *__pos), fsetpos64)
>> +  __nonnull ((1));
>>   # else
>>   #  define fgetpos fgetpos64
>>   #  define fsetpos fsetpos64
>> @@ -834,24 +837,26 @@ extern int __REDIRECT (fsetpos,
>>   #endif
>>   #ifdef __USE_LARGEFILE64
>> -extern int fseeko64 (FILE *__stream, __off64_t __off, int __whence);
>> -extern __off64_t ftello64 (FILE *__stream) __wur;
>> -extern int fgetpos64 (FILE *__restrict __stream, fpos64_t *__restrict 
>> __pos);
>> -extern int fsetpos64 (FILE *__stream, const fpos64_t *__pos);
>> +extern int fseeko64 (FILE *__stream, __off64_t __off, int __whence)
>> +  __nonnull ((1));
>> +extern __off64_t ftello64 (FILE *__stream) __wur __nonnull ((1));
>> +extern int fgetpos64 (FILE *__restrict __stream, fpos64_t *__restrict 
>> __pos)
>> +  __nonnull ((1));
>> +extern int fsetpos64 (FILE *__stream, const fpos64_t *__pos) 
>> __nonnull ((1));
>>   #endif
>>   /* Clear the error and EOF indicators for STREAM.  */
>> -extern void clearerr (FILE *__stream) __THROW;
>> +extern void clearerr (FILE *__stream) __THROW __nonnull ((1));
>>   /* Return the EOF indicator for STREAM.  */
>> -extern int feof (FILE *__stream) __THROW __wur;
>> +extern int feof (FILE *__stream) __THROW __wur __nonnull ((1));
>>   /* Return the error indicator for STREAM.  */
>> -extern int ferror (FILE *__stream) __THROW __wur;
>> +extern int ferror (FILE *__stream) __THROW __wur __nonnull ((1));
>>   #ifdef __USE_MISC
>>   /* Faster versions when locking is not required.  */
>> -extern void clearerr_unlocked (FILE *__stream) __THROW;
>> -extern int feof_unlocked (FILE *__stream) __THROW __wur;
>> -extern int ferror_unlocked (FILE *__stream) __THROW __wur;
>> +extern void clearerr_unlocked (FILE *__stream) __THROW __nonnull ((1));
>> +extern int feof_unlocked (FILE *__stream) __THROW __wur __nonnull ((1));
>> +extern int ferror_unlocked (FILE *__stream) __THROW __wur __nonnull 
>> ((1));
>>   #endif
>> @@ -864,12 +869,12 @@ extern void perror (const char *__s) __COLD;
>>   #ifdef    __USE_POSIX
>>   /* Return the system file descriptor for STREAM.  */
>> -extern int fileno (FILE *__stream) __THROW __wur;
>> +extern int fileno (FILE *__stream) __THROW __wur __nonnull ((1));
>>   #endif /* Use POSIX.  */
>>   #ifdef __USE_MISC
>>   /* Faster version when locking is not required.  */
>> -extern int fileno_unlocked (FILE *__stream) __THROW __wur;
>> +extern int fileno_unlocked (FILE *__stream) __THROW __wur __nonnull 
>> ((1));
>>   #endif
>> @@ -878,7 +883,7 @@ extern int fileno_unlocked (FILE *__stream) 
>> __THROW __wur;
>>      This function is a possible cancellation point and therefore not
>>      marked with __THROW.  */
>> -extern int pclose (FILE *__stream);
>> +extern int pclose (FILE *__stream) __nonnull ((1));
> 
> You didn't patch fclose(3).  Any reason?  I guess it's similarly UB to
> call fclose(NULL).
> 
> Cheers,
> Alex
> 
>>   /* Create a new stream connected to a pipe running the given command.
>> @@ -922,14 +927,14 @@ extern int obstack_vprintf (struct obstack 
>> *__restrict __obstack,
>>   /* These are defined in POSIX.1:1996.  */
>>   /* Acquire ownership of STREAM.  */
>> -extern void flockfile (FILE *__stream) __THROW;
>> +extern void flockfile (FILE *__stream) __THROW __nonnull ((1));
>>   /* Try to acquire ownership of STREAM but do not block if it is not
>>      possible.  */
>> -extern int ftrylockfile (FILE *__stream) __THROW __wur;
>> +extern int ftrylockfile (FILE *__stream) __THROW __wur __nonnull ((1));
>>   /* Relinquish the ownership granted for STREAM.  */
>> -extern void funlockfile (FILE *__stream) __THROW;
>> +extern void funlockfile (FILE *__stream) __THROW __nonnull ((1));
>>   #endif /* POSIX */
>>   #if defined __USE_XOPEN && !defined __USE_XOPEN2K && !defined __USE_GNU
> 

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
Alejandro Colomar May 18, 2023, 6:56 p.m. UTC | #3 
On 5/18/23 20:06, Alex Colomar wrote:
> Hi Xi!
> 
> On 5/18/23 19:25, Xi Ruoyao via Libc-alpha wrote:
>> During the review of a GCC analyzer test case, we found most stdio
>> functions accepting a FILE * argument expect it to be nonnull and just
>> segfault when the argument is NULL.  Add nonnull attribute for them.
>>
>> setbuf is well defined when __stream is NULL so it's not touched.
>>
>> For fputs, fgets, fread, fwrite, fprintf, vfprintf, and their unlocked
>> version, if __stream is empty but there is nothing to read or write,
>> they don't segfault and I'm not sure if the standard allows such a use
>> so I left them out.
>> ---
>>   libio/stdio.h | 119 ++++++++++++++++++++++++++------------------------
>>   1 file changed, 62 insertions(+), 57 deletions(-)
>>
>> diff --git a/libio/stdio.h b/libio/stdio.h
>> index 4cf9f1c012..ae3d7295d4 100644
>> --- a/libio/stdio.h
>> +++ b/libio/stdio.h
>> @@ -232,7 +232,7 @@ extern char *tempnam (const char *__dir, const 
>> char *__pfx)
>>      This function is a possible cancellation point and therefore not
>>      marked with __THROW.  */
>> -extern int fflush (FILE *__stream);
>> +extern int fflush (FILE *__stream) __nonnull ((1));
> 
> flush(NULL) is well defined.  It flushes all streams that can be
> flushed.  This reminds me that I should document that in the SYNOPSIS
> section of the manual page; an oversight on my side.

I just pushed this:

<https://git.kernel.org/pub/scm/docs/man-pages/man-pages.git/commit/?id=cd03c9b8d1f4d43ab7b010d02287dec1805ada34>

commit cd03c9b8d1f4d43ab7b010d02287dec1805ada34 (HEAD -> master, 
korg/master, alx/main, alx/HEAD, main)
Author: Alejandro Colomar <alx@kernel.org>
Date:   Thu May 18 20:53:14 2023 +0200

     fflush.3, unlocked_stdio.3: SYNOPSIS: The streams can be null

     Cc: Xi Ruoyao <xry111@xry111.site>
     Signed-off-by: Alejandro Colomar <alx@kernel.org>

diff --git a/man3/fflush.3 b/man3/fflush.3
index 327786cef..2098bba08 100644
--- a/man3/fflush.3
+++ b/man3/fflush.3
@@ -25,7 +25,7 @@ Standard C library
  .nf
  .B #include <stdio.h>
  .PP
-.BI "int fflush(FILE *" stream );
+.BI "int fflush(FILE *_Nullable " stream );
  .fi
  .SH DESCRIPTION
  For output streams,
diff --git a/man3/unlocked_stdio.3 b/man3/unlocked_stdio.3
index faab29f06..feed708db 100644
--- a/man3/unlocked_stdio.3
+++ b/man3/unlocked_stdio.3
@@ -23,7 +23,7 @@ Standard C library
  .BI "int feof_unlocked(FILE *" stream );
  .BI "int ferror_unlocked(FILE *" stream );
  .BI "int fileno_unlocked(FILE *" stream );
-.BI "int fflush_unlocked(FILE *" stream );
+.BI "int fflush_unlocked(FILE *_Nullable " stream );
  .PP
  .BI "int fgetc_unlocked(FILE *" stream );
  .BI "int fputc_unlocked(int " c ", FILE *" stream );

> 
>>   #ifdef __USE_MISC
>>   /* Faster versions when locking is not required.
>> @@ -241,7 +241,7 @@ extern int fflush (FILE *__stream);
>>      cancellation point.  But due to similarity with an POSIX interface
>>      or due to the implementation it is a cancellation point and
>>      therefore not marked with __THROW.  */
>> -extern int fflush_unlocked (FILE *__stream);
>> +extern int fflush_unlocked (FILE *__stream) __nonnull ((1));
> 
> Without checking, I'll guess that fflush_unlocked(NULL) is also well
> defined.
> 
> I didn't see any other similar cases, but I may have missed some; I
> didn't revise them all thoroughly; please check.
> 

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
Xi Ruoyao May 19, 2023, 4:13 a.m. UTC | #4 
On Thu, 2023-05-18 at 20:29 +0200, Alex Colomar wrote:
> On 5/18/23 20:06, Alex Colomar wrote:
> > Hi Xi!
> > 
> > On 5/18/23 19:25, Xi Ruoyao via Libc-alpha wrote:
> > > During the review of a GCC analyzer test case, we found most stdio
> > > functions accepting a FILE * argument expect it to be nonnull and
> > > just
> > > segfault when the argument is NULL.  Add nonnull attribute for
> > > them.
> > > 
> > > setbuf is well defined when __stream is NULL so it's not touched.
> 
> Is this true?  I couldn't find anything about it in either setbuf(3),
> setbuf(3POSIX), or info libc.
> 
> The buffer can be NULL, but the stream?

I guess when I took the note on the paper I misspelled "fflush" as
"setbuf" for some reason (maybe lack of coffee :( ).

I'll retest all the changes in one hour or two hours.

> 
> > > 
> > > For fputs, fgets, fread, fwrite, fprintf, vfprintf, and their
> > > unlocked
> > > version, if __stream is empty but there is nothing to read or
> > > write,
> > > they don't segfault and I'm not sure if the standard allows such a
> > > use
> > > so I left them out.
> > > ---
> > >   libio/stdio.h | 119 ++++++++++++++++++++++++++------------------
> > > ------
> > >   1 file changed, 62 insertions(+), 57 deletions(-)
> > > 
> > > diff --git a/libio/stdio.h b/libio/stdio.h
> > > index 4cf9f1c012..ae3d7295d4 100644
> > > --- a/libio/stdio.h
> > > +++ b/libio/stdio.h
> > > @@ -232,7 +232,7 @@ extern char *tempnam (const char *__dir, const
> > > char *__pfx)
> > >      This function is a possible cancellation point and therefore
> > > not
> > >      marked with __THROW.  */
> > > -extern int fflush (FILE *__stream);
> > > +extern int fflush (FILE *__stream) __nonnull ((1));
> > 
> > flush(NULL) is well defined.  It flushes all streams that can be
> > flushed.  This reminds me that I should document that in the
> > SYNOPSIS
> > section of the manual page; an oversight on my side.
> > 
> > >   #ifdef __USE_MISC
> > >   /* Faster versions when locking is not required.
> > > @@ -241,7 +241,7 @@ extern int fflush (FILE *__stream);
> > >      cancellation point.  But due to similarity with an POSIX
> > > interface
> > >      or due to the implementation it is a cancellation point and
> > >      therefore not marked with __THROW.  */
> > > -extern int fflush_unlocked (FILE *__stream);
> > > +extern int fflush_unlocked (FILE *__stream) __nonnull ((1));
> > 
> > Without checking, I'll guess that fflush_unlocked(NULL) is also well
> > defined.
> > 
> > I didn't see any other similar cases, but I may have missed some; I
> > didn't revise them all thoroughly; please check.
> > 
> > >   #endif
> > >   #ifdef __USE_GNU
> > > @@ -278,7 +278,7 @@ extern FILE *__REDIRECT (fopen, (const char 
> > > *__restrict __filename,
> > >   extern FILE *__REDIRECT (freopen, (const char *__restrict
> > > __filename,
> > >                      const char *__restrict __modes,
> > >                      FILE *__restrict __stream), freopen64)
> > > -  __wur;
> > > +  __wur __nonnull ((3));
> > >   # else
> > >   #  define fopen fopen64
> > >   #  define freopen freopen64
> > > @@ -335,16 +335,16 @@ extern void setbuf (FILE *__restrict
> > > __stream, 
> > > char *__restrict __buf) __THROW;
> > >      If BUF is not NULL, use N bytes of it for buffering;
> > >      else allocate an internal buffer N bytes long.  */
> > >   extern int setvbuf (FILE *__restrict __stream, char *__restrict
> > > __buf,
> > > -            int __modes, size_t __n) __THROW;
> > > +            int __modes, size_t __n) __THROW __nonnull ((1));
> > >   #ifdef    __USE_MISC
> > >   /* If BUF is NULL, make STREAM unbuffered.
> > >      Else make it use SIZE bytes of BUF for buffering.  */
> > >   extern void setbuffer (FILE *__restrict __stream, char
> > > *__restrict 
> > > __buf,
> > > -               size_t __size) __THROW;
> > > +               size_t __size) __THROW __nonnull ((1));
> > >   /* Make STREAM line-buffered.  */
> > > -extern void setlinebuf (FILE *__stream) __THROW;
> > > +extern void setlinebuf (FILE *__stream) __THROW __nonnull ((1));
> > >   #endif
> > > @@ -418,7 +418,7 @@ extern int dprintf (int __fd, const char 
> > > *__restrict __fmt, ...)
> > >      This function is a possible cancellation point and therefore
> > > not
> > >      marked with __THROW.  */
> > >   extern int fscanf (FILE *__restrict __stream,
> > > -           const char *__restrict __format, ...) __wur;
> > > +           const char *__restrict __format, ...) __wur __nonnull
> > > ((1));
> > >   /* Read formatted input from stdin.
> > >      This function is a possible cancellation point and therefore
> > > not
> > > @@ -439,7 +439,7 @@ extern int sscanf (const char *__restrict __s,
> > >   #  ifdef __REDIRECT
> > >   extern int __REDIRECT (fscanf, (FILE *__restrict __stream,
> > >                   const char *__restrict __format, ...),
> > > -               __isoc23_fscanf) __wur;
> > > +               __isoc23_fscanf) __wur __nonnull ((1));
> > >   extern int __REDIRECT (scanf, (const char *__restrict __format,
> > > ...),
> > >                  __isoc23_scanf) __wur;
> > >   extern int __REDIRECT_NTH (sscanf, (const char *__restrict __s,
> > > @@ -447,7 +447,7 @@ extern int __REDIRECT_NTH (sscanf, (const char
> > > *__restrict __s,
> > >                  __isoc23_sscanf);
> > >   #  else
> > >   extern int __isoc23_fscanf (FILE *__restrict __stream,
> > > -                const char *__restrict __format, ...) __wur;
> > > +                const char *__restrict __format, ...) __wur
> > > __nonnull 
> > > ((1));
> > >   extern int __isoc23_scanf (const char *__restrict __format, ...)
> > > __wur;
> > >   extern int __isoc23_sscanf (const char *__restrict __s,
> > >                   const char *__restrict __format, ...) __THROW;
> > > @@ -459,7 +459,7 @@ extern int __isoc23_sscanf (const char
> > > *__restrict 
> > > __s,
> > >   #  ifdef __REDIRECT
> > >   extern int __REDIRECT (fscanf, (FILE *__restrict __stream,
> > >                   const char *__restrict __format, ...),
> > > -               __isoc99_fscanf) __wur;
> > > +               __isoc99_fscanf) __wur __nonnull ((1));
> > >   extern int __REDIRECT (scanf, (const char *__restrict __format,
> > > ...),
> > >                  __isoc99_scanf) __wur;
> > >   extern int __REDIRECT_NTH (sscanf, (const char *__restrict __s,
> > > @@ -467,7 +467,7 @@ extern int __REDIRECT_NTH (sscanf, (const char
> > > *__restrict __s,
> > >                  __isoc99_sscanf);
> > >   #  else
> > >   extern int __isoc99_fscanf (FILE *__restrict __stream,
> > > -                const char *__restrict __format, ...) __wur;
> > > +                const char *__restrict __format, ...) __wur
> > > __nonnull 
> > > ((1));
> > >   extern int __isoc99_scanf (const char *__restrict __format, ...)
> > > __wur;
> > >   extern int __isoc99_sscanf (const char *__restrict __s,
> > >                   const char *__restrict __format, ...) __THROW;
> > > @@ -485,7 +485,7 @@ extern int __isoc99_sscanf (const char
> > > *__restrict 
> > > __s,
> > >      marked with __THROW.  */
> > >   extern int vfscanf (FILE *__restrict __s, const char *__restrict
> > > __format,
> > >               __gnuc_va_list __arg)
> > > -     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur;
> > > +     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur
> > > __nonnull 
> > > ((1));
> > >   /* Read formatted input from stdin into argument list ARG.
> > > @@ -508,7 +508,7 @@ extern int __REDIRECT (vfscanf,
> > >                  (FILE *__restrict __s,
> > >               const char *__restrict __format, __gnuc_va_list
> > > __arg),
> > >                  __isoc23_vfscanf)
> > > -     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur;
> > > +     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur
> > > __nonnull 
> > > ((1));
> > >   extern int __REDIRECT (vscanf, (const char *__restrict __format,
> > >                   __gnuc_va_list __arg), __isoc23_vscanf)
> > >        __attribute__ ((__format__ (__scanf__, 1, 0))) __wur;
> > > @@ -520,7 +520,7 @@ extern int __REDIRECT_NTH (vsscanf,
> > >   #   elif !defined __REDIRECT
> > >   extern int __isoc23_vfscanf (FILE *__restrict __s,
> > >                    const char *__restrict __format,
> > > -                 __gnuc_va_list __arg) __wur;
> > > +                 __gnuc_va_list __arg) __wur __nonnull ((1));
> > >   extern int __isoc23_vscanf (const char *__restrict __format,
> > >                   __gnuc_va_list __arg) __wur;
> > >   extern int __isoc23_vsscanf (const char *__restrict __s,
> > > @@ -537,7 +537,7 @@ extern int __REDIRECT (vfscanf,
> > >                  (FILE *__restrict __s,
> > >               const char *__restrict __format, __gnuc_va_list
> > > __arg),
> > >                  __isoc99_vfscanf)
> > > -     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur;
> > > +     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur
> > > __nonnull 
> > > ((1));
> > >   extern int __REDIRECT (vscanf, (const char *__restrict __format,
> > >                   __gnuc_va_list __arg), __isoc99_vscanf)
> > >        __attribute__ ((__format__ (__scanf__, 1, 0))) __wur;
> > > @@ -549,7 +549,7 @@ extern int __REDIRECT_NTH (vsscanf,
> > >   #   elif !defined __REDIRECT
> > >   extern int __isoc99_vfscanf (FILE *__restrict __s,
> > >                    const char *__restrict __format,
> > > -                 __gnuc_va_list __arg) __wur;
> > > +                 __gnuc_va_list __arg) __wur __nonnull ((1));
> > >   extern int __isoc99_vscanf (const char *__restrict __format,
> > >                   __gnuc_va_list __arg) __wur;
> > >   extern int __isoc99_vsscanf (const char *__restrict __s,
> > > @@ -568,8 +568,8 @@ extern int __isoc99_vsscanf (const char 
> > > *__restrict __s,
> > >      These functions are possible cancellation points and
> > > therefore not
> > >      marked with __THROW.  */
> > > -extern int fgetc (FILE *__stream);
> > > -extern int getc (FILE *__stream);
> > > +extern int fgetc (FILE *__stream) __nonnull ((1));
> > > +extern int getc (FILE *__stream) __nonnull ((1));
> > >   /* Read a character from stdin.
> > > @@ -582,7 +582,7 @@ extern int getchar (void);
> > >      These functions are possible cancellation points and
> > > therefore not
> > >      marked with __THROW.  */
> > > -extern int getc_unlocked (FILE *__stream);
> > > +extern int getc_unlocked (FILE *__stream) __nonnull ((1));
> > >   extern int getchar_unlocked (void);
> > >   #endif /* Use POSIX.  */
> > > @@ -593,7 +593,7 @@ extern int getchar_unlocked (void);
> > >      cancellation point.  But due to similarity with an POSIX
> > > interface
> > >      or due to the implementation it is a cancellation point and
> > >      therefore not marked with __THROW.  */
> > > -extern int fgetc_unlocked (FILE *__stream);
> > > +extern int fgetc_unlocked (FILE *__stream) __nonnull ((1));
> > >   #endif /* Use MISC.  */
> > > @@ -604,8 +604,8 @@ extern int fgetc_unlocked (FILE *__stream);
> > >      These functions is a possible cancellation point and
> > > therefore not
> > 
> > Aside: here's a typo mixing plural and singular in the sentence (not
> > part of your commit).  I'll prepare a patch for it.
> > 
> > >      marked with __THROW.  */
> > > -extern int fputc (int __c, FILE *__stream);
> > > -extern int putc (int __c, FILE *__stream);
> > > +extern int fputc (int __c, FILE *__stream) __nonnull ((2));
> > > +extern int putc (int __c, FILE *__stream) __nonnull ((2));
> > >   /* Write a character to stdout.
> > > @@ -620,7 +620,7 @@ extern int putchar (int __c);
> > >      cancellation point.  But due to similarity with an POSIX
> > > interface
> > >      or due to the implementation it is a cancellation point and
> > >      therefore not marked with __THROW.  */
> > > -extern int fputc_unlocked (int __c, FILE *__stream);
> > > +extern int fputc_unlocked (int __c, FILE *__stream) __nonnull
> > > ((2));
> > >   #endif /* Use MISC.  */
> > >   #ifdef __USE_POSIX199506
> > > @@ -628,7 +628,7 @@ extern int fputc_unlocked (int __c, FILE
> > > *__stream);
> > >      These functions are possible cancellation points and
> > > therefore not
> > >      marked with __THROW.  */
> > > -extern int putc_unlocked (int __c, FILE *__stream);
> > > +extern int putc_unlocked (int __c, FILE *__stream) __nonnull
> > > ((2));
> > >   extern int putchar_unlocked (int __c);
> > >   #endif /* Use POSIX.  */
> > > @@ -636,10 +636,10 @@ extern int putchar_unlocked (int __c);
> > >   #if defined __USE_MISC \
> > >       || (defined __USE_XOPEN && !defined __USE_XOPEN2K)
> > >   /* Get a word (int) from STREAM.  */
> > > -extern int getw (FILE *__stream);
> > > +extern int getw (FILE *__stream) __nonnull ((1));
> > >   /* Write a word (int) to STREAM.  */
> > > -extern int putw (int __w, FILE *__stream);
> > > +extern int putw (int __w, FILE *__stream) __nonnull ((2));
> > >   #endif
> > > @@ -689,10 +689,10 @@ extern char *fgets_unlocked (char
> > > *__restrict 
> > > __s, int __n,
> > >      therefore not marked with __THROW.  */
> > >   extern __ssize_t __getdelim (char **__restrict __lineptr,
> > >                                size_t *__restrict __n, int
> > > __delimiter,
> > > -                             FILE *__restrict __stream) __wur;
> > > +                             FILE *__restrict __stream) __wur 
> > > __nonnull ((4));
> > >   extern __ssize_t getdelim (char **__restrict __lineptr,
> > >                              size_t *__restrict __n, int
> > > __delimiter,
> > > -                           FILE *__restrict __stream) __wur;
> > > +                           FILE *__restrict __stream) __wur
> > > __nonnull 
> > > ((4));
> > >   /* Like `getdelim', but reads up to a newline.
> > > @@ -702,7 +702,7 @@ extern __ssize_t getdelim (char **__restrict 
> > > __lineptr,
> > >      therefore not marked with __THROW.  */
> > >   extern __ssize_t getline (char **__restrict __lineptr,
> > >                             size_t *__restrict __n,
> > > -                          FILE *__restrict __stream) __wur;
> > > +                          FILE *__restrict __stream) __wur
> > > __nonnull 
> > > ((3));
> > >   #endif
> > > @@ -723,7 +723,7 @@ extern int puts (const char *__s);
> > >      This function is a possible cancellation point and therefore
> > > not
> > >      marked with __THROW.  */
> > > -extern int ungetc (int __c, FILE *__stream);
> > > +extern int ungetc (int __c, FILE *__stream) __nonnull ((2));
> > >   /* Read chunks of generic data from STREAM.
> > > @@ -768,17 +768,17 @@ extern size_t fwrite_unlocked (const void 
> > > *__restrict __ptr, size_t __size,
> > >      This function is a possible cancellation point and therefore
> > > not
> > >      marked with __THROW.  */
> > > -extern int fseek (FILE *__stream, long int __off, int __whence);
> > > +extern int fseek (FILE *__stream, long int __off, int __whence) 
> > > __nonnull ((1));
> > >   /* Return the current position of STREAM.
> > >      This function is a possible cancellation point and therefore
> > > not
> > >      marked with __THROW.  */
> > > -extern long int ftell (FILE *__stream) __wur;
> > > +extern long int ftell (FILE *__stream) __wur __nonnull ((1));
> > >   /* Rewind to the beginning of STREAM.
> > >      This function is a possible cancellation point and therefore
> > > not
> > >      marked with __THROW.  */
> > > -extern void rewind (FILE *__stream);
> > > +extern void rewind (FILE *__stream) __nonnull ((1));
> > >   /* The Single Unix Specification, Version 2, specifies an
> > > alternative,
> > >      more adequate interface for the two functions above which
> > > deal with
> > > @@ -791,18 +791,19 @@ extern void rewind (FILE *__stream);
> > >      This function is a possible cancellation point and therefore
> > > not
> > >      marked with __THROW.  */
> > > -extern int fseeko (FILE *__stream, __off_t __off, int __whence);
> > > +extern int fseeko (FILE *__stream, __off_t __off, int __whence) 
> > > __nonnull ((1));
> > >   /* Return the current position of STREAM.
> > >      This function is a possible cancellation point and therefore
> > > not
> > >      marked with __THROW.  */
> > > -extern __off_t ftello (FILE *__stream) __wur;
> > > +extern __off_t ftello (FILE *__stream) __wur __nonnull ((1));
> > >   # else
> > >   #  ifdef __REDIRECT
> > >   extern int __REDIRECT (fseeko,
> > >                  (FILE *__stream, __off64_t __off, int __whence),
> > > -               fseeko64);
> > > -extern __off64_t __REDIRECT (ftello, (FILE *__stream), ftello64);
> > > +               fseeko64) __nonnull ((1));
> > > +extern __off64_t __REDIRECT (ftello, (FILE *__stream), ftello64)
> > > +  __nonnull ((1));
> > >   #  else
> > >   #   define fseeko fseeko64
> > >   #   define ftello ftello64
> > > @@ -815,18 +816,20 @@ extern __off64_t __REDIRECT (ftello, (FILE 
> > > *__stream), ftello64);
> > >      This function is a possible cancellation point and therefore
> > > not
> > >      marked with __THROW.  */
> > > -extern int fgetpos (FILE *__restrict __stream, fpos_t *__restrict
> > > __pos);
> > > +extern int fgetpos (FILE *__restrict __stream, fpos_t *__restrict
> > > __pos)
> > > +  __nonnull ((1));
> > >   /* Set STREAM's position.
> > >      This function is a possible cancellation point and therefore
> > > not
> > >      marked with __THROW.  */
> > > -extern int fsetpos (FILE *__stream, const fpos_t *__pos);
> > > +extern int fsetpos (FILE *__stream, const fpos_t *__pos)
> > > __nonnull 
> > > ((1));
> > >   #else
> > >   # ifdef __REDIRECT
> > >   extern int __REDIRECT (fgetpos, (FILE *__restrict __stream,
> > > -                 fpos_t *__restrict __pos), fgetpos64);
> > > +                 fpos_t *__restrict __pos), fgetpos64) __nonnull
> > > ((1));
> > >   extern int __REDIRECT (fsetpos,
> > > -               (FILE *__stream, const fpos_t *__pos), fsetpos64);
> > > +               (FILE *__stream, const fpos_t *__pos), fsetpos64)
> > > +  __nonnull ((1));
> > >   # else
> > >   #  define fgetpos fgetpos64
> > >   #  define fsetpos fsetpos64
> > > @@ -834,24 +837,26 @@ extern int __REDIRECT (fsetpos,
> > >   #endif
> > >   #ifdef __USE_LARGEFILE64
> > > -extern int fseeko64 (FILE *__stream, __off64_t __off, int
> > > __whence);
> > > -extern __off64_t ftello64 (FILE *__stream) __wur;
> > > -extern int fgetpos64 (FILE *__restrict __stream, fpos64_t
> > > *__restrict 
> > > __pos);
> > > -extern int fsetpos64 (FILE *__stream, const fpos64_t *__pos);
> > > +extern int fseeko64 (FILE *__stream, __off64_t __off, int
> > > __whence)
> > > +  __nonnull ((1));
> > > +extern __off64_t ftello64 (FILE *__stream) __wur __nonnull ((1));
> > > +extern int fgetpos64 (FILE *__restrict __stream, fpos64_t
> > > *__restrict 
> > > __pos)
> > > +  __nonnull ((1));
> > > +extern int fsetpos64 (FILE *__stream, const fpos64_t *__pos) 
> > > __nonnull ((1));
> > >   #endif
> > >   /* Clear the error and EOF indicators for STREAM.  */
> > > -extern void clearerr (FILE *__stream) __THROW;
> > > +extern void clearerr (FILE *__stream) __THROW __nonnull ((1));
> > >   /* Return the EOF indicator for STREAM.  */
> > > -extern int feof (FILE *__stream) __THROW __wur;
> > > +extern int feof (FILE *__stream) __THROW __wur __nonnull ((1));
> > >   /* Return the error indicator for STREAM.  */
> > > -extern int ferror (FILE *__stream) __THROW __wur;
> > > +extern int ferror (FILE *__stream) __THROW __wur __nonnull ((1));
> > >   #ifdef __USE_MISC
> > >   /* Faster versions when locking is not required.  */
> > > -extern void clearerr_unlocked (FILE *__stream) __THROW;
> > > -extern int feof_unlocked (FILE *__stream) __THROW __wur;
> > > -extern int ferror_unlocked (FILE *__stream) __THROW __wur;
> > > +extern void clearerr_unlocked (FILE *__stream) __THROW __nonnull
> > > ((1));
> > > +extern int feof_unlocked (FILE *__stream) __THROW __wur __nonnull
> > > ((1));
> > > +extern int ferror_unlocked (FILE *__stream) __THROW __wur
> > > __nonnull 
> > > ((1));
> > >   #endif
> > > @@ -864,12 +869,12 @@ extern void perror (const char *__s) __COLD;
> > >   #ifdef    __USE_POSIX
> > >   /* Return the system file descriptor for STREAM.  */
> > > -extern int fileno (FILE *__stream) __THROW __wur;
> > > +extern int fileno (FILE *__stream) __THROW __wur __nonnull ((1));
> > >   #endif /* Use POSIX.  */
> > >   #ifdef __USE_MISC
> > >   /* Faster version when locking is not required.  */
> > > -extern int fileno_unlocked (FILE *__stream) __THROW __wur;
> > > +extern int fileno_unlocked (FILE *__stream) __THROW __wur
> > > __nonnull 
> > > ((1));
> > >   #endif
> > > @@ -878,7 +883,7 @@ extern int fileno_unlocked (FILE *__stream) 
> > > __THROW __wur;
> > >      This function is a possible cancellation point and therefore
> > > not
> > >      marked with __THROW.  */
> > > -extern int pclose (FILE *__stream);
> > > +extern int pclose (FILE *__stream) __nonnull ((1));
> > 
> > You didn't patch fclose(3).  Any reason?  I guess it's similarly UB
> > to
> > call fclose(NULL).
> > 
> > Cheers,
> > Alex
> > 
> > >   /* Create a new stream connected to a pipe running the given
> > > command.
> > > @@ -922,14 +927,14 @@ extern int obstack_vprintf (struct obstack 
> > > *__restrict __obstack,
> > >   /* These are defined in POSIX.1:1996.  */
> > >   /* Acquire ownership of STREAM.  */
> > > -extern void flockfile (FILE *__stream) __THROW;
> > > +extern void flockfile (FILE *__stream) __THROW __nonnull ((1));
> > >   /* Try to acquire ownership of STREAM but do not block if it is
> > > not
> > >      possible.  */
> > > -extern int ftrylockfile (FILE *__stream) __THROW __wur;
> > > +extern int ftrylockfile (FILE *__stream) __THROW __wur __nonnull
> > > ((1));
> > >   /* Relinquish the ownership granted for STREAM.  */
> > > -extern void funlockfile (FILE *__stream) __THROW;
> > > +extern void funlockfile (FILE *__stream) __THROW __nonnull ((1));
> > >   #endif /* POSIX */
> > >   #if defined __USE_XOPEN && !defined __USE_XOPEN2K && !defined
> > > __USE_GNU
> > 
> 
> -- 
> <http://www.alejandro-colomar.es/>
> GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
>
Xi Ruoyao May 19, 2023, 5:21 a.m. UTC | #5 
On Fri, 2023-05-19 at 12:13 +0800, Xi Ruoyao wrote:
> On Thu, 2023-05-18 at 20:29 +0200, Alex Colomar wrote:
> > On 5/18/23 20:06, Alex Colomar wrote:
> > > Hi Xi!
> > > 
> > > On 5/18/23 19:25, Xi Ruoyao via Libc-alpha wrote:
> > > > During the review of a GCC analyzer test case, we found most stdio
> > > > functions accepting a FILE * argument expect it to be nonnull and
> > > > just
> > > > segfault when the argument is NULL.  Add nonnull attribute for
> > > > them.
> > > > 
> > > > setbuf is well defined when __stream is NULL so it's not touched.
> > 
> > Is this true?  I couldn't find anything about it in either setbuf(3),
> > setbuf(3POSIX), or info libc.
> > 
> > The buffer can be NULL, but the stream?
> 
> I guess when I took the note on the paper I misspelled "fflush" as
> "setbuf" for some reason (maybe lack of coffee :( ).
> 
> I'll retest all the changes in one hour or two hours.

I've sent v2 with the mistakes in fflush, fflush_unlocked, and setbuf
fixed.  Thanks for pointing out my mistake!

I guess I need to avoid creating a patch at 0100 AM in the future :).
Alejandro Colomar May 19, 2023, 12:40 p.m. UTC | #6 
On 5/19/23 07:21, Xi Ruoyao via Libc-alpha wrote:
> On Fri, 2023-05-19 at 12:13 +0800, Xi Ruoyao wrote:
>> On Thu, 2023-05-18 at 20:29 +0200, Alex Colomar wrote:

[...]

>>
>> I guess when I took the note on the paper I misspelled "fflush" as
>> "setbuf" for some reason (maybe lack of coffee :( ).
>>
>> I'll retest all the changes in one hour or two hours.
> 
> I've sent v2 with the mistakes in fflush, fflush_unlocked, and setbuf
> fixed.  Thanks for pointing out my mistake! >
> I guess I need to avoid creating a patch at 0100 AM in the future :).

I'm glad to help :-)

How about fclose(3)?  Was it left out on purpose, or was it a sleep(0)
issue too? :p

On 5/18/23 20:06, Alex Colomar via Libc-alpha wrote:
 >> @@ -878,7 +883,7 @@ extern int fileno_unlocked (FILE *__stream)
 >> __THROW __wur;
 >>      This function is a possible cancellation point and therefore not
 >>      marked with __THROW.  */
 >> -extern int pclose (FILE *__stream);
 >> +extern int pclose (FILE *__stream) __nonnull ((1));
 >
 > You didn't patch fclose(3).  Any reason?  I guess it's similarly UB to
 > call fclose(NULL).

Cheers,
Alex

> 

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
Xi Ruoyao May 19, 2023, 1:03 p.m. UTC | #7 
On Fri, 2023-05-19 at 14:40 +0200, Alejandro Colomar wrote:
> On 5/19/23 07:21, Xi Ruoyao via Libc-alpha wrote:
> > On Fri, 2023-05-19 at 12:13 +0800, Xi Ruoyao wrote:
> > > On Thu, 2023-05-18 at 20:29 +0200, Alex Colomar wrote:
> 
> [...]
> 
> > > 
> > > I guess when I took the note on the paper I misspelled "fflush" as
> > > "setbuf" for some reason (maybe lack of coffee :( ).
> > > 
> > > I'll retest all the changes in one hour or two hours.
> > 
> > I've sent v2 with the mistakes in fflush, fflush_unlocked, and
> > setbuf
> > fixed.  Thanks for pointing out my mistake! >
> > I guess I need to avoid creating a patch at 0100 AM in the future
> > :).
> 
> I'm glad to help :-)
> 
> How about fclose(3)?  Was it left out on purpose, or was it a sleep(0)
> issue too? :p

It was done at
https://sourceware.org/git/?p=glibc.git;a=commit;h=71d9e0fe

Actually fclose is the start point of this __nonnull addition:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109570

But unfortunately in Apr I didn't have enough time to check the entire
stdio.h function by function.  Then in the following discussion for the
GCC PR we found ferror should have __nonnull too and I decided to check
all stdio.h functions this time.

> 
> On 5/18/23 20:06, Alex Colomar via Libc-alpha wrote:
>  >> @@ -878,7 +883,7 @@ extern int fileno_unlocked (FILE *__stream)
>  >> __THROW __wur;
>  >>      This function is a possible cancellation point and therefore
> not
>  >>      marked with __THROW.  */
>  >> -extern int pclose (FILE *__stream);
>  >> +extern int pclose (FILE *__stream) __nonnull ((1));
>  >
>  > You didn't patch fclose(3).  Any reason?  I guess it's similarly UB
> to
>  > call fclose(NULL).
> 
> Cheers,
> Alex
> 
> > 
> 
> -- 
> <http://www.alejandro-colomar.es/>
> GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
>
Alejandro Colomar May 19, 2023, 1:07 p.m. UTC | #8 
On 5/19/23 15:03, Xi Ruoyao wrote:
> On Fri, 2023-05-19 at 14:40 +0200, Alejandro Colomar wrote:
>> On 5/19/23 07:21, Xi Ruoyao via Libc-alpha wrote:
>>> On Fri, 2023-05-19 at 12:13 +0800, Xi Ruoyao wrote:
>>>> On Thu, 2023-05-18 at 20:29 +0200, Alex Colomar wrote:
>>
>> [...]
>>
>>>>
>>>> I guess when I took the note on the paper I misspelled "fflush" as
>>>> "setbuf" for some reason (maybe lack of coffee :( ).
>>>>
>>>> I'll retest all the changes in one hour or two hours.
>>>
>>> I've sent v2 with the mistakes in fflush, fflush_unlocked, and
>>> setbuf
>>> fixed.  Thanks for pointing out my mistake! >
>>> I guess I need to avoid creating a patch at 0100 AM in the future
>>> :).
>>
>> I'm glad to help :-)
>>
>> How about fclose(3)?  Was it left out on purpose, or was it a sleep(0)
>> issue too? :p
> 
> It was done at
> https://sourceware.org/git/?p=glibc.git;a=commit;h=71d9e0fe
> 
> Actually fclose is the start point of this __nonnull addition:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109570
> 
> But unfortunately in Apr I didn't have enough time to check the entire
> stdio.h function by function.  Then in the following discussion for the
> GCC PR we found ferror should have __nonnull too and I decided to check
> all stdio.h functions this time.

Good to know!  Makes sense.  LGTM then.

Acked-by: Alejandro Colomar <alx@kernel.org>

Cheers,
Alex


-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
diff mbox series

Patch

diff --git a/libio/stdio.h b/libio/stdio.h
index 4cf9f1c012..ae3d7295d4 100644
--- a/libio/stdio.h
+++ b/libio/stdio.h
@@ -232,7 +232,7 @@  extern char *tempnam (const char *__dir, const char *__pfx)
 
    This function is a possible cancellation point and therefore not
    marked with __THROW.  */
-extern int fflush (FILE *__stream);
+extern int fflush (FILE *__stream) __nonnull ((1));
 
 #ifdef __USE_MISC
 /* Faster versions when locking is not required.
@@ -241,7 +241,7 @@  extern int fflush (FILE *__stream);
    cancellation point.  But due to similarity with an POSIX interface
    or due to the implementation it is a cancellation point and
    therefore not marked with __THROW.  */
-extern int fflush_unlocked (FILE *__stream);
+extern int fflush_unlocked (FILE *__stream) __nonnull ((1));
 #endif
 
 #ifdef __USE_GNU
@@ -278,7 +278,7 @@  extern FILE *__REDIRECT (fopen, (const char *__restrict __filename,
 extern FILE *__REDIRECT (freopen, (const char *__restrict __filename,
 				   const char *__restrict __modes,
 				   FILE *__restrict __stream), freopen64)
-  __wur;
+  __wur __nonnull ((3));
 # else
 #  define fopen fopen64
 #  define freopen freopen64
@@ -335,16 +335,16 @@  extern void setbuf (FILE *__restrict __stream, char *__restrict __buf) __THROW;
    If BUF is not NULL, use N bytes of it for buffering;
    else allocate an internal buffer N bytes long.  */
 extern int setvbuf (FILE *__restrict __stream, char *__restrict __buf,
-		    int __modes, size_t __n) __THROW;
+		    int __modes, size_t __n) __THROW __nonnull ((1));
 
 #ifdef	__USE_MISC
 /* If BUF is NULL, make STREAM unbuffered.
    Else make it use SIZE bytes of BUF for buffering.  */
 extern void setbuffer (FILE *__restrict __stream, char *__restrict __buf,
-		       size_t __size) __THROW;
+		       size_t __size) __THROW __nonnull ((1));
 
 /* Make STREAM line-buffered.  */
-extern void setlinebuf (FILE *__stream) __THROW;
+extern void setlinebuf (FILE *__stream) __THROW __nonnull ((1));
 #endif
 
 
@@ -418,7 +418,7 @@  extern int dprintf (int __fd, const char *__restrict __fmt, ...)
    This function is a possible cancellation point and therefore not
    marked with __THROW.  */
 extern int fscanf (FILE *__restrict __stream,
-		   const char *__restrict __format, ...) __wur;
+		   const char *__restrict __format, ...) __wur __nonnull ((1));
 /* Read formatted input from stdin.
 
    This function is a possible cancellation point and therefore not
@@ -439,7 +439,7 @@  extern int sscanf (const char *__restrict __s,
 #  ifdef __REDIRECT
 extern int __REDIRECT (fscanf, (FILE *__restrict __stream,
 				const char *__restrict __format, ...),
-		       __isoc23_fscanf) __wur;
+		       __isoc23_fscanf) __wur __nonnull ((1));
 extern int __REDIRECT (scanf, (const char *__restrict __format, ...),
 		       __isoc23_scanf) __wur;
 extern int __REDIRECT_NTH (sscanf, (const char *__restrict __s,
@@ -447,7 +447,7 @@  extern int __REDIRECT_NTH (sscanf, (const char *__restrict __s,
 			   __isoc23_sscanf);
 #  else
 extern int __isoc23_fscanf (FILE *__restrict __stream,
-			    const char *__restrict __format, ...) __wur;
+			    const char *__restrict __format, ...) __wur __nonnull ((1));
 extern int __isoc23_scanf (const char *__restrict __format, ...) __wur;
 extern int __isoc23_sscanf (const char *__restrict __s,
 			    const char *__restrict __format, ...) __THROW;
@@ -459,7 +459,7 @@  extern int __isoc23_sscanf (const char *__restrict __s,
 #  ifdef __REDIRECT
 extern int __REDIRECT (fscanf, (FILE *__restrict __stream,
 				const char *__restrict __format, ...),
-		       __isoc99_fscanf) __wur;
+		       __isoc99_fscanf) __wur __nonnull ((1));
 extern int __REDIRECT (scanf, (const char *__restrict __format, ...),
 		       __isoc99_scanf) __wur;
 extern int __REDIRECT_NTH (sscanf, (const char *__restrict __s,
@@ -467,7 +467,7 @@  extern int __REDIRECT_NTH (sscanf, (const char *__restrict __s,
 			   __isoc99_sscanf);
 #  else
 extern int __isoc99_fscanf (FILE *__restrict __stream,
-			    const char *__restrict __format, ...) __wur;
+			    const char *__restrict __format, ...) __wur __nonnull ((1));
 extern int __isoc99_scanf (const char *__restrict __format, ...) __wur;
 extern int __isoc99_sscanf (const char *__restrict __s,
 			    const char *__restrict __format, ...) __THROW;
@@ -485,7 +485,7 @@  extern int __isoc99_sscanf (const char *__restrict __s,
    marked with __THROW.  */
 extern int vfscanf (FILE *__restrict __s, const char *__restrict __format,
 		    __gnuc_va_list __arg)
-     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur;
+     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur __nonnull ((1));
 
 /* Read formatted input from stdin into argument list ARG.
 
@@ -508,7 +508,7 @@  extern int __REDIRECT (vfscanf,
 		       (FILE *__restrict __s,
 			const char *__restrict __format, __gnuc_va_list __arg),
 		       __isoc23_vfscanf)
-     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur;
+     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur __nonnull ((1));
 extern int __REDIRECT (vscanf, (const char *__restrict __format,
 				__gnuc_va_list __arg), __isoc23_vscanf)
      __attribute__ ((__format__ (__scanf__, 1, 0))) __wur;
@@ -520,7 +520,7 @@  extern int __REDIRECT_NTH (vsscanf,
 #   elif !defined __REDIRECT
 extern int __isoc23_vfscanf (FILE *__restrict __s,
 			     const char *__restrict __format,
-			     __gnuc_va_list __arg) __wur;
+			     __gnuc_va_list __arg) __wur __nonnull ((1));
 extern int __isoc23_vscanf (const char *__restrict __format,
 			    __gnuc_va_list __arg) __wur;
 extern int __isoc23_vsscanf (const char *__restrict __s,
@@ -537,7 +537,7 @@  extern int __REDIRECT (vfscanf,
 		       (FILE *__restrict __s,
 			const char *__restrict __format, __gnuc_va_list __arg),
 		       __isoc99_vfscanf)
-     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur;
+     __attribute__ ((__format__ (__scanf__, 2, 0))) __wur __nonnull ((1));
 extern int __REDIRECT (vscanf, (const char *__restrict __format,
 				__gnuc_va_list __arg), __isoc99_vscanf)
      __attribute__ ((__format__ (__scanf__, 1, 0))) __wur;
@@ -549,7 +549,7 @@  extern int __REDIRECT_NTH (vsscanf,
 #   elif !defined __REDIRECT
 extern int __isoc99_vfscanf (FILE *__restrict __s,
 			     const char *__restrict __format,
-			     __gnuc_va_list __arg) __wur;
+			     __gnuc_va_list __arg) __wur __nonnull ((1));
 extern int __isoc99_vscanf (const char *__restrict __format,
 			    __gnuc_va_list __arg) __wur;
 extern int __isoc99_vsscanf (const char *__restrict __s,
@@ -568,8 +568,8 @@  extern int __isoc99_vsscanf (const char *__restrict __s,
 
    These functions are possible cancellation points and therefore not
    marked with __THROW.  */
-extern int fgetc (FILE *__stream);
-extern int getc (FILE *__stream);
+extern int fgetc (FILE *__stream) __nonnull ((1));
+extern int getc (FILE *__stream) __nonnull ((1));
 
 /* Read a character from stdin.
 
@@ -582,7 +582,7 @@  extern int getchar (void);
 
    These functions are possible cancellation points and therefore not
    marked with __THROW.  */
-extern int getc_unlocked (FILE *__stream);
+extern int getc_unlocked (FILE *__stream) __nonnull ((1));
 extern int getchar_unlocked (void);
 #endif /* Use POSIX.  */
 
@@ -593,7 +593,7 @@  extern int getchar_unlocked (void);
    cancellation point.  But due to similarity with an POSIX interface
    or due to the implementation it is a cancellation point and
    therefore not marked with __THROW.  */
-extern int fgetc_unlocked (FILE *__stream);
+extern int fgetc_unlocked (FILE *__stream) __nonnull ((1));
 #endif /* Use MISC.  */
 
 
@@ -604,8 +604,8 @@  extern int fgetc_unlocked (FILE *__stream);
 
    These functions is a possible cancellation point and therefore not
    marked with __THROW.  */
-extern int fputc (int __c, FILE *__stream);
-extern int putc (int __c, FILE *__stream);
+extern int fputc (int __c, FILE *__stream) __nonnull ((2));
+extern int putc (int __c, FILE *__stream) __nonnull ((2));
 
 /* Write a character to stdout.
 
@@ -620,7 +620,7 @@  extern int putchar (int __c);
    cancellation point.  But due to similarity with an POSIX interface
    or due to the implementation it is a cancellation point and
    therefore not marked with __THROW.  */
-extern int fputc_unlocked (int __c, FILE *__stream);
+extern int fputc_unlocked (int __c, FILE *__stream) __nonnull ((2));
 #endif /* Use MISC.  */
 
 #ifdef __USE_POSIX199506
@@ -628,7 +628,7 @@  extern int fputc_unlocked (int __c, FILE *__stream);
 
    These functions are possible cancellation points and therefore not
    marked with __THROW.  */
-extern int putc_unlocked (int __c, FILE *__stream);
+extern int putc_unlocked (int __c, FILE *__stream) __nonnull ((2));
 extern int putchar_unlocked (int __c);
 #endif /* Use POSIX.  */
 
@@ -636,10 +636,10 @@  extern int putchar_unlocked (int __c);
 #if defined __USE_MISC \
     || (defined __USE_XOPEN && !defined __USE_XOPEN2K)
 /* Get a word (int) from STREAM.  */
-extern int getw (FILE *__stream);
+extern int getw (FILE *__stream) __nonnull ((1));
 
 /* Write a word (int) to STREAM.  */
-extern int putw (int __w, FILE *__stream);
+extern int putw (int __w, FILE *__stream) __nonnull ((2));
 #endif
 
 
@@ -689,10 +689,10 @@  extern char *fgets_unlocked (char *__restrict __s, int __n,
    therefore not marked with __THROW.  */
 extern __ssize_t __getdelim (char **__restrict __lineptr,
                              size_t *__restrict __n, int __delimiter,
-                             FILE *__restrict __stream) __wur;
+                             FILE *__restrict __stream) __wur __nonnull ((4));
 extern __ssize_t getdelim (char **__restrict __lineptr,
                            size_t *__restrict __n, int __delimiter,
-                           FILE *__restrict __stream) __wur;
+                           FILE *__restrict __stream) __wur __nonnull ((4));
 
 /* Like `getdelim', but reads up to a newline.
 
@@ -702,7 +702,7 @@  extern __ssize_t getdelim (char **__restrict __lineptr,
    therefore not marked with __THROW.  */
 extern __ssize_t getline (char **__restrict __lineptr,
                           size_t *__restrict __n,
-                          FILE *__restrict __stream) __wur;
+                          FILE *__restrict __stream) __wur __nonnull ((3));
 #endif
 
 
@@ -723,7 +723,7 @@  extern int puts (const char *__s);
 
    This function is a possible cancellation point and therefore not
    marked with __THROW.  */
-extern int ungetc (int __c, FILE *__stream);
+extern int ungetc (int __c, FILE *__stream) __nonnull ((2));
 
 
 /* Read chunks of generic data from STREAM.
@@ -768,17 +768,17 @@  extern size_t fwrite_unlocked (const void *__restrict __ptr, size_t __size,
 
    This function is a possible cancellation point and therefore not
    marked with __THROW.  */
-extern int fseek (FILE *__stream, long int __off, int __whence);
+extern int fseek (FILE *__stream, long int __off, int __whence) __nonnull ((1));
 /* Return the current position of STREAM.
 
    This function is a possible cancellation point and therefore not
    marked with __THROW.  */
-extern long int ftell (FILE *__stream) __wur;
+extern long int ftell (FILE *__stream) __wur __nonnull ((1));
 /* Rewind to the beginning of STREAM.
 
    This function is a possible cancellation point and therefore not
    marked with __THROW.  */
-extern void rewind (FILE *__stream);
+extern void rewind (FILE *__stream) __nonnull ((1));
 
 /* The Single Unix Specification, Version 2, specifies an alternative,
    more adequate interface for the two functions above which deal with
@@ -791,18 +791,19 @@  extern void rewind (FILE *__stream);
 
    This function is a possible cancellation point and therefore not
    marked with __THROW.  */
-extern int fseeko (FILE *__stream, __off_t __off, int __whence);
+extern int fseeko (FILE *__stream, __off_t __off, int __whence) __nonnull ((1));
 /* Return the current position of STREAM.
 
    This function is a possible cancellation point and therefore not
    marked with __THROW.  */
-extern __off_t ftello (FILE *__stream) __wur;
+extern __off_t ftello (FILE *__stream) __wur __nonnull ((1));
 # else
 #  ifdef __REDIRECT
 extern int __REDIRECT (fseeko,
 		       (FILE *__stream, __off64_t __off, int __whence),
-		       fseeko64);
-extern __off64_t __REDIRECT (ftello, (FILE *__stream), ftello64);
+		       fseeko64) __nonnull ((1));
+extern __off64_t __REDIRECT (ftello, (FILE *__stream), ftello64)
+  __nonnull ((1));
 #  else
 #   define fseeko fseeko64
 #   define ftello ftello64
@@ -815,18 +816,20 @@  extern __off64_t __REDIRECT (ftello, (FILE *__stream), ftello64);
 
    This function is a possible cancellation point and therefore not
    marked with __THROW.  */
-extern int fgetpos (FILE *__restrict __stream, fpos_t *__restrict __pos);
+extern int fgetpos (FILE *__restrict __stream, fpos_t *__restrict __pos)
+  __nonnull ((1));
 /* Set STREAM's position.
 
    This function is a possible cancellation point and therefore not
    marked with __THROW.  */
-extern int fsetpos (FILE *__stream, const fpos_t *__pos);
+extern int fsetpos (FILE *__stream, const fpos_t *__pos) __nonnull ((1));
 #else
 # ifdef __REDIRECT
 extern int __REDIRECT (fgetpos, (FILE *__restrict __stream,
-				 fpos_t *__restrict __pos), fgetpos64);
+				 fpos_t *__restrict __pos), fgetpos64) __nonnull ((1));
 extern int __REDIRECT (fsetpos,
-		       (FILE *__stream, const fpos_t *__pos), fsetpos64);
+		       (FILE *__stream, const fpos_t *__pos), fsetpos64)
+  __nonnull ((1));
 # else
 #  define fgetpos fgetpos64
 #  define fsetpos fsetpos64
@@ -834,24 +837,26 @@  extern int __REDIRECT (fsetpos,
 #endif
 
 #ifdef __USE_LARGEFILE64
-extern int fseeko64 (FILE *__stream, __off64_t __off, int __whence);
-extern __off64_t ftello64 (FILE *__stream) __wur;
-extern int fgetpos64 (FILE *__restrict __stream, fpos64_t *__restrict __pos);
-extern int fsetpos64 (FILE *__stream, const fpos64_t *__pos);
+extern int fseeko64 (FILE *__stream, __off64_t __off, int __whence)
+  __nonnull ((1));
+extern __off64_t ftello64 (FILE *__stream) __wur __nonnull ((1));
+extern int fgetpos64 (FILE *__restrict __stream, fpos64_t *__restrict __pos)
+  __nonnull ((1));
+extern int fsetpos64 (FILE *__stream, const fpos64_t *__pos) __nonnull ((1));
 #endif
 
 /* Clear the error and EOF indicators for STREAM.  */
-extern void clearerr (FILE *__stream) __THROW;
+extern void clearerr (FILE *__stream) __THROW __nonnull ((1));
 /* Return the EOF indicator for STREAM.  */
-extern int feof (FILE *__stream) __THROW __wur;
+extern int feof (FILE *__stream) __THROW __wur __nonnull ((1));
 /* Return the error indicator for STREAM.  */
-extern int ferror (FILE *__stream) __THROW __wur;
+extern int ferror (FILE *__stream) __THROW __wur __nonnull ((1));
 
 #ifdef __USE_MISC
 /* Faster versions when locking is not required.  */
-extern void clearerr_unlocked (FILE *__stream) __THROW;
-extern int feof_unlocked (FILE *__stream) __THROW __wur;
-extern int ferror_unlocked (FILE *__stream) __THROW __wur;
+extern void clearerr_unlocked (FILE *__stream) __THROW __nonnull ((1));
+extern int feof_unlocked (FILE *__stream) __THROW __wur __nonnull ((1));
+extern int ferror_unlocked (FILE *__stream) __THROW __wur __nonnull ((1));
 #endif
 
 
@@ -864,12 +869,12 @@  extern void perror (const char *__s) __COLD;
 
 #ifdef	__USE_POSIX
 /* Return the system file descriptor for STREAM.  */
-extern int fileno (FILE *__stream) __THROW __wur;
+extern int fileno (FILE *__stream) __THROW __wur __nonnull ((1));
 #endif /* Use POSIX.  */
 
 #ifdef __USE_MISC
 /* Faster version when locking is not required.  */
-extern int fileno_unlocked (FILE *__stream) __THROW __wur;
+extern int fileno_unlocked (FILE *__stream) __THROW __wur __nonnull ((1));
 #endif
 
 
@@ -878,7 +883,7 @@  extern int fileno_unlocked (FILE *__stream) __THROW __wur;
 
    This function is a possible cancellation point and therefore not
    marked with __THROW.  */
-extern int pclose (FILE *__stream);
+extern int pclose (FILE *__stream) __nonnull ((1));
 
 /* Create a new stream connected to a pipe running the given command.
 
@@ -922,14 +927,14 @@  extern int obstack_vprintf (struct obstack *__restrict __obstack,
 /* These are defined in POSIX.1:1996.  */
 
 /* Acquire ownership of STREAM.  */
-extern void flockfile (FILE *__stream) __THROW;
+extern void flockfile (FILE *__stream) __THROW __nonnull ((1));
 
 /* Try to acquire ownership of STREAM but do not block if it is not
    possible.  */
-extern int ftrylockfile (FILE *__stream) __THROW __wur;
+extern int ftrylockfile (FILE *__stream) __THROW __wur __nonnull ((1));
 
 /* Relinquish the ownership granted for STREAM.  */
-extern void funlockfile (FILE *__stream) __THROW;
+extern void funlockfile (FILE *__stream) __THROW __nonnull ((1));
 #endif /* POSIX */
 
 #if defined __USE_XOPEN && !defined __USE_XOPEN2K && !defined __USE_GNU