Re: [PATCH] add attribute nonstring

[Martin Sebor <msebor at gmail dot com>]

On 11/15/2017 03:09 PM, Steve Ellcey wrote:
> On Fri, 2017-11-10 at 16:11 -0700, Martin Sebor wrote:
> > Okay, thanks.  I've read that you run periodic Glibc builds with
> > the latest GCC so I wanted to give you a heads up on this change
> > and the patch to avoid breaking them.  Unless that's not a concern
> > (do you use -Werror in those builds?) what is the best way to
> > coordinate this update?
> >
> > I CC Paul for his input in case updating tzcode first is important.
> >
> > Martin
>
> Martin, were you going to update this patch with Joseph's comments and
> resubmit?  The tz changes are checked in and the ifreq struct is taken
> care of so all that remains are the changes to cdefs.h to define
> __NONSTRING and then to use in utmp.h.  I don't have any comments on
> that change beyond what Joseph made but I did do a build and run on
> aarch64 with the latest GCC to verify that those changes work for me.

Sure.  I've got the attached patch.  According to the Linux utmp
man page:
       String fields are terminated by a null byte ('\0')
       if they are shorter than the size of the field.

The patch lets Glibc compile fine with the top of GCC 8 trunk.

As a heads up, the new attribute triggers warnings with my patch
that enhances GCC to check for string functions being passed
arrays declared with the attribute.  Some are false positives
that I'll try to handle in GCC but at least one of these looks
like a Glibc bug.  Assuming the GCC patch gets approved there
may be some more fallout for Glibc to deal with.

../sysdeps/unix/getlogin_r.c:83:23: warning: ‘strlen’ argument 1 
declared attribute ‘nonstring’ [-Wstringop-overflow=]
       size_t needed = strlen (ut->ut_user) + 1;
                       ^~~~~~~~~~~~~~~~~~~~
In file included from ../login/utmp.h:29,
                 from ../include/utmp.h:2,
                 from ../sysdeps/unix/getlogin_r.c:26,
                 from ../sysdeps/unix/sysv/linux/getlogin_r.c:25:
../sysdeps/gnu/bits/utmp.h:65:8: note: argument ‘ut_user’ declared here
   char ut_user[UT_NAMESIZE]
        ^~~~~~~

Martin

The -Wstringop-truncation option new in GCC 8 detects common misuses
of the strncat and strncpy function that may result in truncating
the copied string before the terminating NUL.  To avoid false positive
warnings for correct code that intentionally creates sequences of
characters that aren't guaranteed to be NUL-terminated, arrays that
are intended to store such sequences should be decorated with a new
nonstring attribute.  This change add this attribute to Glibc and
uses it to suppress such false positives.

ChangeLog:
	* misc/sys/cdefs.h (__attribute_nonstring__): New macro.
	* sysdeps/gnu/bits/utmp.h (struct utmp): Use it.

diff --git a/misc/sys/cdefs.h b/misc/sys/cdefs.h
index cfd39d5..a603cb9 100644
--- a/misc/sys/cdefs.h
+++ b/misc/sys/cdefs.h
@@ -407,6 +407,15 @@
 # endif
 #endif
 
+#if __GNUC_PREREQ (8, 0)
+/* Describes a char array whose address can safely be passed as the first
+   argument to strncpy and strncat, as the char array is not necessarily
+   a NUL-terminated string.  */
+# define __attribute_nonstring__ __attribute__ ((__nonstring__))
+#else
+# define __attribute_nonstring__
+#endif
+
 #if (!defined _Static_assert && !defined __cplusplus \
      && (defined __STDC_VERSION__ ? __STDC_VERSION__ : 0) < 201112 \
      && (!__GNUC_PREREQ (4, 6) || defined __STRICT_ANSI__))
diff --git a/sysdeps/gnu/bits/utmp.h b/sysdeps/gnu/bits/utmp.h
index 2ee11cb..71c9fa2 100644
--- a/sysdeps/gnu/bits/utmp.h
+++ b/sysdeps/gnu/bits/utmp.h
@@ -59,10 +59,13 @@ struct utmp
 {
   short int ut_type;           /* Type of login.  */
   pid_t ut_pid;                        /* Process ID of login process.  */
-  char ut_line[UT_LINESIZE];   /* Devicename.  */
+  char ut_line[UT_LINESIZE]
+    __attribute_nonstring__;   /* Devicename.  */
   char ut_id[4];               /* Inittab ID.  */
-  char ut_user[UT_NAMESIZE];   /* Username.  */
-  char ut_host[UT_HOSTSIZE];   /* Hostname for remote login.  */
+  char ut_user[UT_NAMESIZE]
+    __attribute_nonstring__;   /* Username.  */
+  char ut_host[UT_HOSTSIZE]
+    __attribute_nonstring__;   /* Hostname for remote login.  */
   struct exit_status ut_exit;  /* Exit status of a process marked
                                   as DEAD_PROCESS.  */
 /* The ut_session and ut_tv fields must be the same size when compiled