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 6239eb0..c6912ff 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 5e5a3cf..1377bca 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 1cbe729..ff631f0 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 483b9fe..0fbaf48 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 dc9ad02..eb3cd75 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 d43f7c0..53ce0f7 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 eee5d6b..15fb750 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 f3b37e1..cf6b335 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 85b6c33..606d0e5 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 bc7e867..3dbb521 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 99648f5..0a03065 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 022903e..0ccd790 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 980fe80..3520727 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 5de3966..7f91ff7 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 97a3f7a..1696762 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 a9a5180..5c75eb8 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 95faf56..4889660 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 708e3fc..f22a366 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@ --- *