aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBen Harris <bjh21@bjh21.me.uk>2023-02-26 14:24:38 +0000
committerBen Harris <bjh21@bjh21.me.uk>2023-02-26 22:47:18 +0000
commit93be3f7ccaa63b0fd953bcfd88d685b47b76605e (patch)
treebd23eeedb0c6c75492b67379c2fd29226e828a94
parent9dbcfa765ba59a8201425df18bec09c7bc334c5e (diff)
downloadpuzzles-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.c4
-rw-r--r--random.c4
2 files changed, 4 insertions, 4 deletions
diff --git a/cube.c b/cube.c
index 69a9d35..3b4e975 100644
--- a/cube.c
+++ b/cube.c
@@ -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)
diff --git a/random.c b/random.c
index 5527d6f..bd11f16 100644
--- a/random.c
+++ b/random.c
@@ -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;
}