shithub: choc

Info  •  Files  •  Log  •  Branches

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;