marshmallow dynamic loader #327

Closed
wants to merge 19 commits into
from

Conversation

Projects
None yet
6 participants
@krnlyng
Member

krnlyng commented Nov 20, 2016

we have put together a working version for marshmallow based ports (mal-, ghosalmartin, Notkit)
#326 is required
it was suggested to add ghosalmartin/libhybris@cd5aacb
and some of https://github.com/ghosalmartin/libhybris/commits/master?author=vicamo maybe in a different PR.

if it is not merged it shall be at least for reference, since we have been spreading code over many repositories
if one other hand it is considered for merging, let me know what needs to change.

the hooks might be confusing:

  • i removed some duplicates
  • moved some to their header comment
  • fixed a typo

the linker is not build with -std=gnu++11 but with -std=c++0x (only a minor tweak was required)

@ghosalmartin

This comment has been minimized.

Show comment
Hide comment
@ghosalmartin

ghosalmartin Nov 21, 2016

lgtm, the arm64 stuff can wait, I don't think its vital since I've run this on a pure arm64 rootfs with no issues arising from the lack of commits

lgtm, the arm64 stuff can wait, I don't think its vital since I've run this on a pure arm64 rootfs with no issues arising from the lack of commits

@ghosalmartin

This comment has been minimized.

Show comment
Hide comment
@ghosalmartin

ghosalmartin Nov 21, 2016

also we need to hook into:
{"__progname", &program_invocation_name},
{"pthread_getspecific", my_pthread_getspecific},

and implement

void *my_pthread_getspecific(pthread_key_t key) { if(key == 0) return 0; return pthread_getspecific(key); }

Due to https://github.com/CyanogenMod/android_bionic/blob/cm-13.0/tests/pthread_test.cpp#L185

worked all this out with lots and lots of help from krnlyng :D

ghosalmartin commented Nov 21, 2016

also we need to hook into:
{"__progname", &program_invocation_name},
{"pthread_getspecific", my_pthread_getspecific},

and implement

void *my_pthread_getspecific(pthread_key_t key) { if(key == 0) return 0; return pthread_getspecific(key); }

Due to https://github.com/CyanogenMod/android_bionic/blob/cm-13.0/tests/pthread_test.cpp#L185

worked all this out with lots and lots of help from krnlyng :D

@krnlyng

This comment has been minimized.

Show comment
Hide comment
@krnlyng

krnlyng Nov 21, 2016

Member

those two are specific to specific egl implementations, should these be added in this pr or later?
also the __progname hook is necessary since some android lib tried to strcmp(__progname, "zygote") and __progname was 0 which caused a crash.

Member

krnlyng commented Nov 21, 2016

those two are specific to specific egl implementations, should these be added in this pr or later?
also the __progname hook is necessary since some android lib tried to strcmp(__progname, "zygote") and __progname was 0 which caused a crash.

@ghosalmartin

This comment has been minimized.

Show comment
Hide comment
@ghosalmartin

ghosalmartin Nov 21, 2016

Either or really, I see no advantage of giving it even more visibility in this instance, but I could be wrong

Either or really, I see no advantage of giving it even more visibility in this instance, but I could be wrong

@morphis

This comment has been minimized.

Show comment
Hide comment
@morphis

morphis Nov 22, 2016

Member

Actually my plan was to merge https://code.launchpad.net/~libhybris-maintainers/libhybris/+git/libhybris/+ref/master at some point into the official upstream as this one has multiple improvements to make the life with libhybris a lot easier. One of its main advantages is dynamic-linker loading.

What do you guys think about this?

Member

morphis commented Nov 22, 2016

Actually my plan was to merge https://code.launchpad.net/~libhybris-maintainers/libhybris/+git/libhybris/+ref/master at some point into the official upstream as this one has multiple improvements to make the life with libhybris a lot easier. One of its main advantages is dynamic-linker loading.

What do you guys think about this?

@krnlyng

This comment has been minimized.

Show comment
Hide comment
@krnlyng

krnlyng Nov 22, 2016

Member

thats why i wrote so defensive "if it is considered for merging"

there are several advantages for this version too:

  1. it works with qualcomm devices (the referenced version crashes sometime during link time)

  2. the referenced version adds _hybris_hook to every hooked function, making functions that could be directly hooked slower

  3. it is here now

3 has an obvious solution, 1 can probably done with both versions eventually, 2 is probably nitpicking but a similar thing can be done with a tiny wrapper generator that is only active when a specific env variable is set i have accidentally put the output of that into a comment inside #325 one additional advantage of this tracing functionality is that it can print even unhooked functions when they are called

the dynamic linker loading is neat but is it necessary? i would assume the 6.0 linker can handle 5.0 binaries since i am using blobs written for 5.0 just fine with the marshmallow linker, the only real reason why the marshmallow linker is necessary seems to be android specific compressed relocations

i know there are other fixes and improvements that should be added here too, or we can merge the referenced versions and add fixes the other way round

Member

krnlyng commented Nov 22, 2016

thats why i wrote so defensive "if it is considered for merging"

there are several advantages for this version too:

  1. it works with qualcomm devices (the referenced version crashes sometime during link time)

  2. the referenced version adds _hybris_hook to every hooked function, making functions that could be directly hooked slower

  3. it is here now

3 has an obvious solution, 1 can probably done with both versions eventually, 2 is probably nitpicking but a similar thing can be done with a tiny wrapper generator that is only active when a specific env variable is set i have accidentally put the output of that into a comment inside #325 one additional advantage of this tracing functionality is that it can print even unhooked functions when they are called

the dynamic linker loading is neat but is it necessary? i would assume the 6.0 linker can handle 5.0 binaries since i am using blobs written for 5.0 just fine with the marshmallow linker, the only real reason why the marshmallow linker is necessary seems to be android specific compressed relocations

i know there are other fixes and improvements that should be added here too, or we can merge the referenced versions and add fixes the other way round

@morphis

This comment has been minimized.

Show comment
Hide comment
@morphis

morphis Nov 22, 2016

Member

It is sadly not true that the Android 6 linker can handle all Android 5 binaries. We came across some real problems we couldn't figure out.

My overall aim is that we all (Canonical, Jolla, Community) combine our efforts again around libhybris and work on a single code base, which didn't worked really well. Sadly we diverged quite a bit due to imminent product requirements and release schedule but I would like to come back and bring things back together.

The dynamic linker loading is quite essential for us as we build a single Ubuntu rootfs which is the exactly the same on all devices we support. That way we can't include different libhybris versions for different devices. Also we don't want to risk too much on devices using older Android versions as they are already in the field and doing things like switching the hybris linker is not really what we want to do in terms of maintenance.

I agree with 1. and 2. For 2. we can find a better solution and 1. is just about porting a fix over. 3. is the actual problem, give the small amount of time I have for this I would like to get as much as possible over and then let us work on a code base together.

So my proposed way of combining things would be:

  1. Merge https://code.launchpad.net/~libhybris-maintainers/libhybris/+git/libhybris/+ref/master with keepign debian packaging etc. We can add rpm packaging in here too which then allows us to get rid as much as possible of possible packaging branches.

  2. Merge any relevant patches for you guys on top and improve things where needed.

What do you think about this?

Member

morphis commented Nov 22, 2016

It is sadly not true that the Android 6 linker can handle all Android 5 binaries. We came across some real problems we couldn't figure out.

My overall aim is that we all (Canonical, Jolla, Community) combine our efforts again around libhybris and work on a single code base, which didn't worked really well. Sadly we diverged quite a bit due to imminent product requirements and release schedule but I would like to come back and bring things back together.

The dynamic linker loading is quite essential for us as we build a single Ubuntu rootfs which is the exactly the same on all devices we support. That way we can't include different libhybris versions for different devices. Also we don't want to risk too much on devices using older Android versions as they are already in the field and doing things like switching the hybris linker is not really what we want to do in terms of maintenance.

I agree with 1. and 2. For 2. we can find a better solution and 1. is just about porting a fix over. 3. is the actual problem, give the small amount of time I have for this I would like to get as much as possible over and then let us work on a code base together.

So my proposed way of combining things would be:

  1. Merge https://code.launchpad.net/~libhybris-maintainers/libhybris/+git/libhybris/+ref/master with keepign debian packaging etc. We can add rpm packaging in here too which then allows us to get rid as much as possible of possible packaging branches.

  2. Merge any relevant patches for you guys on top and improve things where needed.

What do you think about this?

@sledges

This comment has been minimized.

Show comment
Hide comment
@sledges

sledges Nov 23, 2016

Contributor

@morphis, could you please make a PR that merges your work? It is a truly welcome initiative to unify the codebase, so that we get more eyes and hands on the same bugs and challenges of the future.

We could then clearly see what the things that you need are, and how they're implemented.

Contributor

sledges commented Nov 23, 2016

@morphis, could you please make a PR that merges your work? It is a truly welcome initiative to unify the codebase, so that we get more eyes and hands on the same bugs and challenges of the future.

We could then clearly see what the things that you need are, and how they're implemented.

@morphis

This comment has been minimized.

Show comment
Hide comment
@morphis

morphis Nov 23, 2016

Member

@sledges Yes I can do that. However it will be a merge-all PR which will do a big giant merge to preserve history but ignores all conflicts.

Member

morphis commented Nov 23, 2016

@sledges Yes I can do that. However it will be a merge-all PR which will do a big giant merge to preserve history but ignores all conflicts.

@morphis

This comment has been minimized.

Show comment
Hide comment
@morphis

morphis Nov 23, 2016

Member

@sledges Done, see #328

Member

morphis commented Nov 23, 2016

@sledges Done, see #328

krnlyng and others added some commits Dec 3, 2016

hooks: add pthread_mutex_timedlock
Bug: https://bugs.launchpad.net/avila-private/+bug/1516902
Signed-off-by: You-Sheng Yang <vicamo.yang@canonical.com>
hook: add pthread_condattr_setclock
pthread_condattr_setclock() was first introduced in Android 5.0 and is
directly called by libbacktrace, which is then linked into libutil.

Signed-off-by: You-Sheng Yang <vicamo@gmail.com>
hooks: fix arm64 shared pthread ptr implementation
libhybris previously stores memory addresses of shared mutexes in a
unsigned integer. When running on arm64 or any other __LP64__ systems,
this results in wrong pointer addresses returned. In this patch we
re-type to storage type to uintptr_t, which guarantees sufficient space
for holding a memory address, and then corretly set HYBRIS_SHM_MASK and
HYBRIS_SHM_MASK_TOP to valid values on both systems.
hybris: functions which contain pthread but are not pthread functions…
… should not be hooked to negative numbers if no hook exists

@krnlyng krnlyng closed this Jan 31, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment