SUPPORT_DIRECT_MALLOC and SUPPORT_REMOVE_MALLOC_LEGACY

This supports three build modes:

No additional -D options, default legacy mode

-DSUPPORT_DIRECT_MALLOC, magic will use direct calls to libc malloc/free
 and will leave in place the symbols now renamed as mallocMagicLegacy()
 freeMagicLegacy() and callocMagicLegacy().

-DSUPPORT_DIRECT_MALLOC -DSUPPORT_REMOVE_MALLOC_LEGACY as above but will
 remove the three legacy functions from the binary to provide assurance
 they can not be used.

The system malloc is thread-safe the legacy magic malloc has a global
deferred free pointer and the mmap() allocate has a free-list that is
not thread-safe making use of free not thread-safe.
This could of course be improved with the use of
atomic_compare_and_exchange operations but for what gain ?

Then there is the additional function call overhead (of the indirection)
and a few tests/branches inserted into a commonly used code paths around
memory allocation, it hides the call site of the malloc/free usage from
the compiler which maybe have special optimization cases.

The existing malloc/free makes static code analysis around memory
allocation more problematic, also use of runtime analysers will operate
better with a fail-fast to bad memory usage.

utils/magic.h

@@ -133,9 +133,11 @@ extern char *SysLibPath;	/* Path for finding system
  * Just for the sake of robustness, though, we define malloc and free
  * here to error strings.
  */
+#ifndef SUPPORT_DIRECT_MALLOC
 #define	malloc	You_should_use_the_Magic_procedure_mallocMagic_instead
 #define	free	You_should_use_the_Magic_procedure_freeMagic_instead
 #define calloc	You_should_use_the_Magic_procedure_callocMagic_instead
+#endif
 
 /* ---------- Flag for global variables (for readability) ------------- */
 

utils/malloc.c

@@ -43,14 +43,18 @@ static char rcsid[] __attribute__ ((unused)) = "$Header: /usr/cvsroot/magic-8.0/
 #include <string.h>
 
 #include "tcltk/tclmagic.h"
+#define _MAGIC__UTILS__MALLOC_H__NOINLINE
 #include "utils/magic.h"
 #include "utils/malloc.h"
 
 /* Normally we're supposed to warn against the use of standard malloc()	*/
 /* and free(), but obviously that doesn't apply to this file.		*/
 
+#ifndef SUPPORT_DIRECT_MALLOC
+/* this is needed to remove the utils/magic.h defines */
 #undef malloc
 #undef free
+#endif
 
 /* Imports */
 
@@ -64,6 +68,7 @@ extern char *TxGetLine();
  * would no further references would be made to free'ed storage.
  */
 
+#ifndef SUPPORT_REMOVE_MALLOC_LEGACY
 /* Delay free'ing by one call, to accommodate Magic's needs. */
 static char *freeDelayedItem = NULL;
 
@@ -92,14 +97,14 @@ static char *freeDelayedItem = NULL;
 
 /*
  *---------------------------------------------------------------------
- * mallocMagic() --
+ * mallocMagicLegacy() --
  *
  *	memory allocator with support for one-delayed-item free'ing
  *---------------------------------------------------------------------
  */
 
 void *
-mallocMagic(nbytes)
+mallocMagicLegacy(nbytes)
     size_t nbytes;
 {
     void *p;
@@ -127,14 +132,14 @@ mallocMagic(nbytes)
 
 /*
  *---------------------------------------------------------------------
- * freeMagic() --
+ * freeMagicLegacy() --
  *
  *	one-delayed-item memory deallocation
  *---------------------------------------------------------------------
  */
 
 void
-freeMagic(cp)
+freeMagicLegacy(cp)
     void *cp;
 {
     if (cp == NULL)
@@ -150,14 +155,14 @@ freeMagic(cp)
 
 /*
  *---------------------------------------------------------------------
- * callocMagic() --
+ * callocMagicLegacy() --
  *
  *	allocate memory and initialize it to all zero bytes.
  *---------------------------------------------------------------------
  */
 
 void *
-callocMagic(nbytes)
+callocMagicLegacy(nbytes)
     size_t nbytes;
 {
     void *cp;
@@ -168,3 +173,42 @@ callocMagic(nbytes)
     return (cp);
 }
 
+#endif /* SUPPORT_REMOVE_MALLOC_LEGACY */
+
+
+/*
+ *  NOTICE: non-inline form of emitted functions, keep in sync with malloc.h
+ */
+#pragma weak freeMagic1_init = freeMagic1_init_func
+free_magic1_t freeMagic1_init_func() {
+    return NULL;
+}
+
+#pragma weak freeMagic1 = freeMagic1_func
+void freeMagic1_func(free_magic1_t* m1, void* ptr) {
+    //if(*m1) /* this if() is here to help inliner remove the call to free() when it can */
+    /* this is not the inline form here so if() is commented out */
+    {
+#if (defined(SUPPORT_DIRECT_MALLOC) || defined(SUPPORT_REMOVE_MALLOC_LEGACY))
+	free(*m1); /* no need for NULL check with free() */
+#else
+	/* but freeMagicLegacy() does not like NULL passed, so extra if() penalty here */
+	if(*m1) freeMagicLegacy(*m1);
+#endif
+    }
+    *m1 = ptr;
+}
+
+#pragma weak freeMagic1_end = freeMagic1_end_func
+void freeMagic1_end_func(free_magic1_t* m1) {
+    //if(*m1) /* this if() is here to help inliner remove the call to free() when it can */
+    /* this is not the inline form here so if() is commented out */
+    {
+#if (defined(SUPPORT_DIRECT_MALLOC) || defined(SUPPORT_REMOVE_MALLOC_LEGACY))
+	free(*m1); /* no need for NULL check with free() */
+#else
+	/* but freeMagicLegacy() does not like NULL passed, so extra if() penalty here */
+	if(*m1) freeMagicLegacy(*m1);
+#endif
+    }
+}

utils/malloc.h

@@ -22,8 +22,95 @@
 #ifndef _MAGIC__UTILS__MALLOC_H
 #define _MAGIC__UTILS__MALLOC_H
 
-extern void *mallocMagic(size_t);
+#include <stdlib.h>
-extern void *callocMagic(size_t);
+
-extern void freeMagic(void *);
+
+/* build time configuration check */
+#if (!defined(SUPPORT_DIRECT_MALLOC) && defined(SUPPORT_REMOVE_MALLOC_LEGACY))
+ #error "ERROR: Unspported build configuration SUPPORT_REMOVE_MALLOC_LEGACY is defined but SUPPORT_DIRECT_MALLOC is undefined"
+#endif
+
+
+#ifdef SUPPORT_DIRECT_MALLOC
+
+#define mallocMagic malloc
+#define callocMagic calloc
+#define freeMagic free
+
+#else
+
+extern void *mallocMagicLegacy(size_t);
+#define mallocMagic(size) mallocMagicLegacy(size)
+
+/* renamed like this, so there is no performance loss if the byte count
+ *  can be computed at compile time.
+ */
+extern void *callocMagicLegacy(size_t);
+#define callocMagic(nmemb, size) callocMagicLegacy((nmemb) * (size))
+
+extern void freeMagicLegacy(void *);
+#define freeMagic(ptr) freeMagicLegacy(ptr)
+
+#endif /* SUPPORT_DIRECT_MALLOC */
+
+
+typedef void* free_magic1_t;
+
+/* TODO this should be moved to autoconf/build/toolchain detection, this does exist
+ * in autoconf and in another changeset, so I come back and fixup/remove this later.
+ */
+#if (defined(__STDC__) && defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 199901L))
+ /* C99 or later */
+ #ifndef __inline__
+   /* you'd have thought on linux this was enabled already, TODO check bplane module inlines */
+   #define __inline__ inline
+ #endif
+#endif
+
+#if (!defined(_MAGIC__UTILS__MALLOC_H__NOINLINE) && defined(__inline__))
+
+/* TODO this (__extern_inline__) should be moved to autoconf/build/toolchain detection */
+#define __extern_inline__ inline
+
+/*
+ *  NOTICE: inline form, keep in sync with malloc.c copied
+ */
+__extern_inline__ free_magic1_t freeMagic1_init() {
+    return NULL;
+}
+__extern_inline__ void freeMagic1(free_magic1_t* m1, void* ptr) {
+    if(*m1) /* this if() is here to help inliner remove the call to free() when it can */
+    {
+#if (defined(SUPPORT_DIRECT_MALLOC) || defined(SUPPORT_REMOVE_MALLOC_LEGACY))
+	free(*m1); /* no need for NULL check with free() */
+#else
+	freeMagicLegacy(*m1);
+#endif
+    }
+    *m1 = ptr;
+}
+__extern_inline__ void freeMagic1_end(free_magic1_t* m1) {
+    if(*m1) /* this if() is here to help inliner remove the call to free() when it can */
+    {
+#if (defined(SUPPORT_DIRECT_MALLOC) || defined(SUPPORT_REMOVE_MALLOC_LEGACY))
+	free(*m1); /* no need for NULL check with free() */
+#else
+	freeMagicLegacy(*m1);
+#endif
+    }
+}
+
+#else
+
+#define freeMagic1_init() freeMagic1_init_func()
+#define freeMagic1(m1, ptr) freeMagic1_func((m1), (ptr))
+#define freeMagic1_end(m1) freeMagic1_end_func((m1))
+
+#endif /* !_MAGIC__UTILS__MALLOC_H__NOINLINE && __inline__ */
+
+/* we'll emit a function call interface just in case a platform won't inline and we can redirect */
+extern free_magic1_t freeMagic1_init_func(void);
+extern void freeMagic1_func(free_magic1_t* m1, void* ptr);
+extern void freeMagic1_end_func(free_magic1_t* m1);
 
 #endif /* _MAGIC__UTILS__MALLOC_H */