Remove an incorrect use of the address operator

[lexprfuncall]

The address of the local variable created_threads is a different location than the data it points to. Incorrectly treating these values as being the same can cause out-of-bounds writes to the stack.

Resolves #59

[lexprfuncall]

I'm guessing the (bool *) was needed because the type of &created_threads is (bool **).

[guangli-dai]

Since this a bug fix, can you provide the before and after test outcome following the forced usage of alloca here?

[lexprfuncall]

Since this a bug fix, can you provide the before and after test outcome following the forced usage of alloca here?

I wasn't able to reproduce a warning with just the change in that issue comment. However, by removing the cast, a warning is generated

$  gcc -std=gnu11 -Wall -Wextra -Wsign-compare -Wundef -Wno-format-zero-length -Wpointer-arith -Wno-missing-braces -Wno-missing-field-initializers -Wno-missing-attributes -pipe -g3 -fvisibility=hidden -Wimplicit-fallthrough -Wdeprecated-declarations -O3 -funroll-loops -fno-builtin -c -D_GNU_SOURCE -D_REENTRANT -Iinclude -Iinclude -DJEMALLOC_UNIT_TEST -Itest/include -Itest/include -DJEMALLOC_JET -DJEMALLOC_NO_PRIVATE_NAMESPACE -o src/background_thread.jet.sym.o src/background_thread.c
<NIT_TEST -Itest/include -Itest/include -DJEMALLOC_JET -DJEMALLOC_NO_PRIVATE_NAMESPACE -o src/background_thread.jet.sym.o src/background_thread.c
src/background_thread.c: In function ‘background_thread0_work’:
src/background_thread.c:452:25: error: passing argument 4 of ‘check_background_thread_creation’ from incompatible pointer type [-Wincompatible-pointer-types]
  452 |                         &created_threads)) {
      |                         ^~~~~~~~~~~~~~~~
      |                         |
      |                         _Bool **
src/background_thread.c:374:11: note: expected ‘_Bool *’ but argument is of type ‘_Bool **’
  374 |     bool *created_threads) {
      |           ^
$

[guangli-dai]

a warning is generated

Under what setup do you see this warning? My gcc 11.5.0 does not give any warning.

I was able to reproduce the failing tests with these commands: make clean && ./configure CFLAGS="-D__STDC_NO_VLA__" && make -j64 && make tests -j64 && make check -j64 with an error: expect_deferred_purging:test/unit/hpa_background_thread.c:177: Failed assertion: (0) == (empty_ndirty) --> 0 != 1: Should have seen a background purge.

With this patch, I don't see the error anymore. But if the warning you show is real, it may be breaking some other compatibilities.

[lexprfuncall]

Under what setup do you see this warning? My gcc 11.5.0 does not give any warning.

Sorry, I think I misunderstood your previous request. What I did was probably not so interesting: I modified the base revision removing the (bool *) cast.

I was able to reproduce the failing tests with these commands: make clean && ./configure CFLAGS="-D__STDC_NO_VLA__" && make -j64 && make tests -j64 && make check -j64 with an error: expect_deferred_purging:test/unit/hpa_background_thread.c:177: Failed assertion: (0) == (empty_ndirty) --> 0 != 1: Should have seen a background purge.

With this patch, I don't see the error anymore. But if the warning you show is real, it may be breaking some other compatibilities.

Okay, without the PR, when I add -D__STDC_NO_VLA__=1 I reliably see either that same test crash

=== test/unit/hpa_background_thread ===
test_hpa_background_thread_a0_initialized (non-reentrant): pass
expect_deferred_purging:test/unit/hpa_background_thread.c:177: Failed assertion: (0) == (empty_ndirty) --> 0 != 1: Should have seen a background purge
test_hpa_background_thread_purges (non-reentrant): fail

or a related test crash

=== test/unit/background_thread_enable ===
test_deferred (non-reentrant): pass
Segmentation fault (core dumped)
Test harness error: test/unit/background_thread_enable w/ MALLOC_CONF=""
Use prefix to debug, e.g. JEMALLOC_TEST_PREFIX="gdb --args" sh test/test.sh test/unit/background_thread_enable
make: *** [Makefile:728: check_unit] Error 1
make: *** Waiting for unfinished jobs....

With this PR, no tests crash.

As an aside, according to the documentation, MSVC defines __STDC_NO_VLA__ when compiling in its C11 or C17 modes suggesting this bug might actually affect a lot of users.

[guangli-dai]

Great! Now that we can confirm the this PR indeed fixes the bug as expected and from what I can see, there should be no side effects on environment where __STDC_NO_VLA__ is not defined (all-green CIs confirm that).