]> Savannah Git Hosting - gnulib.git/commitdiff
flexmember: port better to GCC + valgrind
authorPaul Eggert <eggert@cs.ucla.edu>
Wed, 7 Sep 2016 09:01:42 +0000 (02:01 -0700)
committerPaul Eggert <eggert@cs.ucla.edu>
Wed, 7 Sep 2016 09:02:40 +0000 (02:02 -0700)
With a char[] flexible array member in a struct with nontrivial
alignment, GCC-generated code can access past the end of the
array, because GCC assumes there are padding bytes to get the
struct aligned.  So the common idiom of malloc (offsetof (struct
s, m), n) does not properly allocate an n-byte trailing member, as
malloc’s argument should be the next multiple of alignof (struct s).
See GCC Bug#66661: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66661
Although C11 apparently permits this GCC optimization (i.e., there
was a bug in Gnulib not in GCC), possibly this is a defect in C11.
See the thread containing:
https://gcc.gnu.org/ml/gcc-patches/2016-09/msg00317.html
* lib/flexmember.h: New file.
* lib/fnmatch.c, lib/fts.c, lib/glob.c, lib/idcache.c:
* lib/localename.c, lib/time_rz.c:
Include flexmember.h.
* lib/fnmatch_loop.c (struct patternlist):
* lib/localename.c (struct hash_node):
Use FLEXIBLE_ARRAY_MEMBER.
* lib/fnmatch_loop.c (EXT):
* lib/fts.c (fts_alloc):
* lib/glob.c (glob_in_dir):
* lib/idcache.c (getuser, getuidbyname, getgroup, getgidbyname):
* lib/localename.c (gl_lock_define_initialized):
* lib/time_rz.c (tzalloc):
Use FLEXSIZEOF instead of offsetof.
* m4/flexmember.m4 (AC_C_FLEXIBLE_ARRAY_MEMBER):
Check that the size of the struct can be taken.
* modules/flexmember (Files): Add lib/flexmember.h.
* modules/fnmatch, modules/glob, modules/localename (Depends-on):
Add flexmember.

14 files changed:
ChangeLog
lib/flexmember.h [new file with mode: 0644]
lib/fnmatch.c
lib/fnmatch_loop.c
lib/fts.c
lib/glob.c
lib/idcache.c
lib/localename.c
lib/time_rz.c
m4/flexmember.m4
modules/flexmember
modules/fnmatch
modules/glob
modules/localename

index 77938ef93893c0936098c822d21d758fe521cd26..8391aab9bd7a22ae5872bef7fc34d071d5d859a3 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,37 @@
+2016-09-07  Paul Eggert  <eggert@cs.ucla.edu>
+
+       flexmember: port better to GCC + valgrind
+       With a char[] flexible array member in a struct with nontrivial
+       alignment, GCC-generated code can access past the end of the
+       array, because GCC assumes there are padding bytes to get the
+       struct aligned.  So the common idiom of malloc (offsetof (struct
+       s, m), n) does not properly allocate an n-byte trailing member, as
+       malloc’s argument should be the next multiple of alignof (struct s).
+       See GCC Bug#66661: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66661
+       Although C11 apparently permits this GCC optimization (i.e., there
+       was a bug in Gnulib not in GCC), possibly this is a defect in C11.
+       See the thread containing:
+       https://gcc.gnu.org/ml/gcc-patches/2016-09/msg00317.html
+       * lib/flexmember.h: New file.
+       * lib/fnmatch.c, lib/fts.c, lib/glob.c, lib/idcache.c:
+       * lib/localename.c, lib/time_rz.c:
+       Include flexmember.h.
+       * lib/fnmatch_loop.c (struct patternlist):
+       * lib/localename.c (struct hash_node):
+       Use FLEXIBLE_ARRAY_MEMBER.
+       * lib/fnmatch_loop.c (EXT):
+       * lib/fts.c (fts_alloc):
+       * lib/glob.c (glob_in_dir):
+       * lib/idcache.c (getuser, getuidbyname, getgroup, getgidbyname):
+       * lib/localename.c (gl_lock_define_initialized):
+       * lib/time_rz.c (tzalloc):
+       Use FLEXSIZEOF instead of offsetof.
+       * m4/flexmember.m4 (AC_C_FLEXIBLE_ARRAY_MEMBER):
+       Check that the size of the struct can be taken.
+       * modules/flexmember (Files): Add lib/flexmember.h.
+       * modules/fnmatch, modules/glob, modules/localename (Depends-on):
+       Add flexmember.
+
 2016-09-06  Paul Eggert  <eggert@cs.ucla.edu>
 
        getprogname: port to Solaris 10
diff --git a/lib/flexmember.h b/lib/flexmember.h
new file mode 100644 (file)
index 0000000..00b084c
--- /dev/null
@@ -0,0 +1,39 @@
+/* Sizes of structs with flexible array members.
+
+   Copyright 2016 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+   Written by Paul Eggert.  */
+
+/* Nonzero multiple of alignment of TYPE, suitable for FLEXSIZEOF below.
+   On older platforms without _Alignof, use a pessimistic bound that is
+   safe in practice even if FLEXIBLE_ARRAY_MEMBER is 1.
+   On newer platforms, use _Alignof to get a tighter bound.  */
+
+#if !defined __STDC_VERSION__ || __STDC_VERSION__ < 201112
+# define _GL_XALLOC_ALIGNOF(type) (sizeof (type) & ~ (sizeof (type) - 1))
+#else
+# define _GL_XALLOC_ALIGNOF(type) _Alignof (type)
+#endif
+
+/* Upper bound on the size of a struct of type TYPE with a flexible
+   array member named MEMBER that is followed by N bytes of other data.
+   This is not simply sizeof (TYPE) + N, since it may require
+   alignment and FLEXIBLE_ARRAY_MEMBER may be 1.  Yield a value less
+   than N if and only if arithmetic overflow occurs.  */
+
+#define FLEXSIZEOF(type, member, n) \
+   ((offsetof (type, member) + _GL_XALLOC_ALIGNOF (type) - 1 + (n)) \
+    & ~ (_GL_XALLOC_ALIGNOF (type) - 1))
index 9b3f7161033335ffdebfea90404dab5df1847b89..bd9e587bac594bf325d8390c6617df061f4e68ff 100644 (file)
@@ -67,6 +67,8 @@ extern int fnmatch (const char *pattern, const char *string, int flags);
 # define SIZE_MAX ((size_t) -1)
 #endif
 
+#include "flexmember.h"
+
 /* We often have to test for FNM_FILE_NAME and FNM_PERIOD being both set.  */
 #define NO_LEADING_PERIOD(flags) \
   ((flags & (FNM_FILE_NAME | FNM_PERIOD)) == (FNM_FILE_NAME | FNM_PERIOD))
index a0c00d345691499dd360d393ccb458bd5afa04d9..f41c06f89a1eb14b793b8fd33f32576bce89d8a4 100644 (file)
@@ -1031,7 +1031,7 @@ EXT (INT opt, const CHAR *pattern, const CHAR *string, const CHAR *string_end,
   struct patternlist
   {
     struct patternlist *next;
-    CHAR str[1];
+    CHAR str[FLEXIBLE_ARRAY_MEMBER];
   } *list = NULL;
   struct patternlist **lastp = &list;
   size_t pattern_len = STRLEN (pattern);
@@ -1083,7 +1083,7 @@ EXT (INT opt, const CHAR *pattern, const CHAR *string, const CHAR *string_end,
                     ? pattern_len                                             \
                     : p - startp + 1UL);                                      \
             plensize = plen * sizeof (CHAR);                                  \
-            newpsize = offsetof (struct patternlist, str) + plensize;         \
+            newpsize = FLEXSIZEOF (struct patternlist, str, plensize);        \
             if ((size_t) -1 / sizeof (CHAR) < plen                            \
                 || newpsize < offsetof (struct patternlist, str)              \
                 || ALLOCA_LIMIT <= newpsize)                                  \
index 331d23bf88c67a31ae5b47e9045a3d3fd188776c..52461b18c5d3aa9998627f6b9138b2ce58a6db0e 100644 (file)
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -76,6 +76,7 @@ static char sccsid[] = "@(#)fts.c       8.6 (Berkeley) 8/14/94";
 /* FIXME - use fcntl(F_DUPFD_CLOEXEC)/openat(O_CLOEXEC) once they are
    supported.  */
 # include "cloexec.h"
+# include "flexmember.h"
 # include "openat.h"
 # include "same-inode.h"
 #endif
@@ -1925,17 +1926,7 @@ fts_alloc (FTS *sp, const char *name, register size_t namelen)
          * The file name is a variable length array.  Allocate the FTSENT
          * structure and the file name in one chunk.
          */
-        len = offsetof(FTSENT, fts_name) + namelen + 1;
-        /* Align the allocation size so that it works for FTSENT,
-           so that trailing padding may be referenced by direct access
-           to the flexible array members, without triggering undefined behavior
-           by accessing bytes beyond the heap allocation.  This implicit access
-           was seen for example with ISDOT() and GCC 5.1.1 at -O2.
-           Do not use alignof (FTSENT) here, since C11 prohibits
-           taking the alignment of a structure containing a flexible
-           array member.  */
-        len += alignof (max_align_t) - 1;
-        len &= ~ (alignof (max_align_t) - 1);
+        len = FLEXSIZEOF(FTSENT, fts_name, namelen + 1);
         if ((p = malloc(len)) == NULL)
                 return (NULL);
 
index a2b7c86c3a498431f29037c29dd01764c7951db4..5b2ff9d623eb31dc5f7f34a13cc85f0b77971254 100644 (file)
@@ -90,6 +90,8 @@
 
 #include <fnmatch.h>
 
+#include "flexmember.h"
+
 #ifdef _SC_GETPW_R_SIZE_MAX
 # define GETPW_R_SIZE_MAX()     sysconf (_SC_GETPW_R_SIZE_MAX)
 #else
@@ -1677,7 +1679,8 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
                           struct globnames *newnames;
                           size_t count = names->count * 2;
                           size_t nameoff = offsetof (struct globnames, name);
-                          size_t size = nameoff + count * sizeof (char *);
+                          size_t size = FLEXSIZEOF (struct globnames, name,
+                                                    count * sizeof (char *));
                           if ((SIZE_MAX - nameoff) / 2 / sizeof (char *)
                               < names->count)
                             goto memory_error;
index 2db5109b4c5e6b4fa9307c19d08bed140ef0acff..3dada5d21ace5b1d1d044b250fd2125d2c27c14d 100644 (file)
@@ -27,6 +27,7 @@
 
 #include <unistd.h>
 
+#include "flexmember.h"
 #include "xalloc.h"
 
 #ifdef __DJGPP__
@@ -83,7 +84,7 @@ getuser (uid_t uid)
     {
       struct passwd *pwent = getpwuid (uid);
       char const *name = pwent ? pwent->pw_name : "";
-      match = xmalloc (offsetof (struct userid, name) + strlen (name) + 1);
+      match = xmalloc (FLEXSIZEOF (struct userid, name, strlen (name) + 1));
       match->id.u = uid;
       strcpy (match->name, name);
 
@@ -127,7 +128,7 @@ getuidbyname (const char *user)
     }
 #endif
 
-  tail = xmalloc (offsetof (struct userid, name) + strlen (user) + 1);
+  tail = xmalloc (FLEXSIZEOF (struct userid, name, strlen (user) + 1));
   strcpy (tail->name, user);
 
   /* Add to the head of the list, so most recently used is first.  */
@@ -165,7 +166,7 @@ getgroup (gid_t gid)
     {
       struct group *grent = getgrgid (gid);
       char const *name = grent ? grent->gr_name : "";
-      match = xmalloc (offsetof (struct userid, name) + strlen (name) + 1);
+      match = xmalloc (FLEXSIZEOF (struct userid, name, strlen (name) + 1));
       match->id.g = gid;
       strcpy (match->name, name);
 
@@ -209,7 +210,7 @@ getgidbyname (const char *group)
     }
 #endif
 
-  tail = xmalloc (offsetof (struct userid, name) + strlen (group) + 1);
+  tail = xmalloc (FLEXSIZEOF (struct userid, name, strlen (group) + 1));
   strcpy (tail->name, group);
 
   /* Add to the head of the list, so most recently used is first.  */
index 81ab21304359429637160b8de437efa51b5d281d..f86bd874d01f961d130cb766768522715a6418c4 100644 (file)
@@ -33,6 +33,8 @@
 #include <locale.h>
 #include <string.h>
 
+#include "flexmember.h"
+
 #if HAVE_USELOCALE
 /* Mac OS X 10.5 defines the locale_t type in <xlocale.h>.  */
 # if defined __APPLE__ && defined __MACH__
@@ -2619,7 +2621,7 @@ string_hash (const void *x)
 struct hash_node
   {
     struct hash_node * volatile next;
-    char contents[100]; /* has variable size */
+    char contents[FLEXIBLE_ARRAY_MEMBER];
   };
 
 # define HASH_TABLE_SIZE 257
@@ -2646,7 +2648,7 @@ struniq (const char *string)
   size = strlen (string) + 1;
   new_node =
     (struct hash_node *)
-    malloc (offsetof (struct hash_node, contents[0]) + size);
+    malloc (FLEXSIZEOF (struct hash_node, contents, size));
   if (new_node == NULL)
     /* Out of memory.  Return a statically allocated string.  */
     return "C";
index 10a006ca2ec76903737c3857cfc9941084533868..8bfe988479f3ca7a99cfd4b48b20fb5783a0de00 100644 (file)
@@ -32,6 +32,7 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include "flexmember.h"
 #include "time-internal.h"
 
 #if !HAVE_TZSET
@@ -94,7 +95,7 @@ tzalloc (char const *name)
 {
   size_t name_size = name ? strlen (name) + 1 : 0;
   size_t abbr_size = name_size < ABBR_SIZE_MIN ? ABBR_SIZE_MIN : name_size + 1;
-  timezone_t tz = malloc (offsetof (struct tm_zone, abbrs) + abbr_size);
+  timezone_t tz = malloc (FLEXSIZEOF (struct tm_zone, abbrs, abbr_size));
   if (tz)
     {
       tz->next = NULL;
index baa9ff8fb2c7350b9aacf616113cba11ee02c0a4..155ae9b812023fbc74038e2792c10b395c191a84 100644 (file)
@@ -1,4 +1,4 @@
-# serial 3
+# serial 4
 # Check for flexible array member support.
 
 # Copyright (C) 2006, 2009-2016 Free Software Foundation, Inc.
@@ -19,8 +19,10 @@ AC_DEFUN([AC_C_FLEXIBLE_ARRAY_MEMBER],
             #include <stddef.h>
             struct s { int n; double d[]; };]],
           [[int m = getchar ();
-            struct s *p = malloc (offsetof (struct s, d)
-                                  + m * sizeof (double));
+            size_t nbytes = offsetof (struct s, d) + m * sizeof (double);
+            nbytes += sizeof (struct s) - 1;
+            nbytes -= nbytes % sizeof (struct s);
+            struct s *p = malloc (nbytes);
             p->d[0] = 0.0;
             return p->d != (double *) NULL;]])],
        [ac_cv_c_flexmember=yes],
index 5556f041d8a0be664ad82bc30f2067a0bed68ddc..510767619dd89fc112cca38401a94595cb281ce0 100644 (file)
@@ -2,6 +2,7 @@ Description:
 Flexible array member support
 
 Files:
+lib/flexmember.h
 m4/flexmember.m4
 
 Depends-on:
index a5a49f9d92b2c12c32a3904500003fd1387093ed..ab302361ba2130f4a4c788f45392fd2bc095e349 100644 (file)
@@ -12,6 +12,7 @@ Depends-on:
 extensions
 snippet/arg-nonnull
 alloca          [test -n "$FNMATCH_H"]
+flexmember      [test -n "$FNMATCH_H"]
 stdbool         [test -n "$FNMATCH_H"]
 wchar           [test -n "$FNMATCH_H"]
 wctype-h        [test -n "$FNMATCH_H"]
index 919d7fc0f530a738e219fda6ff7325803de5aeb5..f32fe5f36fef75d3d721f41bb2dbd9e50e42e774 100644 (file)
@@ -18,6 +18,7 @@ alloca          [test -n "$GLOB_H"]
 closedir        [test -n "$GLOB_H"]
 d-type          [test -n "$GLOB_H"]
 dirfd           [test -n "$GLOB_H"]
+flexmember      [test -n "$GLOB_H"]
 fnmatch         [test -n "$GLOB_H"]
 getlogin_r      [test -n "$GLOB_H"]
 memchr          [test -n "$GLOB_H"]
index 603796e06723f9cda293390a06b7c5941932acd2..193c7bd754128523d094d7805a07c5024d1f9ef1 100644 (file)
@@ -9,6 +9,7 @@ m4/intlmacosx.m4
 m4/lcmessage.m4
 
 Depends-on:
+flexmember
 strdup
 lock
 langinfo