* notmuch sha1 implementation broken on (some) big-endian architectures
@ 2013-11-24 11:57 David Bremner
2013-11-24 12:40 ` David Bremner
2013-11-24 16:42 ` Tomi Ollila
0 siblings, 2 replies; 12+ messages in thread
From: David Bremner @ 2013-11-24 11:57 UTC (permalink / raw)
To: notmuch
The following code, when linked with libnotmuch.a and libutil.a does a
passable imitation of sha1sum on amd64 (and I guess also i386) but
computes a different digest on powerpc and probably sparc and s390x.
In the long run we should maybe outsource hash computations to
e.g. librhash, but I'd like a simpler fix for 0.17, if possible
P.S. I blame Austin for adding the "missing-headers" test which found
this bug ;).
/* 8<----------------------------------------- */
#include <stdio.h>
#include "notmuch.h"
char * notmuch_sha1_of_file(const char* filename);
int
main (int argc, char **argv)
{
char *digest = notmuch_sha1_of_file (argv[1]);
printf("%s %s\n",digest,argv[1]);
return 0;
}
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: notmuch sha1 implementation broken on (some) big-endian architectures 2013-11-24 11:57 notmuch sha1 implementation broken on (some) big-endian architectures David Bremner @ 2013-11-24 12:40 ` David Bremner 2013-11-24 16:42 ` Tomi Ollila 1 sibling, 0 replies; 12+ messages in thread From: David Bremner @ 2013-11-24 12:40 UTC (permalink / raw) To: notmuch David Bremner <david@tethera.net> writes: > The following code, when linked with libnotmuch.a and libutil.a does a > passable imitation of sha1sum on amd64 (and I guess also i386) but > computes a different digest on powerpc and probably sparc and s390x. > > In the long run we should maybe outsource hash computations to > e.g. librhash, but I'd like a simpler fix for 0.17, if possible Out of curiousity, I tried out a similar example with librhash, and it works fine on powerpc. #include <errno.h> #include "rhash.h" /* LibRHash interface */ int main(int argc, char *argv[]) { char digest[64]; char output[130]; rhash_library_init(); /* initialize static data */ int res = rhash_file(RHASH_SHA1, argv[1], digest); if(res < 0) { fprintf(stderr, "LibRHash error: %s: %s\n", argv[1], strerror(errno)); return 1; } /* convert binary digest to hexadecimal string */ rhash_print_bytes(output, digest, rhash_get_digest_size(RHASH_SHA1),RHPR_HEX); printf("%s (%s) = %s\n", rhash_get_name(RHASH_SHA1), argv[1], output); return 0; } ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: notmuch sha1 implementation broken on (some) big-endian architectures 2013-11-24 11:57 notmuch sha1 implementation broken on (some) big-endian architectures David Bremner 2013-11-24 12:40 ` David Bremner @ 2013-11-24 16:42 ` Tomi Ollila 2013-11-24 21:29 ` [PATCH 1/2] lib: fix byte order test in libsha1.c david 1 sibling, 1 reply; 12+ messages in thread From: Tomi Ollila @ 2013-11-24 16:42 UTC (permalink / raw) To: David Bremner, notmuch On Sun, Nov 24 2013, David Bremner <david@tethera.net> wrote: > The following code, when linked with libnotmuch.a and libutil.a does a > passable imitation of sha1sum on amd64 (and I guess also i386) but > computes a different digest on powerpc and probably sparc and s390x. > > In the long run we should maybe outsource hash computations to > e.g. librhash, but I'd like a simpler fix for 0.17, if possible > > P.S. I blame Austin for adding the "missing-headers" test which found > this bug ;). This is interesting problem, I would have guessed that this would fails on LITTLE_ENDIAN machines easier, if ever... ... especially as there is line lib/libsha1.c:52:#if (PLATFORM_BYTE_ORDER == IS_LITTLE_ENDIAN) ... but... I could not find any (other) matches for PLATFORM_BYTE_ORDER nor IS_LITTLE_ENDIAN in source code or in /usr/include/**/*.h or in /usr/lib/gcc/**/*.h I did some testing and it seems that #if (FOOOO == BBBAAARRR) <code> #endif will have <code> in output file in case neither of the above are defined... :/ So, this could work: #if <endian.h> // for BYTE_ORDER && LITTLE_ENDIAN and then #if (BYTE_ORDER == LITTLE_ENDIAN) ... to replace lib/libsha1.c:53 (53 now after endian.h added) Please test on BIG_ENDIAN machine... In case this works, then we'd need to inform users that their long/missing Message ID:s are now coded differently in their databases... Tomi ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] lib: fix byte order test in libsha1.c 2013-11-24 16:42 ` Tomi Ollila @ 2013-11-24 21:29 ` david 2013-11-24 21:29 ` [PATCH 2/2] NEWS: News for big endian sha1 bug fix david 2013-11-25 9:43 ` [PATCH 1/2] lib: fix byte order test in libsha1.c Tomi Ollila 0 siblings, 2 replies; 12+ messages in thread From: david @ 2013-11-24 21:29 UTC (permalink / raw) To: notmuch From: David Bremner <david@tethera.net> Previously PLATFORM_BYTE_ORDER and IS_LITTLE_ENDIAN were not defined, so the little endian code was always compiled in. This will have the effect that the "SHA1s" on big endian architectures will change (i.e. become actual sha1s). So someone re-indexing their database could conceivable lose tags on messages without a message-id header. --- lib/libsha1.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/libsha1.c b/lib/libsha1.c index 5d16f6a..794854b 100644 --- a/lib/libsha1.c +++ b/lib/libsha1.c @@ -49,11 +49,17 @@ extern "C" #define bswap_32(x) ((rotr32((x), 24) & 0x00ff00ff) | (rotr32((x), 8) & 0xff00ff00)) -#if (PLATFORM_BYTE_ORDER == IS_LITTLE_ENDIAN) -#define bsw_32(p,n) \ - { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); } +#ifdef __BYTE_ORDER__ +# if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__) +# define bsw_32(p,n) \ + { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); } +# elif (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__) +# define bsw_32(p,n) +# else +# error "unknown byte order" +# endif #else -#define bsw_32(p,n) +# error "macro __BYTE_ORDER__ is not defined" #endif #define SHA1_MASK (SHA1_BLOCK_SIZE - 1) -- 1.8.4.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/2] NEWS: News for big endian sha1 bug fix. 2013-11-24 21:29 ` [PATCH 1/2] lib: fix byte order test in libsha1.c david @ 2013-11-24 21:29 ` david 2013-11-25 3:10 ` Austin Clements 2013-11-26 13:07 ` [PATCH] util: detect byte order david 2013-11-25 9:43 ` [PATCH 1/2] lib: fix byte order test in libsha1.c Tomi Ollila 1 sibling, 2 replies; 12+ messages in thread From: david @ 2013-11-24 21:29 UTC (permalink / raw) To: notmuch From: David Bremner <david@tethera.net> We could give more details about how to migrate tags, but I'm not sure that it's a practical problem, or just a theoretical one. --- NEWS | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 3383ecf..31c6284 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,18 @@ -Notmuch 0.17~rc1 (2013-11-20) +Notmuch 0.17~rc2 (2013-xx-yy) ============================= +Incompatible change in SHA1 computation +--------------------------------------- + +Previously on big endian architectures like sparc and powerpc the +computation of SHA1 hashes was incorrect. This meant that messages +with overlong or missing message-ids were given different computed +message-ids than on more common little endian architectures like i386 +and amd64. If you use notmuch on a big endian architecture, you are +strongly advised to make a backup of your tags using `notmuch dump` +before this upgrade. It should be possible to migrate the tags using a +script. + Command-Line Interface ---------------------- -- 1.8.4.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] NEWS: News for big endian sha1 bug fix. 2013-11-24 21:29 ` [PATCH 2/2] NEWS: News for big endian sha1 bug fix david @ 2013-11-25 3:10 ` Austin Clements 2013-11-26 13:07 ` [PATCH] util: detect byte order david 1 sibling, 0 replies; 12+ messages in thread From: Austin Clements @ 2013-11-25 3:10 UTC (permalink / raw) To: david; +Cc: notmuch Quoth david@tethera.net on Nov 24 at 5:29 pm: > From: David Bremner <david@tethera.net> > > We could give more details about how to migrate tags, but I'm not sure > that it's a practical problem, or just a theoretical one. > --- > NEWS | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/NEWS b/NEWS > index 3383ecf..31c6284 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,6 +1,18 @@ > -Notmuch 0.17~rc1 (2013-11-20) > +Notmuch 0.17~rc2 (2013-xx-yy) > ============================= > > +Incompatible change in SHA1 computation > +--------------------------------------- > + > +Previously on big endian architectures like sparc and powerpc the > +computation of SHA1 hashes was incorrect. This meant that messages > +with overlong or missing message-ids were given different computed > +message-ids than on more common little endian architectures like i386 > +and amd64. If you use notmuch on a big endian architecture, you are > +strongly advised to make a backup of your tags using `notmuch dump` > +before this upgrade. It should be possible to migrate the tags using a > +script. > + Should this mention how to find such messages? Something like notmuch dump | awk '/^notmuch-sha1-[0-9a-f]{40} / {system("notmuch search id:" $1)}' ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] util: detect byte order 2013-11-24 21:29 ` [PATCH 2/2] NEWS: News for big endian sha1 bug fix david 2013-11-25 3:10 ` Austin Clements @ 2013-11-26 13:07 ` david 2013-11-26 17:42 ` Tomi Ollila 1 sibling, 1 reply; 12+ messages in thread From: david @ 2013-11-26 13:07 UTC (permalink / raw) To: notmuch From: David Bremner <david@tethera.net> Unfortunately old versions of GCC and clang do not provide byte order macros, so we re-invent them. If UTIL_BYTE_ORDER is not defined, we fall back to macros supported by recent versions of GCC and clang --- I pushed the series id:1385328583-24602-1-git-send-email-david@tethera.net; unfortunately that broke compilation on old versions of GCC, in particular on our buildbot. Here is a proposed fix for the fix. configure | 26 ++++++++++++++++++++++++-- lib/libsha1.c | 19 +++++-------------- util/endian-util.h | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 16 deletions(-) create mode 100644 util/endian-util.h diff --git a/configure b/configure index 1a8e939..02d6396 100755 --- a/configure +++ b/configure @@ -441,6 +441,21 @@ else EOF fi +printf "Checking byte order... " +cat> _byteorder.c <<EOF +#include <stdio.h> +#include <stdint.h> +#include <string.h> +uint32_t test = 0x31323334; +char buf[5]; +int main() { memcpy(buf, &test, 4); buf[4] = '\0'; printf("%s\n", buf); return 0; } +EOF +${CC} ${CFLAGS} _byteorder.c -o _byteorder > /dev/null 2>&1 +util_byte_order=$(./_byteorder) +echo $util_byte_order + +#rm -f _byteorder _byteorder.c + if [ $errors -gt 0 ]; then cat <<EOF @@ -702,6 +717,9 @@ prefix = ${PREFIX} # LIBDIR_IN_LDCONFIG value below is still set correctly. libdir = ${LIBDIR:=\$(prefix)/lib} +# byte order within a 32 bit word. 4321 = little, 1234 = big, 0 = guess +UTIL_BYTE_ORDER = ${util_byte_order} + # Whether libdir is in a path configured into ldconfig LIBDIR_IN_LDCONFIG = ${libdir_in_ldconfig} @@ -807,7 +825,9 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS) \\ -DHAVE_STRSEP=\$(HAVE_STRSEP) \\ -DSTD_GETPWUID=\$(STD_GETPWUID) \\ -DSTD_ASCTIME=\$(STD_ASCTIME) \\ - -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) + -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) \\ + -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER) + CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS) \\ \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\ \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS) \\ @@ -815,6 +835,8 @@ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS) \\ -DHAVE_STRSEP=\$(HAVE_STRSEP) \\ -DSTD_GETPWUID=\$(STD_GETPWUID) \\ -DSTD_ASCTIME=\$(STD_ASCTIME) \\ - -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) + -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) \\ + -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER) + CONFIGURE_LDFLAGS = \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS) EOF diff --git a/lib/libsha1.c b/lib/libsha1.c index 87c7c52..3566ed7 100644 --- a/lib/libsha1.c +++ b/lib/libsha1.c @@ -34,7 +34,7 @@ */ #include <string.h> /* for memcpy() etc. */ - +#include "endian-util.h" #include "libsha1.h" #if defined(__cplusplus) @@ -49,20 +49,11 @@ extern "C" #define bswap_32(x) ((rotr32((x), 24) & 0x00ff00ff) | (rotr32((x), 8) & 0xff00ff00)) -/* The macros __BYTE_ORDER__ and __ORDER_*_ENDIAN__ are GNU C - * extensions. They are also supported by clang as of v3.2 */ - -#ifdef __BYTE_ORDER__ -# if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__) -# define bsw_32(p,n) \ - { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); } -# elif (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__) -# define bsw_32(p,n) -# else -# error "unknown byte order" -# endif +#if (UTIL_BYTE_ORDER == UTIL_ORDER_LITTLE_ENDIAN) +# define bsw_32(p,n) \ + { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); } #else -# error "macro __BYTE_ORDER__ is not defined" +# define bsw_32(p,n) #endif #define SHA1_MASK (SHA1_BLOCK_SIZE - 1) diff --git a/util/endian-util.h b/util/endian-util.h new file mode 100644 index 0000000..cbecf66 --- /dev/null +++ b/util/endian-util.h @@ -0,0 +1,39 @@ +/* this file mimics the macros present in recent GCC and CLANG */ + +#ifndef _ENDIAN_UTIL_H +#define _ENDIAN_UTIL_H + +/* This are prefixed with UTIL to avoid collisions + * + * You can use something like the following to define UTIL_BYTE_ORDER + * in a configure script. + */ +#if 0 +#include <stdio.h> +#include <stdint.h> +#include <string.h> +uint32_t test = 0x31323334; +char buf[5]; +int main() { memcpy(buf, &test, 4); buf[4] = '\0'; printf("%s\n", buf); return 0; } +#endif + +#define UTIL_ORDER_LITTLE_ENDIAN 4321 +#define UTIL_ORDER_BIG_ENDIAN 1234 + + +#if !defined(UTIL_BYTE_ORDER) || ((UTIL_BYTE_ORDER != UTIL_ORDER_LITTLE_ENDIAN) && \ + (UTIL_BYTE_ORDER != UTIL_ORDER_LITTLE_ENDIAN)) +#ifdef __BYTE_ORDER__ +# if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__) +# define UTIL_BYTE_ORDER UTIL_ORDER_LITTLE_ENDIAN +# elif (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__) +# define UTIL_BYTE_ORDER UTIL_ORDER_BIG_ENDIAN +# else +# error "Unsupported __BYTE_ORDER__" +# endif +#else +# error "UTIL_BYTE_ORDER not correctly defined and __BYTE_ORDER__ not defined." +#endif +#endif + +#endif -- 1.8.4.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] util: detect byte order 2013-11-26 13:07 ` [PATCH] util: detect byte order david @ 2013-11-26 17:42 ` Tomi Ollila 2013-11-27 0:38 ` [PATCH v2] " david 0 siblings, 1 reply; 12+ messages in thread From: Tomi Ollila @ 2013-11-26 17:42 UTC (permalink / raw) To: david, notmuch On Tue, Nov 26 2013, david@tethera.net wrote: > From: David Bremner <david@tethera.net> > > Unfortunately old versions of GCC and clang do not provide byte order > macros, so we re-invent them. Brief comments inline :D > > If UTIL_BYTE_ORDER is not defined, we fall back to macros supported by > recent versions of GCC and clang This is not entirely accurate as -DUTIL_BYTE_ORDER=... is given to compiler... maybe If UTIL_BYTE_ORDER is not defined, empty or zero (?) > --- > > I pushed the series > id:1385328583-24602-1-git-send-email-david@tethera.net; unfortunately > that broke compilation on old versions of GCC, in particular on our > buildbot. Here is a proposed fix for the fix. > > configure | 26 ++++++++++++++++++++++++-- > lib/libsha1.c | 19 +++++-------------- > util/endian-util.h | 39 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 68 insertions(+), 16 deletions(-) > create mode 100644 util/endian-util.h > > diff --git a/configure b/configure > index 1a8e939..02d6396 100755 > --- a/configure > +++ b/configure > @@ -441,6 +441,21 @@ else > EOF > fi > > +printf "Checking byte order... " > +cat> _byteorder.c <<EOF > +#include <stdio.h> > +#include <stdint.h> > +#include <string.h> > +uint32_t test = 0x31323334; > +char buf[5]; > +int main() { memcpy(buf, &test, 4); buf[4] = '\0'; printf("%s\n", buf); return 0; } > +EOF > +${CC} ${CFLAGS} _byteorder.c -o _byteorder > /dev/null 2>&1 > +util_byte_order=$(./_byteorder) > +echo $util_byte_order Austin version with 0x34333231 > + > +#rm -f _byteorder _byteorder.c > + > if [ $errors -gt 0 ]; then > cat <<EOF > > @@ -702,6 +717,9 @@ prefix = ${PREFIX} > # LIBDIR_IN_LDCONFIG value below is still set correctly. > libdir = ${LIBDIR:=\$(prefix)/lib} > > +# byte order within a 32 bit word. 4321 = little, 1234 = big, 0 = guess > +UTIL_BYTE_ORDER = ${util_byte_order} > + > # Whether libdir is in a path configured into ldconfig > LIBDIR_IN_LDCONFIG = ${libdir_in_ldconfig} > > @@ -807,7 +825,9 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS) \\ > -DHAVE_STRSEP=\$(HAVE_STRSEP) \\ > -DSTD_GETPWUID=\$(STD_GETPWUID) \\ > -DSTD_ASCTIME=\$(STD_ASCTIME) \\ > - -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) > + -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) \\ > + -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER) > + > CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS) \\ > \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\ > \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS) \\ > @@ -815,6 +835,8 @@ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS) \\ > -DHAVE_STRSEP=\$(HAVE_STRSEP) \\ > -DSTD_GETPWUID=\$(STD_GETPWUID) \\ > -DSTD_ASCTIME=\$(STD_ASCTIME) \\ > - -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) > + -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) \\ > + -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER) > + > CONFIGURE_LDFLAGS = \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS) > EOF > diff --git a/lib/libsha1.c b/lib/libsha1.c > index 87c7c52..3566ed7 100644 > --- a/lib/libsha1.c > +++ b/lib/libsha1.c > @@ -34,7 +34,7 @@ > */ > > #include <string.h> /* for memcpy() etc. */ > - > +#include "endian-util.h" > #include "libsha1.h" > > #if defined(__cplusplus) > @@ -49,20 +49,11 @@ extern "C" > > #define bswap_32(x) ((rotr32((x), 24) & 0x00ff00ff) | (rotr32((x), 8) & 0xff00ff00)) > > -/* The macros __BYTE_ORDER__ and __ORDER_*_ENDIAN__ are GNU C > - * extensions. They are also supported by clang as of v3.2 */ > - > -#ifdef __BYTE_ORDER__ > -# if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__) > -# define bsw_32(p,n) \ > - { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); } > -# elif (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__) > -# define bsw_32(p,n) > -# else > -# error "unknown byte order" > -# endif > +#if (UTIL_BYTE_ORDER == UTIL_ORDER_LITTLE_ENDIAN) > +# define bsw_32(p,n) \ > + { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); } > #else > -# error "macro __BYTE_ORDER__ is not defined" > +# define bsw_32(p,n) > #endif I'd keep the BIG_ENDIAN check too, as we don't know what would happen w/ PDP_ENDIAN... > > #define SHA1_MASK (SHA1_BLOCK_SIZE - 1) > diff --git a/util/endian-util.h b/util/endian-util.h > new file mode 100644 > index 0000000..cbecf66 > --- /dev/null > +++ b/util/endian-util.h > @@ -0,0 +1,39 @@ > +/* this file mimics the macros present in recent GCC and CLANG */ > + > +#ifndef _ENDIAN_UTIL_H > +#define _ENDIAN_UTIL_H > + > +/* This are prefixed with UTIL to avoid collisions > + * > + * You can use something like the following to define UTIL_BYTE_ORDER > + * in a configure script. > + */ > +#if 0 > +#include <stdio.h> > +#include <stdint.h> > +#include <string.h> > +uint32_t test = 0x31323334; > +char buf[5]; > +int main() { memcpy(buf, &test, 4); buf[4] = '\0'; printf("%s\n", buf); return 0; } > +#endif The "fixed simpler version" version, or just reference configure. > + > +#define UTIL_ORDER_LITTLE_ENDIAN 4321 > +#define UTIL_ORDER_BIG_ENDIAN 1234 swap :D > + > + > +#if !defined(UTIL_BYTE_ORDER) || ((UTIL_BYTE_ORDER != UTIL_ORDER_LITTLE_ENDIAN) && \ > + (UTIL_BYTE_ORDER != UTIL_ORDER_LITTLE_ENDIAN)) for "guess" this needs to be #if !UTIL_BYTE_ORDER || ((UTIL_BYTE... ... as UTIL_BYTE_ORDER could be defined as 0 (or empty?) in configure. $ gcc -dM -DUTIL_BYTE_ORDER= -E - </dev/null | grep -i util #define UTIL_BYTE_ORDER Tomi > +#ifdef __BYTE_ORDER__ > +# if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__) > +# define UTIL_BYTE_ORDER UTIL_ORDER_LITTLE_ENDIAN > +# elif (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__) > +# define UTIL_BYTE_ORDER UTIL_ORDER_BIG_ENDIAN > +# else > +# error "Unsupported __BYTE_ORDER__" > +# endif > +#else > +# error "UTIL_BYTE_ORDER not correctly defined and __BYTE_ORDER__ not defined." > +#endif > +#endif > + > +#endif > -- > 1.8.4.2 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] util: detect byte order 2013-11-26 17:42 ` Tomi Ollila @ 2013-11-27 0:38 ` david 2013-11-27 8:04 ` Tomi Ollila 0 siblings, 1 reply; 12+ messages in thread From: david @ 2013-11-27 0:38 UTC (permalink / raw) To: notmuch From: David Bremner <david@tethera.net> Unfortunately old versions of GCC and clang do not provide byte order macros, so we re-invent them. If UTIL_BYTE_ORDER is not defined or defined to 0, we fall back to macros supported by recent versions of GCC and clang --- I think I got all of Tomi's comments, including the most serious one about the test in endian-utils.h (LITTLE twice instead of BIG, LITTLE). configure | 24 ++++++++++++++++++++++-- lib/libsha1.c | 21 +++++++-------------- util/endian-util.h | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 16 deletions(-) create mode 100644 util/endian-util.h diff --git a/configure b/configure index 1a8e939..a0c6771 100755 --- a/configure +++ b/configure @@ -441,6 +441,19 @@ else EOF fi +printf "Checking byte order... " +cat> _byteorder.c <<EOF +#include <stdio.h> +#include <stdint.h> +uint32_t test = 0x34333231; +int main() { printf("%.4s\n", (const char*)&test); return 0; } +EOF +${CC} ${CFLAGS} _byteorder.c -o _byteorder > /dev/null 2>&1 +util_byte_order=$(./_byteorder) +echo $util_byte_order + +#rm -f _byteorder _byteorder.c + if [ $errors -gt 0 ]; then cat <<EOF @@ -702,6 +715,9 @@ prefix = ${PREFIX} # LIBDIR_IN_LDCONFIG value below is still set correctly. libdir = ${LIBDIR:=\$(prefix)/lib} +# byte order within a 32 bit word. 1234 = little, 4321 = big, 0 = guess +UTIL_BYTE_ORDER = ${util_byte_order} + # Whether libdir is in a path configured into ldconfig LIBDIR_IN_LDCONFIG = ${libdir_in_ldconfig} @@ -807,7 +823,9 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS) \\ -DHAVE_STRSEP=\$(HAVE_STRSEP) \\ -DSTD_GETPWUID=\$(STD_GETPWUID) \\ -DSTD_ASCTIME=\$(STD_ASCTIME) \\ - -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) + -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) \\ + -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER) + CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS) \\ \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\ \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS) \\ @@ -815,6 +833,8 @@ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS) \\ -DHAVE_STRSEP=\$(HAVE_STRSEP) \\ -DSTD_GETPWUID=\$(STD_GETPWUID) \\ -DSTD_ASCTIME=\$(STD_ASCTIME) \\ - -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) + -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) \\ + -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER) + CONFIGURE_LDFLAGS = \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS) EOF diff --git a/lib/libsha1.c b/lib/libsha1.c index 87c7c52..aaaa4eb 100644 --- a/lib/libsha1.c +++ b/lib/libsha1.c @@ -34,7 +34,7 @@ */ #include <string.h> /* for memcpy() etc. */ - +#include "endian-util.h" #include "libsha1.h" #if defined(__cplusplus) @@ -49,20 +49,13 @@ extern "C" #define bswap_32(x) ((rotr32((x), 24) & 0x00ff00ff) | (rotr32((x), 8) & 0xff00ff00)) -/* The macros __BYTE_ORDER__ and __ORDER_*_ENDIAN__ are GNU C - * extensions. They are also supported by clang as of v3.2 */ - -#ifdef __BYTE_ORDER__ -# if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__) -# define bsw_32(p,n) \ - { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); } -# elif (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__) -# define bsw_32(p,n) -# else -# error "unknown byte order" -# endif +#if (UTIL_BYTE_ORDER == UTIL_ORDER_LITTLE_ENDIAN) +# define bsw_32(p,n) \ + { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); } +#elif (UTIL_BYTE_ORDER == UTIL_ORDER_BIG_ENDIAN) +# define bsw_32(p,n) #else -# error "macro __BYTE_ORDER__ is not defined" +# error "Unsupported byte order" #endif #define SHA1_MASK (SHA1_BLOCK_SIZE - 1) diff --git a/util/endian-util.h b/util/endian-util.h new file mode 100644 index 0000000..bc80c40 --- /dev/null +++ b/util/endian-util.h @@ -0,0 +1,38 @@ +/* this file mimics the macros present in recent GCC and CLANG */ + +#ifndef _ENDIAN_UTIL_H +#define _ENDIAN_UTIL_H + +/* This are prefixed with UTIL to avoid collisions + * + * You can use something like the following to define UTIL_BYTE_ORDER + * in a configure script. + */ +#if 0 +#include <stdio.h> +#include <stdint.h> +uint32_t test = 0x34333231; +int main() { printf("%.4s\n", (const char*)&test); return 0; } +#endif + +#define UTIL_ORDER_BIG_ENDIAN 4321 +#define UTIL_ORDER_LITTLE_ENDIAN 1234 + + +#if !defined(UTIL_BYTE_ORDER) || ((UTIL_BYTE_ORDER != UTIL_ORDER_BIG_ENDIAN) && \ + (UTIL_BYTE_ORDER != UTIL_ORDER_LITTLE_ENDIAN)) +#undef UTIL_BYTE_ORDER +#ifdef __BYTE_ORDER__ +# if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__) +# define UTIL_BYTE_ORDER UTIL_ORDER_LITTLE_ENDIAN +# elif (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__) +# define UTIL_BYTE_ORDER UTIL_ORDER_BIG_ENDIAN +# else +# error "Unsupported __BYTE_ORDER__" +# endif +#else +# error "UTIL_BYTE_ORDER not correctly defined and __BYTE_ORDER__ not defined." +#endif +#endif + +#endif -- 1.8.4.2 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2] util: detect byte order 2013-11-27 0:38 ` [PATCH v2] " david @ 2013-11-27 8:04 ` Tomi Ollila 2013-11-27 12:26 ` David Bremner 0 siblings, 1 reply; 12+ messages in thread From: Tomi Ollila @ 2013-11-27 8:04 UTC (permalink / raw) To: david, notmuch On Wed, Nov 27 2013, david@tethera.net wrote: > From: David Bremner <david@tethera.net> > > Unfortunately old versions of GCC and clang do not provide byte order > macros, so we re-invent them. > > If UTIL_BYTE_ORDER is not defined or defined to 0, we fall back to > macros supported by recent versions of GCC and clang > --- > > I think I got all of Tomi's comments, including the most serious one > about the test in endian-utils.h (LITTLE twice instead of BIG, > LITTLE). Agreed. LGTM. You may want to amend the '#' out of the line +#rm -f _byteorder _byteorder.c before pushing ? Tomi > > configure | 24 ++++++++++++++++++++++-- > lib/libsha1.c | 21 +++++++-------------- > util/endian-util.h | 38 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 67 insertions(+), 16 deletions(-) > create mode 100644 util/endian-util.h > > diff --git a/configure b/configure > index 1a8e939..a0c6771 100755 > --- a/configure > +++ b/configure > @@ -441,6 +441,19 @@ else > EOF > fi > > +printf "Checking byte order... " > +cat> _byteorder.c <<EOF > +#include <stdio.h> > +#include <stdint.h> > +uint32_t test = 0x34333231; > +int main() { printf("%.4s\n", (const char*)&test); return 0; } > +EOF > +${CC} ${CFLAGS} _byteorder.c -o _byteorder > /dev/null 2>&1 > +util_byte_order=$(./_byteorder) > +echo $util_byte_order > + > +#rm -f _byteorder _byteorder.c > + > if [ $errors -gt 0 ]; then > cat <<EOF > > @@ -702,6 +715,9 @@ prefix = ${PREFIX} > # LIBDIR_IN_LDCONFIG value below is still set correctly. > libdir = ${LIBDIR:=\$(prefix)/lib} > > +# byte order within a 32 bit word. 1234 = little, 4321 = big, 0 = guess > +UTIL_BYTE_ORDER = ${util_byte_order} > + > # Whether libdir is in a path configured into ldconfig > LIBDIR_IN_LDCONFIG = ${libdir_in_ldconfig} > > @@ -807,7 +823,9 @@ CONFIGURE_CFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS) \\ > -DHAVE_STRSEP=\$(HAVE_STRSEP) \\ > -DSTD_GETPWUID=\$(STD_GETPWUID) \\ > -DSTD_ASCTIME=\$(STD_ASCTIME) \\ > - -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) > + -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) \\ > + -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER) > + > CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS) \\ > \$(TALLOC_CFLAGS) -DHAVE_VALGRIND=\$(HAVE_VALGRIND) \\ > \$(VALGRIND_CFLAGS) \$(XAPIAN_CXXFLAGS) \\ > @@ -815,6 +833,8 @@ CONFIGURE_CXXFLAGS = -DHAVE_GETLINE=\$(HAVE_GETLINE) \$(GMIME_CFLAGS) \\ > -DHAVE_STRSEP=\$(HAVE_STRSEP) \\ > -DSTD_GETPWUID=\$(STD_GETPWUID) \\ > -DSTD_ASCTIME=\$(STD_ASCTIME) \\ > - -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) > + -DHAVE_XAPIAN_COMPACT=\$(HAVE_XAPIAN_COMPACT) \\ > + -DUTIL_BYTE_ORDER=\$(UTIL_BYTE_ORDER) > + > CONFIGURE_LDFLAGS = \$(GMIME_LDFLAGS) \$(TALLOC_LDFLAGS) \$(XAPIAN_LDFLAGS) > EOF > diff --git a/lib/libsha1.c b/lib/libsha1.c > index 87c7c52..aaaa4eb 100644 > --- a/lib/libsha1.c > +++ b/lib/libsha1.c > @@ -34,7 +34,7 @@ > */ > > #include <string.h> /* for memcpy() etc. */ > - > +#include "endian-util.h" > #include "libsha1.h" > > #if defined(__cplusplus) > @@ -49,20 +49,13 @@ extern "C" > > #define bswap_32(x) ((rotr32((x), 24) & 0x00ff00ff) | (rotr32((x), 8) & 0xff00ff00)) > > -/* The macros __BYTE_ORDER__ and __ORDER_*_ENDIAN__ are GNU C > - * extensions. They are also supported by clang as of v3.2 */ > - > -#ifdef __BYTE_ORDER__ > -# if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__) > -# define bsw_32(p,n) \ > - { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); } > -# elif (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__) > -# define bsw_32(p,n) > -# else > -# error "unknown byte order" > -# endif > +#if (UTIL_BYTE_ORDER == UTIL_ORDER_LITTLE_ENDIAN) > +# define bsw_32(p,n) \ > + { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); } > +#elif (UTIL_BYTE_ORDER == UTIL_ORDER_BIG_ENDIAN) > +# define bsw_32(p,n) > #else > -# error "macro __BYTE_ORDER__ is not defined" > +# error "Unsupported byte order" > #endif > > #define SHA1_MASK (SHA1_BLOCK_SIZE - 1) > diff --git a/util/endian-util.h b/util/endian-util.h > new file mode 100644 > index 0000000..bc80c40 > --- /dev/null > +++ b/util/endian-util.h > @@ -0,0 +1,38 @@ > +/* this file mimics the macros present in recent GCC and CLANG */ > + > +#ifndef _ENDIAN_UTIL_H > +#define _ENDIAN_UTIL_H > + > +/* This are prefixed with UTIL to avoid collisions > + * > + * You can use something like the following to define UTIL_BYTE_ORDER > + * in a configure script. > + */ > +#if 0 > +#include <stdio.h> > +#include <stdint.h> > +uint32_t test = 0x34333231; > +int main() { printf("%.4s\n", (const char*)&test); return 0; } > +#endif > + > +#define UTIL_ORDER_BIG_ENDIAN 4321 > +#define UTIL_ORDER_LITTLE_ENDIAN 1234 > + > + > +#if !defined(UTIL_BYTE_ORDER) || ((UTIL_BYTE_ORDER != UTIL_ORDER_BIG_ENDIAN) && \ > + (UTIL_BYTE_ORDER != UTIL_ORDER_LITTLE_ENDIAN)) > +#undef UTIL_BYTE_ORDER > +#ifdef __BYTE_ORDER__ > +# if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__) > +# define UTIL_BYTE_ORDER UTIL_ORDER_LITTLE_ENDIAN > +# elif (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__) > +# define UTIL_BYTE_ORDER UTIL_ORDER_BIG_ENDIAN > +# else > +# error "Unsupported __BYTE_ORDER__" > +# endif > +#else > +# error "UTIL_BYTE_ORDER not correctly defined and __BYTE_ORDER__ not defined." > +#endif > +#endif > + > +#endif > -- > 1.8.4.2 > > _______________________________________________ > notmuch mailing list > notmuch@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] util: detect byte order 2013-11-27 8:04 ` Tomi Ollila @ 2013-11-27 12:26 ` David Bremner 0 siblings, 0 replies; 12+ messages in thread From: David Bremner @ 2013-11-27 12:26 UTC (permalink / raw) To: Tomi Ollila, notmuch Tomi Ollila <tomi.ollila@iki.fi> writes: > > You may want to amend the '#' out of the line > > +#rm -f _byteorder _byteorder.c > done and pushed. d ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] lib: fix byte order test in libsha1.c 2013-11-24 21:29 ` [PATCH 1/2] lib: fix byte order test in libsha1.c david 2013-11-24 21:29 ` [PATCH 2/2] NEWS: News for big endian sha1 bug fix david @ 2013-11-25 9:43 ` Tomi Ollila 1 sibling, 0 replies; 12+ messages in thread From: Tomi Ollila @ 2013-11-25 9:43 UTC (permalink / raw) To: david, notmuch On Sun, Nov 24 2013, david@tethera.net wrote: > From: David Bremner <david@tethera.net> > > Previously PLATFORM_BYTE_ORDER and IS_LITTLE_ENDIAN were not defined, > so the little endian code was always compiled in. > > This will have the effect that the "SHA1s" on big endian architectures > will change (i.e. become actual sha1s). So someone re-indexing their > database could conceivable lose tags on messages without a message-id > header. > --- > lib/libsha1.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/lib/libsha1.c b/lib/libsha1.c > index 5d16f6a..794854b 100644 > --- a/lib/libsha1.c > +++ b/lib/libsha1.c > @@ -49,11 +49,17 @@ extern "C" > > #define bswap_32(x) ((rotr32((x), 24) & 0x00ff00ff) | (rotr32((x), 8) & 0xff00ff00)) > > -#if (PLATFORM_BYTE_ORDER == IS_LITTLE_ENDIAN) > -#define bsw_32(p,n) \ > - { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); } > +#ifdef __BYTE_ORDER__ > +# if (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__) > +# define bsw_32(p,n) \ > + { int _i = (n); while(_i--) ((uint32_t*)p)[_i] = bswap_32(((uint32_t*)p)[_i]); } > +# elif (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__) > +# define bsw_32(p,n) > +# else > +# error "unknown byte order" > +# endif > #else > -#define bsw_32(p,n) > +# error "macro __BYTE_ORDER__ is not defined" > #endif LGTM. When you modify te NEWS entry by adding instructions how to find out those sha1's of message-id:s that are affected (Austin's code) You could perhaps add note of the origin of these macros (i.e. something like /* These "Common Predefined Macros" below are GNU C extensions */ ) See http://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html for reference. Tomi > > #define SHA1_MASK (SHA1_BLOCK_SIZE - 1) > -- > 1.8.4.2 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-11-27 12:26 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-11-24 11:57 notmuch sha1 implementation broken on (some) big-endian architectures David Bremner 2013-11-24 12:40 ` David Bremner 2013-11-24 16:42 ` Tomi Ollila 2013-11-24 21:29 ` [PATCH 1/2] lib: fix byte order test in libsha1.c david 2013-11-24 21:29 ` [PATCH 2/2] NEWS: News for big endian sha1 bug fix david 2013-11-25 3:10 ` Austin Clements 2013-11-26 13:07 ` [PATCH] util: detect byte order david 2013-11-26 17:42 ` Tomi Ollila 2013-11-27 0:38 ` [PATCH v2] " david 2013-11-27 8:04 ` Tomi Ollila 2013-11-27 12:26 ` David Bremner 2013-11-25 9:43 ` [PATCH 1/2] lib: fix byte order test in libsha1.c Tomi Ollila
Code repositories for project(s) associated with this public inbox https://yhetil.org/notmuch.git/ 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).