rusticl: mark CheckedPtr::write_checked as unsafe

While nullity of the CheckedPtr object is checked, writing to a raw
pointer safely requires that several other invariants be satisfied, so
it should be marked as unsafe to reflect that.

v2: reorder commits for cherry-picking and remove alignment check

Reviewed-by: Karol Herbst <kherbst@redhat.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33989>
This commit is contained in:
Seán de Búrca 2025-03-12 11:28:07 -07:00 committed by Marge Bot
parent 1164d39c38
commit 474d8b6316
6 changed files with 46 additions and 12 deletions

View file

@ -348,7 +348,9 @@ fn get_device_ids(
// num_devices returns the number of OpenCL devices available that match device_type. If
// num_devices is NULL, this argument is ignored.
num_devices.write_checked(devs.len() as cl_uint);
// SAFETY: Caller is responsible for providing a null pointer or one valid
// for a write of `size_of::<cl_uint>()`.
unsafe { num_devices.write_checked(devs.len() as cl_uint) };
if !devices.is_null() {
let n = min(num_entries as usize, devs.len());
@ -429,7 +431,9 @@ fn get_host_timer(device_id: cl_device_id, host_timestamp: *mut cl_ulong) -> CLR
}
// Currently the best clock we have for the host_timestamp
host_timestamp.write_checked(device.screen().get_timestamp());
// SAFETY: Caller is responsible for providing a pointer valid for a write
// of `size_of::<cl_ulong>()`.
unsafe { host_timestamp.write_checked(device.screen().get_timestamp()) };
Ok(())
}

View file

@ -549,7 +549,12 @@ extern "C" fn clLinkProgram(
Err(e) => (ptr::null_mut(), e),
};
errcode_ret.write_checked(err);
// Correct behavior when `errcode_ret` is null is unspecified, but by
// analogy, we fail silently in that case.
// SAFETY: Caller is responsible for providing a pointer valid for a write
// of `size_of::<cl_int>()`.
unsafe { errcode_ret.write_checked(err) };
ptr
}

View file

@ -343,7 +343,11 @@ fn create_kernels_in_program(
}
num_kernels += 1;
}
num_kernels_ret.write_checked(num_kernels);
// SAFETY: Caller is responsible for providing a pointer valid for a write
// of `size_of::<cl_uint>()`.
unsafe { num_kernels_ret.write_checked(num_kernels) };
Ok(())
}

View file

@ -919,7 +919,9 @@ fn get_supported_image_formats(
// `num_image_formats` should be the full count of supported formats,
// regardless of the value of `num_entries`. It may be null, in which case
// it is ignored.
num_image_formats.write_checked(res.len() as cl_uint);
// SAFETY: Callers are responsible for providing either a null pointer or
// one for which a write of `size_of::<cl_uint>()` is valid.
unsafe { num_image_formats.write_checked(res.len() as cl_uint) };
// `image_formats` may be null, in which case it is ignored.
let num_entries_to_write = cmp::min(res.len(), num_entries as usize);
@ -3155,8 +3157,12 @@ fn get_gl_object_info(
match &m.gl_obj {
Some(gl_obj) => {
gl_object_type.write_checked(gl_obj.gl_object_type);
gl_object_name.write_checked(gl_obj.gl_object_name);
// Either `gl_object_type` or `gl_object_name` may be null, in which
// case they are ignored.
// SAFETY: Caller is responsible for providing null pointers or ones
// which are valid for a write of the appropriate size.
unsafe { gl_object_type.write_checked(gl_obj.gl_object_type) };
unsafe { gl_object_name.write_checked(gl_obj.gl_object_name) };
}
None => {
// CL_INVALID_GL_OBJECT if there is no GL object associated with memobj.

View file

@ -57,11 +57,17 @@ fn get_platform_ids(
// specific OpenCL platform. If the platforms argument is NULL, then this argument is ignored. The
// number of OpenCL platforms returned is the minimum of the value specified by num_entries or the
// number of OpenCL platforms available.
platforms.write_checked(Platform::get().as_ptr());
// SAFETY: Caller is responsible for providing a null pointer or one valid
// for a write of `num_entries * size_of::<cl_platform_id>()`. We are
// guaranteed to write at most one value, and if `platforms` is non-null,
// `num_entries` is guaranteed to be at least 1.
unsafe { platforms.write_checked(Platform::get().as_ptr()) };
// num_platforms returns the number of OpenCL platforms available. If num_platforms is NULL, then
// this argument is ignored.
num_platforms.write_checked(1);
// SAFETY: Caller is responsible for providing a null pointer or one valid
// for a write of `size_of::<cl_uint>()`.
unsafe { num_platforms.write_checked(1) };
Ok(())
}

View file

@ -63,7 +63,14 @@ pub trait CheckedPtr<T> {
/// other invariants of [`std::ptr::copy`].
unsafe fn copy_from_checked(self, src: *const T, count: usize);
fn write_checked(self, val: T);
/// Overwrites a memory location with the given value without reading or
/// dropping the old value.
///
/// # Safety
///
/// The nullity of `self` is checked. `self` must fulfill all other
/// invariants of [`std::ptr::write`].
unsafe fn write_checked(self, val: T);
}
impl<T> CheckedPtr<T> for *mut T {
@ -77,10 +84,12 @@ impl<T> CheckedPtr<T> for *mut T {
}
}
fn write_checked(self, val: T) {
unsafe fn write_checked(self, val: T) {
if !self.is_null() {
// SAFETY: Caller is responsible for satisfying all invariants save
// pointer nullity.
unsafe {
*self = val;
self.write(val);
}
}
}