gh-96678: Fix UB of null pointer arithmetic

[matthiasgoergens]

• Issue: Undefined behaviour in main and 3.11 #96678

Automerge-Triggered-By: GH:pablogsal

[pablogsal]

Have the assert ever triggered in the test suite? Do you have the stack trace at that point?

[pablogsal]

Also can you add a news entry?

[matthiasgoergens]

Have the assert ever triggered in the test suite? Do you have the stack trace at that point?

Yes, the assert triggers in the test suite. You can see ths when you look at the build bot's test run for that commit. I don't know whether that build bot gives a stack trace, though.

I can try to give you stack trace.

[pablogsal]

Have the assert ever triggered in the test suite? Do you have the stack trace at that point?

Yes, the assert triggers in the test suite. You can see ths when you look at the build bot's test run for that commit. I don't know whether that build bot ein gives so a stack trace, though.

I can try to give you stack trace.

No need, just add the NEWS entry and I can land this. Interestingly this code has been here since the beginning of time :P

[pablogsal]

Ah, but this should have been detected by the UB buildbot no? This means that it happens only in a subprocess or there is some shenanigans again that prevent this to surface in release mode

[pablogsal]

Ah, indeed. All failures happen in subprocesses. But this should be detected by the buildbot in any case I think.

[matthiasgoergens]

Ah, but this should have been detected by the UB buildbot no? This means that it happens only in a subprocess or there is some shenanigans again that prevent this to surface in release mode

I explained the shenanigans in the last comment on the issue.

Basically, it's only undefined behaviour in standard C.

We are using -fwrapv which makes it defined behaviour. That's why the sanitiser doesn't complain.

My goal here to clean this up anyway, so we can consider running without -fwrapv, and perhaps get a few more C compiler optimisations.thst way.

[pablogsal]

We are using -fwrapv which makes it defined behaviour. That's why the sanitiser doesn't complain.

But for this case we need -fwrapv-pointer. I assume that's also passed in release mode?

[pablogsal]

In any case, I am landing the PR. Thanks a lot for working on this. Your work really makes a difference ❤️

[matthiasgoergens]

Just from reading the compiler docs, I would agree.

But in practice, -fwrapv seems to be enough for some reason.

[matthiasgoergens]

My goal is to make it so that we can run with -fstrict-overflow (if we want to.)

[matthiasgoergens]

@pablogsal faster-cpython/ideas#458 might be interesting for you in this context.

[kumaraditya303]

The bot isn't merging this because 'awaiting review' was added mistakenly, I am fixing the labels.

[miss-islington]

Thanks @matthiasgoergens for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

[bedevere-bot]

GH-96796 is a backport of this pull request to the 3.11 branch.

[bedevere-bot]

GH-96797 is a backport of this pull request to the 3.10 branch.