On Thu, 8 Feb 2007, Ralf Baechle wrote:
> Well, some of the warnings are also simply due to broken code. This is
> the result of preprocessing the code without Yoichi's patch applied:
>
> [...]
> if (sp >= (long)0x80000000 && sp < (long)0xc0000000)
> usp = ((((int)(int)(sp)) & 0x1fffffff) | 0xa0000000);
> else if ((long long)sp >= (long long)(0x8000000000000000LL | ((0LL)<<59) | (0)) &&
> (long long)sp < (long long)(0x8000000000000000LL | ((8LL)<<59) | (0)))
> usp = (0x8000000000000000LL | (((long long)2)<<59) | ((((long long)sp) & -1)));
>
> else {
> do { __asm__ __volatile__("break %0" : : "i" (512)); } while (0);
> usp = sp;
> }
> [...]
>
> So (0x8000000000000000LL | ((0LL)<<59) | (0)) is 0x8000000000000000LL which
> then is casted to _signed_ long long, so becomes -9223372036854775808, the
> most negative representable number so the two "comparison is always true
> due to limited range of data type" warnings are perfectly correct.
They are neither correct nor expected. And the problem is not
0x8000000000000000LL being the most negative representable number, but
"sp" being a variable of a 32-bit type and being compared against the
constant. In this case GCC seems to completely ignore the cast to "long
long" and treats it as if the comparison was done between types of
different widths.
Try building this program:
$ cat range.c
int foo(sp_t sp)
{
if (sp >= (long)0x80000000 && sp < (long)0xc0000000)
return 0;
else if ((long long)sp >= (long long)0x8000000000000000LL &&
(long long)sp < (long long)0xc000000000000000LL)
return 1;
else
return 2;
}
$ mips64el-linux-gcc -O2 -Wall -Dsp_t=long -c range.c
$ mips64el-linux-gcc -O2 -Wall -Dsp_t=int -c range.c
range.c: In function 'foo':
range.c:3: warning: comparison is always false due to limited range of data type
range.c:3: warning: comparison is always true due to limited range of data type
range.c:5: warning: comparison is always true due to limited range of data type
range.c:6: warning: comparison is always false due to limited range of data type
$
Notice how GCC complains about both 0x8000000000000000LL and
0xc000000000000000LL.
I think there was a bug report associated with it -- let me see... Yep:
"http://gcc.gnu.org/bugzilla/show_bug.cgi?id=12963";.
> Treating addresses as signed is a dangerous thing and we reallly only
> should do it where extending 32-bit addresses to 64-bit because that's
> what the architecture does. So I would suggest as part of cleaning u the
> mess something like below totally untested patch.
I have used signed types here on purpose not to cross to the user segment
(positive range) accidentally. But I do not insist on keeping them if
they were to hurt somebody's eyes. Your change is not going to fix the
problem anyway -- if I change the condition in the program above to:
else if ((unsigned long long)sp >= (unsigned long long)0x8000000000000000ULL &&
(unsigned long long)sp < (unsigned long long)0xc000000000000000ULL)
then unfortunately the warnings persist (I am pretty sure I did this kind
of testing before committing these bits, to make sure the warning was
unavoidable).
Maciej
