Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Windows assertion in out-of-tree debug build #89538

Closed
zooba opened this issue Oct 5, 2021 · 6 comments
Closed

Windows assertion in out-of-tree debug build #89538

zooba opened this issue Oct 5, 2021 · 6 comments
Assignees
@zooba
Labels
3.11 only security fixes OS-windows

Comments

@zooba
Copy link
Member

zooba commented Oct 5, 2021

BPO 45375
Nosy @pfmoore, @tjguk, @zware, @eryksun, @zooba, @pablogsal
PRs
  • bpo-45375: Fix assertion failure due to searching for stdlib in unnormalised paths #28735
  • bpo-45375: Fix off by one error in buffer allocation #28764
    		•

    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:

    assignee = 'https://github.com/zooba'
closed_at = <Date 2021-10-06.19:09:45.673>
created_at = <Date 2021-10-05.11:38:41.394>
labels = ['OS-windows', '3.11']
title = 'Windows assertion in out-of-tree debug build'
updated_at = <Date 2021-10-06.19:09:45.673>
user = 'https://github.com/zooba'

    bugs.python.org fields:

    activity = <Date 2021-10-06.19:09:45.673>
actor = 'steve.dower'
assignee = 'steve.dower'
closed = True
closed_date = <Date 2021-10-06.19:09:45.673>
closer = 'steve.dower'
components = ['Windows']
creation = <Date 2021-10-05.11:38:41.394>
creator = 'steve.dower'
dependencies = []
files = []
hgrepos = []
issue_num = 45375
keywords = ['patch']
message_count = 6.0
messages = ['403229', '403230', '403236', '403304', '403322', '403333']
nosy_count = 6.0
nosy_names = ['paul.moore', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower', 'pablogsal']
pr_nums = ['28735', '28764']
priority = 'high'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue45375'
versions = ['Python 3.11']
    @zooba
    Copy link
    Member Author

    zooba commented Oct 5, 2021

    As seen in the release build for 3.11a1, an assertion is raised when attempting to launch the debug build out of tree.

       _RegenTestFrozenmain:
     Regenerate test_frozenmain.h
     D:\a\1\b\bin\amd64\python_d.exe Programs\freeze_test_frozenmain.py [Programs/test_frozenmain.h](https://github.com/python/cpython/blob/main/Programs/test_frozenmain.h)
     Assertion failed: stdlibdir[wcslen(prefix)] == SEP, file D:\a\1\s\PC\getpathp.c, line 300

    This causes the release to fail.

    @zooba zooba added release-blocker 3.11 only security fixes labels Oct 5, 2021
    @zooba zooba self-assigned this Oct 5, 2021
    @zooba
    Copy link
    Member Author

    zooba commented Oct 5, 2021

    I believe this is because getpath has hit its final fallback case of looking in ".\DLLs" and ".\Lib" for the standard library, and has not performed proper normalisation on these paths. So after removing the last segment, the result is "." which does not end in SEP.

    So at least in this case, we can probably just ignore the assertion. However, I'm concerned that this would open up other cases where search paths may be added that are otherwise unintended.

    The expected behaviour when the stdlib can't be found is a detailed error message, not a debug assertion/crash.

    @zooba
    Copy link
    Member Author

    zooba commented Oct 5, 2021

    New changeset 5146877 by Steve Dower in branch 'main':
    bpo-45375: Fix assertion failure due to searching for stdlib in unnormalised paths (GH-28735)
    5146877

    @zooba zooba closed this as completed Oct 5, 2021
    @eryksun
    Copy link
    Contributor

    eryksun commented Oct 6, 2021

    I left a message on the PR a day ago about a one-off error in the allocation of buff. The size should be MAXPATHLEN + 1. That's what's used everywhere else in PC/getpathp.c and what gets passed in the PathCchCombineEx(buff, MAXPATHLEN + 1, ...) call.

    @eryksun eryksun reopened this Oct 6, 2021
    @zooba
    Copy link
    Member Author

    zooba commented Oct 6, 2021

    Thanks for mentioning it! New PR to fix

    @zooba
    Copy link
    Member Author

    zooba commented Oct 6, 2021

    New changeset 6c942a8 by Steve Dower in branch 'main':
    bpo-45375: Fix off by one error in buffer allocation (GH-28764)
    6c942a8

    @zooba zooba closed this as completed Oct 6, 2021
    @zooba zooba closed this as completed Oct 6, 2021
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @zooba zooba

    Labels
    3.11 only security fixes OS-windows
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    2 participants
    @eryksun @zooba