-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Disable tsrm_ls_cache usage on Cygwin #16920
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
Conversation
Otherwise, `--enable-opache-jit` won't even build there.
So that means we'd be returning 0 on Cygwin from tsrm_get_ls_cache_tcb_offset
.
But does the JIT actually work? I.e. do we then have the dynamic TLS resolution working? I see a WIN64 case here
php-src/ext/opcache/jit/zend_jit_ir.c
Line 3243 in cd977ae
I have no idea about the details. I haven't used Cygwin for many years (and frankly, I don't see much point in using this at all nowadays, given that there is WSL2). What I know is that a swoole user/developer confirmed that the patch would work for them. And I know that _WIN64
is not defined in Cygwin environments; despite the name, Cygwin is more like a POSIX environment running on Windows without VM.
If JIT does not work at all on Cygwin, we should disable --enable-opcache-jit
for that environment altogether. No need to change C code.
Well I can't test this easily, but usually when a platform isn't supported it's not enough to just stub out the return value, see e.g. #16902 (comment)
I had a closer look, and possibly #16568 is the proper solution (they actually configure --enable-opcache --disable-opcache-jit
). But I wonder whether there may be other consumers of tsrm_get_ls_cache_tcb_offset()
in which case we cannot simply drop that function if JIT is disabled. If there are no other consumers, that function would have better been defined in OPcache.
And thinking about that more, we cannot apply #16568, since that would kill phpize builds of OPcache JIT (possible done by distros).
Not sure what to do.
I would expect it just uses the same TLS mechanism as the win32 subsystem does, so I can try to have a brief look to see how this would play out. Would need to install Cygwin and figure out how that works so see you in a bit I guess...
I think I can do that. Have a little bit of experience with Cygwin at least.
Well I also just got a Cygwin system going, just had to compile&install re2c manually, it's currently compiling php-src.
It also goes wrong with the fiber asm, always fun to see at the end of a slow compile that I had to pass --disable-fiber-asm
...
Well, it might not matter anyway as opcache doesn't seem to load for me (curiously I only get an .a archive file):
$ ./sapi/cli/php -c .
Failed loading /home/niels/php-src/modules/opcache.a: Exec format error
and come to think of it, is the PHP_WIN32 flag even defined? I don't think so. But that would break opcache anyway as we have some #if
guards for ASLR stuff on Windows...
EDIT: we should probably just discourage people from building with Cygwin anyway since this is probably just the tip of the iceberg...
Well, it might not matter anyway as opcache doesn't seem to load for me (curiously I only get an .a archive file):
See https://bugs.php.net/bug.php?id=65497. :)
How did you build? On Cygwin, you can run .bat files, but you're not supposed to do that for native Cygwin builds. So you could do ./buildconf.bat
or ./buildconf
, but you want the latter (like on Linux). If the latter, PHP_WIN32
will not be defined.
Well I also just got a Cygwin system going, just had to compile&install re2c manually
Need to do this now. :)
Hm, so it's not working at all anyway? I'm not sure what to make of this tbh. How are the swoole people running opcache then (on Cygwin)?
How did you build? On Cygwin, you can run .bat files, but you're not supposed to do that for native Cygwin builds. So you could do ./buildconf.bat or ./buildconf, but you want the latter (like on Linux).
./buildconf
./configure --enable-debug --disable-all --enable-opcache --enable-zts --disable-fiber-asm
make -j4
If the latter, PHP_WIN32 will not be defined.
This means opcache will have issues with ASLR that are not properly mitigated.
Hm, so it's not working at all anyway?
Not sure. This was long time ago, and maybe I've just screwed up back then.
How are the swoole people running opcache then (on Cygwin)?
I don't know if they actually run it. The CI was only about building, but no tests.
Then I guess this is enough as I don't think opcache (and therefore the JIT) can work anyway...
I don't think opcache (and therefore the JIT) can work anyway
Well, but why would they even build with OPcache enabled, if it can't be used?
I'm still trying to complete a minimal build on Cygwin. I've also stumbled upon --disable-fiber-asm
(should not be enabled when --disable-all
is given, in my opinion), and now had to install PHP. Latest available version is 7.3.7-2. So much for that.
Well, but why would they even build with OPcache enabled, if it can't be used?
Because they're doing hacky stuff to compile opcache statically:
I don't want to support any of this, they seem that they don't know what they're doing with this code.
Ah, completely forgot about that ticket. Then I agree it's good enough to fix the build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good enough for now
Just in case someone wants to pursue this
Well, it might not matter anyway as opcache doesn't seem to load for me (curiously I only get an .a archive file):
/bin/sh /cygdrive/d/git/php/php-src/libtool --silent --preserve-dup-deps --tag=CC --mode=link cc -shared -I/cygdrive/d/git/php/php-src/include -I/cygdrive/d/git/php/php-src/main -I/cygdrive/d/git/php/php-src -I/cygdrive/d/git/php/php-src/ext/date/lib -I/cygdrive/d/git/php/php-src/TSRM -I/cygdrive/d/git/php/php-src/Zend -D_GNU_SOURCE -fno-common -Wstrict-prototypes -Wformat-truncation -Wlogical-op -Wduplicated-cond -Wno-clobbered -Wall -Wextra -Wno-strict-aliasing -Wno-unused-parameter -Wno-sign-compare -g -O2 -ffp-contract=off -fvisibility=hidden -Wimplicit-fallthrough=1 -DZEND_SIGNALS -o ext/opcache/opcache.la -export-dynamic -avoid-version -prefer-pic -module -rpath /cygdrive/d/git/php/php-src/modules ext/opcache/ZendAccelerator.lo ext/opcache/zend_accelerator_blacklist.lo ext/opcache/zend_accelerator_debug.lo ext/opcache/zend_accelerator_hash.lo ext/opcache/zend_accelerator_module.lo ext/opcache/zend_persist.lo ext/opcache/zend_persist_calc.lo ext/opcache/zend_file_cache.lo ext/opcache/zend_shared_alloc.lo ext/opcache/zend_accelerator_util_funcs.lo ext/opcache/shared_alloc_shm.lo ext/opcache/shared_alloc_mmap.lo ext/opcache/shared_alloc_posix.lo -lrt
libtool: link: warning: undefined symbols not allowed in x86_64-pc-cygwin shared libraries
I can imagine that you would need to link against import libraries, in this case likely php.dll, but there is no such DLL; instead there is a big php.exe.
we should probably just discourage people from building with Cygwin anyway since this is probably just the tip of the iceberg...
Indeed. There's likely a lot of work to do to have a reliable build.
This means opcache will have issues with ASLR that are not properly mitigated.
Didn't think about that yesterday, but if only a single process is involved, there are no ASLR issues (at least preloading could be supported and compilation of internal classes). That is not relevant for CLI, but some SAPIs would benefit; I think FrankenPHP (if ever ported to Windows), maybe Swoole (not sure how it's working), Apache (if we get rid of starting up PHP on the control process; see #7865), and even if users use separate OPcache instances for FCGI (opcache.cache_id
). And there could even be something like FPM which uses worker threads instead of forked children). We should probably replace the preprocessor conditionals with something else, so a prebuilt OPcache could adapt to the environment. I don't like using an INI setting, but don't have a better idea for now.
Otherwise,
--enable-opache-jit
won't even build there.Note that this is supposed to supersede #16568.