Skip to content

gh-126024: fix UBSan failure in unicodeobject.c:find_first_nonascii#127566

Merged
colesbury merged 5 commits into
python:mainfrom
picnixz:fix/ubsan/misaligned-loads-126024
Dec 6, 2024
Merged

gh-126024: fix UBSan failure in unicodeobject.c:find_first_nonascii#127566
colesbury merged 5 commits into
python:mainfrom
picnixz:fix/ubsan/misaligned-loads-126024

Conversation

@picnixz

@picnixz picnixz commented Dec 3, 2024

Copy link
Copy Markdown
Member

Not sure we can do anything except wrapping the call in a function. Adding an allow list may seem too hard since the line would change everytime we change the code. Or we can just forget about the optimized unaligned load.

@picnixz picnixz changed the title gh-126024: fix UBSan failure in unicodeobject.c gh-126024: fix UBSan failure in unicodeobject.c:find_first_nonascii Dec 3, 2024
Comment thread Objects/unicodeobject.c Outdated
Comment thread Objects/unicodeobject.c
@colesbury
colesbury merged commit 36c6178 into python:main Dec 6, 2024
@colesbury

Copy link
Copy Markdown
Contributor

Does this need backports to 3.12 and 3.13?

@colesbury

Copy link
Copy Markdown
Contributor

Does this need backports to 3.12 and 3.13?

Looks like the relevant code is not in 3.13 or earlier, so I guess not.

@picnixz
picnixz deleted the fix/ubsan/misaligned-loads-126024 branch December 6, 2024 14:39
@picnixz

picnixz commented Dec 6, 2024

Copy link
Copy Markdown
Member Author

Looks like the relevant code is not in 3.13 or earlier, so I guess not.

Yup, it's only a recent code (that's why I did 2 PRs).

@picnixz

picnixz commented Dec 6, 2024

Copy link
Copy Markdown
Member Author

Hum, we have a failure: https://github.com/python/cpython/actions/runs/12200589926/job/34037087571.

I'm not sure what happened :(

@colesbury

Copy link
Copy Markdown
Contributor

Ok, let's revert it for now while we investigate.

colesbury added a commit to colesbury/cpython that referenced this pull request Dec 6, 2024
…irst_nonascii` (pythonGH-127566)"

Some hypothesis tests are failing.

This reverts commit 36c6178.
@picnixz

picnixz commented Dec 6, 2024

Copy link
Copy Markdown
Member Author

I'll investigate now.

@colesbury

Copy link
Copy Markdown
Contributor

Hmm... from my limited testing, the failure looks like it also reproduces main before the PR merged.

@picnixz

picnixz commented Dec 6, 2024

Copy link
Copy Markdown
Member Author

(I also deduced it; I commented the revert PR). Sorry for the false alarm. But now we have another issue (for which I'll open a new ticket).

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.

3 participants