Fix fibers tests on ppc64le #7710
Conversation
|
Filed to see possible regressions |
|
Just to clarify, based on your comment in https://bugs.php.net/bug.php?id=81689 this does not fix the ppc64le issue yet, right? |
|
Yes, this 2 tests still fail, but as I see other places with The whole build log could be found at https://gitlab.alpinelinux.org/alpine/aports/-/jobs/554235 |
| @@ -443,7 +443,7 @@ static ZEND_STACK_ALIGNED void zend_fiber_execute(zend_fiber_transfer *transfer) | |||
| zend_fiber *fiber = EG(active_fiber); | |||
|
|
|||
| /* Determine the current error_reporting ini setting. */ | |||
| zend_long error_reporting = INI_INT("error_reporting"); | |||
| int error_reporting = INI_INT("error_reporting"); | |||
There was a problem hiding this comment.
zend_ini_long() returns zend_long, so why should we use a narrower type here?
There was a problem hiding this comment.
This value is later assigned to EG(error_reporting), an int, so I suggested this change thinking perhaps there was something unusual happening where it wasn't being cast properly on that assignment because it was stored in a long.
Thinking about it further, I'm not sure this is origin of the problem anyway. The problem appears to be with saving and restoring EG(error_reporting), which is done within zend_fiber_capture_vm_state and zend_fiber_restore_vm_state. Given the types there, I'm not seeing why this is failing.
There was a problem hiding this comment.
wondering as git grep INI_INT returns mostly int so it needs separate PR to normalize
btw gonna try 5459ed4 as it the fixed looks related
There was a problem hiding this comment.
but it does not help too
There was a problem hiding this comment.
As tests fixed, the remaining question is clean-up still needed?
There was a problem hiding this comment.
Commit 492af9f adds new function for parsing ints
There was a problem hiding this comment.
Somehow it still needs to fix type or use cast
There was a problem hiding this comment.
At least counters could affect all little-endian arches
There was a problem hiding this comment.
@cmb69 any suggestion to fix it?
|
Bug was fixed via 7773 but the PR still makes sense to fix int/long issue with |
Fixes https://bugs.php.net/bug.php?id=81689