Merge pull request #3493 from pygame-community/ankith26-sanitizers

Add sanitize flag to dev.py, fix UBSAN issues and run UBSAN check on CI

Author: ankith26

.github/workflows/run-ubuntu-checks.yml

@@ -101,7 +101,7 @@ jobs:
       id: build-pygame-ce
       run: |
         pyenv global ${{ matrix.python }}-debug
-        python dev.py build --lax --coverage
+        python dev.py build --lax --coverage --sanitize undefined
 
     - name: Run tests
       env:

dev.py

@@ -15,6 +15,8 @@
 from pathlib import Path
 from typing import Any, Union
 
+from buildconfig.get_version import version
+
 MOD_NAME = "pygame-ce"
 DIST_DIR = "dist"
 
@@ -204,9 +206,11 @@ def __init__(self) -> None:
 
     def cmd_build(self):
         wheel_dir = self.args.get("wheel", DIST_DIR)
+        quiet = self.args.get("quiet", False)
         debug = self.args.get("debug", False)
         lax = self.args.get("lax", False)
         sdl3 = self.args.get("sdl3", False)
+        sanitize = self.args.get("sanitize")
         coverage = self.args.get("coverage", False)
         if wheel_dir and coverage:
             pprint("Cannot pass --wheel and --coverage together", Colors.RED)
@@ -228,6 +232,8 @@ def cmd_build(self):
 
         if not wheel_dir:
             # editable install
+            if not quiet:
+                install_args.append("-Ceditable-verbose=true")
             install_args.append("--editable")
 
         install_args.append(".")
@@ -245,15 +251,19 @@ def cmd_build(self):
         if coverage:
             install_args.extend(COVERAGE_ARGS)
 
+        if sanitize:
+            install_args.append(f"-Csetup-args=-Db_sanitize={sanitize}")
+
         info_str = f"with {debug=}, {lax=}, {sdl3=}, and {coverage=}"
         if wheel_dir:
             pprint(f"Building wheel at '{wheel_dir}' ({info_str})")
             cmd_run(
                 [self.py, "-m", "pip", "wheel", "-v", "-w", wheel_dir, *install_args]
             )
             pprint("Installing wheel")
+            mod_name = f"{MOD_NAME}=={version}"
             pip_install(
-                self.py, ["--no-index", "--force", "--find-links", wheel_dir, MOD_NAME]
+                self.py, ["--no-index", "--force", "--find-links", wheel_dir, mod_name]
             )
         else:
             pprint(f"Installing in editable mode ({info_str})")
@@ -352,6 +362,11 @@ def parse_args(self):
                 f"wheel (if not passed, '{DIST_DIR}' is used)"
             ),
         )
+        build_parser.add_argument(
+            "--quiet",
+            action="store_true",
+            help="Silence build log in editable install (doing editable-verbose=false)",
+        )
         build_parser.add_argument(
             "--debug",
             action="store_true",
@@ -367,6 +382,20 @@ def parse_args(self):
             action="store_true",
             help="Build against SDL3 instead of the default SDL2",
         )
+        build_parser.add_argument(
+            "--sanitize",
+            choices=[
+                "address",
+                "undefined",
+                "address,undefined",
+                "leak",
+                "thread",
+                "memory",
+                "none",
+            ],
+            default="none",
+            help="Enable compiler sanitizers. Defaults to 'none'.",
+        )
         build_parser.add_argument(
             "--coverage",
             action="store_true",

src_c/SDL_gfx/SDL_gfxPrimitives.c

@@ -2782,7 +2782,7 @@ _aalineColor(SDL_Surface *dst, Sint16 x1, Sint16 y1, Sint16 x2, Sint16 y2,
         /*
          * Not-so-portable version: erradj = ((Uint64)dx << 32) / (Uint64)dy;
          */
-        erradj = ((dx << 16) / dy) << 16;
+        erradj = (Uint32)((dx << 16) / dy) << 16;
 
         /*
          * draw all pixels other than the first and last
@@ -2819,7 +2819,7 @@ _aalineColor(SDL_Surface *dst, Sint16 x1, Sint16 y1, Sint16 x2, Sint16 y2,
         /*
          * Not-so-portable version: erradj = ((Uint64)dy << 32) / (Uint64)dx;
          */
-        erradj = ((dy << 16) / dx) << 16;
+        erradj = (Uint32)((dy << 16) / dx) << 16;
 
         /*
          * draw all pixels other than the first and last

src_c/alphablit.c

@@ -2448,7 +2448,7 @@ alphablit_alpha(SDL_BlitInfo *info)
     int srcbpp = PG_FORMAT_BytesPerPixel(srcfmt);
     int dstbpp = PG_FORMAT_BytesPerPixel(dstfmt);
     Uint8 dR, dG, dB, dA, sR, sG, sB, sA;
-    int dRi, dGi, dBi, dAi, sRi, sGi, sBi, sAi;
+    Uint32 dRi, dGi, dBi, dAi, sRi, sGi, sBi, sAi;
     Uint32 modulateA = info->src_blanket_alpha;
     Uint32 pixel;
 

src_c/color.c

@@ -1737,7 +1737,7 @@ _color_inv(pgColorObject *color)
 static PyObject *
 _color_int(pgColorObject *color)
 {
-    Uint32 tmp = (color->data[0] << 24) + (color->data[1] << 16) +
+    Uint32 tmp = ((Uint32)color->data[0] << 24) + (color->data[1] << 16) +
                  (color->data[2] << 8) + color->data[3];
     return PyLong_FromUnsignedLong(tmp);
 }
@@ -1748,7 +1748,7 @@ _color_int(pgColorObject *color)
 static PyObject *
 _color_float(pgColorObject *color)
 {
-    Uint32 tmp = ((color->data[0] << 24) + (color->data[1] << 16) +
+    Uint32 tmp = (((Uint32)color->data[0] << 24) + (color->data[1] << 16) +
                   (color->data[2] << 8) + color->data[3]);
     return PyFloat_FromDouble((double)tmp);
 }

src_c/freetype/ft_wrap.h

@@ -41,8 +41,8 @@
 #define FX6_TRUNC(x) ((x) >> 6)
 #define FX16_CEIL_TO_FX6(x) (((x) + 1023L) >> 10)
 #define FX16_ROUND_TO_INT(x) (((x) + 32768L) >> 16)
-#define INT_TO_FX6(i) ((FT_Fixed)((i) << 6))
-#define INT_TO_FX16(i) ((FT_Fixed)((i) << 16))
+#define INT_TO_FX6(i) ((FT_Fixed)((unsigned long long)(i) << 6))
+#define INT_TO_FX16(i) ((FT_Fixed)((unsigned long long)(i) << 16))
 #define FX16_TO_DBL(x) ((x) * 1.52587890625e-5 /* 2.0^-16 */)
 #define DBL_TO_FX16(d) ((FT_Fixed)((d) * 65536.0))
 #define FX6_TO_DBL(x) ((x) * 1.5625e-2 /* 2.0^-6 */)

src_c/mask.c

@@ -763,7 +763,8 @@ set_pixel_color(Uint8 *pixel, Uint8 bpp, Uint32 color)
 
         case 3:
 #if SDL_BYTEORDER == SDL_LIL_ENDIAN
-            *(Uint16 *)pixel = color;
+            pixel[0] = color;
+            pixel[1] = color >> 8;
             pixel[2] = color >> 16;
 #else  /* != SDL_LIL_ENDIAN */
             pixel[2] = color;

src_c/pixelcopy.c

@@ -734,7 +734,7 @@ array_to_surface(PyObject *self, PyObject *arg)
             else {
                 Uint32 alpha = 0;
                 if (format->Amask) {
-                    alpha = 255 >> Aloss << Ashift;
+                    alpha = 255u >> Aloss << Ashift;
                 }
                 switch (view_p->itemsize) {
                     case sizeof(Uint8):

src_c/rotozoom.c

@@ -236,8 +236,8 @@ transformSurfaceRGBA(SDL_Surface *src, SDL_Surface *dst, int cx, int cy,
     /*
      * Variable setup
      */
-    xd = ((src->w - dst->w) << 15);
-    yd = ((src->h - dst->h) << 15);
+    xd = ((unsigned long long)(src->w - dst->w) << 15);
+    yd = ((unsigned long long)(src->h - dst->h) << 15);
     ax = (cx << 16) - (icos * cx);
     ay = (cy << 16) - (isin * cx);
     sw = src->w - 1;

src_c/surface.c

@@ -717,16 +717,16 @@ surface_init(pgSurfaceObject *self, PyObject *args, PyObject *kwds)
         if (flags & PGS_SRCALPHA) {
             switch (bpp) {
                 case 16:
-                    Rmask = 0xF << 8;
-                    Gmask = 0xF << 4;
-                    Bmask = 0xF;
-                    Amask = 0xF << 12;
+                    Rmask = 0xFu << 8;
+                    Gmask = 0xFu << 4;
+                    Bmask = 0xFu;
+                    Amask = 0xFu << 12;
                     break;
                 case 32:
-                    Rmask = 0xFF << 16;
-                    Gmask = 0xFF << 8;
-                    Bmask = 0xFF;
-                    Amask = 0xFF << 24;
+                    Rmask = 0xFFu << 16;
+                    Gmask = 0xFFu << 8;
+                    Bmask = 0xFFu;
+                    Amask = 0xFFu << 24;
                     break;
                 default:
                     PyErr_SetString(
@@ -793,20 +793,20 @@ surface_init(pgSurfaceObject *self, PyObject *args, PyObject *kwds)
         if (flags & PGS_SRCALPHA) {
             switch (bpp) {
                 case 16:
-                    Rmask = 0xF << 8;
-                    Gmask = 0xF << 4;
-                    Bmask = 0xF;
-                    Amask = 0xF << 12;
+                    Rmask = 0xFu << 8;
+                    Gmask = 0xFu << 4;
+                    Bmask = 0xFu;
+                    Amask = 0xFu << 12;
                     break;
                 case 24:
                     bpp = 32;
                     // we automatically step up to 32 if video is 24, fall
                     // through to case below
                 case 32:
-                    Rmask = 0xFF << 16;
-                    Gmask = 0xFF << 8;
-                    Bmask = 0xFF;
-                    Amask = 0xFF << 24;
+                    Rmask = 0xFFu << 16;
+                    Gmask = 0xFFu << 8;
+                    Bmask = 0xFFu;
+                    Amask = 0xFFu << 24;
                     break;
                 default:
                     PyErr_SetString(