From 385f6f93bf3ff62af3a872aac2ab2fb43893975d Mon Sep 17 00:00:00 2001 From: Mukilan Thiyagarajan Date: Mon, 13 May 2024 16:37:19 +0530 Subject: [PATCH] android: use `jemalloc` on Android (#32273) This is a fix for the crash issue in 64-bit ARM [#32175][1]. 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. [1]: https://github.com/servo/servo/issues/32175 Signed-off-by: Mukilan Thiyagarajan --- components/allocator/Cargo.toml | 4 ++-- components/allocator/lib.rs | 13 ++----------- components/profile/Cargo.toml | 4 ++-- components/profile/mem.rs | 14 +++++++------- ports/jniapi/build.rs | 11 +++++++++++ 5 files changed, 24 insertions(+), 22 deletions(-) diff --git a/components/allocator/Cargo.toml b/components/allocator/Cargo.toml index 3978241094d..75c2bc15d29 100644 --- a/components/allocator/Cargo.toml +++ b/components/allocator/Cargo.toml @@ -12,7 +12,7 @@ path = "lib.rs" [features] use-system-allocator = ["libc"] -[target.'cfg(not(any(windows, target_os = "android", target_env = "ohos")))'.dependencies] +[target.'cfg(not(any(windows, target_env = "ohos")))'.dependencies] jemallocator = { workspace = true } jemalloc-sys = { workspace = true } libc = { workspace = true, optional = true } @@ -20,5 +20,5 @@ libc = { workspace = true, optional = true } [target.'cfg(windows)'.dependencies] winapi = { workspace = true, features = ["heapapi"] } -[target.'cfg(any(target_os = "android", target_env = "ohos"))'.dependencies] +[target.'cfg(target_env = "ohos")'.dependencies] libc = { workspace = true } diff --git a/components/allocator/lib.rs b/components/allocator/lib.rs index a28916375df..c572f9e552e 100644 --- a/components/allocator/lib.rs +++ b/components/allocator/lib.rs @@ -9,12 +9,7 @@ static ALLOC: Allocator = Allocator; pub use crate::platform::*; -#[cfg(not(any( - windows, - target_os = "android", - feature = "use-system-allocator", - target_env = "ohos" -)))] +#[cfg(not(any(windows, feature = "use-system-allocator", target_env = "ohos")))] mod platform { use std::os::raw::c_void; @@ -37,11 +32,7 @@ mod platform { #[cfg(all( not(windows), - any( - target_os = "android", - feature = "use-system-allocator", - target_env = "ohos" - ) + any(feature = "use-system-allocator", target_env = "ohos") ))] mod platform { pub use std::alloc::System as Allocator; diff --git a/components/profile/Cargo.toml b/components/profile/Cargo.toml index 36aca49b6df..c12bb701000 100644 --- a/components/profile/Cargo.toml +++ b/components/profile/Cargo.toml @@ -23,7 +23,7 @@ task_info = { path = "../../support/rust-task_info" } [target.'cfg(target_os = "linux")'.dependencies] regex = { workspace = true } -[target.'cfg(not(any(target_os = "windows", target_os = "android")))'.dependencies] +[target.'cfg(not(target_os = "windows"))'.dependencies] libc = { workspace = true } -[target.'cfg(not(any(target_os = "windows", target_os = "android", target_env = "ohos")))'.dependencies] +[target.'cfg(not(any(target_os = "windows", target_env = "ohos")))'.dependencies] jemalloc-sys = { workspace = true } diff --git a/components/profile/mem.rs b/components/profile/mem.rs index 56bcdeb5a0d..565cae95c53 100644 --- a/components/profile/mem.rs +++ b/components/profile/mem.rs @@ -387,16 +387,16 @@ impl ReportsForest { //--------------------------------------------------------------------------- mod system_reporter { - #[cfg(not(any(target_os = "windows", target_os = "android", target_env = "ohos")))] + #[cfg(not(any(target_os = "windows", target_env = "ohos")))] use std::ffi::CString; - #[cfg(not(any(target_os = "windows", target_os = "android", target_env = "ohos")))] + #[cfg(not(any(target_os = "windows", target_env = "ohos")))] use std::mem::size_of; - #[cfg(not(any(target_os = "windows", target_os = "android", target_env = "ohos")))] + #[cfg(not(any(target_os = "windows", target_env = "ohos")))] use std::ptr::null_mut; #[cfg(target_os = "linux")] use libc::c_int; - #[cfg(not(any(target_os = "windows", target_os = "android", target_env = "ohos")))] + #[cfg(not(any(target_os = "windows", target_env = "ohos")))] use libc::{c_void, size_t}; use profile_traits::mem::{Report, ReportKind, ReporterRequest}; use profile_traits::path; @@ -499,10 +499,10 @@ mod system_reporter { None } - #[cfg(not(any(target_os = "windows", target_os = "android", target_env = "ohos")))] + #[cfg(not(any(target_os = "windows", target_env = "ohos")))] use jemalloc_sys::mallctl; - #[cfg(not(any(target_os = "windows", target_os = "android", target_env = "ohos")))] + #[cfg(not(any(target_os = "windows", target_env = "ohos")))] fn jemalloc_stat(value_name: &str) -> Option { // Before we request the measurement of interest, we first send an "epoch" // request. Without that jemalloc gives cached statistics(!) which can be @@ -549,7 +549,7 @@ mod system_reporter { Some(value as usize) } - #[cfg(any(target_os = "windows", target_os = "android", target_env = "ohos"))] + #[cfg(any(target_os = "windows", target_env = "ohos"))] fn jemalloc_stat(_value_name: &str) -> Option { None } diff --git a/ports/jniapi/build.rs b/ports/jniapi/build.rs index 896487d80d5..2e787743861 100644 --- a/ports/jniapi/build.rs +++ b/ports/jniapi/build.rs @@ -4,6 +4,7 @@ use std::env; use std::fs::File; +use std::io::Write; use std::path::PathBuf; use gl_generator::{Api, Fallbacks, Profile, Registry}; @@ -35,6 +36,16 @@ fn main() { println!("cargo:rustc-link-lib=EGL"); } + // FIXME: We need this workaround since jemalloc-sys still links + // to libgcc instead of libunwind, but Android NDK 23c and above + // don't have libgcc. We can't disable jemalloc for Android as + // in 64-bit aarch builds, the system allocator uses tagged + // pointers by default which causes the assertions in SM & mozjs + // to fail. See https://github.com/servo/servo/issues/32175. + let mut libgcc = File::create(dest.join("libgcc.a")).unwrap(); + libgcc.write_all(b"INPUT(-lunwind)").unwrap(); + println!("cargo:rustc-link-search=native={}", dest.display()); + let mut default_prefs = PathBuf::from(env!("CARGO_MANIFEST_DIR")); default_prefs.push("../../resources/prefs.json"); let prefs: Value = serde_json::from_reader(File::open(&default_prefs).unwrap()).unwrap();