shithub: choc

Download patch

ref: ef256921f31cfe2a3a8452fe30a3c0d96b416347
parent: a9a3ca3c0d8cea7fe24724a8f9bc17aa3c367677
author: Simon Howard <fraggle@soulsphere.org>
date: Fri Jul 10 23:38:49 EDT 2015

doom: Avoid overflow for spawn angle calculation.

Integer overflow is undefined and this breaks when using Clang with
-O2 optimization turned on. This fixes #572 (thanks to David Majnemer
for insight into fixing this bug).

--- a/src/doom/g_game.c
+++ b/src/doom/g_game.c
@@ -1168,26 +1168,26 @@
         fixed_t xa, ya;
         signed int an;
 
-        an = (ANG45 * ((signed int) mthing->angle / 45));
-        // Right-shifting a negative signed integer is implementation-defined,
-        // so divide instead.
-        an /= 1 << ANGLETOFINESHIFT;
+        // This calculation overflows in Vanilla Doom, but here we deliberately
+        // avoid integer overflow as it is undefined behavior, so the value of
+        // 'an' will always be positive.
+        an = (ANG45 >> ANGLETOFINESHIFT) * ((signed int) mthing->angle / 45);
 
         switch (an)
         {
-            case -4096:
+            case 4096:  // -4096:
                 xa = finetangent[2048];    // finecosine[-4096]
                 ya = finetangent[0];       // finesine[-4096]
                 break;
-            case -3072:
+            case 5120:  // -3072:
                 xa = finetangent[3072];    // finecosine[-3072]
                 ya = finetangent[1024];    // finesine[-3072]
                 break;
-            case -2048:
+            case 6144:  // -2048:
                 xa = finesine[0];          // finecosine[-2048]
                 ya = finetangent[2048];    // finesine[-2048]
                 break;
-            case -1024:
+            case 7168:  // -1024:
                 xa = finesine[1024];       // finecosine[-1024]
                 ya = finetangent[3072];    // finesine[-1024]
                 break;
@@ -1195,7 +1195,6 @@
             case 1024:
             case 2048:
             case 3072:
-            case 4096:
                 xa = finecosine[an];
                 ya = finesine[an];
                 break;