chiark / gitweb /
progs/..., symm/...: Fix 32-bit right-shift idiom.
authorMark Wooding <mdw@distorted.org.uk>
Tue, 30 Oct 2018 22:05:18 +0000 (22:05 +0000)
committerMark Wooding <mdw@distorted.org.uk>
Sat, 24 Nov 2018 20:12:17 +0000 (20:12 +0000)
This one has a long and troubled history.  Writing

x >> 32

is undefined behaviour if x is only 32 bits wide.  On the other hand, if
it's /not/, then this is necessary to get hold of the upper bits.

The obvious escape plan is to write

(x >> 16) >> 16

(the parentheses are unfortunately necessary), but some Microsoft
compilers managed do bungle compiling this: they merged the two shifts
together and then decided that a shift by 32 places was a no-op.

So I wrote

((x&~MASK32) >> 16) >> 16

which stood for many years.  Unfortunately this is really wrong too: if
x is wider than 32 bits, that's nice, but MASK32 /isn't/ necessarily, so
~MASK32 is all-bits zero and the high bits of x are just lost.

Fix this by casting MASK32 to the-type-of-x before inverting it.

Ugh.

18 files changed:
progs/cookie.c
progs/dsig.c
symm/blkc.h
symm/has160.c
symm/hash.h
symm/mars-mktab.c
symm/md4.c
symm/md5.c
symm/rmd128.c
symm/rmd160.c
symm/rmd256.c
symm/rmd320.c
symm/sha.c
symm/sha256.c
symm/sha3.c
symm/sha512.c
symm/tiger.c
symm/whirlpool.c

index 6239eb0c62dc023430d66c6be790a619b7130241..c6912ffd39541a4f04ddfb43ad5bd059a540d5bc 100644 (file)
@@ -80,7 +80,7 @@ typedef struct cookie {
   octet *_p = (octet *)(p);                                            \
   const cookie *_c = (c);                                              \
   STORE32(_p + 0, _c->k);                                              \
-  STORE32(_p + 4, ((_c->exp & ~MASK32) >> 16) >> 16);                  \
+  STORE32(_p + 4, ((_c->exp & ~(unsigned long)MASK32) >> 16) >> 16);   \
   STORE32(_p + 8, _c->exp);                                            \
 } while (0)
 
@@ -97,7 +97,8 @@ typedef struct cookie {
   cookie *_c = (c);                                                    \
   const octet *_p = (const octet *)(p);                                        \
   _c->k = LOAD32(_p + 0);                                              \
-  _c->exp = ((time_t)(((LOAD32(_p + 4) << 16) << 16) & ~MASK32) |      \
+  _c->exp = ((time_t)(((LOAD32(_p + 4) << 16) << 16) &                 \
+                     ~(unsigned long)MASK32) |                         \
             (time_t)LOAD32(_p + 8));                                   \
 } while (0)
 
index 5e5a3cfee2c041e33308f48ec24b41a580602431..1377bcaab287ec23c8ca48b83b5607f2db2fef1c 100644 (file)
@@ -233,7 +233,8 @@ static int bget(block *b, FILE *fp, unsigned bin)
        octet buf[8];
        if (fread(buf, sizeof(buf), 1, fp) < 1)
          return (E_EOF);
-       b->t = ((time_t)(((LOAD32(buf + 0) << 16) << 16) & ~MASK32) |
+       b->t = ((time_t)(((LOAD32(buf + 0) << 16) << 16) &
+                        ~(unsigned long)MASK32) |
                (time_t)LOAD32(buf + 4));
       } else {
        if (getstring(fp, &b->d, GSF_FILE))
@@ -325,7 +326,8 @@ static void blob(block *b, dstr *d)
        STORE32(d->buf + d->len, 0xffffffff);
        STORE32(d->buf + d->len + 4, 0xffffffff);
       } else {
-       STORE32(d->buf + d->len, ((b->t & ~MASK32) >> 16) >> 16);
+       STORE32(d->buf + d->len,
+               ((b->t & ~(unsigned long)MASK32) >> 16) >> 16);
        STORE32(d->buf + d->len + 4, b->t);
       }
       d->len += 8;
index 1cbe729c73573db7034ede47aa7910c92c43e685..ff631f0912827d713ea1a11d9d0fcb3a45319f31 100644 (file)
   unsigned _i; BLKC_W(w); unsigned long _x = x;                                \
   for (_i = 0; _i < PRE##_BLKSZ / 4; _i++) {                           \
     *_w++ = U32(_x);                                                   \
-    _x = ((_x & ~MASK32) >> 16) >> 16;                                 \
+    _x = ((_x & ~(unsigned long)MASK32) >> 16) >> 16;                  \
   }                                                                    \
 } while (0)
 
   unsigned _i; BLKC_W(w); unsigned long _x = x;        _w += PRE##_BLKSZ / 4;  \
   for (_i = 0; _i < PRE##_BLKSZ / 4; _i++) {                           \
     *--_w = U32(_x);                                                   \
-    _x = ((_x & ~MASK32) >> 16) >> 16;                                 \
+    _x = ((_x & ~(unsigned long)MASK32) >> 16) >> 16;                  \
   }                                                                    \
 } while (0)
 
index 483b9fe8a089f146fa915b233d0d40e7a2d47023..0fbaf48886449d4b1d5e8c5e91ff8b946406a338 100644 (file)
@@ -172,7 +172,7 @@ void has160_set(has160_ctx *ctx, const void *buf, unsigned long count)
   ctx->e = LOAD32_L(p + 16);
   ctx->off = 0;
   ctx->nl = U32(count);
-  ctx->nh = U32(((count & ~MASK32) >> 16) >> 16);
+  ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16);
 }
 
 /* --- @has160_hash@ --- *
index dc9ad024d14f70a500e30e5e25742c547278969f..eb3cd75ee1bae51b359731d7eb6532b9f6bf5603 100644 (file)
                                                                        \
   {                                                                    \
     uint32 _l = U32(_bsz);                                             \
-    uint32 _h = ((_bsz & ~MASK32) >> 16) >> 16;                                \
+    uint32 _h = ((_bsz & ~(size_t)MASK32) >> 16) >> 16;                        \
     _bctx->nh += _h;                                                   \
     _bctx->nl += _l;                                                   \
-    if (_bctx->nl < _l || _bctx->nl & ~MASK32)                         \
+    if (_bctx->nl < _l || _bctx->nl & ~(uint32)MASK32)                 \
       _bctx->nh++;                                                     \
   }                                                                    \
                                                                        \
index d43f7c07ab99cbe3a7644fc5f81369d851977ce1..53ce0f717ab29ed9c874a9b9274ec0c113a7c657 100644 (file)
@@ -154,10 +154,10 @@ void sha_hash(sha_ctx *ctx, const void *buf, size_t sz)
 
   {
     uint32 _l = ((uint32) ((_bsz) & MASK32));
-    uint32 _h = ((_bsz & ~MASK32) >> 16) >> 16;
+    uint32 _h = ((_bsz & ~(size_t)MASK32) >> 16) >> 16;
     _bctx->nh += _h;
     _bctx->nl += _l;
-    if (_bctx->nl < _l || _bctx->nl & ~MASK32)
+    if (_bctx->nl < _l || _bctx->nl & ~(uint32)MASK32)
       _bctx->nh++;
   }
   if (_bctx->off + _bsz < SHA_BUFSZ) {
index eee5d6b3bf9c5a9adcf5a68cc59ce5ca5eae4406..15fb750304792522f13b13bbf497d35971360844 100644 (file)
@@ -185,7 +185,7 @@ void md4_set(md4_ctx *ctx, const void *buf, unsigned long count)
   ctx->d = LOAD32_L(p + 12);
   ctx->off = 0;
   ctx->nl = U32(count);
-  ctx->nh = U32(((count & ~MASK32) >> 16) >> 16);
+  ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16);
 }
 
 /* --- @md4_hash@ --- *
index f3b37e19a79fa37525a5441c786ecf52e08ef137..cf6b3355c93c86e28dfa044015a5aec49ec589d7 100644 (file)
@@ -204,7 +204,7 @@ void md5_set(md5_ctx *ctx, const void *buf, unsigned long count)
   ctx->d = LOAD32_L(p + 12);
   ctx->off = 0;
   ctx->nl = U32(count);
-  ctx->nh = U32(((count & ~MASK32) >> 16) >> 16);
+  ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16);
 }
 
 /* --- @md5_hash@ --- *
index 85b6c33eb2046cceb3ef9f77791c65e1582d7350..606d0e57538e458b8e34ab5ca1c7f2984e1dd569 100644 (file)
@@ -284,7 +284,7 @@ void rmd128_set(rmd128_ctx *ctx, const void *buf, unsigned long count)
   ctx->d = LOAD32_L(p + 12);
   ctx->off = 0;
   ctx->nl = U32(count);
-  ctx->nh = U32(((count & ~MASK32) >> 16) >> 16);
+  ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16);
 }
 
 /* --- @rmd128_hash@ --- *
index bc7e86724773b84a189075dc03d2edbe818d654d..3dbb5210893386317d51314104ede319a6e482e6 100644 (file)
@@ -325,7 +325,7 @@ void rmd160_set(rmd160_ctx *ctx, const void *buf, unsigned long count)
   ctx->e = LOAD32_L(p + 16);
   ctx->off = 0;
   ctx->nl = U32(count);
-  ctx->nh = U32(((count & ~MASK32) >> 16) >> 16);
+  ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16);
 }
 
 /* --- @rmd160_hash@ --- *
index 99648f5fd948279a2dc5dddbaeb87d8ddb1bf4fd..0a03065b3fae22ddc5ec62592d4e740b1b9be54f 100644 (file)
@@ -294,7 +294,7 @@ void rmd256_set(rmd256_ctx *ctx, const void *buf, unsigned long count)
   ctx->D = LOAD32_L(p + 28);
   ctx->off = 0;
   ctx->nl = U32(count);
-  ctx->nh = U32(((count & ~MASK32) >> 16) >> 16);
+  ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16);
 }
 
 /* --- @rmd256_hash@ --- *
index 022903e7e7edacf58d7d3bc546dad3e4927709ab..0ccd790eac76aa1429bbd5112bd5adb51b076f72 100644 (file)
@@ -340,7 +340,7 @@ void rmd320_set(rmd320_ctx *ctx, const void *buf, unsigned long count)
   ctx->E = LOAD32_L(p + 36);
   ctx->off = 0;
   ctx->nl = U32(count);
-  ctx->nh = U32(((count & ~MASK32) >> 16) >> 16);
+  ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16);
 }
 
 /* --- @rmd320_hash@ --- *
index 980fe802908667ef5722adf01ab9d567df13d629..3520727314c1fecf3ba06c55028cc3b36fe1a039 100644 (file)
@@ -210,7 +210,7 @@ void sha_set(sha_ctx *ctx, const void *buf, unsigned long count)
   ctx->e = LOAD32(p + 16);
   ctx->off = 0;
   ctx->nl = U32(count);
-  ctx->nh = U32(((count & ~MASK32) >> 16) >> 16);
+  ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16);
 }
 
 /* --- @sha_hash@ --- *
index 5de3966d9387dbe107aec0fbb7f506d70026eb11..7f91ff745be383c986a3fe15a525d1d7d3758cdf 100644 (file)
@@ -212,7 +212,7 @@ void sha256_set(sha256_ctx *ctx, const void *buf, unsigned long count)
   ctx->h = LOAD32(p + 28);
   ctx->off = 0;
   ctx->nl = U32(count);
-  ctx->nh = U32(((count & ~MASK32) >> 16) >> 16);
+  ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16);
 }
 
 /* --- @sha256_hash@, @sha224_hash@ --- *
index 97a3f7a8afe7b8d755abbdd082bcb4e3fdfdb504..16967629071f3f6650680efe4c7ada962bdfb619 100644 (file)
@@ -231,7 +231,7 @@ static void leftenc_sz(shake_ctx *ctx, size_t n)
   octet b[9];
   unsigned i;
 
-  SET64(t, ((n&~MASK32) >> 16) >> 16, n&MASK32);
+  SET64(t, ((n&~(size_t)MASK32) >> 16) >> 16, n&MASK32);
   STORE64_B_(b + 1, t);
   for (i = 1; i < 8 && !b[i]; i++);
   i--; b[i] = 8 - i;
@@ -244,7 +244,7 @@ static void rightenc_sz(shake_ctx *ctx, size_t n)
   octet b[9];
   unsigned i;
 
-  SET64(t, ((n&~MASK32) >> 16) >> 16, n&MASK32);
+  SET64(t, ((n&~(size_t)MASK32) >> 16) >> 16, n&MASK32);
   STORE64_B_(b, t);
   for (i = 0; i < 7 && !b[i]; i++);
   b[8] = 8 - i;
index a9a5180c63bea8a39e855b6f873fb1c00cdf9612..5c75eb8ee8268c93db4d9bd50e95c08f6421eb8d 100644 (file)
@@ -275,7 +275,7 @@ void sha512_set(sha512_ctx *ctx, const void *buf, unsigned long count)
   LOAD64_(ctx->h, p + 56);
   ctx->off = 0;
   ctx->nl = U32(count);
-  ctx->nh = U32(((count & ~MASK32) >> 16) >> 16);
+  ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16);
 }
 
 /* --- @sha512_hash@, @sha384_hash@ --- *
index 95faf56f0bbd1c19521c8939f4f2bf443dffaf28..488966020c9573d77df4bcb45a8dc9d71ce74183 100644 (file)
@@ -99,7 +99,7 @@ void tiger_set(tiger_ctx *ctx, const void *buf, unsigned long count)
   LOAD64_L_(ctx->c, p + 16);
   ctx->off = 0;
   ctx->nl = U32(count);
-  ctx->nh = U32(((count & ~MASK32) >> 16) >> 16);
+  ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16);
 }
 
 /* --- @tiger_hash@ --- *
index 708e3fce3bef56857a8843320e09814b907cefea..f22a366ad200d83efcbbf1463a3118524bb74b4f 100644 (file)
@@ -206,7 +206,7 @@ void whirlpool_set(whirlpool_ctx *ctx, const void *buf, unsigned long count)
   }
   ctx->off = 0;
   ctx->nl = U32(count);
-  ctx->nh = U32(((count & ~MASK32) >> 16) >> 16);
+  ctx->nh = U32(((count & ~(unsigned long)MASK32) >> 16) >> 16);
 }
 
 /* --- @whirlpool_hash@, @whirlpool256_hash@ --- *