Fixed bug 3468 - _allshr in SDL_stdlib.c is not working properly

Mark Pizzolato

On Windows with Visual Studio, when building SDL as a static library using the x86 (32bit) mode, several intrinsic operations are implemented in code in SDL_stdlib.c.

One of these, _allshr() is not properly implemented and fails for some input.  As a result, some operations on 64bit data elements (long long) don't always work.

I classified this bug as a blocker since things absolutely don't work when the affected code is invoked.  The affected code is only invoked when SDL is compiled in x86 mode on Visual Studio when building a SDL as a static library.  This build environment isn't common, and hence the bug hasn't been noticed previously.

I reopened #2537 and mentioned this problem and provided a fix.  That fix is provided again here along with test code which could be added to some of the SDL test code.  This test code verifies that the x86 intrinsic routines produce the same results as the native x64 instructions which these routines emulate under the Microsoft compiler.  The point of the tests is to make sure that Visual Studio x86 code produces the same results as Visual Studio x64 code.  Some of the arguments (or boundary conditions) may produce different results on other compiler environments, so the tests really shouldn't be run on all compilers.  The test driver only actually exercised code when the compiler defines _MSC_VER, so the driver can generically be invoked without issue.

src/stdlib/SDL_stdlib.c

@@ -375,35 +375,35 @@ _ftol2_sse()
     _ftol();
 }
 
-/* 64-bit math operators for 32-bit systems */
-void
-__declspec(naked)
-_allmul()
-{
-    /* *INDENT-OFF* */
-    __asm {
-        mov         eax, dword ptr[esp+8]
-        mov         ecx, dword ptr[esp+10h]
-        or          ecx, eax
-        mov         ecx, dword ptr[esp+0Ch]
-        jne         hard
-        mov         eax, dword ptr[esp+4]
-        mul         ecx
-        ret         10h
-hard:
-        push        ebx
-        mul         ecx
-        mov         ebx, eax
-        mov         eax, dword ptr[esp+8]
-        mul         dword ptr[esp+14h]
-        add         ebx, eax
-        mov         eax, dword ptr[esp+8]
-        mul         ecx
-        add         edx, ebx
-        pop         ebx
-        ret         10h
-    }
-    /* *INDENT-ON* */
+/* 64-bit math operators for 32-bit systems */
+void
+__declspec(naked)
+_allmul()
+{
+    /* *INDENT-OFF* */
+    __asm {
+        mov         eax, dword ptr[esp+8]
+        mov         ecx, dword ptr[esp+10h]
+        or          ecx, eax
+        mov         ecx, dword ptr[esp+0Ch]
+        jne         hard
+        mov         eax, dword ptr[esp+4]
+        mul         ecx
+        ret         10h
+hard:
+        push        ebx
+        mul         ecx
+        mov         ebx, eax
+        mov         eax, dword ptr[esp+8]
+        mul         dword ptr[esp+14h]
+        add         ebx, eax
+        mov         eax, dword ptr[esp+8]
+        mul         ecx
+        add         edx, ebx
+        pop         ebx
+        ret         10h
+    }
+    /* *INDENT-ON* */
 }
 
 void
@@ -914,8 +914,8 @@ _allshr()
 {
     /* *INDENT-OFF* */
     __asm {
-        cmp         cl,40h
-        jae         RETZERO
+        cmp         cl,3Fh
+        jae         RETSIGN
         cmp         cl,20h
         jae         MORE32
         shrd        eax,edx,cl
@@ -923,13 +923,13 @@ _allshr()
         ret
 MORE32:
         mov         eax,edx
-        xor         edx,edx
+        sar         edx,1Fh
         and         cl,1Fh
         sar         eax,cl
         ret
-RETZERO:
-        xor         eax,eax
-        xor         edx,edx
+RETSIGN:
+        sar         edx,1Fh
+        mov         eax,edx
         ret
     }
     /* *INDENT-ON* */