Skip to content

Conversation

@vstinner
Copy link
Member

Fix ltrace debug feature if the stdout encoding is not UTF-8.

If the stdout encoding is not UTF-8, the first call to
lltrace_resume_frame() indirectly sets lltrace to 0 when calling
unicode_check_encoding_errors() which calls
encodings.search_function().

Add test_lltrace.test_lltrace() test.

Fix __ltrace__ debug feature if the stdout encoding is not UTF-8.

If the stdout encoding is not UTF-8, the first call to
lltrace_resume_frame() indirectly sets lltrace to 0 when calling
unicode_check_encoding_errors() which calls
encodings.search_function().

Add test_lltrace.test_lltrace() test.
@vstinner
Copy link
Member Author

@sweeneyde @serhiy-storchaka: Would you mind to review this fix for Python 3.10? The fix is different and I adapted the test from the main branch for 3.10.

Without the fix but with the new test, LC_ALL=en_US.iso88591 ./python -m test test_lltrace fails. With the fix, the test pass.

@sweeneyde
Copy link
Member

I'm not sure why Windows CI is failing, the tests pass on my Windows machine.

Maybe it has something to do with not calling fflush(stdout) at the end of prtrace? Or maybe it's an encoding issue?

@vstinner
Copy link
Member Author

I'm not sure why Windows CI is failing, the tests pass on my Windows machine.

In the 3.10 branch, the Windows CI builds Python in release mode. I fixed the test.

@vstinner
Copy link
Member Author

@sweeneyde: the CI tests now pass.

Copy link
Member

@sweeneyde sweeneyde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. The local variable is nicer, but I agree that minimizing changes (EXT_POP and call_function) is more important.

@vstinner
Copy link
Member Author

The local variable is nicer, but I agree that minimizing changes (EXT_POP and call_function) is more important.

Adding an argument to call_function() can make the performance worse, and the code more complicated depending if LLTRACE macro is defined or not. I really want to minimize the risk of regression in 3.10 stable branch. __ltrace__ is a niche debug tool, it's mostly used by core developers to hack Python.

@vstinner vstinner merged commit 9369942 into python:3.10 May 25, 2022
@vstinner vstinner deleted the fix_lltrace_310 branch May 25, 2022 22:16
@vstinner
Copy link
Member Author

I hesitated to fix __ltrace__ typo: add a second L, but Python 3.10.0 was released last year, I prefer to not change it in a minor 3.10.x release. It's also documented at: https://docs.python.org/3.10/using/configure.html#python-debug-build

@vstinner
Copy link
Member Author

It reminds me the weird typo of float.__set_format__(): it was previously known as float.__setformat__() in Python 3.7. I removed the method in Python 3.11.

@vstinner
Copy link
Member Author

Oh, for the float method, I backported my change fixing the typo: #31558

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants