Conversation

picnixz
  • remove _read_null() as it's a tricky one (and the compiler didn't seem to complain)
  • instead of using 1/0 arithmetic, we explicitly raise SIGFPE

@picnixz
fix UBSan failures in faulthandler.c
a0b2967
- reading from NULL is no more an undefined behavior for C11
- instead of using 1/0 arithmetic, we explicitly raise SIGFPE
@bedevere-appbedevere-app bot mentioned this pull request May 15, 2025
Open
@picnixzpicnixz requested review from vstinner and encukou May 15, 2025 12:17
vstinner
vstinner reviewed May 15, 2025
View reviewed changes
vstinner
vstinner reviewed May 15, 2025
View reviewed changes
picnixz
picnixz commented May 15, 2025
View reviewed changes
picnixz added 2 commits May 15, 2025 17:30
@picnixz
Update Modules/faulthandler.c
130da64
@picnixz
remove faulthandler._read_null()
05d7ff1
Reading from NULL is an undefined behavior and `faulthandler`
should not check for low-level C about undefined behaviors.
@picnixzpicnixz requested a review from vstinner May 16, 2025 09:03
@picnixz

Should I actually add a NEWS entry? the function was not documented but it had a docstring.

vstinner
vstinner approved these changes May 17, 2025
View reviewed changes

Choose a reason for hiding this comment

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

LGTM

@vstinner

Should I actually add a NEWS entry? the function was not documented but it had a docstring.

It doesn't need a NEWS entry since it's a private function written for test_faulthandler.

@picnixzpicnixz merged commit 0a160bf into python:main May 18, 2025
42 checks passed
@picnixzpicnixz deleted the fix/ubsan/faulthandler-133157 branch May 18, 2025 08:16
Sign up for free to join this conversation on . Already have an account? Sign in to comment

@vstinner vstinner vstinner approved these changes

@encukou encukou Awaiting requested review from encukou

No one assigned
skip news
None yet
No milestone

Successfully merging this pull request may close these issues.