Skip to content

gh-133157: remove usage of _Py_NO_SANITIZE_UNDEFINED in pyexpat#134050

Closed
picnixz wants to merge 7 commits intopython:mainfrom
picnixz:fix/ubsan/expat-133157
Closed

gh-133157: remove usage of _Py_NO_SANITIZE_UNDEFINED in pyexpat#134050
picnixz wants to merge 7 commits intopython:mainfrom
picnixz:fix/ubsan/expat-133157

Conversation

@picnixz
Copy link
Copy Markdown
Member

@picnixz picnixz commented May 15, 2025

IIUC ISO C11, we can use "union casts" (which are actually compound literals, and thus constructors) to pass use unions. The idea is to consider each different function pointer as a union of possible function pointers and store them as such. In some sense, we're doing singledispatch with known registered types.

@picnixz

This comment was marked as resolved.

@picnixz picnixz marked this pull request as draft May 15, 2025 13:09
@picnixz

This comment was marked as resolved.

@picnixz
Copy link
Copy Markdown
Member Author

picnixz commented May 15, 2025

Hum, looks like it worked. But I don't know why the pointer alternative didn't work (in clang it does?). I guess it's because of some optimization and it got eliminated (or maybe what I did was also a UB c:)

@picnixz picnixz marked this pull request as ready for review May 15, 2025 13:52
@picnixz picnixz requested a review from vstinner May 15, 2025 15:31
Comment thread Lib/test/test_pyexpat.py Outdated
@picnixz
Copy link
Copy Markdown
Member Author

picnixz commented Jun 10, 2025

@encukou Could you have a look at this one please? I don't have a better alternative than using hacky unions (though it's a recommended construction as per ISO C11)

@picnixz picnixz force-pushed the fix/ubsan/expat-133157 branch from aac33de to a4cf579 Compare June 10, 2025 09:27
encukou added a commit to encukou/cpython that referenced this pull request Jun 10, 2025
@encukou
Copy link
Copy Markdown
Member

encukou commented Jun 10, 2025

I think this could be simpler, see #135346. But I might be missing something.

@picnixz
Copy link
Copy Markdown
Member Author

picnixz commented Jun 13, 2025

Closing in favor of #135346

@picnixz picnixz closed this Jun 13, 2025
@picnixz picnixz deleted the fix/ubsan/expat-133157 branch June 13, 2025 16:47
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