Subject: Re: bin/36678: crossbuild of -current and netbsd-4 fails on OS/X
To: None <gnats-bugs@NetBSD.org>
From: Giles Lean <giles.lean@pobox.com>
List: netbsd-bugs
Date: 07/29/2007 21:33:38
A while back I filed this PR:

    bin/36678
    crossbuild of -current and netbsd-4 fails on OS/X

Here's a fix which works for both -current and the netbsd-4 branch.

The explanation is longer than the fix but the change required touches
tools/compat/configure.ac and nothing to do with autoconf and friends
is ever simple.

Background
==========

I think the change that introduced this problem was this commit
to common/lib/libc/hash/rmd160/rmd160.c:

  revision 1.3
  date: 2007/07/18 13:57:54;  author: joerg;  state: Exp;  lines: +4 -4
  Fix SIGBUS issues on strict alignment issues. Use le32dec in RMD160
  as the data pointer to RMD160_Update doesn't have to be aligned.
  In SHA256_Update and SHA512_Update, only operate directly on the passed
  in data if no left-over in the context exists and the data is correctly
  aligned. The problem was exposed by the audit-packages rewrite in C
  and reported for the libnbcompat version in PR pkg/36662.

The impact of the change is that we now need le32dec() in the host
build environment for xinstall to link.

Fix
===

The suggestion I made when I filed PR bin/36678 was not fully correct:
just cutting and pasting le32dec() from sys/sys/endian.h into
tools/compat/compat_defs.h breaks NetBSD hosted builds.

A full fix is to include le32dec() into compat_defs.h but to provide it
with protection via tests of autoconf generated macros.

Thus, the following fix is in two parts:

a) an update for tools/compat/configure.ac
b) an update for tools/compat/compat_defs.h

There are some guidelines in src/compat/tools/Makefile about running
'make regen' when configure.ac changes: below I describe a process
that worked for me.  It took a while to figure out; perhaps the
comments in the Makefile could be expanded a bit?

(Experts on the build process will find the following a bit wordy,
but it might help someone else sometime ... even me. :-)

1. preparation: select an architecture and build the host tools
   and autoconf:

      $ cd src
      $ ./build.sh -U -u -m i386 -O ... tools

   Add the TOOLDIR directory this command populates to $PATH and
   then build (nb)autoconf:

      $ cd tools/autoconf
      $ nbmake-i386
      $ nbmake-i386 install
      $ cd ../..             # back to top level src/ directory

2. apply the patch from the end of this message to to configure.ac
   and config_defs.h:

      $ patch -p1 < ...

3. build the new version of configure and nbtool_config.h.in:

      $ cd tools/compat
      $ nbmake-i386 regen
      $ nbautoheader
      $ cd ../..

4. test that the build still works, at least past linking xinstall

   (There is more information on the testing I did below, and I
   removed the objdir and tooldir populated in #1 before running this
   command, just to be safe.)

      $ ./build.sh -U -u -m i386 -O ... tools

If all is well that should mean that four files in tools/compat
can be committed ...

     configure.ac, configure, nbtool_config.h.in and config_defs.h

... and PR bin/36678 closed. :-)

n.b. The version of autoheader seems to have changed since
nbtool_config.h was last committed, so nbtool_config.h may show text
diffs where there is no functional change.  Not my doing:

  -/* Define to 1 if using `alloca.c'. */
  +/* Define if using `alloca.c'. */

Notes on the changes
====================

In the patch I have included not just le32dec() which is the function
xinstall tripped over but all the related functions as well.  This
wasn't the most minimal change, but it preserves commonality with
sys/sys/endian.h which I preferred.

In including the material from sys/sys/endian.h into compat_defs.h I
also added __GNUC_PREREQ__ from sys/sys/cdefs.h, as the new code
uses __GNUC_PREREQ__.

The patch is applicable to the netbsd-4 branch as well as to -current,
but there are nother outstanding changes to compat_defs.h in -current
that might be pulled up to the netbsd-4 branch first to ease merging.

  revision 1.59
  date: 2007/07/03 12:11:09;  author: nakayama;  state: Exp;  lines: +6 -1
  Add strndup(3) to libnbcompat, since estrndup(3) added into efun.c calls it.

  revision 1.58
  date: 2007/01/09 17:34:27;  author: ginsbach;  state: Exp;  lines: +2 -2
  Fix tools build, binstall/xinstall, for systems without id_t, e.g.
  MacOS X 10.3.9.  This should better match the NetBSD definition of id_t
  in sys/sys/types.h.

(I haven't figured out what other files those commits touched.)

Testing
=======

I have tested with these changes using "./build.sh ... tools" with
the following combinations:

a) netbsd-current/i386     building on NetBSD 4.0_BETA2/i386
b) netbsd-4/i386           (ditto)

For the following I used a copy of src/tools/compat/configure and
nbtool_config.h.in generated on NetBSD for each of -current and
netbsd-4 rather than run autoconf or autoheader:

c) netbsd-current/i386     building on OS/X 10.4.10 PPC
d) netbsd-4/i386           (ditto)                            # building

That's not an exhaustive set of tests -- or even full builds yet --
but it's a start.  "It works for me." :-)

(I did try a build using HOST_CC="gcc -m64" and HOST_CXX="g++ -m64" on
OS X to check that building 64 bit host tools is OK, but that doesn't
fly with or without my changes.  I'll look into it another time.)

Regards,

Giles

--- src/tools/compat/configure.ac.orig	2007-07-28 22:39:03.000000000 +1000
+++ src/tools/compat/configure.ac	2007-07-28 22:38:53.000000000 +1000
@@ -142,6 +142,8 @@
 
 AC_CHECK_DECLS([bswap16, bswap32, bswap64],,, [#include <machine/bswap.h>])
 
+AC_CHECK_DECLS([be16enc, le16enc, be16dec, le16dec, be32enc, le32enc, be32dec, le32dec, be64enc, le64enc, be64dec, le64dec],,, [#include <sys/endian.h>])
+
 AC_CHECK_DECLS([fstatvfs],,, [#include <sys/statvfs.h>])
 
 AC_CHECK_DECLS([setgroupent, setpassent],,, [
--- src/tools/compat/compat_defs.h.orig	2007-07-28 22:39:03.000000000 +1000
+++ src/tools/compat/compat_defs.h	2007-07-28 22:38:53.000000000 +1000
@@ -89,6 +89,25 @@
 
 /* Some things usually in BSD <sys/cdefs.h>. */
 
+/*
+ * Macro to test if we're using a GNU C compiler of a specific vintage
+ * or later, for e.g. features that appeared in a particular version
+ * of GNU C.  Usage:
+ *
+ *	#if __GNUC_PREREQ__(major, minor)
+ *	...cool feature...
+ *	#else
+ *	...delete feature...
+ *	#endif
+ */
+#ifdef __GNUC__
+#define	__GNUC_PREREQ__(x, y)						\
+	((__GNUC__ == (x) && __GNUC_MINOR__ >= (y)) ||			\
+	 (__GNUC__ > (x)))
+#else
+#define	__GNUC_PREREQ__(x, y)	0
+#endif
+
 #ifndef __CONCAT
 #define	__CONCAT(x,y)	x ## y
 #endif
@@ -647,6 +666,204 @@
 #define le64toh(x)	htole64(x)
 #endif
 
+/*
+ * Routines to encode/decode big- and little-endian multi-octet values
+ * to/from an octet stream.
+ */
+
+#if __GNUC_PREREQ__(2, 95)
+
+#define __GEN_ENDIAN_ENC(bits, endian) \
+static __inline __unused void \
+endian ## bits ## enc(void *dst, uint ## bits ## _t u) \
+{ \
+	u = hto ## endian ## bits (u); \
+	__builtin_memcpy(dst, &u, sizeof(u)); \
+}
+
+#if !HAVE_DECL_BE16ENC
+__GEN_ENDIAN_ENC(16, be)
+#endif
+#if !HAVE_DECL_BE32ENC
+__GEN_ENDIAN_ENC(32, be)
+#endif
+#if !HAVE_DECL_BE64ENC
+__GEN_ENDIAN_ENC(64, be)
+#endif
+#if !HAVE_DECL_LE16ENC
+__GEN_ENDIAN_ENC(16, le)
+#endif
+#if !HAVE_DECL_LE32ENC
+__GEN_ENDIAN_ENC(32, le)
+#endif
+#if !HAVE_DECL_LE64ENC
+__GEN_ENDIAN_ENC(64, le)
+#endif
+#undef __GEN_ENDIAN_ENC
+
+#define __GEN_ENDIAN_DEC(bits, endian) \
+static __inline __unused uint ## bits ## _t \
+endian ## bits ## dec(const void *buf) \
+{ \
+	uint ## bits ## _t u; \
+	__builtin_memcpy(&u, buf, sizeof(u)); \
+	return endian ## bits ## toh (u); \
+}
+
+#if !HAVE_DECL_BE16DEC
+__GEN_ENDIAN_DEC(16, be)
+#endif
+#if !HAVE_DECL_BE32DEC
+__GEN_ENDIAN_DEC(32, be)
+#endif
+#if !HAVE_DECL_BE64DEC
+__GEN_ENDIAN_DEC(64, be)
+#endif
+#if !HAVE_DECL_LE16DEC
+__GEN_ENDIAN_DEC(16, le)
+#endif
+#if !HAVE_DECL_LE32DEC
+__GEN_ENDIAN_DEC(32, le)
+#endif
+#if !HAVE_DECL_LE64DEC
+__GEN_ENDIAN_DEC(64, le)
+#endif
+#undef __GEN_ENDIAN_DEC
+
+#else	/* !(GCC >= 2.95) */
+
+#if !HAVE_DECL_BE16ENC
+static __inline void __unused
+be16enc(void *buf, uint16_t u)
+{
+	uint8_t *p = (uint8_t *)buf;
+
+	p[0] = (uint8_t)(((unsigned)u >> 8) & 0xff);
+	p[1] = (uint8_t)(u & 0xff);
+}
+#endif
+
+#if !HAVE_DECL_LE16ENC
+static __inline void __unused
+le16enc(void *buf, uint16_t u)
+{
+	uint8_t *p = (uint8_t *)buf;
+
+	p[0] = (uint8_t)(u & 0xff);
+	p[1] = (uint8_t)(((unsigned)u >> 8) & 0xff);
+}
+#endif
+
+#if !HAVE_DECL_BE16DEC
+static __inline uint16_t __unused
+be16dec(const void *buf)
+{
+	const uint8_t *p = (const uint8_t *)buf;
+
+	return (uint16_t)((p[0] << 8) | p[1]);
+}
+#endif
+
+#if !HAVE_DECL_LE16DEC
+static __inline uint16_t __unused
+le16dec(const void *buf)
+{
+	const uint8_t *p = (const uint8_t *)buf;
+
+	return (uint16_t)((p[1] << 8) | p[0]);
+}
+#endif
+
+#if !HAVE_DECL_BE32ENC
+static __inline void __unused
+be32enc(void *buf, uint32_t u)
+{
+	uint8_t *p = (uint8_t *)buf;
+
+	p[0] = (uint8_t)((u >> 24) & 0xff);
+	p[1] = (uint8_t)((u >> 16) & 0xff);
+	p[2] = (uint8_t)((u >> 8) & 0xff);
+	p[3] = (uint8_t)(u & 0xff);
+}
+#endif
+
+#if !HAVE_DECL_LE32ENC
+static __inline void __unused
+le32enc(void *buf, uint32_t u)
+{
+	uint8_t *p = (uint8_t *)buf;
+
+	p[0] = (uint8_t)(u & 0xff);
+	p[1] = (uint8_t)((u >> 8) & 0xff);
+	p[2] = (uint8_t)((u >> 16) & 0xff);
+	p[3] = (uint8_t)((u >> 24) & 0xff);
+}
+#endif
+
+#if !HAVE_DECL_BE32DEC
+static __inline uint32_t __unused
+be32dec(const void *buf)
+{
+	const uint8_t *p = (const uint8_t *)buf;
+
+	return ((p[0] << 24) | (p[1] << 16) | (p[2] << 8) | p[3]);
+}
+#endif
+
+#if !HAVE_DECL_LE32DEC
+static __inline uint32_t __unused
+le32dec(const void *buf)
+{
+	const uint8_t *p = (const uint8_t *)buf;
+
+	return ((p[3] << 24) | (p[2] << 16) | (p[1] << 8) | p[0]);
+}
+#endif
+
+#if !HAVE_DECL_BE64ENC
+static __inline void __unused
+be64enc(void *buf, uint64_t u)
+{
+	uint8_t *p = (uint8_t *)buf;
+
+	be32enc(p, (uint32_t)(u >> 32));
+	be32enc(p + 4, (uint32_t)(u & 0xffffffffULL));
+}
+#endif
+
+#if !HAVE_DECL_LE64ENC
+static __inline void __unused
+le64enc(void *buf, uint64_t u)
+{
+	uint8_t *p = (uint8_t *)buf;
+
+	le32enc(p, (uint32_t)(u & 0xffffffffULL));
+	le32enc(p + 4, (uint32_t)(u >> 32));
+}
+#endif
+
+#if !HAVE_DECL_BE64DEC
+static __inline uint64_t __unused
+be64dec(const void *buf)
+{
+	const uint8_t *p = (const uint8_t *)buf;
+
+	return (((uint64_t)be32dec(p) << 32) | be32dec(p + 4));
+}
+#endif
+
+#if !HAVE_DECL_LE64DEC
+static __inline uint64_t __unused
+le64dec(const void *buf)
+{
+	const uint8_t *p = (const uint8_t *)buf;
+
+	return (le32dec(p) | ((uint64_t)le32dec(p + 4) << 32));
+}
+#endif
+
+#endif	/* GCC >= 2.95 */
+
 /* <sys/mman.h> */
 
 #ifndef MAP_FILE