-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Attempt to fix bug #79092 (Building with clang+lld-9 results in a broken PHP binary) #5123
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
configure.ac
Outdated
EXTRA_LDFLAGS_PROGRAM="$EXTRA_LDFLAGS_PROGRAM -Wl,-zcommon-page-size=2097152 -Wl,-zmax-page-size=2097152" | ||
AC_MSG_CHECKING(linker support for -zcommon-page-size=2097152) | ||
save_LDFLAGS=$LDFLAGS | ||
$LDFLAGS="$LDFLAGS -Wl,-zcommon-page-size=2097152 -Wl,-zmax-page-size=2097152" |
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 should probably be LDFLAGS="..."
(without the $
).
configure.ac
Outdated
AC_MSG_RESULT([no]) | ||
AC_MSG_CHECKING(linker support for -zmax-page-size=2097152) | ||
save_LDFLAGS=$LDFLAGS | ||
$LDFLAGS="$LDFLAGS -Wl,-zmax-page-size=2097152" |
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.
Same as above (no $
).
Thanks for trying to fix this, but unfortunately it doesn't seem to help. Apparently lld-9 doesn't have any problems generating a valid executable binary with the Could we perhaps add a configure flag (say |
@falken42 thanks for testing and fixes. "configure" also tries to run the compiled program. Could you, please, try to compile and execute this simple program by hand. (may be "configure" use another linker"). "lld" doesn't support "-zcommon-page-size" in the same way as GNU ld and gold. |
@dstogov Interesting, it looks like configure might not be using the specified LDFLAGS for the test executable? (Sorry, I'm not that great at understanding configure scripts.) Building and running the simple test program by hand definitely results in a bad binary:
But with your patches to configure.ac, configure still seems to think it's supported:
|
After checking config.log, it does indeed appear that the LDFLAGS aren't being passed when building the conftest binary. Given the following configure command line:
The relevant section in config.log is here, without
|
@falken42 thanks for investigation. I'll check, how to fix this. |
@falken42 please test. According to config.log it should work now. |
@dstogov Looks like that fixed it, thank you! Here's the relevant lines from configure:
And the resulting php binary executes as expected:
|
Merged into PHP-7.4 as ce44cd3 |
This enables cross-compiling edge cases to override checks with php_cv_have_common_page_size and php_cv_have_max_page_size cache variables when target matches one of the conditions in case pattern. Not done as link check yet due to Clang 9 bug and similar issues: php#5123
This enables cross-compiling edge cases to override checks with php_cv_have_common_page_size and php_cv_have_max_page_size cache variables when target matches one of the conditions in case pattern. Not done as link check yet due to Clang 9 bug and similar issues: #5123
No description provided.