Linux 5.15's New "-Werror" Behavior Is Causing A Lot Of Pain

[oiaohm]

This is from the paper that you referenced: ( http://www.open-std.org/jtc1/sc22/wg...docs/n1570.pdf )

Under 6.3.1 "Arithmetic operands":
"—The rank of any unsigned integer type shall equal the rank of the corresponding signed integer type, if any."

Does this not mean that the conversion ranks of uint64_t and int64_t are equal?

Under 6.5.9 "Equality operators":
"If both of the operands have arithmetic type, the usual arithmetic conversions are performed."

And under 6.3.1.8 "Usual arithmetic conversions":
"Otherwise, if the operand that has unsigned integer type has rank greater or
equal to the rank of the type of the other operand, then the operand with
signed integer type is converted to the type of the operand with unsigned
integer type."

Does the combination of these definitions not mean that the typecast in the code section in my previous post is optional, in terms of defined behavior?

indepe equal is cursed in rank system. First problem of cursed you find in older C compilers with ranking boost system.

You can have the right to left and left to right ranking boost in case of equal. Yes both valid under the C Standard to use because this was not defined as invalid to use this is true undefined behaviour. This is what makes reading the standard you also have to be reading between the lines looking for the items that should have been defined that were not.

So signed and unsigned being equal in rank end up go back to order of operation ranking in some cases.

Signed==unsigned stuff gets fun is the compiler a left to right or a right to left ranking boost or no boost. Left to right boost Signed here is now higher than unsigned yes right to left is unsigned is now higher than signed. All three options are valid under the C standard.

This cursed here is not absolutely forbin by C11 or even newer versions of the C standard. Rank boost should be forbin.

Now we into the bits of evil written into the C standard itself.

No two signed integer types shall have the same rank, even if they have the same representation.

and then

The rank of any unsigned integer type shall equal the rank of the corresponding signed integer type, if any.

The bad word in the second bit is corresponding.

So you can have a ranking table that looks like the following.

Greatest rank at top least the bottom with the rank number next to them with the higher number being higher rank
3 unsigned int
3 signed int
2 unsigned
2 signed
1 int

Yes you would have been hoping for
4 unsigned
4 unsigned int
3 int
2 signed
1 signed int.
Both are technically lists correct to standard. Yes first list allows some really creative screw ups.

Yes is not the corresponding type to unsigned int, signed int and unsigned is signed right that is what is leading to the first wacky list generation..

Yes the two signed integers types being forbid from having the same rank and that the unsigned has to have equal or greater rank to its signed equal is what can generate the first rank list stupidity when equal is used instead of greater for the unsigned.

Lets get into the third bit cursed or the second bit of cursed written into the standard C.

These that you used are type defines not in fact covered by the C standard ranking clearly so the compiler could have shoved them into a stupid location that can be dependant on what order they were declared in the header files. Wrong placement int64_t could have end up with a higher rank than uint64_t. How do you declare in C that uint64_t is the corresponding type to int64_t that right you don't. Yes the C standard does not clearly define how typedef are in fact added to the rank system so welcome to another bit of undefined behaviour where the compiler is valid todo anything to you. Yes the fact you used uint64_t int64_t brings fun as these are technically extended integer type.

The rank of any standard integer type shall be greater than the rank of any extended integer type with the same width.

Yes this is the complete total of guidance for how extended integer types glue into the rank system in the complete C standard .
So you have a unsigned int in gcc that 32 bit and you have uint32_t and you do the following.

signed int si = -1;
uint32_t ui = ~0;
if(si == ui)
if(si != ui)

So without the cast here by standard everything is getting covered to signed int who cares about the splat because unit32_t is lower rank because its a extended integer type. Remember its perfectly valid for the compiler at this point to refuse to build as well. Yes if the cast to unsigned that I added in the past had been there the person changing unsigned int to uint32_t would not have resulted in code break by standard.

Rank system for type conversion in C is cursed. Seams fine until you attempt to follow the rules and generate stupidity of course sometimes compiler developers do this as the following blog demos.

https://blog.regehr.org/archives/268

Yes as noted here there are gcc out modified by different parties that instead correctly transforming the signed value to unsigned it will transform to unsigned to signed if there is no cast. Yes this is a pure problem ubuntu screwed up the rank in that blog but other parties have done it.

The reality is do not trust C rank system for type conversion to handle signed conversion correctly ever. Its not written into the C standard correctly to start off with to make sure that signed conversion is always right. There are so many ways where the conversion can go the wrong way of unsigned to signed its not funny and be perfectly valid to C standard.

[indepe]

Remember that signed is meant to have a higher conversion rank by the standard so to solve by the standard without developer guidance ui is to be converted to signed so perform undefined behaviour .

This is from the paper that you referenced: ( http://www.open-std.org/jtc1/sc22/wg...docs/n1570.pdf )

Under 6.3.1 "Arithmetic operands":
"—The rank of any unsigned integer type shall equal the rank of the corresponding signed integer type, if any."

Does this not mean that the conversion ranks of uint64_t and int64_t are equal?


Under 6.5.9 "Equality operators":
"If both of the operands have arithmetic type, the usual arithmetic conversions are performed."

And under 6.3.1.8 "Usual arithmetic conversions":
"Otherwise, if the operand that has unsigned integer type has rank greater or
equal to the rank of the type of the other operand, then the operand with
signed integer type is converted to the type of the operand with unsigned
integer type."

Does the combination of these definitions not mean that the typecast in the code section in my previous post is optional, in terms of defined behavior?

[indepe]

I see where you want with this, but it is the only expectable outcome of comparing two registers with equal bit patterns. I just think it's good that C agrees with reality, for once, so that it's safe to know how the machine works.

It is only possibly expected *if* you are aware that the two operands are mixed signed and unsigned.

The problem is that unless you wrote the code yourself 5 minutes ago, you often are not aware of that, and then it is very unexpected if (-1) compares equal to a positive number.

So the reality is not the legalese that you are talking about, but the fact that the appearance of the source code hides the actual behavior. Unless you use a typecast.

[oiaohm]

Thanks for the nice resource, but I fail to see that I have a problem with that rule. For same-sized comparison between unsigned and signed, I see these rules (on that page):

• The rank of any unsigned integer type shall equal the rank of the corresponding signed integer type, if any.
• If the operand that has unsigned integer type has rank greater than or equal to the rank of the type of the other operand, the operand with signed integer type is converted to the type of the operand with unsigned integer type.
• That modulo rule for converting to unsigned: «Otherwise, if the new type is unsigned, the value is converted by repeatedly adding or subtracting one more than the maximum value that can be represented in the new type until the value is in the range of the new type.»

In other words, these are all unsigned comparisons with standard-defined outcomes (because the types have equal rank, unsigned takes precedence, and modulo value semantics guarrantees that -1 becomes ~0U):

This is where you need to read the C Standard it self not just run on the overview.

http://www.open-std.org/jtc1/sc22/wg...docs/n1570.pdf

[FONT=serif]6.3.1.3 Signed and unsigned integers[/FONT]
1 When a value with integer type is converted to another integer type other than_Bool,ifthe value can be represented by the new type, it is unchanged.
2 Otherwise, if the new type is unsigned, the value is converted by repeatedly adding or subtracting one more than the maximum value that can be represented in the new type until the value is in the range of the new type.60)
3 Otherwise, the new type is signed and the value cannot be represented in it; either the result is implementation-defined or an implementation-defined signal is raised.

Problem is option 3 and the ranks.

signed int si = -1;
unsigned int ui = ~0;

if(si == ui) // does what it looks like
if(si != ui) // does what it looks like
if(si < ui) // doesn't do what it looks like

This is where I say that the two (in)equality comparisons are fine, because the defined behavior coincides with what it looks like. And when the code is guarranteed to do what it looks like, -Wsign-compare should shut up.

So -Wsign-compare is right to complain about all locations the issue is point 3 of the standard. Now you are presume on the compare signed si is converted unsigned. But if the unsigned ui is converted to signed in the compare you are defined undefined behavour. This is a case you are missing casts. si should be cast to unsigned so that ui unsigned will not be converted by the compiler to signed and go into defined undefined behaviour.

Remember that signed is meant to have a higher conversion rank by the standard so to solve by the standard without developer guidance ui is to be converted to signed so perform undefined behaviour .

signed int si = -1;
unsigned int ui = ~0;

if((unsigned int)si == ui) // does what it looks like
if((unsigned int)si != ui) // does what it looks like

Once you do that -Wsign-compare will stop complaining and

Like it or not -Wsign-compare is right you are wrong. With the case and with the rank rules the conversion is meant to be the other way if you exactly follow the C standard and then the C standard says to the compiler for unsigned to signed the compiler can do what ever it likes including refuse to build the code. So yes -Werror + -Wsign-compare is 100 percent C11 and C89 standard conforming that the code will not build(implementation-defined signal for you).

PS please also note signed to unsigned compare even done right where -Wsign-compare will not raise warning can be a performance problem on platforms where signed is not most significant bit. The modulo rule is designed to suite most significant bit signed flags. So comparing or converting between signed and unsigned integer values should be avoided where ever coder can.

The reality here the C standard says perform a subtraction and that is what platforms that signed is not the most significant bit platforms can end up doing the most significant bit platforms just straight up compare the two registers/memory locations instead of subtraction conversion. This is quite a bit of performance difference. So code example you gave is wrong from a standard point of view and a performance point of view(when dealing with cross platform).

Mixing signed and unsigned need to be avoided with C its a problem child.

[andreano]

are you saying that if a comparison of (-1) and a positive number says that they are equal, it does what it looks like?

I see where you want with this, but it is the only expectable outcome of comparing two registers with equal bit patterns. I just think it's good that C agrees with reality, for once, so that it's safe to know how the machine works.

[indepe]

For me, -Werror as a developer is a no-brainer; Warnings are simply errors where the compiler attempts to correct the error automatically, and tells the programmer it tried to do so. Here is the thing with error-correcting software; in order to correct an error that you are reasonably sure *is* an error, you first need to assume missing data and then assume the intent of the programmer. See DWIM if you want to know a worst case example of what that can lead to. Ignore at your own peril.

Yes, a no-brainer, except perhaps for experimental code where you are just trying out something.

That said, yes, there are more strict compilers out there, so for shipped code -Werror should be off.

The Linux kernel isn't adequately described as "shipped code". Most users don't build the kernel themselves, it is usually shipped as part of a distribution, pre-built and along with a specific toolchain and compiler that comes along with that distribution. And that is usually (I hope) a compiler that has been tested extensively with that kernel.

For those who go through the trouble of building the kernel, especially if they do so with a different and unusually strict compiler, I think it is acceptable if they need an extra step to override that setting when necessary. As long as that is easy enough, and they have enough time to prepare for this situation

Also, the kernel needs to adhere to higher standards than "shipped code" in general.

[indepe]

signed int si = -1;
unsigned int ui = ~0;

if(si == ui) // does what it looks like
if(si != ui) // does what it looks like
if(si < ui) // doesn't do what it looks like

This is where I say that the two (in)equality comparisons are fine, because the defined behavior coincides with what it looks like. And when the code is guarranteed to do what it looks like, -Wsign-compare should shut up.

Maybe if the type definitions are right above that comparison, and you see them right there. However are you saying that in general, if a comparison of (-1) and a positive number says that they are equal, it does what it looks like?

I strongly disagree and think that behavior should not be left implict, that it is important to make it explicit by using a typecast, for readability if nothing else, especially in a code base like the kernel which is shared by thousands of engineers. And that typecast will resolve the warning, which is a really good warning.

[wertigon]

For me, -Werror as a developer is a no-brainer; Warnings are simply errors where the compiler attempts to correct the error automatically, and tells the programmer it tried to do so. Here is the thing with error-correcting software; in order to correct an error that you are reasonably sure *is* an error, you first need to assume missing data and then assume the intent of the programmer. See DWIM if you want to know a worst case example of what that can lead to. Ignore at your own peril.

That said, yes, there are more strict compilers out there, so for shipped code -Werror should be off.

[pal666]

Enabling -Werror by default is guaranteed to break the build for some users down the road, for no fault of theirs other than using a different compiler (version) or build configuration than the code was tested against by developers / automated testing.

if those users have non-tested by upsream config or compiler, then it's either their fault of not using distro kernel, or their job to fix kernel for their setup

[andreano]

https://wiki.sei.cmu.edu/confluence/...nversion+rules

> The rank of any extended signed integer type relative to another extended signed integer type with the same precision is implementation-defined but still subject to the other rules for determining the integer conversion rank.

Thanks for the nice resource, but I fail to see that I have a problem with that rule. For same-sized comparison between unsigned and signed, I see these rules (on that page):

• The rank of any unsigned integer type shall equal the rank of the corresponding signed integer type, if any.
• If the operand that has unsigned integer type has rank greater than or equal to the rank of the type of the other operand, the operand with signed integer type is converted to the type of the operand with unsigned integer type.
• That modulo rule for converting to unsigned: «Otherwise, if the new type is unsigned, the value is converted by repeatedly adding or subtracting one more than the maximum value that can be represented in the new type until the value is in the range of the new type.»

In other words, these are all unsigned comparisons with standard-defined outcomes (because the types have equal rank, unsigned takes precedence, and modulo value semantics guarrantees that -1 becomes ~0U):

signed int si = -1;
unsigned int ui = ~0;

if(si == ui) // does what it looks like
if(si != ui) // does what it looks like
if(si < ui) // doesn't do what it looks like

This is where I say that the two (in)equality comparisons are fine, because the defined behavior coincides with what it looks like. And when the code is guarranteed to do what it looks like, -Wsign-compare should shut up.

Of course, that inequality comparison is defined the wrong way, and had the signed type had highest rank, this wouldn't even be defined. So yes, -Wsign-compare is of course legitimate in a lot of cases.