Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MinGW linker call php8ts.dll php8embed.dll that compiled out by MSVC. #17476

Open
TenHian opened this issue Jan 15, 2025 · 12 comments
Open

MinGW linker call php8ts.dll php8embed.dll that compiled out by MSVC. #17476

TenHian opened this issue Jan 15, 2025 · 12 comments

Comments

@TenHian
Copy link

TenHian commented Jan 15, 2025

Description

Recently I was porting FrankenPHP to windows. It is a web server code in GO and C. GO can only call GNU C code. On Windows its MinGW.
I've tried to compile PHP on Windows with MinGW, and find it out that it even have no rules for organise a compilation.

Cause PHP on Windows only support for MSVC officially. Of course there's also Clang and ICC, but MSVC is main stream. And I find that MinGW-w64 linker could link the .dll that MSVC out. It's pretty versatile, except for some MSVC features that lead compile-time-error.
So, I've tried to change code in php-8.3.0, get rid of somethings like “__vectorcall” "__forceinline", and change the Makefile to make php8embed.dll out. And then, MinGW-w64 gcc could link the php8ts.dll and php8embed.dll that MSVC out. That makes FrankenPHP could be port to windows.

I don't know the feature that make the dll compiled by MSVC callable by MinGW, will this be accepted by the mainline.
Here is changes on php-8.3.0, see it as a demo. changes

@TenHian
Copy link
Author

TenHian commented Jan 15, 2025

@cmb69

@cmb69
Copy link
Member

cmb69 commented Jan 15, 2025

Recently I was porting FrankenPHP to windows.

👍

GO can only call GNU C code.

Hmm, really? Are there limitations regarding the FFI? Could these be resolved otherwise (e.g. patching the FFI)? Is it using libffi?

Cause PHP on Windows only support for MSVC officially. Of course there's also Clang and ICC, but MSVC is main stream.

Right. Clang support is basically available (I'm working on some improvements); don't know about ICC support (might be completely broken).

But of course, you could try to build with MinGW/MSYS via the autotools build chain (as far as I know, the swoole developers do that with Cygwin). I've tried that recently, but besides that this is super slow, it doesn't work; I think a lot of adjustments would have to be made to php-src to properly support this.

So, I've tried to change code in php-8.3.0, get rid of somethings like “__vectorcall” "__forceinline", […]

I understand that __vectorcall might need to be changed, but where is the problem with __forceinline? That should not be of concern for the linker.

[…] and change the Makefile to make php8embed.dll out.

In my opinion, being able to build embed as shared library makes generally sense. We already do this with for phpdbg (--enable-phpdbg vs. --enable-phpdbgs) (we just should not support both for a single configuration, since the result has duplicate build rules, which are not necessarily supported by something else than NMake; and even NMake complains about that).

For reference: dunglas/frankenphp#420. It seems there was some progress using https://github.com/crazywhalecc/static-php-cli. But it looks like you've managed to get a step further.

@cmb69
Copy link
Member

cmb69 commented Jan 15, 2025

Possibly a first step: #17480.

@cmb69
Copy link
Member

cmb69 commented Jan 16, 2025

Next baby step: #17482.

@TenHian
Copy link
Author

TenHian commented Jan 16, 2025

But of course, you could try to build with MinGW/MSYS via the autotools build chain (as far as I know, the swoole developers do that with Cygwin). I've tried that recently, but besides that this is super slow, it doesn't work; I think a lot of adjustments would have to be made to php-src to properly support this.

Yep, I surely have tried those ways.
(1) Build php with MinGW-w64/MSYS2 by autotools build chain on Windows.
Unfortunately config.w32.h is missing during make, but configure doesn't generate it. I had a cursory look and config.w32.h is highly bound to MSVC. Compiling with MinGW should be in a completely broken state.
(2) Build with Cygwin.

I've tried that recently, but besides that this is super slow, it doesn't work;

In addition to what you said, I'm also having problems compiling libphp.so with linking WinMain instead of main.

libs/libphp.bundle: $(PHP_GLOBAL_OBJS) $(PHP_SAPI_OBJS)
	$(CC) $(MH_BUNDLE_FLAGS) $(CFLAGS_CLEAN) $(EXTRA_CFLAGS) $(LDFLAGS) $(EXTRA_LDFLAGS) $(PHP_GLOBAL_OBJS:.lo=.o) $(PHP_SAPI_OBJS:.lo=.o) $(PHP_FRAMEWORKS) $(EXTRA_LIBS) $(ZEND_EXTRA_LIBS) -o $@ && cp $@ libs/libphp.so

@TenHian
Copy link
Author

TenHian commented Jan 16, 2025

Possibly a first step: #17480.

I' ve tried --enable-embed=shared, but dont see php8embed.dll out.

Next baby step: #17482.

Could it be like that?

# elif defined(_MSC_VER) && defined(_A_MACRO)
#  define zend_always_inline inline
#  define zend_never_inline __declspec(noinline)
# elif defined(_MSC_VER) && !defined(_A_MACRO)
#  define zend_always_inline __forceinline
#  define zend_never_inline __declspec(noinline)
# else

It seems like __declspec(noinline) could be accepted by MinGW, but not support __forceinline, weird.

@cmb69
Copy link
Member

cmb69 commented Jan 16, 2025

I' ve tried --enable-embed=shared, but dont see php8embed.dll out.

Hmm, just checked again, this time with configure --enable-snapshot-build --enable-embed=shared, and after nmake:

$ dumpbin /exports x64\Release_TS\php8embed.dll | grep php_embed
          1    0 00001000 php_embed_init = php_embed_init
          2    1 00005000 php_embed_module = php_embed_module
          3    2 000011C0 php_embed_shutdown = php_embed_shutdown

Did you get any relevant errors during configure or nmake?

Also check the generated Makefile; the value of the variable SAPI_TARGETS should contain php8embed.dll, not php8embed.lib.

# elif defined(_MSC_VER) && defined(_A_MACRO)
#  define zend_always_inline inline
#  define zend_never_inline __declspec(noinline)
# elif defined(_MSC_VER) && !defined(_A_MACRO)
#  define zend_always_inline __forceinline
#  define zend_never_inline __declspec(noinline)
# else

Hm, I don't quite understand why it wouldn't work as is; shouldn't the following be chosen by MinGW GCC (it is for me):

# if defined(__GNUC__)
# if __GNUC__ >= 3
# define zend_always_inline inline __attribute__((always_inline))
# define zend_never_inline __attribute__((noinline))
# else

And _MSC_VER shouldn't even be defined when building with MinGW GCC.

Regarding the general build issues with MinGW: I think this is generally broken. The Windows build chain (based on confutils.js) only supports MSVC and native Clang (and maybe ICC), while the autotools toolchain likely falls about our C code, where we sometimes use _MSC_VER, ZEND_WIN32 and _WIN32 etc. pretty arbitrarily, and may not even consider some files (mostly in win32/) although we probably should. Thinking about this, it seems a lot of effort to support MinGW builds, and I'm not sure it's worth it. Alone wrt file paths, it is a difficult environment as it supports both Windows style filenames as well as POSIX style.

Mixing the tooling, like you did with using an MSVC build with MinGW, seems easier on the surface, but I would expect issues down the road (stability and security related, at least). Not sure what to do about that.

Isn't there a way to build FrankenPHP with native Windows tooling (MSVC or Clang)? Looking at https://frankenphp.dev/docs/compile/, I fail to understand where https://github.com/crazywhalecc/static-php-cli would even be helpful.

@TenHian
Copy link
Author

TenHian commented Jan 17, 2025

Hmm, just checked again, this time with configure --enable-snapshot-build --enable-embed=shared, and after nmake:

It's been compiled out. It should compile out stably, which is no problem. Definitely there was a mistake in my previous step, not sure which one though, deleted and redid it once and it was fine.

I understand that __vectorcall might need to be changed, but where is the problem with __forceinline? That should not be of concern for the linker.

On this point I was completely wrong, bittersweet haha. The error about __forceinline is entirely caused by MinGW-w64 gcc's preprocessor dealing with bare __forceinline in header files like win32/codepage.h. This problem has been completely fixed in #17482 .

Regarding the general build issues with MinGW: I think this is generally broken. The Windows build chain (based on confutils.js) only supports MSVC and native Clang (and maybe ICC), while the autotools toolchain likely falls about our C code, where we sometimes use _MSC_VER, ZEND_WIN32 and _WIN32 etc. pretty arbitrarily, and may not even consider some files (mostly in win32/) although we probably should. Thinking about this, it seems a lot of effort to support MinGW builds, and I'm not sure it's worth it. Alone wrt file paths, it is a difficult environment as it supports both Windows style filenames as well as POSIX style.

Surely a lot of effort to and it doesnt worth till now.

Mixing the tooling, like you did with using an MSVC build with MinGW, seems easier on the surface, but I would expect issues down the road (stability and security related, at least). Not sure what to do about that.

I've done some testing at this point and found that this approach is not that unreliable. The methodology for the tests done earlier was to run the .phpt under php-src using frankenphp's cli mode. It turned out surprisingly well. Since the previous report wasn't saved, I was thinking of compiling the php cli api through MinGW-w64 for executing these .phpt to get the passing rate. Since the main functions of php are in php8ts.dll, there shouldn't be a big problem if there's no problem with the calls. As for security I don't know where to look.

@cmb69
Copy link
Member

cmb69 commented Jan 17, 2025

Thank you for further checking!

I had a closer look at building FrankenPHP, and found that lack of "native" building is a general issue of CGO, which assumes a MinGW environment on Windows. It doesn't look like there is a way around; might be able to use a cl.exe wrapper for compilation, but linking is unlikely to be fixable.

So I've tried to build FrankenPHP using Windows Go binaries and MinGW binaries without using MSYS, and finally succeeded to get a frankenphp.exe. However, apparently it doesn't work; might be me; had serious issues with Caddy already. Such a build chain might be beneficial, since it could more easily use the same dependencies as possibly some other PHP extensions.

Anyhow, I had issues with the included zend_atomic.h; didn't compile because of _InterlockedExchange8 etc. being undefined. I've applied a patch, but in hindsight:

  • I think the patch still makes sense (a bit cleaner than it is now)
  • there shouldn't have been the need for the patch; my environment apparently used the wrong headers
  • still, this would be a good example of lurking issues with the mixed build chains due to ABI incompatibilities

I'll try to look into this further. One point to watch out are the calling conventions. The need to remove/replace __vectorcall would likely be a serious issue if FrankenPHP calls any such function; that likely only works for some cases, due to differences between the default x64 calling convention and __vectorcall. And x86 and ARM64 might face similar issues.

@cmb69
Copy link
Member

cmb69 commented Jan 17, 2025

However, apparently it doesn't work; might be me; had serious issues with Caddy already.

I've managed to get a basically working frankenphp.exe (only minimal testing with phpinfo() for now) with https://jmeubank.github.io/tdm-gcc/articles/2021-05/10.3.0-release. Possibly the relevant difference is that this already links with a pre-built MinGW libpthread, while my former build linked against a self-built libpthread. Might not matter in practice for now, since there are apparently incompatibilites between FrankenPHP and ext/parallel anyway (and the latter might be the only relevant PHP extension for use with FrankenPHP using pthreads on Windows).

The zend_atomic.h issues still give me a head-ache. The Interlocked* functions are apparently MSVC intrinsics, so there not available with MinGW gcc. Using C11 atomics for MSVC would likely not be helpful for portability (if that's even properly supported now; need to check), since (MinGW) gcc apparently does not support these. Might not be a real issue in practice, though.

And there is missing support for zend_max_execution_timers on Windows. Not sure how relevant that would be for FrankenPHP.

@cmb69
Copy link
Member

cmb69 commented Jan 17, 2025

Ugh, there appears to be a pretty serious issue that I had ignored so far:

C:\php-sdk\phpdev\vs17\x64\php-src\TSRM/tsrm_win32.h:69:1: warning: 'thread' attribute directive ignored [-Wattributes]
   69 | TSRMLS_CACHE_EXTERN()

Since we're building with MinGW gcc TSRM_WIN32 is defined, and that causes

php-src/TSRM/TSRM.h

Lines 145 to 149 in dd42ebe

#ifdef TSRM_WIN32
# define TSRM_TLS __declspec(thread)
#else
# define TSRM_TLS __thread
#endif

to go with __declspec(thread); however, that is not understood by gcc, and apparently just dropped. I suppose that causes thread-safety issues.

A simple fix:

 TSRM/TSRM.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/TSRM/TSRM.h b/TSRM/TSRM.h
index 80d6cbad04..7837c04201 100644
--- a/TSRM/TSRM.h
+++ b/TSRM/TSRM.h
@@ -142,7 +142,7 @@ TSRM_API bool tsrm_is_shutdown(void);
 TSRM_API const char *tsrm_api_name(void);
 TSRM_API bool tsrm_is_managed_thread(void);
 
-#ifdef TSRM_WIN32
+#if defined(TSRM_WIN32) && defined(_MSC_VER)
 # define TSRM_TLS __declspec(thread)
 #else
 # define TSRM_TLS __thread

might be sufficient, but I'm not sure. Obviously, the warning is gone this way, but mixing Windows threads and pthreads might not be the best idea.

@TenHian
Copy link
Author

TenHian commented Jan 20, 2025

The zend_atomic.h issues still give me a head-ache. The Interlocked* functions are apparently MSVC intrinsics, so there not available with MinGW gcc. Using C11 atomics for MSVC would likely not be helpful for portability (if that's even properly supported now; need to check), since (MinGW) gcc apparently does not support these. Might not be a real issue in practice, though.

The _InterlockedExchange implemented in /msys64/mingw64/include/psdk_inc/intrin-impl.h

#if !__has_builtin(_InterlockedExchange)
__INTRINSICS_USEINLINE 
__LONG32 _InterlockedExchange(__LONG32 volatile *Target, __LONG32 Value) {
    return __sync_lock_test_and_set(Target, Value);
}
#endif

I'm having trouble understanding that it implements _InterlockedExchange and _InterlockedExchange64, but not _InterlockedExchange8. Maybe like this:

#if !__has_builtin(_InterlockedExchange8)
__INTRINSICS_USEINLINE 
char _InterlockedExchange8(char volatile *Target, char Value) {
    return __sync_fetch_and_store_1(Target, Value);
}
#endif

But MinGW-w64 not implements __sync_fetch_and_store_1, so i get a inline asm:

char _InterlockedExchange8(volatile char *Target, char Value) {
    __asm__ __volatile__(
      "lock xchgl %%eax, (%1)\n"
      : "=a" (Value), "+m" (*Target) : "1" (Value) : "memory"
    );
    return Value;
}

But LDFLAGS has -pie.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants