Build fails on musl 1.2.4 - lfs64

[andypost]

Description

After removal of patches alpinelinux/aports@cab9b78 Alpinelinux now fails to compile with following output:

/builds/alpine/aports/testing/php83/src/php-8.3.0alpha3/main/streams/cast.c:139:2: error: incompatible function pointer types initializing 'cookie_seek_function_t *' (aka 'int (*)(void *, long *, int)') with an expression of type 'int (void *, zend_off_t, int)' (aka 'int (void *, long, int)') [-Wincompatible-function-pointer-types]
        stream_cookie_seeker, stream_cookie_closer
        ^~~~~~~~~~~~~~~~~~~~
1 error generated.
make: *** [Makefile:2600: main/streams/cast.lo] Error 1

Discussion https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/48728#note_321723

Build fixed by removing check

php-src/main/streams/cast.c

Lines 105 to 122 in 62a9408

	# ifdef COOKIE_SEEKER_USES_OFF64_T
	static int stream_cookie_seeker(void *cookie, off64_t *position, int whence)
	{
	*position = php_stream_seek((php_stream *)cookie, (zend_off_t)*position, whence);
	if (*position == -1) {
	return -1;
	}
	return 0;
	}
	# else
	static int stream_cookie_seeker(void *cookie, zend_off_t position, int whence)
	{
	return php_stream_seek((php_stream *)cookie, position, whence);
	}
	# endif

PHP Version

PHP 8.1.21

Operating System

Alpinelinux

[andypost]

Using following patch to build

diff --git a/main/streams/cast.c b/main/streams/cast.c
index 3bad65fbac..05cab34658 100644
--- a/main/streams/cast.c
+++ b/main/streams/cast.c
@@ -102,8 +102,7 @@ static ssize_t stream_cookie_writer(void *cookie, const char *buffer, size_t siz
 	return php_stream_write(((php_stream *)cookie), (char *)buffer, size);
 }
 
-# ifdef COOKIE_SEEKER_USES_OFF64_T
-static int stream_cookie_seeker(void *cookie, off64_t *position, int whence)
+static int stream_cookie_seeker(void *cookie, off_t *position, int whence)
 {
 
 	*position = php_stream_seek((php_stream *)cookie, (zend_off_t)*position, whence);
@@ -113,13 +112,6 @@ static int stream_cookie_seeker(void *cookie, off64_t *position, int whence)
 	}
 	return 0;
 }
-# else
-static int stream_cookie_seeker(void *cookie, zend_off_t position, int whence)
-{
-
-	return php_stream_seek((php_stream *)cookie, position, whence);
-}
-# endif
 
 static int stream_cookie_closer(void *cookie)
 {

[andypost]

From https://musl.libc.org/releases.html

On the API level, the legacy "LFS64" ("large file support") interfaces, which were provided by macros remapping them to their standard names (#define stat64 stat and similar) have been deprecated and are no longer provided under the _GNU_SOURCE feature profile, only under explicit _LARGEFILE64_SOURCE. The latter will also be removed in a future version. Builds broken by this change can be fixed short-term by adding -D_LARGEFILE64_SOURCE to CFLAGS, but should be fixed to use the standard interfaces.

[andypost]

The detection code needs to account musl which using 64-bit IO by default for a long time and only since 1.2.4 the compatibility was removed

Probably the place to fix is

php-src/build/php.m4

Lines 1427 to 1512 in 0b0cec5

	dnl
	dnl PHP_FOPENCOOKIE
	dnl
	AC_DEFUN([PHP_FOPENCOOKIE], [
	AC_CHECK_FUNC(fopencookie, [have_glibc_fopencookie=yes])
	if test "$have_glibc_fopencookie" = "yes"; then
	dnl This comes in two flavors: newer glibcs (since 2.1.2?) have a type called
	dnl cookie_io_functions_t.
	AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
	#define _GNU_SOURCE
	#include <stdio.h>
	]], [[cookie_io_functions_t cookie;]])],[have_cookie_io_functions_t=yes],[])
	if test "$have_cookie_io_functions_t" = "yes"; then
	cookie_io_functions_t=cookie_io_functions_t
	have_fopen_cookie=yes
	dnl Even newer glibcs have a different seeker definition.
	AC_RUN_IFELSE([AC_LANG_SOURCE([[
	#define _GNU_SOURCE
	#include <stdio.h>
	#include <stdlib.h>
	struct cookiedata {
	off64_t pos;
	};
	ssize_t reader(void *cookie, char *buffer, size_t size)
	{ return size; }
	ssize_t writer(void *cookie, const char *buffer, size_t size)
	{ return size; }
	int closer(void *cookie)
	{ return 0; }
	int seeker(void *cookie, off64_t *position, int whence)
	{ ((struct cookiedata*)cookie)->pos = *position; return 0; }
	cookie_io_functions_t funcs = {reader, writer, seeker, closer};
	int main(void) {
	struct cookiedata g = { 0 };
	FILE *fp = fopencookie(&g, "r", funcs);
	if (fp && fseek(fp, 8192, SEEK_SET) == 0 && g.pos == 8192)
	return 0;
	return 1;
	}
	]])], [
	cookie_io_functions_use_off64_t=yes
	], [
	cookie_io_functions_use_off64_t=no
	], [
	dnl Cross compilation.
	case $host_alias in
	*linux*)
	cookie_io_functions_use_off64_t=yes
	;;
	*)
	cookie_io_functions_use_off64_t=no
	;;
	esac
	])
	else
	dnl Older glibc versions (up to 2.1.2?) call it _IO_cookie_io_functions_t.
	AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
	#define _GNU_SOURCE
	#include <stdio.h>
	]], [[_IO_cookie_io_functions_t cookie;]])], [have_IO_cookie_io_functions_t=yes], [])
	if test "$have_cookie_io_functions_t" = "yes" ; then
	cookie_io_functions_t=_IO_cookie_io_functions_t
	have_fopen_cookie=yes
	fi
	fi
	if test "$have_fopen_cookie" = "yes" ; then
	AC_DEFINE(HAVE_FOPENCOOKIE, 1, [ ])
	AC_DEFINE_UNQUOTED(COOKIE_IO_FUNCTIONS_T, $cookie_io_functions_t, [ ])
	if test "$cookie_io_functions_use_off64_t" = "yes" ; then
	AC_DEFINE(COOKIE_SEEKER_USES_OFF64_T, 1, [ ])
	fi
	fi
	fi
	])

[petk]

I've only quickly checked this issue. From what I understand is that off_t should be used today everywhere instead of off64_t type. However, I don't find anything about this written in the glibc, that this can be done on current platforms, everywhere.

Musl has quite an attitude - https://wiki.musl-libc.org/faq#Q:-Do-I-need-to-define-%3Ccode%3E_LARGEFILE64_SOURCE%3C/code%3E-to-get-64bit-%3Ccode%3Eoff_t%3C/code%3E?

Those options are for building apps, not building libc.
-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE have nothing to do with supporting large files properly but with exposing the idiotic legacy interfaces with 64 on the end of their names (like open64, lseek64, etc.). only -D_FILE_OFFSET_BITS=64 should ever be used, anywhere, even on glibc.
under musl, off_t is always 64-bit. the -D_LARGEFILE64_SOURCE option is however honored to support compiling applications using the legacy open64, etc. nonsense, but it just #defines them away to the version without 64 on the end.

I'm always in for following latest standards but having proper portability and I guess we should consider using the off_t then everywhere in the php-src code.

We probably could do the "temporary" fix with defining #define _LARGEFILE64_SOURCE in cast.c and in the php.m4 test and it would work for Musl, but I somehow don't understand why then this will be also removed in the future MUSL versions.... It's like musl is trying to move the issue to glibc and vice versa :D

Or maybe we define the Glibc's feature #define _FILE_OFFSET_BITS 64 and use off_t safely everywhere.

I'm not sure yet how this would work everywhere and how it would affect php-src on some possible 32-bit architectures 🤔 I'll check more later about this.

[andypost]

FYI Alpine require following patch to build with musl >= 1.2.4 and Alpine edge just upgraded to 1.2.5

[orlitzky]

We have a report of this on Gentoo now: https://bugs.gentoo.org/928072

Do I understand correctly that appending -D_LARGEFILE64_SOURCE to CPPFLAGS will work around the issue, albeit temporarily? Is there any risk in doing that across the board? I don't have a musl system yet so the usual trial and error approach will be expensive in this case.

[andypost]

FYI Alpinelinux using following patch as workaround https://gitlab.alpinelinux.org/alpine/aports/-/blob/master/community/php83/fix-lfs64.patch

[petk]

We have a report of this on Gentoo now: https://bugs.gentoo.org/928072

Do I understand correctly that appending -D_LARGEFILE64_SOURCE to CPPFLAGS will work around the issue, albeit temporarily? Is there any risk in doing that across the board? I don't have a musl system yet so the usual trial and error approach will be expensive in this case.

This would need to be tested a bit, but from the php-src point of view, another solution would be to integrate the AC_SYS_LARGEFILE and investigate further how it affects existing code base on 32-bit machines.

Otherwise, yes. Appending it to CPPFLAGS can be a workaround for specific systems in some cases. On Alpine though this wouldn't be sufficient, because also the fopencookie check should be adjusted accordingly to use the off_t.

For example, this would be the more proper way to fix this. Because, for example, on 32-bit AIX systems (although this may be very outdated already and not relevant anymore), a different symbol needs to be defined (the _LARGE_FILES).

So, something like this:

diff --git a/build/php.m4 b/build/php.m4
index be5763c520..7bbee67e00 100644
--- a/build/php.m4
+++ b/build/php.m4
@@ -1183,7 +1183,7 @@ AC_DEFUN([PHP_PWRITE_TEST],[
   AC_CACHE_CHECK(whether pwrite works,ac_cv_pwrite,[
     PHP_DOES_PWRITE_WORK
     if test "$ac_cv_pwrite" = "no"; then
-      PHP_DOES_PWRITE_WORK([ssize_t pwrite(int, void *, size_t, off64_t);])
+      PHP_DOES_PWRITE_WORK([ssize_t pwrite(int, void *, size_t, off_t);])
       if test "$ac_cv_pwrite" = "yes"; then
         ac_cv_pwrite=64
       fi
@@ -1205,7 +1205,7 @@ AC_DEFUN([PHP_PREAD_TEST],[
   AC_CACHE_CHECK(whether pread works,ac_cv_pread,[
     PHP_DOES_PREAD_WORK
     if test "$ac_cv_pread" = "no"; then
-      PHP_DOES_PREAD_WORK([ssize_t pread(int, void *, size_t, off64_t);])
+      PHP_DOES_PREAD_WORK([ssize_t pread(int, void *, size_t, off_t);])
       if test "$ac_cv_pread" = "yes"; then
         ac_cv_pread=64
       fi
@@ -1353,7 +1353,7 @@ AC_RUN_IFELSE([AC_LANG_SOURCE([[
 #include <stdlib.h>
 
 struct cookiedata {
-  off64_t pos;
+  off_t pos;
 };
 
 ssize_t reader(void *cookie, char *buffer, size_t size)
@@ -1362,7 +1362,7 @@ ssize_t writer(void *cookie, const char *buffer, size_t size)
 { return size; }
 int closer(void *cookie)
 { return 0; }
-int seeker(void *cookie, off64_t *position, int whence)
+int seeker(void *cookie, off_t *position, int whence)
 { ((struct cookiedata*)cookie)->pos = *position; return 0; }
 
 cookie_io_functions_t funcs = {reader, writer, seeker, closer};
diff --git a/configure.ac b/configure.ac
index 2bf60c434d..837484aaa1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -270,6 +270,8 @@ if test "$ac_cv_safe_to_define___extensions__" = yes ; then
   AC_MSG_RESULT(yes)
 fi
 
+AC_SYS_LARGEFILE
+
 dnl Include Zend configurations.
 dnl ----------------------------------------------------------------------------
 
diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c
index 464a465fd3..a77924e970 100644
--- a/ext/zend_test/test.c
+++ b/ext/zend_test/test.c
@@ -1368,12 +1368,9 @@ PHP_ZEND_TEST_API int gh11934b_ffi_var_test_cdata;
 /**
  * This function allows us to simulate early return of copy_file_range by setting the limit_copy_file_range ini setting.
  */
-#ifdef __MUSL__
-typedef off_t off64_t;
-#endif
-PHP_ZEND_TEST_API ssize_t copy_file_range(int fd_in, off64_t *off_in, int fd_out, off64_t *off_out, size_t len, unsigned int flags)
+PHP_ZEND_TEST_API ssize_t copy_file_range(int fd_in, off_t *off_in, int fd_out, off_t *off_out, size_t len, unsigned int flags)
 {
-	ssize_t (*original_copy_file_range)(int, off64_t *, int, off64_t *, size_t, unsigned int) = dlsym(RTLD_NEXT, "copy_file_range");
+	ssize_t (*original_copy_file_range)(int, off_t *, int, off_t *, size_t, unsigned int) = dlsym(RTLD_NEXT, "copy_file_range");
 	if (ZT_G(limit_copy_file_range) >= Z_L(0)) {
 		len = ZT_G(limit_copy_file_range);
 	}
diff --git a/main/php.h b/main/php.h
index b10ded010d..a07c08c675 100644
--- a/main/php.h
+++ b/main/php.h
@@ -287,11 +287,11 @@ extern char **environ;
 #endif	/* ifndef PHP_WIN32 */
 
 #ifdef PHP_PWRITE_64
-ssize_t pwrite(int, void *, size_t, off64_t);
+ssize_t pwrite(int, void *, size_t, off_t);
 #endif
 
 #ifdef PHP_PREAD_64
-ssize_t pread(int, void *, size_t, off64_t);
+ssize_t pread(int, void *, size_t, off_t);
 #endif
 
 BEGIN_EXTERN_C()
diff --git a/main/streams/cast.c b/main/streams/cast.c
index dbeeff7b09..c7ab366e63 100644
--- a/main/streams/cast.c
+++ b/main/streams/cast.c
@@ -103,7 +103,7 @@ static ssize_t stream_cookie_writer(void *cookie, const char *buffer, size_t siz
 }
 
 # ifdef COOKIE_SEEKER_USES_OFF64_T
-static int stream_cookie_seeker(void *cookie, off64_t *position, int whence)
+static int stream_cookie_seeker(void *cookie, off_t *position, int whence)
 {
 
 	*position = php_stream_seek((php_stream *)cookie, (zend_off_t)*position, whence);

[andypost]

Thank you! queued for Alpine&8.3

[arnaud-lb]

For 8.2/8.3 we may be able to fix the issue without _LARGEFILE64_SOURCE / _FILE_OFFSET_BITS / _LARGE_FILES.

In glibc the position parameter is always __off64_t*. Also, fopencookie is defined under _GNU_SOURCE, which also implies that off64_t is defined.

The __off_t position signature (non-off64 and non-pointer) has been replaced by __off64_t* position 24 years ago.

On FreeBSD the parameter is also off64_t*.

On musl it has always been off_t*.

So it should be safe to use stream_cookie_seeker(void *cookie, off64_t *position, int whence) when the autoconf test passes, and stream_cookie_seeker(void *cookie, off_t *position, int whence) otherwise:

diff --git a/main/streams/cast.c b/main/streams/cast.c
index 3bad65fbac1..8d9f4a9d2d5 100644
--- a/main/streams/cast.c
+++ b/main/streams/cast.c
@@ -104,6 +104,9 @@ static ssize_t stream_cookie_writer(void *cookie, const char *buffer, size_t siz
 
 # ifdef COOKIE_SEEKER_USES_OFF64_T
 static int stream_cookie_seeker(void *cookie, off64_t *position, int whence)
+# else
+static int stream_cookie_seeker(void *cookie, off_t *position, int whence)
+# endif
 {
 
 	*position = php_stream_seek((php_stream *)cookie, (zend_off_t)*position, whence);
@@ -113,13 +116,6 @@ static int stream_cookie_seeker(void *cookie, off64_t *position, int whence)
 	}
 	return 0;
 }
-# else
-static int stream_cookie_seeker(void *cookie, zend_off_t position, int whence)
-{
-
-	return php_stream_seek((php_stream *)cookie, position, whence);
-}
-# endif
 
 static int stream_cookie_closer(void *cookie)
 {

WDYT?

[petk]

@arnaud-lb this sounds great to me. I'd rather leave it to you to apply this patch because I don't have such overview nor understanding of the entire php-src C code base yet. Otherwise, this else removed condition was intended for Windows I think. Would it work ok there also?

P.S.: It would be even better to not use the Autoconf's AC_SYS_LARGEFILE because it does too many things in some cases. It applies compiler flags, and also affects time types. It's almost a bit overengineered :D