[1.] One line summary of the problem:
[PATCH] m68k: Patch for broken lsl64() "old code" with incorrect shift
[2.] Full description of the problem/report:
In arch/m68k/math-emu/multi_arith.h, lsl64() doesn't calculate a
correct HI_WORD(*dest) when count < 32, due to an incorrect shift
operation. This line:
HI_WORD(*dest) = (HI_WORD(*dest) << count) | (LO_WORD(*dest) >> count);
...should be...
HI_WORD(*dest) = (HI_WORD(*dest) << count) | (LO_WORD(*dest) >>
(32 - count));
This function is ifdef'd out, marked as "old code". However, I fell
into the same trap into which I suspect others have fallen (or will
fall): On rare occasion when I need to implement my own library
functions, I depend on the Linux kernel to provide solid code. This
bug, albeit in old code, led me astray.
I'm asking that this old code (lsl64) be corrected or removed.
[3.] Keywords (i.e., modules, networking, kernel):
kernel, arch, m68k, math-emu, multi_arith.h, lsl64
[4.] Kernel information
2.6.39.1
[5.] Most recent kernel version which did not have the bug:
Unknown
[6.] Output of Oops.. message (if applicable) with symbolic
information resolved (see Documentation/oops-tracing.txt)
N/A
[7.] A small shell script or example program which triggers the
problem (if possible)
-----------------------------------------------------------------
#include <stdio.h>
#define LO_WORD(ll) (((unsigned int *) &ll)[1])
#define HI_WORD(ll) (((unsigned int *) &ll)[0])
static inline void lsl64_broken(int count, unsigned long long *dest)
{
if (count < 32) {
HI_WORD(*dest) = (HI_WORD(*dest) << count)
| (LO_WORD(*dest) >> count);
LO_WORD(*dest) <<= count;
return;
}
count -= 32;
HI_WORD(*dest) = LO_WORD(*dest) << count;
LO_WORD(*dest) = 0;
}
static inline void lsl64_fixed(int count, unsigned long long *dest)
{
if (count < 32) {
HI_WORD(*dest) = (HI_WORD(*dest) << count)
| (LO_WORD(*dest) >> (32 - count));
LO_WORD(*dest) <<= count;
return;
}
count -= 32;
HI_WORD(*dest) = LO_WORD(*dest) << count;
LO_WORD(*dest) = 0;
}
static inline void lsr64(int count, unsigned long long *dest)
{
if (count < 32) {
LO_WORD(*dest) = (LO_WORD(*dest) >> count)
| (HI_WORD(*dest) << (32 - count));
HI_WORD(*dest) >>= count;
return;
}
count -= 32;
LO_WORD(*dest) = HI_WORD(*dest) >> count;
HI_WORD(*dest) = 0;
}
int main( void )
{
long long testVal = 0x0000100000001000L;
long long testVal_broken = testVal;
long long testVal_fixed = testVal;
printf( "begin testVal_broken =\t\t0x%016llx\n", testVal_broken );
lsr64( 1, &testVal_broken );
printf( "intermediate testVal_broken =\t0x%016llx\n", testVal_broken );
lsl64_broken( 1, &testVal_broken );
printf( "final testVal_broken =\t\t0x%016llx\n", testVal_broken );
printf( "final matches begin?\t\t%s\n", testVal_broken == testVal ?
"TRUE" : "FALSE" );
printf( "\nbegin testVal_fixed =\t\t0x%016llx\n", testVal_fixed );
lsr64( 1, &testVal_fixed );
printf( "intermediate testVal_fixed =\t0x%016llx\n", testVal_fixed );
lsl64_fixed( 1, &testVal_fixed );
printf( "final testVal_fixed =\t\t0x%016llx\n", testVal_fixed );
printf( "final matches begin?\t\t%s\n", testVal_fixed == testVal ?
"TRUE" : "FALSE" );
return 0;
}
-----------------------------------------------------------------
[8.] Environment
Found via code inspection
[9.] Patch
Signed-off-by: Jonathan Elchison <[email protected]>
-----------------------------------------------------------------
diff -up linux-2.6/arch/m68k/math-emu/multi_arith.h
linux-2.6_orig/arch/m68k/math-emu/multi_arith.h
--- linux-2.6/arch/m68k/math-emu/multi_arith.h 2011-06-10
22:50:32.538711320 -0400
+++ linux-2.6_orig/arch/m68k/math-emu/multi_arith.h 2011-06-10
22:47:43.407285452 -0400
@@ -236,7 +236,7 @@ static inline void lsl64(int count, unsi
{
if (count < 32) {
HI_WORD(*dest) = (HI_WORD(*dest) << count)
- | (LO_WORD(*dest) >> (32 - count));
+ | (LO_WORD(*dest) >> count);
LO_WORD(*dest) <<= count;
return;
}
-----------------------------------------------------------------
[10.] Responses
Please personally CC me on responses to this post.
--
Jonathan Elchison
[email protected]
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
