Skip to content

Commit

Permalink
Auto merge of #19061 - servo:ft-alloc, r=mbrubeck
Browse files Browse the repository at this point in the history
FreeType: don’t use usable_size() as deallocation size

Instead use C-level malloc()/free() so that the size doesn’t need to be known during deallocation, since FreeType doesn’t provide it.

Hopefully fixes #19058

Depends on https://github.com/alexcrichton/jemallocator/pull/21

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19061)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Oct 30, 2017
2 parents 2627906 + 6319ad0 commit f180991
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 50 deletions.
15 changes: 8 additions & 7 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion components/allocator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@ path = "lib.rs"
[features]
unstable = ["kernel32-sys", "jemallocator"]

[dependencies]
libc = "0.2" # Only used when 'unstable' is disabled, but looks like Cargo cannot express that.

[target.'cfg(not(windows))'.dependencies]
jemallocator = { version = "0.1.3", optional = true }
jemallocator = { version = "0.1.4", optional = true }

[target.'cfg(windows)'.dependencies]
kernel32-sys = { version = "0.2.1", optional = true }
17 changes: 13 additions & 4 deletions components/allocator/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@

#[cfg(feature = "unstable")]
#[global_allocator]
static ALLOC: platform::Allocator = platform::Allocator;
static ALLOC: Allocator = Allocator;

pub use platform::usable_size;
pub use platform::*;


#[cfg(all(feature = "unstable", not(windows)))]
Expand All @@ -25,6 +25,11 @@ mod platform {
pub unsafe extern "C" fn usable_size(ptr: *const c_void) -> usize {
jemallocator::usable_size(ptr)
}

/// Memory allocation APIs compatible with libc
pub mod libc_compat {
pub use super::jemallocator::ffi::{malloc, realloc, free};
}
}

#[cfg(all(feature = "unstable", windows))]
Expand Down Expand Up @@ -57,6 +62,10 @@ mod platform {
pub unsafe extern "C" fn usable_size(_ptr: *const c_void) -> usize {
0
}
}


/// Memory allocation APIs compatible with libc
pub mod libc_compat {
extern crate libc;
pub use self::libc::{malloc, realloc, free};
}
}
50 changes: 12 additions & 38 deletions components/gfx/platform/freetype/font_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ use freetype::freetype::FT_Memory;
use freetype::freetype::FT_MemoryRec_;
use freetype::freetype::FT_New_Library;
use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
use servo_allocator::libc_compat::{malloc, realloc, free};
use servo_allocator::usable_size;
use std::mem;
use std::os::raw::{c_long, c_void};
use std::ptr;
use std::rc::Rc;
Expand All @@ -22,64 +22,38 @@ pub struct User {
size: usize,
}

// FreeType doesn't require any particular alignment for allocations.
const FT_ALIGNMENT: usize = 1;

extern fn ft_alloc(mem: FT_Memory, req_size: c_long) -> *mut c_void {
assert!(FT_ALIGNMENT == 1);
let mut vec = Vec::<u8>::with_capacity(req_size as usize);
let ptr = vec.as_mut_ptr() as *mut c_void;
mem::forget(vec);

unsafe {
let actual_size = usable_size(ptr as *const _);
let ptr = malloc(req_size as usize);
let ptr = ptr as *mut c_void; // libc::c_void vs std::os::raw::c_void
let actual_size = usable_size(ptr);
let user = (*mem).user as *mut User;
(*user).size += actual_size;
ptr
}

ptr
}

extern fn ft_free(mem: FT_Memory, ptr: *mut c_void) {
unsafe {
let actual_size = usable_size(ptr as *const _);
let actual_size = usable_size(ptr);
let user = (*mem).user as *mut User;
(*user).size -= actual_size;

assert!(FT_ALIGNMENT == 1);
mem::drop(Vec::<u8>::from_raw_parts(ptr as *mut u8, actual_size, 0))
free(ptr as *mut _);
}
}

extern fn ft_realloc(mem: FT_Memory, _old_size: c_long, new_req_size: c_long,
old_ptr: *mut c_void) -> *mut c_void {
let old_actual_size;
let mut vec;
unsafe {
old_actual_size = usable_size(old_ptr as *const _);
let old_size = old_actual_size as usize;
vec = Vec::<u8>::from_raw_parts(old_ptr as *mut u8, old_size, old_size);
};

let new_req_size = new_req_size as usize;
if new_req_size > old_actual_size {
vec.reserve_exact(new_req_size - old_actual_size)
} else if new_req_size < old_actual_size {
vec.truncate(new_req_size);
vec.shrink_to_fit()
}

let new_ptr = vec.as_mut_ptr() as *mut c_void;
mem::forget(vec);

unsafe {
let new_actual_size = usable_size(new_ptr as *const _);
let old_actual_size = usable_size(old_ptr);
let new_ptr = realloc(old_ptr as *mut _, new_req_size as usize);
let new_ptr = new_ptr as *mut c_void;
let new_actual_size = usable_size(new_ptr);
let user = (*mem).user as *mut User;
(*user).size += new_actual_size;
(*user).size -= old_actual_size;
new_ptr
}

new_ptr
}

// A |*mut User| field in a struct triggers a "use of `#[derive]` with a raw pointer" warning from
Expand Down

0 comments on commit f180991

Please sign in to comment.