[PATCHv2] stddef: fix offsetof compliance

[Eric Blake]

* m4/stddef_h.m4 (gl_STDDEF_H): Check for Solaris bug.
* modules/stddef (Makefile.am): Substitute new witness.
* lib/stddef.in.h (offsetof): Provide working replacement.
* doc/posix-headers/stddef.texi (stddef.h): Document it.
* tests/test-stddef.c: Add tests for offsetof.

Signed-off-by: Eric Blake <address@hidden>
---

> For that matter, we could compute the correct offsetof expression at
> configure time, for any given compiler, by pre-processing a dummy file
> and running that output through the Makefile.am sed snippet that we
> stick into our replacement stddef.h.  I guess I'll play with that approach.

Like this.  Although I don't know if it is still acceptable for C++.

In response to Paul:
> Also, unless this offsetof bug occurs in real code, it might
> be better to not worry about it.  The bug is a noisy compile-time
> bug, so it's not like we have to explicitly test for
> it and make some noise on our own.

But we already worry about under-parenthesized NULL in our replacement
stddef.h, even if that is also unlikely to be in real code, so it
isn't that much more work to make our stddef.h fix offsetof.  Also, we
may discover some compiler where offsetof returns a signed or wrong
size int, beyond the two compilers I just tested (gcc and Solaris
/usr/bin/cc); whereas missing parens is a noisy error, a wrong type
could lead to silent miscompilation if left unfixed.  I can't check in
test-stddef.c to test for such platforms without also making it
compile on Solaris; so even if the fix for now is for a bug unlikely
to hit real code, this patch lays the groundwork for fixing other
buggy platforms.

But I'll hold off for another day or two before pushing this, in case
there are any more comments or suggested improvements.

 ChangeLog                     |    9 +++++++++
 doc/posix-headers/stddef.texi |    5 +++++
 lib/stddef.in.h               |    6 ++++++
 m4/stddef_h.m4                |   39 ++++++++++++++++++++++++++++++++++++++-
 modules/stddef                |    2 ++
 tests/test-stddef.c           |   10 ++++++++++
 6 files changed, 70 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 86de94d..068b630 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2010-08-16  Eric Blake  <address@hidden>
+
+       stddef: fix offsetof compliance
+       * m4/stddef_h.m4 (gl_STDDEF_H): Check for Solaris bug.
+       * modules/stddef (Makefile.am): Substitute new witness.
+       * lib/stddef.in.h (offsetof): Provide working replacement.
+       * doc/posix-headers/stddef.texi (stddef.h): Document it.
+       * tests/test-stddef.c: Add tests for offsetof.
+
 2010-08-15  Bruno Haible  <address@hidden>

        stpncpy: Allow stpncpy to be defined as a macro.
diff --git a/doc/posix-headers/stddef.texi b/doc/posix-headers/stddef.texi
index 84db56c..f51aef6 100644
--- a/doc/posix-headers/stddef.texi
+++ b/doc/posix-headers/stddef.texi
@@ -14,6 +14,11 @@ stddef.h
 Some platforms provide a @code{NULL} macro that cannot be used in arbitrary
 expressions:
 NetBSD 5.0
+
address@hidden
+Some platforms provide an @code{offsetof} macro that cannot be used in
+arbitrary expressions:
+Solaris 10
 @end itemize

 Portability problems not fixed by Gnulib:
diff --git a/lib/stddef.in.h b/lib/stddef.in.h
index 4bfd866..741ace4 100644
--- a/lib/stddef.in.h
+++ b/lib/stddef.in.h
@@ -76,6 +76,12 @@
 # endif
 #endif

+/* On Solaris 10, the definition of offsetof lacks proper parentheses.  */
+#if @REPLACE_OFFSETOF@
+# undef offsetof
+# define offsetof(__type, __member) (@OFFSETOF_IMPLEMENTATION@)
+#endif
+
 /* Some platforms lack wchar_t.  */
 #if address@hidden@
 # define wchar_t int
diff --git a/m4/stddef_h.m4 b/m4/stddef_h.m4
index c3ae569..3399f17 100644
--- a/m4/stddef_h.m4
+++ b/m4/stddef_h.m4
@@ -1,5 +1,5 @@
 dnl A placeholder for POSIX 2008 <stddef.h>, for platforms that have issues.
-# stddef_h.m4 serial 2
+# stddef_h.m4 serial 3
 dnl Copyright (C) 2009, 2010 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -24,6 +24,41 @@ AC_DEFUN([gl_STDDEF_H],
     REPLACE_NULL=1
     STDDEF_H=stddef.h
   fi
+  AC_CACHE_CHECK([whether offsetof can be used in arbitrary expressions],
+    [gl_cv_decl_offsetof_works],
+    [AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <stddef.h>
+      struct a { char b; };
+      int test[2 * (sizeof offsetof (struct a, b) == sizeof (size_t)) - 1];
+]])],
+      [gl_cv_decl_offsetof_works=yes],
+      [gl_cv_decl_offsetof_works=no])])
+  if test $gl_cv_decl_offsetof_works = no; then
+    REPLACE_OFFSETOF=1
+    STDDEF_H=stddef.h
+    # We can't write a portable offsetof replacement.  But no fear - since
+    # offsetof is a macro, and the given compiler knows the magic non-portable
+    # incantation for implementing it, we can just borrow that version.
+    dnl FIXME: Once we can assume autoconf 2.66, we can rely on conftest.i
+    dnl instead of duplicating AC_PREPROC_IFELSE guts.
+    gl_offsetof_sed1='s/^BEGIN (\(.*\) END$/\1/p'
+    gl_offsetof_sed2='s/&/\\&/g'
+    AC_CACHE_CHECK([how offsetof is implemented],
+      [gl_cv_decl_offsetof_impl],
+      [cat <<EOF > conftest.$ac_ext
+#include <stddef.h>
+BEGIN offsetof (__type, __member) END
+EOF
+       if AC_TRY_EVAL([ac_cpp conftest.$ac_ext]) > conftest.i; then
+         gl_cv_decl_offsetof_impl=`sed -n "$gl_offsetof_sed1" conftest.i`
+       else
+         # Best guess; not standards-compliant, but likely to work
+         gl_cv_decl_offsetof_impl="((size_t) (&(((__type *) 0)->__member)))"
+       fi
+       rm -f conftest.$ac_ext conftest.i
+    ])
+    OFFSETOF_IMPLEMENTATION=`echo "$gl_cv_decl_offsetof_impl" \
+      | sed "$gl_offsetof_sed2"`
+  fi
   if test -n "$STDDEF_H"; then
     gl_CHECK_NEXT_HEADERS([stddef.h])
   fi
@@ -40,6 +75,8 @@ AC_DEFUN([gl_STDDEF_H_DEFAULTS],
 [
   dnl Assume proper GNU behavior unless another module says otherwise.
   REPLACE_NULL=0;                AC_SUBST([REPLACE_NULL])
+  REPLACE_OFFSETOF=0;            AC_SUBST([REPLACE_OFFSETOF])
+  OFFSETOF_IMPLEMENTATION=;      AC_SUBST([OFFSETOF_IMPLEMENTATION])
   HAVE_WCHAR_T=1;                AC_SUBST([HAVE_WCHAR_T])
   STDDEF_H='';                   AC_SUBST([STDDEF_H])
 ])
diff --git a/modules/stddef b/modules/stddef
index a6175c2..b3a58a6 100644
--- a/modules/stddef
+++ b/modules/stddef
@@ -25,6 +25,8 @@ stddef.h: stddef.in.h
              -e 's|@''NEXT_STDDEF_H''@|$(NEXT_STDDEF_H)|g' \
              -e 's|@''HAVE_WCHAR_T''@|$(HAVE_WCHAR_T)|g' \
              -e 's|@''REPLACE_NULL''@|$(REPLACE_NULL)|g' \
+             -e 's|@''REPLACE_OFFSETOF''@|$(REPLACE_OFFSETOF)|g' \
+             -e 's|@''OFFSETOF_IMPLEMENTATION''@|$(OFFSETOF_IMPLEMENTATION)|g' 
\
              < $(srcdir)/stddef.in.h; \
        } > address@hidden && \
        mv address@hidden $@
diff --git a/tests/test-stddef.c b/tests/test-stddef.c
index d047e57..b0e9ad9 100644
--- a/tests/test-stddef.c
+++ b/tests/test-stddef.c
@@ -31,6 +31,16 @@ size_t c = 2;
    per POSIX 2008.  */
 verify (sizeof NULL == sizeof (void *));

+/* Check that offsetof produces integer constants with correct type.  */
+struct d
+{
+  char e;
+  char f;
+};
+verify (sizeof offsetof (struct d, e) == sizeof (size_t));
+verify (offsetof (struct d, e) < -1); /* Must be unsigned.  */
+verify (offsetof (struct d, f) == 1);
+
 int
 main (void)
 {
-- 
1.7.2.1