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

[orlitzky]

Full AC_SYS_LARGEFILE with off_t is the only combination I can even begin to comprehend, but whatever you decide to do I'm willing to backport the patch and let our unstable users beta test it.

[arnaud-lb]

@petk are you sure about Windows? I believe this was for old glibcs before this change

AC_SYS_LARGEFILE is the was to go, but this is too big of a change for stable versions.

[andypost]

Thank you! changed patches for Alpinelinux

[petk]

@petk are you sure about Windows? I believe this was for old glibcs before this change

AC_SYS_LARGEFILE is the was to go, but this is too big of a change for stable versions.

I guess it works on Windows then. Nice. About the AC_SYS_LARGEFILE there is another issue with these macros. They define symbols in main/php_config.h file which is not included correctly yet. These symbols should be the first definition before any system header inclusing. So these _LARGEFILE64_SOURCE / _FILE_OFFSET_BITS / _LARGE_FILES wouldn't even be used in php-src on some places. Unless they would be passed to compiler via -D flags. It's better for the time being to not use it yet.

[andypost]

Thanks, it means that official docker images should remove this flags on next release

• https://github.com/docker-library/php/blob/f140206f63dfbbd3a423d962ca40727ade295b6b/8.2-rc/alpine3.19/cli/Dockerfile#L50-L53
• https://github.com/docker-library/php/blob/8480cf5cff5759bc4c1475f6873420edc2518218/8.2/alpine3.19/cli/Dockerfile#L50-L53
• Add CFLAGS ENV to Dockerfile docker-library/php#272

# Enable linker optimization (this sorts the hash buckets to improve cache locality, and is non-default)
# https://github.com/docker-library/php/issues/272
# -D_LARGEFILE_SOURCE and -D_FILE_OFFSET_BITS=64 (https://www.php.net/manual/en/intro.filesystem.php)
ENV PHP_CFLAGS="-fstack-protector-strong -fpic -fpie -O2 -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64"

[petk]

@andypost thanks for this info. It won't cause issues if they even stay there in this particular case. Because it is specific for the Alpine build. But, I'll recheck it out.

[andypost]

@petk thank you! yes, I mean only alpine-based images