Skip to content

Commit

Permalink
android: use jemalloc on Android (#32273)
Browse files Browse the repository at this point in the history
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]: #32175

Signed-off-by: Mukilan Thiyagarajan <mukilan@igalia.com>
  • Loading branch information
mukilan committed May 13, 2024
1 parent 1d66ea2 commit 385f6f9
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 22 deletions.
4 changes: 2 additions & 2 deletions components/allocator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ 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 }

[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 }
13 changes: 2 additions & 11 deletions components/allocator/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions components/profile/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
14 changes: 7 additions & 7 deletions components/profile/mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<usize> {
// Before we request the measurement of interest, we first send an "epoch"
// request. Without that jemalloc gives cached statistics(!) which can be
Expand Down Expand Up @@ -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<usize> {
None
}
Expand Down
11 changes: 11 additions & 0 deletions ports/jniapi/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit 385f6f9

Please sign in to comment.