This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Alternative fix for BUILD_MAP_UNPACK_WITH_CALL opcode in 3.5
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ncoghlan Nosy List: barry, ishcherb, larry, mark.dickinson, ncoghlan, petr.viktorin, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2017-02-12 14:28 by serhiy.storchaka, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
BUILD_MAP_UNPACK_WITH_CALL-no-function_location.patch serhiy.storchaka, 2017-02-12 14:29 review
Pull Requests
URL Status Linked Edit
PR 169 merged ncoghlan, 2017-02-19 07:24
PR 558 merged ncoghlan, 2017-03-08 09:10
Messages (10)
msg287637 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-02-12 14:28
Proposed patch is an alternative fix of issue27286 for Python 3.5. Rather than fixing the compiler and bumping the magic number it just avoids using incorrect value in eval loop. The patch can be useful for disro maintainers that don't want to recompile all .pyc files when update Python to 3.5.3. I think it would be the best way to fix issue27286. Now it can be combined with either reverting issue27286 changes or applying the workaround https://github.com/encukou/cpython/commit/magic-number-workaround .

The patch makes the high byte of oparg be used only when it is unambiguous (equal to 1). In that case the error message contains a function name. Otherwise the error message doesn't contain a function name. This is small usability regression, but the BUILD_MAP_UNPACK_WITH_CALL opcode is very rarely used and shouldn't raise an exception in normal case.
msg287649 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-02-12 20:38
Thanks Serhiy, I think that will work well downstream in combination with Petr's patch to accept both magic numbers.
msg288116 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-02-19 07:31
In putting together a PR for this, I think it *only* makes sense if we also push Petr's change upstream to accept the legacy bytecode files in 3.5.4+.

The reason is that the patch actually causes a test case to fail due to "f()" change to "function" in an error message.
msg288506 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-02-24 07:17
For easier cross-referencing, note that http://bugs.python.org/issue29514 is the issue proposing a test case that ensures future maintainers are aware of the practical problems created by bumping the bytecode magic number in a maintenance release.
msg288508 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-02-24 08:23
I updated the PR to cover both Serhiy's change to make the legacy bytecode safe to interpret, and a tweaked version of Petr's change to allow the legacy bytecode to be imported.

The main tweaks to the latter were:

- more in depth comments explaining the changes
- switching the C level changes to rely on a couple of extern variables exported from import.c rather than scattering the backwards compatibility numbers throughout the code

I'm also cc'ing Larry into the discussion as 3.5 release manager. Larry, most of the context for this change is actually in http://bugs.python.org/issue29514 where we reported the problems with the magic number change that prompted Petr to create a downstream patch for Fedora to restore compatibility with the legacy bytecode magic number.

While the answer for 3.6+ is "Let's try to avoid ever doing this again", it turns out this is tricky enough to handle that it would be nice to have a shared solution upstream in 3.5.4+ that redistributors can collectively adopt, rather than everyone needing to come up with their own workaround for the problem.
msg289170 - (view) Author: Iryna Shcherbina (ishcherb) * Date: 2017-03-07 16:00
If I may ask, what was the decision on this matter?

We are planning to rebase Python 3.5 for Fedora and this currently blocks us, if we do not work this around with a patch.
Let me know if there is anything I can help with to speed up the process.
msg289200 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-03-08 05:21
I just realised I need to add some test cases to test_importlib/source/test_file_loader.py before merging it, but since Larry hasn't objected to the proposed approach, I'm going to go ahead and implement this fix.
msg289206 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-03-08 06:44
Merged (with test cases) in https://github.com/python/cpython/commit/93602e3af70d3b9f98ae2da654b16b3382b68d50

The test cases even cover ensuring the backwards compatibility also applies to frozen bytecode :)
msg290264 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-03-24 22:41
New changeset 0410bee6e6c87f37e91f6cae3627d8e3704075f1 by Nick Coghlan in branch '3.5':
bpo-29537: Also cover 3.5.2 in NEWS entry
https://github.com/python/cpython/commit/0410bee6e6c87f37e91f6cae3627d8e3704075f1
msg290271 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2017-03-24 22:42
New changeset 93602e3af70d3b9f98ae2da654b16b3382b68d50 by Nick Coghlan in branch '3.5':
[3.5] bpo-29537: Tolerate legacy invalid bytecode (#169)
https://github.com/python/cpython/commit/93602e3af70d3b9f98ae2da654b16b3382b68d50
History
Date User Action Args
2022-04-11 14:58:43adminsetgithub: 73723
2017-03-24 22:42:19ncoghlansetmessages: + msg290271
2017-03-24 22:41:21ncoghlansetmessages: + msg290264
2017-03-08 09:10:57ncoghlansetpull_requests: + pull_request459
2017-03-08 06:44:44ncoghlansetstatus: open -> closed
resolution: fixed
messages: + msg289206

stage: patch review -> resolved
2017-03-08 05:21:47ncoghlansetmessages: + msg289200
2017-03-07 16:00:16ishcherbsetmessages: + msg289170
2017-02-24 08:23:37ncoghlansetnosy: + larry
messages: + msg288508
2017-02-24 07:17:30ncoghlansetmessages: + msg288506
2017-02-24 07:09:02ncoghlansettype: behavior
2017-02-24 07:08:46ncoghlansetassignee: ncoghlan
2017-02-19 07:31:31ncoghlansetmessages: + msg288116
2017-02-19 07:24:46ncoghlansetpull_requests: + pull_request135
2017-02-13 13:00:02mark.dickinsonsetnosy: + mark.dickinson
2017-02-13 12:30:00ishcherbsetnosy: + ishcherb
2017-02-12 20:38:19ncoghlansetmessages: + msg287649
2017-02-12 14:29:23serhiy.storchakasetfiles: + BUILD_MAP_UNPACK_WITH_CALL-no-function_location.patch
keywords: + patch
2017-02-12 14:28:54serhiy.storchakacreate