diff options
| author | Ben Harris <bjh21@bjh21.me.uk> | 2023-02-26 14:24:38 +0000 |
|---|---|---|
| committer | Ben Harris <bjh21@bjh21.me.uk> | 2023-02-26 22:47:18 +0000 |
| commit | 93be3f7ccaa63b0fd953bcfd88d685b47b76605e (patch) | |
| tree | bd23eeedb0c6c75492b67379c2fd29226e828a94 | |
| parent | 9dbcfa765ba59a8201425df18bec09c7bc334c5e (diff) | |
| download | puzzles-93be3f7ccaa63b0fd953bcfd88d685b47b76605e.zip puzzles-93be3f7ccaa63b0fd953bcfd88d685b47b76605e.tar.gz puzzles-93be3f7ccaa63b0fd953bcfd88d685b47b76605e.tar.bz2 puzzles-93be3f7ccaa63b0fd953bcfd88d685b47b76605e.tar.xz | |
Be more careful with type of left operand of <<
On a 32-bit system, evaluating 1<<31 causes undefined behaviour because
1 is signed and so it produces signed overflow. UBSan has spotted a
couple of occasions where this happens in Puzzles, so in each case I've
converted the left operand to the unsigned result type we actually want.
| -rw-r--r-- | cube.c | 4 | ||||
| -rw-r--r-- | random.c | 4 |
2 files changed, 4 insertions, 4 deletions
@@ -202,8 +202,8 @@ struct game_grid { }; #define SET_SQUARE(state, i, val) \ - ((state)->bluemask[(i)/32] &= ~(1 << ((i)%32)), \ - (state)->bluemask[(i)/32] |= ((!!val) << ((i)%32))) + ((state)->bluemask[(i)/32] &= ~(1UL << ((i)%32)), \ + (state)->bluemask[(i)/32] |= ((unsigned long)(!!val) << ((i)%32))) #define GET_SQUARE(state, i) \ (((state)->bluemask[(i)/32] >> ((i)%32)) & 1) @@ -254,12 +254,12 @@ unsigned long random_bits(random_state *state, int bits) } /* - * `(1 << bits) - 1' is not good enough, since if bits==32 on a + * `(1UL << bits) - 1' is not good enough, since if bits==32 on a * 32-bit machine, behaviour is undefined and Intel has a nasty * habit of shifting left by zero instead. We'll shift by * bits-1 and then separately shift by one. */ - ret &= (1 << (bits-1)) * 2 - 1; + ret &= (1UL << (bits-1)) * 2 - 1; return ret; } |