fixes gmic on arm by linking libz last not first

[its-pointless]

this fixes gmic on arm breaking. You are meant to link the system libs last otherwise you get interfering unwind implementations and other weirdness. WIthout this gmic doesn't work on arm device.

[tomty89]

I am not sure I get the sense of this PR. This seems all sort of random. How does the order even matter here? And how even does libz while every Termux programs and libraries links to the same libz? Is the libz in arm devices even linked to libunwind? (I know libunwind is linked to liblzma, but not even vice versa).

Since gmic is linked to libc++, I wonder if it actually has something to do with this:
https://github.com/termux/termux-packages/blob/master/build-package.sh#L757
Maybe the gmic package (or libc++ package) should pull the libunwind package because of this?
(Or maybe this is just wrong)

[its-pointless]

Gmic has undefined symbol _Unwind_Resume. If you load libz.so first you get system unwinder symbol. we do not want that. Arm uses different unwinder... which is likely why its different android/ndk#289

[tomty89]

Told you. It just needs a rebuild, not a patch:

$ readelf -Ws /system/lib64/libc++.so | grep _Unwind_Resume
  1904: 00000000000bac78   252 FUNC    GLOBAL DEFAULT   12 _Unwind_Resume
  2192: 00000000000bad74   248 FUNC    GLOBAL DEFAULT   12 _Unwind_Resume_or_Rethrow
$ readelf -Ws $PREFIX/lib/libc++_shared.so | grep _Unwind_Resume
$

[its-pointless]

No it dossn't because rebuilding it gets the same result. Iv rebuilt it several times. It was rebuilt with new libc++ 2.31 is a new build.

…

On Wed., 27 Jun. 2018, 4:39 pm tomty89, ***@***.***> wrote: Told you. It just needs a rebuild, not a patch: $ readelf -Ws /system/lib64/libc++.so | grep _Unwind_Resume 1904: 00000000000bac78 252 FUNC GLOBAL DEFAULT 12 _Unwind_Resume 2192: 00000000000bad74 248 FUNC GLOBAL DEFAULT 12 _Unwind_Resume_or_Rethrow $ readelf -Ws $PREFIX/lib/libc++_shared.so | grep _Unwind_Resume $ — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2588 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AP-Ot1__9U1R08e6RejDjHlVEWrrmBb7ks5uAyiTgaJpZM4U5FpM> .

[its-pointless]

If you look in build-package.sh it mentions arm unwind.a being added to linker script.

[tomty89]

I have quoted the line in my first comment.

That's why any libc++ linking package (gmic, graphicsmagick...) in arm could need to have libunwind in their TERMUX_PKG_DEPENDS, so that -lunwind doesn't lead them to the one in NDK.

And doing so shouldn't prevent the Android arm libz from working, since it has no dynamic linkage to libunwind.

[tomty89]

@fornwall What was the -lunwind trick actually for? Was it meant to lead the linkage to some libunwind in the NDK? The line has always seemd nasty to me. Not only because linking to Android's libunwind is known to be problematic, but also the fact that it appears to assume libc++ linking will never be done directly via libc++_shared.so; also from its comment it seem to assume the linking will only be done statically, while there's no evidence showing that that will always be the fact?

And I think the actual behavior is doomed to vary on whether Termux libunwind was built in the process?

With the recent NDK/libc++ version bump, does it invalidate the need of the trick anyway (no matter it was ever appropriate or not)?

[its-pointless]

C++ exceptions rely on unwind library.

[fornwall]

There is a rebuilt gmic package at version 2.3.1-1 now, so you can check on arm if that fixes the problem (I don't have a 32-bit arm device availalbe at the moment)?

About the history behind this, see d1f566f:

Attach arm issues with C++ exception unwinding

See https://github.com/android-ndk/ndk/issues/379

Fixes https://github.com/termux/termux-packages/issues/1163

Fixes issues with gdb segfaulting on arm on an unrecognized command.

I'm uncomfortable as well as I don't really understand the details here. Will try to read up on it, and suggestions are welcome :).

[tomty89]

Never mind the line in build-package.sh is probably not relevant (as neither of the involved packages are linked to libc++_shared.so via libstdc++.so AFAIK).

So I bump into 32-bit (arm) only problem with gmic as well, and it is indeed triggered by the order of the dependency table. It seems specific to me though, that libGraphicsMagick++ has to be ordered after libz (and the position of libGraphicsMagick and others don't seem to matter). I even to tried build libGraphicsMagick(++) without all the optional dependencies (including but not limited to libz), didn't seem to change the story.

Interestingly, a local build of libz also makes the problem go away:

$ cd gmic-2.3.2/build/
$ ls -l $PREFIX/lib/libz.*
lrwxrwxrwx 1 u0_a312 u0_a312 19 Jul  4 05:29 /data/data/com.termux/files/usr/lib/libz.so -> /system/lib/libz.so
$ rm gmic
$ make
[  0%] Built target gmic_extra_headers
[ 33%] Linking CXX executable gmic
[100%] Built target gmic
$ rm $PREFIX/lib/libz.so
$ ./gmic version


[gmic] G'MIC encountered a fatal error. Please submit a bug report, at: https://framagit.org/dtschump/gmic/issues

$ gmic version


[gmic] G'MIC encountered a fatal error. Please submit a bug report, at: https://framagit.org/dtschump/gmic/issues

$ cp $PREFIX/local/lib/libz.so* $PREFIX/lib
$ ./gmic version
[gmic]-0./ Start G'MIC interpreter.

  gmic: GREYC's Magic for Image Computing.

        Version 2.3.2, Copyright (c) 2008-2018, David Tschumperle.
        (https://gmic.eu)

[gmic]-0./ End G'MIC interpreter.
$ gmic version
[gmic]-0./ Start G'MIC interpreter.

  gmic: GREYC's Magic for Image Computing.

        Version 2.3.2, Copyright (c) 2008-2018, David Tschumperle.
        (https://gmic.eu)

[gmic]-0./ End G'MIC interpreter.
$

(Perhaps it's really time to build our own zlib?)

I no longer oppose changing the order in the Makefile, as I don't think the order is supposed to matter anyway (in other words I think this is just some Android bug). It's just that some of the details in this PR still doesn't make sense (don't we just need to make sure GMIC_CLI_LIBS starts with $(MAGICK_LIBS)? I don't know what the # before $(MAGICK_LIBS) does though, as I had hard times building gmic on Termux with src/Makefile.

I created a PR to switch to cmake as it works so much better on Termux, yet it somehow fails with some weird undeclared identifer errors on Travis CI.