[issue26107] PEP 511: code.co_lnotab: use signed line number delta to support moving instructions in an optimizer

[Roundup Robot]

Roundup Robot added the comment:

New changeset 16f60cd918e0 by Victor Stinner in branch 'default':
PEP 511
https://hg.python.org/peps/rev/16f60cd918e0

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26107] PEP 511: code.co_lnotab: use signed line number delta to support moving instructions in an optimizer

[Roundup Robot]

Roundup Robot added the comment:

New changeset c6fb1651ea2e by Victor Stinner in branch 'default':
Issue #26107: Fix typo in Objects/lnotab_notes.txt
https://hg.python.org/cpython/rev/c6fb1651ea2e

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26107] PEP 511: code.co_lnotab: use signed line number delta to support moving instructions in an optimizer

[STINNER Victor]

STINNER Victor added the comment:

I closed issues #16956 and #21385 as duplicates of this issue.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26107] PEP 511: code.co_lnotab: use signed line number delta to support moving instructions in an optimizer

[STINNER Victor]

STINNER Victor added the comment:

> Nice work, my issue21385 is also related. Basically, transforming non-Python 
> code into Python meant that all line number information, which otherwise 
> would have been useful for debugging, had to be discarded, or debug builds of 
> Python would dump cores.

Ok, it looks like there are multiple use cases for negative line numbers, and 
the change doesn't really break anything in practice. I tried to explain 
exactly who is impacted and how to update the code in the Porting section of 
What's New in Python 3.6.

For each review Serhiy. I pushed the the change to Python 3.6.

--
resolution:  -> fixed
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26107] PEP 511: code.co_lnotab: use signed line number delta to support moving instructions in an optimizer

[Roundup Robot]

Roundup Robot added the comment:

New changeset 775b74e0e103 by Victor Stinner in branch 'default':
co_lnotab supports negative line number delta
https://hg.python.org/cpython/rev/775b74e0e103

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26107] PEP 511: code.co_lnotab: use signed line number delta to support moving instructions in an optimizer

[Antti Haapala]

Antti Haapala added the comment:

Nice work, my issue21385 is also related. Basically, transforming non-Python 
code into Python meant that all line number information, which otherwise would 
have been useful for debugging, had to be discarded, or debug builds of Python 
would dump cores.

So, bye "assert(d_lineno >= 0);", you won't be missed.

--
nosy: +ztane

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26107] PEP 511: code.co_lnotab: use signed line number delta to support moving instructions in an optimizer

[STINNER Victor]

STINNER Victor added the comment:

Patch version 4:

* finish to update Objects/lnotab_notes.txt
* update _PyCode_CheckLineNumber() in codeobject.c
* set importlib MAGIC to 3361

I don't expect my patch to be complete nor perfect. IMHO it's fine to adjust 
the code later if needed.

I would like to integrate FAT Python changes step by step. It looks like the 
general idea of AST optimizers is well accepted.

--
Added file: http://bugs.python.org/file41652/lnotab-4.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26107] PEP 511: code.co_lnotab: use signed line number delta to support moving instructions in an optimizer

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

The launcher on Windows does a range check.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26107] PEP 511: code.co_lnotab: use signed line number delta to support moving instructions in an optimizer

[Brett Cannon]

Brett Cannon added the comment:

There's technically no need to worry about ranged values as the magic number is 
purely an equality check to see if the interpreter matches what the .pyc was 
created with. I guess there might be third-party code that does a range check, 
but that's bad as importlib checks the raw bytes only; using a number is mostly 
a convenience for changing it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26107] PEP 511: code.co_lnotab: use signed line number delta to support moving instructions in an optimizer

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

But there is no need to increase it by 10. I suppose the gap is added to allow 
updating bytecode in maintained releases, but in process of developing next 
version we don't need this.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26107] PEP 511: code.co_lnotab: use signed line number delta to support moving instructions in an optimizer

[STINNER Victor]

STINNER Victor added the comment:

Brett Cannon added the comment:
> I just wanted to comment on "don't change importlib MAGIC: we only change it 
> between Python minor versions": that's actually not true. Feel free to up the 
> number whenever you make a change that affects eval.c or bytecode. Otherwise 
> .pyc files won't be regenerated. And that number is cheap anyway and isn't 
> about to run out, so don't worry about updating it multiple times before the 
> code sees a public release.

Since my patch may break setup of multiple python developers, it can
be worth to increase this number, ok :-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26107] PEP 511: code.co_lnotab: use signed line number delta to support moving instructions in an optimizer

[Brett Cannon]

Brett Cannon added the comment:

I just wanted to comment on "don't change importlib MAGIC: we only change it 
between Python minor versions": that's actually not true. Feel free to up the 
number whenever you make a change that affects eval.c or bytecode. Otherwise 
.pyc files won't be regenerated. And that number is cheap anyway and isn't 
about to run out, so don't worry about updating it multiple times before the 
code sees a public release.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26107] PEP 511: code.co_lnotab: use signed line number delta to support moving instructions in an optimizer

[STINNER Victor]

STINNER Victor added the comment:

> Are you aware of tools decoding directly co_lnotab?

Ah! I found Ned Batchelder's coverage project which has a _bytes_lines() method 
"adapted from dis.py in the standard library". The method uses directly 
co_lnotab to compute line numbers.

Ok, *this project* will have to be updated if it wants to support fatoptimizer 
and other code transformers producing negative line numbers.

Maybe I can contribute to it with a patch if my change to CPython 3.6 is 
accepted ;-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26107] PEP 511: code.co_lnotab: use signed line number delta to support moving instructions in an optimizer

[STINNER Victor]

STINNER Victor added the comment:

We have many unit test in the Python test suite which rely on exact line 
numbers. Examples:

* test_sys_settrace
* test_pdb
* test_dis

I know them because they were all broken when my fatoptimizer project had bugs 
related to line numbers :-)

With my patch, the full Python test suite pass whereas my patch doesn't modify 
any test.


> I afraid this patch can cause problems with code tracing where it is assumed 
> that lines are increased monotonically and *instr_lb <= frame->f_lasti < 
> *instr_ub. We should carefully analyze the effect of the patch on the tracing.

First, my patch has no impact on frame->f_lasti.

The trace module and test_sys_settrace use frame.f_lineno which 
PyFrame_GetLineNumber(). This function returns f->f_lineno if the frame has a 
trace function, or PyCode_Addr2Line(). PyCode_Addr2Line() and 
PyFrame_GetLineNumber() still work with my patch.

When you trace a program, "negative line delta" and "negative instruction 
offset" are not new in Python: it's a basic requirement to support loop, when 
you compare two instructions seen by the tracer.

To be clear, my patch does *not* introduce negative line number delta in 
co_lnotab. It only *adds support* for negative line number delta. If a tool 
decodes co_lnotab using 8-bit unsigned number for line number delta, the tool 
still works even with the patch. It only starts to return wrong line numbers if 
you start debugging a program which has negative line numbers.

If you use fatoptimizer, you get such negative delta. But if you use an 
optimizer, you should be prepared to some subtle differences. The good practice 
is to disable all optimizers when you debug code. It's also really hard (or 
impossible) to debug C code optimized with -O3. I always use gcc -O0 to debug 
CPython.


> Before committing you must ask Guido for approval. AFAIK his had objections 
> against code transformations that make debugging harder.

Are you aware of tools decoding directly co_lnotab?

Oh, I forgot the old Misc/gdbinit script which *does* decode directly 
co_lnotab. Does anyone still use it? If yes, it should also be updated.

I failed to reproduce the bug with Misc/gdbinit, beacuse bug only occurs if you 
debug a program which uses negative line number, and CPython doesn't produce 
negative line number in co_lnotab by default... So it would be "nice" to also 
support negative line number in Misc/gdbinit, but maybe it's ok to let this old 
script dying? :-D

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26107] PEP 511: code.co_lnotab: use signed line number delta to support moving instructions in an optimizer

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Yes, you patch supersedes issue16956 patch.

Added new comments on Rietveld for lnotab_notes.txt.

I afraid this patch can cause problems with code tracing where it is assumed 
that lines are increased monotonically and *instr_lb <= frame->f_lasti < 
*instr_ub. We should carefully analyze the effect of the patch on the tracing.

Before committing you must ask Guido for approval. AFAIK his had objections 
against code transformations that make debugging harder.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26107] PEP 511: code.co_lnotab: use signed line number delta to support moving instructions in an optimizer

[STINNER Victor]

STINNER Victor added the comment:

> A patch was proposed in issue16956. And issue17611 is related.

I don't see directly the link between this issue and the issue17611, but cool 
if it helps to implement new optimizations :-)

I compared my patch with issue16956 patch:

* my patch mentions also the change in Lib/importlib/_bootstrap_external.py
* my patch updates also dis.py
* my patch updates also Objects/lnotab_notes.txt
* issue16956 patch changes compiler_while(), this change is not directly 
related to making line number delta signed

Additionally, my patch uses better names in the peephole optimizer, but it's 
not directly related to the issue. By the way, this change should be commited 
in a separated patch.

I prefer to push my recent. By the way, it's up to date, whereas issue16956 
patch requires a rebase.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26107] PEP 511: code.co_lnotab: use signed line number delta to support moving instructions in an optimizer

[STINNER Victor]

STINNER Victor added the comment:

Patch version 3:

* When I reviewed issue #16956 patch, I noticed that I forgot to extract my 
changes on codeobject.c :-/ (FAT Python became a giant patch, it's hard to 
browse it!)
* Fix typo in dis.py (regression of patch 2)

--
Added file: http://bugs.python.org/file41646/lnotab-3.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26107] PEP 511: code.co_lnotab: use signed line number delta to support moving instructions in an optimizer

[STINNER Victor]

STINNER Victor added the comment:

Patch version 2 to take Serhiy's review in account:

* complete Objects/lnotab_notes.txt update. Replace (300, 300) delta example 
with (300, 200) delta for better readability (300 is the bytecode offset delta, 
200 is the line number delta)
* fix added assertion in peephole: don't check the reference counter for empty 
byte string (which can be the empty byte string singleton)
* restore addrmap name in peephole
* don't change importlib MAGIC: we only change it between Python minor 
versions, it was already changed for Python 3.6a0
* assemble_lnotab() divide with positive numbers to avoid undefined behaviour 
on C
* dis.py: use "if line_incr >= 0x80: line_incr -= 0x100" instead of 
struct.unpack() to convert unsigned to signed

Note: avoid also useless "if (x != NULL)" checks before calling PyMem_Free(). 
PyMem_Free(NULL) is well specified: do nothing.

--
Added file: http://bugs.python.org/file41645/lnotab-2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26107] PEP 511: code.co_lnotab: use signed line number delta to support moving instructions in an optimizer

[STINNER Victor]

Changes by STINNER Victor :


--
title: code.co_lnotab: use signed line number delta to support moving 
instructions in an optimizer -> PEP 511: code.co_lnotab: use signed line number 
delta to support moving instructions in an optimizer

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com