sqlcipher / android-database-sqlcipher

Android SQLite API based on SQLCipher
https://www.zetetic.net/sqlcipher/sqlcipher-for-android/
Other
2.73k stars 564 forks source link

Support arm64v8a and x86_64 (64-bit) in Sqlcipher #151

Closed gmetal closed 8 years ago

gmetal commented 9 years ago

With the 64-bit capable android devices already out there, it is necessary to be able to have native 64-bit compilation for increased performance. MIPS64 has not been included, because the corresponding AOSP version does not seem to compile.

developernotes commented 9 years ago

Hi @gmetal,

Thanks so much for this pull request, it is quite large! We look forward to reviewing and getting back with any feedback we may have.

eeVoskos commented 9 years ago

Any news on this? It would be a nice feature to have.

developernotes commented 9 years ago

Hi @gmetal

With regard to this pull request, due to the number of changes we would need you to complete a contributor agreement before we can proceed, more information can be found here.

Next, we would like to ask you to par down the changes to only those required to adding support for arm64v8a and x86_64. You have included additional changes such as including alternative source locations for OpenSSL which we would prefer not to include. Also, there are changes to the minimum SDK version, we plan to continue to support Android 2.1 and up. As a point of reference, please make sure your changes are compatible with the current SQLCipher for Android test suite.

gmetal commented 9 years ago

Hello, I sent the contributor agreement earlier today. Hope that is in order.

Now in terms of the patch, I removed the partial mips64 support and the alternative OpenSSL source location.

My patch does not change the minimum SDK version. However, since 64-bit support was introduced in android-21, I did include some alternative submodules needed to compile the 64-bit version libraries. However, if you have a closer look, you'll see that the 32-bit versions of the library have not been touched, and are being built like they used too.

I also had my patched tested with the test suite, on an x86_64 emulator and on Nexus 9 (arm64-v8a) and in both cases all tests were successful.

developernotes commented 9 years ago

Hello @gmetal,

Again, thank you for the time you have put into this pull request, we do certainly appreciate it. After reviewing the contents I do have one concern moving forward. We prefer to build all OpenSSL binary artifacts from source based on our submodule checkout, assembled using this script for the various platforms needed.

Your pull request includes libssl.a binaries for both arm64-v8a and x86_64 which are not used by SQLCipher, as well as libcrypto.a binaries which are not built from the script referenced above. It is important that we are able to rebuild libcrypto.a for all supported platforms in an ongoing basis. Without this we have no means to validate the origins of the binaries, nor update them as newer versions of OpenSSL are released.

Can you provide any feedback on this?

gmetal commented 9 years ago

Hello @developernotes

The included libcrypto.a and libssl.a have been built from the AOSP openssl repository, which provides all the necessary support for building openssl for all AOSP platforms. I can understand not wanting to include such binaries in the codebase. However, after doing a quick check on the OpenSSL releases, it appears that they started supporting the arm64-v8a architecture only very recently (e.g. on the 1.0.2a). I can have a go at updating the build script with arm64-v8a support, but I will also have to bump the openssl repository to the 1.0.2a release.

developernotes commented 9 years ago

Hi @gmetal,

That would be great, please note the OpenSSL submodule in external/openssl has already been updated to the 1.0.2a tag with our latest release.

gmetal commented 9 years ago

Hello @developernotes, I have updated the build script such that it patches the openssl module's configure scripts to enable the 64-bit support. I've also made a pull request with openssl - https://github.com/openssl/openssl/pull/251 - although I am pretty sure that it will take some time before (if even) it is accepted. In the meanwhile I've included the patch at the root of the project. It is applied automatically by the build script. I've also updated the libcrypto.a binaries for the 64-bit builds, with those produced by this process. Have a look and let me know if this is acceptable.

developernotes commented 9 years ago

Hello @gmetal

First, I would like to thank you for your interest in extending SQLCipher for Android to support additional platforms. It looks like you've done a great job of wrapping everything up tightly, including submitting the patch to OpenSSL. We will monitor your pending openssl/openssl#251 pull request and once the OpenSSL has applied your patch for x86_64 and arm64-v8a support in an official release we will integrate the pull request into the upstream SQLCipher for Android repository. Thanks again for all your hard work.

ZhuYuxuan commented 9 years ago

Should I tell you that you sent this email with wrong address I'm not @gmetal,please have a check.

    通过 Android 版 Outlook 发送

On Wed, Apr 22, 2015 at 7:05 AM -0700, "Nick Parker" notifications@github.com wrote: Hello @gmetal

First, I would like to thank you for your interest in extending SQLCipher for Android to support additional platforms. It looks like you've done a great job of wrapping everything up tightly, including submitting the patch to OpenSSL. We will monitor your pending openssl/openssl#251 pull request and once the OpenSSL has applied your patch for x86_64 and arm64-v8a support in an official release we will integrate the pull request into the upstream SQLCipher for Android repository. Thanks again for all your hard work.


Reply to this email directly or view it on GitHub: https://github.com/sqlcipher/android-database-sqlcipher/pull/151#issuecomment-95191044

developernotes commented 9 years ago

Hello @ZhuYuxuan

Thanks for sharing, that must have been a mistake within the Github Issues system.

lkjh654 commented 9 years ago

@gmetal I'm not able to build the library from sources.

I have checked on the following platforms: OSX 10.10.3 Ubuntu-14.04.2 (32 and 64bit) I have tried to use the pre-built openssl library and to build it on my own (build-openssl-libraries.sh).

The error is always the same:

[arm64-v8a] StaticLibrary  : libicuuc.a
[arm64-v8a] SharedLibrary  : libsqlcipher_android.so
/home/sh/tmp/android-database-sqlcipher/external/android-libs/arm64-v8a/libcrypto.a: error adding symbols: File in wrong format
collect2: error: ld returned 1 exit status
make[1]: *** [/home/sh/tmp/android-database-sqlcipher/obj/local/arm64-v8a/libsqlcipher_android.so] Error 1
make[1]: Leaving directory `/home/sh/tmp/android-database-sqlcipher/external'
make: *** [build-external] Error 2
gmetal commented 9 years ago

@lkjh654 I've updated the libcrypto libraries and the build script. It appears that the previous script was not cleaning up properly, leading to invalid static binaries.

lkjh654 commented 9 years ago

@gmetal Yes, it's working for me now. Thanks!

anti43 commented 9 years ago

@lkjh654 would you mind sharing your build result ?

Regards,

Andreas

lkjh654 commented 9 years ago

@anti43 here you are: https://dl.dropboxusercontent.com/u/674187/libs.zip but I hope you're aware you shouldn't use any binaries from unknown sources :)

anti43 commented 9 years ago

@lkjh654 thanks, its just to confirm the Samsnung S6 would work with this fix :)

anti43 commented 9 years ago

@lkjh654 hey, does this really work for you ? I get some

failed: Cannot load library: soinfo_relocate(linker.cpp:980): cannot locate symbol "__exidx_end" referenced by "libsqlcipher_android.so"...

on app start

lkjh654 commented 9 years ago

@anti43 It did work for S6, but unfortunately not for 32bit phones.

I added LOCAL_LDFLAGS += -fuse-ld=bfd to force the linker and compiled again. It seems to fix the issue.

I've updated the package on Dropbox, can you try?

gmetal commented 9 years ago

@lkjh654 Can you please be a bit more specific about the problem you faced so that I can update the source accordingly?

lkjh654 commented 9 years ago

Hi @gmetal

Yes, sure. I've encountered the same issue as @anti43 on 32bit phones:

failed: Cannot load library: soinfo_relocate(linker.cpp:980): cannot locate symbol "__exidx_end" referenced by "libsqlcipher_android.so"...

I googled it and it is supposed to be caused by some linker issue in the new versions of the NDK (https://groups.google.com/forum/#!topic/android-ndk/oaq3jGe1XT0)

Here are my changes: https://gist.github.com/lkjh654/4332d65d10fb4404ce37

gmetal commented 9 years ago

@lkjh654 I committed your fix, apart from the changes to the project.properties, which are not needed. I've verified that the code completes the test application without problems.

Thanks for pointing out the issue and the solution.

brodybits commented 9 years ago

I just issued pull request #172 thanks to comments by @lkjh654. I just discovered it may overlap 40fea00d67365eead76d885c88653d489ecf091d by @gmetal but PR #172 can be directly merged. UPDATE: alternative would be to cherry-pick 40fea00d67365eead76d885c88653d489ecf091d.

anti43 commented 9 years ago

@lkjh654 I can confirm it works on 32bit again; have no dev access to GS6 now so will have to try later this week with 64bit! Thank you!

brodybits commented 8 years ago

I just filed openssl/openssl#333 to ask them when they can look at the changes in openssl/openssl#251.

gmetal commented 8 years ago

@brodybits Nice. I will have a look at the MIPS64 support you are asking, but it will take a while.

brodybits commented 8 years ago

@gmetal the only reason I was asking about MIPS64 is that they already support the Android build for the MIPS (32-bit) architecture. But then someone has to test it (!)

gmetal commented 8 years ago

@brodybits Unfortunately, it appears that I cannot simply modify the openssl makefiles to get a MIPS64 binary. I've tried it with the latest ndk-10e and both the 4.9 and clang3.6 toolchains without success. It fails in the sha1-mips.S assembly file, with unrecognised assembly commands.

draekko commented 8 years ago

I had to use the following patch to get it to compile with platform android-21 and ndk r10e with a port of the patches to the latest sqlcipher (as of yesterday). I was getting errors for missing type char16_t and char32_t with compilation of the 64bit targets so i had to set the compiler to c++11 (-std=c++11). With this it now compiles.

https://copy.com/4EAumwdfv2D72ysp

developernotes commented 8 years ago

No updates at this point, we are still waiting for OpenSSL to add direct support for building arm64-v8a and x86_64 architectures before it can be merged.

On 31 Aug 2015, at 19:24, Pasin Suriyentrakorn wrote:

Any updates on merging this PR? or Do you have any other suggestion to build 64 bits binary?


Reply to this email directly or view it on GitHub: https://github.com/sqlcipher/android-database-sqlcipher/pull/151#issuecomment-136535431

Nick Parker

dustedrob commented 8 years ago

@lkjh654 Where can we download your build? I tried the dropbox link but it's not working anymore. Thanks!

kayaatakan commented 8 years ago

@developernotes Ok, so it has been some time, what is the status of support for arm64-v8a and x86_64? This means that SqlCipher does not support Samsung S6 which is out on the market quite a while now. Is there any way of using SqlCipher for those architectures at the moment? If so, how?

TheNephilim88 commented 8 years ago

@kayaatakan Why do you think its not running on Samsung Galaxy S6? Its working perfectly fine on x64-platforms - try it out on yourself... Its afaik just not optimized for x64...

developernotes commented 8 years ago

Hello @kayaatakan

Per this portion of the thread, we are still awaiting direct support for building the arm64v8a and x86_64 versions of OpenSSL. This will require changes within the OpenSSL build system which appears to still be pending. Once OpenSSL directly supports targeting those platforms we will be able to proceed in adding support to SQLCipher for Android.

kayaatakan commented 8 years ago

@developernotes So, I see that there is no direct support for arm64-v8a and x86_64 yet. But according to @TheNephilim88, he says it is not optimized for those. Is it possible to manually build the project, to get the .so files for those? Maybe by modifying below line at android-database-sqlcipher/jni/Application.mk APP_ABI := armeabi armeabi-v7a x86 to APP_ABI := armeabi armeabi-v7a x86 x86_64 arm64-v8a and build the project to get those files? Even if it is not optimized, something that can enable us to run our application on Samsung S6 for example?

atakankaya commented 8 years ago

Basically is there any way of making SqlCipher work on those devices at the moment?

dustedrob commented 8 years ago

@atakankaya I got it to work with devices like the Samsung S6 by downloading the @gmetal fork and building from source.

sjlombardo commented 8 years ago

@kayaatakan @atakankaya: What @TheNephilim88 was saying is simply that the library is not built specifically for that architecture, however, SQLCipher should run just fine on AArch64 platforms, since they will fall back to armv7a.

If you are actually seeing a crash when running on a device, then it is almost definitely related to including a different native library (i.e. other than SQLCipher) with arm64-v8a support due to Android’s preferential loading process.

To fix this, you can simply remove the packaging of any platform libraries for arm64-v8a, so that you are bundling the native libraries only for the versions supported by all components (e.g. armv7a). Once the architecture for all native components are consistent then the application should run fine on arm64-v8a devices.

Please let us know if that clarifies the situation, and if the suggestion resolves any issues you are seeing.

GuillaumeBourge commented 8 years ago

@atakankaya Can you help to build the @gmetal source or send me directly binairies please ? I have already 64bits lib in my app, i do not want modify these lib to integrate cipher in my app..

gmetal commented 8 years ago

Hello, I've setup a travis job to build my patch and upload the produced binaries in github. You can find my builds here: https://github.com/gmetal/android-database-sqlcipher/releases

GuillaumeBourge commented 8 years ago

Alright, a big thanks to you for your work and help !

2016-01-22 13:49 GMT+01:00 gmetal notifications@github.com:

Hello, I've setup a travis job to build my patch and upload the produced binaries in github. You can find my builds here: https://github.com/gmetal/android-database-sqlcipher/releases

— Reply to this email directly or view it on GitHub https://github.com/sqlcipher/android-database-sqlcipher/pull/151#issuecomment-173911418 .

kayaatakan commented 8 years ago

Hi all, sorry for the late response. I used the suggestion of @sjlombardo In my case, problem was the sqlcipher libs were unable to be found. An example: Suppose you have another library that supports arm64-v8a. You would create a directory under libs named "arm64-v8a" and put that library there. However, sqlcipher does not have any libs for this ABI, and you would expect the system to use the sqlcipher libs from your arm64-v7a folder then. But that's not the case. On installation time, the installer looks for a matching folder under your libs folder with the ABI of your device. Then just copies that folder directly. Suppose the device has arm64-v8a ABI, then the arm64-v8a folder contents would be copied. But there is no sqlcipher libs for arm64-v8a. So the app would crash since it cannot find the sqlcipher library. So my problem was not really related to Sqlcipher itself but the way I included them in my project.

sjlombardo commented 8 years ago

Hello @kayaatakan - excellent, I'm very glad that worked for you!

gmetal commented 8 years ago

@developernotes my patch to openssl has been rejected, since the functionality has been implemented in their master branch. When 1.1.0 is released, you will be able to produce 64-bit android openssl builds without any additional patches, etc.

GuillaumeBourge commented 8 years ago

Hi !

Thanks you for notice me, i will look at their version ! :)

2016-03-02 15:56 GMT+01:00 gmetal notifications@github.com:

@developernotes https://github.com/developernotes my patch to openssl has been rejected, since the functionality has been implemented in their master branch. When 1.1.0 is released, you will be able to produce 64-bit android openssl builds without any additional patches, etc.

— Reply to this email directly or view it on GitHub https://github.com/sqlcipher/android-database-sqlcipher/pull/151#issuecomment-191271505 .

developernotes commented 8 years ago

Hi @gmetal

Thank you for the follow up, we are looking forward to their 1.1.0 release. Take care!