Pointer bug in `background_thread0_work()` if `VARIABLE_ARRAY` used `alloca()` implementation

[fuhsnn]

Under rare conditions, VLA declarations in jemalloc might use alloca() under the hood:

jemalloc/include/jemalloc/internal/jemalloc_internal_types.h

Lines 124 to 140 in bfb63ea

	/* Declare a variable-length array. */
	#if __STDC_VERSION__ < 199901L || defined(__STDC_NO_VLA__)
	# ifdef _MSC_VER
	# include <malloc.h>
	# define alloca _alloca
	# else
	# ifdef JEMALLOC_HAS_ALLOCA_H
	# include <alloca.h>
	# else
	# include <stdlib.h>
	# endif
	# endif
	# define VARIABLE_ARRAY_UNSAFE(type, name, count) \
	type *name = alloca(sizeof(type) * (count))
	#else
	# define VARIABLE_ARRAY_UNSAFE(type, name, count) type name[(count)]
	#endif

This particular VLA in background_thread0_work(),

jemalloc/src/background_thread.c

Line 437 in bfb63ea

VARIABLE_ARRAY(bool, created_threads, const_max_background_threads);

has its address taken here:

jemalloc/src/background_thread.c

Lines 449 to 453 in bfb63ea

	if (check_background_thread_creation(tsd,
	const_max_background_threads, &n_created,
	(bool *)&created_threads)) {
	continue;
	}

With alloca() the variable is actually a pointer, so &created_threads wouldn't be the intended array space but instead the address of the pointer variable itself (bool **), which leads to test failing, hanging or segfault.

Only found because I was disabling VLA in my C compiler for fun, but can be reproduced on GCC/Clang by forcing alloca:

diff --git a/src/background_thread.c b/src/background_thread.c
index 2eb08dd..96ef481 100644
--- a/src/background_thread.c
+++ b/src/background_thread.c
@@ -434,7 +434,7 @@ background_thread0_work(tsd_t *tsd) {
 	 */
 	const size_t const_max_background_threads = max_background_threads;
 	assert(const_max_background_threads > 0);
-	VARIABLE_ARRAY(bool, created_threads, const_max_background_threads);
+	bool *created_threads = __builtin_alloca(sizeof(bool) * const_max_background_threads);
 	unsigned i;
 	for (i = 1; i < const_max_background_threads; i++) {
 		created_threads[i] = false;

Will file a PR.

[lexprfuncall]

@fuhsnn Thanks for reporting this issue. From what I see, it looks like check_background_thread_creation is expecting a bool * so rather than passing (bool *)&created_threads, passing created_threads by itself seems more appropriate. I am not sure why the (bool *)& was added in the first place. I will double check the commit history for clues and put together a PR to fix this.