Skip to content

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Sep 19, 2019

@vstinner
Copy link
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
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
Member Author

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

Copy link
Member

@zware zware left a comment

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.

@vstinner
Copy link
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
Copy link
Member

zware commented Sep 19, 2019

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
Member Author

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

@matrixise
Copy link
Member

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

@vstinner
Copy link
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