MatthieuDartiailh / bytecode

Python module to modify bytecode
https://bytecode.readthedocs.io/
MIT License
302 stars 38 forks source link

Fix the handling EXTENDED_ARG 0 #28

Closed MatthieuDartiailh closed 6 years ago

MatthieuDartiailh commented 6 years ago

The issue identified/fixed in PR 24 results from the fact that CPython sometimes emits meaningless EXTENDED_ARG (meaningless in that the argument fits in 8 bits). As a consequence, the size inferred from the argument leads to the wrong offset (since we miss one opcode) and breaks jump targets. To be safe against this, we manually keep track of the number of seen extended args to compute the size. Ideally CPython should not emit such bytecode.

This patch fixes #25 since we never leave EXTENDED_ARG opcodes when we are not supposed to. However, it should be possible to convert back a ConcreteBytecode obtained use extended_arg=True, so we support computing its stack_effect.

codecov-io commented 6 years ago

Codecov Report

Merging #28 into master will increase coverage by 0.1%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #28     +/-   ##
=========================================
+ Coverage   93.22%   93.33%   +0.1%     
=========================================
  Files          17       17             
  Lines        2481     2519     +38     
=========================================
+ Hits         2313     2351     +38     
  Misses        168      168
Impacted Files Coverage Δ
bytecode/tests/test_concrete.py 99.69% <100%> (+0.01%) :arrow_up:
bytecode/tests/test_cfg.py 99.62% <100%> (ø) :arrow_up:
bytecode/concrete.py 94.5% <100%> (+0.2%) :arrow_up:
bytecode/instr.py 93.75% <100%> (+0.06%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d3f9470...2992f72. Read the comment docs.

MatthieuDartiailh commented 6 years ago

@localhuman could you give this a look and check it does not break your code ?

localhuman commented 6 years ago

Our tests still pass after these changes 😉

MatthieuDartiailh commented 6 years ago

I just fixed one last corner case. I will leave it open for a week in case anybody comes up with something else.