kiwix / kiwix-build

Kiwix & openZIM build engine
GNU General Public License v3.0
86 stars 42 forks source link

Exclude libgcc and libunwind from linked libraries on Android. #704

Closed mgautierfr closed 2 months ago

mgautierfr commented 2 months ago

As said in [1], unwinder is linked automatically by Clang and shared library must not re-export it.

ndk after r23 (we are using r21) should do this. Else we must take care to not re-export it.

From [2], we should pass the flags Wl,--exclude-libs,libgcc.a and -Wl,--exclude-libs,libunwind.a to avoid re-export _Unwind_Resume methods.

Fix https://github.com/kiwix/kiwix-android/issues/3661

[1] https://android.googlesource.com/platform/ndk/+/master/docs/BuildSystemMaintainers.md#Unwinding [2] https://github.com/android/ndk/issues/889#issuecomment-464334469

kelson42 commented 2 months ago

@mgautierfr I guess it fixes the unwind error, but not the crash scenario?

mgautierfr commented 2 months ago

@MohitMaliFtechiz, can you test with build libraries in https://tmp.kiwix.org/ci/dev_preview/fix_android_compilation please ?

I guess it fixes the unwind error, but not the crash scenario?

The crash is due to the broken unwind. So it also fixes the crash. A small repro case made by @MohitMaliFtechiz now don't crash on my side with this fix.

MohitMaliFtechiz commented 2 months ago

@mgautierfr, @kelson42 I have tested the test binary that you provided me and also with the https://tmp.kiwix.org/ci/dev_preview/fix_android_compilation, and the application is not crashing with these binary on the arm architecture. I can read the zim file without crashing. Also, the testZim is now starts showing on the library screen so the crash scenario is fixed for the arm architecture.

Screenshot from 2024-06-05 10-48-41 Screenshot from 2024-06-05 10-48-29

mgautierfr commented 2 months ago

Perfect. I'm merging.