Merge branch '2529-add-__attribute__-malloc-for-isc_mempool_get' into 'main'

Resolve "Add __attribute__((malloc)) for isc_mempool_get"

Closes #2529

See merge request isc-projects/bind9!4858

configure.ac

@@ -422,6 +422,34 @@ AC_COMPILE_IFELSE(
 #
 AX_GCC_FUNC_ATTRIBUTE([noreturn])
 
+#
+# check for GCC malloc attribute
+#
+AX_GCC_FUNC_ATTRIBUTE([malloc])
+
+AC_MSG_CHECKING([for extended malloc attributes])
+AC_COMPILE_IFELSE(
+  [AC_LANG_PROGRAM(
+     [[
+       #include <stddef.h>
+       #include <stdlib.h>
+	   __attribute__ ((malloc, malloc (free, 1))
+	   void * xmalloc(size_t sz) { return malloc(sz); }
+     ]],
+     [[
+       void *p = xmalloc(8);
+	   free(p);
+     ]])],
+  [AC_MSG_RESULT(yes)
+   AC_DEFINE([HAVE_MALLOC_EXT_ATTR], [1], [define if extended attributes for malloc are available])
+  ],
+  [AC_MSG_RESULT(no)])
+
+#
+# check for GCC returns_nonnull attribute
+#
+AX_GCC_FUNC_ATTRIBUTE([returns_nonnull])
+
 #
 # check if we have kqueue
 #

lib/isc/include/isc/attributes.h

@@ -24,3 +24,63 @@
 #else
 #define ISC_NORETURN
 #endif
+
+#if HAVE_FUNC_ATTRIBUTE_RETURNS_NONNULL
+#define ISC_ATTR_RETURNS_NONNULL __attribute__((returns_nonnull))
+#else
+#define ISC_ATTR_RETURNS_NONNULL
+#endif
+
+#ifdef HAVE_FUNC_ATTRIBUTE_MALLOC
+/*
+ * Indicates that a function is malloc-like, i.e., that the
+ * pointer P returned by the function cannot alias any other
+ * pointer valid when the function returns.
+ */
+#define ISC_ATTR_MALLOC __attribute__((malloc))
+#if HAVE_MALLOC_EXT_ATTR
+/*
+ * Associates deallocator as a suitable deallocation function
+ * for pointers returned from the function marked with the attribute.
+ */
+#define ISC_ATTR_DEALLOCATOR(deallocator) __attribute__((malloc(deallocator)))
+/*
+ * Similar to ISC_ATTR_DEALLOCATOR, but allows to speficy an index "idx",
+ * which denotes the positional argument to which when the pointer is passed
+ * in calls to deallocator has the effect of deallocating it.
+ */
+#define ISC_ATTR_DEALLOCATOR_IDX(deallocator, idx) \
+	__attribute__((malloc(deallocator, idx)))
+/*
+ * Combines both ISC_ATTR_MALLOC and ISC_ATTR_DEALLOCATOR attributes.
+ */
+#define ISC_ATTR_MALLOC_DEALLOCATOR(deallocator) \
+	__attribute__((malloc, malloc(deallocator)))
+/*
+ * Similar to ISC_ATTR_MALLOC_DEALLOCATOR, but allows to speficy an index "idx",
+ * which denotes the positional argument to which when the pointer is passed
+ * in calls to deallocator has the effect of deallocating it.
+ */
+#define ISC_ATTR_MALLOC_DEALLOCATOR_IDX(deallocator, idx) \
+	__attribute__((malloc, malloc(deallocator, idx)))
+#else /* #ifdef HAVE_MALLOC_EXT_ATTR */
+/*
+ * There is support for malloc attribute but not for
+ * extended attributes, so macros that combine attribute malloc
+ * with a deallocator will only expand to malloc attribute.
+ */
+#define ISC_ATTR_DEALLOCATOR(deallocator)
+#define ISC_ATTR_DEALLOCATOR_IDX(deallocator, idx)
+#define ISC_ATTR_MALLOC_DEALLOCATOR(deallocator)	  ISC_ATTR_MALLOC
+#define ISC_ATTR_MALLOC_DEALLOCATOR_IDX(deallocator, idx) ISC_ATTR_MALLOC
+#endif
+#else /* #ifdef HAVE_FUNC_ATTRIBUTE_MALLOC */
+/*
+ * There is no support for malloc attribute.
+ */
+#define ISC_ATTR_MALLOC
+#define ISC_ATTR_DEALLOCATOR(deallocator)
+#define ISC_ATTR_DEALLOCATOR_IDX(deallocator, idx)
+#define ISC_ATTR_MALLOC_DEALLOCATOR(deallocator)
+#define ISC_ATTR_MALLOC_DEALLOCATOR_IDX(deallocator, idx)
+#endif /* HAVE_FUNC_ATTRIBUTE_MALLOC */

lib/isc/include/isc/mem.h

@@ -17,6 +17,7 @@
 #include <stdbool.h>
 #include <stdio.h>
 
+#include <isc/attributes.h>
 #include <isc/lang.h>
 #include <isc/mutex.h>
 #include <isc/platform.h>
@@ -508,19 +509,54 @@ isc_mempool_setfillcount(isc_mempool_t *mpctx, unsigned int limit);
  *\li	limit > 0
  */
 
+#if defined(UNIT_TESTING) && defined(malloc)
+/*
+ * cmocka.h redefined malloc as a macro, we #undef it
+ * to avoid replacing ISC_ATTR_MALLOC with garbage.
+ */
+#pragma push_macro("malloc")
+#undef malloc
+#define POP_MALLOC_MACRO 1
+#endif
+
 /*
  * Pseudo-private functions for use via macros.  Do not call directly.
  */
+void ISCMEMFUNC(putanddetach)(isc_mem_t **, void *, size_t _ISC_MEM_FLARG);
+void ISCMEMFUNC(put)(isc_mem_t *, void *, size_t _ISC_MEM_FLARG);
+void ISCMEMFUNC(free)(isc_mem_t *, void *_ISC_MEM_FLARG);
+
+ISC_ATTR_RETURNS_NONNULL
+ISC_ATTR_MALLOC_DEALLOCATOR_IDX(ISCMEMFUNC(put), 2)
 void *ISCMEMFUNC(get)(isc_mem_t *, size_t _ISC_MEM_FLARG);
-void  ISCMEMFUNC(putanddetach)(isc_mem_t **, void *, size_t _ISC_MEM_FLARG);
+
-void  ISCMEMFUNC(put)(isc_mem_t *, void *, size_t _ISC_MEM_FLARG);
+ISC_ATTR_RETURNS_NONNULL
+ISC_ATTR_MALLOC_DEALLOCATOR_IDX(ISCMEMFUNC(free), 2)
 void *ISCMEMFUNC(allocate)(isc_mem_t *, size_t _ISC_MEM_FLARG);
+
+ISC_ATTR_RETURNS_NONNULL
+ISC_ATTR_DEALLOCATOR_IDX(ISCMEMFUNC(free), 2)
 void *ISCMEMFUNC(reallocate)(isc_mem_t *, void *, size_t _ISC_MEM_FLARG);
-void  ISCMEMFUNC(free)(isc_mem_t *, void *_ISC_MEM_FLARG);
+
+ISC_ATTR_RETURNS_NONNULL
+ISC_ATTR_MALLOC_DEALLOCATOR_IDX(ISCMEMFUNC(free), 2)
 char *ISCMEMFUNC(strdup)(isc_mem_t *, const char *_ISC_MEM_FLARG);
+
+ISC_ATTR_RETURNS_NONNULL
+ISC_ATTR_MALLOC_DEALLOCATOR_IDX(ISCMEMFUNC(free), 2)
 char *ISCMEMFUNC(strndup)(isc_mem_t *, const char *, size_t _ISC_MEM_FLARG);
+
+ISC_ATTR_MALLOC_DEALLOCATOR_IDX(ISCMEMPOOLFUNC(put), 2)
 void *ISCMEMPOOLFUNC(get)(isc_mempool_t *_ISC_MEM_FLARG);
-void  ISCMEMPOOLFUNC(put)(isc_mempool_t *, void *_ISC_MEM_FLARG);
+
+void ISCMEMPOOLFUNC(put)(isc_mempool_t *, void *_ISC_MEM_FLARG);
+
+#ifdef POP_MALLOC_MACRO
+/*
+ * Restore cmocka.h macro for malloc.
+ */
+#pragma pop_macro("malloc")
+#endif
 
 ISC_LANG_ENDDECLS
 

lib/isc/mem.c

@@ -1366,15 +1366,7 @@ isc__mempool_get(isc_mempool_t *mpctx FLARG) {
 		}
 	}
 
-	/*
-	 * If we didn't get any items, return NULL.
-	 */
 	item = mpctx->items;
-	if (ISC_UNLIKELY(item == NULL)) {
-		atomic_fetch_sub_release(&mpctx->allocated, 1);
-		goto out;
-	}
-
 	mpctx->items = item->next;
 
 	INSIST(atomic_fetch_sub_release(&mpctx->freecount, 1) > 0);
@@ -1382,7 +1374,6 @@ isc__mempool_get(isc_mempool_t *mpctx FLARG) {
 
 	ADD_TRACE(mpctx->mctx, item, mpctx->size, file, line);
 
-out:
 	MPCTXUNLOCK(mpctx);
 
 	return (item);