PEP 7: Mention signed overflow explicitly

[matthiasgoergens]

This is a draft for discussion of python/cpython#96821

[gvanrossum]

I'm not sure what this is trying to say. Do you mean that you can cause Python to ignore such overflows by passing -fwrapv? Or do you mean that those overflows are ignored because -fwrapv is being passed? I'm not sure that a compiler flag is really the same as a C dialect (we'd have to document a ton of dialects otherwise).

[matthiasgoergens]

For a long, long time CPython's C source code (or at least some parts of it) assumed that signed integers use twos-complement and that signed integer overflow is defined behaviour. Of course, in the C standard signed overflow is undefined behaviour.

Initially that seemed fine, but then GCC 4.1 (I think) started taking advantage of the undefined behaviour to optimise code more. That's when we introduced the -fwrapv flag. Later on clang came along, and needed the same treatment.

So that's the latter of the two possibilities you mentioned. (Though not exactly, because the overflow isn't ignored.)

As far as I can tell the flag also make bit shifting of negative numbers and arithmetic on the null pointer valid operations, amongst other things.

My aspiration with the linked issue python/cpython#96821 is to relieve CPython's C code from the reliance on this assumption.

That way you can build CPython with a standards compliant compiler, even if it doesn't support that flag. As far as I can tell, msvc doesn't have this flag (we definitely don't pass it) and we just got lucky so far that msvc seems to be doing the right thing for us.

The other reason to get by without -fwrapv is to enable more optimisations in GCC and clang.

About which C flags to document: I would suggest to document any settings that shift the border of defined vs undefined behaviour. Ie any settings (or assumptions!) that change anything mention in the C standard, if that makes sense?

But not flags that don't have such an effect. Eg nothing about optimisation levels, or warnings or exactly what libraries we are linking etc.

I don't have strong opinions on where to put this information. A reviewer on the linked issue suggested considering a change to this PEP, so I made a draft PR for the section that looked the most appropriate to me.

Of course, whether we want to drop reliance on defined signed integer overflow or -fwrapv at all is another question. The text in this PR is written from the perspective of dropping those two things.

In practice, we would want to drop the assumption first and clean up the code. And once we are reasonably sure, we can drop the flag, too.

[gvanrossum]

Hm, that's all fair. I haven't followed this detail in a long time, I had actually assumed we did those arithmetic operations using unsigned (at least since GCC started "abusing" the undefined behavior), but I stand corrected. I had also assumed that the "dialect" was more about specifying the version of the C standard used and which optional parts thereof. That was probably naive.

Nevertheless, I wonder if this could be expressed better, e.g. replacing "tacitly allowed" with something like "depended on the undefined behavior of allowing" (or something like that), and explicitly calling out GCC and Clang when mentioning -fwrapv. (As you say MSVC doesn't support that option, at least not in that form, but who knows what we do use there. IIUC the compiler flags are hidden in XML files.)

Finally -- is python/cpython#96739 the only fix needed to get rid of the UB?

[matthiasgoergens]

I'm definitely open to changing the formulation. What I wrote was just a quick first draft to get the conversation started.

Given that even you had trouble understanding, I'd say there's definitely room for improving the wording!

I wrote 'tacitly', because from looking through the history and issues it seemed always like -fwrapv was just intended as a stopgap until the code could be fixed, and was never meant as a conscious decision to write in non-standard C. But that's just my interpretation.

People's prompt reactions when me and others pointed out instances of normally-undefined behaviour (that are only defined thanks to -fwrapv) seem to strengthen the case that this was never a conscious choice.

IIUC the compiler flags are hidden in XML files.

Interesting! Where are those XML files? None of the XML files in the main repository seem applicable. https://github.com/python/buildmaster-config/blob/8484356e8a0854084e186d9c9eb47b3b0c6eb628/master/custom/factories.py#L311-L315 has some config, but it's not in XML. All the other relevant config I could find seems to live in places like configure.ac and Makefile.pre.in and so on.

Finally -- is python/cpython#96739 the only fix needed to get rid of the UB?

I'm running some experiments to see what undefined behaviours I can round up.

There's eg this null-pointer arithmetic at https://github.com/python/cpython/blob/0f2b469ce1a6f123ad9e151b1771651b3e1d2de6/Modules/_testcapimodule.c#L4922 as demonstrated by the first commit in python/cpython#96915

There's definitely still some UB lurking in audioop.c:

0:00:01 load avg: 4.11 [  8/437/1] test_sunau crashed (Exit code 1)
/home/matthias/prog/python/cpythons/strict_overflow/Modules/audioop.c:1561:43: runtime error: left shift of negative value -24
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/matthias/prog/python/cpythons/strict_overflow/Modules/audioop.c:1561:43 in

And there might be some more elsewhere.

I had also assumed that the "dialect" was more about specifying the version of the C standard used and which optional parts thereof. That was probably naive.

That's definitely the most important part of that section. Whether you want to use the word 'dialect' or some other term to describe variants like -fwrapv or -fno-delete-null-pointer-checks (to take an example from the Linux kernel) is just a matter of taste and definition.

Given that the term 'dialect' is already used for K&R C vs C89 vs C11 etc, it might be useful to settle on a different term for the impact these flags have?

Apropos the Linux kernel: they made the conscious choice to pick lots of compiler flags that make their effective dialect of C into a much more defined language.

Going down that route is definitely worth considering and discussing. Nobody is forcing us to put up with all the UB of the C standard. We can deliberately choose to write in a more defined variant of C, too. (As long as all the compilers we care about support the necessary flags.)

[matthiasgoergens]

P.S. I took my use of the word dialect from here:

We have the absurd situation that C, specifically constructed to write the UNIX kernel, cannot be used to write operating systems. In fact, Linux and other operating systems are written in an unstable dialect of C that is produced by using a number of special flags that turn off compiler transformations based on undefined behavior (with no guarantees about future “optimizations”). The Postgres database also needs some of these flags as does the libsodium encryption library and even the machine learning tensor-flow package.

https://www.yodaiken.com/2021/05/19/undefined-behavior-in-c-is-a-reading-error/

[gvanrossum]

You gotta find someone else to argue, sorry.

[brettcannon]

I'm not seeing the benefit of this PR's change. All it's saying is CPython was compiled with a certain flag up to a certain point. But PEP 7 is not about how CPython is compiled but how to write C code within the project. Without specific guidance, e.g. "all arithmetic operations that may overflow are expected to ...," I'm not seeing what use core developers are supposed to derive from this to guide their coding.

[brettcannon]

The change doesn't present concrete guidelines on what core devs should do, just saying a certain compiler flag for some compilers has typically been used. More concrete guidelines of how code should be written is necessary.

[brettcannon]

Since no updates have been made based on the feedback provided, I'm closing this PR.