The error handling code isn't entirely right in two places.
One of the code blocks is dead because of an always-false condition, and
another code block is missing the assignment of a NULL pointer.
Getting the exact same behaviour is not entirely possible because you
can't extend the size of a shared memory region after it was made with
the Windows APIs we use, unless we destroy the region and recreate it,
but that has other consequences.
However, it certainly shouldn't crash.
Closes GH-14707.
* Include from build dir first
This fixes out of tree builds by ensuring that configure artifacts are included
from the build dir.
Before, out of tree builds would preferably include files from the src dir, as
the include path was defined as follows (ignoring includes from ext/ and sapi/) :
-I$(top_builddir)/main
-I$(top_srcdir)
-I$(top_builddir)/TSRM
-I$(top_builddir)/Zend
-I$(top_srcdir)/main
-I$(top_srcdir)/Zend
-I$(top_srcdir)/TSRM
-I$(top_builddir)/
As a result, an out of tree build would include configure artifacts such as
`main/php_config.h` from the src dir.
After this change, the include path is defined as follows:
-I$(top_builddir)/main
-I$(top_builddir)
-I$(top_srcdir)/main
-I$(top_srcdir)
-I$(top_builddir)/TSRM
-I$(top_builddir)/Zend
-I$(top_srcdir)/Zend
-I$(top_srcdir)/TSRM
* Fix extension include path for out of tree builds
* Include config.h with the brackets form
`#include "config.h"` searches in the directory containing the including-file
before any other include path. This can include the wrong config.h when building
out of tree and a config.h exists in the source tree.
Using `#include <config.h>` uses exclusively the include path, and gives
priority to the build dir.
* Replace WIN32 conditions with _WIN32 or PHP_WIN32
WIN32 is defined by the SDK and not defined all the time on Windows by
compilers or the environment. _WIN32 is defined as 1 when the
compilation target is 32-bit ARM, 64-bit ARM, x86, or x64. Otherwise,
undefined.
This syncs these usages one step further.
Upstream libgd has replaced WIN32 with _WIN32 via
c60d9fe577
PHP_WIN32 is added to ext/sockets/sockets.stub.php as done in other
*.stub.php files at this point.
* Use PHP_WIN32 in ext/random
* Use PHP_WIN32 in ext/sockets
* Use _WIN32 in xxhash.h as done upstream
See https://github.com/Cyan4973/xxHash/pull/931
* Update end comment with PHP_WIN32
Although the issue mentioned FreeBSD, this is a broader problem:
the current ARM64 code to load the TLS offset assumes a setup with
the non-default TLS model. This problem can also apply on some
configurations on other platforms.
Closes GH-11236.
autoconf/libtool generating code to test features missed `void` for
C calls prototypes w/o arguments.
Note that specific changes related to libtool have to be upstreamed.
Co-authored-by: Peter Kokot <petk@php.net>
close GH-13732
On shutdown in ZTS the following happens:
- https://github.com/php/php-src/blob/master/Zend/zend.c#L1124-L1125
gets executed. This destroys global persistent resources and destroys
the modules. Furthermore, the modules are unloaded too.
- Further down, `ts_free_id(executor_globals_id)` gets executed, which
calls `executor_globals_dtor`. This function destroys persistent
resources for each thread.
Notice that in the last step, the modules that the persistent resource
belong to may already have been destroyed. This means that accessing
globals will cause a crash (I previously fixed this with ifdef magic),
or when the module is dynamically loaded we'll try jumping to a
destructor that is no longer loaded in memory. These scenarios cause
crashes.
It's not possible to move the `ts_free_id` call upwards, because that
may break assumptions of callers, and furthermore this would deallocate
the executor globals structure, which means that any access to those
will cause a segfault.
This patch adds a new API to the TSRM that allows running a callback on
a certain resource type. We use this API to destroy the persistent
resources in all threads prior to the module destruction, and keep the
rest of the resource dtor intact.
I verified this fix on Apache with postgres, both dynamically and
statically.
Fixes GH-12974.
The TSRM keeps a hashtable mapping the thread IDs to the thread resource pointers.
It's possible that the thread disappears without us knowing, and then another thread
gets spawned some time later with the same ID as the disappeared thread.
Note that since it's a new thread the TSRM key pointer and cached pointer will be NULL.
The Apache request handler `php_handler()` will try to fetch some fields from the SAPI globals.
It uses a lazy thread resource allocation by calling `ts_resource(0);`.
This allocates a thread resource and sets up the TSRM pointers if they haven't been set up yet.
At least, that's what's supposed to happen. But since we are in a situation where the thread ID
still has the resources of the *old* thread associated in the hashtable,
the loop in `ts_resource_ex` will find that thread resource and assume the thread has been setup
already. But this is not the case since this thread is actually a new thread, just reusing the ID
of the old one, without any relation whatsoever to the old thread.
Because of this assumption, the TSRM pointers will not be setup, leading to a
NULL pointer dereference when trying to access the SAPI globals.
We can easily detect this scenario: if we're in the fallback path, and the pointer is NULL,
and we're looking for our own thread resource, we know we're actually reusing a thread ID.
In that case, we'll free up the old thread resources gracefully (gracefully because
there might still be resources open like database connection which need to be
shut down cleanly). After freeing the resources, we'll create the new resources for
this thread as if the stale resources never existed in the first place.
From that point forward, it is as if that situation never occurred.
The fact that this situation happens isn't that bad because a child process containing
threads will eventually be respawned anyway by the SAPI, so the stale thread resources
won't remain forever.
Note that we can't simply assign our own TSRM pointers to the existing
thread resource for our ID, since it was actually from a different thread
(just with the same ID!). Furthermore, the dynamically loaded extensions
have their own pointer, which is only set when their constructor is
called, so we'd have to call their constructor anyway...
I also tried to call the dtor and then the ctor again for those resources
on the pre-existing thread resource to reuse storage, but that didn't work properly
because other code doesn't expect something like that to happen, which breaks assumptions,
and this in turn caused Valgrind to (rightfully) complain about memory bugs.
Note 2: I also had to fix a bug in the core globals destruction because it
always assumed that the thread destroying them was the owning thread,
which on TSRM shutdown isn't always the case. A similar bug was fixed
recently with the JIT globals.
Closes GH-10863.
The next generation of C compilers is going to enforce the C standard
more strictly:
https://wiki.gentoo.org/wiki/Modern_C_porting
One warning that will soon become an error is -Wstrict-prototypes.
This is relatively easy to catch in most code (it will fail to
compile), but inside of autoconf tests it can go unnoticed because
many feature-test compilations fail by design. For example,
$ export CFLAGS="$CFLAGS -Werror=strict-prototypes"
$ ./configure
...
checking if iconv supports errno... no
configure: error: iconv does not support errno
(this is on a system where iconv *does* support errno). If errno
support were optional, that test would have "silently" disabled
it. The underlying issue here, from config.log, is
conftest.c:211:5: error: function declaration isn't a prototype
[-Werror=strict-prototypes]
211 | int main() {
This commit goes through all of our autoconf tests, replacing main()
with main(void). Up to equivalent types and variable renamings, that's
one of the two valid signatures, and satisfies the compiler (gcc-12 in
this case).
Fixes GH-10751
* ext/opcache/zend_jit: cast function to fix -Wincompatible-pointer-types
Regression by commit a21195650e
* TSRM/win32: fix ts_allocate_dtor cast
The dtor was casted to ts_allocate_ctor; luckily, ts_allocate_dtor and
ts_allocate_ctor just happen to be the same type.
The recently committed fix for GH-9944 did only indirectly cater to
that, namely because in this case `CreateFileMapping()` with a zero
size couldn't be created. As of PHP 8.2.0, the mappings of the actual
SHM and the info segment have been merged, so creating a zero size SHM
would be possible unless we explicitly prohibit this.
`shm_get()` (not to be confused with `shmget()`) returns `NULL` if
reallocation fails; we need to cater to that when calling the function.
Closes GH-9872.
`shm->addr` cannot be `NULL` here, so the whole check is completely
pointless.
Given that this is merely a clean-up and not an actual bug fix, we do
without a NEWS entry.
Closes GH-9873.
There is not much point in having two distinct file mappings; since the
info mapping is very small and of fixed size, we can put it at the
beginning of a single mapping. Besides the obvious resource savings,
that also simplifies the error handling.
Closes GH-8648.