Fix GH-13727: PHP_TEST_BUILD macro generating invalid call test proto… #13732

devnexen · 2024-03-16T17:01:40Z

…types.

petk · 2024-03-17T00:46:12Z

That's fine with me. Except there are a lot of these to fix or to set properly ... Entire Autoconf actually :D So, I'm not sure this will fix all the issues currently present or if fixing one item in PHP-8.2 branch just like that is sufficient.

Edit: I'd then suggest to replace all these main() to main(void) and similar prototypes, and where possible use the AC_LANG_PROGRAM. With Autoconf 2.72 many of these are already fixed but still not all. So this also won't realistically cause issues with compilers out there in at least a year or even more. But, yes, we'll need to address this issue also.

petk · 2024-03-17T08:02:03Z

In PHP-8.3 and master branches this is already mostly fixed. There aren't that many of these actually. In PHP-8.3 and master branches there will be few minor merge conflicts but this will at least work a bit better with Autotools 2.72 where installed. For older installations, this won't be an issue anyway since the compilers don't have this set by default yet.

Let's do this one in the PHP-8.2 branch:

diff --git a/TSRM/threads.m4 b/TSRM/threads.m4
index dc5719bbec..c42b328dd6 100644
--- a/TSRM/threads.m4
+++ b/TSRM/threads.m4
@@ -73,7 +73,7 @@ void *thread_routine(void *data) {
     return data;
 }
 
-int main() {
+int main(void) {
     pthread_t thd;
     pthread_mutexattr_t mattr;
     int data = 1;
diff --git a/Zend/Zend.m4 b/Zend/Zend.m4
index 06b19c85c7..e7a6603ddb 100644
--- a/Zend/Zend.m4
+++ b/Zend/Zend.m4
@@ -220,7 +220,7 @@ typedef union _mm_align_test {
 #define ZEND_MM_ALIGNMENT (sizeof(mm_align_test))
 #endif
 
-int main()
+int main(void)
 {
   size_t i = ZEND_MM_ALIGNMENT;
   int zeros = 0;
diff --git a/build/php.m4 b/build/php.m4
index 6ebc54b1b4..2bc0733a97 100644
--- a/build/php.m4
+++ b/build/php.m4
@@ -1034,7 +1034,7 @@ AC_DEFUN([_PHP_CHECK_SIZEOF], [
 #endif
 $3
 
-int main()
+int main(void)
 {
 	FILE *fp = fopen("conftestval", "w");
 	if (!fp) return(1);
@@ -1102,7 +1102,7 @@ AC_CACHE_CHECK(for type of reentrant time-related functions, ac_cv_time_r_type,[
 AC_RUN_IFELSE([AC_LANG_SOURCE([[
 #include <time.h>
 
-int main() {
+int main(void) {
 char buf[27];
 struct tm t;
 time_t old = 0;
@@ -1118,7 +1118,7 @@ return (1);
 ],[
   AC_RUN_IFELSE([AC_LANG_SOURCE([[
 #include <time.h>
-int main() {
+int main(void) {
   struct tm t, *s;
   time_t old = 0;
   char buf[27], *p;
@@ -1159,7 +1159,7 @@ AC_DEFUN([PHP_DOES_PWRITE_WORK],[
 #include <errno.h>
 #include <stdlib.h>
 $1
-    int main() {
+    int main(void) {
     int fd = open("conftest_in", O_WRONLY|O_CREAT, 0600);
 
     if (fd < 0) return 1;
@@ -1193,7 +1193,7 @@ AC_DEFUN([PHP_DOES_PREAD_WORK],[
 #include <errno.h>
 #include <stdlib.h>
 $1
-    int main() {
+    int main(void) {
     char buf[3];
     int fd = open("conftest_in", O_RDONLY);
     if (fd < 0) return 1;
@@ -1261,27 +1261,27 @@ dnl PHP_MISSING_TIME_R_DECL
 dnl
 AC_DEFUN([PHP_MISSING_TIME_R_DECL],[
   AC_MSG_CHECKING([for missing declarations of reentrant functions])
-  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <time.h>]], [[struct tm *(*func)() = localtime_r]])],[
+  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <time.h>]], [[struct tm *(*func)(void) = localtime_r]])],[
     :
   ],[
     AC_DEFINE(MISSING_LOCALTIME_R_DECL,1,[Whether localtime_r is declared])
   ])
-  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <time.h>]], [[struct tm *(*func)() = gmtime_r]])],[
+  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <time.h>]], [[struct tm *(*func)(void) = gmtime_r]])],[
     :
   ],[
     AC_DEFINE(MISSING_GMTIME_R_DECL,1,[Whether gmtime_r is declared])
   ])
-  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <time.h>]], [[char *(*func)() = asctime_r]])],[
+  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <time.h>]], [[char *(*func)(void) = asctime_r]])],[
     :
   ],[
     AC_DEFINE(MISSING_ASCTIME_R_DECL,1,[Whether asctime_r is declared])
   ])
-  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <time.h>]], [[char *(*func)() = ctime_r]])],[
+  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <time.h>]], [[char *(*func)(void) = ctime_r]])],[
     :
   ],[
     AC_DEFINE(MISSING_CTIME_R_DECL,1,[Whether ctime_r is declared])
   ])
-  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <string.h>]], [[char *(*func)() = strtok_r]])],[
+  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <string.h>]], [[char *(*func)(void) = strtok_r]])],[
     :
   ],[
     AC_DEFINE(MISSING_STRTOK_R_DECL,1,[Whether strtok_r is declared])
@@ -1315,7 +1315,7 @@ dnl See if we have broken header files like SunOS has.
 dnl
 AC_DEFUN([PHP_MISSING_FCLOSE_DECL],[
   AC_MSG_CHECKING([for fclose declaration])
-  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <stdio.h>]], [[int (*func)() = fclose]])],[
+  AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include <stdio.h>]], [[int (*func)(void) = fclose]])],[
     AC_DEFINE(MISSING_FCLOSE_DECL,0,[ ])
     AC_MSG_RESULT([ok])
   ],[
@@ -1405,7 +1405,7 @@ struct s
   int i;
   char c[1];
 };
-int main()
+int main(void)
 {
   struct s *s = malloc(sizeof(struct s) + 3);
   s->i = 3;
@@ -1463,7 +1463,7 @@ int seeker(void *cookie, off64_t *position, int whence)
 
 cookie_io_functions_t funcs = {reader, writer, seeker, closer};
 
-int main() {
+int main(void) {
   struct cookiedata g = { 0 };
   FILE *fp = fopencookie(&g, "r", funcs);
 
@@ -1590,7 +1590,7 @@ AC_DEFUN([PHP_CHECK_FUNC_LIB],[
   if test "$found" = "yes"; then
     ac_libs=$LIBS
     LIBS="$LIBS -l$2"
-    AC_RUN_IFELSE([AC_LANG_SOURCE([[int main() { return (0); }]])],[found=yes],[found=no],[
+    AC_RUN_IFELSE([AC_LANG_PROGRAM()],[found=yes],[found=no],[
       dnl Cross compilation.
       found=yes
     ])
@@ -1643,8 +1643,8 @@ AC_DEFUN([PHP_TEST_BUILD], [
   LIBS="$4 $LIBS"
   AC_LINK_IFELSE([AC_LANG_SOURCE([[
     $5
-    char $1();
-    int main() {
+    char $1(void);
+    int main(void) {
       $1();
       return 0;
     }
@@ -2296,7 +2296,7 @@ AC_DEFUN([PHP_TEST_WRITE_STDOUT],[
 
 #define TEXT "This is the test message -- "
 
-int main()
+int main(void)
 {
   int n;
 
diff --git a/configure.ac b/configure.ac
index 701e6d5b96..de0eac8498 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1120,7 +1120,7 @@ case $host_alias in
     save_LDFLAGS=$LDFLAGS
     LDFLAGS="$LDFLAGS -Wl,-zcommon-page-size=2097152 -Wl,-zmax-page-size=2097152"
     AC_RUN_IFELSE(
-        [AC_LANG_SOURCE([[int main() {return 0;}]])],
+        [AC_LANG_PROGRAM()],
         [ac_cv_common_page_size=yes],
         [ac_cv_common_page_size=no],
         [ac_cv_common_page_size=no])
@@ -1134,7 +1134,7 @@ case $host_alias in
         save_LDFLAGS=$LDFLAGS
         LDFLAGS="$LDFLAGS -Wl,-zmax-page-size=2097152"
         AC_RUN_IFELSE(
-            [AC_LANG_SOURCE([[int main() {return 0;}]])],
+            [AC_LANG_PROGRAM()],
             [ac_cv_max_page_size=yes],
             [ac_cv_max_page_size=no],
             [ac_cv_max_page_size=no])
diff --git a/ext/gd/config.m4 b/ext/gd/config.m4
index 9bdd6bb11c..a8b1c76d2d 100644
--- a/ext/gd/config.m4
+++ b/ext/gd/config.m4
@@ -239,7 +239,7 @@ dnl Various checks for GD features
 
     PHP_TEST_BUILD(foobar, [], [
       AC_MSG_ERROR([GD build test failed. Please check the config.log for details.])
-    ], [ $GD_SHARED_LIBADD ], [char foobar () {}])
+    ], [ $GD_SHARED_LIBADD ], [char foobar (void) {}])
 
   else
     extra_sources="gd_compat.c"
diff --git a/ext/iconv/config.m4 b/ext/iconv/config.m4
index ac57c81e7d..5d408de833 100644
--- a/ext/iconv/config.m4
+++ b/ext/iconv/config.m4
@@ -30,7 +30,7 @@ if test "$PHP_ICONV" != "no"; then
       AC_MSG_CHECKING([if using GNU libiconv])
       AC_RUN_IFELSE([AC_LANG_SOURCE([[
 #include <iconv.h>
-int main() {
+int main(void) {
   printf("%d", _libiconv_version);
   return 0;
 }
@@ -90,7 +90,7 @@ int main() {
 #include <iconv.h>
 #include <errno.h>
 
-int main() {
+int main(void) {
   iconv_t cd;
   cd = iconv_open( "*blahblah*", "*blahblahblah*" );
   if (cd == (iconv_t)(-1)) {
@@ -117,7 +117,7 @@ int main() {
 #include <iconv.h>
 #include <stdlib.h>
 
-int main() {
+int main(void) {
   iconv_t cd = iconv_open( "UTF-8//IGNORE", "UTF-8" );
   if(cd == (iconv_t)-1) {
     return 1;
diff --git a/ext/opcache/config.m4 b/ext/opcache/config.m4
index d35efbc689..6bf07ad35e 100644
--- a/ext/opcache/config.m4
+++ b/ext/opcache/config.m4
@@ -121,7 +121,7 @@ if test "$PHP_OPCACHE" != "no"; then
 #include <unistd.h>
 #include <string.h>
 
-int main() {
+int main(void) {
   pid_t pid;
   int status;
   int ipc_id;
@@ -200,7 +200,7 @@ int main() {
 # define MAP_FAILED ((void*)-1)
 #endif
 
-int main() {
+int main(void) {
   pid_t pid;
   int status;
   char *shm;
@@ -262,7 +262,7 @@ int main() {
 # define MAP_FAILED ((void*)-1)
 #endif
 
-int main() {
+int main(void) {
   pid_t pid;
   int status;
   int fd;
diff --git a/ext/standard/config.m4 b/ext/standard/config.m4
index 8e77e75134..1f25edb9ee 100644
--- a/ext/standard/config.m4
+++ b/ext/standard/config.m4
@@ -81,7 +81,7 @@ if test "$PHP_EXTERNAL_LIBCRYPT" != "no"; then
 #include <stdlib.h>
 #include <string.h>
 
-int main() {
+int main(void) {
 #if HAVE_CRYPT
 	char *encrypted = crypt("rasmuslerdorf","rl");
 	return !encrypted || strcmp(encrypted,"rl.3StKT.4T8M");
@@ -111,7 +111,7 @@ int main() {
 #include <stdlib.h>
 #include <string.h>
 
-int main() {
+int main(void) {
 #if HAVE_CRYPT
 	char *encrypted = crypt("rasmuslerdorf","_J9..rasm");
 	return !encrypted || strcmp(encrypted,"_J9..rasmBYk8r9AiWNc");
@@ -141,7 +141,7 @@ int main() {
 #include <stdlib.h>
 #include <string.h>
 
-int main() {
+int main(void) {
 #if HAVE_CRYPT
 	char salt[15], answer[40];
 	char *encrypted;
@@ -181,7 +181,7 @@ int main() {
 #include <stdlib.h>
 #include <string.h>
 
-int main() {
+int main(void) {
 #if HAVE_CRYPT
 	char salt[30], answer[70];
 	char *encrypted;
@@ -218,7 +218,7 @@ int main() {
 #include <stdlib.h>
 #include <string.h>
 
-int main() {
+int main(void) {
 #if HAVE_CRYPT
 	char salt[21], answer[21+86];
 	char *encrypted;
@@ -254,7 +254,7 @@ int main() {
 #include <stdlib.h>
 #include <string.h>
 
-int main() {
+int main(void) {
 #if HAVE_CRYPT
 	char salt[21], answer[21+43];
 	char *encrypted;
diff --git a/sapi/fpm/config.m4 b/sapi/fpm/config.m4
index aa2a85d2ca..0a44003ff5 100644
--- a/sapi/fpm/config.m4
+++ b/sapi/fpm/config.m4
@@ -90,7 +90,7 @@ AC_DEFUN([AC_FPM_CLOCK],
       #include <mach/clock.h>
       #include <mach/mach_error.h>
 
-      int main()
+      int main(void)
       {
         kern_return_t ret; clock_serv_t aClock; mach_timespec_t aTime;
         ret = host_get_clock_service(mach_host_self(), REALTIME_CLOCK, &aClock);
@@ -158,7 +158,7 @@ AC_DEFUN([AC_FPM_TRACE],
       #define PTRACE_PEEKDATA PT_READ_D
       #endif
 
-      int main()
+      int main(void)
       {
         long v1 = (unsigned int) -1; /* copy will fail if sizeof(long) == 8 and we've got "int ptrace()" */
         long v2;
@@ -263,7 +263,7 @@ AC_DEFUN([AC_FPM_TRACE],
       #include <sys/stat.h>
       #include <fcntl.h>
       #include <stdio.h>
-      int main()
+      int main(void)
       {
         long v1 = (unsigned int) -1, v2 = 0;
         char buf[128];
@@ -604,7 +604,7 @@ if test "$PHP_FPM" != "no"; then
     AC_CHECK_HEADERS([sys/acl.h])
 
     AC_COMPILE_IFELSE([AC_LANG_SOURCE([[#include <sys/acl.h>
-      int main()
+      int main(void)
       {
         acl_t acl;
         acl_entry_t user, group;
@@ -623,7 +623,7 @@ if test "$PHP_FPM" != "no"; then
           AC_MSG_RESULT([yes])
         ],[
           AC_RUN_IFELSE([AC_LANG_SOURCE([[#include <sys/acl.h>
-            int main()
+            int main(void)
             {
               acl_t acl;
               acl_entry_t user, group;

There is still libtool to patch though but that I'd leave as it is maybe (I'm not sure):

diff --git a/build/libtool.m4 b/build/libtool.m4
index 8ee7b45301..5b518d50e4 100644
--- a/build/libtool.m4
+++ b/build/libtool.m4
@@ -277,7 +277,7 @@ dnl This sometimes fails to find confdefs.h, for some reason.
 dnl [#]line __oline__ "[$]0"
 [#]line __oline__ "configure"
 #include "confdefs.h"
-int main() {
+int main(void) {
 ; return 0; }
 EOF
 if AC_TRY_EVAL(ac_link) && test -s conftest${ac_exeext}; then
@@ -978,8 +978,8 @@ else
 #  endif
 #endif
 
-void fnord() { int i=42;}
-int main ()
+void fnord(void) { int i=42;}
+int main (void)
 {
   void *self = dlopen (0, LT_DLGLOBAL|LT_DLLAZY_OR_NOW);
   int status = $lt_dlunknown;
@@ -2750,7 +2750,7 @@ _LT_AC_TAGVAR(objext, $1)=$objext
 lt_simple_compile_test_code="int some_variable = 0;"
 
 # Code to be used in simple link tests
-lt_simple_link_test_code='int main(){return(0);}'
+lt_simple_link_test_code='int main(void){return(0);}'
 
 _LT_AC_SYS_COMPILER
 
@@ -4640,7 +4640,7 @@ void nm_test_func(){}
 #ifdef __cplusplus
 }
 #endif
-int main(){nm_test_var='a';nm_test_func();return(0);}
+int main(void){nm_test_var='a';nm_test_func();return(0);}
 EOF
 
   if AC_TRY_EVAL(ac_compile); then
diff --git a/build/ltmain.sh b/build/ltmain.sh
index 2f1c8c9dc8..55a73c50b9 100755
--- a/build/ltmain.sh
+++ b/build/ltmain.sh
@@ -3598,7 +3598,7 @@ EOF
 	  # whether they linked in statically or dynamically with ldd.
 	  $rm conftest.c
 	  cat > conftest.c <<EOF
-	  int main() { return 0; }
+	  int main(void) { return 0; }
 EOF
 	  $rm conftest
 	  if $LTCC $LTCFLAGS -o conftest conftest.c $deplibs; then

build/php.m4

petk · 2024-03-17T21:22:05Z

If we're changing the libtool, I'd just send the prototypes fix also to the upstream libtool in the meantime so it's fixed also there and we have one less thing to worry about the additional change in these bundled old libtool files. Otherwise, the libtool existing prototypes (without void) don't seem to cause issues since most of these macros aren't even used in PHP build. Only that underscore macro which is outdated and obsolete anyway... I'll recheck this tomorrow or at least very soon...

petk

Those libtool patches are in queue to be sent also to Libtool in the following days. For now I've opened a pull request on GitHub mirror.

I think that's fine as it is. Thanks @devnexen

Perhaps it would be just good to write a proper commit message what this change does and that the patch for the bundled/forked libtool files has been sent also upstream (to not cause confusion about another change of these files and too much divergence from the upstream libtool version 1.5.26).

autoconf/libtool generating code to test features missed `void` for C calls prototypes w/o arguments. Note that specific changes related to libtool have to upstreamed.

devnexen · 2024-03-18T06:37:07Z

@petk, I see your patch but it seems on github it s just a mirror of the official repo. In those cases, they have a proper repo and a mailing list, for sending patches.

Might be this one ? but if you have contributed to libtool already ignore me :)

petk · 2024-03-18T06:42:29Z

I'll send the patch to the Libtool mailing list soon also, yes.

Edit: sent to mailing list also ✔️ but I'm not sure how frequently it is refreshed.

petk · 2024-03-20T15:03:55Z

Just for the info, now also CMake will have this fixed. So basically, almost all C build tools and such will be able to do check with -Werror=strict-prototypes in the future when compilers have this enabled by default. 🥳

github-actions bot added the Category: Build System label Mar 16, 2024

devnexen requested a review from petk March 16, 2024 17:05

petk linked an issue Mar 17, 2024 that may be closed by this pull request

PHP_TEST_BUILD macro false negatives with -Werror=strict-prototypes #13727

Closed

devnexen requested review from iluuu1994, bukka and dstogov as code owners March 17, 2024 08:24

github-actions bot added SAPI: fpm Category: Engine Extension: gd Extension: iconv Extension: opcache Extension: standard labels Mar 17, 2024

petk reviewed Mar 17, 2024

View reviewed changes

build/php.m4 Outdated Show resolved Hide resolved

devnexen force-pushed the fix_gh13727 branch from 8678894 to 25995c7 Compare March 17, 2024 21:12

petk mentioned this pull request Mar 18, 2024

Fix Wstrict-prototypes warnings autotools-mirror/libtool#7

Open

petk approved these changes Mar 18, 2024

View reviewed changes

Fix phpGH-13727: macro generating invalid call test prototypes fixes.

5284a95

autoconf/libtool generating code to test features missed `void` for C calls prototypes w/o arguments. Note that specific changes related to libtool have to upstreamed.

devnexen force-pushed the fix_gh13727 branch from 25995c7 to 5284a95 Compare March 18, 2024 06:38

devnexen closed this in 868257a Mar 18, 2024

petk mentioned this pull request Apr 21, 2024

Fix GH-13727: Building with -Werror=strict-prototypes #14029

Merged

ryandesign mentioned this pull request Jun 13, 2024

error: incompatible function pointer types initializing 'int (*)(void)' with an expression of type 'int (FILE *)' (aka 'int (struct __sFILE *)') [-Werror,-Wincompatible-function-pointer-types] #14549

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix GH-13727: PHP_TEST_BUILD macro generating invalid call test proto… #13732

Fix GH-13727: PHP_TEST_BUILD macro generating invalid call test proto… #13732

devnexen commented Mar 16, 2024

petk commented Mar 17, 2024 •

edited

Loading

petk commented Mar 17, 2024

petk commented Mar 17, 2024 •

edited

Loading

petk left a comment

devnexen commented Mar 18, 2024 •

edited

Loading

petk commented Mar 18, 2024 •

edited

Loading

petk commented Mar 20, 2024

Fix GH-13727: PHP_TEST_BUILD macro generating invalid call test proto… #13732

Fix GH-13727: PHP_TEST_BUILD macro generating invalid call test proto… #13732

Conversation

devnexen commented Mar 16, 2024

petk commented Mar 17, 2024 • edited Loading

petk commented Mar 17, 2024

petk commented Mar 17, 2024 • edited Loading

petk left a comment

Choose a reason for hiding this comment

devnexen commented Mar 18, 2024 • edited Loading

petk commented Mar 18, 2024 • edited Loading

petk commented Mar 20, 2024

petk commented Mar 17, 2024 •

edited

Loading

petk commented Mar 17, 2024 •

edited

Loading

devnexen commented Mar 18, 2024 •

edited

Loading

petk commented Mar 18, 2024 •

edited

Loading