GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-133157: remove usage of `_Py_NO_SANITIZE_UNDEFINED` in `pyexpat` by picnixz · Pull Request #134050 · python/cpython

Conversation

@picnixz

@picnixz picnixz commented May 15, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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.

  • Issue: UBsan: Remove _Py_NO_SANITIZE_UNDEFINED #133157

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@picnixz
fix UBSan failures for PyExpat
9f8b1cc
@bedevere-app bedevere-app Bot mentioned this pull request May 15, 2025
UBsan: Remove _Py_NO_SANITIZE_UNDEFINED #133157
Closed
@bedevere-app bedevere-app Bot added the awaiting core review label May 15, 2025
@picnixz picnixz added skip news topic-XML labels May 15, 2025
@picnixz
picnixz requested a review from encukou May 15, 2025 12:20
@picnixz

This comment was marked as resolved.

Sign in to view
@picnixz
picnixz marked this pull request as draft May 15, 2025 13:09
@bedevere-app bedevere-app Bot removed the awaiting core review label May 15, 2025
@picnixz

This comment was marked as resolved.

Sign in to view
@picnixz
fix GCC issue?
0361267
@picnixz

picnixz commented May 15, 2025

Copy link
Copy Markdown
Member Author

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:)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@picnixz
picnixz marked this pull request as ready for review May 15, 2025 13:52
@bedevere-app bedevere-app Bot added the awaiting core review label May 15, 2025
bateller
bateller approved these changes May 15, 2025
View reviewed changes
@picnixz
picnixz requested a review from vstinner May 15, 2025 15:31
picnixz
picnixz commented May 20, 2025
View reviewed changes
Comment thread Lib/test/test_pyexpat.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
picnixz added 4 commits May 20, 2025 12:57
@picnixz
update test name
f26893f
@picnixz
Merge remote-tracking branch 'upstream/main' into fix/ubsan/expat-133157
840c4a2
@picnixz
refine test
52da4ae
@picnixz
add test comment
460725e
@picnixz

picnixz commented Jun 10, 2025

Copy link
Copy Markdown
Member Author

@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)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@picnixz
fix tests
a4cf579
@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
Restore test from pythongh-134050
2d4e590
@encukou encukou mentioned this pull request Jun 10, 2025
gh-133157: remove usage of _Py_NO_SANITIZE_UNDEFINED in pyexpat #135346
Merged
@encukou

encukou commented Jun 10, 2025

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@picnixz

picnixz commented Jun 13, 2025

Copy link
Copy Markdown
Member Author

Closing in favor of #135346
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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

Reviewers

@encukou encukou Awaiting requested review from encukou
@vstinner vstinner Awaiting requested review from vstinner
1 more reviewer
@bateller bateller bateller approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting core review skip news topic-XML

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@picnixz @encukou @bateller
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.