Skip to content

GH-127903: Fix a crash on DEBUG builds when calling _copy_characters #127876

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
Merged
Eclips4 merged 7 commits into from
Jan 3, 2025
Merged

GH-127903: Fix a crash on DEBUG builds when calling _copy_characters #127876

Eclips4 merged 7 commits into from
Jan 3, 2025

Conversation

@shadchin
Copy link
Contributor

@shadchin shadchin commented Dec 12, 2024 •
edited by bedevere-app bot
Loading

Reproduce on Python 3.12.8+ or 3.13.1+, main work fine, but there is a problem there too:

./configure --with-pydebug
make
./python --version
Python 3.13.1+
./python -c 'import datetime as dt; dt.datetime(2013, 11, 10, 14, 20, 59).strftime("%z")'
Segmentation fault

No need to check to if we don't write there

@shadchin shadchin changed the title Fix segmentation fault Fix segmentation fault in pydebug mode Dec 12, 2024
@Eclips4
Copy link
Member

Eclips4 commented Dec 12, 2024

Hi @shadchin! 🙂

For some reason, it doesn't segfault on the current main, but it does on 3.13+ and 3.12+.
The fix looks correct to me, thank you.
Could you please create an issue with a short explanation of the problem and then create a news entry?

@Eclips4 Eclips4 added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Dec 12, 2024
@shadchin shadchin mentioned this pull request Dec 13, 2024
Closed
@shadchin shadchin changed the title Fix segmentation fault in pydebug mode GH-127903: Fix segmentation fault in pydebug mode Dec 13, 2024
@shadchin
Copy link
Contributor Author

shadchin commented Dec 13, 2024

Done

@skirpichev
Copy link
Member

skirpichev commented Dec 13, 2024

Why not add a test?

@Eclips4
Copy link
Member

Eclips4 commented Dec 13, 2024

Why not add a test?

Adding a test would be good, but I'm not sure where it should be placed. In Lib/test/datetimetester.py since reproducer contains datetime? Perhaps, but in fact it was caused by calling _PyUnicodeWriter_WriteSubstring in _datetimemodule::wrap_strftime, which is not part of datetime.

@skirpichev
Copy link
Member

skirpichev commented Dec 13, 2024

I'm not sure where it should be placed

test_str.py?

@skirpichev
Copy link
Member

skirpichev commented Dec 14, 2024

@shadchin, please avoid force-pushing (squashing, etc) if your PR is marked as ready for review.

@skirpichev skirpichev requested a review from Eclips4 December 14, 2024 06:52
picnixz
picnixz reviewed Dec 14, 2024
View reviewed changes
@picnixz picnixz changed the title GH-127903: Fix segmentation fault in pydebug mode GH-127903: Fix a crash on DEBUG builds when calling _copy_characters Dec 14, 2024
picnixz
picnixz reviewed Dec 29, 2024
View reviewed changes
picnixz
picnixz approved these changes Jan 3, 2025
View reviewed changes
Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

With this, we can also hunt for cases when we pass to = NULL and how many != 0 and it will be easier to debug in the future.

LGTM.

Eclips4
Eclips4 reviewed Jan 3, 2025
View reviewed changes
@Eclips4 Eclips4 enabled auto-merge (squash) January 3, 2025 18:06
@Eclips4 Eclips4 merged commit 46cb634 into python:main Jan 3, 2025
41 checks passed
@miss-islington-app
Copy link

miss-islington-app bot commented Jan 3, 2025

Thanks @shadchin for the PR, and @Eclips4 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

@bedevere-app
Copy link

bedevere-app bot commented Jan 3, 2025

GH-128458 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Jan 3, 2025
@bedevere-app
Copy link

bedevere-app bot commented Jan 3, 2025

GH-128459 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Jan 3, 2025
@bedevere-app
Copy link

bedevere-app bot commented Jan 3, 2025

GH-128458 is a backport of this pull request to the 3.13 branch.

@bedevere-app
Copy link

bedevere-app bot commented Jan 3, 2025

GH-128459 is a backport of this pull request to the 3.12 branch.

@shadchin shadchin deleted the fix_segfault branch January 4, 2025 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Eclips4 Eclips4 Eclips4 left review comments

@skirpichev skirpichev skirpichev approved these changes

@picnixz picnixz picnixz approved these changes

Assignees

No one assigned

Labels

None yet

Milestone

No milestone