Skip to content
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

Use AX_CHECK_COMPILE_FLAG macro to check for -fvisibility=hidden #2124

Merged
merged 2 commits into from
Jan 6, 2017
Merged

Use AX_CHECK_COMPILE_FLAG macro to check for -fvisibility=hidden #2124

merged 2 commits into from
Jan 6, 2017

Conversation

orlitzky
Copy link
Contributor

The existing check for -fvisibility=hidden support came from a time
when only GCC had it. The test for it used a regular expression to
parse the GCC major version from the output of $CC --version, and
would look for version 4 or greater.

The regular expression used to accomplish this is doomed, however,
since GCC can be built with a custom version string
(--with-pkgversion). Moreover, the CC variable can be set to
something that confuses it but is otherwise valid. For example, it
would fail with CC=x86_64-pc-linux-gnu-gcc.

This commit fixes two aspects of the feature test. First, it no longer
limits the test to GCC. At least clang now supports the flag, and can
make use of it when GCC is its backend. Second, support for the flag
is tested directly, by attempting to pass it to the compiler, rather
than by parsing its version string. The latter is accomplished with a
new macro, AX_CHECK_COMPILE_FLAG, taken from the autoconf archive.
The new macro has been added to acinclude.m4, and the test stanza in
configure.in has been replaced with a single call to the new macro.

Note that the new macro calls AC_PREREQ(2.64) -- a more stringent
requirement than the existing AC_PREREQ(2.59) in configure.in. The
difference represents about six years of autoconf releases, from v2.59
in December of 2003 to v2.64 in July of 2009.

This problem was noticed by Brian Evans, who also suggested the fix.

PHP-Bug: 73062

…ort.

The existing check for -fvisibility=hidden support came from a time
when only GCC had it. The test for it used a regular expression to
parse the GCC major version from the output of `$CC --version`, and
would look for version 4 or greater.

The regular expression used to accomplish this is doomed, however,
since GCC can be built with a custom version string
(--with-pkgversion). Moreover, the $CC variable can be set to
something that confuses it but is otherwise valid. For example, it
would fail with CC=x86_64-pc-linux-gnu-gcc.

This commit fixes two aspects of the feature test. First, it no longer
limits the test to GCC. At least clang now supports the flag, and can
make use of it when GCC is its backend. Second, support for the flag
is tested directly, by attempting to pass it to the compiler, rather
than by parsing its version string. The latter is accomplished with a
new macro, AX_CHECK_COMPILE_FLAG, taken from the autoconf archive.
The new macro has been added to acinclude.m4, and the test stanza in
configure.in has been replaced with a single call to the new macro.

Note that the new macro calls AC_PREREQ(2.64) -- a more stringent
requirement than the existing AC_PREREQ(2.59) in configure.in. The
difference represents about six years of autoconf releases, from v2.59
in December of 2003 to v2.64 in July of 2009.

This problem was noticed by Brian Evans, who also suggested the fix.

PHP-Bug: 73062
@bukka
Copy link
Member

bukka commented Sep 14, 2016

I'm not sure if we can actually use it due to license of AX_CHECK_COMPILE_FLAG:

http://git.savannah.gnu.org/gitweb/?p=autoconf-archive.git;a=blob_plain;f=m4/ax_check_compile_flag.m4

It looks to me that it is GPL code but I'm not sure either what consequences it could have... :)

@orlitzky
Copy link
Contributor Author

Good catch. There are a number of files in the repository that have a non-PHP license block at the top, indicating that their licenses differ from the default PHP-3.01 license. In particular, there are other autoconf macros with a GPL license like AX_CHECK_COMPILE_FLAG has; for example, TSRM/m4/gethostbyname.m4.

It would be better to ship ax_check_compile_flag.m4 separately. But where should I put that file? Should I create a top-level m4 directory, then then use something like m4_include to load it?

In commit 086f9ad, I added a new macro AX_CHECK_COMPILE_FLAG from the
autoconf archive. Jakub Zelenka pointed out that the license of the
macro (GPL-3+ with exception) does not agree with the license of PHP
itself (PHP-3.01). We should therefore ship the macro in a separate
file with its own license header. That is allowed and is done for
many other files in the PHP repository.

This commit adds a new top-level "m4" directory and places the
upstream ax_check_compile_flag.m4 file in it. The macro is no longer
inlined; instead, our acinclude.m4 now includes the aforementioned
file with m4_include.

PHP-Bug: 73062
Pull-Request: 2124
@php-pulls php-pulls merged commit 672178f into php:master Jan 6, 2017
@krakjoe
Copy link
Member

krakjoe commented Jan 6, 2017

Merged 9e18b6e

Thanks :)

@krakjoe
Copy link
Member

krakjoe commented Jan 6, 2017

This breaks some stuff ... pecl extensions can't compile because they are looking for the m4.

I'm on it ...

php-pulls pushed a commit that referenced this pull request Jan 6, 2017
php-pulls pushed a commit that referenced this pull request Jan 6, 2017
php-pulls pushed a commit that referenced this pull request Jan 6, 2017
php-pulls pushed a commit that referenced this pull request Jan 6, 2017
@krakjoe
Copy link
Member

krakjoe commented Jan 6, 2017

Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants