Improve the lltrace feature with the Py_Debug mode

[matrixise]

BPO	25571
Nosy	@vstinner, @gwk, @matrixise, @Vgr255
Files	issue25571.patch issue25571-2.diff issue25571-3.diff issue25571-4.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields: ```python assignee = 'https://github.com/vstinner' closed_at = None created_at = labels = ['interpreter-core', 'type-feature'] title = 'Improve the lltrace feature with the Py_Debug mode' updated_at = user = 'https://github.com/matrixise' ``` bugs.python.org fields: ```python activity = actor = 'vstinner' assignee = 'vstinner' closed = False closed_date = None closer = None components = ['Interpreter Core'] creation = creator = 'matrixise' dependencies = [] files = ['40964', '43747', '43992', '43997'] hgrepos = [] issue_num = 25571 keywords = ['patch'] message_count = 20.0 messages = ['254218', '254219', '255347', '270552', '270553', '270859', '270860', '270863', '270864', '271914', '271915', '271931', '271933', '278723', '294378', '294463', '298435', '298438', '298849', '298904'] nosy_count = 6.0 nosy_names = ['vstinner', 'akaptur', 'gwk', 'matrixise', 'xcombelle', 'abarry'] pr_nums = [] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = 'enhancement' url = 'https://bugs.python.org/issue25571' versions = ['Python 3.6'] ```

[matrixise]

When we use the Py_Debug flag for the compilation, we can see the Bytecodes and the opcode arguments.

Here is a small patch, it will show the label of the opcode in the ceval.c file.

[matrixise]

Here is my patch with the improvement.

[vstinner]

What is lltrace? I never used it :-( Is it documented? Can you give examples of output before/after?

We may make your change optional to give the choice of the output.

[matrixise]

Here is a small example of lltrace when you enable it.

stephane@sg1 ~/s/h/cpython> ./python 
Python 3.6.0a3+ (default:0d8f139a6e19+, Jul 16 2016, 11:59:46) 
[GCC 6.1.1 20160621 (Red Hat 6.1.1-3)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> print
<built-in function print>
>>> print("hello")
hello
>>> __ltrace__ = None
>>> print("hello")
0: LOAD_NAME, 0
push <built-in function print>
2: LOAD_CONST, 0
push 'hello'
4: CALL_FUNCTION, 1
ext_pop 'hello'
hello
ext_pop <built-in function print>
push None
6: PRINT_EXPR
pop None
8: LOAD_CONST, 1
push None
10: RETURN_VALUE
pop None
>>> del __ltrace__
0: DELETE_NAME, 0
2: LOAD_CONST, 0
push None
4: RETURN_VALUE
pop None
>>> print("hello")
hello
>>> print("hello")

[matrixise]

here is the patch, if you want to test it, just use the REPL and add __ltrace__ = None in the REPL.

[f99e2c47-6344-48ee-8423-2bbba951464a]

I made some comment on code in review.

A thing that worry me that there is zero automated test.

In my opinion the minimal should be to test the expected output.

A nice to have would be a test for the write_contents function of makeopcodetranslations.py

[matrixise]

Totally agree with you, I am going to check that, because currently, there is no test with this "hidden" feature in Python.

I am going to discuss with Victor about this point.

Thanks,

Stephane

On 07/20, Xavier Combelle wrote:

Xavier Combelle added the comment:

I made some comment on code in review.

A thing that worry me that there is zero automated test.

In my opinion the minimal should be to test the expected output.

A nice to have would be a test for the write_contents function of makeopcodetranslations.py

---------- nosy: +xcombelle

---

Python tracker \report@bugs.python.org\ \http://bugs.python.org/issue25571\

---

---

Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/stephane%40wirtel.be

[vstinner]

It would be nice to have unit tests and docs :-)

For unit test, you can use a script like: ---

def func():
    return 1

__ltrace__ = True
func()

---

Run the script with test.support.assert_python_ok(), and check stdout.

To skip the test if Python is compiled in released mode, you can use: ---

# Were we compiled --with-pydebug or with #define Py_DEBUG?
Py_DEBUG = hasattr(sys, 'gettotalrefcount')

... @unittest.skipUnless(Py_DEBUG, 'need Py_DEBUG') ---

[vstinner]

Sorry, I don't know what is the best place to add new unit tests.

I don't know neither what is the best place to document the __ltrace__ feature. Maybe somewhere near:

• https://docs.python.org/dev/library/debug.html

Or maybe:

• https://docs.python.org/dev/library/trace.html

[matrixise]

Here is a small patch for the __ltrace__ feature.

I would like to propose a new test, __ltrace__ should be defined in the globals() dictionary and there is no check on the value associated to this key. Maybe we could defined it as a boolean, True or False.

[cd293b3e-6d38-412b-8370-a46a9aaee518]

Left some comments on Rietveld; mostly documentation-related (and pointed out some typos).

I'm also +1 on having to specify a True value rather than just defining the variable at all (i.e. setting __ltrace to False is the same as not defining it). If you change this, I'd probably force the value to be an actual bool, i.e. True or False (much like __debug; except that it's read-only and a syntax error to assign to it).

[matrixise]

Here is a new version of my patch "bpo-25571-4.diff" with the comments of Emanuel Barry.

[cd293b3e-6d38-412b-8370-a46a9aaee518]

Maybe someone who knows this feature better can weigh in, but otherwise LGTM.

[matrixise]

I have to update my patch to python-3.7, I will provide a patch asap.

[matrixise]

@haypo, you told me there is an alternative to my patch, provided by an other dev. what's the bpo issue for the alternative.

[vstinner]

https://bugs.python.org/issue29400

[5d088e7f-313b-42e6-aa21-f85bbf47e2f0]

@matrixise, I'm the author of the alternative in bpo-29400, and I'm finally finding the time to get back into it. I'm going to make a push this week to clean it up; your feedback would be much appreciated!

[vstinner]

I still strongly prefer bpo-29400 over "lltrace", since it would be usable in release mode.

[matrixise]

@gwk no problem for a review of your patch

@haypo as discussed before, maybe we could remove the lltrace feature because this one is never used by the developers.

or we could ask on @python-dev ML.

If there is a better solution and this one could replace the current lltrace, I am ok with that.

[vstinner]

I suggest to wait until sys.settrace() supports bytecode level tracing, then rewrite lltrace on top of it, open a new issue to remove C lltrace and close this issue. You need to write to python-dev if you want to remove the current C lltrace.

[sweeneyde]

I opened issue https://github.com/python/cpython/issues/91462 and PR https://github.com/python/cpython/pull/91463, which are similar to some of the patches here.

I see lltrace as a useful feature for debugging the python compiler and bytecode interpreter, or what could be useful if the output was more understandable. sys.settrace and tracing systems based on it are nice for debugging Python code, but I think debugging the interpreter itself is a different use-case with different requirements.

Say I implement some new Python feature that relies on a new opcode, but I unknowingly have an off-by-one error that makes the code accidentally skip over a POP_TOP. I wind up with a segfault on some assert(EMPTY()). But that information isn't very helpful, so I enable lltrace (whether that's by setting __ltrace__ in the appropriate module or manually setting lltrace = 1 in the c code). Then I can re-build the interpreter and see that just before the crash, there's a RETURN_VALUE instead of a POP_TOP, and which function that occurred in. Any additional printf statements I add to ceval.c will be put in context by the surrounding lltrace output, which is nice as well. The feature is especially helpful if the error occurs during a _bootstrap_python phase of the build process, where the failure can be rather opaque, and could occur before a C debugger can be attached conveniently.

I want something dead simple and transparent and understandable at the c level, as if I had just littered the code with my own printf statements, but better. So I would actually prefer that the feature not be pluggable. If I'm touching something involving how python functions themselves are called, I want print statements that completely sidestep all of that.