Skip to content

Commit f2837a1

Browse files
committed
Fix SIGBUS crash on macOS arm64 for any \setmainfont{} call
Tectonic 0.16.x crashes with SIGBUS (exit 138) on macOS arm64 on any `\setmainfont{}` call. This PR fixes the root cause and several additional bugs introduced in the `xetex_layout` Rust port (#1138). ROOT CAUSE `CFArray`'s `Index` implementation (`crates/mac_core/src/array.rs`) was unsound. `CFArrayGetValueAtIndex` returns the stored `CFTypeRef` value directly (i.e., the pointer *is* the value), but the code treated it as a *pointer to* `T` and dereferenced it: ```rust let ptr = CFArrayGetValueAtIndex(...).cast::<T>(); unsafe { &*ptr } // reads CF object internals as a NonNull — garbage ``` When `find_font_with_name` called `matches[0].clone()`, this garbage pointer was passed to `CFRetain`, crashing immediately with `EXC_BAD_ACCESS` in `CFRetain`. FIX Replace the broken `Index<usize>` impl with a `get(index) -> T` method that correctly calls `CFRetain` via `new_borrowed` on the returned `CFTypeRef`. ADDITIONAL FIXES While investigating this issue, I stumbled upon other bugs which I initially thought were causing the problem here with `\setmainfont{}` but weren't. I fixed them anyway: - `c_api.rs`: `Fixed` type on macOS defined as `u32` instead of `i32`, mismatching the C `SInt32` typedef. Negative `scaled_size` values (used by TeX for font probing) became ~65K pt sizes. Changed to `i32`. - `font.rs:277`: AFM file reading used the already-closed main font `handle` instead of `afm_handle`. Changed to `afm_handle`. - `font.rs` (macOS init): Several `.unwrap()` calls could panic across `extern "C"` boundaries (undefined behavior). Replaced with `ok_or(())?` / `.map_err()` error propagation. - `mac_core/font.rs`: `CTFont::new_descriptor` unconditionally unwrapped a potentially-null CoreText return (the original C++ code checked for null). Changed return type to `Option<CTFont>`.
1 parent e080610 commit f2837a1

5 files changed

Lines changed: 39 additions & 30 deletions

File tree

crates/mac_core/src/array.rs

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use super::{sys, CoreType};
22
use std::marker::PhantomData;
3-
use std::ops::Index;
43
use std::ptr;
54
use std::ptr::NonNull;
65

@@ -53,20 +52,22 @@ impl<T: CoreType> CFArray<T> {
5352
pub fn is_empty(&self) -> bool {
5453
self.len() == 0
5554
}
56-
}
57-
58-
impl<T: CoreType> Index<usize> for CFArray<T> {
59-
type Output = T;
6055

61-
fn index(&self, index: usize) -> &Self::Output {
56+
/// Get a borrowed value at the given index. The returned value has its reference count
57+
/// incremented and will be released when dropped.
58+
///
59+
/// `CFArrayGetValueAtIndex` returns the stored `CFTypeRef` value directly (not a pointer
60+
/// to it), so we must construct a new borrowed `T` from it rather than casting to `&T`.
61+
pub fn get(&self, index: usize) -> T {
6262
if index >= self.len() {
6363
panic!("Index {index} out of bounds for CFArray");
6464
}
6565
// SAFETY: Internal pointer is guaranteed valid. Index has been verified in-bounds.
66-
let ptr =
67-
unsafe { sys::CFArrayGetValueAtIndex(self.0.cast().as_ptr(), index as sys::CFIndex) }
68-
.cast::<T>();
69-
// SAFETY: API contracts ensure all values are of the correct type and live for our lifetime.
70-
unsafe { &*ptr }
66+
let value =
67+
unsafe { sys::CFArrayGetValueAtIndex(self.0.cast().as_ptr(), index as sys::CFIndex) };
68+
// SAFETY: The returned value is a valid CFTypeRef for type T.
69+
// new_borrowed calls CFRetain, giving us ownership of a new reference.
70+
let ptr = NonNull::new(value.cast_mut()).unwrap();
71+
unsafe { T::new_borrowed(ptr.cast()) }
7172
}
7273
}

crates/mac_core/src/font.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,8 @@ cfty! {
8585

8686
impl CTFont {
8787
/// Create a new font from a [`CTFontDescriptor`] and a size to render at.
88-
pub fn new_descriptor(descriptor: &CTFontDescriptor, size: f64) -> CTFont {
88+
/// Returns `None` if CoreText cannot create the font (rare, but possible).
89+
pub fn new_descriptor(descriptor: &CTFontDescriptor, size: f64) -> Option<CTFont> {
8990
// SAFETY: Provided descriptor is guaranteed valid. Any size will work. Null matrix is always
9091
// allowed.
9192
let ptr = unsafe {
@@ -95,9 +96,8 @@ impl CTFont {
9596
ptr::null_mut(),
9697
)
9798
};
98-
let ptr = NonNull::new(ptr.cast_mut()).unwrap();
9999
// SAFETY: If non-null, pointer from CTFontCreateWithFontDescriptor is a new, owned CTFont.
100-
unsafe { CTFont::new_owned(ptr) }
100+
NonNull::new(ptr.cast_mut()).map(|ptr| unsafe { CTFont::new_owned(ptr) })
101101
}
102102

103103
/// Get an attribute of this font, if present

crates/xetex_layout/src/c_api.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ pub struct GlyphBBox {
3030
pub type Fixed = i32;
3131
/// cbindgen:ignore
3232
#[cfg(target_os = "macos")]
33-
pub type Fixed = u32;
33+
pub type Fixed = i32;
3434
pub type OTTag = u32;
3535
pub type GlyphID = u16;
3636
/// cbindgen:ignore

crates/xetex_layout/src/font.rs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ impl Font {
274274
let sz = engine.input_get_size(afm_handle);
275275
let mut backing_data2 = vec![0; sz];
276276
engine
277-
.input_read(handle, &mut backing_data2)
277+
.input_read(afm_handle, &mut backing_data2)
278278
.expect("failed to read AFM file");
279279

280280
self.ft_face().attach_stream_mem(backing_data2).unwrap();
@@ -358,13 +358,15 @@ impl Font {
358358
CFDictionary::new([(FontAttribute::CascadeList.to_str(), empty_cascade_list)]);
359359

360360
*descriptor = descriptor.copy_with_attrs(&attributes);
361-
*font_ref = Some(CTFont::new_descriptor(
362-
descriptor,
363-
self.point_size as f64 * 72.0 / 72.27,
364-
));
361+
*font_ref = Some(
362+
CTFont::new_descriptor(descriptor, self.point_size as f64 * 72.0 / 72.27)
363+
.ok_or(())?,
364+
);
365365
let mut index = 0;
366-
let pathname = get_file_name_from_ct_font(font_ref.as_ref().unwrap(), &mut index).unwrap();
367-
self.initialize_ft(pathname.to_str().unwrap(), index as usize)
366+
let pathname =
367+
get_file_name_from_ct_font(font_ref.as_ref().unwrap(), &mut index).ok_or(())?;
368+
let pathname_str = pathname.to_str().map_err(|_| ())?;
369+
self.initialize_ft(pathname_str, index as usize)
368370
}
369371

370372
pub(crate) fn ft_face(&self) -> std::sync::MutexGuard<'_, ft::Face> {

crates/xetex_layout/src/manager/mac.rs

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@ fn find_fonts_with_name(name: CFString, key: FontAttribute) -> CFArray<CTFontDes
2121
fn find_font_with_name(name: CFString, key: FontAttribute) -> Option<CTFontDescriptor> {
2222
let matches = find_fonts_with_name(name, key);
2323

24-
let mut matched = None;
2524
if !matches.is_empty() {
26-
matched = Some(matches[0].clone());
25+
Some(matches.get(0))
26+
} else {
27+
None
2728
}
28-
matched
2929
}
3030

3131
fn append_name_to_list(font: &CTFont, name_list: &mut Vec<CString>, name_key: FontNameKey) {
@@ -48,14 +48,16 @@ impl MacBackend {
4848

4949
fn add_fonts_to_caches(&self, maps: &mut FontMaps, members: CFArray<CTFontDescriptor>) {
5050
for i in 0..members.len() {
51-
let font = &members[i];
51+
let font = members.get(i);
5252
let names = self.read_names(font.clone());
53-
maps.add_to_maps(self, font.clone(), &names)
53+
maps.add_to_maps(self, font, &names)
5454
}
5555
}
5656

5757
fn add_font_and_siblings_to_caches(&self, maps: &mut FontMaps, font: &CTFontDescriptor) {
58-
let font = CTFont::new_descriptor(font, 10.0);
58+
let Some(font) = CTFont::new_descriptor(font, 10.0) else {
59+
return;
60+
};
5961
let attr = font.attr(FontAttribute::FamilyName).unwrap();
6062
// SAFETY: CFString has no generic parameters
6163
let family = unsafe { attr.downcast::<CFString>() }.unwrap();
@@ -79,7 +81,9 @@ impl FontManagerBackend for MacBackend {
7981
fn get_platform_font_desc<'a>(&'a self, font: &'a PlatformFontRef) -> Cow<'a, CStr> {
8082
let mut path = Cow::Borrowed(c"[unknown]");
8183

82-
let ct_font = CTFont::new_descriptor(font, 0.0);
84+
let Some(ct_font) = CTFont::new_descriptor(font, 0.0) else {
85+
return path;
86+
};
8387
let url = ct_font
8488
.attr(FontAttribute::URL)
8589
// SAFETY: CFUrl has no generic parameters
@@ -154,7 +158,9 @@ impl FontManagerBackend for MacBackend {
154158

155159
names.ps_name = Some(ps_name.get_cstring());
156160

157-
let font = CTFont::new_descriptor(&font, 0.0);
161+
let Some(font) = CTFont::new_descriptor(&font, 0.0) else {
162+
return names;
163+
};
158164
append_name_to_list(&font, &mut names.full_names, FontNameKey::Full);
159165
append_name_to_list(&font, &mut names.family_names, FontNameKey::Family);
160166
append_name_to_list(&font, &mut names.style_names, FontNameKey::Style);

0 commit comments

Comments
 (0)