Skip to content

Commit 3e33e1e

Browse files
committed
Check linker compatibility directly from HMODULE
Checking the linker compatibility with extranous `ImageLoad()` calls is possible, but unnecessary, since the modules are either already loaded or loaded shortly afterwards, so that we can get the required information directly from the module handles. And actually, doing `ImageLoad()` as well as `LoadLibrary()` leaves a tiny room for a race condition, because both functions will lookup the module in the search path, so there is no *guarantee* that both are dealing with the same module. Dropping the `ImageLoad()` calls also has the advantage to no longer face the issue reported in bug #79557. A very minor additional advantage is that we no longer have to link against Imagehlp.dll. Furthermore, there is no need to check for CRT compatibility multiple times, so we can simplify the signature of `php_win32_crt_compatible`, and at the same time clean up main.c a bit. These changes require to change the signature of the exported `php_win32_image_compatible` and `php_win32_crt_compatible` functions, which now expect a `HMODULE` and nothing, respectively, instead of the module name.
1 parent 6fa2493 commit 3e33e1e

File tree

8 files changed

+49
-54
lines changed

8 files changed

+49
-54
lines changed

UPGRADING.INTERNALS

+4
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,10 @@ PHP 8.0 INTERNALS UPGRADE NOTES
373373
- zend_get_op_array_extension_handle()
374374
10. Argument zend_extension to const char* in Zend Engine 4.0:
375375
- zend_get_resource_handle()
376+
11. Argument const char * to HMODULE in Zend Engine 4.0:
377+
- php_win32_image_compatible()
378+
12. const char * argument dropped in Zend Engine 4.0:
379+
- php_win32_crt_compatible()
376380

377381
u. Instead of overwriting zend_error_cb extensions with debugging, monitoring
378382
use-cases catching Errors/Exceptions are strongly encouraged to use

Zend/zend_extensions.c

+7
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,13 @@ zend_result zend_load_extension(const char *path)
4141
#endif
4242
return FAILURE;
4343
}
44+
#ifdef ZEND_WIN32
45+
char *err;
46+
if (!php_win32_image_compatible(handle, &err)) {
47+
zend_error(E_CORE_WARNING, err);
48+
return FAILURE;
49+
}
50+
#endif
4451
return zend_load_extension_handle(handle, path);
4552
#else
4653
fprintf(stderr, "Extensions are not supported on this platform.\n");

ext/standard/dl.c

+2-4
Original file line numberDiff line numberDiff line change
@@ -161,19 +161,17 @@ PHPAPI int php_load_extension(const char *filename, int type, int start_now)
161161
efree(orig_libpath);
162162
efree(err1);
163163
}
164+
efree(libpath);
164165

165166
#ifdef PHP_WIN32
166-
if (!php_win32_image_compatible(libpath, &err1)) {
167+
if (!php_win32_image_compatible(handle, &err1)) {
167168
php_error_docref(NULL, error_type, err1);
168169
efree(err1);
169-
efree(libpath);
170170
DL_UNLOAD(handle);
171171
return FAILURE;
172172
}
173173
#endif
174174

175-
efree(libpath);
176-
177175
get_module = (zend_module_entry *(*)(void)) DL_FETCH_SYMBOL(handle, "get_module");
178176

179177
/* Some OS prepend _ to symbol names while their dynamic linker

main/main.c

+1-10
Original file line numberDiff line numberDiff line change
@@ -2055,21 +2055,12 @@ int php_module_startup(sapi_module_struct *sf, zend_module_entry *additional_mod
20552055
#endif
20562056

20572057
#ifdef PHP_WIN32
2058-
# if PHP_LINKER_MAJOR == 14
2059-
/* Extend for other CRT if needed. */
2060-
# if PHP_DEBUG
2061-
# define PHP_VCRUNTIME "vcruntime140d.dll"
2062-
# else
2063-
# define PHP_VCRUNTIME "vcruntime140.dll"
2064-
# endif
20652058
char *img_err;
2066-
if (!php_win32_crt_compatible(PHP_VCRUNTIME, &img_err)) {
2059+
if (!php_win32_crt_compatible(&img_err)) {
20672060
php_error(E_CORE_WARNING, img_err);
20682061
efree(img_err);
20692062
return FAILURE;
20702063
}
2071-
# undef PHP_VCRUNTIME
2072-
# endif
20732064

20742065
/* start up winsock services */
20752066
if (WSAStartup(wVersionRequested, &wsaData) != 0) {

main/php_ini.c

+2-9
Original file line numberDiff line numberDiff line change
@@ -331,13 +331,6 @@ static void php_load_zend_extension_cb(void *arg)
331331
#endif
332332

333333
if (IS_ABSOLUTE_PATH(filename, length)) {
334-
#ifdef PHP_WIN32
335-
char *err;
336-
if (!php_win32_image_compatible(filename, &err)) {
337-
php_error(E_CORE_WARNING, err);
338-
return;
339-
}
340-
#endif
341334
zend_load_extension(filename);
342335
} else {
343336
DL_HANDLE handle;
@@ -381,13 +374,13 @@ static void php_load_zend_extension_cb(void *arg)
381374

382375
efree(orig_libpath);
383376
efree(err1);
377+
efree(libpath);
384378
}
385379

386380
#ifdef PHP_WIN32
387-
if (!php_win32_image_compatible(libpath, &err1)) {
381+
if (!php_win32_image_compatible(handle, &err1)) {
388382
php_error(E_CORE_WARNING, err1);
389383
efree(err1);
390-
efree(libpath);
391384
DL_UNLOAD(handle);
392385
return;
393386
}

win32/build/confutils.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -3401,7 +3401,7 @@ function toolset_setup_common_ldlags()
34013401
function toolset_setup_common_libs()
34023402
{
34033403
// urlmon.lib ole32.lib oleaut32.lib uuid.lib gdi32.lib winspool.lib comdlg32.lib
3404-
DEFINE("LIBS", "kernel32.lib ole32.lib user32.lib advapi32.lib shell32.lib ws2_32.lib Dnsapi.lib psapi.lib bcrypt.lib imagehlp.lib");
3404+
DEFINE("LIBS", "kernel32.lib ole32.lib user32.lib advapi32.lib shell32.lib ws2_32.lib Dnsapi.lib psapi.lib bcrypt.lib");
34053405
}
34063406

34073407
function toolset_setup_build_mode()

win32/winutil.c

+30-28
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
#include "codepage.h"
2121
#include <bcrypt.h>
2222
#include <lmcons.h>
23-
#include <imagehlp.h>
2423

2524

2625
PHP_WINUTIL_API char *php_win32_error_to_msg(HRESULT error)
@@ -436,26 +435,13 @@ PHP_WINUTIL_API char *php_win32_get_username(void)
436435
return uname;
437436
}/*}}}*/
438437

439-
static zend_always_inline BOOL is_compatible(const char *name, BOOL is_smaller, char *format, char **err)
438+
static zend_always_inline BOOL is_compatible(HMODULE handle, BOOL is_smaller, char *format, char **err)
440439
{/*{{{*/
441-
/* work around ImageLoad() issue */
442-
const char *name_stripped = name;
443-
if (name[0] == '.' && IS_SLASH(name[1])) {
444-
name_stripped += 2;
445-
}
446-
447-
PLOADED_IMAGE img = ImageLoad(name_stripped, NULL);
440+
PIMAGE_DOS_HEADER dosHeader = (PIMAGE_DOS_HEADER) handle;
441+
PIMAGE_NT_HEADERS pNTHeader = (PIMAGE_NT_HEADERS)((char *) dosHeader + dosHeader->e_lfanew);
448442

449-
if (!img) {
450-
DWORD _err = GetLastError();
451-
char *err_txt = php_win32_error_to_msg(_err);
452-
spprintf(err, 0, "Failed to load %s, %s", name, err_txt);
453-
free(err_txt);
454-
return FALSE;
455-
}
456-
457-
DWORD major = img->FileHeader->OptionalHeader.MajorLinkerVersion;
458-
DWORD minor = img->FileHeader->OptionalHeader.MinorLinkerVersion;
443+
DWORD major = pNTHeader->OptionalHeader.MajorLinkerVersion;
444+
DWORD minor = pNTHeader->OptionalHeader.MinorLinkerVersion;
459445

460446
#if PHP_LINKER_MAJOR == 14
461447
/* VS 2015, 2017 and 2019 are binary compatible, but only forward compatible.
@@ -474,23 +460,39 @@ static zend_always_inline BOOL is_compatible(const char *name, BOOL is_smaller,
474460
if (PHP_LINKER_MAJOR != major)
475461
#endif
476462
{
477-
spprintf(err, 0, format, name, major, minor, PHP_LINKER_MAJOR, PHP_LINKER_MINOR);
478-
ImageUnload(img);
463+
char buf[MAX_PATH];
464+
if (GetModuleFileName(handle, buf, sizeof(buf)) != 0) {
465+
spprintf(err, 0, format, buf, major, minor, PHP_LINKER_MAJOR, PHP_LINKER_MINOR);
466+
} else {
467+
spprintf(err, 0, "Can't retrieve the module name (error %u)", GetLastError());
468+
}
479469
return FALSE;
480470
}
481-
ImageUnload(img);
482471

483472
return TRUE;
484473
}/*}}}*/
485474

486-
PHP_WINUTIL_API BOOL php_win32_image_compatible(const char *name, char **err)
475+
PHP_WINUTIL_API BOOL php_win32_image_compatible(HMODULE handle, char **err)
487476
{/*{{{*/
488-
return is_compatible(name, TRUE, "Can't load module '%s' as it's linked with %u.%u, but the core is linked with %d.%d", err);
477+
return is_compatible(handle, TRUE, "Can't load module '%s' as it's linked with %u.%u, but the core is linked with %d.%d", err);
489478
}/*}}}*/
490479

491-
/* Expect a CRT name DLL. */
492-
PHP_WINUTIL_API BOOL php_win32_crt_compatible(const char *name, char **err)
480+
/* Expect a CRT module handle */
481+
PHP_WINUTIL_API BOOL php_win32_crt_compatible(char **err)
493482
{/*{{{*/
494-
return is_compatible(name, FALSE, "'%s' %u.%u is not compatible with this PHP build linked with %d.%d", err);
483+
#if PHP_LINKER_MAJOR == 14
484+
/* Extend for other CRT if needed. */
485+
# if PHP_DEBUG
486+
const char *crt_name = "vcruntime140d.dll";
487+
# else
488+
const char *crt_name = "vcruntime140.dll";
489+
# endif
490+
HMODULE handle = GetModuleHandle(crt_name);
491+
if (handle == NULL) {
492+
spprintf(err, 0, "Can't get handle of module %s (error %u)", crt_name, GetLastError());
493+
return FALSE;
494+
}
495+
return is_compatible(handle, FALSE, "'%s' %u.%u is not compatible with this PHP build linked with %d.%d", err);
496+
#endif
497+
return TRUE;
495498
}/*}}}*/
496-

win32/winutil.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ PHP_WINUTIL_API int php_win32_code_to_errno(unsigned long w32Err);
5353

5454
PHP_WINUTIL_API char *php_win32_get_username(void);
5555

56-
PHP_WINUTIL_API BOOL php_win32_image_compatible(const char *img, char **err);
57-
PHP_WINUTIL_API BOOL php_win32_crt_compatible(const char *img, char **err);
56+
PHP_WINUTIL_API BOOL php_win32_image_compatible(HMODULE handle, char **err);
57+
PHP_WINUTIL_API BOOL php_win32_crt_compatible(char **err);
5858

5959
#endif

0 commit comments

Comments
 (0)