-
Notifications
You must be signed in to change notification settings - Fork 338
PHP-8.0 support #471
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
PHP-8.0 support #471
Conversation
zend/callable.cpp
Outdated
| // is incorrect in their view. another place to put this may be | ||
| // hiding it behind the fname | ||
| _argv[_argc + 1].type = reinterpret_cast<zend_type>(this); | ||
| // _argv[_argc + 1].type = reinterpret_cast<zend_type>(this); |
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.
I couldn't solve this - I don't know what functionality breaks by not fixing it. Its related to the zend_type becoming a struct
| #if PHP_VERSION_ID < 80000 | ||
| info->return_reference = false; | ||
| info->_is_variadic = false; | ||
| info->type = ZEND_TYPE_ENCODE((int)_return, true); |
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.
I wonder if there needs to be a similar line for PHP 8 or not ?
Meaning - I'm wondering if info->type needs some default initialization or not
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.
I was running into various SIGSEV's with functions that utilised pass by reference.
__strchrnul (s=0x202c6e6f69746169 <error: Cannot access memory at address 0x202c6e6f69746169>, c=c@entry=124) at src/string/strchrnul.c:19
19 src/string/strchrnul.c: No such file or directory.
(gdb) backtrace
#0 __strchrnul (s=0x202c6e6f69746169 <error: Cannot access memory at address 0x202c6e6f69746169>, c=c@entry=124) at src/string/strchrnul.c:19
#1 0x00007fee50a5f4f4 in strchr (s=<optimized out>, c=124) at src/string/strchr.c:5
#2 0x000055d31751a242 in zend_register_functions ()
#3 0x000055d31751a7aa in ?? ()
#4 0x00007fee4fed96e5 in Php::ClassImpl::initialize (this=0x7fee5020b7f0, base=0x7fee4f93bcd0, prefix=...) at zend/classimpl.cpp:1457
The following fixed it:
#if PHP_VERSION_ID >= 80000
info->type = (zend_type) ZEND_TYPE_INIT_CODE((int)_return, true, 0);
#endif
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.
Thanks!, I've added it to my PR/patch.
| int retval = 0; | ||
| zend_string *tmp_str; | ||
| tmp_str = zend_string_init(key, size, 0); | ||
| retval = has_property(Z_OBJ_P(_val), tmp_str, 0, nullptr); | ||
| zend_string_release(tmp_str); | ||
| return retval; |
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.
This can probably happen with some way of getting a pointer to the key or something like that but I couldn't find the appropriate PHP macro for it
stmboyd
left a comment
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.
I've left a few comments that should help get this to compile again. However, I feel like having #if all over the place is unwise, and it would be better to do it once at the top of the file with a using statement.
20f2061 to
831c386
Compare
|
Hi @gnat42 . I tried to compile a class using your code but at runtime is failing with the following error The class I have implemented is the following Do you have any ideas what could be the problem? This worked alright in PHP 7.3 |
|
hello @Dringho if you look at #469 @axelusarov submitted a change/fix for this specific issue - I just haven't had a time to update my PR with their fix. |
I added that fix and my class extension compiled and worked :) thank you very much both. |
|
hello @Dringho and @axelusarov I've pushed a new version of the PR that includes the change. If you can make sure it works for you that would be helpful. |
|
I have a crash when trying to execute my code based on this branch Not sure where the cause is. |
|
Si it seems that it is related to the JIT feature added in PHP8, I'm trying to disable the AVX optimizations using |
|
Are there any intentions to merge this? |
| #else | ||
| if ((int)_return) { | ||
| info->type = (zend_type)ZEND_TYPE_INIT_CODE((int)_return, true, 0); | ||
| } |
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.
type needs to be initialized for void functions as well. Otherwise the uninitialized value is used and can lead to early segfaults.
I was able to solve this with
else {
info->type = (zend_type)ZEND_TYPE_INIT_NONE(0);
}
``
|
@edhelas Interestingly I have that same crash on PHP 8.1.9 when compiled against Fedora 36, but don't have the crash when running against PHP 8.1.9 on CentOS 7... I'm investigating. Have you found a solution? |
|
@gnat42 We choose to move to another solution regarding our needs. I might try again but not anytime soon. Sorry. |
|
Ah ok, well in the in-between messaging, I can say that I fixed the crash for my extension by applying the change @flaviozavan suggested. So I will add that to my patch/PR |
|
One more bit of information for people using this patch. Reflection is broken with these changes. For example with the following simple extension and the following php The output is as follows on PHP 8.x. On php 7.x the is Parameter: date, type ?\DateTime... I'm trying to find out what's going on but everything I've seen so far is correct. Anyone with suggestions would be appreciated. |
|
This latest push (Sep 8) fixes the issue. |
|
This has been merged into a maintained fork here: fast-debug#8 |
Initial support for PHP 8.0.