[cfe-users] -Wconversion does not produce warnings when it should
Hi,
consider the following code:
uint16_t testa4(uint8_t x, uint8_t y) {
return ( (uint16_t)x << 0 )
| ( (uint16_t)y << 8 );
}
uint16_t testb3(uint16_t x, uint16_t y) {
return x|y;
}
uint16_t testb4(uint16_t x, uint16_t y) {
return x+y;
}
int testb4x(uint16_t x, uint16_t y) {
return x+y;
}
I'm using clang 11.0.0 on Intel x86_64 and when I compile the above code
with -Wconversion enabled, then I see only one warning, namely for testa4.
However, according to the C standard, the expression after the return
statement has type int in all 4 cases. So why is the warning not
triggered for testb3 and testb4?
In fact, it's convenient that testb3 does not trigger a warning. Yes, x
and y are promoted to int before the bitwise or is performed, but the
result indeed fits into an uint16_t, regardless of x and y. So if clang
is smart and keeps track of the range of the expressions, then it is
understandable that there is no warning for testb3. However, then the
"keeping track of ranges" feature seems to fail for testa4, because
there a warning is indeed produced even though the two operands of the
bitwise-or are both uint16_t.
However, the case of testb4 is different. If we set x an y to 0x,
then the result will be 0x1FFFE. You can confirm that this is the case
by running testb4x. Then, shouldn't testb4 trigger a warning? The result
is clearly and int _and_ the result may not fit into an uint16_t.
I'm lost. As far as I understand, -Wconversion should issue a warning
when an int is converted to uint16_t without an explicit cast. For all I
know, x+y and x|y are ints, yet there is no warning in testb3 and
testb4. This seems like a bug to me.
Can you elaborate what is happening here?
Kind Regards,
Sven
___
cfe-users mailing list
cfe-users@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users
Re: [cfe-users] -Wconversion does not produce warnings when it should
Hi, > even though the two operands of the bitwise-or are both uint16_t. That's not true. I had many more versions of the test code before I posted here and I got confused. In testa4, the operands of the bitwise-or are both ints due to the bitshift which performs and integer promotion. Kind Regards, Sven ___ cfe-users mailing list cfe-users@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users
Re: [cfe-users] -Wconversion does not produce warnings when it should
On Mon, Nov 16, 2020 at 12:36 PM Sven Köhler via cfe-users
wrote:
>
> Hi,
>
> consider the following code:
>
> uint16_t testa4(uint8_t x, uint8_t y) {
> return ( (uint16_t)x << 0 )
> | ( (uint16_t)y << 8 );
> }
> uint16_t testb3(uint16_t x, uint16_t y) {
> return x|y;
> }
> uint16_t testb4(uint16_t x, uint16_t y) {
> return x+y;
> }
> int testb4x(uint16_t x, uint16_t y) {
> return x+y;
> }
>
>
>
> I'm using clang 11.0.0 on Intel x86_64 and when I compile the above code
> with -Wconversion enabled, then I see only one warning, namely for testa4.
>
> However, according to the C standard, the expression after the return
> statement has type int in all 4 cases. So why is the warning not
> triggered for testb3 and testb4?
>
> In fact, it's convenient that testb3 does not trigger a warning. Yes, x
> and y are promoted to int before the bitwise or is performed, but the
> result indeed fits into an uint16_t, regardless of x and y. So if clang
> is smart and keeps track of the range of the expressions, then it is
> understandable that there is no warning for testb3. However, then the
> "keeping track of ranges" feature seems to fail for testa4, because
> there a warning is indeed produced even though the two operands of the
> bitwise-or are both uint16_t.
>
> However, the case of testb4 is different. If we set x an y to 0x,
> then the result will be 0x1FFFE. You can confirm that this is the case
> by running testb4x. Then, shouldn't testb4 trigger a warning? The result
> is clearly and int _and_ the result may not fit into an uint16_t.
>
>
> I'm lost. As far as I understand, -Wconversion should issue a warning
> when an int is converted to uint16_t without an explicit cast. For all I
> know, x+y and x|y are ints, yet there is no warning in testb3 and
> testb4. This seems like a bug to me.
>
>
> Can you elaborate what is happening here?
I believe/would guess the goal is to reduce false positives (where a
false positive is defined as "warning on code where the code does what
the author intended").
In the taste of testb4 this code is "as good as" the unsigned int
equivalent, say (unigned x(unsigned a, unsigned b) { return a + b; })
- wraparound occurs in both cases, so the code perhaps isn't too
surprising/the semantics of widening/narrowing conversions don't
really impact the behavior. The uint16 version of the code and the
unsigned int version of the code would appear to the user to work in
the same way.
But, yeah, once the expressions are sufficiently complicated, as in
testa4, clang's attempt to silence benign cases of the warning gives
up and you get warnings. It's best-effort sort of stuff.
___
cfe-users mailing list
cfe-users@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users
Re: [cfe-users] -Wconversion does not produce warnings when it should
Am 16.11.20 um 21:49 schrieb David Blaikie via cfe-users:
> On Mon, Nov 16, 2020 at 12:36 PM Sven Köhler via cfe-users
> wrote:
>>
>> Can you elaborate what is happening here?
>
> I believe/would guess the goal is to reduce false positives (where a
> false positive is defined as "warning on code where the code does what
> the author intended").
>
> In the taste of testb4 this code is "as good as" the unsigned int
> equivalent, say (unigned x(unsigned a, unsigned b) { return a + b; })
> - wraparound occurs in both cases, so the code perhaps isn't too
> surprising/the semantics of widening/narrowing conversions don't
> really impact the behavior.
I do appreciate your explanation. But I do not appreciate the compiler
guessing that wrap around is OK.
The C standard could also be interpreted as "wrap around only occurs
with types of rank equal to (unsigned) int and above" as all other types
are promoted to int prior to any arithmetic.
(My guess for the origin of this promotion would be that int is the
natural word size of the underlying CPU.)
With most modern architectures, int is 32 bit which is a reasonable size
for wraparounds. Unfortunately, int doesn't always have a fixed size.
> The uint16 version of the code and the
> unsigned int version of the code would appear to the user to work in
> the same way.
I would appreciate that only if my understanding of the C language was
that arithmetic on two uint16_t operands is performed with uint16_t
precision. But that is simply false. It is performed with 32 bit
precision (if int is 32 bit).
> But, yeah, once the expressions are sufficiently complicated, as in
> testa4, clang's attempt to silence benign cases of the warning gives
> up and you get warnings. It's best-effort sort of stuff.
I would vote for -Wconversion=strict or -Wimplicit-int-conversion=strict
where we get a warning for every int conversion that loses precision.
I would be very happy if there was a clang option to generate warnings
in such cases.
> ___
> cfe-users mailing list
> cfe-users@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users
>
___
cfe-users mailing list
cfe-users@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users
Re: [cfe-users] -Wconversion does not produce warnings when it should
On Mon, Nov 16, 2020 at 4:49 PM Sven Köhler via cfe-users
wrote:
>
> Am 16.11.20 um 21:49 schrieb David Blaikie via cfe-users:
> > On Mon, Nov 16, 2020 at 12:36 PM Sven Köhler via cfe-users
> > wrote:
> >>
> >> Can you elaborate what is happening here?
> >
> > I believe/would guess the goal is to reduce false positives (where a
> > false positive is defined as "warning on code where the code does what
> > the author intended").
> >
> > In the taste of testb4 this code is "as good as" the unsigned int
> > equivalent, say (unigned x(unsigned a, unsigned b) { return a + b; })
> > - wraparound occurs in both cases, so the code perhaps isn't too
> > surprising/the semantics of widening/narrowing conversions don't
> > really impact the behavior.
>
> I do appreciate your explanation. But I do not appreciate the compiler
> guessing that wrap around is OK.
>
> The C standard could also be interpreted as "wrap around only occurs
> with types of rank equal to (unsigned) int and above" as all other types
> are promoted to int prior to any arithmetic.
> (My guess for the origin of this promotion would be that int is the
> natural word size of the underlying CPU.)
>
> With most modern architectures, int is 32 bit which is a reasonable size
> for wraparounds. Unfortunately, int doesn't always have a fixed size.
>
> > The uint16 version of the code and the
> > unsigned int version of the code would appear to the user to work in
> > the same way.
>
> I would appreciate that only if my understanding of the C language was
> that arithmetic on two uint16_t operands is performed with uint16_t
> precision. But that is simply false. It is performed with 32 bit
> precision (if int is 32 bit).
>
> > But, yeah, once the expressions are sufficiently complicated, as in
> > testa4, clang's attempt to silence benign cases of the warning gives
> > up and you get warnings. It's best-effort sort of stuff.
>
> I would vote for -Wconversion=strict or -Wimplicit-int-conversion=strict
> where we get a warning for every int conversion that loses precision.
>
> I would be very happy if there was a clang option to generate warnings
> in such cases.
In general, Clang warnings are designed to have very low false
positive rates so they're more likely to be enabled by users even on
existing codebases (eg: where the work to cleanup existing violations
of the warning is fruitful in terms of fixing bugs rather than only
suppressing a warning where it doesn't indicate any bug was present) -
so it may not be especially desirable to have such a strict form of
the warning in clang. Not to say "certainly not", but that it might be
a bit trickier to motivate/justify.
Might be that something more like a clang-tidy check could be more
suited to your needs.
- Dave
___
cfe-users mailing list
cfe-users@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users
Re: [cfe-users] -Wconversion does not produce warnings when it should
> On Nov 16, 2020, at 6:42 PM, David Blaikie via cfe-users
> wrote:
>
> On Mon, Nov 16, 2020 at 4:49 PM Sven Köhler via cfe-users
> mailto:cfe-users@lists.llvm.org>> wrote:
>>
>> Am 16.11.20 um 21:49 schrieb David Blaikie via cfe-users:
>>> On Mon, Nov 16, 2020 at 12:36 PM Sven Köhler via cfe-users
>>> wrote:
Can you elaborate what is happening here?
>>>
>>> I believe/would guess the goal is to reduce false positives (where a
>>> false positive is defined as "warning on code where the code does what
>>> the author intended").
>>>
>>> In the taste of testb4 this code is "as good as" the unsigned int
>>> equivalent, say (unigned x(unsigned a, unsigned b) { return a + b; })
>>> - wraparound occurs in both cases, so the code perhaps isn't too
>>> surprising/the semantics of widening/narrowing conversions don't
>>> really impact the behavior.
>>
>> I do appreciate your explanation. But I do not appreciate the compiler
>> guessing that wrap around is OK.
>>
>> The C standard could also be interpreted as "wrap around only occurs
>> with types of rank equal to (unsigned) int and above" as all other types
>> are promoted to int prior to any arithmetic.
>> (My guess for the origin of this promotion would be that int is the
>> natural word size of the underlying CPU.)
>>
>> With most modern architectures, int is 32 bit which is a reasonable size
>> for wraparounds. Unfortunately, int doesn't always have a fixed size.
>>
>>> The uint16 version of the code and the
>>> unsigned int version of the code would appear to the user to work in
>>> the same way.
>>
>> I would appreciate that only if my understanding of the C language was
>> that arithmetic on two uint16_t operands is performed with uint16_t
>> precision. But that is simply false. It is performed with 32 bit
>> precision (if int is 32 bit).
>>
>>> But, yeah, once the expressions are sufficiently complicated, as in
>>> testa4, clang's attempt to silence benign cases of the warning gives
>>> up and you get warnings. It's best-effort sort of stuff.
>>
>> I would vote for -Wconversion=strict or -Wimplicit-int-conversion=strict
>> where we get a warning for every int conversion that loses precision.
>>
>> I would be very happy if there was a clang option to generate warnings
>> in such cases.
>
> In general, Clang warnings are designed to have very low false
> positive rates so they're more likely to be enabled by users even on
> existing codebases (eg: where the work to cleanup existing violations
> of the warning is fruitful in terms of fixing bugs rather than only
> suppressing a warning where it doesn't indicate any bug was present) -
> so it may not be especially desirable to have such a strict form of
> the warning in clang. Not to say "certainly not", but that it might be
> a bit trickier to motivate/justify.
>
> Might be that something more like a clang-tidy check could be more
> suited to your needs.
If it hasn’t already been mentioned in this thread, people should be made aware
of “Value Sanitizer”.
(Which, curiously, doesn’t seem to be in the clang docs)
Anyway, it checks (at runtime), every time a value is assigned from a larger
integral type to a smaller one.
If the value changes, then it raises an error.
So if you have an int with the value 23, and assign it to a char, nothing
happens.
So if you have an int with the value 523, and assign it to a char, you get an
error.
— Marshall
___
cfe-users mailing list
cfe-users@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-users
