Skip to content

Commit fa35523

Browse files
authored
Fix test_none_collisions on 32-bit systems (#69)
There are two issues at play here: 1. Python version of `map_hash` unnecessarily performs hash truncation even if the hash is already 32-bit wide, which potentially converts it from signed int to unsigned long. 2. The `test_none_collisions` test generates a collision node with hash greater than 2^32. Both of these are problematic on 32-bit systems, where `sizeof(Py_hash_t)` is 4, and so anything that doesn't fit into `Py_hash_t` gets bit-mangled, breaking the `hash(x) != x` invariance that the test relies upon. Fixes: #53 Fixes: #50
1 parent 3f8cb24 commit fa35523

File tree

3 files changed

+22
-7
lines changed

3 files changed

+22
-7
lines changed

.github/workflows/tests.yml

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,13 @@ jobs:
1616
matrix:
1717
python-version: [3.6, 3.7, 3.8, 3.9, 3.10.0-beta.4]
1818
os: [windows-latest, ubuntu-latest, macos-latest]
19+
arch: [x64, x86]
20+
exclude:
21+
# 32-bit Python is only available on Windows
22+
- os: ubuntu-latest
23+
arch: x86
24+
- os: macos-latest
25+
arch: x86
1926

2027
steps:
2128
- uses: actions/checkout@v2
@@ -38,11 +45,12 @@ jobs:
3845
if: steps.release.outputs.version == 0
3946
with:
4047
python-version: ${{ matrix.python-version }}
48+
architecture: ${{ matrix.arch }}
4149

4250
- name: Test
4351
if: steps.release.outputs.version == 0
4452
run: |
45-
pip install -e .[test]
53+
pip install --verbose -e .[test]
4654
flake8 immutables/ tests/
4755
mypy immutables/
4856
python -m pytest -v

immutables/map.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@
1919

2020
def map_hash(o):
2121
x = hash(o)
22-
return (x & 0xffffffff) ^ ((x >> 32) & 0xffffffff)
22+
if sys.hash_info.width > 32:
23+
return (x & 0xffffffff) ^ ((x >> 32) & 0xffffffff)
24+
else:
25+
return x
2326

2427

2528
def map_mask(hash, shift):

tests/test_none_keys.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import ctypes
12
import unittest
23

34
from immutables.map import map_hash, map_mask, Map as PyMap
@@ -6,16 +7,19 @@
67

78
none_hash = map_hash(None)
89
assert(none_hash != 1)
9-
assert((none_hash >> 32) == 0)
10+
assert(none_hash.bit_length() <= 32)
1011

11-
not_collision = 0xffffffff & (~none_hash)
12+
none_hash_u = ctypes.c_size_t(none_hash).value
13+
not_collision = 0xffffffff & (~none_hash_u)
1214

1315
mask = 0x7ffffffff
14-
none_collisions = [none_hash & (mask >> shift)
16+
none_collisions = [none_hash_u & (mask >> shift)
1517
for shift in reversed(range(0, 32, 5))]
1618
assert(len(none_collisions) == 7)
17-
none_collisions = [h | (not_collision & (mask << shift))
18-
for shift, h in zip(range(5, 37, 5), none_collisions)]
19+
none_collisions = [
20+
ctypes.c_ssize_t(h | (not_collision & (mask << shift))).value
21+
for shift, h in zip(range(5, 37, 5), none_collisions)
22+
]
1923

2024

2125
class NoneCollision(HashKey):

0 commit comments

Comments
 (0)