Error while compiling with Android NDK r15b

[destman]

When compiling with Android NDK r15b i got errors like this:
Cannot find proper types to use for recv args
Version of curl is 7.54.1
The problem is that NDK r15b switch to clang and use new headers now. So when configure checks if some functions have valid arguments invalid code is generated.
I fix this problem by adding __attribute__((overloadable)) before some functions (recv, send, select, getnameinfo) in acinclude.m4
But not sure how it influence other platforms...

[bagder]

Can you elaborate exactly how that "invalid code" is generated and why? We can run that configure check perfectly fine with clang on other platforms...

And yes, __attribute__ is not portable.

[destman]

Here is part of config.log

configure:29798: $? = 1
configure: failed program was:
| /* confdefs.h */
| #define PACKAGE_NAME "curl"
| #define PACKAGE_TARNAME "curl"
| #define PACKAGE_VERSION "-"
| #define PACKAGE_STRING "curl -"
| #define PACKAGE_BUGREPORT "a suitable curl mailing list: https://curl.haxx.se/mail/"
| #define PACKAGE_URL ""
| #define PACKAGE "curl"
| #define VERSION "-"
| #define OS "arm-unknown-linux-androideabi"
| #define HAVE_SYS_TYPES_H 1
| #define HAVE_STDINT_H 1
| #define HAVE_INTTYPES_H 1
| #define STDC_HEADERS 1
| #define HAVE_SYS_TYPES_H 1
| #define HAVE_SYS_STAT_H 1
| #define HAVE_STDLIB_H 1
| #define HAVE_STRING_H 1
| #define HAVE_MEMORY_H 1
| #define HAVE_STRINGS_H 1
| #define HAVE_INTTYPES_H 1
| #define HAVE_STDINT_H 1
| #define HAVE_UNISTD_H 1
| #define SIZEOF_LONG 4
| #define SIZEOF_VOIDP 4
| #define CURL_PULL_SYS_TYPES_H 1
| #define CURL_PULL_STDINT_H 1
| #define CURL_PULL_INTTYPES_H 1
| #define CURL_TYPEOF_CURL_OFF_T int64_t
| #define CURL_FORMAT_CURL_OFF_T "lld"
| #define CURL_FORMAT_CURL_OFF_TU "llu"
| #define CURL_FORMAT_OFF_T "%lld"
| #define CURL_SIZEOF_CURL_OFF_T 8
| #define CURL_SUFFIX_CURL_OFF_T LL
| #define CURL_SUFFIX_CURL_OFF_TU ULL
| #define _FILE_OFFSET_BITS 64
| #define HAVE_DLFCN_H 1
| #define LT_OBJDIR ".libs/"
| #define HAVE_LDAP_SSL 1
| #define TIME_WITH_SYS_TIME 1
| #define HAVE_SYS_TYPES_H 1
| #define HAVE_SYS_TIME_H 1
| #define HAVE_TIME_H 1
| #define HAVE_CLOCK_GETTIME_MONOTONIC 1
| #define HAVE_ZLIB_H 1
| #define HAVE_LIBZ 1
| #define CURL_DISABLE_LDAP 1
| #define CURL_DISABLE_LDAPS 1
| #define HAVE_SOCKADDR_IN6_SIN6_SCOPE_ID 1
| #define HAVE_LIBSSL 1
| #define HAVE_OPENSSL_X509_H 1
| #define USE_OPENSSL 1
| #define HAVE_OPENSSL_RSA_H 1
| #define USE_OPENSSL 1
| #define HAVE_OPENSSL_CRYPTO_H 1
| #define USE_OPENSSL 1
| #define HAVE_OPENSSL_PEM_H 1
| #define USE_OPENSSL 1
| #define HAVE_OPENSSL_SSL_H 1
| #define USE_OPENSSL 1
| #define HAVE_OPENSSL_ERR_H 1
| #define USE_OPENSSL 1
| #define HAVE_OPENSSL_PKCS12_H 1
| #define HAVE_OPENSSL_ENGINE_H 1
| #define HAVE_ENGINE_LOAD_BUILTIN_ENGINES 1
| #define HAVE_ENGINE_CLEANUP 1
| #define HAVE_SSL_GET_SHUTDOWN 1
| #define HAVE_LIBRESSL 1
| #define STDC_HEADERS 1
| #define HAVE_MALLOC_H 1
| #define HAVE_MEMORY_H 1
| #define HAVE_SYS_TYPES_H 1
| #define HAVE_SYS_TIME_H 1
| #define HAVE_SYS_SELECT_H 1
| #define HAVE_SYS_SOCKET_H 1
| #define HAVE_SYS_IOCTL_H 1
| #define HAVE_SYS_UIO_H 1
| #define HAVE_ASSERT_H 1
| #define HAVE_UNISTD_H 1
| #define HAVE_STDLIB_H 1
| #define HAVE_LIMITS_H 1
| #define HAVE_ARPA_INET_H 1
| #define HAVE_NET_IF_H 1
| #define HAVE_NETINET_IN_H 1
| #define HAVE_SYS_UN_H 1
| #define HAVE_NETINET_TCP_H 1
| #define HAVE_NETDB_H 1
| #define HAVE_SYS_STAT_H 1
| #define HAVE_SYS_PARAM_H 1
| #define HAVE_TERMIOS_H 1
| #define HAVE_TERMIO_H 1
| #define HAVE_FCNTL_H 1
| #define HAVE_ALLOCA_H 1
| #define HAVE_TIME_H 1
| #define HAVE_PWD_H 1
| #define HAVE_UTIME_H 1
| #define HAVE_SYS_POLL_H 1
| #define HAVE_POLL_H 1
| #define HAVE_SYS_RESOURCE_H 1
| #define HAVE_LIBGEN_H 1
| #define HAVE_LOCALE_H 1
| #define HAVE_ERRNO_H 1
| #define HAVE_STDBOOL_H 1
| #define HAVE_SYS_WAIT_H 1
| #define HAVE_SETJMP_H 1
| #define HAVE_VARIADIC_MACROS_C99 1
| #define HAVE_VARIADIC_MACROS_GCC 1
| #define TIME_WITH_SYS_TIME 1
| #define HAVE_SYS_TYPES_H 1
| #define HAVE_SYS_TIME_H 1
| #define HAVE_TIME_H 1
| #define HAVE_SYS_SOCKET_H 1
| #define HAVE_STRUCT_TIMEVAL 1
| #define SIZEOF_SIZE_T 4
| #define SIZEOF_LONG 4
| #define SIZEOF_INT 4
| #define SIZEOF_SHORT 2
| #define CURL_SIZEOF_LONG 4
| #define SIZEOF_TIME_T 4
| #define SIZEOF_OFF_T 8
| #define HAVE_LONGLONG 1
| #define HAVE_LL 1
| #define HAVE_BOOL_T 1
| #define HAVE_SYS_TYPES_H 1
| #define HAVE_SYS_SOCKET_H 1
| #define CURL_PULL_SYS_TYPES_H 1
| #define CURL_PULL_SYS_SOCKET_H 1
| #define CURL_TYPEOF_CURL_SOCKLEN_T socklen_t
| #define CURL_SIZEOF_CURL_SOCKLEN_T 4
| #define HAVE_SYS_TYPES_H 1
| #define HAVE_POLL_H 1
| #define HAVE_SYS_POLL_H 1
| #define HAVE_STRUCT_SOCKADDR_STORAGE 1
| #define HAVE_SIGNAL_H 1
| #define HAVE_SIG_ATOMIC_T 1
| #define RETSIGTYPE void
| #define HAVE_SYS_SELECT_H 1
| #define HAVE_SYS_SOCKET_H 1
| #define SELECT_TYPE_ARG1 int
| #define SELECT_TYPE_ARG234 fd_set *
| #define SELECT_TYPE_RETV int
| #define SELECT_QUAL_ARG5 
| #define SELECT_TYPE_ARG5 struct timeval *
| #define HAVE_SELECT 1
| #define HAVE_SYS_TYPES_H 1
| #define HAVE_SYS_SOCKET_H 1
| /* end confdefs.h.  */
| 
| 
| #undef inline
| #ifdef HAVE_WINDOWS_H
| #ifndef WIN32_LEAN_AND_MEAN
| #define WIN32_LEAN_AND_MEAN
| #endif
| #include <windows.h>
| #ifdef HAVE_WINSOCK2_H
| #include <winsock2.h>
| #else
| #ifdef HAVE_WINSOCK_H
| #include <winsock.h>
| #endif
| #endif
| #define RECVCALLCONV PASCAL
| #else
| #ifdef HAVE_SYS_TYPES_H
| #include <sys/types.h>
| #endif
| #ifdef HAVE_SYS_SOCKET_H
| #include <sys/socket.h>
| #endif
| #define RECVCALLCONV
| #endif
|                       extern ssize_t RECVCALLCONV
|                       recv(SOCKET, void *, unsigned int, int);
| 
| int main (void)
| {
| 
|                       SOCKET s=0;
|                       void * buf=0;
|                       unsigned int len=0;
|                       int flags=0;
|                       ssize_t res = recv(s, buf, len, flags);
| 
|  ;
|  return 0;
| }
| 
configure:29798: clang -c -O3 -isystem /Users/destman/tmp/build/Android/arm/include -Qunused-arguments    conftest.c >&5
conftest.c:174:36: error: expected identifier
                      recv(SOCKET, void *, unsigned int, unsigned int);
                                   ^
conftest.c:174:23: error: redeclaration of 'recv' must have the 'overloadable' attribute
                      recv(SOCKET, void *, unsigned int, unsigned int);
                      ^
/Users/destman/tmp/toolchain/arm/bin/../sysroot/usr/include/sys/socket.h:332:9: note: previous overload of function is here
ssize_t recv(int, void*, size_t, int) __overloadable __RENAME_CLANG(recv);
        ^
conftest.c:179:23: error: use of undeclared identifier 'SOCKET'
                      SOCKET s=0;
                      ^
conftest.c:183:42: error: use of undeclared identifier 's'
                      ssize_t res = recv(s, buf, len, flags);
                                         ^
4 errors generated.

[destman]

As i mentions in first post problem comes from new headers that NDK ships.
They have overloadable attribute. http://clang.llvm.org/docs/AttributeReference.html#overloadable:

If a function is marked with the overloadable attribute, then all declarations and definitions of functions
with that name, except for at most one (see the note below about unmarked overloads), must have
the overloadable attribute. In addition, redeclarations of a function with the overloadable attribute must have the overloadable attribute, and redeclarations of a function without the overloadable attribute must not have the overloadable attribute.
Support for unmarked overloads is not present in some versions of clang. You may query for it using __has_extension(overloadable_unmarked).

[bagder]

Ah, thanks, now I get it. That's makes it a little tricky to make the detection work.

So does the check work if the prototype in the check is removed completely? (If so, we could remove it with preprocessor magic)

[destman]

Removing prototype also helps :)
I change aciclude.m4 like this (recv, send, select, getnameinfo are also commented):

#define RECVCALLCONV
#endif
                      //extern $recv_retv RECVCALLCONV
                      //recv($recv_arg1, $recv_arg2, $recv_arg3, $recv_arg4);
                    ]],[[
                      $recv_arg1 s=0;
                      $recv_arg2 buf=0;

Here is part of config.log:

configure:29674: checking for recv
configure:29712: clang -o conftest -O3 -isystem /Users/destman/tmp/build/Android/arm/include -Qunused-arguments    -lc++_static -lm -ldl -latomic -L/Users/destman/tmp/build/Android/arm/lib   conftest.c -lssl -lcrypto -lz  >&5
configure:29712: $? = 0
configure:29714: result: yes
configure:29729: checking types of args and return type for recv
configure:29786: clang -c -O3 -isystem /Users/destman/tmp/build/Android/arm/include -Qunused-arguments    conftest.c >&5
configure:29786: $? = 0
configure:29800: result: int,char *,size_t,int,int

[bagder]

Hm, removing the prototype completely turns out to be a bad idea. Removing the proto actually makes the test succeed on the first attempt for my test on Linux, as it is the proto is what causes the typical errors. It renders the test rather useless.

But also, if we provide the __attribute__ on the prototype in there, doesn't that also make the test succeed with whatever is thrown at it? I mean if we'd do it like this:

--- a/acinclude.m4
+++ b/acinclude.m4
@@ -1381,12 +1381,16 @@ AC_DEFUN([CURL_CHECK_FUNC_RECV], [
 #include <sys/types.h>
 #endif
 #ifdef HAVE_SYS_SOCKET_H
 #include <sys/socket.h>
 #endif
+#ifdef __ANDROID__
+#define __attribute__((overloadable))
+#else
 #define RECVCALLCONV
 #endif
+#endif
                       extern $recv_retv RECVCALLCONV
                       recv($recv_arg1, $recv_arg2, $recv_arg3, $recv_arg4);
                     ]],[[
                       $recv_arg1 s=0;
                       $recv_arg2 buf=0;

Not exactly sure what the best way to proceed from this is.

[destman]

Inserting __attribute__((overloadable)) before name of function works fine:

@@ -1083,9 +1083,13 @@
 #include <netdb.h>
 #endif
 #define GNICALLCONV
 #endif
-                    extern int GNICALLCONV getnameinfo($gni_arg1, $gni_arg2,
+                    extern int GNICALLCONV 
+#ifdef __ANDROID__
+__attribute__((overloadable))
+#endif
+				getnameinfo($gni_arg1, $gni_arg2,
                                            char *, $gni_arg46,
                                            char *, $gni_arg46,
                                            $gni_arg7);
                   ]],[[
@@ -1387,8 +1391,11 @@
 #endif
 #define RECVCALLCONV
 #endif
                       extern $recv_retv RECVCALLCONV
+#ifdef __ANDROID__
+__attribute__((overloadable))
+#endif
                       recv($recv_arg1, $recv_arg2, $recv_arg3, $recv_arg4);
                     ]],[[
                       $recv_arg1 s=0;
                       $recv_arg2 buf=0;
@@ -1521,8 +1528,11 @@
 #endif
 #define SENDCALLCONV
 #endif
                       extern $send_retv SENDCALLCONV
+#ifdef __ANDROID__
+__attribute__((overloadable))
+#endif
                       send($send_arg1, $send_arg2, $send_arg3, $send_arg4);
                     ]],[[
                       $send_arg1 s=0;
                       $send_arg3 len=0;
@@ -2377,13 +2387,17 @@
                       long tv_sec;
                       long tv_usec;
                     };
 #endif
-                    extern $sel_retv SELECTCALLCONV select($sel_arg1,
-                                                           $sel_arg234,
-                                                           $sel_arg234,
-                                                           $sel_arg234,
-                                                           $sel_arg5);
+                    extern $sel_retv SELECTCALLCONV 
+#ifdef __ANDROID__
+__attribute__((overloadable))
+#endif
+			select($sel_arg1,
+					$sel_arg234,
+					$sel_arg234,
+					$sel_arg234,
+					$sel_arg5);
                   ]],[[
                     $sel_arg1   nfds=0;
                     $sel_arg234 rfds=0;
                     $sel_arg234 wfds=0;

[bagder]

Okay, cool! But you mentioned this is news in some Android NDK version and exists in r15b, right? So won't this risk breaking the build for older Android builds? Should the #ifdef in there be more complicated?

[destman]

Maybe it is better to add new one. I did not see any define that is passed to compiler if unified headers are used.
Here is history of this madness:

https://android.googlesource.com/platform/ndk/+/master/docs/UnifiedHeaders.md#Supporting-Unified-Headers-in-Your-Build-System
Before NDK r14, we had a set of libc headers for each API version. In many cases these headers were incorrect. Many exposed APIs that didn‘t exist, and others didn’t expose APIs that did.

In NDK r14 (as an opt in feature) we unified these into a single set of headers, called unified headers. This single header path is used for every platform level. API level guards are handled with #ifdef. These headers can be found in prebuilts/ndk/headers.

Unified headers are built directly from the Android platform, so they are up to date and correct (or at the very least, any bugs in the NDK headers will also be a bug in the platform headers, which means we're much more likely to find them).

In r15 unified headers are used by default. In r16, the old headers have been removed.

[bagder]

And there was no header earlier that we could use to detect a pre-r15 version?

[bagder]

Hm, I suppose an unknown __attribute__ there will only generate a warning anyway so hopefully it won't cause a problem...

[bagder]

Thanks!