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

8.4.0 fails to build on cross-compiled android ARM #12086

Closed
12932 opened this issue Oct 11, 2023 · 26 comments
Closed

8.4.0 fails to build on cross-compiled android ARM #12086

12932 opened this issue Oct 11, 2023 · 26 comments
Labels

Comments

@12932
Copy link
Contributor

12932 commented Oct 11, 2023

I did this

Build curl:

configure: Configured to build curl/libcurl:

  Host setup:       arm-unknown-linux-androideabi
  Install prefix:   /usr/local
  Compiler:         arm-linux-androideabi-gcc -std=gnu11
   CFLAGS:          -D__ANDROID_API__=21 -fPIC -Werror-implicit-function-declaration -O2 -Wno-system-headers
   CPPFLAGS:        -isystem /home/ubuntu/dev/curl/src/../interface/include -isystem /home/ubuntu/dev/curl/src/../interface/include/ANDROIDARM -isystem /home/ubuntu/dev/curl/src/../interface/include/LINUX -fPIC
   LDFLAGS:         -L/home/ubuntu/dev/curl/src/../interface/lib/ANDROIDARM -fPIC
   LIBS:            -lssl -lcrypto -ldl

  curl version:     8.4.0
  SSL:              enabled (OpenSSL)
  SSH:              no      (--with-{libssh,libssh2})
  zlib:             no      (--with-zlib)
  brotli:           no      (--with-brotli)
  zstd:             no      (--with-zstd)
  GSS-API:          no      (--with-gssapi)
  GSASL:            no      (libgsasl not found)
  TLS-SRP:          enabled
  resolver:         POSIX threaded
  IPv6:             no      (--enable-ipv6)
  Unix sockets:     enabled
  IDN:              no      (--with-{libidn2,winidn})
  Build libcurl:    Shared=no, Static=yes
  Built-in manual:  no      (--enable-manual)
  --libcurl option: enabled (--disable-libcurl-option)
  Verbose errors:   enabled (--disable-verbose)
  Code coverage:    disabled
  SSPI:             no      (--enable-sspi)
  ca cert bundle:   no
  ca cert path:     no
  ca fallback:      no
  LDAP:             no      (--enable-ldap / --with-ldap-lib / --with-lber-lib)
  LDAPS:            no      (--enable-ldaps)
  RTSP:             enabled
  RTMP:             no      (--with-librtmp)
  PSL:              no      (libpsl not found)
  Alt-svc:          enabled (--disable-alt-svc)
  Headers API:      enabled (--disable-headers-api)
  HSTS:             enabled (--disable-hsts)
  HTTP1:            enabled (internal)
  HTTP2:            no      (--with-nghttp2, --with-hyper)
  HTTP3:            no      (--with-ngtcp2 --with-nghttp3, --with-quiche, --with-msh3)
  ECH:              no      (--enable-ech)
  WebSockets:       no      (--enable-websockets)
  Protocols:        DICT FILE FTP FTPS GOPHER GOPHERS HTTP HTTPS IMAP IMAPS MQTT POP3 POP3S RTSP SMB SMBS SMTP SMTPS TELNET TFTP
  Features:         AsynchDNS HSTS HTTPS-proxy Largefile NTLM SSL TLS-SRP UnixSockets alt-svc threadsafe

Making all in lib
xmake[1]: Entering directory `/home/ubuntu/dev/curl/src/build.ANDROIDARM.P/lib'
xmake[2]: Entering directory `/home/ubuntu/dev/curl/src/build.ANDROIDARM.P/lib'
  CC       libcurl_la-altsvc.lo
  CC       libcurl_la-amigaos.lo
  CC       libcurl_la-asyn-ares.lo
  CC       libcurl_la-asyn-thread.lo
  CC       libcurl_la-base64.lo
  CC       libcurl_la-bufq.lo
  CC       libcurl_la-bufref.lo
  CC       libcurl_la-c-hyper.lo
  CC       libcurl_la-cf-h1-proxy.lo
  CC       libcurl_la-cf-h2-proxy.lo
  CC       libcurl_la-cf-haproxy.lo
  CC       libcurl_la-cf-https-connect.lo
  CC       libcurl_la-cf-socket.lo
  CC       libcurl_la-cfilters.lo
  CC       libcurl_la-conncache.lo
  CC       libcurl_la-connect.lo
  CC       libcurl_la-content_encoding.lo
  CC       libcurl_la-cookie.lo
  CC       libcurl_la-curl_addrinfo.lo
  CC       libcurl_la-curl_des.lo
  CC       libcurl_la-curl_endian.lo
  CC       libcurl_la-curl_fnmatch.lo
  CC       libcurl_la-curl_get_line.lo
  CC       libcurl_la-curl_gethostname.lo
  CC       libcurl_la-curl_gssapi.lo
  CC       libcurl_la-curl_memrchr.lo
  CC       libcurl_la-curl_multibyte.lo
  CC       libcurl_la-curl_ntlm_core.lo
  CC       libcurl_la-curl_ntlm_wb.lo
  CC       libcurl_la-curl_path.lo
  CC       libcurl_la-curl_range.lo
  CC       libcurl_la-curl_rtmp.lo
  CC       libcurl_la-curl_sasl.lo
  CC       libcurl_la-curl_sspi.lo
  CC       libcurl_la-curl_threads.lo
  CC       libcurl_la-curl_trc.lo
  CC       libcurl_la-dict.lo
  CC       libcurl_la-doh.lo
  CC       libcurl_la-dynbuf.lo
  CC       libcurl_la-dynhds.lo
  CC       libcurl_la-easy.lo
  CC       libcurl_la-easygetopt.lo
  CC       libcurl_la-easyoptions.lo
  CC       libcurl_la-escape.lo
  CC       libcurl_la-file.lo
  CC       libcurl_la-fileinfo.lo
  CC       libcurl_la-fopen.lo
  CC       libcurl_la-formdata.lo
/home/ubuntu/dev/curl/src/curl-8.4.0/lib/formdata.c: In function 'fseeko_wrapper':
/home/ubuntu/dev/curl/src/curl-8.4.0/lib/formdata.c:796:3: error: implicit declaration of function 'fseeko' [-Werror=implicit-function-declaration]
   return fseeko(stream, (off_t)offset, whence);
   ^
cc1: some warnings being treated as errors
xmake[2]: *** [libcurl_la-formdata.lo] Error 1
xmake[2]: Leaving directory `/home/ubuntu/dev/curl/src/build.ANDROIDARM.P/lib'
xmake[1]: *** [all] Error 2
xmake[1]: Leaving directory `/home/ubuntu/dev/curl/src/build.ANDROIDARM.P/lib'
make: *** [all-recursive] Error 1
xmake: *** [ANDROIDARMP.OBJ/dummy] Error 2
[Make] (E) Program terminated. Errors previously reported

I expected the following

Curl to build successfully

curl/libcurl version

NA

operating system

Linux 7a4b33d7e1d4 5.15.90.1-microsoft-standard-WSL2 #1 SMP Fri Jan 27 02:56:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

@vszakats vszakats added the build label Oct 11, 2023
@dfandrich
Copy link
Contributor

dfandrich commented Oct 11, 2023 via email

@12932
Copy link
Contributor Author

12932 commented Oct 11, 2023

ubuntu@7a4b33d7e1d4:/$ nm -D /android-ndk-arm/sysroot/usr/lib/libc.so | grep fseeko
0000a851 T fseeko

@cajus
Copy link

cajus commented Oct 11, 2023

Nearly the same output in the CI here:

formdata.c:796:10: error: call to undeclared function 'fseeko'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
  return fseeko(stream, (off_t)offset, whence);
         ^
formdata.c:796:10: note: did you mean 'fseek'?
/home/dev/.conan/data/android_ndk_installer/r26/bje/stable/package/b925f19ad6eb509f29bf6c0a5b52e2a00411ce69/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/include/stdio.h:215:5: note: 'fseek' declared here
int fseek(FILE* _Nonnull __fp, long __offset, int __whence);
    ^
1 error generated.

Seems to depend on the target min SDK version: https://stackoverflow.com/questions/55571842/android-error-use-of-undeclared-identifier-fseeko

Snippet from stdio.h:

int fseek(FILE* _Nonnull __fp, long __offset, int __whence);
long ftell(FILE* _Nonnull __fp);

/* See https://android.googlesource.com/platform/bionic/+/master/docs/32-bit-abi.md */
#if defined(__USE_FILE_OFFSET64)

#if __ANDROID_API__ >= 24
int fgetpos(FILE* _Nonnull __fp, fpos_t* _Nonnull __pos) __RENAME(fgetpos64) __INTRODUCED_IN(24);
int fsetpos(FILE* _Nonnull __fp, const fpos_t* _Nonnull __pos) __RENAME(fsetpos64) __INTRODUCED_IN(24);
int fseeko(FILE* _Nonnull __fp, off_t __offset, int __whence) __RENAME(fseeko64) __INTRODUCED_IN(24);
off_t ftello(FILE* _Nonnull __fp) __RENAME(ftello64) __INTRODUCED_IN(24);
#endif /* __ANDROID_API__ >= 24 */

@cajus
Copy link

cajus commented Oct 11, 2023

As a quick and dirty workaround, I skipped the fseeko check for android.

diff -Naur a/CMakeLists.txt b/CMakeLists.txt
--- a/CMakeLists.txt	2023-10-09 08:21:51.000000000 +0200
+++ b/CMakeLists.txt	2023-10-11 12:53:58.055399342 +0200
@@ -1180,7 +1180,9 @@
 check_symbol_exists(freeaddrinfo   "${CURL_INCLUDES}" HAVE_FREEADDRINFO)
 check_symbol_exists(pipe           "${CURL_INCLUDES}" HAVE_PIPE)
 check_symbol_exists(ftruncate      "${CURL_INCLUDES}" HAVE_FTRUNCATE)
-check_symbol_exists(fseeko         "${CURL_INCLUDES};stdio.h" HAVE_FSEEKO)
+if(CMAKE_SYSTEM_NAME NOT STREQUAL Android)
+    check_symbol_exists(fseeko         "${CURL_INCLUDES};stdio.h" HAVE_FSEEKO)
+endif()
 check_symbol_exists(_fseeki64      "${CURL_INCLUDES};stdio.h" HAVE__FSEEKI64)
 check_symbol_exists(getpeername    "${CURL_INCLUDES}" HAVE_GETPEERNAME)
 check_symbol_exists(getsockname    "${CURL_INCLUDES}" HAVE_GETSOCKNAME)
diff -Naur a/configure.ac b/configure.ac
--- a/configure.ac	2023-10-05 09:58:50.000000000 +0200
+++ b/configure.ac	2023-10-11 13:13:07.486490432 +0200
@@ -3577,6 +3577,14 @@
     ;;
 esac
 
+case $host in
+   *android*)
+     ac_cv_func_fseeko=no
+     skipcheck_fseeko=yes
+     AC_MSG_NOTICE([skip check for fseeko on android])
+    ;;
+esac
+
 AC_CHECK_DECLS([getpwuid_r], [], [AC_DEFINE(HAVE_DECL_GETPWUID_R_MISSING, 1, "Set if getpwuid_r() declaration is missing")],
         [[#include <pwd.h>
           #include <sys/types.h>]])

@vszakats
Copy link
Member

vszakats commented Oct 11, 2023

Another workaround is configuring with: cmake -DHAVE_FSEEKO=0 (and possibly export ac_cv_func_fseeko='no' for autotools.)

I wonder why the detection and use turn out differently.

Does this help perventing false-detection with CMake?:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 1b19c681d..23cd906dc 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1180,7 +1180,7 @@ check_symbol_exists(getifaddrs     "${CURL_INCLUDES};stdlib.h" HAVE_GETIFADDRS)
 check_symbol_exists(freeaddrinfo   "${CURL_INCLUDES}" HAVE_FREEADDRINFO)
 check_symbol_exists(pipe           "${CURL_INCLUDES}" HAVE_PIPE)
 check_symbol_exists(ftruncate      "${CURL_INCLUDES}" HAVE_FTRUNCATE)
-check_symbol_exists(fseeko         "${CURL_INCLUDES};stdio.h" HAVE_FSEEKO)
+check_symbol_exists(fseeko         "stdio.h" HAVE_FSEEKO)
 check_symbol_exists(_fseeki64      "${CURL_INCLUDES};stdio.h" HAVE__FSEEKI64)
 check_symbol_exists(getpeername    "${CURL_INCLUDES}" HAVE_GETPEERNAME)
 check_symbol_exists(getsockname    "${CURL_INCLUDES}" HAVE_GETSOCKNAME)

@vvb2060
Copy link
Contributor

vvb2060 commented Oct 11, 2023

https://android.googlesource.com/platform/bionic/+/refs/heads/main/docs/32-bit-abi.md
for 32 bit (defined __USE_FILE_OFFSET64), fseeko requires minapi >= 24, 64 bit don’t have this issue.
I think there is something wrong with the check code.

@vszakats
Copy link
Member

vszakats commented Oct 11, 2023

Please try #12086 (comment) and report if it changes detection results.

It could clear the question if anything in CURL_INCLUDES might
skew detection results the wrong way.

@bagder
Copy link
Member

bagder commented Oct 12, 2023

@12932 this still seems to imply that configure found the fseeko function which is why it was enabled. Does config.log reveal any clues as to why it would do wrong for that specific function and not the others?

@cajus
Copy link

cajus commented Oct 12, 2023

configure:43527: checking for fseeko
configure:43527: /home/dev/.conan/data/android_ndk_installer/r26/bje/stable/package/b925f19ad6eb509f29bf6c0a5b52e2a00411ce69/toolchains/llvm/prebuilt/linux-x86_64/bin/armv7a-linux-androideabi23-clang -o conftest -O3 --sysroot=/home/dev/.conan/data/android_ndk_installer/r26/bje/stable/package/b925f19ad6eb509f29bf6c0a5b52e2a00411ce69/toolchains/llvm/prebuilt/linux-x86_64/sysroot -fPIC -fPIC -Qunused-arguments -Wno-pointer-bool-conversion -I/home/dev/.conan/data/openssl/3.0.11/bje/stable/package/c078f6735e61e43f18849088a49c08c26c3fd3ec/include -I/home/dev/.conan/data/zlib/1.3/bje/stable/package/f0c177dad580ee9cdcb0b0da457d467f5bdab4ab/include -DNDEBUG -I/home/dev/.conan/data/zlib/1.3/bje/stable/package/f0c177dad580ee9cdcb0b0da457d467f5bdab4ab/include -I/home/dev/.conan/data/openssl/3.0.11/bje/stable/package/c078f6735e61e43f18849088a49c08c26c3fd3ec/include --sysroot=/home/dev/.conan/data/android_ndk_installer/r26/bje/stable/package/b925f19ad6eb509f29bf6c0a5b52e2a00411ce69/toolchains/llvm/prebuilt/linux-x86_64/sysroot -L/home/dev/.conan/data/java_installer/19.0.2/bje/stable/package/44fcf6b9a7fb86b2586303e3db40189d3b511830/lib -L/home/dev/.conan/data/openssl/3.0.11/bje/stable/package/c078f6735e61e43f18849088a49c08c26c3fd3ec/lib -L/home/dev/.conan/data/zlib/1.3/bje/stable/package/f0c177dad580ee9cdcb0b0da457d467f5bdab4ab/lib -Wl,--hash-style=both -L/home/dev/.conan/data/zlib/1.3/bje/stable/package/f0c177dad580ee9cdcb0b0da457d467f5bdab4ab/lib -L/home/dev/.conan/data/openssl/3.0.11/bje/stable/package/c078f6735e61e43f18849088a49c08c26c3fd3ec/lib conftest.c -lssl -lcrypto -lz -lssl -lcrypto -lz >&5
configure:43527: $? = 0
configure:43527: result: yes

Maybe the required defines for the minimum targed sdk are missing there, so that it finds it?

@bagder
Copy link
Member

bagder commented Oct 12, 2023

Maybe the required defines for the minimum targed sdk are missing there, so that it finds it?

The check links a test program that uses the symbol, and the linking works. So the function is definitely present there. So yes it could be a problem with the define, if the define causes the symbol to somehow get redefined and make it not found by the linker.

@cajus
Copy link

cajus commented Oct 12, 2023

But:

$ /home/dev/.conan/data/android_ndk_installer/r26/bje/stable/package/b925f19ad6eb509f29bf6c0a5b52e2a00411ce69/toolchains/llvm/prebuilt/linux-x86_64/bin/armv7a-linux-androideabi23-clang -dM -E - < /dev/null | grep ANDROID
#define __ANDROID_API__ __ANDROID_MIN_SDK_VERSION__
#define __ANDROID_MIN_SDK_VERSION__ 23
#define __ANDROID__ 1

and the check in stdio.h is based on

#if __ANDROID_API__ >= 24

But seems to trigger. It also triggers if I write <= 24, and I failed to output the value of __ANDROID_API__ in stdio.h using #pragma message. No messages are printed at all.

@bagder
Copy link
Member

bagder commented Oct 12, 2023

Those are just defines that don't change the fseeko name though. So they will not prevent the linker from finding the fseeko function that you evidently have.

@cajus
Copy link

cajus commented Oct 12, 2023

Doesn't it? __ANDROID_API__ is __ANDROID_MIN_SDK_VERSION__, which is 23 in my case. The visibility of fseeko in stdio.h is coupled to __ANDROID_API__. If I change that one to #if 0, it compiles.

@bagder
Copy link
Member

bagder commented Oct 12, 2023

The visibility of fseeko in stdio.h

The prototype being provided in that header only changes if you get a compiler warning or not, it does not prevent the function from being able to link.

@cajus
Copy link

cajus commented Oct 12, 2023

Oups - too many terminals open error. You're right, I just tricked myself. Sorry.

@vszakats
Copy link
Member

Does __ANDROID_API__ remain 23 while CMake/autotools are compiling the test snippet?

@bagder
Copy link
Member

bagder commented Oct 12, 2023

ubuntu@7a4b33d7e1d4:/$ nm -D /android-ndk-arm/sysroot/usr/lib/libc.so | grep fseeko
0000a851 T fseeko

So yes, this is a compiler warning you turn into an error with -Werror-implicit-function-declaration because apparently the header file does not provide the prototype in your case.

The function is however present in libc, so chances are it would work if you removed that compiler option?

@MarcelRaad
Copy link
Member

MarcelRaad commented Oct 12, 2023

this is a compiler warning you turn into an error with -Werror-implicit-function-declaration because apparently the header file does not provide the prototype in your case.

The function is however present in libc, so chances are it would work if you removed that compiler option?

We actually set -Werror-implicit-function-declaration unconditionally in curl itself:

tmp_CFLAGS="$tmp_CFLAGS -Werror-implicit-function-declaration"

It was actually me who did that in 9168e24. That's because it's required for C99 and later anyway.

(Edit: ah no, that's for GCC, and the Android toolchain uses clang.)

@vvb2060
Copy link
Contributor

vvb2060 commented Oct 12, 2023

The function is however present in libc

ndk uses the same libc.so for all API levels

@bagder
Copy link
Member

bagder commented Oct 12, 2023

We actually set -Werror-implicit-function-declaration unconditionally in curl itself:

Sure, because it makes sense. But this Android setup does not make sense...

@cajus
Copy link

cajus commented Oct 12, 2023

And the method is available for 32bit abi >= 24 only. I need to build for >= 23.

If I do

diff -Naur a/m4/curl-compilers.m4 b/m4/curl-compilers.m4
--- a/m4/curl-compilers.m4	2023-10-05 09:58:50.000000000 +0200
+++ b/m4/curl-compilers.m4	2023-10-12 16:43:26.504482752 +0200
@@ -506,7 +506,7 @@
         dnl Disable warnings for unused arguments, otherwise clang will
         dnl warn about compile-time arguments used during link-time, like
         dnl -O and -g and -pedantic.
-        tmp_CFLAGS="$tmp_CFLAGS -Qunused-arguments"
+        tmp_CFLAGS="$tmp_CFLAGS -Qunused-arguments -Wno-implicit-function-declaration"
         ;;
         #
       DEC_C)

it compiles and links. Can't say if it works.

@vvb2060
Copy link
Contributor

vvb2060 commented Oct 12, 2023

Crash at runtime unless static build.
for 32-bit devices, if min api < 24, HAVE_FSEEKO should be false

bagder added a commit that referenced this issue Oct 19, 2023
... and make the code require both symbol and declaration.

This is because for Android, the symbol is always present in the lib at
build-time even when not actually available in run-time.

Reported-by: 12932 on github
Fixes #12086
@bagder
Copy link
Member

bagder commented Oct 19, 2023

Please try if #12158 works for Android builds. It now requires that the declaration is also present before using fseeko function. If this works, we also need to add a similar check in cmake.

@12932
Copy link
Contributor Author

12932 commented Oct 19, 2023

Sorry i've been quiet, I have been busy with other things

But this Android setup does not make sense...

I'm not a fan of it either 😭

I'll try out #12158 tomorrow

@cajus
Copy link

cajus commented Oct 20, 2023

Just placed the patch in the CI. Using autotools:

armv7:

checking for fseeko... yes
checking whether fseeko is declared... no

armv8:

checking for fseeko... yes
checking whether fseeko is declared... yes

Both targets build. No runtime test yet.

@bagder
Copy link
Member

bagder commented Oct 22, 2023

I think this is good enough and I am going to proceed and merge the current fix, as it at least is a step forward.

@bagder bagder closed this as completed in f4ff410 Oct 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
7 participants