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

Broken feature detection with unified headers #332

Closed
mperepelitsyn opened this issue Mar 17, 2017 · 17 comments
Closed

Broken feature detection with unified headers #332

mperepelitsyn opened this issue Mar 17, 2017 · 17 comments
Assignees
Milestone

Comments

@mperepelitsyn
Copy link

mperepelitsyn commented Mar 17, 2017

Description

I've encountered a couple of issues with unified headers in r14 that I didn't see using r13b.

  1. Boost Log
    Missing: pthread_spin_init
    Detection: #if defined(_POSIX_SPIN_LOCKS) && _POSIX_SPIN_LOCKS > 0
    Code: https://github.com/boostorg/log/blob/boost-1.58.0/include/boost/log/detail/spin_mutex.hpp#L198
    Available in API >= 24

  2. Boost Thread
    Missing: pthread_mutex_timedlock
    Detection: #if _POSIX_TIMEOUTS >= 0 && _POSIX_TIMEOUTS>=200112L
    Code: https://github.com/boostorg/thread/blob/develop/include/boost/thread/pthread/mutex.hpp#L251
    Available in API >= 21

  3. Sqlite3
    Missing: mmap
    Detection:

    # if defined(__linux__) \
      || defined(_WIN32) \
      || (defined(__APPLE__) && defined(__MACH__)) \
      || defined(__sun) \
      || defined(__FreeBSD__) \
      || defined(__DragonFly__)
    #   define SQLITE_MAX_MMAP_SIZE 0x7fff0000  /* 2147418112 */
    # else
    #   define SQLITE_MAX_MMAP_SIZE 0
    # endif
    

    Available in API >= 21

Environment Details

  • NDK Version: 14.0.3770861
  • Build sytem: custom
  • Host OS: Linux
  • Compiler: Clang
  • ABI: x86
  • STL: gnustl_shared
  • NDK API level: 19
  • Device API level: 19
@enh
Copy link
Contributor

enh commented Mar 17, 2017

for the POSIX* stuff, does boost do the right thing and fall back to sysconf if we leave them undefined (since deleting these lines would be the quick fix)? i'm assuming not.

the sqlite one doesn't make sense. did you copy & paste the wrong section of the file?

@DanAlbert
Copy link
Member

@jmgao: think you could use the versioner to apply guards to all the sysconf stuff?

@jmgao
Copy link
Contributor

jmgao commented Mar 17, 2017

Don't think so, this probably has to be annotated by hand.

@DanAlbert DanAlbert assigned DanAlbert and unassigned jmgao Mar 17, 2017
@DanAlbert
Copy link
Member

I'm guessing no volunteers for that...

@mperepelitsyn
Copy link
Author

@enh updated the description to include links to relevant code. If those macros are undefined, the features won't be used even if they are available for the API level.

Regarding sqlite3, it just assumes that mmap is here if __linux__ is defined.

sys/mman.h:

#if defined(__USE_FILE_OFFSET64)

#if __ANDROID_API__ >= 21
void* mmap(void*, size_t, int, int, int, off_t) __RENAME(mmap64) __INTRODUCED_IN(21);
#endif /* __ANDROID_API__ >= 21 */

#else
void* mmap(void*, size_t, int, int, int, off_t);
#endif

And in my case I think I happen to have __USE_FILE_OFFSET64 defined but __ANDROID_API__ is 19.

@DanAlbert
Copy link
Member

Ah, so same problem as we had in #324 for mmap. That one should at least be easy to fix.

hubot pushed a commit to aosp-mirror/platform_bionic that referenced this issue Mar 20, 2017
Without this, setting `__USE_FILE_OFFSET64` and targeting pre-L made
mmap entirely unavailable.

Test: make checkbuild
Bug: android/ndk#332
Change-Id: I9f61c44f8d9ab5c7cae845c9f89a7d889c6df365
@enh
Copy link
Contributor

enh commented Mar 21, 2017

https://android-review.googlesource.com/355702 should fix these and others.

hubot pushed a commit to aosp-mirror/platform_bionic that referenced this issue Mar 21, 2017
Bug: android/ndk#332
Test: builds
Change-Id: I249c214d34244a1149ba6b1160e8eafc2cdbcdea
@4brunu
Copy link

4brunu commented Mar 22, 2017

I'm also having an error building SQLite in SQLiteCpp.

Not sure if it's the same issue, or if I should open another issue.

Information:(11708, 38) expanded from macro 'ArraySize'
#define ArraySize(X)    ((int)(sizeof(X)/sizeof(X[0])))
                                     ^~~
/Users/user/Developer/MobileCPPBaseProject/deps/SQLiteCpp/sqlite3/sqlite3.c:29693:14: error: invalid application of 'sizeof' to an incomplete type 'struct unix_syscall []'
  for(i++; i<ArraySize(aSyscall); i++){
             ^~~~~~~~~~~~~~~~~~~
/Users/user/Developer/MobileCPPBaseProject/deps/SQLiteCpp/sqlite3/sqlite3.c:11708:38: note: expanded from macro 'ArraySize'
#define ArraySize(X)    ((int)(sizeof(X)/sizeof(X[0])))
                                     ^~~
6 errors generated.
ninja: build stopped: subcommand failed.
:app:externalNativeBuildDebug FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':app:externalNativeBuildDebug'.
> Build command failed.
  Error while executing process /Applications/android-sdk/cmake/3.6.3155560/bin/cmake with arguments {--build /Users/user/Developer/MobileCPPBaseProject/android_project/MobileCPPAndroid/app/.externalNativeBuild/cmake/debug/armeabi-v7a --target mobilecpp}
  [1/40] Building CXX object deps/json11/CMakeFiles/json11.dir/json11.cpp.o
  [2/40] Building CXX object deps/SQLiteCpp/CMakeFiles/SQLiteCpp.dir/src/Backup.cpp.o
  [3/40] Building CXX object deps/SQLiteCpp/CMakeFiles/SQLiteCpp.dir/src/Column.cpp.o
  [4/40] Building CXX object deps/SQLiteCpp/CMakeFiles/SQLiteCpp.dir/src/Database.cpp.o
  [5/40] Building CXX object deps/SQLiteCpp/CMakeFiles/SQLiteCpp.dir/src/Exception.cpp.o
  [6/40] Building CXX object deps/SQLiteCpp/CMakeFiles/SQLiteCpp.dir/src/Statement.cpp.o
  [7/40] Building CXX object deps/SQLiteCpp/CMakeFiles/SQLiteCpp.dir/src/Transaction.cpp.o
  [8/40] Building C object deps/SQLiteCpp/sqlite3/CMakeFiles/sqlite3.dir/sqlite3.c.o
  [9/40] Building CXX object deps/djinni/support-lib/CMakeFiles/djinni.dir/jni/djinni_support.cpp.o
  [10/40] Building CXX object deps/djinni/support-lib/CMakeFiles/djinni.dir/jni/djinni_main.cpp.o
  /Users/user/Developer/MobileCPPBaseProject/deps/SQLiteCpp/src/Database.cpp:31:13: warning: unused variable 'OPEN_MEMORY' [-Wunused-const-variable]
1 warning generated.
[11/40] Linking CXX static library deps/SQLiteCpp/libSQLiteCpp.a
[12/40] Linking CXX shared library ../../../../build/intermediates/cmake/debug/obj/armeabi-v7a/libdjinni.so
FAILED: /Users/user/Developer/MobileCPPBaseProject/android_project/MobileCPPAndroid/app/.externalNativeBuild/cmake/debug/armeabi-v7a/launch-c /Applications/android-sdk/ndk-bundle/toolchains/llvm/prebuilt/darwin-x86_64/bin/clang  --target=armv7-none-linux-androideabi --gcc-toolchain=/Applications/android-sdk/ndk-bundle/toolchains/arm-linux-androideabi-4.9/prebuilt/darwin-x86_64 --sysroot=/Applications/android-sdk/ndk-bundle/sysroot -DSQLITE_ENABLE_COLUMN_METADATA -I../../../../../../../deps/SQLiteCpp/include -I../../../../../../../deps/SQLiteCpp/sqlite3 -isystem /Applications/android-sdk/ndk-bundle/sysroot/usr/include/arm-linux-androideabi -D__ANDROID_API__=14 -g -DANDROID -ffunction-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -march=armv7-a -mfloat-abi=softfp -mfpu=vfpv3-d16 -fno-integrated-as -mthumb -Wa,--noexecstack -Wformat -Werror=format-security -isystem /Applications/android-sdk/ndk-bundle/sysroot/usr/include/arm-linux-androideabi -D__ANDROID_API__=14 -g -DANDROID -ffunction-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -march=armv7-a -mfloat-abi=softfp -mfpu=vfpv3-d16 -fno-integrated-as -mthumb -Wa,--noexecstack -Wformat -Werror=format-security  -Wall -O0 -fno-limit-debug-info -O0 -fno-limit-debug-info    -fstack-protector -Wall -Winit-self -Wswitch-enum -Wshadow -Winline -MD -MT deps/SQLiteCpp/sqlite3/CMakeFiles/sqlite3.dir/sqlite3.c.o -MF deps/SQLiteCpp/sqlite3/CMakeFiles/sqlite3.dir/sqlite3.c.o.d -o deps/SQLiteCpp/sqlite3/CMakeFiles/sqlite3.dir/sqlite3.c.o   -c /Users/user/Developer/MobileCPPBaseProject/deps/SQLiteCpp/sqlite3/sqlite3.c
/Users/user/Developer/MobileCPPBaseProject/deps/SQLiteCpp/sqlite3/sqlite3.c:29559:42: error: use of undeclared identifier 'mmap'
{ "mmap",         (sqlite3_syscall_ptr)mmap,            0 },
^
/Users/user/Developer/MobileCPPBaseProject/deps/SQLiteCpp/sqlite3/sqlite3.c:29636:22: error: invalid application of 'sizeof' to an incomplete type 'struct unix_syscall []'
for(i=0; i<sizeof(aSyscall)/sizeof(aSyscall[0]); i++){
^~~~~~~~~~
/Users/user/Developer/MobileCPPBaseProject/deps/SQLiteCpp/sqlite3/sqlite3.c:29645:22: error: invalid application of 'sizeof' to an incomplete type 'struct unix_syscall []'
for(i=0; i<sizeof(aSyscall)/sizeof(aSyscall[0]); i++){
^~~~~~~~~~
/Users/user/Developer/MobileCPPBaseProject/deps/SQLiteCpp/sqlite3/sqlite3.c:29672:20: error: invalid application of 'sizeof' to an incomplete type 'struct unix_syscall []'
for(i=0; i<sizeof(aSyscall)/sizeof(aSyscall[0]); i++){
^~~~~~~~~~
/Users/user/Developer/MobileCPPBaseProject/deps/SQLiteCpp/sqlite3/sqlite3.c:29689:16: error: invalid application of 'sizeof' to an incomplete type 'struct unix_syscall []'
for(i=0; i<ArraySize(aSyscall)-1; i++){
^~~~~~~~~~~~~~~~~~~
/Users/user/Developer/MobileCPPBaseProject/deps/SQLiteCpp/sqlite3/sqlite3.c:11708:38: note: expanded from macro 'ArraySize'
#define ArraySize(X)    ((int)(sizeof(X)/sizeof(X[0])))
^~~
/Users/user/Developer/MobileCPPBaseProject/deps/SQLiteCpp/sqlite3/sqlite3.c:29693:14: error: invalid application of 'sizeof' to an incomplete type 'struct unix_syscall []'
for(i++; i<ArraySize(aSyscall); i++){
^~~~~~~~~~~~~~~~~~~
/Users/user/Developer/MobileCPPBaseProject/deps/SQLiteCpp/sqlite3/sqlite3.c:11708:38: note: expanded from macro 'ArraySize'
#define ArraySize(X)    ((int)(sizeof(X)/sizeof(X[0])))
^~~
6 errors generated.
ninja: build stopped: subcommand failed.

Environment Details

NDK Version: 14.0.3770861
Build sytem: CMake
Host OS: macOS
Compiler: Clang
ABI: x86
STL: c++_static

@mperepelitsyn
Copy link
Author

@4brunu, looks the same, you are missing mmap

@DanAlbert
Copy link
Member

Just a heads up: this is going to be in r15 but won't be in beta 1. We've got a bunch of new code in the headers (better FORTIFY support) that requires a Clang newer than the one we have in the NDK right now and the new Clang has some issues with arm5, so we're not taking that update just yet. We should have another update really soon, but it won't be in time for beta 1. Apologies for the inconvenience!

@DanAlbert DanAlbert added this to the r15 milestone Mar 24, 2017
@Zingam
Copy link

Zingam commented Apr 8, 2017

r16 - Remove deprecated headers <- Does this imply that the specific
per version duplicated header files and sets in "platforms" will disappear?

@enh
Copy link
Contributor

enh commented Apr 8, 2017

@Zingam yes, the duplicated headers will be removed as soon as we can get folks switched over to the up-to-date unified headers (which are basically the same headers we use to build the OS itself).

@alexcohn
Copy link

FWIW, we build SQlite3 adding -D_FILE_OFFSET_BITS=32

hubot pushed a commit to aosp-mirror/platform_bionic that referenced this issue Jul 10, 2017
We had several bugs filed saying "if I set _FILE_OFFSET_BITS=64 when
targeting an API < L, various functions are missing". Instead of
saying "yes, they are", we quietly just modified the header files to
expose the non-64-bit variants. This makes no sense. We can't just say
"oh, yeah, we don't have a version of this function that agrees with
your calling code about how large off_t is, but here's a version that
doesn't: I'm sure it'll be fine".

_FILE_OFFSET_BITS=64 on Android LP32 has always been a game of chance,
but that game should be "are all the functions my code needs available
at compile time?", not "will my code actually work at run time?".

Bug: android/ndk#449
Bug: android/ndk#442
Bug: android/ndk#333
Bug: android/ndk#332
Bug: android/ndk#324
Test: builds
Change-Id: Ib095251d3e21e77ed50cc3575388107fecec4ecd
@ghost
Copy link

ghost commented Oct 5, 2017

@alexcohn , how Adding -D_FILE_OFFSET_BITS=32 solves the issue ? This solution is working but how ?

@enh
Copy link
Contributor

enh commented Oct 5, 2017

he means it fixes "missing mmap" problems, because there was a 32-bit off_t mmap in old API levels. you only need new API levels if you want a 64-bit off_t.

fmsmartcommerce added a commit to nexmart/android-sqlite-native-driver that referenced this issue Nov 10, 2017
@noangel
Copy link

noangel commented Nov 15, 2017

updated to NDK 16 release today and got this in sqlite.c:

sqlite3.c:27147:14: error: call to 'mmap' declared with attribute error: mmap is not available with _FILE_OFFSET_BITS=64 when using GCC until android-21. Either raise your minSdkVersion, disable _FILE_OFFSET_BITS=64, or switch to Clang.
pMem = mmap(0, szRegion,

solved by adding

#ifdef __ANDROID__
#define SQLITE_DISABLE_LFS
#endif

to head of sqlite.c.

@enh
Copy link
Contributor

enh commented Nov 15, 2017

switching to clang would be a better choice. we're removing GCC in r18, so the sooner you switch the better. (we might want to change the error message to just say "switch to clang"...)

sepehrst pushed a commit to spsforks/android-bionic-libc that referenced this issue Apr 22, 2024
Without this, setting `__USE_FILE_OFFSET64` and targeting pre-L made
mmap entirely unavailable.

Test: make checkbuild
Bug: android/ndk#332
Change-Id: I9f61c44f8d9ab5c7cae845c9f89a7d889c6df365
sepehrst pushed a commit to spsforks/android-bionic-libc that referenced this issue Apr 22, 2024
We had several bugs filed saying "if I set _FILE_OFFSET_BITS=64 when
targeting an API < L, various functions are missing". Instead of
saying "yes, they are", we quietly just modified the header files to
expose the non-64-bit variants. This makes no sense. We can't just say
"oh, yeah, we don't have a version of this function that agrees with
your calling code about how large off_t is, but here's a version that
doesn't: I'm sure it'll be fine".

_FILE_OFFSET_BITS=64 on Android LP32 has always been a game of chance,
but that game should be "are all the functions my code needs available
at compile time?", not "will my code actually work at run time?".

Bug: android/ndk#442
Bug: android/ndk#333
Bug: android/ndk#332
Bug: android/ndk#324
Test: builds
Change-Id: Ib095251d3e21e77ed50cc3575388107fecec4ecd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants