From c5a12719194f8a04813b1a5dc62f0fee9fc67733 Mon Sep 17 00:00:00 2001 From: Wordman Date: Tue, 10 Dec 2019 23:48:33 +0200 Subject: [PATCH 1/4] Properly fix `Jump out of too many nested blocks` error (overrides previous fix, as merging them would be too complex and unneeded) --- goto.py | 68 +++++++++++++++++------------ test_goto.py | 120 ++++++++++++++++++++++++++++++--------------------- 2 files changed, 113 insertions(+), 75 deletions(-) diff --git a/goto.py b/goto.py index 3c67459..81a4ae5 100644 --- a/goto.py +++ b/goto.py @@ -86,6 +86,19 @@ def _parse_instructions(code): extended_arg_offset = None yield (dis.opname[opcode], oparg, offset) +def _get_instruction_size(opname, oparg=0): + size = 1 + + extended_arg = oparg >> _BYTECODE.argument_bits + if extended_arg != 0: + size += _get_instruction_size('EXTENDED_ARG', extended_arg) + oparg &= (1 << _BYTECODE.argument_bits) - 1 + + opcode = dis.opmap[opname] + if opcode >= _BYTECODE.have_argument: + size += _BYTECODE.argument.size + + return size def _write_instruction(buf, pos, opname, oparg=0): extended_arg = oparg >> _BYTECODE.argument_bits @@ -164,37 +177,38 @@ def _patch_code(code): target_depth = len(target_stack) if origin_stack[:target_depth] != target_stack: raise SyntaxError('Jump into different block') + + size = 0 + for i in range(len(origin_stack) - target_depth): + size += _get_instruction_size('POP_BLOCK') + size += _get_instruction_size('JUMP_ABSOLUTE', target // _BYTECODE.jump_unit) + + moved_to_end = False + if pos + size > end: + # not enough space, add at end + pos = _write_instruction(buf, pos, 'JUMP_ABSOLUTE', len(buf) // _BYTECODE.jump_unit) + + if pos > end: + raise SyntaxError('Internal error - not enough bytecode space') + + size += _get_instruction_size('JUMP_ABSOLUTE', end // _BYTECODE.jump_unit) + + moved_to_end = True + pos = len(buf) + buf.extend([0] * size) - failed = False try: for i in range(len(origin_stack) - target_depth): pos = _write_instruction(buf, pos, 'POP_BLOCK') - - if target >= end: - rel_target = (target - pos) // _BYTECODE.jump_unit - oparg_bits = 0 - - while True: - rel_target -= (1 + _BYTECODE.argument.size) // _BYTECODE.jump_unit - if rel_target >> oparg_bits == 0: - pos = _write_instruction(buf, pos, 'EXTENDED_ARG', 0) - break - - oparg_bits += _BYTECODE.argument_bits - if rel_target >> oparg_bits == 0: - break - - pos = _write_instruction(buf, pos, 'JUMP_FORWARD', rel_target) - else: - pos = _write_instruction(buf, pos, 'JUMP_ABSOLUTE', target // _BYTECODE.jump_unit) - - except (IndexError, struct.error): - failed = True - - if failed or pos > end: - raise SyntaxError('Jump out of too many nested blocks') - - _inject_nop_sled(buf, pos, end) + pos = _write_instruction(buf, pos, 'JUMP_ABSOLUTE', target // _BYTECODE.jump_unit) + except (IndexError, struct.error) as e: + raise SyntaxError("Internal error", e) + + if moved_to_end: + pos = _write_instruction(buf, pos, 'JUMP_ABSOLUTE', end // _BYTECODE.jump_unit) + + else: + _inject_nop_sled(buf, pos, end) return _make_code(code, _array_to_bytes(buf)) diff --git a/test_goto.py b/test_goto.py index 7ff1dc1..cabc7b4 100644 --- a/test_goto.py +++ b/test_goto.py @@ -71,63 +71,87 @@ def func(): pytest.raises(SyntaxError, with_goto, func) -if sys.version_info >= (3, 6): - def test_jump_out_of_nested_2_loops(): - @with_goto - def func(): - x = 1 - for i in range(2): - for j in range(2): - # These are more than 256 bytes of bytecode, requiring - # a JUMP_FORWARD below on Python 3.6+, since the absolute - # address would be too large, after leaving two blocks. - x += x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x - x += x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x - x += x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x +def test_jump_out_of_nested_2_loops(): + @with_goto + def func(): + x = 1 + for i in range(2): + for j in range(2): + # These are more than 256 bytes of bytecode + x += x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x + x += x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x + x += x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x + + goto .end + label .end + return (i, j) + assert func() == (0, 0) + +def test_jump_out_of_nested_3_loops(): + @with_goto + def func(): + for i in range(2): + for j in range(2): + for k in range(2): goto .end - label .end - return (i, j) + label .end + return (i, j, k) - assert func() == (0, 0) + assert func() == (0, 0, 0) - def test_jump_out_of_nested_3_loops(): - def func(): - for i in range(2): - for j in range(2): - for k in range(2): +def test_jump_out_of_nested_4_loops(): + @with_goto + def func(): + for i in range(2): + for j in range(2): + for k in range(2): + for m in range(2): goto .end - label .end - return (i, j, k) - - pytest.raises(SyntaxError, with_goto, func) -else: - def test_jump_out_of_nested_4_loops(): - @with_goto - def func(): - for i in range(2): - for j in range(2): - for k in range(2): - for m in range(2): - goto .end - label .end - return (i, j, k, m) + label .end + return (i, j, k, m) - assert func() == (0, 0, 0, 0) + assert func() == (0, 0, 0, 0) - def test_jump_out_of_nested_5_loops(): - def func(): - for i in range(2): - for j in range(2): - for k in range(2): - for m in range(2): - for n in range(2): - goto .end - label .end - return (i, j, k, m, n) +def test_jump_out_of_nested_5_loops(): + @with_goto + def func(): + for i in range(2): + for j in range(2): + for k in range(2): + for m in range(2): + for n in range(2): + goto .end + label .end + return (i, j, k, m, n) - pytest.raises(SyntaxError, with_goto, func) + assert func() == (0, 0, 0, 0, 0) + +def test_jump_out_of_nested_11_loops(): + @with_goto + def func(): + x = 1 + for i1 in range(2): + for i2 in range(2): + for i3 in range(2): + for i4 in range(2): + for i5 in range(2): + for i6 in range(2): + for i7 in range(2): + for i8 in range(2): + for i9 in range(2): + for i10 in range(2): + for i11 in range(2): + # These are more than 256 bytes of bytecode + x += x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x + x += x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x + x += x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x + + goto .end + label .end + return (i1, i2, i3, i4, i5, i6, i7, i8, i9, i10, i11) + assert func() == (0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0) def test_jump_across_loops(): def func(): From 8b431a23da4d52e1291690640fad05a549d4a819 Mon Sep 17 00:00:00 2001 From: condut Date: Wed, 11 Dec 2019 00:46:46 +0200 Subject: [PATCH 2/4] Rename new SyntaxError to be more descriptive in case ever encountered. I was not able to get it to happen for pretty huge functions (much much larger than the ones in the test), though, so there's a good chance it's unreachable. --- goto.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/goto.py b/goto.py index 81a4ae5..18fcf1d 100644 --- a/goto.py +++ b/goto.py @@ -189,7 +189,7 @@ def _patch_code(code): pos = _write_instruction(buf, pos, 'JUMP_ABSOLUTE', len(buf) // _BYTECODE.jump_unit) if pos > end: - raise SyntaxError('Internal error - not enough bytecode space') + raise SyntaxError('Goto in an incredibly huge function') # not sure if reachable size += _get_instruction_size('JUMP_ABSOLUTE', end // _BYTECODE.jump_unit) From c1613f5c2b4d3aabdd20cb12ffb271e71fd7fedd Mon Sep 17 00:00:00 2001 From: condut Date: Sat, 14 Dec 2019 01:27:03 +0200 Subject: [PATCH 3/4] Style & test updates (other updates to commit in following commits) --- goto.py | 25 +++++++++++------------ test_goto.py | 56 +++++++++++----------------------------------------- 2 files changed, 24 insertions(+), 57 deletions(-) diff --git a/goto.py b/goto.py index 18fcf1d..0e79b3e 100644 --- a/goto.py +++ b/goto.py @@ -88,17 +88,17 @@ def _parse_instructions(code): def _get_instruction_size(opname, oparg=0): size = 1 - + extended_arg = oparg >> _BYTECODE.argument_bits if extended_arg != 0: size += _get_instruction_size('EXTENDED_ARG', extended_arg) oparg &= (1 << _BYTECODE.argument_bits) - 1 - + opcode = dis.opmap[opname] - if opcode >= _BYTECODE.have_argument: + if opcode >= _BYTECODE.have_argument: size += _BYTECODE.argument.size - - return size + + return size def _write_instruction(buf, pos, opname, oparg=0): extended_arg = oparg >> _BYTECODE.argument_bits @@ -177,22 +177,22 @@ def _patch_code(code): target_depth = len(target_stack) if origin_stack[:target_depth] != target_stack: raise SyntaxError('Jump into different block') - + size = 0 for i in range(len(origin_stack) - target_depth): size += _get_instruction_size('POP_BLOCK') size += _get_instruction_size('JUMP_ABSOLUTE', target // _BYTECODE.jump_unit) - + moved_to_end = False if pos + size > end: # not enough space, add at end pos = _write_instruction(buf, pos, 'JUMP_ABSOLUTE', len(buf) // _BYTECODE.jump_unit) - + if pos > end: raise SyntaxError('Goto in an incredibly huge function') # not sure if reachable - + size += _get_instruction_size('JUMP_ABSOLUTE', end // _BYTECODE.jump_unit) - + moved_to_end = True pos = len(buf) buf.extend([0] * size) @@ -202,11 +202,10 @@ def _patch_code(code): pos = _write_instruction(buf, pos, 'POP_BLOCK') pos = _write_instruction(buf, pos, 'JUMP_ABSOLUTE', target // _BYTECODE.jump_unit) except (IndexError, struct.error) as e: - raise SyntaxError("Internal error", e) - + raise SyntaxError("Internal error") + if moved_to_end: pos = _write_instruction(buf, pos, 'JUMP_ABSOLUTE', end // _BYTECODE.jump_unit) - else: _inject_nop_sled(buf, pos, end) diff --git a/test_goto.py b/test_goto.py index cabc7b4..6b12f0c 100644 --- a/test_goto.py +++ b/test_goto.py @@ -88,45 +88,6 @@ def func(): assert func() == (0, 0) -def test_jump_out_of_nested_3_loops(): - @with_goto - def func(): - for i in range(2): - for j in range(2): - for k in range(2): - goto .end - label .end - return (i, j, k) - - assert func() == (0, 0, 0) - -def test_jump_out_of_nested_4_loops(): - @with_goto - def func(): - for i in range(2): - for j in range(2): - for k in range(2): - for m in range(2): - goto .end - label .end - return (i, j, k, m) - - assert func() == (0, 0, 0, 0) - -def test_jump_out_of_nested_5_loops(): - @with_goto - def func(): - for i in range(2): - for j in range(2): - for k in range(2): - for m in range(2): - for n in range(2): - goto .end - label .end - return (i, j, k, m, n) - - assert func() == (0, 0, 0, 0, 0) - def test_jump_out_of_nested_11_loops(): @with_goto def func(): @@ -142,11 +103,18 @@ def func(): for i9 in range(2): for i10 in range(2): for i11 in range(2): - # These are more than 256 bytes of bytecode - x += x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x - x += x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x - x += x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x+x - + # These are more than + # 256 bytes of bytecode + x += (x+x+x+x+x+x+x+x+x+ + x+x+x+x+x+x+x+x+x+ + x+x+x+x+x+x+x+x+x) + x += (x+x+x+x+x+x+x+x+x+ + x+x+x+x+x+x+x+x+x+ + x+x+x+x+x+x+x+x+x) + x += (x+x+x+x+x+x+x+x+x+ + x+x+x+x+x+x+x+x+x+ + x+x+x+x+x+x+x+x+x) + goto .end label .end return (i1, i2, i3, i4, i5, i6, i7, i8, i9, i10, i11) From df4038234aa2a679177c2512fcdf708056ecfc07 Mon Sep 17 00:00:00 2001 From: condut Date: Sat, 14 Dec 2019 01:35:36 +0200 Subject: [PATCH 4/4] Refactor the ops sizing/writing to avoid code duplication (This was originally in the second PR, but since you want to take this one first, I think it's a good commit to add here, since it's relevant to this change and would make diffing against future PRs easier) --- goto.py | 53 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/goto.py b/goto.py index 0e79b3e..cc6d746 100644 --- a/goto.py +++ b/goto.py @@ -100,6 +100,15 @@ def _get_instruction_size(opname, oparg=0): return size +def _get_instructions_size(ops): + size = 0 + for op in ops: + if isinstance(op, str): + size += _get_instruction_size(op) + else: + size += _get_instruction_size(*op) + return size + def _write_instruction(buf, pos, opname, oparg=0): extended_arg = oparg >> _BYTECODE.argument_bits if extended_arg != 0: @@ -116,6 +125,13 @@ def _write_instruction(buf, pos, opname, oparg=0): return pos +def _write_instructions(buf, pos, ops): + for op in ops: + if isinstance(op, str): + pos = _write_instruction(buf, pos, op) + else: + pos = _write_instruction(buf, pos, *op) + return pos def _find_labels_and_gotos(code): labels = {} @@ -178,35 +194,28 @@ def _patch_code(code): if origin_stack[:target_depth] != target_stack: raise SyntaxError('Jump into different block') - size = 0 + ops = [] for i in range(len(origin_stack) - target_depth): - size += _get_instruction_size('POP_BLOCK') - size += _get_instruction_size('JUMP_ABSOLUTE', target // _BYTECODE.jump_unit) + ops.append('POP_BLOCK') + ops.append(('JUMP_ABSOLUTE', target // _BYTECODE.jump_unit)) - moved_to_end = False - if pos + size > end: - # not enough space, add at end - pos = _write_instruction(buf, pos, 'JUMP_ABSOLUTE', len(buf) // _BYTECODE.jump_unit) + if pos + _get_instructions_size(ops) > end: + # not enough space, add code at buffer end and jump there + buf_end = len(buf) - if pos > end: - raise SyntaxError('Goto in an incredibly huge function') # not sure if reachable + go_to_end_ops = [('JUMP_ABSOLUTE', buf_end // _BYTECODE.jump_unit)] - size += _get_instruction_size('JUMP_ABSOLUTE', end // _BYTECODE.jump_unit) + if pos + _get_instructions_size(go_to_end_ops) > end: + # not sure if reachable + raise SyntaxError('Goto in an incredibly huge function') - moved_to_end = True - pos = len(buf) - buf.extend([0] * size) + pos = _write_instructions(buf, pos, go_to_end_ops) + _inject_nop_sled(buf, pos, end) - try: - for i in range(len(origin_stack) - target_depth): - pos = _write_instruction(buf, pos, 'POP_BLOCK') - pos = _write_instruction(buf, pos, 'JUMP_ABSOLUTE', target // _BYTECODE.jump_unit) - except (IndexError, struct.error) as e: - raise SyntaxError("Internal error") - - if moved_to_end: - pos = _write_instruction(buf, pos, 'JUMP_ABSOLUTE', end // _BYTECODE.jump_unit) + buf.extend([0] * _get_instructions_size(ops)) + _write_instructions(buf, buf_end, ops) else: + pos = _write_instructions(buf, pos, ops) _inject_nop_sled(buf, pos, end) return _make_code(code, _array_to_bytes(buf))