Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

bpo-38205: Convert Py_UNREACHABLE() macro to a function#16280

Closed
vstinner wants to merge 3 commits into
python:masterpython/cpython:masterfrom
vstinner:unreachableCopy head branch name to clipboard
Closed

bpo-38205: Convert Py_UNREACHABLE() macro to a function#16280
vstinner wants to merge 3 commits into
python:masterpython/cpython:masterfrom
vstinner:unreachableCopy head branch name to clipboard

Conversation

@vstinner

@vstinner vstinner commented Sep 19, 2019

Copy link
Copy Markdown
Member

Comment thread Python/errors.c
@vstinner

Copy link
Copy Markdown
Member Author

This change:

  • Now dump the Python traceback of all threads when Py_UNREACHABLE() is called: call Py_FatalError()
  • Call DebugBreak() on Windows in debug mode (Py_FatalError)
  • Should make Python binary a few bytes smaller in debug mode: replace fputs()+abort() calls with a single call to Py_UNREACHABLE()
  • Keep the short version of the xkcd joke in debug mode

I didn't keep the long version of the joke since RANDALL_WAS_HERE compilation flag is not documented anyway. If someone wants to keep it, I suggest to document it in Misc/SpecialBuilds.txt. Moreover, I dislike untested code :-/ I'm also fine with merging the two xkcd texts into a single longer one if you prefer.

Recently, @serhiy-storchaka fixed "from future import barry_as_FLUFL": this easteregg doesn't need to rebuild Python, so I'm fine with keep it.

@vstinner

Copy link
Copy Markdown
Member Author

You can test this change by (1) applying this change:

diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c
index a40eb421f5..a85c3fbaae 100644
--- a/Python/bltinmodule.c
+++ b/Python/bltinmodule.c
@@ -1552,6 +1552,7 @@ builtin_len(PyObject *module, PyObject *obj)
         assert(PyErr_Occurred());
         return NULL;
     }
+    if (res == 123456789) { Py_UNREACHABLE(); }
     return PyLong_FromSsize_t(res);
 }
 

(2) run this script:

class HaveLength:
    def __len__(self):
        return 123456789

def func(obj):
    print("len", len(obj))

def main():
    func(HaveLength())

main()

Output with this change (in debug mode):

$ ./python x.py 
Fatal Python error: We've reached an unreachable state. Anything is possible.
The limits were in our heads all along. Follow your dreams.
https://xkcd.com/2200
Python runtime state: initialized

Current thread 0x00007f81b54ec740 (most recent call first):
  File "/home/vstinner/python/master/x.py", line 6 in func
  File "/home/vstinner/python/master/x.py", line 9 in main
  File "/home/vstinner/python/master/x.py", line 11 in <module>
Aborted (core dumped)

Output without this change (in debug mode):

$ ./python x.py
ERROR:

We've reached an unreachable state. Anything is possible.
The limits were in our heads all along. Follow your dreams.

https://xkcd.com/2200
Aborted (core dumped)

This main enhancement is that you now get the Python traceback where the bug occurred.

@vstinner

Copy link
Copy Markdown
Member Author

This change fix https://bugs.python.org/issue38205

@zware zware left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One nit about the release-mode message, but the overall change looks reasonable to me.

Comment thread Python/errors.c Outdated
@vstinner

Copy link
Copy Markdown
Member Author

@zware: @serhiy-storchaka prefers to keep a macro rather converting it to a function: https://bugs.python.org/issue38205#msg352782 What's the opinion?

@zware

zware commented Sep 19, 2019

Copy link
Copy Markdown
Member

I'm somewhat in favor of keeping it as a macro just for complete backwards compatibility, even if it's implemented as a private _Py_Unreachable function. I will note that about the only third-party usage of Py_UNREACHABLE that comes up in a quick GitHub search is in py3c which checks #if defined(Py_UNREACHABLE), which may not play nicely with a function.

@vstinner

Copy link
Copy Markdown
Member Author

I wrote PR #16290 which fix https://bugs.python.org/issue38205 but keeps Py_UNREACHABLE() as a macro.

@matrixise

Copy link
Copy Markdown
Member

@vstinner I think you can close this one and merge PR #16290

@vstinner

Copy link
Copy Markdown
Member Author

It was decided to merge PR #16290 instead.

@vstinner vstinner closed this Sep 20, 2019
@vstinner vstinner deleted the unreachable branch September 20, 2019 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.