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

Incorrectly choosing 64 bit function signatures. #364

Closed
gpx1000 opened this issue Apr 11, 2017 · 6 comments
Closed

Incorrectly choosing 64 bit function signatures. #364

gpx1000 opened this issue Apr 11, 2017 · 6 comments
Assignees
Milestone

Comments

@gpx1000
Copy link

gpx1000 commented Apr 11, 2017

The latest beta NDK seems to chose ftello64 and fseeko64 when building a armeabi-v7a making the clang link step fail with an undefined reference.
To repo this, attempt to build minizip as a static lib with clang then link that static lib anywhere. Observe that it works with ndk 14 (stable) but fails with 15.0.3869509 RC1

Environment Details

Not all of these will be relevant to every bug, but please provide as much
information as you can.

  • NDK Version: 15.0.3869609-beta1
  • Build sytem: cmake
  • Host OS: Windows
  • Compiler: clang
  • ABI: armeabi-v7a
  • STL: c++_static/c++_shared
  • NDK API level: 23
  • Device API level: 24
@enh
Copy link
Contributor

enh commented Apr 11, 2017

are you sure you didn't define _FILE_OFFSET_BITS=64? we'll need a reproduceable test case...

@DanAlbert DanAlbert self-assigned this Apr 17, 2017
@DanAlbert
Copy link
Member

Assuming this is the _FILE_OFFSET_BITS=64 issue, we've actually already fixed this in bionic, it just hasn't made it to the NDK. If that's the case, you can still use the deprecated headers in r15 beta 1 to avoid this problem. https://android.googlesource.com/platform/ndk/+/master/docs/UnifiedHeaders.md#Using-Unified-Headers will tell you how.

We'll have that problem fixed in beta 2, at which point you'll definitely want to give the new headers another shot because the old headers will be going away in r16.

@DanAlbert DanAlbert added this to the r15 milestone Apr 17, 2017
@gpx1000
Copy link
Author

gpx1000 commented Apr 17, 2017

Confirmed that the Unified headers causes this bug in Beta 1.

@alexcohn
Copy link

alexcohn commented Apr 24, 2017

FWIW, we do use minizip and our fix was to modify one line:

-#if (!defined(_WIN32)) && (!defined(WIN32)) && (!defined(__APPLE__))
+#if (!defined(_WIN32)) && (!defined(WIN32)) && (!defined(__APPLE__)) && (!defined(ANDROID))

(actually, we had -DIOAPI_NO_64 even before).

@gpx1000
Copy link
Author

gpx1000 commented Apr 24, 2017

@alexcohn thanks for the tip. That does look like a decent change to solve the same problem. I doubt we'd have a need to unzip a file that's larger than 4gigs so it should be valid in all usecases.
That said, I'm opting for avoiding making a custom change to a multi-project dependent library and instead turning off unified headers until r16 is released.

@DanAlbert
Copy link
Member

We've merged the latest headers into r15.

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

4 participants