lkml.org 
[lkml]   [2011]   [Jun]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
Subject[PATCH] m68k: Patch for broken lsl64() "old code" with incorrect shift
[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 <jelchison@gmail.com>

-----------------------------------------------------------------
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
jelchison@gmail.com


\
 
 \ /
  Last update: 2011-06-11 05:53    [W:0.045 / U:0.804 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site