Skip to content

Commit

Permalink
GH-91389: Fix dis position information for CACHEs (GH-93663)
Browse files Browse the repository at this point in the history
  • Loading branch information
brandtbucher authored Jun 16, 2022
1 parent 4f85cec commit f8e576b
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 12 deletions.
32 changes: 22 additions & 10 deletions Lib/dis.py
Original file line number Diff line number Diff line change
Expand Up @@ -497,17 +497,29 @@ def _get_instructions_bytes(code, varname_from_oparg=None,
yield Instruction(_all_opname[op], op,
arg, argval, argrepr,
offset, starts_line, is_jump_target, positions)
if show_caches and _inline_cache_entries[deop]:
for name, caches in _cache_format[opname[deop]].items():
data = code[offset + 2: offset + 2 + caches * 2]
argrepr = f"{name}: {int.from_bytes(data, sys.byteorder)}"
for _ in range(caches):
offset += 2
yield Instruction(
"CACHE", 0, 0, None, argrepr, offset, None, False, None
)
# Only show the actual value for the first cache entry:
caches = _inline_cache_entries[deop]
if not caches:
continue
if not show_caches:
# We still need to advance the co_positions iterator:
for _ in range(caches):
next(co_positions, ())
continue
for name, size in _cache_format[opname[deop]].items():
for i in range(size):
offset += 2
# Only show the fancy argrepr for a CACHE instruction when it's
# the first entry for a particular cache value and the
# instruction using it is actually quickened:
if i == 0 and op != deop:
data = code[offset: offset + 2 * size]
argrepr = f"{name}: {int.from_bytes(data, sys.byteorder)}"
else:
argrepr = ""
yield Instruction(
"CACHE", CACHE, 0, None, argrepr, offset, None, False,
Positions(*next(co_positions, ()))
)

def disassemble(co, lasti=-1, *, file=None, show_caches=False, adaptive=False):
"""Disassemble a code object."""
Expand Down
36 changes: 34 additions & 2 deletions Lib/test/test_dis.py
Original file line number Diff line number Diff line change
Expand Up @@ -1197,8 +1197,10 @@ def test_show_caches(self):
caches = list(self.get_cached_values(quickened, adaptive))
for cache in caches:
self.assertRegex(cache, pattern)
self.assertEqual(caches.count(""), 8)
self.assertEqual(len(caches), 22)
total_caches = 22
empty_caches = 8 if adaptive and quickened else total_caches
self.assertEqual(caches.count(""), empty_caches)
self.assertEqual(len(caches), total_caches)


class DisWithFileTests(DisTests):
Expand Down Expand Up @@ -1751,6 +1753,36 @@ def test_co_positions_missing_info(self):
self.assertIsNone(positions.col_offset)
self.assertIsNone(positions.end_col_offset)

@requires_debug_ranges()
def test_co_positions_with_lots_of_caches(self):
def roots(a, b, c):
d = b**2 - 4 * a * c
yield (-b - cmath.sqrt(d)) / (2 * a)
if d:
yield (-b + cmath.sqrt(d)) / (2 * a)
code = roots.__code__
ops = code.co_code[::2]
cache_opcode = opcode.opmap["CACHE"]
caches = sum(op == cache_opcode for op in ops)
non_caches = len(ops) - caches
# Make sure we have "lots of caches". If not, roots should be changed:
assert 1 / 3 <= caches / non_caches, "this test needs more caches!"
for show_caches in (False, True):
for adaptive in (False, True):
with self.subTest(f"{adaptive=}, {show_caches=}"):
co_positions = [
positions
for op, positions in zip(ops, code.co_positions(), strict=True)
if show_caches or op != cache_opcode
]
dis_positions = [
instruction.positions
for instruction in dis.get_instructions(
code, adaptive=adaptive, show_caches=show_caches
)
]
self.assertEqual(co_positions, dis_positions)

# get_instructions has its own tests above, so can rely on it to validate
# the object oriented API
class BytecodeTests(InstructionTestCase, DisTestBase):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix an issue where :mod:`dis` utilities could report missing or incorrect
position information in the presence of ``CACHE`` entries.

0 comments on commit f8e576b

Please sign in to comment.