Skip to content

Commit 0b234df

Browse files
durvesh1992meta-codesync[bot]
authored andcommitted
Fix off-by-one in exception-table emit_item at bit-24 boundary (#124)
Summary: `ExceptionTable.emit_item` in `compiler/pyassem.py` packs a value 6 bits at a time. The three lower group thresholds use `>=`, but the top group used `>`: ```python if value > (1 << 24): # <- bug: should be >= self.write_byte((value >> 24) | CONTINUATION_BIT | msb) msb = 0 for i in (18, 12, 6): if value >= (1 << i): # the other thresholds use >= ... ``` At exactly `value == 1 << 24` (16777216) the leading 6-bit group is skipped, so the value is emitted as `40 40 40 00` and decodes back to **0** — a corrupt exception-table entry. Values one below/above encode fine, so it is a precise boundary bug. This mirrors CPythons `assemble_emit_exception_table_item` in `Python/assemble.c`, which uses `value >= 1 << 24`. The inconsistency within the same function (one `>`, three `>=`) is the tell. ### Impact `emit_item` is fed block `start`/`size`/`target` byte-offsets and `depth_lasti` via `emit_entry`, so any of those equal to `16777216` yields a table entry that points at offset 0. ### Fix Use `>=`, matching the other thresholds and CPython. One character. ### Tests Added round-trip tests to `test_exception_table.py` over all four group boundaries `(1<<i)-1, 1<<i, (1<<i)+1` for `i in {6,12,18,24}` plus `0/1/(1<<30)-1`. Verified the suite fails only at `16777216` before the fix and passes after: ``` BUG(>): failing values = [16777216] FIX(>=): failing values = [] ``` Pull Request resolved: #124 Reviewed By: yoney Differential Revision: D110795289 Pulled By: alexmalyshev fbshipit-source-id: 4306347afe8dfcb6fca966cddb1918255146683e
1 parent 52458db commit 0b234df

2 files changed

Lines changed: 33 additions & 1 deletion

File tree

cinderx/PythonLib/cinderx/compiler/pyassem.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3661,7 +3661,7 @@ def emit_item(self, value: int, msb: int) -> None:
36613661
assert value >= 0 and value < (1 << 30)
36623662
CONTINUATION_BIT = 64
36633663

3664-
if value > (1 << 24):
3664+
if value >= (1 << 24):
36653665
self.write_byte((value >> 24) | CONTINUATION_BIT | msb)
36663666
msb = 0
36673667
for i in (18, 12, 6):

cinderx/PythonLib/test_cinderx/test_compiler/test_exception_table.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,35 @@ def test_encoding(self) -> None:
3838
]
3939
actual = ParsedExceptionTable.from_bytes(table).entries
4040
self.assertEqual(expected, actual)
41+
42+
@staticmethod
43+
def _decode_item(buf: bytes) -> int:
44+
# Decode a single varint emitted by emit_item(value, msb=0): 6 data bits
45+
# per byte, continuation bit (64) set on every byte but the last.
46+
it = iter(buf)
47+
b = next(it)
48+
value = b & 0x3F
49+
while b & 64:
50+
b = next(it)
51+
value = (value << 6) | (b & 0x3F)
52+
return value
53+
54+
def test_emit_item_roundtrip_across_group_boundaries(self) -> None:
55+
# emit_item packs a value 6 bits at a time. Each group threshold uses
56+
# `value >= (1 << i)`; the top group (bit 24) must use the same `>=`.
57+
# A `>` there drops bit 24 at exactly value == 1 << 24, corrupting the
58+
# entry (it decodes back to 0).
59+
values = []
60+
for i in (6, 12, 18, 24):
61+
values += [(1 << i) - 1, 1 << i, (1 << i) + 1]
62+
values += [0, 1, (1 << 30) - 1]
63+
for value in values:
64+
with self.subTest(value=value):
65+
e = ExceptionTable()
66+
e.emit_item(value, 0)
67+
self.assertEqual(self._decode_item(e.getTable()), value)
68+
69+
def test_emit_item_bit24_boundary(self) -> None:
70+
e = ExceptionTable()
71+
e.emit_item(1 << 24, 0)
72+
self.assertEqual(self._decode_item(e.getTable()), 1 << 24)

0 commit comments

Comments
 (0)