Repository navigation
Build system updates for modern C #10751
Description
Description
Ref: https://wiki.gentoo.org/wiki/Modern_C_porting
clang-16 (imminent) and probably gcc-14 (next yearish) are going to turn some warnings into errors:
- -Werror=implicit-function-declaration
- -Werror=implicit-int
- -Werror=int-conversion (this is in Clang 15, actually)
- -Werror=incompatible-function-pointer-types (GCC does not have a specific equivalent error, use -Werror=incompatible-pointer-types instead when testing)
Issues in the PHP source itself will probably be caught by developers, but issues buried in the build system are more subtle. For example, in ext/iconv/config.m4,
if test -z "$iconv_impl_name"; then
AC_MSG_CHECKING([if using GNU libiconv])
AC_RUN_IFELSE([AC_LANG_SOURCE([[
#include <iconv.h>
int main() {
printf("%d", _libiconv_version);
return 0;
}
]])],[
AC_MSG_RESULT(yes)
iconv_impl_name="gnu_libiconv"
],[
AC_MSG_RESULT(no)
],[
AC_MSG_RESULT([no, cross-compiling])
])
fiThat feature test is missing #include <stdio.h>, so printf() is undefined when it is used. Thus the feature test can fail with clang-16 when it should pass.
The other issue is visible above is,
int main() { ... }That needs the correct signature,
int main(int argc, char** argv) { ... }The same fix is needed in a few other m4 files, but is relatively easy to grep for.
PHP Version
8.2.3
Operating System
No response
Activity
mvorisek commentedon Mar 2, 2023
maybe we want to assert some configure checks using Github CI?
Girgias commentedon Mar 3, 2023
If those compiler flags are enabled via
CFLAGS=would this trigger the warnings or not? Could you also provide a PR (if not already done so)?orlitzky commentedon Mar 3, 2023
Yes, adding those flags to
$CFLAGSis usually enough to reproduce the problem.I can handle the PR. I would have just done that in the first place but I've been swamped this week and didn't want to forget about it.
A CI check might be a good idea. This was caught by a CI check on Gentoo. However that check also turned up a handful of false positives, where a function is "implicitly declared" in the
./configuretest because it doesn't actually exist in the header. Which is of course expected if you're looking inside some header for a function and it isn't there. That was for-Werror=implicit-function-declaration, but the other errors/warnings might be more suitable for grepping without causing false positives; I'd have to think about it.orlitzky commentedon Mar 5, 2023
PR at #10785
Using gcc-12 with
grepping
config.log(with the default build options) hits only-Wimplicit-function-declaration. Those look like false positives and I don't know what to do about them.The others, you could probably just add a grep to the CI. If the warnings aren't triggered now, rejecting code that does trigger them can at worst make everybody stop and think. But I can't imagine (right now...) a reason why you would want to write standards non-compliant autoconf tests for anything other than
implicit-function-definition.*/*.m4: update main() signatures.
ext/iconv/config.m4: add missing stdio.h include.
Girgias commentedon Mar 7, 2023
Thank you!
andypost commentedon Mar 25, 2023
The issue incomplete, imap also needs work, just faced https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/45389#note_298190
Girgias commentedon Mar 26, 2023
Please open up a new issue
andypost commentedon Mar 26, 2023
Filed #10947
andypost commentedon Dec 15, 2023
Is it possible to backport to 8.2 branch?