fix various UB issues

* Fix "applying non-zero offset to NULL pointer". * Fix "non-aligned access to struct members" (maptexture_t).
fabiangreffrath · Jan 26, 2025 · ca89b73 · ca89b73
1 parent bdd5630
commit ca89b73
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 9 deletions.
diff --git a/src/m_array.h b/src/m_array.h
@@ -98,7 +98,7 @@ inline static void array_clear(const void *v)
         }                                 \
     } while (0)
 
-#define array_end(v) ((v) + array_size(v))
+#define array_end(v) ((v) ? (v) + array_ptr(v)->size : (v))
 
 #define array_foreach(ptr, v) \
     for (ptr = (v); ptr < array_end(v); ++ptr)

diff --git a/src/m_fixed.h b/src/m_fixed.h
@@ -58,10 +58,9 @@
 #define FRACUNIT (1<<FRACBITS)
 #define FIXED2DOUBLE(x) ((x)/(double)FRACUNIT)
 #define FRACMASK (FRACUNIT - 1)
-#define FRACFILL(x, o) ((x) | ((o) < 0 ? (FRACMASK << (32 - FRACBITS)) : 0))
 
 #define IntToFixed(x) ((x) << FRACBITS)
-#define FixedToInt(x) FRACFILL((x) >> FRACBITS, (x))
+#define FixedToInt(x) ((x) >> FRACBITS)
 
 typedef int fixed_t;
 

diff --git a/src/p_spec.c b/src/p_spec.c
@@ -151,7 +151,7 @@ void P_InitPicAnims (void)
   for (i=0 ; animdefs[i].istexture != -1 ; i++)
     {
       // 1/11/98 killough -- removed limit by array-doubling
-      if (lastanim >= anims + maxanims)
+      if (!anims || lastanim >= anims + maxanims)
         {
           size_t newmax = maxanims ? maxanims*2 : MAXANIMS;
           anims = Z_Realloc(anims, newmax*sizeof(*anims), PU_STATIC, 0);   // killough

diff --git a/src/r_data.c b/src/r_data.c
@@ -66,22 +66,26 @@
 // and possibly other attributes.
 //
 
-typedef struct
+#if defined(_MSC_VER)
+#pragma pack(push, 1)
+#endif
+
+typedef PACKED_PREFIX struct
 {
   short originx;
   short originy;
   short patch;
   short stepdir;         // unused in Doom but might be used in Phase 2 Boom
   short colormap;        // unused in Doom but might be used in Phase 2 Boom
-} mappatch_t;
+} PACKED_SUFFIX mappatch_t;
 
 
 //
 // Texture definition.
 // A DOOM wall texture is a list of patches
 // which are to be combined in a predefined order.
 //
-typedef struct
+typedef PACKED_PREFIX struct
 {
   char       name[8];
   int        masked;
@@ -90,8 +94,11 @@ typedef struct
   char       pad[4];       // unused in Doom but might be used in Boom Phase 2
   short      patchcount;
   mappatch_t patches[1];
-} maptexture_t;
+} PACKED_SUFFIX maptexture_t;
 
+#if defined(_MSC_VER)
+#pragma pack(pop)
+#endif
 
 // A single patch from a texture definition, basically
 // a rectangular area within the texture rectangle.

diff --git a/src/z_zone.c b/src/z_zone.c
@@ -96,11 +96,13 @@ void *Z_Malloc(size_t size, pu_tag tag, void **user)
 
 void Z_Free(void *p)
 {
-  memblock_t *block = (memblock_t *)((char *) p - HEADER_SIZE);
+  memblock_t *block;
 
   if (!p)
     return;
 
+  block = (memblock_t *)((char *) p - HEADER_SIZE);
+
   if (block->id != ZONEID)
     I_Error("Z_Free: freed a pointer without ZONEID");
   block->id = 0;              // Nullify id so another free fails