search

servo / servo

Servo, the embeddable, independent, memory-safe, modular, parallel web rendering engine
https://servo.org
Mozilla Public License 2.0
26.25k stars 2.92k forks source link

android: use `jemalloc` on Android #32273

Closed mukilan closed 2 weeks ago

mukilan commented 2 weeks ago

This is a fix for the crash issue in 64-bit ARM #32175 and is essentially a revert of 5395c3e from the first Android PR.

When targeting Android 11 and above, 64-bit ARM platforms have the 'Tagged Pointer' feature enabled by default which causes memory allocated using the system allocator to have a non-zero 'tag' set in the highest byte of heap addresses.

This is incompatible with SpiderMonkey which assumes that only the bottom 48 bits are set and asserts this at various points.

Both Servo and Gecko have a similar architecture where the pointer to a heap allocated DOM struct is encoded as a JS::Value and stored in the DOM_OBJECT_SLOT (reserved slot) of the JSObject which reflects the native DOM struct.

As observed in #32175, even Gecko crashes with jemalloc disabled which suggests that support for using the native system allocator with tagged pointers enabled by default is not present at the moment.

sagudev commented 2 weeks ago

I think android used to have jemalloc as system allocator, but that changed in recent years. I wonder what would happen if we use ASAN on android?

mrobinson commented 2 weeks ago

@mukilan It looks like we disabled jemalloc for Android in #31086, but I cannot recall why not. Was it a build issue?

mrobinson commented 2 weeks ago

@mukilan It looks like we disabled jemalloc for Android in #31086, but I cannot recall why not. Was it a build issue?

It's explained in this PR :facepalm:. The issue is that jemalloc-sys has an implicit binary dependency on libgcc which isn't included by default in newer Android NDKs. Out of curiosity, does this fix the issue? https://github.com/tikv/jemallocator/commit/805804ac837821408167afd2dd6d93c96c13a82d.

mukilan commented 2 weeks ago

@mukilan It looks like we disabled jemalloc for Android in #31086, but I cannot recall why not. Was it a build issue?

It's explained in this PR 🤦. The issue is that jemalloc-sys has an implicit binary dependency on libgcc which isn't included by default in newer Android NDKs. Out of curiosity, does this fix the issue? tikv/jemallocator@805804a.

I don't think this would fix it as the issue is basically with this line that forces us to link withlibgcc.a.

mukilan commented 2 weeks ago

I think android used to have jemalloc as system allocator, but that changed in recent years. I wonder what would happen if we use ASAN on android?

Good question! Looks like ASan is not recommended any more and Hardware Address Santizier (HWSAan] is recommended. It also looks like HWASan uses the tagged pointers support in aarch64.