(In reply to comment #0)
> In tgsi_ureg.c:1498
> "if (ureg->vs_inputs[i/32] & (1 << (i%32))) {"
> when i==31 then 1 << 31 may be computed.
> With gcc option -std=c99 this is wrong because the result cannot be int.
> 
> Perhaps the right instruction is
> "if (ureg->vs_inputs[i/32] & ((unsigned) 1 << (i%32))) {"

Where is -std=c99 coming from? Also I'd guess the better thing is 1U instead of (unsigned)1 :)

Feel free to send a patch btw.

Yes I agree that 1U is better. My C knowledge is limited, I am more
interested into Fortran.

-std=c99 is in CFLAGS in the Makefile produced by configure and configure.ac.

Not much familiar with patch, please help yourself.

For the same reason I believe that in uniforms.c:79
"if (prog->SamplersUsed & (1 << s)) {" should be
"if (prog->SamplersUsed & (1U << s)) {".
This is running glxgears.

Also in matrix.c:1156 the line
"#define ONE(x)  (1 <<(x+16))"
should be
"#define ONE(x)  (1U<<(x+16))"
because ONE(15) later, as at line 1226, produces 16 bits left shift of 65536,
which is bigger than int.
This is running teapot.

I'm not sure why you're filing these against nouveau... I'm sure there are _tons_ of these throughout mesa...

Does gcc warn about this? Are you _sure_ that (signed int)1 << 31 is illegal in C99?

I'm sure that technically shifting any bits into the sign bit of a signed number produces undefined behavior according to C99, so yes, (1<<31) technically is undefined. As a matter of practicality, on basically any CPU, 1<<31 will produce a valid negative number, but the compiler sees it as trying to assign the constant 2147483648 to a 32-bit signed container, which is outside of the 2^31-1 range that it can accept.

In the C abstract machine, this would be a serious problem. Obviously you can make such an assignment in hardware, but the result (when interpreted as signed) is not a value of 2147483648, but a value of -2147483648, which is why the compiler gives a warning. It's weird to read that (1<<31) is less than 0. If you don't really mean to assign a value of -2147483648 or 2147483648 and just want a bit pattern, using unsigned is almost always the way to go.

I know that Solaris Studio happens to give a warning for the same case, if that means anything.

In m_matrix.c:1215
"   if (m[15] == 1.0F) mask |= (1<<31);"
should be
"   if (m[15] == 1.0F) mask |= (1U<<31);"
because 1<<31 does not fit in int.
From teapot.

(In reply to comment #5)
> I'm not sure why you're filing these against nouveau... I'm sure there are
> _tons_ of these throughout mesa...
> 
> Does gcc warn about this? Are you _sure_ that (signed int)1 << 31 is illegal
> in C99?
Only now I am reading this note.
I compiled mesa with gcc -fsanitize=undefined option, gcc runtime is issuing
several, not tons, of runtime error messages, it seems to me that 
(signed int) 1 << 31 is undefined if computed as int.
The runtime error messages only appear with the -std=c99 option.

(In reply to comment #6)
> I'm sure that technically shifting any bits into the sign bit of a signed
> number produces undefined behavior according to C99, so yes, (1<<31)
> technically is undefined. As a matter of practicality, on basically any CPU,
> 1<<31 will produce a valid negative number, but the compiler sees it as
> trying to assign the constant 2147483648 to a 32-bit signed container, which
> is outside of the 2^31-1 range that it can accept.
> 
> In the C abstract machine, this would be a serious problem. Obviously you
> can make such an assignment in hardware, but the result (when interpreted as
> signed) is not a value of 2147483648, but a value of -2147483648, which is
> why the compiler gives a warning. It's weird to read that (1<<31) is less
> than 0. If you don't really mean to assign a value of -2147483648 or
> 2147483648 and just want a bit pattern, using unsigned is almost always the
> way to go.
> 
> I know that Solaris Studio happens to give a warning for the same case, if
> that means anything.

Thanks for the appreciated explanation.
Maybe substituting 1<<31 with 1U<<31 clears any undefined behaviour on the many architectures where mesa runs.

(In reply to comment #5)
> Does gcc warn about this? Are you _sure_ that (signed int)1 << 31 is illegal
> in C99?

And as tedious as this is to mention, C99 section 6.5.7.4 does state this is undefined behavior. I know the "C abstract machine" is far removed from real problems with drivers and whatnot -- and I appreciate the awesome work you all do on Mesa. I just wanted to clarify that there is spec support for this claim.

Another one in nouveau_fence.c:170
"#define NOUVEAU_FENCE_MAX_SPINS (1 << 31)"
should be
"#define NOUVEAU_FENCE_MAX_SPINS (1U<< 31)"

In texstore.c lines 1281 and 1340 255 << 24 is computed in PACK_COLOR_8888.

More issues:
u_pack_color.h:365
"uc->ui = (a << 24) | (r << 16) | (g << 8) | b;"
should be
"uc->ui = ((unsigned int)a << 24) | (r << 16) | (g << 8) | b;"

m_matrix.c line 1156
"#define ONE(x)  (1<<(x+16))"
should be
"#define ONE(x)  (1U<<(x+16))"
because of ONE(15).
At line 1215
"if (m[15] == 1.0F) mask |= (1<<31);"
should be
"if (m[15] == 1.0F) mask |= (1U<<31);"

(In reply to comment #10)
> And as tedious as this is to mention, C99 section 6.5.7.4 does state this is
> undefined behavior. I know the "C abstract machine" is far removed from real
> problems with drivers and whatnot -- and I appreciate the awesome work you
> all do on Mesa. I just wanted to clarify that there is spec support for this
> claim.

LLVM compiler site has 3 article series on issues with undefined behavior (see URL field), it's a very good read.

Coccinelle might be able to help detection and patching of issues like this: http://lwn.net/Articles/380835/

Please submit patches fixing these issues to the mesa-dev mailing list, instead of waiting here for anyone else to do it.

If comment #15 is directed to me,
I believe I already did my part by exposing this issue on mesa bugzilla.
The C99 standard clearly states that such shifts are undefined and
then against the standard.
Mesa works on many platforms and it should be at least standard
compliant to ensure its correct execution.
Maybe this undefined behaviour provokes some glitches in mesa, maybe not.
I have not the expertise nor the time to submit patches.
Are you telling me nobody is going to fix these bugs?
If you are happy with that I am happy.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/mesa/mesa/issues/980.