alloc: restore trust to native malloc

mkuoppal · mkuoppal · commit 9a4e1551527b · 2024-12-07T14:22:11.000+02:00
Do not kneel in front of bad static analyzers
and try to align your allocs. This method of
extending/aligning over the native malloc uapi
will render some of the memory/malloc analyzer
detection useless, like not seeing out of bounds
accesses on non aligned ones.

Instead of catering to some bad analyzer, use native
malloc and thus give full visibility to proper ones.
If some architecture still needs aligment to long,
it should be special cased and not taint the sane ones.

Remove aligment padding from alloc(), return
void ptrs like in native malloc and deny zero sized
allocs.
diff --git a/include/global.h b/include/global.h
@@ -320,8 +320,8 @@ extern char *dupstr_n(const char *string,
                       unsigned *lenout) NONNULL NONNULLPTRS;
 
 /* declare alloc.c's alloc(); allocations made with it use ordinary free() */
-extern long *alloc(unsigned int) NONNULL;  /* alloc.c */
-extern long *re_alloc(long *, unsigned int) NONNULL;
+extern void *alloc(unsigned int) NONNULL;  /* alloc.c */
+extern void *re_alloc(void *, unsigned int) NONNULL;
 
 /* Used for consistency checks of various data files; declare it here so
    that utility programs which include config.h but not hack.h can see it. */
diff --git a/src/alloc.c b/src/alloc.c
@@ -21,55 +21,40 @@ extern unsigned FITSuint_(unsigned long long, const char *, int) NONNULLARG2;
 
 char *fmt_ptr(const genericptr) NONNULL;
 
-/*
- * For historical reasons, nethack's alloc() returns 'long *' rather
- * than 'void *' or 'char *'.
- *
- * Some static analysis complains if it can't deduce that the number
- * of bytes being allocated is a multiple of 'sizeof (long)'.  It
- * recognizes that the following manipulation overcomes that via
- * rounding the requested length up to the next long.  NetHack doesn't
- * make a lot of tiny allocations, so this shouldn't waste much memory
- * regardless of whether malloc() does something similar.  NetHack
- * isn't expected to call alloc(0), but if that happens treat it as
- * alloc(sizeof (long)) instead.
- */
-#define ForceAlignedLength(LTH) \
-    do {                                                        \
-        if (!(LTH) || (LTH) % sizeof (long) != 0)               \
-            (LTH) += sizeof (long) - (LTH) % sizeof (long);     \
-    } while (0)
-
-long *alloc(unsigned int) NONNULL;
-long *re_alloc(long *, unsigned int) NONNULL;
+void *alloc(unsigned int) NONNULL;
+void *re_alloc(void *, unsigned int) NONNULL;
 ATTRNORETURN extern void panic(const char *, ...) PRINTF_F(1, 2) NORETURN;
 
-long *
-alloc(unsigned int lth)
+void *
+alloc(unsigned int bytes)
 {
-    genericptr_t ptr;
+    void *p;
+
+    if (!bytes)
+        panic("Zero sized allocs not allowed");
 
-    ForceAlignedLength(lth);
-    ptr = malloc(lth);
-    if (!ptr)
-        panic("Memory allocation failure; cannot get %u bytes", lth);
+    p = malloc(bytes);
+    if (!p)
+        panic("Memory allocation failure; cannot get %u bytes", bytes);
 
-    return (long *) ptr;
+    return p;
 }
 
 /* realloc() call that might get substituted by nhrealloc(p,n,file,line) */
-long *
-re_alloc(long *oldptr, unsigned int newlth)
+void *
+re_alloc(void *old, unsigned int newlth)
 {
-    long *newptr;
+    void *p;
+
+    if (!newlth)
+        panic("Zero sized reallocs not allowed");
 
-    ForceAlignedLength(newlth);
-    newptr = (long *) realloc((genericptr_t) oldptr, (size_t) newlth);
+    p = realloc(old, (size_t) newlth);
     /* "extend to":  assume it won't ever fail if asked to shrink */
-    if (newlth && !newptr)
+    if (!p)
         panic("Memory allocation failure; cannot extend to %u bytes", newlth);
 
-    return newptr;
+    return p;
 }
 
 #ifdef HAS_PTR_FMT
