public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] improve out-of-bounds checking with GCC 10 attribute access [BZ #25219]
@ 2020-04-30 22:12 Martin Sebor
  2020-04-30 22:37 ` Dmitry V. Levin
  2020-05-01  2:42 ` DJ Delorie
  0 siblings, 2 replies; 11+ messages in thread
From: Martin Sebor @ 2020-04-30 22:12 UTC (permalink / raw)
  To: GNU C Library

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

To extend the detection of out-of-bounds accesses by GCC built-in
functions like memcpy and strcpy to user-defined functions, GCC 10
introduces a new function attribute named access with this syntax:

   access (access-mode, ref-index [, size-index])

Besides specifying the form of the access (read-only, read-write, and
write-only), it associates the pointer parameter with a parameter that
indicates the size (in elements) of the array the pointer points to.

The attribute lets GCC diagnose not just the subset of out-of-bounds
accesses involving constant offsets and sizes but also those where
the offsets into the buffers or their sizes aren't constant (they may
be in some range whose lower or upper bound makes the access decidedly
invalid).  The constant offset/size limitation affects _FORTIFY_SOURCE
but not GCC warnings.

The attached patch adds the new access attribute to the subset of Glibc
declarations that are already covered by _FORTIFY_SOURCE and that don't
have corresponding GCC built-in equivalents.

Besides building Glibc and running the tests I've tested the patch
by creating calls to the modified functions, one valid and one with
an out-of-bounds access, and checking for the expected warnings.
The test is attached for reference (I don't know of an easy way
to add it to the Glibc test suite).

Martin

PS A further extension (not in this patch) is also associating const
pointer parameters with attribute access.  This lets GCC diagnose
out-of-bounds reads and in the future may also let it diagnose
uninitialized reads through such pointers.

[-- Attachment #2: glibc-25219.diff --]
[-- Type: text/x-patch, Size: 28399 bytes --]

BZ #25219 - improve out-of-bounds checking with GCC 10 attribute access

ChangeLog:

	* misc/sys/cdefs.h (__attr_access): New macro.
	* libio/bits/stdio2.h (__fgets_chk): Add attribute access.
	(__fgets_unlocked_chk): Same.
	* posix/bits/unistd.h (__read_chk, __read_alias): Same.
	(__pread_chk, __pread64_chk, __pread_alias, __pread64_alias): Same.
	(__readlink_chk, __readlink_alias): Same.
	(__readlinkat_chk, __readlinkat_alias): Same.
	(__getcwd_chk, __getcwd_alias): Same.
	(__confstr_chk,  __confstr_alias): Same.
	(__getgroups_chk, __getgroups_alias): Same.
	(__ttyname_r_chk, __ttyname_r_alias): Same.
	(__getlogin_r_chk, __getlogin_r_alias): Same.
	(__gethostname_chk, __gethostname_alias): Same.
	(__getdomainname_chk, __getdomainname_alias): Same.
	* posix/unistd.h (read, write, pread, pwrite): Same.
	(pread64, pwrite64): Same.
	(getcwd, confstr, getgrtoups, ttyname_r): Same.
	(readlinkat, getlogin_r, gethostname, sethostname): Same.
	(getdomainname, setdomainname, swab, genentropy): Same.
	* stdlib/bits/stdlib.h (__ptsname_r_chk): Same
	 (__mbstowcs_chk, __mbstowcs_alias): Same
	 (__wcstombs_chk, __wcstombs_alias): Same
	* stdlib/stdlib.h (mbstowcs, wcstombs): Same
	* string/bits/string_fortified.h (explicit_bzero_chk): Same
	* string/string.h (meccmpy, memrrchr, strxfrm, strcoll_l): Same.
	(memmem, __xpg_strerror_r, strerror_r): Same.
	(explicit_bzero, memfrob): Same.

diff --git a/libio/bits/stdio2.h b/libio/bits/stdio2.h
index 60bc81735e..eff1c1fc09 100644
--- a/libio/bits/stdio2.h
+++ b/libio/bits/stdio2.h
@@ -241,7 +241,8 @@ gets (char *__str)
 #endif
 
 extern char *__fgets_chk (char *__restrict __s, size_t __size, int __n,
-			  FILE *__restrict __stream) __wur;
+			  FILE *__restrict __stream)
+  __wur __attr_access ((__read_only__, 1, 2));
 extern char *__REDIRECT (__fgets_alias,
 			 (char *__restrict __s, int __n,
 			  FILE *__restrict __stream), fgets) __wur;
@@ -299,7 +300,8 @@ fread (void *__restrict __ptr, size_t __size, size_t __n,
 
 #ifdef __USE_GNU
 extern char *__fgets_unlocked_chk (char *__restrict __s, size_t __size,
-				   int __n, FILE *__restrict __stream) __wur;
+				   int __n, FILE *__restrict __stream)
+  __wur __attr_access ((__write_only__, 1, 2));
 extern char *__REDIRECT (__fgets_unlocked_alias,
 			 (char *__restrict __s, int __n,
 			  FILE *__restrict __stream), fgets_unlocked) __wur;
diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h
index 9fa371ab86..4365a39caf 100644
--- a/misc/sys/cdefs.h
+++ b/misc/sys/cdefs.h
@@ -548,4 +548,14 @@ _Static_assert (0, "IEEE 128-bits long double requires redirection on this platf
 # define __HAVE_GENERIC_SELECTION 0
 #endif
 
+#if __GNUC_PREREQ (10, 0)
+/* Denotes a function pointer argument ref-index that can be used
+   to access size-index elements of the pointed-to array according
+   to access mode:
+     access (access-mode, <ref-index> [, <size-index>])  */
+#define __attr_access(x) __attribute__ ((__access__ x))
+#else
+#  define __attr_access(x)
+#endif
+
 #endif	 /* sys/cdefs.h */
diff --git a/posix/bits/unistd.h b/posix/bits/unistd.h
index b8a8211d83..725a83eb0d 100644
--- a/posix/bits/unistd.h
+++ b/posix/bits/unistd.h
@@ -21,9 +21,11 @@
 #endif
 
 extern ssize_t __read_chk (int __fd, void *__buf, size_t __nbytes,
-			   size_t __buflen) __wur;
+			   size_t __buflen)
+  __wur __attr_access ((__write_only__, 2, 3));
 extern ssize_t __REDIRECT (__read_alias, (int __fd, void *__buf,
-					  size_t __nbytes), read) __wur;
+					  size_t __nbytes), read)
+  __wur __attr_access ((__write_only__, 2, 3));
 extern ssize_t __REDIRECT (__read_chk_warn,
 			   (int __fd, void *__buf, size_t __nbytes,
 			    size_t __buflen), __read_chk)
@@ -46,15 +48,19 @@ read (int __fd, void *__buf, size_t __nbytes)
 
 #ifdef __USE_UNIX98
 extern ssize_t __pread_chk (int __fd, void *__buf, size_t __nbytes,
-			    __off_t __offset, size_t __bufsize) __wur;
+			    __off_t __offset, size_t __bufsize)
+  __wur __attr_access ((__write_only__, 2, 3));
 extern ssize_t __pread64_chk (int __fd, void *__buf, size_t __nbytes,
-			      __off64_t __offset, size_t __bufsize) __wur;
+			      __off64_t __offset, size_t __bufsize)
+  __wur __attr_access ((__write_only__, 2, 3));
 extern ssize_t __REDIRECT (__pread_alias,
 			   (int __fd, void *__buf, size_t __nbytes,
-			    __off_t __offset), pread) __wur;
+			    __off_t __offset), pread)
+  __wur __attr_access ((__write_only__, 2, 3));
 extern ssize_t __REDIRECT (__pread64_alias,
 			   (int __fd, void *__buf, size_t __nbytes,
-			    __off64_t __offset), pread64) __wur;
+			    __off64_t __offset), pread64)
+  __wur __attr_access ((__write_only__, 2, 3));
 extern ssize_t __REDIRECT (__pread_chk_warn,
 			   (int __fd, void *__buf, size_t __nbytes,
 			    __off_t __offset, size_t __bufsize), __pread_chk)
@@ -123,11 +129,11 @@ pread64 (int __fd, void *__buf, size_t __nbytes, __off64_t __offset)
 extern ssize_t __readlink_chk (const char *__restrict __path,
 			       char *__restrict __buf, size_t __len,
 			       size_t __buflen)
-     __THROW __nonnull ((1, 2)) __wur;
+     __THROW __nonnull ((1, 2)) __wur __attr_access ((__write_only__, 2, 3));
 extern ssize_t __REDIRECT_NTH (__readlink_alias,
 			       (const char *__restrict __path,
 				char *__restrict __buf, size_t __len), readlink)
-     __nonnull ((1, 2)) __wur;
+     __nonnull ((1, 2)) __wur __attr_access ((__write_only__, 2, 3));
 extern ssize_t __REDIRECT_NTH (__readlink_chk_warn,
 			       (const char *__restrict __path,
 				char *__restrict __buf, size_t __len,
@@ -155,12 +161,12 @@ __NTH (readlink (const char *__restrict __path, char *__restrict __buf,
 extern ssize_t __readlinkat_chk (int __fd, const char *__restrict __path,
 				 char *__restrict __buf, size_t __len,
 				 size_t __buflen)
-     __THROW __nonnull ((2, 3)) __wur;
+     __THROW __nonnull ((2, 3)) __wur __attr_access ((__write_only__, 3, 4));
 extern ssize_t __REDIRECT_NTH (__readlinkat_alias,
 			       (int __fd, const char *__restrict __path,
 				char *__restrict __buf, size_t __len),
 			       readlinkat)
-     __nonnull ((2, 3)) __wur;
+     __nonnull ((2, 3)) __wur __attr_access ((__write_only__, 3, 4));
 extern ssize_t __REDIRECT_NTH (__readlinkat_chk_warn,
 			       (int __fd, const char *__restrict __path,
 				char *__restrict __buf, size_t __len,
@@ -187,9 +193,10 @@ __NTH (readlinkat (int __fd, const char *__restrict __path,
 #endif
 
 extern char *__getcwd_chk (char *__buf, size_t __size, size_t __buflen)
-     __THROW __wur;
+     __THROW __wur __attr_access ((__write_only__, 1, 2));
 extern char *__REDIRECT_NTH (__getcwd_alias,
-			     (char *__buf, size_t __size), getcwd) __wur;
+			     (char *__buf, size_t __size), getcwd)
+  __wur __attr_access ((__write_only__, 1, 2));
 extern char *__REDIRECT_NTH (__getcwd_chk_warn,
 			     (char *__buf, size_t __size, size_t __buflen),
 			     __getcwd_chk)
@@ -212,7 +219,7 @@ __NTH (getcwd (char *__buf, size_t __size))
 
 #if defined __USE_MISC || defined __USE_XOPEN_EXTENDED
 extern char *__getwd_chk (char *__buf, size_t buflen)
-     __THROW __nonnull ((1)) __wur;
+     __THROW __nonnull ((1)) __wur __attr_access ((__write_only__, 1, 2));
 extern char *__REDIRECT_NTH (__getwd_warn, (char *__buf), getwd)
      __nonnull ((1)) __wur __warnattr ("please use getcwd instead, as getwd "
 				       "doesn't specify buffer size");
@@ -227,9 +234,11 @@ __NTH (getwd (char *__buf))
 #endif
 
 extern size_t __confstr_chk (int __name, char *__buf, size_t __len,
-			     size_t __buflen) __THROW;
+			     size_t __buflen) __THROW
+  __attr_access ((__write_only__, 2, 3));
 extern size_t __REDIRECT_NTH (__confstr_alias, (int __name, char *__buf,
-						size_t __len), confstr);
+						size_t __len), confstr)
+   __attr_access ((__write_only__, 2, 3));
 extern size_t __REDIRECT_NTH (__confstr_chk_warn,
 			      (int __name, char *__buf, size_t __len,
 			       size_t __buflen), __confstr_chk)
@@ -252,9 +261,9 @@ __NTH (confstr (int __name, char *__buf, size_t __len))
 
 
 extern int __getgroups_chk (int __size, __gid_t __list[], size_t __listlen)
-     __THROW __wur;
+  __THROW __wur __attr_access ((__write_only__, 2, 1));
 extern int __REDIRECT_NTH (__getgroups_alias, (int __size, __gid_t __list[]),
-			   getgroups) __wur;
+			   getgroups) __wur __attr_access ((__write_only__, 2, 1));
 extern int __REDIRECT_NTH (__getgroups_chk_warn,
 			   (int __size, __gid_t __list[], size_t __listlen),
 			   __getgroups_chk)
@@ -277,7 +286,8 @@ __NTH (getgroups (int __size, __gid_t __list[]))
 
 
 extern int __ttyname_r_chk (int __fd, char *__buf, size_t __buflen,
-			    size_t __nreal) __THROW __nonnull ((2));
+			    size_t __nreal) __THROW __nonnull ((2))
+   __attr_access ((__write_only__, 2, 3));
 extern int __REDIRECT_NTH (__ttyname_r_alias, (int __fd, char *__buf,
 					       size_t __buflen), ttyname_r)
      __nonnull ((2));
@@ -304,7 +314,7 @@ __NTH (ttyname_r (int __fd, char *__buf, size_t __buflen))
 
 #ifdef __USE_POSIX199506
 extern int __getlogin_r_chk (char *__buf, size_t __buflen, size_t __nreal)
-     __nonnull ((1));
+     __nonnull ((1)) __attr_access ((__write_only__, 1, 2));
 extern int __REDIRECT (__getlogin_r_alias, (char *__buf, size_t __buflen),
 		       getlogin_r) __nonnull ((1));
 extern int __REDIRECT (__getlogin_r_chk_warn,
@@ -331,9 +341,10 @@ getlogin_r (char *__buf, size_t __buflen)
 
 #if defined __USE_MISC || defined __USE_UNIX98
 extern int __gethostname_chk (char *__buf, size_t __buflen, size_t __nreal)
-     __THROW __nonnull ((1));
+     __THROW __nonnull ((1)) __attr_access ((__write_only__, 1, 2));
 extern int __REDIRECT_NTH (__gethostname_alias, (char *__buf, size_t __buflen),
-			   gethostname) __nonnull ((1));
+			   gethostname)
+  __nonnull ((1)) __attr_access ((__write_only__, 1, 2));
 extern int __REDIRECT_NTH (__gethostname_chk_warn,
 			   (char *__buf, size_t __buflen, size_t __nreal),
 			   __gethostname_chk)
@@ -358,10 +369,11 @@ __NTH (gethostname (char *__buf, size_t __buflen))
 
 #if defined __USE_MISC || (defined __USE_XOPEN && !defined __USE_UNIX98)
 extern int __getdomainname_chk (char *__buf, size_t __buflen, size_t __nreal)
-     __THROW __nonnull ((1)) __wur;
+     __THROW __nonnull ((1)) __wur __attr_access ((__write_only__, 1, 2));
 extern int __REDIRECT_NTH (__getdomainname_alias, (char *__buf,
 						   size_t __buflen),
-			   getdomainname) __nonnull ((1)) __wur;
+			   getdomainname) __nonnull ((1))
+  __wur __attr_access ((__write_only__, 1, 2));
 extern int __REDIRECT_NTH (__getdomainname_chk_warn,
 			   (char *__buf, size_t __buflen, size_t __nreal),
 			   __getdomainname_chk)
diff --git a/posix/unistd.h b/posix/unistd.h
index 0dd4200ad6..a4c831ae90 100644
--- a/posix/unistd.h
+++ b/posix/unistd.h
@@ -357,13 +357,15 @@ extern int close (int __fd);
 
    This function is a cancellation point and therefore not marked with
    __THROW.  */
-extern ssize_t read (int __fd, void *__buf, size_t __nbytes) __wur;
+extern ssize_t read (int __fd, void *__buf, size_t __nbytes) __wur
+    __attr_access ((__write_only__, 2, 3));
 
 /* Write N bytes of BUF to FD.  Return the number written, or -1.
 
    This function is a cancellation point and therefore not marked with
    __THROW.  */
-extern ssize_t write (int __fd, const void *__buf, size_t __n) __wur;
+extern ssize_t write (int __fd, const void *__buf, size_t __n) __wur
+    __attr_access ((__read_only__, 2, 3));
 
 #if defined __USE_UNIX98 || defined __USE_XOPEN2K8
 # ifndef __USE_FILE_OFFSET64
@@ -374,7 +376,8 @@ extern ssize_t write (int __fd, const void *__buf, size_t __n) __wur;
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t pread (int __fd, void *__buf, size_t __nbytes,
-		      __off_t __offset) __wur;
+		      __off_t __offset) __wur
+    __attr_access ((__write_only__, 2, 3));
 
 /* Write N bytes of BUF to FD at the given position OFFSET without
    changing the file pointer.  Return the number written, or -1.
@@ -382,15 +385,19 @@ extern ssize_t pread (int __fd, void *__buf, size_t __nbytes,
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t pwrite (int __fd, const void *__buf, size_t __n,
-		       __off_t __offset) __wur;
+		       __off_t __offset) __wur
+    __attr_access ((__read_only__, 2, 3));
+
 # else
 #  ifdef __REDIRECT
 extern ssize_t __REDIRECT (pread, (int __fd, void *__buf, size_t __nbytes,
 				   __off64_t __offset),
-			   pread64) __wur;
+			   pread64) __wur
+    __attr_access ((__write_only__, 2, 3));
 extern ssize_t __REDIRECT (pwrite, (int __fd, const void *__buf,
 				    size_t __nbytes, __off64_t __offset),
-			   pwrite64) __wur;
+			   pwrite64) __wur
+    __attr_access ((__read_only__, 2, 3));
 #  else
 #   define pread pread64
 #   define pwrite pwrite64
@@ -402,11 +409,13 @@ extern ssize_t __REDIRECT (pwrite, (int __fd, const void *__buf,
    changing the file pointer.  Return the number read, -1 for errors
    or 0 for EOF.  */
 extern ssize_t pread64 (int __fd, void *__buf, size_t __nbytes,
-			__off64_t __offset) __wur;
+			__off64_t __offset) __wur
+    __attr_access ((__write_only__, 2, 3));
 /* Write N bytes of BUF to FD at the given position OFFSET without
    changing the file pointer.  Return the number written, or -1.  */
 extern ssize_t pwrite64 (int __fd, const void *__buf, size_t __n,
-			 __off64_t __offset) __wur;
+			 __off64_t __offset) __wur
+    __attr_access ((__read_only__, 2, 3));
 # endif
 #endif
 
@@ -523,7 +532,8 @@ extern char *get_current_dir_name (void) __THROW;
    If successful, return BUF.  If not, put an error message in
    BUF and return NULL.  BUF should be at least PATH_MAX bytes long.  */
 extern char *getwd (char *__buf)
-     __THROW __nonnull ((1)) __attribute_deprecated__ __wur;
+     __THROW __nonnull ((1)) __attribute_deprecated__ __wur
+    __attr_access ((__write_only__, 1));
 #endif
 
 
@@ -620,7 +630,8 @@ extern long int sysconf (int __name) __THROW;
 
 #ifdef	__USE_POSIX2
 /* Get the value of the string-valued system variable NAME.  */
-extern size_t confstr (int __name, char *__buf, size_t __len) __THROW;
+extern size_t confstr (int __name, char *__buf, size_t __len) __THROW
+    __attribute__ ((access (__write_only__, 2, 3)));
 #endif
 
 
@@ -686,8 +697,8 @@ extern __gid_t getegid (void) __THROW;
 /* If SIZE is zero, return the number of supplementary groups
    the calling process is in.  Otherwise, fill in the group IDs
    of its supplementary groups in LIST and return the number written.  */
-extern int getgroups (int __size, __gid_t __list[]) __THROW __wur;
-
+extern int getgroups (int __size, __gid_t __list[]) __THROW __wur
+    __attr_access ((__write_only__, 2, 1));
 #ifdef	__USE_GNU
 /* Return nonzero iff the calling process is in group GID.  */
 extern int group_member (__gid_t __gid) __THROW;
@@ -772,7 +783,7 @@ extern char *ttyname (int __fd) __THROW;
 /* Store at most BUFLEN characters of the pathname of the terminal FD is
    open on in BUF.  Return 0 on success, otherwise an error number.  */
 extern int ttyname_r (int __fd, char *__buf, size_t __buflen)
-     __THROW __nonnull ((2)) __wur;
+     __THROW __nonnull ((2)) __wur __attr_access ((__write_only__, 2, 3));
 
 /* Return 1 if FD is a valid descriptor associated
    with a terminal, zero if not.  */
@@ -807,7 +818,8 @@ extern int symlink (const char *__from, const char *__to)
    Returns the number of characters read, or -1 for errors.  */
 extern ssize_t readlink (const char *__restrict __path,
 			 char *__restrict __buf, size_t __len)
-     __THROW __nonnull ((1, 2)) __wur;
+     __THROW __nonnull ((1, 2)) __wur __attr_access ((__write_only__, 2, 3));
+
 #endif /* Use POSIX.1-2001.  */
 
 #ifdef __USE_ATFILE
@@ -818,7 +830,7 @@ extern int symlinkat (const char *__from, int __tofd,
 /* Like readlink but a relative PATH is interpreted relative to FD.  */
 extern ssize_t readlinkat (int __fd, const char *__restrict __path,
 			   char *__restrict __buf, size_t __len)
-     __THROW __nonnull ((2, 3)) __wur;
+     __THROW __nonnull ((2, 3)) __wur __attr_access ((__read_only__, 3, 4));
 #endif
 
 /* Remove the link NAME.  */
@@ -853,7 +865,8 @@ extern char *getlogin (void);
 
    This function is a possible cancellation point and therefore not
    marked with __THROW.  */
-extern int getlogin_r (char *__name, size_t __name_len) __nonnull ((1));
+extern int getlogin_r (char *__name, size_t __name_len) __nonnull ((1))
+    __attr_access ((__write_only__, 1, 2));
 #endif
 
 #ifdef	__USE_MISC
@@ -874,7 +887,8 @@ extern int setlogin (const char *__name) __THROW __nonnull ((1));
 /* Put the name of the current host in no more than LEN bytes of NAME.
    The result is null-terminated if LEN is large enough for the full
    name and the terminator.  */
-extern int gethostname (char *__name, size_t __len) __THROW __nonnull ((1));
+extern int gethostname (char *__name, size_t __len) __THROW __nonnull ((1))
+    __attr_access ((__write_only__, 1, 2));
 #endif
 
 
@@ -882,7 +896,7 @@ extern int gethostname (char *__name, size_t __len) __THROW __nonnull ((1));
 /* Set the name of the current host to NAME, which is LEN bytes long.
    This call is restricted to the super-user.  */
 extern int sethostname (const char *__name, size_t __len)
-     __THROW __nonnull ((1)) __wur;
+     __THROW __nonnull ((1)) __wur __attr_access ((__read_only__, 1, 2));
 
 /* Set the current machine's Internet number to ID.
    This call is restricted to the super-user.  */
@@ -893,10 +907,9 @@ extern int sethostid (long int __id) __THROW __wur;
    Called just like `gethostname' and `sethostname'.
    The NIS domain name is usually the empty string when not using NIS.  */
 extern int getdomainname (char *__name, size_t __len)
-     __THROW __nonnull ((1)) __wur;
+     __THROW __nonnull ((1)) __wur __attr_access ((__write_only__, 1, 2));
 extern int setdomainname (const char *__name, size_t __len)
-     __THROW __nonnull ((1)) __wur;
-
+     __THROW __nonnull ((1)) __wur __attr_access ((__read_only__, 1, 2));
 
 /* Revoke access permissions to all processes currently communicating
    with the control terminal, and then send a SIGHUP signal to the process
@@ -1131,7 +1144,9 @@ extern char *crypt (const char *__key, const char *__salt)
    range [FROM - N + 1, FROM - 1].  If N is odd the first byte in FROM
    is without partner.  */
 extern void swab (const void *__restrict __from, void *__restrict __to,
-		  ssize_t __n) __THROW __nonnull ((1, 2));
+		  ssize_t __n) __THROW __nonnull ((1, 2))
+    __attr_access ((__read_only__, 1, 3))
+    __attr_access ((__write_only__, 2, 3));
 #endif
 
 
@@ -1158,7 +1173,8 @@ extern int pthread_atfork (void (*__prepare) (void),
 #ifdef __USE_MISC
 /* Write LENGTH bytes of randomness starting at BUFFER.  Return 0 on
    success or -1 on error.  */
-int getentropy (void *__buffer, size_t __length) __wur;
+int getentropy (void *__buffer, size_t __length) __wur
+    __attr_access ((__write_only__, 1, 2));
 #endif
 
 /* Define some macros helping to catch buffer overflows.  */
diff --git a/stdlib/bits/stdlib.h b/stdlib/bits/stdlib.h
index bfdee75073..9134d3f36b 100644
--- a/stdlib/bits/stdlib.h
+++ b/stdlib/bits/stdlib.h
@@ -50,10 +50,11 @@ __NTH (realpath (const char *__restrict __name, char *__restrict __resolved))
 
 
 extern int __ptsname_r_chk (int __fd, char *__buf, size_t __buflen,
-			    size_t __nreal) __THROW __nonnull ((2));
+			    size_t __nreal) __THROW __nonnull ((2))
+    __attr_access ((__write_only__, 2, 3));
 extern int __REDIRECT_NTH (__ptsname_r_alias, (int __fd, char *__buf,
 					       size_t __buflen), ptsname_r)
-     __nonnull ((2));
+     __nonnull ((2)) __attr_access ((__write_only__, 2, 3));
 extern int __REDIRECT_NTH (__ptsname_r_chk_warn,
 			   (int __fd, char *__buf, size_t __buflen,
 			    size_t __nreal), __ptsname_r_chk)
@@ -97,11 +98,13 @@ __NTH (wctomb (char *__s, wchar_t __wchar))
 
 extern size_t __mbstowcs_chk (wchar_t *__restrict __dst,
 			      const char *__restrict __src,
-			      size_t __len, size_t __dstlen) __THROW;
+			      size_t __len, size_t __dstlen) __THROW
+    __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
 extern size_t __REDIRECT_NTH (__mbstowcs_alias,
 			      (wchar_t *__restrict __dst,
 			       const char *__restrict __src,
-			       size_t __len), mbstowcs);
+			       size_t __len), mbstowcs)
+    __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
 extern size_t __REDIRECT_NTH (__mbstowcs_chk_warn,
 			      (wchar_t *__restrict __dst,
 			       const char *__restrict __src,
@@ -129,11 +132,13 @@ __NTH (mbstowcs (wchar_t *__restrict __dst, const char *__restrict __src,
 
 extern size_t __wcstombs_chk (char *__restrict __dst,
 			      const wchar_t *__restrict __src,
-			      size_t __len, size_t __dstlen) __THROW;
+			      size_t __len, size_t __dstlen) __THROW
+  __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
 extern size_t __REDIRECT_NTH (__wcstombs_alias,
 			      (char *__restrict __dst,
 			       const wchar_t *__restrict __src,
-			       size_t __len), wcstombs);
+			       size_t __len), wcstombs)
+  __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
 extern size_t __REDIRECT_NTH (__wcstombs_chk_warn,
 			      (char *__restrict __dst,
 			       const wchar_t *__restrict __src,
diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h
index 9b7537c545..dd779bd740 100644
--- a/stdlib/stdlib.h
+++ b/stdlib/stdlib.h
@@ -931,12 +931,13 @@ extern int wctomb (char *__s, wchar_t __wchar) __THROW;
 
 /* Convert a multibyte string to a wide char string.  */
 extern size_t mbstowcs (wchar_t *__restrict  __pwcs,
-			const char *__restrict __s, size_t __n) __THROW;
+			const char *__restrict __s, size_t __n) __THROW
+    __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
 /* Convert a wide char string to multibyte string.  */
 extern size_t wcstombs (char *__restrict __s,
 			const wchar_t *__restrict __pwcs, size_t __n)
-     __THROW;
-
+     __THROW
+  __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
 
 #ifdef __USE_MISC
 /* Determine whether the string value of RESPONSE matches the affirmation
@@ -990,7 +991,7 @@ extern char *ptsname (int __fd) __THROW __wur;
    terminal associated with the master FD is open on in BUF.
    Return 0 on success, otherwise an error number.  */
 extern int ptsname_r (int __fd, char *__buf, size_t __buflen)
-     __THROW __nonnull ((2));
+     __THROW __nonnull ((2)) __attr_access ((__write_only__, 2, 3));
 
 /* Open a master pseudo terminal and return its file descriptor.  */
 extern int getpt (void);
diff --git a/string/bits/string_fortified.h b/string/bits/string_fortified.h
index e4d07cb50c..309d0f39b2 100644
--- a/string/bits/string_fortified.h
+++ b/string/bits/string_fortified.h
@@ -75,7 +75,7 @@ __NTH (memset (void *__dest, int __ch, size_t __len))
 # include <bits/strings_fortified.h>
 
 void __explicit_bzero_chk (void *__dest, size_t __len, size_t __destlen)
-  __THROW __nonnull ((1));
+  __THROW __nonnull ((1)) __attr_access ((__write_only__, 1, 2));
 
 __fortify_function void
 __NTH (explicit_bzero (void *__dest, size_t __len))
@@ -108,7 +108,8 @@ __NTH (strncpy (char *__restrict __dest, const char *__restrict __src,
 
 /* XXX We have no corresponding builtin yet.  */
 extern char *__stpncpy_chk (char *__dest, const char *__src, size_t __n,
-			    size_t __destlen) __THROW;
+			    size_t __destlen) __THROW
+  __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
 extern char *__REDIRECT_NTH (__stpncpy_alias, (char *__dest, const char *__src,
 					       size_t __n), stpncpy);
 
diff --git a/string/string.h b/string/string.h
index a0f2860cc2..e85e06ddc4 100644
--- a/string/string.h
+++ b/string/string.h
@@ -53,7 +53,7 @@ extern void *memmove (void *__dest, const void *__src, size_t __n)
 #if defined __USE_MISC || defined __USE_XOPEN || __GLIBC_USE (ISOC2X)
 extern void *memccpy (void *__restrict __dest, const void *__restrict __src,
 		      int __c, size_t __n)
-     __THROW __nonnull ((1, 2));
+    __THROW __nonnull ((1, 2)) __attr_access ((__write_only__, 1, 4));
 #endif /* Misc || X/Open.  */
 
 
@@ -108,9 +108,11 @@ extern void *rawmemchr (const void *__s, int __c)
 /* Search N bytes of S for the final occurrence of C.  */
 # ifdef __CORRECT_ISO_CPP_STRING_H_PROTO
 extern "C++" void *memrchr (void *__s, int __c, size_t __n)
-      __THROW __asm ("memrchr") __attribute_pure__ __nonnull ((1));
+      __THROW __asm ("memrchr") __attribute_pure__ __nonnull ((1))
+      __attr_access ((__read_only__, 1, 3));
 extern "C++" const void *memrchr (const void *__s, int __c, size_t __n)
-      __THROW __asm ("memrchr") __attribute_pure__ __nonnull ((1));
+      __THROW __asm ("memrchr") __attribute_pure__ __nonnull ((1))
+      __attr_access ((__read_only__, 1, 3));
 # else
 extern void *memrchr (const void *__s, int __c, size_t __n)
       __THROW __attribute_pure__ __nonnull ((1));
@@ -146,7 +148,7 @@ extern int strcoll (const char *__s1, const char *__s2)
 /* Put a transformation of SRC into no more than N bytes of DEST.  */
 extern size_t strxfrm (char *__restrict __dest,
 		       const char *__restrict __src, size_t __n)
-     __THROW __nonnull ((2));
+    __THROW __nonnull ((2)) __attr_access ((__write_only__, 1, 3));
 
 #ifdef __USE_XOPEN2K8
 /* POSIX.1-2008 extended locale interface (see locale.h).  */
@@ -158,7 +160,8 @@ extern int strcoll_l (const char *__s1, const char *__s2, locale_t __l)
 /* Put a transformation of SRC into no more than N bytes of DEST,
    using sorting rules from L.  */
 extern size_t strxfrm_l (char *__dest, const char *__src, size_t __n,
-			 locale_t __l) __THROW __nonnull ((2, 4));
+			 locale_t __l) __THROW __nonnull ((2, 4))
+     __attr_access ((__write_only__, 1, 3));
 #endif
 
 #if (defined __USE_XOPEN_EXTENDED || defined __USE_XOPEN2K8	\
@@ -368,7 +371,9 @@ extern char *strcasestr (const char *__haystack, const char *__needle)
    HAYSTACK is HAYSTACKLEN bytes long.  */
 extern void *memmem (const void *__haystack, size_t __haystacklen,
 		     const void *__needle, size_t __needlelen)
-     __THROW __attribute_pure__ __nonnull ((1, 3));
+     __THROW __attribute_pure__ __nonnull ((1, 3))
+    __attr_access ((__read_only__, 1, 2))
+    __attr_access ((__read_only__, 3, 4));
 
 /* Copy N bytes of SRC to DEST, return pointer to bytes after the
    last written byte.  */
@@ -409,17 +414,18 @@ extern char *strerror (int __errnum) __THROW;
 #  ifdef __REDIRECT_NTH
 extern int __REDIRECT_NTH (strerror_r,
 			   (int __errnum, char *__buf, size_t __buflen),
-			   __xpg_strerror_r) __nonnull ((2));
+			   __xpg_strerror_r) __nonnull ((2))
+    __attr_access ((__write_only__, 2, 3));
 #  else
 extern int __xpg_strerror_r (int __errnum, char *__buf, size_t __buflen)
-     __THROW __nonnull ((2));
+     __THROW __nonnull ((2)) __attr_access ((__write_only__, 2, 3));
 #   define strerror_r __xpg_strerror_r
 #  endif
 # else
 /* If a temporary buffer is required, at most BUFLEN bytes of BUF will be
    used.  */
 extern char *strerror_r (int __errnum, char *__buf, size_t __buflen)
-     __THROW __nonnull ((2)) __wur;
+     __THROW __nonnull ((2)) __wur  __attr_access ((__write_only__, 2, 3));
 # endif
 #endif
 
@@ -433,7 +439,8 @@ extern char *strerror_l (int __errnum, locale_t __l) __THROW;
 
 /* Set N bytes of S to 0.  The compiler will not delete a call to this
    function, even if S is dead after the call.  */
-extern void explicit_bzero (void *__s, size_t __n) __THROW __nonnull ((1));
+extern void explicit_bzero (void *__s, size_t __n) __THROW __nonnull ((1))
+    __attr_access ((__write_only__, 1, 2));
 
 /* Return the next DELIM-delimited token from *STRINGP,
    terminating it with a '\0', and update *STRINGP to point past it.  */
@@ -471,7 +478,8 @@ extern int strverscmp (const char *__s1, const char *__s2)
 extern char *strfry (char *__string) __THROW __nonnull ((1));
 
 /* Frobnicate N bytes of S.  */
-extern void *memfrob (void *__s, size_t __n) __THROW __nonnull ((1));
+extern void *memfrob (void *__s, size_t __n) __THROW __nonnull ((1))
+    __attr_access ((__write_only__, 1, 2));
 
 # ifndef basename
 /* Return the file name within directory of FILENAME.  We don't

[-- Attachment #3: test-access-warn.c --]
[-- Type: text/x-csrc, Size: 5756 bytes --]

/* BZ #25219 - improve out-of-bounds checking with GCC 10 attribute access */

#include <stdlib.h>
#include <string.h>
#include <unistd.h>


/* <stdlib.h> */

int nowarn_ptsname_r (int fd, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return ptsname_r (fd, a + i, sizeof a);   // nowarn
}

int warn_ptsname_r (int fd, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return ptsname_r (fd, a + i, sizeof a);   // warn
}


int nowarn_mbstowcs (const char *s, int i)
{
  wchar_t a[4];
  if (i < 0)
    i = 0;
  return mbstowcs (a + i, s, sizeof a / sizeof *a);   // nowarn
}

int warn_mbstowcs (const char *s, int i)
{
  wchar_t a[4];
  if (i < 1)
    i = 1;
  return mbstowcs (a + i, s, sizeof a / sizeof *a);   // warn
}


int nowarn_wcstombs (const wchar_t *s, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return wcstombs (a + i, s, sizeof a / sizeof *a);   // nowarn
}

int warn_wcstombs (const wchar_t *s, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return wcstombs (a + i, s, sizeof a / sizeof *a);   // warn
}


/* <string.h> */

int nowarn_memccpy (const void *s, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return memccpy (a + i, s, 'x', sizeof a);   // nowarn
}

int warn_memccpy (const char *s, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return memccpy (a + i, s, 'x', sizeof a);   // warn
}


int nowarn_memrchr (int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return memrchr (a + i, 'x', sizeof a);   // nowarn
}

int warn_memrchr (int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return memrchr (a + i, 'x', sizeof a);   // warn
}


int nowarn_strxfrm (const char *s, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return strxfrm (a + i, s, sizeof a);   // nowarn
}

int warn_strxfrm (const char *s, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return strxfrm (a + i, s, sizeof a);   // warn
}


int nowarn_memmem (int i)
{
  char a[4] = "123";
  char b[4] = "321";
  if (i < 0)
    i = 0;
  return memmem (a + i, sizeof a, b + i, sizeof b);   // nowarn
}

int warn_memmem (int i)
{
  char a[4] = "123";
  char b[4] = "321";
  if (i < 1)
    i = 1;
  return memmem (a + i, sizeof a, b + i, sizeof b);   // warn
}


int nowarn_strerror_r (int e, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return strerror_r (e, a + i, sizeof a);   // nowarn
}

int warn_strerror_r (int e, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return strerror_r (e, a + i, sizeof a);   // warn
}


void nowarn_explicit_bzero (int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  explicit_bzero (a + i, sizeof a);   // nowarn
}

void warn_explicit_bzero (int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  explicit_bzero (a + i, sizeof a);   // warn
}


void nowarn_memfrob (int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  memfrob (a + i, sizeof a);   // nowarn
}

void warn_memfrob (int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  memfrob (a + i, sizeof a);   // warn
}

/* <unistd.h> */


int nowarn_read (int fd, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return read (fd, a + i, sizeof a);   // nowarn
}

int warn_read (int fd, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return read (fd, a + i, sizeof a);   // warn
}


int nowarn_pread (int fd, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return pread (fd, a + i, sizeof a, 0);   // nowarn
}

int warn_pread (int fd, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return pread (fd, a + i, sizeof a, 0);   // warn
}


int nowarn_pread64 (int fd, int i)
{
  char a[4];
  if (i < 0)
    i = 1;
  return pread64 (fd, a + i, sizeof a, 0);   // nowarn
}

int warn_pread64 (int fd, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return pread64 (fd, a + i, sizeof a, 0);   // warn
}


int nowarn_readlink (const char *path, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return readlink (path, a + i, sizeof a);   // nowarn
}

int warn_readlink (const char *path, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return readlink (path, a + i, sizeof a);   // warn
}


int nowarn_readlinkat (int fd, const char *path, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return readlinkat (fd, path, a + i, sizeof a);   // nowarn
}

int warn_readlinkat (int fd, const char *path, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return readlinkat (fd, path, a + i, sizeof a);   // warn
}


char* nowarn_getcwd (int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return getcwd (a + i, sizeof a);   // nowarn
}

char* warn_getcwd (int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return getcwd (a + i, sizeof a);   // warn
}


int nowarn_confstr (int name, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return confstr (name, a + i, sizeof a);   // nowarn
}

int warn_confstr (int name, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return confstr (name, a + i, sizeof a);   // warn
}


int nowarn_ttyname_r (int name, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return ttyname_r (name, a + i, sizeof a);   // nowarn
}

int warn_ttyname_r (int name, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return ttyname_r (name, a + i, sizeof a);   // warn
}


int nowarn_getlogin_r (int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return getlogin_r (a + i, sizeof a);   // nowarn
}

int warn_getlogin_r (int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return getlogin_r (a + i, sizeof a);   // warn
}


int nowarn_gethostname (int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return gethostname (a + i, sizeof a);   // nowarn
}

int warn_gethostname (int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return gethostname (a + i, sizeof a);   // warn
}


int nowarn_getdomainname (int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return getdomainname (a + i, sizeof a);   // nowarn
}

int warn_getdomainname (int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return getdomainname (a + i, sizeof a);   // warn
}

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

* Re: [PATCH] improve out-of-bounds checking with GCC 10 attribute access [BZ #25219]
  2020-04-30 22:12 [PATCH] improve out-of-bounds checking with GCC 10 attribute access [BZ #25219] Martin Sebor
@ 2020-04-30 22:37 ` Dmitry V. Levin
  2020-05-01  2:42 ` DJ Delorie
  1 sibling, 0 replies; 11+ messages in thread
From: Dmitry V. Levin @ 2020-04-30 22:37 UTC (permalink / raw)
  To: libc-alpha

On Thu, Apr 30, 2020 at 04:12:32PM -0600, Martin Sebor via Libc-alpha wrote:
[...]
> --- a/libio/bits/stdio2.h
> +++ b/libio/bits/stdio2.h
> @@ -241,7 +241,8 @@ gets (char *__str)
>  #endif
>  
>  extern char *__fgets_chk (char *__restrict __s, size_t __size, int __n,
> -			  FILE *__restrict __stream) __wur;
> +			  FILE *__restrict __stream)
> +  __wur __attr_access ((__read_only__, 1, 2));

This has to be __write_only__, i.e. the same attribute that is being added
to __fgets_unlocked_chk below.

>  extern char *__REDIRECT (__fgets_alias,
>  			 (char *__restrict __s, int __n,
>  			  FILE *__restrict __stream), fgets) __wur;
> @@ -299,7 +300,8 @@ fread (void *__restrict __ptr, size_t __size, size_t __n,
>  
>  #ifdef __USE_GNU
>  extern char *__fgets_unlocked_chk (char *__restrict __s, size_t __size,
> -				   int __n, FILE *__restrict __stream) __wur;
> +				   int __n, FILE *__restrict __stream)
> +  __wur __attr_access ((__write_only__, 1, 2));
>  extern char *__REDIRECT (__fgets_unlocked_alias,
>  			 (char *__restrict __s, int __n,
>  			  FILE *__restrict __stream), fgets_unlocked) __wur;


-- 
ldv

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

* Re: [PATCH] improve out-of-bounds checking with GCC 10 attribute access [BZ #25219]
  2020-04-30 22:12 [PATCH] improve out-of-bounds checking with GCC 10 attribute access [BZ #25219] Martin Sebor
  2020-04-30 22:37 ` Dmitry V. Levin
@ 2020-05-01  2:42 ` DJ Delorie
  2020-05-01 19:54   ` Martin Sebor
  1 sibling, 1 reply; 11+ messages in thread
From: DJ Delorie @ 2020-05-01  2:42 UTC (permalink / raw)
  To: Martin Sebor; +Cc: libc-alpha


Mostly ok; a few buggy ones and some questions...

Martin Sebor via Libc-alpha <libc-alpha@sourceware.org> writes:
>  extern char *__fgets_chk (char *__restrict __s, size_t __size, int __n,
> -			  FILE *__restrict __stream) __wur;
> +			  FILE *__restrict __stream)
> +  __wur __attr_access ((__read_only__, 1, 2));

__s[__size]

Does __read_only__ mean that fgets only reads from __s?  Because that's backwards.

>  #ifdef __USE_GNU
>  extern char *__fgets_unlocked_chk (char *__restrict __s, size_t __size,
> -				   int __n, FILE *__restrict __stream) __wur;
> +				   int __n, FILE *__restrict __stream)
> +  __wur __attr_access ((__write_only__, 1, 2));

__s[__size] ok.

> +#if __GNUC_PREREQ (10, 0)
> +/* Denotes a function pointer argument ref-index that can be used
> +   to access size-index elements of the pointed-to array according
> +   to access mode:
> +     access (access-mode, <ref-index> [, <size-index>])  */

IMHO comment should state that the first argument is index 1.

IMHO should document what happens when size-index is missing.

> +#define __attr_access(x) __attribute__ ((__access__ x))
> +#else
> +#  define __attr_access(x)
> +#endif
> +

Ok.

>  extern ssize_t __read_chk (int __fd, void *__buf, size_t __nbytes,
> -			   size_t __buflen) __wur;
> +			   size_t __buflen)
> +  __wur __attr_access ((__write_only__, 2, 3));

read_chk() writes to buf

>  extern ssize_t __REDIRECT (__read_alias, (int __fd, void *__buf,
> -					  size_t __nbytes), read) __wur;
> +					  size_t __nbytes), read)
> +  __wur __attr_access ((__write_only__, 2, 3));

__buf[__nbytes] ok

>  extern ssize_t __pread_chk (int __fd, void *__buf, size_t __nbytes,
> -			    __off_t __offset, size_t __bufsize) __wur;
> +			    __off_t __offset, size_t __bufsize)
> +  __wur __attr_access ((__write_only__, 2, 3));

__buf[__nbytes] ok

>  extern ssize_t __pread64_chk (int __fd, void *__buf, size_t __nbytes,
> -			      __off64_t __offset, size_t __bufsize) __wur;
> +			      __off64_t __offset, size_t __bufsize)
> +  __wur __attr_access ((__write_only__, 2, 3));

__buf[__nbytes] ok

>  extern ssize_t __REDIRECT (__pread_alias,
>  			   (int __fd, void *__buf, size_t __nbytes,
> -			    __off_t __offset), pread) __wur;
> +			    __off_t __offset), pread)
> +  __wur __attr_access ((__write_only__, 2, 3));

__buf[__nbytes] ok

>  extern ssize_t __REDIRECT (__pread64_alias,
>  			   (int __fd, void *__buf, size_t __nbytes,
> -			    __off64_t __offset), pread64) __wur;
> +			    __off64_t __offset), pread64)
> +  __wur __attr_access ((__write_only__, 2, 3));

__buf[__nbytes] ok

>  extern ssize_t __readlink_chk (const char *__restrict __path,
>  			       char *__restrict __buf, size_t __len,
>  			       size_t __buflen)
> -     __THROW __nonnull ((1, 2)) __wur;
> +     __THROW __nonnull ((1, 2)) __wur __attr_access ((__write_only__, 2, 3));

__buf[__len] ok

>  extern ssize_t __REDIRECT_NTH (__readlink_alias,
>  			       (const char *__restrict __path,
>  				char *__restrict __buf, size_t __len), readlink)
> -     __nonnull ((1, 2)) __wur;
> +     __nonnull ((1, 2)) __wur __attr_access ((__write_only__, 2, 3));

__buf[__len] ok

>  extern ssize_t __readlinkat_chk (int __fd, const char *__restrict __path,
>  				 char *__restrict __buf, size_t __len,
>  				 size_t __buflen)
> -     __THROW __nonnull ((2, 3)) __wur;
> +     __THROW __nonnull ((2, 3)) __wur __attr_access ((__write_only__, 3, 4));

__buf[__len] ok

>  extern ssize_t __REDIRECT_NTH (__readlinkat_alias,
>  			       (int __fd, const char *__restrict __path,
>  				char *__restrict __buf, size_t __len),
>  			       readlinkat)
> -     __nonnull ((2, 3)) __wur;
> +     __nonnull ((2, 3)) __wur __attr_access ((__write_only__, 3, 4));

__buf[__len] ok

>  extern char *__getcwd_chk (char *__buf, size_t __size, size_t __buflen)
> -     __THROW __wur;
> +     __THROW __wur __attr_access ((__write_only__, 1, 2));

__buf[__size] ok

>  extern char *__REDIRECT_NTH (__getcwd_alias,
> -			     (char *__buf, size_t __size), getcwd) __wur;
> +			     (char *__buf, size_t __size), getcwd)
> +  __wur __attr_access ((__write_only__, 1, 2));

__buf[__size] ok

>  extern char *__getwd_chk (char *__buf, size_t buflen)
> -     __THROW __nonnull ((1)) __wur;
> +     __THROW __nonnull ((1)) __wur __attr_access ((__write_only__, 1, 2));

__buf[__size] ok

>  extern size_t __confstr_chk (int __name, char *__buf, size_t __len,
> -			     size_t __buflen) __THROW;
> +			     size_t __buflen) __THROW
> +  __attr_access ((__write_only__, 2, 3));

__buf[__len] ok

>  extern size_t __REDIRECT_NTH (__confstr_alias, (int __name, char *__buf,
> -						size_t __len), confstr);
> +						size_t __len), confstr)
> +   __attr_access ((__write_only__, 2, 3));

__buf[__len] ok

>  extern int __getgroups_chk (int __size, __gid_t __list[], size_t __listlen)
> -     __THROW __wur;
> +  __THROW __wur __attr_access ((__write_only__, 2, 1));

__list[__size] ok

>  extern int __REDIRECT_NTH (__getgroups_alias, (int __size, __gid_t __list[]),
> -			   getgroups) __wur;
> +			   getgroups) __wur __attr_access ((__write_only__, 2, 1));

__list[__size] ok

>  extern int __ttyname_r_chk (int __fd, char *__buf, size_t __buflen,
> -			    size_t __nreal) __THROW __nonnull ((2));
> +			    size_t __nreal) __THROW __nonnull ((2))
> +   __attr_access ((__write_only__, 2, 3));

__buf[__buflen] ok

>  extern int __getlogin_r_chk (char *__buf, size_t __buflen, size_t __nreal)
> -     __nonnull ((1));
> +     __nonnull ((1)) __attr_access ((__write_only__, 1, 2));

__buf[__buflen] ok

>  extern int __gethostname_chk (char *__buf, size_t __buflen, size_t __nreal)
> -     __THROW __nonnull ((1));
> +     __THROW __nonnull ((1)) __attr_access ((__write_only__, 1, 2));

__buf[__buflen] ok

>  extern int __REDIRECT_NTH (__gethostname_alias, (char *__buf, size_t __buflen),
> -			   gethostname) __nonnull ((1));
> +			   gethostname)
> +  __nonnull ((1)) __attr_access ((__write_only__, 1, 2));

__buf[__buflen] ok

>  extern int __getdomainname_chk (char *__buf, size_t __buflen, size_t __nreal)
> -     __THROW __nonnull ((1)) __wur;
> +     __THROW __nonnull ((1)) __wur __attr_access ((__write_only__, 1, 2));

__buf[__buflen] ok

>  extern int __REDIRECT_NTH (__getdomainname_alias, (char *__buf,
>  						   size_t __buflen),
> -			   getdomainname) __nonnull ((1)) __wur;
> +			   getdomainname) __nonnull ((1))
> +  __wur __attr_access ((__write_only__, 1, 2));

__buf[__buflen] ok

> -extern ssize_t read (int __fd, void *__buf, size_t __nbytes) __wur;
> +extern ssize_t read (int __fd, void *__buf, size_t __nbytes) __wur
> +    __attr_access ((__write_only__, 2, 3));

__buf[__nbytes] ok

> -extern ssize_t write (int __fd, const void *__buf, size_t __n) __wur;
> +extern ssize_t write (int __fd, const void *__buf, size_t __n) __wur
> +    __attr_access ((__read_only__, 2, 3));

__buf[__n] ok.

>  extern ssize_t pread (int __fd, void *__buf, size_t __nbytes,
> -		      __off_t __offset) __wur;
> +		      __off_t __offset) __wur
> +    __attr_access ((__write_only__, 2, 3));

__buf[__nbytes] ok

>  extern ssize_t pwrite (int __fd, const void *__buf, size_t __n,
> -		       __off_t __offset) __wur;
> +		       __off_t __offset) __wur
> +    __attr_access ((__read_only__, 2, 3));

__buf[__n] ok

>  extern ssize_t __REDIRECT (pread, (int __fd, void *__buf, size_t __nbytes,
>  				   __off64_t __offset),
> -			   pread64) __wur;
> +			   pread64) __wur
> +    __attr_access ((__write_only__, 2, 3));

__buf[__nbytes] ok

>  extern ssize_t __REDIRECT (pwrite, (int __fd, const void *__buf,
>  				    size_t __nbytes, __off64_t __offset),
> -			   pwrite64) __wur;
> +			   pwrite64) __wur
> +    __attr_access ((__read_only__, 2, 3));

__buf[__nbytes] ok

>  extern ssize_t pread64 (int __fd, void *__buf, size_t __nbytes,
> -			__off64_t __offset) __wur;
> +			__off64_t __offset) __wur
> +    __attr_access ((__write_only__, 2, 3));

__buf[__nbytes] ok

>  extern ssize_t pwrite64 (int __fd, const void *__buf, size_t __n,
> -			 __off64_t __offset) __wur;
> +			 __off64_t __offset) __wur
> +    __attr_access ((__read_only__, 2, 3));

__buf[__n] ok

>  extern char *getwd (char *__buf)
> -     __THROW __nonnull ((1)) __attribute_deprecated__ __wur;
> +     __THROW __nonnull ((1)) __attribute_deprecated__ __wur
> +    __attr_access ((__write_only__, 1));

__buf[???]

> -extern size_t confstr (int __name, char *__buf, size_t __len) __THROW;
> +extern size_t confstr (int __name, char *__buf, size_t __len) __THROW
> +    __attribute__ ((access (__write_only__, 2, 3)));

__buf[__len] ok

NOTE: does not use the __attr_access macro

>  #endif
>  
>  
> -extern int getgroups (int __size, __gid_t __list[]) __THROW __wur;
> -
> +extern int getgroups (int __size, __gid_t __list[]) __THROW __wur
> +    __attr_access ((__write_only__, 2, 1));

__list[__size] ok

>  extern int ttyname_r (int __fd, char *__buf, size_t __buflen)
> -     __THROW __nonnull ((2)) __wur;
> +     __THROW __nonnull ((2)) __wur __attr_access ((__write_only__, 2, 3));

__buf[__buflen] ok

>  extern ssize_t readlink (const char *__restrict __path,
>  			 char *__restrict __buf, size_t __len)
> -     __THROW __nonnull ((1, 2)) __wur;
> +     __THROW __nonnull ((1, 2)) __wur __attr_access ((__write_only__, 2, 3));

__buf[__len] ok

>  extern ssize_t readlinkat (int __fd, const char *__restrict __path,
>  			   char *__restrict __buf, size_t __len)
> -     __THROW __nonnull ((2, 3)) __wur;
> +     __THROW __nonnull ((2, 3)) __wur __attr_access ((__read_only__, 3, 4));

__buf[__len] ok

> -extern int getlogin_r (char *__name, size_t __name_len) __nonnull ((1));
> +extern int getlogin_r (char *__name, size_t __name_len) __nonnull ((1))
> +    __attr_access ((__write_only__, 1, 2));

__name[__name_len] ok

> -extern int gethostname (char *__name, size_t __len) __THROW __nonnull ((1));
> +extern int gethostname (char *__name, size_t __len) __THROW __nonnull ((1))
> +    __attr_access ((__write_only__, 1, 2));

__name[__len] ok

>  extern int sethostname (const char *__name, size_t __len)
> -     __THROW __nonnull ((1)) __wur;
> +     __THROW __nonnull ((1)) __wur __attr_access ((__read_only__, 1, 2));

__name[__len] ok

>  extern int getdomainname (char *__name, size_t __len)
> -     __THROW __nonnull ((1)) __wur;
> +     __THROW __nonnull ((1)) __wur __attr_access ((__write_only__, 1, 2));

__name[__len] ok

>  extern int setdomainname (const char *__name, size_t __len)
> -     __THROW __nonnull ((1)) __wur;
> -
> +     __THROW __nonnull ((1)) __wur __attr_access ((__read_only__, 1, 2));

__name[__len] ok

>  extern void swab (const void *__restrict __from, void *__restrict __to,
> -		  ssize_t __n) __THROW __nonnull ((1, 2));
> +		  ssize_t __n) __THROW __nonnull ((1, 2))
> +    __attr_access ((__read_only__, 1, 3))
> +    __attr_access ((__write_only__, 2, 3));

__from[__n], __to[__n] ok

> -int getentropy (void *__buffer, size_t __length) __wur;
> +int getentropy (void *__buffer, size_t __length) __wur
> +    __attr_access ((__write_only__, 1, 2));

__buffer[__length] ok

>  extern int __ptsname_r_chk (int __fd, char *__buf, size_t __buflen,
> -			    size_t __nreal) __THROW __nonnull ((2));
> +			    size_t __nreal) __THROW __nonnull ((2))
> +    __attr_access ((__write_only__, 2, 3));

__buf[__buflen] ok

>  extern int __REDIRECT_NTH (__ptsname_r_alias, (int __fd, char *__buf,
>  					       size_t __buflen), ptsname_r)
> -     __nonnull ((2));
> +     __nonnull ((2)) __attr_access ((__write_only__, 2, 3));

__buf[__buflen] ok

>  extern size_t __mbstowcs_chk (wchar_t *__restrict __dst,
>  			      const char *__restrict __src,
> -			      size_t __len, size_t __dstlen) __THROW;
> +			      size_t __len, size_t __dstlen) __THROW
> +    __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));

__dst[__len], __src[???] ok

>  extern size_t __REDIRECT_NTH (__mbstowcs_alias,
>  			      (wchar_t *__restrict __dst,
>  			       const char *__restrict __src,
> -			       size_t __len), mbstowcs);
> +			       size_t __len), mbstowcs)
> +    __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));

__dst[__len], __src[???] ok

>  extern size_t __wcstombs_chk (char *__restrict __dst,
>  			      const wchar_t *__restrict __src,
> -			      size_t __len, size_t __dstlen) __THROW;
> +			      size_t __len, size_t __dstlen) __THROW
> +  __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));

__dst[__len], __src[???] ok

>  extern size_t __REDIRECT_NTH (__wcstombs_alias,
>  			      (char *__restrict __dst,
>  			       const wchar_t *__restrict __src,
> -			       size_t __len), wcstombs);
> +			       size_t __len), wcstombs)
> +  __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));

__dst[__len], __src[???] ok

>  extern size_t mbstowcs (wchar_t *__restrict  __pwcs,
> -			const char *__restrict __s, size_t __n) __THROW;
> +			const char *__restrict __s, size_t __n) __THROW
> +    __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));

__pwcs[__n], __s[???] ok

>  extern size_t wcstombs (char *__restrict __s,
>  			const wchar_t *__restrict __pwcs, size_t __n)
> -     __THROW;
> -
> +     __THROW
> +  __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));

__s[__n], __pwcs[???] ok

>  extern int ptsname_r (int __fd, char *__buf, size_t __buflen)
> -     __THROW __nonnull ((2));
> +     __THROW __nonnull ((2)) __attr_access ((__write_only__, 2, 3));

__buf[__buflen] ok

>  void __explicit_bzero_chk (void *__dest, size_t __len, size_t __destlen)
> -  __THROW __nonnull ((1));
> +  __THROW __nonnull ((1)) __attr_access ((__write_only__, 1, 2));

__dest[__len] ok

>  extern char *__stpncpy_chk (char *__dest, const char *__src, size_t __n,
> -			    size_t __destlen) __THROW;
> +			    size_t __destlen) __THROW
> +  __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));

__dest[__n], __src[???] ok

>  extern void *memccpy (void *__restrict __dest, const void *__restrict __src,
>  		      int __c, size_t __n)
> -     __THROW __nonnull ((1, 2));
> +    __THROW __nonnull ((1, 2)) __attr_access ((__write_only__, 1, 4));

__dest[__n] - missing __src[__n] ?

>  extern "C++" void *memrchr (void *__s, int __c, size_t __n)
> -      __THROW __asm ("memrchr") __attribute_pure__ __nonnull ((1));
> +      __THROW __asm ("memrchr") __attribute_pure__ __nonnull ((1))
> +      __attr_access ((__read_only__, 1, 3));

__s[__n] ok

>  extern "C++" const void *memrchr (const void *__s, int __c, size_t __n)
> -      __THROW __asm ("memrchr") __attribute_pure__ __nonnull ((1));
> +      __THROW __asm ("memrchr") __attribute_pure__ __nonnull ((1))
> +      __attr_access ((__read_only__, 1, 3));

__s[__n] ok

>  extern size_t strxfrm (char *__restrict __dest,
>  		       const char *__restrict __src, size_t __n)
> -     __THROW __nonnull ((2));
> +    __THROW __nonnull ((2)) __attr_access ((__write_only__, 1, 3));

__dest[__n] - missing __src[???] ?

>  extern size_t strxfrm_l (char *__dest, const char *__src, size_t __n,
> -			 locale_t __l) __THROW __nonnull ((2, 4));
> +			 locale_t __l) __THROW __nonnull ((2, 4))
> +     __attr_access ((__write_only__, 1, 3));

__dest[__n] - missing __src[???] ?

>  extern void *memmem (const void *__haystack, size_t __haystacklen,
>  		     const void *__needle, size_t __needlelen)
> -     __THROW __attribute_pure__ __nonnull ((1, 3));
> +     __THROW __attribute_pure__ __nonnull ((1, 3))
> +    __attr_access ((__read_only__, 1, 2))
> +    __attr_access ((__read_only__, 3, 4));

__haystack[__haystacklen], __needle[__needlelen] ok

>  extern int __REDIRECT_NTH (strerror_r,
>  			   (int __errnum, char *__buf, size_t __buflen),
> -			   __xpg_strerror_r) __nonnull ((2));
> +			   __xpg_strerror_r) __nonnull ((2))
> +    __attr_access ((__write_only__, 2, 3));

__buf[__buflen] ok

>  extern int __xpg_strerror_r (int __errnum, char *__buf, size_t __buflen)
> -     __THROW __nonnull ((2));
> +     __THROW __nonnull ((2)) __attr_access ((__write_only__, 2, 3));

__buf[__buflen] ok

>  extern char *strerror_r (int __errnum, char *__buf, size_t __buflen)
> -     __THROW __nonnull ((2)) __wur;
> +     __THROW __nonnull ((2)) __wur  __attr_access ((__write_only__, 2, 3));

__buf[__buflen] ok

>  /* Set N bytes of S to 0.  The compiler will not delete a call to this
>     function, even if S is dead after the call.  */
> -extern void explicit_bzero (void *__s, size_t __n) __THROW __nonnull ((1));
> +extern void explicit_bzero (void *__s, size_t __n) __THROW __nonnull ((1))
> +    __attr_access ((__write_only__, 1, 2));

__s[__n] ok

> -extern void *memfrob (void *__s, size_t __n) __THROW __nonnull ((1));
> +extern void *memfrob (void *__s, size_t __n) __THROW __nonnull ((1))
> +    __attr_access ((__write_only__, 1, 2));

__s[__n] ok

> [3:text/x-csrc Hide Save:test-access-warn.c (13kB)]
>
> /* BZ #25219 - improve out-of-bounds checking with GCC 10 attribute access */

There are ways of adding such tests into glibc, but I'll defer any
review of such efforts until/unless we actually want to add one.


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

* Re: [PATCH] improve out-of-bounds checking with GCC 10 attribute access [BZ #25219]
  2020-05-01  2:42 ` DJ Delorie
@ 2020-05-01 19:54   ` Martin Sebor
  2020-05-01 22:02     ` DJ Delorie
  0 siblings, 1 reply; 11+ messages in thread
From: Martin Sebor @ 2020-05-01 19:54 UTC (permalink / raw)
  To: DJ Delorie; +Cc: libc-alpha

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

On 4/30/20 8:42 PM, DJ Delorie wrote:
> 
> Mostly ok; a few buggy ones and some questions...

Thanks for the careful review!

> 
> Martin Sebor via Libc-alpha <libc-alpha@sourceware.org> writes:
>>   extern char *__fgets_chk (char *__restrict __s, size_t __size, int __n,
>> -			  FILE *__restrict __stream) __wur;
>> +			  FILE *__restrict __stream)
>> +  __wur __attr_access ((__read_only__, 1, 2));
> 
> __s[__size]
> 
> Does __read_only__ mean that fgets only reads from __s?  Because that's backwards.

Yes, that's wrong.  Good catch!  I completely missed stdio when
testing so I also didn't notice I forgot to add the attribute to
fgets() itself.  I've fixed that in the updated patch.

> 
>>   #ifdef __USE_GNU
>>   extern char *__fgets_unlocked_chk (char *__restrict __s, size_t __size,
>> -				   int __n, FILE *__restrict __stream) __wur;
>> +				   int __n, FILE *__restrict __stream)
>> +  __wur __attr_access ((__write_only__, 1, 2));
> 
> __s[__size] ok.
> 
>> +#if __GNUC_PREREQ (10, 0)
>> +/* Denotes a function pointer argument ref-index that can be used
>> +   to access size-index elements of the pointed-to array according
>> +   to access mode:
>> +     access (access-mode, <ref-index> [, <size-index>])  */
> 
> IMHO comment should state that the first argument is index 1.
> 
> IMHO should document what happens when size-index is missing.

I've tweaked the comment a bit.  I hesitate to go into a lot of
detail here and would expect people needing it to read the manual.

> 
>> +#define __attr_access(x) __attribute__ ((__access__ x))
>> +#else
>> +#  define __attr_access(x)
>> +#endif
>> +
> 
> Ok.
> 
>>   extern ssize_t __read_chk (int __fd, void *__buf, size_t __nbytes,
>> -			   size_t __buflen) __wur;
>> +			   size_t __buflen)
>> +  __wur __attr_access ((__write_only__, 2, 3));
> 
> read_chk() writes to buf
> 
>>   extern ssize_t __REDIRECT (__read_alias, (int __fd, void *__buf,
>> -					  size_t __nbytes), read) __wur;
>> +					  size_t __nbytes), read)
>> +  __wur __attr_access ((__write_only__, 2, 3));
> 
> __buf[__nbytes] ok
> 
>>   extern ssize_t __pread_chk (int __fd, void *__buf, size_t __nbytes,
>> -			    __off_t __offset, size_t __bufsize) __wur;
>> +			    __off_t __offset, size_t __bufsize)
>> +  __wur __attr_access ((__write_only__, 2, 3));
> 
> __buf[__nbytes] ok
> 
>>   extern ssize_t __pread64_chk (int __fd, void *__buf, size_t __nbytes,
>> -			      __off64_t __offset, size_t __bufsize) __wur;
>> +			      __off64_t __offset, size_t __bufsize)
>> +  __wur __attr_access ((__write_only__, 2, 3));
> 
> __buf[__nbytes] ok
> 
>>   extern ssize_t __REDIRECT (__pread_alias,
>>   			   (int __fd, void *__buf, size_t __nbytes,
>> -			    __off_t __offset), pread) __wur;
>> +			    __off_t __offset), pread)
>> +  __wur __attr_access ((__write_only__, 2, 3));
> 
> __buf[__nbytes] ok
> 
>>   extern ssize_t __REDIRECT (__pread64_alias,
>>   			   (int __fd, void *__buf, size_t __nbytes,
>> -			    __off64_t __offset), pread64) __wur;
>> +			    __off64_t __offset), pread64)
>> +  __wur __attr_access ((__write_only__, 2, 3));
> 
> __buf[__nbytes] ok
> 
>>   extern ssize_t __readlink_chk (const char *__restrict __path,
>>   			       char *__restrict __buf, size_t __len,
>>   			       size_t __buflen)
>> -     __THROW __nonnull ((1, 2)) __wur;
>> +     __THROW __nonnull ((1, 2)) __wur __attr_access ((__write_only__, 2, 3));
> 
> __buf[__len] ok
> 
>>   extern ssize_t __REDIRECT_NTH (__readlink_alias,
>>   			       (const char *__restrict __path,
>>   				char *__restrict __buf, size_t __len), readlink)
>> -     __nonnull ((1, 2)) __wur;
>> +     __nonnull ((1, 2)) __wur __attr_access ((__write_only__, 2, 3));
> 
> __buf[__len] ok
> 
>>   extern ssize_t __readlinkat_chk (int __fd, const char *__restrict __path,
>>   				 char *__restrict __buf, size_t __len,
>>   				 size_t __buflen)
>> -     __THROW __nonnull ((2, 3)) __wur;
>> +     __THROW __nonnull ((2, 3)) __wur __attr_access ((__write_only__, 3, 4));
> 
> __buf[__len] ok
> 
>>   extern ssize_t __REDIRECT_NTH (__readlinkat_alias,
>>   			       (int __fd, const char *__restrict __path,
>>   				char *__restrict __buf, size_t __len),
>>   			       readlinkat)
>> -     __nonnull ((2, 3)) __wur;
>> +     __nonnull ((2, 3)) __wur __attr_access ((__write_only__, 3, 4));
> 
> __buf[__len] ok
> 
>>   extern char *__getcwd_chk (char *__buf, size_t __size, size_t __buflen)
>> -     __THROW __wur;
>> +     __THROW __wur __attr_access ((__write_only__, 1, 2));
> 
> __buf[__size] ok
> 
>>   extern char *__REDIRECT_NTH (__getcwd_alias,
>> -			     (char *__buf, size_t __size), getcwd) __wur;
>> +			     (char *__buf, size_t __size), getcwd)
>> +  __wur __attr_access ((__write_only__, 1, 2));
> 
> __buf[__size] ok
> 
>>   extern char *__getwd_chk (char *__buf, size_t buflen)
>> -     __THROW __nonnull ((1)) __wur;
>> +     __THROW __nonnull ((1)) __wur __attr_access ((__write_only__, 1, 2));
> 
> __buf[__size] ok
> 
>>   extern size_t __confstr_chk (int __name, char *__buf, size_t __len,
>> -			     size_t __buflen) __THROW;
>> +			     size_t __buflen) __THROW
>> +  __attr_access ((__write_only__, 2, 3));
> 
> __buf[__len] ok
> 
>>   extern size_t __REDIRECT_NTH (__confstr_alias, (int __name, char *__buf,
>> -						size_t __len), confstr);
>> +						size_t __len), confstr)
>> +   __attr_access ((__write_only__, 2, 3));
> 
> __buf[__len] ok
> 
>>   extern int __getgroups_chk (int __size, __gid_t __list[], size_t __listlen)
>> -     __THROW __wur;
>> +  __THROW __wur __attr_access ((__write_only__, 2, 1));
> 
> __list[__size] ok
> 
>>   extern int __REDIRECT_NTH (__getgroups_alias, (int __size, __gid_t __list[]),
>> -			   getgroups) __wur;
>> +			   getgroups) __wur __attr_access ((__write_only__, 2, 1));
> 
> __list[__size] ok
> 
>>   extern int __ttyname_r_chk (int __fd, char *__buf, size_t __buflen,
>> -			    size_t __nreal) __THROW __nonnull ((2));
>> +			    size_t __nreal) __THROW __nonnull ((2))
>> +   __attr_access ((__write_only__, 2, 3));
> 
> __buf[__buflen] ok
> 
>>   extern int __getlogin_r_chk (char *__buf, size_t __buflen, size_t __nreal)
>> -     __nonnull ((1));
>> +     __nonnull ((1)) __attr_access ((__write_only__, 1, 2));
> 
> __buf[__buflen] ok
> 
>>   extern int __gethostname_chk (char *__buf, size_t __buflen, size_t __nreal)
>> -     __THROW __nonnull ((1));
>> +     __THROW __nonnull ((1)) __attr_access ((__write_only__, 1, 2));
> 
> __buf[__buflen] ok
> 
>>   extern int __REDIRECT_NTH (__gethostname_alias, (char *__buf, size_t __buflen),
>> -			   gethostname) __nonnull ((1));
>> +			   gethostname)
>> +  __nonnull ((1)) __attr_access ((__write_only__, 1, 2));
> 
> __buf[__buflen] ok
> 
>>   extern int __getdomainname_chk (char *__buf, size_t __buflen, size_t __nreal)
>> -     __THROW __nonnull ((1)) __wur;
>> +     __THROW __nonnull ((1)) __wur __attr_access ((__write_only__, 1, 2));
> 
> __buf[__buflen] ok
> 
>>   extern int __REDIRECT_NTH (__getdomainname_alias, (char *__buf,
>>   						   size_t __buflen),
>> -			   getdomainname) __nonnull ((1)) __wur;
>> +			   getdomainname) __nonnull ((1))
>> +  __wur __attr_access ((__write_only__, 1, 2));
> 
> __buf[__buflen] ok
> 
>> -extern ssize_t read (int __fd, void *__buf, size_t __nbytes) __wur;
>> +extern ssize_t read (int __fd, void *__buf, size_t __nbytes) __wur
>> +    __attr_access ((__write_only__, 2, 3));
> 
> __buf[__nbytes] ok
> 
>> -extern ssize_t write (int __fd, const void *__buf, size_t __n) __wur;
>> +extern ssize_t write (int __fd, const void *__buf, size_t __n) __wur
>> +    __attr_access ((__read_only__, 2, 3));
> 
> __buf[__n] ok.
> 
>>   extern ssize_t pread (int __fd, void *__buf, size_t __nbytes,
>> -		      __off_t __offset) __wur;
>> +		      __off_t __offset) __wur
>> +    __attr_access ((__write_only__, 2, 3));
> 
> __buf[__nbytes] ok
> 
>>   extern ssize_t pwrite (int __fd, const void *__buf, size_t __n,
>> -		       __off_t __offset) __wur;
>> +		       __off_t __offset) __wur
>> +    __attr_access ((__read_only__, 2, 3));
> 
> __buf[__n] ok
> 
>>   extern ssize_t __REDIRECT (pread, (int __fd, void *__buf, size_t __nbytes,
>>   				   __off64_t __offset),
>> -			   pread64) __wur;
>> +			   pread64) __wur
>> +    __attr_access ((__write_only__, 2, 3));
> 
> __buf[__nbytes] ok
> 
>>   extern ssize_t __REDIRECT (pwrite, (int __fd, const void *__buf,
>>   				    size_t __nbytes, __off64_t __offset),
>> -			   pwrite64) __wur;
>> +			   pwrite64) __wur
>> +    __attr_access ((__read_only__, 2, 3));
> 
> __buf[__nbytes] ok
> 
>>   extern ssize_t pread64 (int __fd, void *__buf, size_t __nbytes,
>> -			__off64_t __offset) __wur;
>> +			__off64_t __offset) __wur
>> +    __attr_access ((__write_only__, 2, 3));
> 
> __buf[__nbytes] ok
> 
>>   extern ssize_t pwrite64 (int __fd, const void *__buf, size_t __n,
>> -			 __off64_t __offset) __wur;
>> +			 __off64_t __offset) __wur
>> +    __attr_access ((__read_only__, 2, 3));
> 
> __buf[__n] ok
> 
>>   extern char *getwd (char *__buf)
>> -     __THROW __nonnull ((1)) __attribute_deprecated__ __wur;
>> +     __THROW __nonnull ((1)) __attribute_deprecated__ __wur
>> +    __attr_access ((__write_only__, 1));
> 
> __buf[???]

When size-index is missing at least one byte of the array must be
accessible (or the pointer must be null).  There's no way to specify
a constant size with the current syntax.  In the future I'd like to
try to teach GCC to get it from the argument itself (for ordinary
arrays as well as for VLAs):

   extern char *getwd (char __buf[PATH_MAX)
     __attr_access ((__write_only__, 1)) ...;

>> -extern size_t confstr (int __name, char *__buf, size_t __len) __THROW;
>> +extern size_t confstr (int __name, char *__buf, size_t __len) __THROW
>> +    __attribute__ ((access (__write_only__, 2, 3)));
> 
> __buf[__len] ok
> 
> NOTE: does not use the __attr_access macro

Fixed, thanks.

> 
>>   #endif
>>   
>>   
>> -extern int getgroups (int __size, __gid_t __list[]) __THROW __wur;
>> -
>> +extern int getgroups (int __size, __gid_t __list[]) __THROW __wur
>> +    __attr_access ((__write_only__, 2, 1));
> 
> __list[__size] ok
> 
>>   extern int ttyname_r (int __fd, char *__buf, size_t __buflen)
>> -     __THROW __nonnull ((2)) __wur;
>> +     __THROW __nonnull ((2)) __wur __attr_access ((__write_only__, 2, 3));
> 
> __buf[__buflen] ok
> 
>>   extern ssize_t readlink (const char *__restrict __path,
>>   			 char *__restrict __buf, size_t __len)
>> -     __THROW __nonnull ((1, 2)) __wur;
>> +     __THROW __nonnull ((1, 2)) __wur __attr_access ((__write_only__, 2, 3));
> 
> __buf[__len] ok
> 
>>   extern ssize_t readlinkat (int __fd, const char *__restrict __path,
>>   			   char *__restrict __buf, size_t __len)
>> -     __THROW __nonnull ((2, 3)) __wur;
>> +     __THROW __nonnull ((2, 3)) __wur __attr_access ((__read_only__, 3, 4));
> 
> __buf[__len] ok
> 
>> -extern int getlogin_r (char *__name, size_t __name_len) __nonnull ((1));
>> +extern int getlogin_r (char *__name, size_t __name_len) __nonnull ((1))
>> +    __attr_access ((__write_only__, 1, 2));
> 
> __name[__name_len] ok
> 
>> -extern int gethostname (char *__name, size_t __len) __THROW __nonnull ((1));
>> +extern int gethostname (char *__name, size_t __len) __THROW __nonnull ((1))
>> +    __attr_access ((__write_only__, 1, 2));
> 
> __name[__len] ok
> 
>>   extern int sethostname (const char *__name, size_t __len)
>> -     __THROW __nonnull ((1)) __wur;
>> +     __THROW __nonnull ((1)) __wur __attr_access ((__read_only__, 1, 2));
> 
> __name[__len] ok
> 
>>   extern int getdomainname (char *__name, size_t __len)
>> -     __THROW __nonnull ((1)) __wur;
>> +     __THROW __nonnull ((1)) __wur __attr_access ((__write_only__, 1, 2));
> 
> __name[__len] ok
> 
>>   extern int setdomainname (const char *__name, size_t __len)
>> -     __THROW __nonnull ((1)) __wur;
>> -
>> +     __THROW __nonnull ((1)) __wur __attr_access ((__read_only__, 1, 2));
> 
> __name[__len] ok
> 
>>   extern void swab (const void *__restrict __from, void *__restrict __to,
>> -		  ssize_t __n) __THROW __nonnull ((1, 2));
>> +		  ssize_t __n) __THROW __nonnull ((1, 2))
>> +    __attr_access ((__read_only__, 1, 3))
>> +    __attr_access ((__write_only__, 2, 3));
> 
> __from[__n], __to[__n] ok
> 
>> -int getentropy (void *__buffer, size_t __length) __wur;
>> +int getentropy (void *__buffer, size_t __length) __wur
>> +    __attr_access ((__write_only__, 1, 2));
> 
> __buffer[__length] ok
> 
>>   extern int __ptsname_r_chk (int __fd, char *__buf, size_t __buflen,
>> -			    size_t __nreal) __THROW __nonnull ((2));
>> +			    size_t __nreal) __THROW __nonnull ((2))
>> +    __attr_access ((__write_only__, 2, 3));
> 
> __buf[__buflen] ok
> 
>>   extern int __REDIRECT_NTH (__ptsname_r_alias, (int __fd, char *__buf,
>>   					       size_t __buflen), ptsname_r)
>> -     __nonnull ((2));
>> +     __nonnull ((2)) __attr_access ((__write_only__, 2, 3));
> 
> __buf[__buflen] ok
> 
>>   extern size_t __mbstowcs_chk (wchar_t *__restrict __dst,
>>   			      const char *__restrict __src,
>> -			      size_t __len, size_t __dstlen) __THROW;
>> +			      size_t __len, size_t __dstlen) __THROW
>> +    __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
> 
> __dst[__len], __src[???] ok

Similar as in getwd, __src must be an array of at least one element
(or null).

> 
>>   extern size_t __REDIRECT_NTH (__mbstowcs_alias,
>>   			      (wchar_t *__restrict __dst,
>>   			       const char *__restrict __src,
>> -			       size_t __len), mbstowcs);
>> +			       size_t __len), mbstowcs)
>> +    __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
> 
> __dst[__len], __src[???] ok
> 
>>   extern size_t __wcstombs_chk (char *__restrict __dst,
>>   			      const wchar_t *__restrict __src,
>> -			      size_t __len, size_t __dstlen) __THROW;
>> +			      size_t __len, size_t __dstlen) __THROW
>> +  __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
> 
> __dst[__len], __src[???] ok

Same as above (array of 1 w

> 
>>   extern size_t __REDIRECT_NTH (__wcstombs_alias,
>>   			      (char *__restrict __dst,
>>   			       const wchar_t *__restrict __src,
>> -			       size_t __len), wcstombs);
>> +			       size_t __len), wcstombs)
>> +  __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
> 
> __dst[__len], __src[???] ok
> 
>>   extern size_t mbstowcs (wchar_t *__restrict  __pwcs,
>> -			const char *__restrict __s, size_t __n) __THROW;
>> +			const char *__restrict __s, size_t __n) __THROW
>> +    __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
> 
> __pwcs[__n], __s[???] ok
> 
>>   extern size_t wcstombs (char *__restrict __s,
>>   			const wchar_t *__restrict __pwcs, size_t __n)
>> -     __THROW;
>> -
>> +     __THROW
>> +  __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
> 
> __s[__n], __pwcs[???] ok
> 
>>   extern int ptsname_r (int __fd, char *__buf, size_t __buflen)
>> -     __THROW __nonnull ((2));
>> +     __THROW __nonnull ((2)) __attr_access ((__write_only__, 2, 3));
> 
> __buf[__buflen] ok
> 
>>   void __explicit_bzero_chk (void *__dest, size_t __len, size_t __destlen)
>> -  __THROW __nonnull ((1));
>> +  __THROW __nonnull ((1)) __attr_access ((__write_only__, 1, 2));
> 
> __dest[__len] ok
> 
>>   extern char *__stpncpy_chk (char *__dest, const char *__src, size_t __n,
>> -			    size_t __destlen) __THROW;
>> +			    size_t __destlen) __THROW
>> +  __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
> 
> __dest[__n], __src[???] ok
> 
>>   extern void *memccpy (void *__restrict __dest, const void *__restrict __src,
>>   		      int __c, size_t __n)
>> -     __THROW __nonnull ((1, 2));
>> +    __THROW __nonnull ((1, 2)) __attr_access ((__write_only__, 1, 4));
> 
> __dest[__n] - missing __src[__n] ?
> 
>>   extern "C++" void *memrchr (void *__s, int __c, size_t __n)
>> -      __THROW __asm ("memrchr") __attribute_pure__ __nonnull ((1));
>> +      __THROW __asm ("memrchr") __attribute_pure__ __nonnull ((1))
>> +      __attr_access ((__read_only__, 1, 3));
> 
> __s[__n] ok
> 
>>   extern "C++" const void *memrchr (const void *__s, int __c, size_t __n)
>> -      __THROW __asm ("memrchr") __attribute_pure__ __nonnull ((1));
>> +      __THROW __asm ("memrchr") __attribute_pure__ __nonnull ((1))
>> +      __attr_access ((__read_only__, 1, 3));
> 
> __s[__n] ok
> 
>>   extern size_t strxfrm (char *__restrict __dest,
>>   		       const char *__restrict __src, size_t __n)
>> -     __THROW __nonnull ((2));
>> +    __THROW __nonnull ((2)) __attr_access ((__write_only__, 1, 3));
> 
> __dest[__n] - missing __src[???] ?
> 
>>   extern size_t strxfrm_l (char *__dest, const char *__src, size_t __n,
>> -			 locale_t __l) __THROW __nonnull ((2, 4));
>> +			 locale_t __l) __THROW __nonnull ((2, 4))
>> +     __attr_access ((__write_only__, 1, 3));
> 
> __dest[__n] - missing __src[???] ?
> 
>>   extern void *memmem (const void *__haystack, size_t __haystacklen,
>>   		     const void *__needle, size_t __needlelen)
>> -     __THROW __attribute_pure__ __nonnull ((1, 3));
>> +     __THROW __attribute_pure__ __nonnull ((1, 3))
>> +    __attr_access ((__read_only__, 1, 2))
>> +    __attr_access ((__read_only__, 3, 4));
> 
> __haystack[__haystacklen], __needle[__needlelen] ok
> 
>>   extern int __REDIRECT_NTH (strerror_r,
>>   			   (int __errnum, char *__buf, size_t __buflen),
>> -			   __xpg_strerror_r) __nonnull ((2));
>> +			   __xpg_strerror_r) __nonnull ((2))
>> +    __attr_access ((__write_only__, 2, 3));
> 
> __buf[__buflen] ok
> 
>>   extern int __xpg_strerror_r (int __errnum, char *__buf, size_t __buflen)
>> -     __THROW __nonnull ((2));
>> +     __THROW __nonnull ((2)) __attr_access ((__write_only__, 2, 3));
> 
> __buf[__buflen] ok
> 
>>   extern char *strerror_r (int __errnum, char *__buf, size_t __buflen)
>> -     __THROW __nonnull ((2)) __wur;
>> +     __THROW __nonnull ((2)) __wur  __attr_access ((__write_only__, 2, 3));
> 
> __buf[__buflen] ok
> 
>>   /* Set N bytes of S to 0.  The compiler will not delete a call to this
>>      function, even if S is dead after the call.  */
>> -extern void explicit_bzero (void *__s, size_t __n) __THROW __nonnull ((1));
>> +extern void explicit_bzero (void *__s, size_t __n) __THROW __nonnull ((1))
>> +    __attr_access ((__write_only__, 1, 2));
> 
> __s[__n] ok
> 
>> -extern void *memfrob (void *__s, size_t __n) __THROW __nonnull ((1));
>> +extern void *memfrob (void *__s, size_t __n) __THROW __nonnull ((1))
>> +    __attr_access ((__write_only__, 1, 2));
> 
> __s[__n] ok
> 
>> [3:text/x-csrc Hide Save:test-access-warn.c (13kB)]
>>
>> /* BZ #25219 - improve out-of-bounds checking with GCC 10 attribute access */
> 
> There are ways of adding such tests into glibc, but I'll defer any
> review of such efforts until/unless we actually want to add one.

Sounds good.  In the meantime, the attached revision fixes
the problems pointed out above.  I also include the enhanced
test I used.

Martin

[-- Attachment #2: glibc-25219.diff --]
[-- Type: text/x-patch, Size: 32293 bytes --]

BZ #25219 - improve out-of-bounds checking with GCC 10 attribute access

ChangeLog:

	* misc/sys/cdefs.h (__attr_access): New macro.
	* libio/bits/stdio2.h (__sprintf_chk): Add attribute access.
	(__vsprintf_chk, __snprintf_chk): Same.
	(__fgets_chk, __fgets_alias, fgets, fgets_unlocked): Same.
	(__fgets_unlocked_chk, __fgets_unlocked_alias): Same.
	* posix/bits/unistd.h (__read_chk, __read_alias): Same.
	(__pread_chk, __pread64_chk, __pread_alias, __pread64_alias): Same.
	(__readlink_chk, __readlink_alias): Same.
	(__readlinkat_chk, __readlinkat_alias): Same.
	(__getcwd_chk, __getcwd_alias, __getwd_chk): Same.
	(__confstr_chk, __confstr_alias): Same.
	(__getgroups_chk, __getgroups_alias): Same.
	(__ttyname_r_chk, __ttyname_r_alias): Same.
	(__getlogin_r_chk, __getlogin_r_alias): Same.
	(__gethostname_chk, __gethostname_alias): Same.
	(__getdomainname_chk, __getdomainname_alias): Same.
	* posix/unistd.h (read, write, pread, pwrite): Same.
	(pread64, pwrite64): Same.
	(getcwd, getwd, confstr, getgrtoups, ttyname_r): Same.
	(readlinkat, getlogin_r, gethostname, sethostname): Same.
	(getdomainname, setdomainname, swab, genentropy): Same.
	* stdlib/bits/stdlib.h (__ptsname_r_chk): Same
	 (__mbstowcs_chk, __mbstowcs_alias): Same
	 (__wcstombs_chk, __wcstombs_alias): Same
	* stdlib/stdlib.h (mbstowcs, wcstombs): Same
	* string/bits/string_fortified.h (explicit_bzero_chk): Same
	* string/string.h (meccmpy, memrrchr, strxfrm, strcoll_l): Same.
	(memmem, __xpg_strerror_r, strerror_r): Same.
	(explicit_bzero, memfrob): Same.

diff --git a/libio/bits/stdio2.h b/libio/bits/stdio2.h
index 60bc81735e..ff9202c2cb 100644
--- a/libio/bits/stdio2.h
+++ b/libio/bits/stdio2.h
@@ -24,10 +24,12 @@
 #endif
 
 extern int __sprintf_chk (char *__restrict __s, int __flag, size_t __slen,
-			  const char *__restrict __format, ...) __THROW;
+			  const char *__restrict __format, ...) __THROW
+    __attr_access ((__write_only__, 1, 3));
 extern int __vsprintf_chk (char *__restrict __s, int __flag, size_t __slen,
 			   const char *__restrict __format,
-			   __gnuc_va_list __ap) __THROW;
+			   __gnuc_va_list __ap) __THROW
+    __attr_access ((__write_only__, 1, 3));
 
 #ifdef __va_arg_pack
 __fortify_function int
@@ -54,7 +56,8 @@ __NTH (vsprintf (char *__restrict __s, const char *__restrict __fmt,
 
 extern int __snprintf_chk (char *__restrict __s, size_t __n, int __flag,
 			   size_t __slen, const char *__restrict __format,
-			   ...) __THROW;
+			   ...) __THROW
+    __attr_access ((__write_only__, 1, 2));
 extern int __vsnprintf_chk (char *__restrict __s, size_t __n, int __flag,
 			    size_t __slen, const char *__restrict __format,
 			    __gnuc_va_list __ap) __THROW;
@@ -241,17 +244,19 @@ gets (char *__str)
 #endif
 
 extern char *__fgets_chk (char *__restrict __s, size_t __size, int __n,
-			  FILE *__restrict __stream) __wur;
+			  FILE *__restrict __stream)
+    __wur __attr_access ((__write_only__, 1, 3));
 extern char *__REDIRECT (__fgets_alias,
 			 (char *__restrict __s, int __n,
-			  FILE *__restrict __stream), fgets) __wur;
+			  FILE *__restrict __stream), fgets)
+    __wur __attr_access ((__write_only__, 1, 2));
 extern char *__REDIRECT (__fgets_chk_warn,
 			 (char *__restrict __s, size_t __size, int __n,
 			  FILE *__restrict __stream), __fgets_chk)
      __wur __warnattr ("fgets called with bigger size than length "
 		       "of destination buffer");
 
-__fortify_function __wur char *
+__fortify_function __wur __attr_access ((__write_only__, 1, 2)) char *
 fgets (char *__restrict __s, int __n, FILE *__restrict __stream)
 {
   if (__bos (__s) != (size_t) -1)
@@ -299,17 +304,19 @@ fread (void *__restrict __ptr, size_t __size, size_t __n,
 
 #ifdef __USE_GNU
 extern char *__fgets_unlocked_chk (char *__restrict __s, size_t __size,
-				   int __n, FILE *__restrict __stream) __wur;
+				   int __n, FILE *__restrict __stream)
+    __wur __attr_access ((__write_only__, 1, 3));
 extern char *__REDIRECT (__fgets_unlocked_alias,
 			 (char *__restrict __s, int __n,
-			  FILE *__restrict __stream), fgets_unlocked) __wur;
+			  FILE *__restrict __stream), fgets_unlocked)
+    __wur __attr_access ((__write_only__, 1, 2));
 extern char *__REDIRECT (__fgets_unlocked_chk_warn,
 			 (char *__restrict __s, size_t __size, int __n,
 			  FILE *__restrict __stream), __fgets_unlocked_chk)
      __wur __warnattr ("fgets_unlocked called with bigger size than length "
 		       "of destination buffer");
 
-__fortify_function __wur char *
+__fortify_function __wur __attr_access ((__write_only__, 1, 2)) char *
 fgets_unlocked (char *__restrict __s, int __n, FILE *__restrict __stream)
 {
   if (__bos (__s) != (size_t) -1)
diff --git a/libio/stdio.h b/libio/stdio.h
index 21ef36ae70..07f2d9afb5 100644
--- a/libio/stdio.h
+++ b/libio/stdio.h
@@ -566,7 +566,7 @@ extern int putw (int __w, FILE *__stream);
    This function is a possible cancellation point and therefore not
    marked with __THROW.  */
 extern char *fgets (char *__restrict __s, int __n, FILE *__restrict __stream)
-     __wur;
+     __wur __attr_access ((__write_only__, 1, 2));
 
 #if __GLIBC_USE (DEPRECATED_GETS)
 /* Get a newline-terminated string from stdin, removing the newline.
@@ -589,7 +589,8 @@ extern char *gets (char *__s) __wur __attribute_deprecated__;
    or due to the implementation it is a cancellation point and
    therefore not marked with __THROW.  */
 extern char *fgets_unlocked (char *__restrict __s, int __n,
-			     FILE *__restrict __stream) __wur;
+			     FILE *__restrict __stream) __wur
+    __attr_access ((__write_only__, 1, 2));
 #endif
 
 
diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h
index 9fa371ab86..19d9cc5cfe 100644
--- a/misc/sys/cdefs.h
+++ b/misc/sys/cdefs.h
@@ -548,4 +548,15 @@ _Static_assert (0, "IEEE 128-bits long double requires redirection on this platf
 # define __HAVE_GENERIC_SELECTION 0
 #endif
 
+#if __GNUC_PREREQ (10, 0)
+/* Designates a 1-based positional argument ref-index of pointer type
+   that can be used to access size-index elements of the pointed-to
+   array according to access mode, or at least one element when
+   size-index is not provided:
+     access (access-mode, <ref-index> [, <size-index>])  */
+#define __attr_access(x) __attribute__ ((__access__ x))
+#else
+#  define __attr_access(x)
+#endif
+
 #endif	 /* sys/cdefs.h */
diff --git a/posix/bits/unistd.h b/posix/bits/unistd.h
index b8a8211d83..725a83eb0d 100644
--- a/posix/bits/unistd.h
+++ b/posix/bits/unistd.h
@@ -21,9 +21,11 @@
 #endif
 
 extern ssize_t __read_chk (int __fd, void *__buf, size_t __nbytes,
-			   size_t __buflen) __wur;
+			   size_t __buflen)
+  __wur __attr_access ((__write_only__, 2, 3));
 extern ssize_t __REDIRECT (__read_alias, (int __fd, void *__buf,
-					  size_t __nbytes), read) __wur;
+					  size_t __nbytes), read)
+  __wur __attr_access ((__write_only__, 2, 3));
 extern ssize_t __REDIRECT (__read_chk_warn,
 			   (int __fd, void *__buf, size_t __nbytes,
 			    size_t __buflen), __read_chk)
@@ -46,15 +48,19 @@ read (int __fd, void *__buf, size_t __nbytes)
 
 #ifdef __USE_UNIX98
 extern ssize_t __pread_chk (int __fd, void *__buf, size_t __nbytes,
-			    __off_t __offset, size_t __bufsize) __wur;
+			    __off_t __offset, size_t __bufsize)
+  __wur __attr_access ((__write_only__, 2, 3));
 extern ssize_t __pread64_chk (int __fd, void *__buf, size_t __nbytes,
-			      __off64_t __offset, size_t __bufsize) __wur;
+			      __off64_t __offset, size_t __bufsize)
+  __wur __attr_access ((__write_only__, 2, 3));
 extern ssize_t __REDIRECT (__pread_alias,
 			   (int __fd, void *__buf, size_t __nbytes,
-			    __off_t __offset), pread) __wur;
+			    __off_t __offset), pread)
+  __wur __attr_access ((__write_only__, 2, 3));
 extern ssize_t __REDIRECT (__pread64_alias,
 			   (int __fd, void *__buf, size_t __nbytes,
-			    __off64_t __offset), pread64) __wur;
+			    __off64_t __offset), pread64)
+  __wur __attr_access ((__write_only__, 2, 3));
 extern ssize_t __REDIRECT (__pread_chk_warn,
 			   (int __fd, void *__buf, size_t __nbytes,
 			    __off_t __offset, size_t __bufsize), __pread_chk)
@@ -123,11 +129,11 @@ pread64 (int __fd, void *__buf, size_t __nbytes, __off64_t __offset)
 extern ssize_t __readlink_chk (const char *__restrict __path,
 			       char *__restrict __buf, size_t __len,
 			       size_t __buflen)
-     __THROW __nonnull ((1, 2)) __wur;
+     __THROW __nonnull ((1, 2)) __wur __attr_access ((__write_only__, 2, 3));
 extern ssize_t __REDIRECT_NTH (__readlink_alias,
 			       (const char *__restrict __path,
 				char *__restrict __buf, size_t __len), readlink)
-     __nonnull ((1, 2)) __wur;
+     __nonnull ((1, 2)) __wur __attr_access ((__write_only__, 2, 3));
 extern ssize_t __REDIRECT_NTH (__readlink_chk_warn,
 			       (const char *__restrict __path,
 				char *__restrict __buf, size_t __len,
@@ -155,12 +161,12 @@ __NTH (readlink (const char *__restrict __path, char *__restrict __buf,
 extern ssize_t __readlinkat_chk (int __fd, const char *__restrict __path,
 				 char *__restrict __buf, size_t __len,
 				 size_t __buflen)
-     __THROW __nonnull ((2, 3)) __wur;
+     __THROW __nonnull ((2, 3)) __wur __attr_access ((__write_only__, 3, 4));
 extern ssize_t __REDIRECT_NTH (__readlinkat_alias,
 			       (int __fd, const char *__restrict __path,
 				char *__restrict __buf, size_t __len),
 			       readlinkat)
-     __nonnull ((2, 3)) __wur;
+     __nonnull ((2, 3)) __wur __attr_access ((__write_only__, 3, 4));
 extern ssize_t __REDIRECT_NTH (__readlinkat_chk_warn,
 			       (int __fd, const char *__restrict __path,
 				char *__restrict __buf, size_t __len,
@@ -187,9 +193,10 @@ __NTH (readlinkat (int __fd, const char *__restrict __path,
 #endif
 
 extern char *__getcwd_chk (char *__buf, size_t __size, size_t __buflen)
-     __THROW __wur;
+     __THROW __wur __attr_access ((__write_only__, 1, 2));
 extern char *__REDIRECT_NTH (__getcwd_alias,
-			     (char *__buf, size_t __size), getcwd) __wur;
+			     (char *__buf, size_t __size), getcwd)
+  __wur __attr_access ((__write_only__, 1, 2));
 extern char *__REDIRECT_NTH (__getcwd_chk_warn,
 			     (char *__buf, size_t __size, size_t __buflen),
 			     __getcwd_chk)
@@ -212,7 +219,7 @@ __NTH (getcwd (char *__buf, size_t __size))
 
 #if defined __USE_MISC || defined __USE_XOPEN_EXTENDED
 extern char *__getwd_chk (char *__buf, size_t buflen)
-     __THROW __nonnull ((1)) __wur;
+     __THROW __nonnull ((1)) __wur __attr_access ((__write_only__, 1, 2));
 extern char *__REDIRECT_NTH (__getwd_warn, (char *__buf), getwd)
      __nonnull ((1)) __wur __warnattr ("please use getcwd instead, as getwd "
 				       "doesn't specify buffer size");
@@ -227,9 +234,11 @@ __NTH (getwd (char *__buf))
 #endif
 
 extern size_t __confstr_chk (int __name, char *__buf, size_t __len,
-			     size_t __buflen) __THROW;
+			     size_t __buflen) __THROW
+  __attr_access ((__write_only__, 2, 3));
 extern size_t __REDIRECT_NTH (__confstr_alias, (int __name, char *__buf,
-						size_t __len), confstr);
+						size_t __len), confstr)
+   __attr_access ((__write_only__, 2, 3));
 extern size_t __REDIRECT_NTH (__confstr_chk_warn,
 			      (int __name, char *__buf, size_t __len,
 			       size_t __buflen), __confstr_chk)
@@ -252,9 +261,9 @@ __NTH (confstr (int __name, char *__buf, size_t __len))
 
 
 extern int __getgroups_chk (int __size, __gid_t __list[], size_t __listlen)
-     __THROW __wur;
+  __THROW __wur __attr_access ((__write_only__, 2, 1));
 extern int __REDIRECT_NTH (__getgroups_alias, (int __size, __gid_t __list[]),
-			   getgroups) __wur;
+			   getgroups) __wur __attr_access ((__write_only__, 2, 1));
 extern int __REDIRECT_NTH (__getgroups_chk_warn,
 			   (int __size, __gid_t __list[], size_t __listlen),
 			   __getgroups_chk)
@@ -277,7 +286,8 @@ __NTH (getgroups (int __size, __gid_t __list[]))
 
 
 extern int __ttyname_r_chk (int __fd, char *__buf, size_t __buflen,
-			    size_t __nreal) __THROW __nonnull ((2));
+			    size_t __nreal) __THROW __nonnull ((2))
+   __attr_access ((__write_only__, 2, 3));
 extern int __REDIRECT_NTH (__ttyname_r_alias, (int __fd, char *__buf,
 					       size_t __buflen), ttyname_r)
      __nonnull ((2));
@@ -304,7 +314,7 @@ __NTH (ttyname_r (int __fd, char *__buf, size_t __buflen))
 
 #ifdef __USE_POSIX199506
 extern int __getlogin_r_chk (char *__buf, size_t __buflen, size_t __nreal)
-     __nonnull ((1));
+     __nonnull ((1)) __attr_access ((__write_only__, 1, 2));
 extern int __REDIRECT (__getlogin_r_alias, (char *__buf, size_t __buflen),
 		       getlogin_r) __nonnull ((1));
 extern int __REDIRECT (__getlogin_r_chk_warn,
@@ -331,9 +341,10 @@ getlogin_r (char *__buf, size_t __buflen)
 
 #if defined __USE_MISC || defined __USE_UNIX98
 extern int __gethostname_chk (char *__buf, size_t __buflen, size_t __nreal)
-     __THROW __nonnull ((1));
+     __THROW __nonnull ((1)) __attr_access ((__write_only__, 1, 2));
 extern int __REDIRECT_NTH (__gethostname_alias, (char *__buf, size_t __buflen),
-			   gethostname) __nonnull ((1));
+			   gethostname)
+  __nonnull ((1)) __attr_access ((__write_only__, 1, 2));
 extern int __REDIRECT_NTH (__gethostname_chk_warn,
 			   (char *__buf, size_t __buflen, size_t __nreal),
 			   __gethostname_chk)
@@ -358,10 +369,11 @@ __NTH (gethostname (char *__buf, size_t __buflen))
 
 #if defined __USE_MISC || (defined __USE_XOPEN && !defined __USE_UNIX98)
 extern int __getdomainname_chk (char *__buf, size_t __buflen, size_t __nreal)
-     __THROW __nonnull ((1)) __wur;
+     __THROW __nonnull ((1)) __wur __attr_access ((__write_only__, 1, 2));
 extern int __REDIRECT_NTH (__getdomainname_alias, (char *__buf,
 						   size_t __buflen),
-			   getdomainname) __nonnull ((1)) __wur;
+			   getdomainname) __nonnull ((1))
+  __wur __attr_access ((__write_only__, 1, 2));
 extern int __REDIRECT_NTH (__getdomainname_chk_warn,
 			   (char *__buf, size_t __buflen, size_t __nreal),
 			   __getdomainname_chk)
diff --git a/posix/unistd.h b/posix/unistd.h
index 0dd4200ad6..32b8161619 100644
--- a/posix/unistd.h
+++ b/posix/unistd.h
@@ -357,13 +357,15 @@ extern int close (int __fd);
 
    This function is a cancellation point and therefore not marked with
    __THROW.  */
-extern ssize_t read (int __fd, void *__buf, size_t __nbytes) __wur;
+extern ssize_t read (int __fd, void *__buf, size_t __nbytes) __wur
+    __attr_access ((__write_only__, 2, 3));
 
 /* Write N bytes of BUF to FD.  Return the number written, or -1.
 
    This function is a cancellation point and therefore not marked with
    __THROW.  */
-extern ssize_t write (int __fd, const void *__buf, size_t __n) __wur;
+extern ssize_t write (int __fd, const void *__buf, size_t __n) __wur
+    __attr_access ((__read_only__, 2, 3));
 
 #if defined __USE_UNIX98 || defined __USE_XOPEN2K8
 # ifndef __USE_FILE_OFFSET64
@@ -374,7 +376,8 @@ extern ssize_t write (int __fd, const void *__buf, size_t __n) __wur;
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t pread (int __fd, void *__buf, size_t __nbytes,
-		      __off_t __offset) __wur;
+		      __off_t __offset) __wur
+    __attr_access ((__write_only__, 2, 3));
 
 /* Write N bytes of BUF to FD at the given position OFFSET without
    changing the file pointer.  Return the number written, or -1.
@@ -382,15 +385,19 @@ extern ssize_t pread (int __fd, void *__buf, size_t __nbytes,
    This function is a cancellation point and therefore not marked with
    __THROW.  */
 extern ssize_t pwrite (int __fd, const void *__buf, size_t __n,
-		       __off_t __offset) __wur;
+		       __off_t __offset) __wur
+    __attr_access ((__read_only__, 2, 3));
+
 # else
 #  ifdef __REDIRECT
 extern ssize_t __REDIRECT (pread, (int __fd, void *__buf, size_t __nbytes,
 				   __off64_t __offset),
-			   pread64) __wur;
+			   pread64) __wur
+    __attr_access ((__write_only__, 2, 3));
 extern ssize_t __REDIRECT (pwrite, (int __fd, const void *__buf,
 				    size_t __nbytes, __off64_t __offset),
-			   pwrite64) __wur;
+			   pwrite64) __wur
+    __attr_access ((__read_only__, 2, 3));
 #  else
 #   define pread pread64
 #   define pwrite pwrite64
@@ -402,11 +409,13 @@ extern ssize_t __REDIRECT (pwrite, (int __fd, const void *__buf,
    changing the file pointer.  Return the number read, -1 for errors
    or 0 for EOF.  */
 extern ssize_t pread64 (int __fd, void *__buf, size_t __nbytes,
-			__off64_t __offset) __wur;
+			__off64_t __offset) __wur
+    __attr_access ((__write_only__, 2, 3));
 /* Write N bytes of BUF to FD at the given position OFFSET without
    changing the file pointer.  Return the number written, or -1.  */
 extern ssize_t pwrite64 (int __fd, const void *__buf, size_t __n,
-			 __off64_t __offset) __wur;
+			 __off64_t __offset) __wur
+    __attr_access ((__read_only__, 2, 3));
 # endif
 #endif
 
@@ -508,7 +517,8 @@ extern int fchdir (int __fd) __THROW __wur;
    an array is allocated with `malloc'; the array is SIZE
    bytes long, unless SIZE == 0, in which case it is as
    big as necessary.  */
-extern char *getcwd (char *__buf, size_t __size) __THROW __wur;
+extern char *getcwd (char *__buf, size_t __size) __THROW __wur
+    __attr_access ((__write_only__, 1, 2));
 
 #ifdef	__USE_GNU
 /* Return a malloc'd string containing the current directory name.
@@ -523,7 +533,8 @@ extern char *get_current_dir_name (void) __THROW;
    If successful, return BUF.  If not, put an error message in
    BUF and return NULL.  BUF should be at least PATH_MAX bytes long.  */
 extern char *getwd (char *__buf)
-     __THROW __nonnull ((1)) __attribute_deprecated__ __wur;
+     __THROW __nonnull ((1)) __attribute_deprecated__ __wur
+    __attr_access ((__write_only__, 1));
 #endif
 
 
@@ -620,7 +631,8 @@ extern long int sysconf (int __name) __THROW;
 
 #ifdef	__USE_POSIX2
 /* Get the value of the string-valued system variable NAME.  */
-extern size_t confstr (int __name, char *__buf, size_t __len) __THROW;
+extern size_t confstr (int __name, char *__buf, size_t __len) __THROW
+    __attr_access ((__write_only__, 2, 3));
 #endif
 
 
@@ -686,8 +698,8 @@ extern __gid_t getegid (void) __THROW;
 /* If SIZE is zero, return the number of supplementary groups
    the calling process is in.  Otherwise, fill in the group IDs
    of its supplementary groups in LIST and return the number written.  */
-extern int getgroups (int __size, __gid_t __list[]) __THROW __wur;
-
+extern int getgroups (int __size, __gid_t __list[]) __THROW __wur
+    __attr_access ((__write_only__, 2, 1));
 #ifdef	__USE_GNU
 /* Return nonzero iff the calling process is in group GID.  */
 extern int group_member (__gid_t __gid) __THROW;
@@ -772,7 +784,7 @@ extern char *ttyname (int __fd) __THROW;
 /* Store at most BUFLEN characters of the pathname of the terminal FD is
    open on in BUF.  Return 0 on success, otherwise an error number.  */
 extern int ttyname_r (int __fd, char *__buf, size_t __buflen)
-     __THROW __nonnull ((2)) __wur;
+     __THROW __nonnull ((2)) __wur __attr_access ((__write_only__, 2, 3));
 
 /* Return 1 if FD is a valid descriptor associated
    with a terminal, zero if not.  */
@@ -807,7 +819,8 @@ extern int symlink (const char *__from, const char *__to)
    Returns the number of characters read, or -1 for errors.  */
 extern ssize_t readlink (const char *__restrict __path,
 			 char *__restrict __buf, size_t __len)
-     __THROW __nonnull ((1, 2)) __wur;
+     __THROW __nonnull ((1, 2)) __wur __attr_access ((__write_only__, 2, 3));
+
 #endif /* Use POSIX.1-2001.  */
 
 #ifdef __USE_ATFILE
@@ -818,7 +831,7 @@ extern int symlinkat (const char *__from, int __tofd,
 /* Like readlink but a relative PATH is interpreted relative to FD.  */
 extern ssize_t readlinkat (int __fd, const char *__restrict __path,
 			   char *__restrict __buf, size_t __len)
-     __THROW __nonnull ((2, 3)) __wur;
+     __THROW __nonnull ((2, 3)) __wur __attr_access ((__read_only__, 3, 4));
 #endif
 
 /* Remove the link NAME.  */
@@ -853,7 +866,8 @@ extern char *getlogin (void);
 
    This function is a possible cancellation point and therefore not
    marked with __THROW.  */
-extern int getlogin_r (char *__name, size_t __name_len) __nonnull ((1));
+extern int getlogin_r (char *__name, size_t __name_len) __nonnull ((1))
+    __attr_access ((__write_only__, 1, 2));
 #endif
 
 #ifdef	__USE_MISC
@@ -874,7 +888,8 @@ extern int setlogin (const char *__name) __THROW __nonnull ((1));
 /* Put the name of the current host in no more than LEN bytes of NAME.
    The result is null-terminated if LEN is large enough for the full
    name and the terminator.  */
-extern int gethostname (char *__name, size_t __len) __THROW __nonnull ((1));
+extern int gethostname (char *__name, size_t __len) __THROW __nonnull ((1))
+    __attr_access ((__write_only__, 1, 2));
 #endif
 
 
@@ -882,7 +897,7 @@ extern int gethostname (char *__name, size_t __len) __THROW __nonnull ((1));
 /* Set the name of the current host to NAME, which is LEN bytes long.
    This call is restricted to the super-user.  */
 extern int sethostname (const char *__name, size_t __len)
-     __THROW __nonnull ((1)) __wur;
+     __THROW __nonnull ((1)) __wur __attr_access ((__read_only__, 1, 2));
 
 /* Set the current machine's Internet number to ID.
    This call is restricted to the super-user.  */
@@ -893,10 +908,9 @@ extern int sethostid (long int __id) __THROW __wur;
    Called just like `gethostname' and `sethostname'.
    The NIS domain name is usually the empty string when not using NIS.  */
 extern int getdomainname (char *__name, size_t __len)
-     __THROW __nonnull ((1)) __wur;
+     __THROW __nonnull ((1)) __wur __attr_access ((__write_only__, 1, 2));
 extern int setdomainname (const char *__name, size_t __len)
-     __THROW __nonnull ((1)) __wur;
-
+     __THROW __nonnull ((1)) __wur __attr_access ((__read_only__, 1, 2));
 
 /* Revoke access permissions to all processes currently communicating
    with the control terminal, and then send a SIGHUP signal to the process
@@ -1131,7 +1145,9 @@ extern char *crypt (const char *__key, const char *__salt)
    range [FROM - N + 1, FROM - 1].  If N is odd the first byte in FROM
    is without partner.  */
 extern void swab (const void *__restrict __from, void *__restrict __to,
-		  ssize_t __n) __THROW __nonnull ((1, 2));
+		  ssize_t __n) __THROW __nonnull ((1, 2))
+    __attr_access ((__read_only__, 1, 3))
+    __attr_access ((__write_only__, 2, 3));
 #endif
 
 
@@ -1158,7 +1174,8 @@ extern int pthread_atfork (void (*__prepare) (void),
 #ifdef __USE_MISC
 /* Write LENGTH bytes of randomness starting at BUFFER.  Return 0 on
    success or -1 on error.  */
-int getentropy (void *__buffer, size_t __length) __wur;
+int getentropy (void *__buffer, size_t __length) __wur
+    __attr_access ((__write_only__, 1, 2));
 #endif
 
 /* Define some macros helping to catch buffer overflows.  */
diff --git a/stdlib/bits/stdlib.h b/stdlib/bits/stdlib.h
index bfdee75073..9134d3f36b 100644
--- a/stdlib/bits/stdlib.h
+++ b/stdlib/bits/stdlib.h
@@ -50,10 +50,11 @@ __NTH (realpath (const char *__restrict __name, char *__restrict __resolved))
 
 
 extern int __ptsname_r_chk (int __fd, char *__buf, size_t __buflen,
-			    size_t __nreal) __THROW __nonnull ((2));
+			    size_t __nreal) __THROW __nonnull ((2))
+    __attr_access ((__write_only__, 2, 3));
 extern int __REDIRECT_NTH (__ptsname_r_alias, (int __fd, char *__buf,
 					       size_t __buflen), ptsname_r)
-     __nonnull ((2));
+     __nonnull ((2)) __attr_access ((__write_only__, 2, 3));
 extern int __REDIRECT_NTH (__ptsname_r_chk_warn,
 			   (int __fd, char *__buf, size_t __buflen,
 			    size_t __nreal), __ptsname_r_chk)
@@ -97,11 +98,13 @@ __NTH (wctomb (char *__s, wchar_t __wchar))
 
 extern size_t __mbstowcs_chk (wchar_t *__restrict __dst,
 			      const char *__restrict __src,
-			      size_t __len, size_t __dstlen) __THROW;
+			      size_t __len, size_t __dstlen) __THROW
+    __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
 extern size_t __REDIRECT_NTH (__mbstowcs_alias,
 			      (wchar_t *__restrict __dst,
 			       const char *__restrict __src,
-			       size_t __len), mbstowcs);
+			       size_t __len), mbstowcs)
+    __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
 extern size_t __REDIRECT_NTH (__mbstowcs_chk_warn,
 			      (wchar_t *__restrict __dst,
 			       const char *__restrict __src,
@@ -129,11 +132,13 @@ __NTH (mbstowcs (wchar_t *__restrict __dst, const char *__restrict __src,
 
 extern size_t __wcstombs_chk (char *__restrict __dst,
 			      const wchar_t *__restrict __src,
-			      size_t __len, size_t __dstlen) __THROW;
+			      size_t __len, size_t __dstlen) __THROW
+  __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
 extern size_t __REDIRECT_NTH (__wcstombs_alias,
 			      (char *__restrict __dst,
 			       const wchar_t *__restrict __src,
-			       size_t __len), wcstombs);
+			       size_t __len), wcstombs)
+  __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
 extern size_t __REDIRECT_NTH (__wcstombs_chk_warn,
 			      (char *__restrict __dst,
 			       const wchar_t *__restrict __src,
diff --git a/stdlib/stdlib.h b/stdlib/stdlib.h
index 9b7537c545..dd779bd740 100644
--- a/stdlib/stdlib.h
+++ b/stdlib/stdlib.h
@@ -931,12 +931,13 @@ extern int wctomb (char *__s, wchar_t __wchar) __THROW;
 
 /* Convert a multibyte string to a wide char string.  */
 extern size_t mbstowcs (wchar_t *__restrict  __pwcs,
-			const char *__restrict __s, size_t __n) __THROW;
+			const char *__restrict __s, size_t __n) __THROW
+    __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
 /* Convert a wide char string to multibyte string.  */
 extern size_t wcstombs (char *__restrict __s,
 			const wchar_t *__restrict __pwcs, size_t __n)
-     __THROW;
-
+     __THROW
+  __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
 
 #ifdef __USE_MISC
 /* Determine whether the string value of RESPONSE matches the affirmation
@@ -990,7 +991,7 @@ extern char *ptsname (int __fd) __THROW __wur;
    terminal associated with the master FD is open on in BUF.
    Return 0 on success, otherwise an error number.  */
 extern int ptsname_r (int __fd, char *__buf, size_t __buflen)
-     __THROW __nonnull ((2));
+     __THROW __nonnull ((2)) __attr_access ((__write_only__, 2, 3));
 
 /* Open a master pseudo terminal and return its file descriptor.  */
 extern int getpt (void);
diff --git a/string/bits/string_fortified.h b/string/bits/string_fortified.h
index e4d07cb50c..309d0f39b2 100644
--- a/string/bits/string_fortified.h
+++ b/string/bits/string_fortified.h
@@ -75,7 +75,7 @@ __NTH (memset (void *__dest, int __ch, size_t __len))
 # include <bits/strings_fortified.h>
 
 void __explicit_bzero_chk (void *__dest, size_t __len, size_t __destlen)
-  __THROW __nonnull ((1));
+  __THROW __nonnull ((1)) __attr_access ((__write_only__, 1, 2));
 
 __fortify_function void
 __NTH (explicit_bzero (void *__dest, size_t __len))
@@ -108,7 +108,8 @@ __NTH (strncpy (char *__restrict __dest, const char *__restrict __src,
 
 /* XXX We have no corresponding builtin yet.  */
 extern char *__stpncpy_chk (char *__dest, const char *__src, size_t __n,
-			    size_t __destlen) __THROW;
+			    size_t __destlen) __THROW
+  __attr_access ((__write_only__, 1, 3)) __attr_access ((__read_only__, 2));
 extern char *__REDIRECT_NTH (__stpncpy_alias, (char *__dest, const char *__src,
 					       size_t __n), stpncpy);
 
diff --git a/string/string.h b/string/string.h
index a0f2860cc2..d7ce0f4a1b 100644
--- a/string/string.h
+++ b/string/string.h
@@ -53,7 +53,7 @@ extern void *memmove (void *__dest, const void *__src, size_t __n)
 #if defined __USE_MISC || defined __USE_XOPEN || __GLIBC_USE (ISOC2X)
 extern void *memccpy (void *__restrict __dest, const void *__restrict __src,
 		      int __c, size_t __n)
-     __THROW __nonnull ((1, 2));
+    __THROW __nonnull ((1, 2)) __attr_access ((__write_only__, 1, 4));
 #endif /* Misc || X/Open.  */
 
 
@@ -108,12 +108,15 @@ extern void *rawmemchr (const void *__s, int __c)
 /* Search N bytes of S for the final occurrence of C.  */
 # ifdef __CORRECT_ISO_CPP_STRING_H_PROTO
 extern "C++" void *memrchr (void *__s, int __c, size_t __n)
-      __THROW __asm ("memrchr") __attribute_pure__ __nonnull ((1));
+      __THROW __asm ("memrchr") __attribute_pure__ __nonnull ((1))
+      __attr_access ((__read_only__, 1, 3));
 extern "C++" const void *memrchr (const void *__s, int __c, size_t __n)
-      __THROW __asm ("memrchr") __attribute_pure__ __nonnull ((1));
+      __THROW __asm ("memrchr") __attribute_pure__ __nonnull ((1))
+      __attr_access ((__read_only__, 1, 3));
 # else
 extern void *memrchr (const void *__s, int __c, size_t __n)
-      __THROW __attribute_pure__ __nonnull ((1));
+      __THROW __attribute_pure__ __nonnull ((1))
+      __attr_access ((__read_only__, 1, 3));
 # endif
 #endif
 
@@ -146,7 +149,7 @@ extern int strcoll (const char *__s1, const char *__s2)
 /* Put a transformation of SRC into no more than N bytes of DEST.  */
 extern size_t strxfrm (char *__restrict __dest,
 		       const char *__restrict __src, size_t __n)
-     __THROW __nonnull ((2));
+    __THROW __nonnull ((2)) __attr_access ((__write_only__, 1, 3));
 
 #ifdef __USE_XOPEN2K8
 /* POSIX.1-2008 extended locale interface (see locale.h).  */
@@ -158,7 +161,8 @@ extern int strcoll_l (const char *__s1, const char *__s2, locale_t __l)
 /* Put a transformation of SRC into no more than N bytes of DEST,
    using sorting rules from L.  */
 extern size_t strxfrm_l (char *__dest, const char *__src, size_t __n,
-			 locale_t __l) __THROW __nonnull ((2, 4));
+			 locale_t __l) __THROW __nonnull ((2, 4))
+     __attr_access ((__write_only__, 1, 3));
 #endif
 
 #if (defined __USE_XOPEN_EXTENDED || defined __USE_XOPEN2K8	\
@@ -368,7 +372,9 @@ extern char *strcasestr (const char *__haystack, const char *__needle)
    HAYSTACK is HAYSTACKLEN bytes long.  */
 extern void *memmem (const void *__haystack, size_t __haystacklen,
 		     const void *__needle, size_t __needlelen)
-     __THROW __attribute_pure__ __nonnull ((1, 3));
+     __THROW __attribute_pure__ __nonnull ((1, 3))
+    __attr_access ((__read_only__, 1, 2))
+    __attr_access ((__read_only__, 3, 4));
 
 /* Copy N bytes of SRC to DEST, return pointer to bytes after the
    last written byte.  */
@@ -409,17 +415,18 @@ extern char *strerror (int __errnum) __THROW;
 #  ifdef __REDIRECT_NTH
 extern int __REDIRECT_NTH (strerror_r,
 			   (int __errnum, char *__buf, size_t __buflen),
-			   __xpg_strerror_r) __nonnull ((2));
+			   __xpg_strerror_r) __nonnull ((2))
+    __attr_access ((__write_only__, 2, 3));
 #  else
 extern int __xpg_strerror_r (int __errnum, char *__buf, size_t __buflen)
-     __THROW __nonnull ((2));
+     __THROW __nonnull ((2)) __attr_access ((__write_only__, 2, 3));
 #   define strerror_r __xpg_strerror_r
 #  endif
 # else
 /* If a temporary buffer is required, at most BUFLEN bytes of BUF will be
    used.  */
 extern char *strerror_r (int __errnum, char *__buf, size_t __buflen)
-     __THROW __nonnull ((2)) __wur;
+     __THROW __nonnull ((2)) __wur  __attr_access ((__write_only__, 2, 3));
 # endif
 #endif
 
@@ -433,7 +440,8 @@ extern char *strerror_l (int __errnum, locale_t __l) __THROW;
 
 /* Set N bytes of S to 0.  The compiler will not delete a call to this
    function, even if S is dead after the call.  */
-extern void explicit_bzero (void *__s, size_t __n) __THROW __nonnull ((1));
+extern void explicit_bzero (void *__s, size_t __n) __THROW __nonnull ((1))
+    __attr_access ((__write_only__, 1, 2));
 
 /* Return the next DELIM-delimited token from *STRINGP,
    terminating it with a '\0', and update *STRINGP to point past it.  */
@@ -471,7 +479,8 @@ extern int strverscmp (const char *__s1, const char *__s2)
 extern char *strfry (char *__string) __THROW __nonnull ((1));
 
 /* Frobnicate N bytes of S.  */
-extern void *memfrob (void *__s, size_t __n) __THROW __nonnull ((1));
+extern void *memfrob (void *__s, size_t __n) __THROW __nonnull ((1))
+    __attr_access ((__write_only__, 1, 2));
 
 # ifndef basename
 /* Return the file name within directory of FILENAME.  We don't

[-- Attachment #3: test-access-warn.c --]
[-- Type: text/x-csrc, Size: 8945 bytes --]

/* BZ #25219 - improve out-of-bounds checking with GCC 10 attribute access */

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

/* <stdio.h> */

int nowarn_fgets (FILE *f, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return !!fgets (a + i, sizeof a, f);   // nowarn
}

int warn_fgets (FILE *f, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return !!fgets (a + i, sizeof a, f);   // warn
}


int nowarn_fgets_unlocked (FILE *f, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return !!fgets_unlocked (a + i, sizeof a, f);   // nowarn
}

int warn_fgets_unlocked (FILE *f, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return !!fgets_unlocked (a + i, sizeof a, f);   // warn
}


int nowarn_sprintf (int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return sprintf (a + i, "123");   // nowarn
}

int warn_sprintf (int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return sprintf (a + i, "123");   // warn
}


int nowarn_snprintf (int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return snprintf (a + i, sizeof a, "123");   // nowarn
}

int warn_snprintf (int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return snprintf (a + i, sizeof a, "123");   // warn
}


int nowarn_vsprintf (va_list va, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return vsprintf (a + i, "124", va);   // nowarn
}

int warn_vsprintf (va_list va, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return vsprintf (a + i, "123", va);   // warn
}


int nowarn_vsnprintf (va_list va, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return vsnprintf (a + i, sizeof a, "124", va);   // nowarn
}

int warn_vsnprintf (va_list va, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return vsnprintf (a + i, sizeof a, "123", va);   // warn
}


/* <stdlib.h> */

int nowarn_ptsname_r (int fd, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return ptsname_r (fd, a + i, sizeof a);   // nowarn
}

int warn_ptsname_r (int fd, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return ptsname_r (fd, a + i, sizeof a);   // warn
}


int nowarn_mbstowcs (const char *s, int i)
{
  wchar_t a[4];
  if (i < 0)
    i = 0;
  return mbstowcs (a + i, s, sizeof a / sizeof *a);   // nowarn
}

int warn_mbstowcs (const char *s, int i)
{
  wchar_t a[4];
  if (i < 1)
    i = 1;
  return mbstowcs (a + i, s, sizeof a / sizeof *a);   // warn
}


int nowarn_wcstombs (const wchar_t *s, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return wcstombs (a + i, s, sizeof a / sizeof *a);   // nowarn
}

int warn_wcstombs (const wchar_t *s, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return wcstombs (a + i, s, sizeof a / sizeof *a);   // warn
}


/* <string.h> */

int nowarn_memccpy_1 (int i)
{
  char a[4], b[4] = "123";
  if (i < 0)
    i = 0;
  return !!memccpy (a + i, b, 'x', sizeof a);   // nowarn
}

int nowarn_memccpy_2 (int i)
{
  char a[4], b[4] = "123";
  if (i < 0)
    i = 0;
  return !!memccpy (a, b + i, 'x', sizeof b);   // nowarn
}

int warn_memccpy_1 (int i)
{
  char a[4], b[4] = "123";
  if (i < 1)
    i = 1;
  return !!memccpy (a + i, b, 'x', sizeof a);   // warn
}

int warn_memccpy_2 (int i)
{
  char a[4], b[4] = "123";
  if (i < 1)
    i = 1;
  // This needs GCC support to see that B does not contain an 'x'.
  return !!memccpy (a, b + i, 'x', sizeof b);   // should warn
}


int nowarn_memrchr (int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return !!memrchr (a + i, 'x', sizeof a);   // nowarn
}

int warn_memrchr (int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return !!memrchr (a + i, 'x', sizeof a);   // warn
}


int nowarn_strxfrm (const char *s, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return strxfrm (a + i, s, sizeof a);   // nowarn
}

int warn_strxfrm (const char *s, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return strxfrm (a + i, s, sizeof a);   // warn
}


int nowarn_memmem_1 (int i)
{
  char a[4] = "123";
  char b[4] = "321";
  if (i < 0)
    i = 0;
  return !!memmem (a + i, sizeof a, b, sizeof b);   // nowarn
}

int nowarn_memmem_2 (int i)
{
  char a[4] = "123";
  char b[4] = "321";
  if (i < 0)
    i = 0;
  return !!memmem (a, sizeof a, b + i, sizeof b);   // nowarn
}

int warn_memmem_1 (int i)
{
  char a[4] = "123";
  char b[4] = "321";
  if (i < 1)
    i = 1;
  return !!memmem (a + i, sizeof a, b, sizeof b);   // warn
}

int warn_memmem_2 (int i)
{
  char a[4] = "123";
  char b[4] = "321";
  if (i < 1)
    i = 1;
  return !!memmem (a, sizeof a, b + i, sizeof b);   // warn
}


int nowarn_strerror_r (int e, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return !!strerror_r (e, a + i, sizeof a);   // nowarn
}

int warn_strerror_r (int e, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return !!strerror_r (e, a + i, sizeof a);   // warn
}


void nowarn_explicit_bzero (int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  explicit_bzero (a + i, sizeof a);   // nowarn
}

void warn_explicit_bzero (int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  explicit_bzero (a + i, sizeof a);   // warn
}


void nowarn_memfrob (int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  memfrob (a + i, sizeof a);   // nowarn
}

void warn_memfrob (int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  memfrob (a + i, sizeof a);   // warn
}

/* <unistd.h> */


int nowarn_read (int fd, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return read (fd, a + i, sizeof a);   // nowarn
}

int warn_read (int fd, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return read (fd, a + i, sizeof a);   // warn
}


int nowarn_pread (int fd, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return pread (fd, a + i, sizeof a, 0);   // nowarn
}

int warn_pread (int fd, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return pread (fd, a + i, sizeof a, 0);   // warn
}


int nowarn_pread64 (int fd, int i)
{
  char a[4];
  if (i < 0)
    i = 1;
  return pread64 (fd, a + i, sizeof a, 0);   // nowarn
}

int warn_pread64 (int fd, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return pread64 (fd, a + i, sizeof a, 0);   // warn
}


int nowarn_readlink (const char *path, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return readlink (path, a + i, sizeof a);   // nowarn
}

int warn_readlink (const char *path, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return readlink (path, a + i, sizeof a);   // warn
}


int nowarn_readlinkat (int fd, const char *path, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return readlinkat (fd, path, a + i, sizeof a);   // nowarn
}

int warn_readlinkat (int fd, const char *path, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return readlinkat (fd, path, a + i, sizeof a);   // warn
}


char* nowarn_getcwd (int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return getcwd (a + i, sizeof a);   // nowarn
}

char* warn_getcwd (int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return getcwd (a + i, sizeof a);   // warn
}


char* nowarn_getwd (int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return getwd (a + i);   // nowarn
}

char* warn_getwd (int i)
{
  char a[4];
  if (i < 4)
    i = 4;
  return getwd (a + i);   // warn
}


int nowarn_confstr (int name, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return confstr (name, a + i, sizeof a);   // nowarn
}

int warn_confstr (int name, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return confstr (name, a + i, sizeof a);   // warn
}


int nowarn_ttyname_r (int name, int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return ttyname_r (name, a + i, sizeof a);   // nowarn
}

int warn_ttyname_r (int name, int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return ttyname_r (name, a + i, sizeof a);   // warn
}


int nowarn_getlogin_r (int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return getlogin_r (a + i, sizeof a);   // nowarn
}

int warn_getlogin_r (int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return getlogin_r (a + i, sizeof a);   // warn
}


int nowarn_gethostname (int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return gethostname (a + i, sizeof a);   // nowarn
}

int warn_gethostname (int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return gethostname (a + i, sizeof a);   // warn
}


int nowarn_getdomainname (int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return getdomainname (a + i, sizeof a);   // nowarn
}

int warn_getdomainname (int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return getdomainname (a + i, sizeof a);   // warn
}


void nowarn_swab_1 (int i)
{
  char a[4], b[4];
  if (i < 0)
    i = 0;
  swab (a + i, b, sizeof a);   // nowarn
}

void warn_swab_1 (int i)
{
  char a[4], b[4];
  if (i < 1)
    i = 1;
  swab (a + i, b, sizeof a);   // warn
}


void nowarn_swab_2 (int i)
{
  char a[4], b[4];
  if (i < 0)
    i = 0;
  swab (a, b + i, sizeof a);   // nowarn
}

void warn_swab_2 (int i)
{
  char a[4], b[4];
  if (i < 1)
    i = 1;
  swab (a, b + i, sizeof a);   // warn
}


int nowarn_getentropy (int i)
{
  char a[4];
  if (i < 0)
    i = 0;
  return getentropy (a + i, sizeof a);   // nowarn
}

int warn_getentropy (int i)
{
  char a[4];
  if (i < 1)
    i = 1;
  return getentropy (a + i, sizeof a);   // warn
}

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

* Re: [PATCH] improve out-of-bounds checking with GCC 10 attribute access [BZ #25219]
  2020-05-01 19:54   ` Martin Sebor
@ 2020-05-01 22:02     ` DJ Delorie
  2020-05-04 17:34       ` Martin Sebor
  0 siblings, 1 reply; 11+ messages in thread
From: DJ Delorie @ 2020-05-01 22:02 UTC (permalink / raw)
  To: Martin Sebor; +Cc: libc-alpha

Martin Sebor <msebor@gmail.com> writes:
> Thanks for the careful review!

This new version LGTM.

Reviewed-by: DJ Delorie <dj@redhat.com>

(but IMHO a second set of eyes would be good for this one)

> Yes, that's wrong.  Good catch!  I completely missed stdio when
> testing so I also didn't notice I forgot to add the attribute to
> fgets() itself.  I've fixed that in the updated patch.

Ok.

>> IMHO comment should state that the first argument is index 1.
>> 
>> IMHO should document what happens when size-index is missing.
>
> I've tweaked the comment a bit.  I hesitate to go into a lot of
> detail here and would expect people needing it to read the manual.

Right, but there should be just enough info for someone adding a new use
of it to know what to do, without requiring the gcc docs.  The new
comment is fine.

>> __buf[???]
>
> When size-index is missing at least one byte of the array must be
> accessible (or the pointer must be null).  There's no way to specify
> a constant size with the current syntax.  In the future I'd like to
> try to teach GCC to get it from the argument itself (for ordinary
> arrays as well as for VLAs):

Makes sense, just didn't know.

>> NOTE: does not use the __attr_access macro
>
> Fixed, thanks.

Ok.


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

* Re: [PATCH] improve out-of-bounds checking with GCC 10 attribute access [BZ #25219]
  2020-05-01 22:02     ` DJ Delorie
@ 2020-05-04 17:34       ` Martin Sebor
  2020-05-04 18:40         ` Martin Sebor
  2020-05-06 20:44         ` Joseph Myers
  0 siblings, 2 replies; 11+ messages in thread
From: Martin Sebor @ 2020-05-04 17:34 UTC (permalink / raw)
  To: DJ Delorie; +Cc: libc-alpha

On 5/1/20 4:02 PM, DJ Delorie wrote:
> Martin Sebor <msebor@gmail.com> writes:
>> Thanks for the careful review!
> 
> This new version LGTM.
> 
> Reviewed-by: DJ Delorie <dj@redhat.com>
> 
> (but IMHO a second set of eyes would be good for this one)
> 
>> Yes, that's wrong.  Good catch!  I completely missed stdio when
>> testing so I also didn't notice I forgot to add the attribute to
>> fgets() itself.  I've fixed that in the updated patch.
> 
> Ok.
> 
>>> IMHO comment should state that the first argument is index 1.
>>>
>>> IMHO should document what happens when size-index is missing.
>>
>> I've tweaked the comment a bit.  I hesitate to go into a lot of
>> detail here and would expect people needing it to read the manual.
> 
> Right, but there should be just enough info for someone adding a new use
> of it to know what to do, without requiring the gcc docs.  The new
> comment is fine.
> 
>>> __buf[???]
>>
>> When size-index is missing at least one byte of the array must be
>> accessible (or the pointer must be null).  There's no way to specify
>> a constant size with the current syntax.  In the future I'd like to
>> try to teach GCC to get it from the argument itself (for ordinary
>> arrays as well as for VLAs):
> 
> Makes sense, just didn't know.
> 
>>> NOTE: does not use the __attr_access macro
>>
>> Fixed, thanks.
> 
> Ok.
> 

Thanks.  I have committed the latest patch in
06febd8c6705c816b2f32ee7aa1f4c0184b05248.

Martin

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

* Re: [PATCH] improve out-of-bounds checking with GCC 10 attribute access [BZ #25219]
  2020-05-04 17:34       ` Martin Sebor
@ 2020-05-04 18:40         ` Martin Sebor
  2020-05-06 20:44         ` Joseph Myers
  1 sibling, 0 replies; 11+ messages in thread
From: Martin Sebor @ 2020-05-04 18:40 UTC (permalink / raw)
  To: DJ Delorie; +Cc: libc-alpha

On 5/4/20 11:34 AM, Martin Sebor wrote:
> On 5/1/20 4:02 PM, DJ Delorie wrote:
>> Martin Sebor <msebor@gmail.com> writes:
>>> Thanks for the careful review!
>>
>> This new version LGTM.
>>
>> Reviewed-by: DJ Delorie <dj@redhat.com>
>>
>> (but IMHO a second set of eyes would be good for this one)

I missed this suggestion.  I'd certainly welcome another pair of eyes
on this.  Just to be clear: unlike _FORTIFY_SOURCE, the effect of
the changes is only to trigger warnings in response to the detected
overflow, not actually cause calls to abort at runtime in those cases.

Martin


>>
>>> Yes, that's wrong.  Good catch!  I completely missed stdio when
>>> testing so I also didn't notice I forgot to add the attribute to
>>> fgets() itself.  I've fixed that in the updated patch.
>>
>> Ok.
>>
>>>> IMHO comment should state that the first argument is index 1.
>>>>
>>>> IMHO should document what happens when size-index is missing.
>>>
>>> I've tweaked the comment a bit.  I hesitate to go into a lot of
>>> detail here and would expect people needing it to read the manual.
>>
>> Right, but there should be just enough info for someone adding a new use
>> of it to know what to do, without requiring the gcc docs.  The new
>> comment is fine.
>>
>>>> __buf[???]
>>>
>>> When size-index is missing at least one byte of the array must be
>>> accessible (or the pointer must be null).  There's no way to specify
>>> a constant size with the current syntax.  In the future I'd like to
>>> try to teach GCC to get it from the argument itself (for ordinary
>>> arrays as well as for VLAs):
>>
>> Makes sense, just didn't know.
>>
>>>> NOTE: does not use the __attr_access macro
>>>
>>> Fixed, thanks.
>>
>> Ok.
>>
> 
> Thanks.  I have committed the latest patch in
> 06febd8c6705c816b2f32ee7aa1f4c0184b05248.
> 
> Martin


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

* Re: [PATCH] improve out-of-bounds checking with GCC 10 attribute access [BZ #25219]
  2020-05-04 17:34       ` Martin Sebor
  2020-05-04 18:40         ` Martin Sebor
@ 2020-05-06 20:44         ` Joseph Myers
  2020-05-06 21:08           ` DJ Delorie
  2020-05-06 22:09           ` Martin Sebor
  1 sibling, 2 replies; 11+ messages in thread
From: Joseph Myers @ 2020-05-06 20:44 UTC (permalink / raw)
  To: Martin Sebor; +Cc: DJ Delorie, libc-alpha

How exactly was this patch tested?  I'm seeing glibc testsuite build 
failures with GCC 10 on what looks like every configuration supported by 
build-many-glibcs.py, with errors referencing these attributes.  (I 
haven't bisected to confirm the responsible commit; this is with the first 
run of my bot for GCC 10, and my GCC mainline bot had the build fail 
earlier because of GCC bugs at the relevant time.)

test-errno.c: In function 'do_test':
test-errno.c:122:30: error: argument 1 value -1 is negative [-Werror=stringop-overflow=]
  122 |   fails |= test_wrp (EINVAL, getgroups, -1, 0);
      |                              ^
test-errno.c:65:17: note: in definition of macro 'test_wrp_rv'
   65 |     rtype ret = syscall (__VA_ARGS__);    \
      |                 ^~~~~~~
test-errno.c:122:12: note: in expansion of macro 'test_wrp'
  122 |   fails |= test_wrp (EINVAL, getgroups, -1, 0);
      |            ^~~~~~~~
In file included from ../include/unistd.h:2,
                 from test-errno.c:35:
../posix/unistd.h:701:12: note: in a call to function 'getgroups' declared with attribute 'write_only (2, 1)'
  701 | extern int getgroups (int __size, __gid_t __list[]) __THROW __wur
      |            ^~~~~~~~~
test-errno.c:137:30: error: 'readlink' specified size 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
  137 |   fails |= test_wrp (EINVAL, readlink, "/", buf, -1);
      |                              ^
test-errno.c:65:17: note: in definition of macro 'test_wrp_rv'
   65 |     rtype ret = syscall (__VA_ARGS__);    \
      |                 ^~~~~~~
test-errno.c:137:12: note: in expansion of macro 'test_wrp'
  137 |   fails |= test_wrp (EINVAL, readlink, "/", buf, -1);
      |            ^~~~~~~~
In file included from ../include/unistd.h:2,
                 from test-errno.c:35:
../posix/unistd.h:820:16: note: in a call to function 'readlink' declared with attribute 'write_only (2, 3)'
  820 | extern ssize_t readlink (const char *__restrict __path,
      |                ^~~~~~~~


-- 
Joseph S. Myers
joseph@codesourcery.com

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

* Re: [PATCH] improve out-of-bounds checking with GCC 10 attribute access [BZ #25219]
  2020-05-06 20:44         ` Joseph Myers
@ 2020-05-06 21:08           ` DJ Delorie
  2020-05-06 22:09           ` Martin Sebor
  1 sibling, 0 replies; 11+ messages in thread
From: DJ Delorie @ 2020-05-06 21:08 UTC (permalink / raw)
  To: Joseph Myers; +Cc: msebor, libc-alpha

Joseph Myers <joseph@codesourcery.com> writes:
> test-errno.c:122:30: error: argument 1 value -1 is negative [-Werror=stringop-overflow=]

"size" is "int" but passing a negative buffer size?  If gcc knows that
the size is a count of elements, a warning (or error, if -Werror) seems
appropriate (if annoying ;).  However, I can see we're now in the "how
do I dumb down gcc so I can test things I know it knows are errors?"
territory.

Probably a #pramga GCC warning no-stringop-overflow or equivalent for
that whole test.

> test-errno.c:137:30: error: 'readlink' specified size 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]

Here you're passing -1 to a size_t argument, I think that test needs
tweaking anyway.

>   137 |   fails |= test_wrp (EINVAL, readlink, "/", buf, -1);


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

* Re: [PATCH] improve out-of-bounds checking with GCC 10 attribute access [BZ #25219]
  2020-05-06 20:44         ` Joseph Myers
  2020-05-06 21:08           ` DJ Delorie
@ 2020-05-06 22:09           ` Martin Sebor
  2020-05-06 22:27             ` Joseph Myers
  1 sibling, 1 reply; 11+ messages in thread
From: Martin Sebor @ 2020-05-06 22:09 UTC (permalink / raw)
  To: Joseph Myers; +Cc: DJ Delorie, libc-alpha

On 5/6/20 2:44 PM, Joseph Myers wrote:
> How exactly was this patch tested?

Besides using the test file attached to the patch I also built glibc
on x86_64-linux, ran the test suite, and compared the results to
an unpatched baseline.  I didn't get any errors but apparently that
was because my test script disables -Werror (I forgot about that,
sorry).

> I'm seeing glibc testsuite build
> failures with GCC 10 on what looks like every configuration supported by
> build-many-glibcs.py, with errors referencing these attributes.  (I
> haven't bisected to confirm the responsible commit; this is with the first
> run of my bot for GCC 10, and my GCC mainline bot had the build fail
> earlier because of GCC bugs at the relevant time.)

The warnings look correct to me.  They should be suppressed for
the tests that exercise the APIs failure modes.  There are a few
others that probably fall into the same category:

-Wstringop-overflow Instances:
   ../libio/bits/stdio2.h:70
   ../string/bits/string_fortified.h:106
   ../string/bits/string_fortified.h:83
   test-errno.c:122
   test-errno.c:137

My baseline log shows just the two below (plus a number of
-Wattribute-warning, -Warray-bounds, and -Wunused-value instances);

-Wstringop-overflow Instances:
   ../libio/bits/stdio2.h:67
   ../string/bits/string_fortified.h:106

I can look into suppressing the new ones.  What's the usual way of
doing that in Glibc tests?

Martin

> 
> test-errno.c: In function 'do_test':
> test-errno.c:122:30: error: argument 1 value -1 is negative [-Werror=stringop-overflow=]
>    122 |   fails |= test_wrp (EINVAL, getgroups, -1, 0);
>        |                              ^
> test-errno.c:65:17: note: in definition of macro 'test_wrp_rv'
>     65 |     rtype ret = syscall (__VA_ARGS__);    \
>        |                 ^~~~~~~
> test-errno.c:122:12: note: in expansion of macro 'test_wrp'
>    122 |   fails |= test_wrp (EINVAL, getgroups, -1, 0);
>        |            ^~~~~~~~
> In file included from ../include/unistd.h:2,
>                   from test-errno.c:35:
> ../posix/unistd.h:701:12: note: in a call to function 'getgroups' declared with attribute 'write_only (2, 1)'
>    701 | extern int getgroups (int __size, __gid_t __list[]) __THROW __wur
>        |            ^~~~~~~~~
> test-errno.c:137:30: error: 'readlink' specified size 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
>    137 |   fails |= test_wrp (EINVAL, readlink, "/", buf, -1);
>        |                              ^
> test-errno.c:65:17: note: in definition of macro 'test_wrp_rv'
>     65 |     rtype ret = syscall (__VA_ARGS__);    \
>        |                 ^~~~~~~
> test-errno.c:137:12: note: in expansion of macro 'test_wrp'
>    137 |   fails |= test_wrp (EINVAL, readlink, "/", buf, -1);
>        |            ^~~~~~~~
> In file included from ../include/unistd.h:2,
>                   from test-errno.c:35:
> ../posix/unistd.h:820:16: note: in a call to function 'readlink' declared with attribute 'write_only (2, 3)'
>    820 | extern ssize_t readlink (const char *__restrict __path,
>        |                ^~~~~~~~
> 
> 


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

* Re: [PATCH] improve out-of-bounds checking with GCC 10 attribute access [BZ #25219]
  2020-05-06 22:09           ` Martin Sebor
@ 2020-05-06 22:27             ` Joseph Myers
  0 siblings, 0 replies; 11+ messages in thread
From: Joseph Myers @ 2020-05-06 22:27 UTC (permalink / raw)
  To: Martin Sebor; +Cc: DJ Delorie, libc-alpha

On Wed, 6 May 2020, Martin Sebor wrote:

> I can look into suppressing the new ones.  What's the usual way of
> doing that in Glibc tests?

We typically use the DIAG_* macros from libc-diag.h, with appropriate 
comments justifying the use in a particular case.  Warnings may be ignored 
for the whole file (e.g. string/tester.c) or locally for particular code.  
Note that the GCC version number in DIAG_IGNORE_NEEDS_COMMENT is for the 
most recent GCC version known to produce the warning (it's an indicator of 
which DIAG_* calls might be appropriate to review for whether they are 
still needed).  While if the warning in question isn't supported in GCC 6, 
appropriate __GNUC_PREREQ version conditionals are needed around the 
DIAG_IGNORE_NEEDS_COMMENT call to avoid errors about an unknown option 
when building with older GCC.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

end of thread, other threads:[~2020-05-06 22:27 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-30 22:12 [PATCH] improve out-of-bounds checking with GCC 10 attribute access [BZ #25219] Martin Sebor
2020-04-30 22:37 ` Dmitry V. Levin
2020-05-01  2:42 ` DJ Delorie
2020-05-01 19:54   ` Martin Sebor
2020-05-01 22:02     ` DJ Delorie
2020-05-04 17:34       ` Martin Sebor
2020-05-04 18:40         ` Martin Sebor
2020-05-06 20:44         ` Joseph Myers
2020-05-06 21:08           ` DJ Delorie
2020-05-06 22:09           ` Martin Sebor
2020-05-06 22:27             ` Joseph Myers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).