Skip to content

Commit

Permalink
Mark certain CF functions' first argument as non-NULL
Browse files Browse the repository at this point in the history
The headers are sometimes insufficient in this regard, and you're likely
to be able to cause unsoundness if passing NULL to these.

Part of #695.
  • Loading branch information
madsmtm committed Jan 11, 2025
1 parent 9c07a2f commit a1d256e
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 7 deletions.
20 changes: 20 additions & 0 deletions crates/header-translator/src/rust_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2394,6 +2394,26 @@ impl Ty {
panic!("tried to fix related result type on non-id type")
}
}

pub(crate) fn fix_fn_first_argument_cf_nullability(&mut self, fn_name: &str) {
if let Self::TypeDef {
id,
nullability: nullability @ Nullability::Unspecified,
is_cf: true,
..
} = self
{
let type_name = id.name.strip_suffix("Ref").unwrap_or(&id.name);
// We don't ever want to mark these as non-NULL, as they have NULL
// statics (`kCFAllocatorDefault` and `kODSessionDefault`).
if fn_name.contains(type_name) && !matches!(type_name, "ODSession" | "CFAllocator") {
// Is likely a getter, so let's mark it as non-null (CF will
// usually crash if given an unexpected NULL pointer, but
// we're not entirely sure it will always do so).
*nullability = Nullability::NonNull;
}
}
}
}

/// Strip macros from unexposed types.
Expand Down
4 changes: 4 additions & 0 deletions crates/header-translator/src/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1595,6 +1595,10 @@ impl Stmt {
_ => error!("unknown"),
});

if let Some((_, first_ty)) = arguments.first_mut() {
first_ty.fix_fn_first_argument_cf_nullability(&id.name);
}

// Don't map `CFRetain`, `CFRelease`, `CFAutorelease`, as well
// as custom ones like as `CGColorRelease`, but not things
// like `CMBufferQueueDequeueAndRetain`.
Expand Down
12 changes: 6 additions & 6 deletions framework-crates/objc2-core-foundation/src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ impl CFString {
// encoded strings, see the `as_str_broken` test below.
#[allow(dead_code)]
unsafe fn as_str(&self) -> Option<&str> {
let bytes = unsafe { CFStringGetCStringPtr(Some(self), CFStringEncoding::UTF8) };
let bytes = unsafe { CFStringGetCStringPtr(self, CFStringEncoding::UTF8) };
NonNull::new(bytes as *mut c_char).map(|bytes| {
// SAFETY: The pointer is valid for as long as the CFString is not
// mutated (which the caller ensures it isn't for the lifetime of
Expand Down Expand Up @@ -128,11 +128,11 @@ impl fmt::Display for CFString {
let mut location_utf16 = 0;

loop {
let len_utf16 = unsafe { CFStringGetLength(Some(self)) };
let len_utf16 = unsafe { CFStringGetLength(self) };
let mut read_utf8 = 0;
let read_utf16 = unsafe {
CFStringGetBytes(
Some(self),
self,
CFRange {
location: location_utf16,
length: len_utf16 - location_utf16,
Expand Down Expand Up @@ -218,7 +218,7 @@ mod tests {
.unwrap()
};
assert_eq!(s.to_string(), "�"); // Replacement character
assert_eq!(unsafe { CFStringGetLength(Some(&s)) }, 1);
assert_eq!(unsafe { CFStringGetLength(&s) }, 1);
}

#[test]
Expand All @@ -232,7 +232,7 @@ mod tests {
assert_ne!(
unsafe {
CFStringGetCString(
Some(&s),
&s,
buf.as_mut_ptr().cast(),
buf.len() as _,
CFStringEncoding::UTF8,
Expand Down Expand Up @@ -281,7 +281,7 @@ mod tests {
assert_ne!(
unsafe {
CFStringGetCString(
Some(&s),
&s,
buf.as_mut_ptr().cast(),
buf.len() as _,
CFStringEncoding::UTF8,
Expand Down
2 changes: 1 addition & 1 deletion generated
Submodule generated updated 41 files
+9 −13 CoreFoundation/CFArray.rs
+12 −12 CoreFoundation/CFAttributedString.rs
+7 −11 CoreFoundation/CFBag.rs
+10 −13 CoreFoundation/CFBinaryHeap.rs
+8 −23 CoreFoundation/CFBitVector.rs
+59 −62 CoreFoundation/CFBundle.rs
+17 −23 CoreFoundation/CFCalendar.rs
+4 −10 CoreFoundation/CFCharacterSet.rs
+4 −4 CoreFoundation/CFData.rs
+3 −3 CoreFoundation/CFDate.rs
+11 −16 CoreFoundation/CFDateFormatter.rs
+9 −21 CoreFoundation/CFDictionary.rs
+11 −11 CoreFoundation/CFError.rs
+6 −15 CoreFoundation/CFFileDescriptor.rs
+11 −22 CoreFoundation/CFFileSecurity.rs
+6 −6 CoreFoundation/CFLocale.rs
+6 −8 CoreFoundation/CFMachPort.rs
+10 −11 CoreFoundation/CFMessagePort.rs
+5 −5 CoreFoundation/CFNotificationCenter.rs
+6 −6 CoreFoundation/CFNumber.rs
+10 −12 CoreFoundation/CFNumberFormatter.rs
+8 −8 CoreFoundation/CFPlugIn.rs
+3 −6 CoreFoundation/CFPropertyList.rs
+40 −43 CoreFoundation/CFRunLoop.rs
+7 −11 CoreFoundation/CFSet.rs
+15 −17 CoreFoundation/CFSocket.rs
+29 −29 CoreFoundation/CFStream.rs
+30 −36 CoreFoundation/CFString.rs
+9 −9 CoreFoundation/CFStringTokenizer.rs
+13 −21 CoreFoundation/CFTimeZone.rs
+21 −29 CoreFoundation/CFTree.rs
+52 −69 CoreFoundation/CFURL.rs
+2 −2 CoreFoundation/CFURLAccess.rs
+4 −4 CoreFoundation/CFURLEnumerator.rs
+1 −1 CoreFoundation/CFUUID.rs
+7 −7 CoreFoundation/CFUserNotification.rs
+7 −7 CoreFoundation/CFXMLNode.rs
+12 −15 CoreFoundation/CFXMLParser.rs
+36 −44 OpenDirectory/CFOpenDirectory/CFODNode.rs
+6 −6 OpenDirectory/CFOpenDirectory/CFODQuery.rs
+42 −48 OpenDirectory/CFOpenDirectory/CFODRecord.rs

0 comments on commit a1d256e

Please sign in to comment.