8000 GH-127903: Fix a crash on DEBUG builds when calling `_copy_characters` by shadchin · Pull Request #127876 · python/cpython · GitHub
[go: up one dir, main page]

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.

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

Merged
merged 7 commits into from
Jan 3, 2025

Conversation

shadchin
Copy link
Contributor
@shadchin shadchin commented Dec 12, 2024

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

Done

@skirpichev
8000 Copy link
Contributor

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
Contributor

I'm not sure where it should be placed

test_str.py?

@skirpichev
Copy link
Contributor

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

@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
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 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

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 3, 2025
…icodeobject::_copy_characters`` (pythonGH-127876)

(cherry picked from commit 46cb634)

Co-authored-by: Alexander Shadchin <shadchin@yandex-team.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 3, 2025
…icodeobject::_copy_characters`` (pythonGH-127876)

(cherry picked from commit 46cb634)

Co-authored-by: Alexander Shadchin <shadchin@yandex-team.com>
@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.

Eclips4 pushed a commit that referenced this pull request Jan 3, 2025
…nicodeobject::_copy_characters` (GH-127876) (#128458)

gh-127903: Fix a crash on debug builds when calling `Objects/unicodeobject::_copy_characters`` (GH-127876)
(cherry picked from commit 46cb634)

Co-authored-by: Alexander Shadchin <shadchin@yandex-team.com>
Eclips4 pushed a commit that referenced this pull request Jan 3, 2025
…nicodeobject::_copy_characters` (GH-127876) (#128459)

gh-127903: Fix a crash on debug builds when calling `Objects/unicodeobject::_copy_characters`` (GH-127876)
(cherry picked from commit 46cb634)

Co-authored-by: Alexander Shadchin <shadchin@yandex-team.com>
WolframAlph pushed a commit to WolframAlph/cpython that referenced this pull request Jan 4, 2025
@shadchin shadchin deleted the fix_segfault branch January 4, 2025 13:28
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0