Fixed compilation warnings on big endian system #5373
Conversation
vibhutisawant
force-pushed
the
warning-fixes
branch
from
5e78050 to
6e5377c
Compare
ext/ffi/ffi_parser.c
Outdated
| int ret; | ||
| int accept = -1; | ||
| const unsigned char *accept_pos; | ||
| const unsigned char *accept_pos = NULL; |
There was a problem hiding this comment.
This change is moot, since ffi_parser.c is generated from ffi.g, so the change would likely have to be made at https://github.com/dstogov/llk/blob/70b057f49948a1f5bb37cc5cb1a324605c3903e3/llk_c.php#L262.
ext/pdo/pdo_stmt.c
Outdated
| { | ||
| int flags, idx, old_arg_count = 0; | ||
| zend_class_entry *ce = NULL, *old_ce = NULL; | ||
| zval grp_val, *pgrp, retval, old_ctor_args = {{0}, {{0}}, {0}}; |
There was a problem hiding this comment.
This may be a bug in GCC as to why it reports the missing initializer after digging around a bit:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80454
I wonder if just doing {0} would fix both warnings as this should NULL initialize the whole struct from my (albeit limited) understanding.
Will try to apply this change and run on CI in #5151
There was a problem hiding this comment.
Wouldn't it be cleaner to omit the initializer, and to do ZVAL_UNDEF(old_ctor_args) instead?
There was a problem hiding this comment.
@cmb69 You won't be believe me, but doing that is going to trigger another compiler warning (-Wmaybe-uninitialized). No matter what you do, there's always a warning...
There was a problem hiding this comment.
So I tried just using {0} and Clang complains...
There was a problem hiding this comment.
@Girgias The above mentioned fix throws error on little as well as big endian arch without removing the --enable-werror flag from travis/compile.sh.
ext/pdo/pdo_stmt.c
Outdated
| { | ||
| int flags, idx, old_arg_count = 0; | ||
| zend_class_entry *ce = NULL, *old_ce = NULL; | ||
| zval grp_val, *pgrp, retval, old_ctor_args = {{0}, {{0}}, {0}}; |
There was a problem hiding this comment.
So I tried just using {0} and Clang complains...
|
Commited 446724b and 8300458 so that I can open #5382 to see test failures which arise in some extensions due to the architecture being big endian. @vibhutisawant if you could rebase onto master and force push that would be great :) |
vibhutisawant
force-pushed
the
warning-fixes
branch
from
6e5377c to
170ea72
Compare
|
@Girgias I have done a rebase with master and updated the PR. |
Girgias
left a comment
Girgias
left a comment
There was a problem hiding this comment.
LGTM, although if you could PR the upstream parser lib which generates the FFI parser which would prevent this from reappearing at a later stage.
|
@Girgias the warning thrown due to uninitialized accept_pos is an incorrect warning as @dstogov mentioned in the issue above.Tried using gcc 9.3 to build PHP, however it resulted in new warnings. Also the travis job uses gcc 5.4.0. Since its an intermittent warning, can it be ignored? Or can we set -Wno-error=maybe-uninitialized flag in case of s390x? |
Seems reasonable, I've know GCC8 introduced new warnings in -Wextra that needed to be fixed after I merge #5151 Also can you rebase and drop the FFI change so that I can merge and close this. :) |
|
@vibhutisawant There are no warnings on the Travis build in #5382, so it's not a problem for our CI at least. |
vibhutisawant
force-pushed
the
warning-fixes
branch
from
170ea72 to
17fbd08
Compare
4 participants
This PR is related to bug-79431
Following changes were added as
makecommand fails on big endian architecture(s390x). These changes work fine for little endian architecture too. More details about the same are mentioned in the bug-79431.