diff --git a/.travis.yml b/.travis.yml index 8b80d51bf5..1aa12fbe5d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -57,12 +57,17 @@ before_install: - if [[ $TRAVIS_RUST_VERSION != "nightly" ]] && [[ $TRAVIS_OS_NAME == "windows" ]]; then rustup default stable-msvc; fi - if [[ $TRAVIS_RUST_VERSION == "nightly" ]] && [[ $TRAVIS_OS_NAME == "windows" ]]; then rustup default nightly-msvc; fi +before_script: + - rustup component add clippy + script: - cargo test # TODO: enable GL backend - - (cd wgpu-core && cargo check --all-features) - - if [[ $TRAVIS_OS_NAME == "osx" ]]; then (cd wgpu-native && cargo check --features vulkan-portability); fi - - if [[ $TRAVIS_OS_NAME == "linux" ]]; then cargo check --release; fi + - (cd wgpu-core && cargo clippy --all-features) + - (cd wgpu-native && cargo clippy) + - (cd wgpu-remote && cargo clippy --all-features) + - if [[ $TRAVIS_OS_NAME == "osx" ]]; then (cd wgpu-native && cargo clippy --features vulkan-portability); fi + - if [[ $TRAVIS_OS_NAME == "linux" ]]; then cargo clippy --release; fi - if [[ $TRAVIS_RUST_VERSION == "nightly" ]]; then cargo +nightly install cbindgen; fi - if [[ $TRAVIS_RUST_VERSION == "nightly" ]] && [[ $TRAVIS_OS_NAME == "windows" ]]; then wget -nc -O glfw.zip https://github.com/glfw/glfw/archive/3.3.zip && diff --git a/wgpu-core/src/command/compute.rs b/wgpu-core/src/command/compute.rs index 631488dfec..80bec01e35 100644 --- a/wgpu-core/src/command/compute.rs +++ b/wgpu-core/src/command/compute.rs @@ -225,6 +225,12 @@ pub mod compute_ffi { }; use std::{convert::TryInto, slice}; + /// # Safety + /// + /// This function is unsafe as there is no guarantee that the given pointer is + /// valid for `offset_length` elements. + // TODO: There might be other safety issues, such as using the unsafe + // `RawPass::encode` and `RawPass::encode_slice`. #[no_mangle] pub unsafe extern "C" fn wgpu_compute_pass_set_bind_group( pass: &mut RawPass, diff --git a/wgpu-core/src/command/mod.rs b/wgpu-core/src/command/mod.rs index d623e646df..4f373c22b8 100644 --- a/wgpu-core/src/command/mod.rs +++ b/wgpu-core/src/command/mod.rs @@ -197,7 +197,7 @@ pub struct CommandBufferDescriptor { } #[no_mangle] -pub unsafe extern "C" fn wgpu_command_encoder_begin_compute_pass( +pub extern "C" fn wgpu_command_encoder_begin_compute_pass( encoder_id: id::CommandEncoderId, _desc: Option<&ComputePassDescriptor>, ) -> *mut RawPass { @@ -220,6 +220,11 @@ pub struct RawRenderPass { targets: RawRenderTargets, } +/// # Safety +/// +/// This function is unsafe as there is no guarantee that the given pointer +/// (`RenderPassDescriptor::color_attachments`) is valid for +/// `RenderPassDescriptor::color_attachments_length` elements. #[no_mangle] pub unsafe extern "C" fn wgpu_command_encoder_begin_render_pass( encoder_id: id::CommandEncoderId, diff --git a/wgpu-core/src/command/render.rs b/wgpu-core/src/command/render.rs index 959cf7f04e..29a3609a09 100644 --- a/wgpu-core/src/command/render.rs +++ b/wgpu-core/src/command/render.rs @@ -1127,6 +1127,12 @@ pub mod render_ffi { }; use std::{convert::TryInto, slice}; + /// # Safety + /// + /// This function is unsafe as there is no guarantee that the given pointer is + /// valid for `offset_length` elements. + // TODO: There might be other safety issues, such as using the unsafe + // `RawPass::encode` and `RawPass::encode_slice`. #[no_mangle] pub unsafe extern "C" fn wgpu_render_pass_set_bind_group( pass: &mut RawRenderPass, @@ -1166,6 +1172,12 @@ pub mod render_ffi { }); } + /// # Safety + /// + /// This function is unsafe as there is no guarantee that the given pointers + /// (`buffer_ids` and `offsets`) are valid for `length` elements. + // TODO: There might be other safety issues, such as using the unsafe + // `RawPass::encode` and `RawPass::encode_slice`. #[no_mangle] pub unsafe extern "C" fn wgpu_render_pass_set_vertex_buffers( pass: &mut RawRenderPass, diff --git a/wgpu-core/src/command/transfer.rs b/wgpu-core/src/command/transfer.rs index 0b5a4b4be6..7fe2e8dd7c 100644 --- a/wgpu-core/src/command/transfer.rs +++ b/wgpu-core/src/command/transfer.rs @@ -43,19 +43,31 @@ impl TextureCopyView { fn to_selector(&self, aspects: hal::format::Aspects) -> hal::image::SubresourceRange { let level = self.mip_level as hal::image::Level; let layer = self.array_layer as hal::image::Layer; - hal::image::SubresourceRange { - aspects, - levels: level .. level + 1, - layers: layer .. layer + 1, + + // TODO: Can't satisfy clippy here unless we modify + // `hal::image::SubresourceRange` in gfx to use `std::ops::RangeBounds`. + #[allow(clippy::range_plus_one)] + { + hal::image::SubresourceRange { + aspects, + levels: level .. level + 1, + layers: layer .. layer + 1, + } } } fn to_sub_layers(&self, aspects: hal::format::Aspects) -> hal::image::SubresourceLayers { let layer = self.array_layer as hal::image::Layer; - hal::image::SubresourceLayers { - aspects, - level: self.mip_level as hal::image::Level, - layers: layer .. layer + 1, + // TODO: Can't satisfy clippy here unless we modify + // `hal::image::SubresourceLayers` in gfx to use + // `std::ops::RangeBounds`. + #[allow(clippy::range_plus_one)] + { + hal::image::SubresourceLayers { + aspects, + level: self.mip_level as hal::image::Level, + layers: layer .. layer + 1, + } } } } diff --git a/wgpu-core/src/device/life.rs b/wgpu-core/src/device/life.rs index 4468803857..b7c4934b94 100644 --- a/wgpu-core/src/device/life.rs +++ b/wgpu-core/src/device/life.rs @@ -195,7 +195,7 @@ impl LifetimeTracker { } /// Find the pending entry with the lowest active index. If none can be found that means - /// everything in the allocator can be cleaned up, so std::usize::MAX is correct. + /// everything in the allocator can be cleaned up, so std::usize::MAX is correct. pub fn lowest_active_submission(&self) -> SubmissionIndex { self.active .iter() @@ -225,7 +225,7 @@ impl LifetimeTracker { .active .iter() .position(|a| unsafe { !device.get_fence_status(&a.fence).unwrap() }) - .unwrap_or(self.active.len()); + .unwrap_or_else(|| self.active.len()); let last_done = if done_count != 0 { self.active[done_count - 1].index } else { diff --git a/wgpu-core/src/track/texture.rs b/wgpu-core/src/track/texture.rs index d93997c733..7452f4de71 100644 --- a/wgpu-core/src/track/texture.rs +++ b/wgpu-core/src/track/texture.rs @@ -106,6 +106,10 @@ impl ResourceState for TextureState { if unit.last == usage && TextureUsage::ORDERED.contains(usage) { continue; } + // TODO: Can't satisfy clippy here unless we modify + // `hal::image::SubresourceRange` in gfx to use + // `std::ops::RangeBounds`. + #[allow(clippy::range_plus_one)] let pending = PendingTransition { id, selector: hal::image::SubresourceRange { @@ -178,11 +182,15 @@ impl ResourceState for TextureState { last: end.last, } } else { + // TODO: Can't satisfy clippy here unless we modify + // `hal::image::SubresourceRange` in gfx to use + // `std::ops::RangeBounds`. + #[allow(clippy::range_plus_one)] let pending = PendingTransition { id, selector: hal::image::SubresourceRange { aspects: hal::format::Aspects::empty(), - levels: level .. level+1, + levels: level .. level + 1, layers: layers.clone(), }, usage: start.last .. to_usage, diff --git a/wgpu-native/src/command.rs b/wgpu-native/src/command.rs index ddbb67a6d6..bccdffc5d1 100644 --- a/wgpu-native/src/command.rs +++ b/wgpu-native/src/command.rs @@ -83,6 +83,11 @@ pub extern "C" fn wgpu_command_encoder_copy_texture_to_texture( } +/// # Safety +/// +/// This function is unsafe because improper use may lead to memory +/// problems. For example, a double-free may occur if the function is called +/// twice on the same raw pointer. #[no_mangle] pub unsafe extern "C" fn wgpu_render_pass_end_pass(pass_id: id::RenderPassId) { let (pass_data, encoder_id, targets) = Box::from_raw(pass_id).finish_render(); @@ -114,6 +119,11 @@ pub unsafe extern "C" fn wgpu_render_pass_end_pass(pass_id: id::RenderPassId) { gfx_select!(encoder_id => GLOBAL.command_encoder_run_render_pass(encoder_id, &color_attachments, depth_stencil_attachment, &pass_data)) } +/// # Safety +/// +/// This function is unsafe because improper use may lead to memory +/// problems. For example, a double-free may occur if the function is called +/// twice on the same raw pointer. #[no_mangle] pub unsafe extern "C" fn wgpu_compute_pass_end_pass(pass_id: id::ComputePassId) { let (pass_data, encoder_id) = Box::from_raw(pass_id).finish_compute(); diff --git a/wgpu-native/src/device.rs b/wgpu-native/src/device.rs index f178b014ab..5107869cac 100644 --- a/wgpu-native/src/device.rs +++ b/wgpu-native/src/device.rs @@ -135,6 +135,9 @@ pub extern "C" fn wgpu_create_surface_from_windows_hwnd( )) } +/// # Safety +/// +/// This function is unsafe as it calls an unsafe extern callback. #[no_mangle] pub unsafe extern "C" fn wgpu_request_adapter_async( desc: Option<&core::instance::RequestAdapterOptions>, @@ -181,6 +184,10 @@ pub extern "C" fn wgpu_device_create_buffer( gfx_select!(device_id => GLOBAL.device_create_buffer(device_id, desc, PhantomData)) } +/// # Safety +/// +/// This function is unsafe as there is no guarantee that the given pointer +/// dereferenced in this function is valid. #[no_mangle] pub unsafe extern "C" fn wgpu_device_create_buffer_mapped( device_id: id::DeviceId, @@ -297,6 +304,10 @@ pub extern "C" fn wgpu_device_get_queue(device_id: id::DeviceId) -> id::QueueId device_id } +/// # Safety +/// +/// This function is unsafe as there is no guarantee that the given pointer is +/// valid for `command_buffers_length` elements. #[no_mangle] pub unsafe extern "C" fn wgpu_queue_submit( queue_id: id::QueueId, diff --git a/wgpu-remote/src/lib.rs b/wgpu-remote/src/lib.rs index 310c09f99d..693c504d4e 100644 --- a/wgpu-remote/src/lib.rs +++ b/wgpu-remote/src/lib.rs @@ -70,21 +70,30 @@ pub extern "C" fn wgpu_client_new() -> Infrastructure { } } +/// # Safety +/// +/// This function is unsafe because improper use may lead to memory +/// problems. For example, a double-free may occur if the function is called +/// twice on the same raw pointer. #[no_mangle] -pub extern "C" fn wgpu_client_delete(client: *mut Client) { +pub unsafe extern "C" fn wgpu_client_delete(client: *mut Client) { log::info!("Terminating WGPU client"); - let _client = unsafe { Box::from_raw(client) }; + let _client = Box::from_raw(client); } +/// # Safety +/// +/// This function is unsafe as there is no guarantee that the given pointer is +/// valid for `id_length` elements. #[no_mangle] -pub extern "C" fn wgpu_client_make_adapter_ids( +pub unsafe extern "C" fn wgpu_client_make_adapter_ids( client: &Client, ids: *mut id::AdapterId, id_length: usize, ) -> usize { let mut identities = client.identities.lock(); assert_ne!(id_length, 0); - let mut ids = unsafe { slice::from_raw_parts_mut(ids, id_length) }.iter_mut(); + let mut ids = slice::from_raw_parts_mut(ids, id_length).iter_mut(); *ids.next().unwrap() = identities.vulkan.adapters.alloc(Backend::Vulkan); @@ -100,14 +109,18 @@ pub extern "C" fn wgpu_client_make_adapter_ids( id_length - ids.len() } +/// # Safety +/// +/// This function is unsafe as there is no guarantee that the given pointer is +/// valid for `id_length` elements. #[no_mangle] -pub extern "C" fn wgpu_client_kill_adapter_ids( +pub unsafe extern "C" fn wgpu_client_kill_adapter_ids( client: &Client, ids: *const id::AdapterId, id_length: usize, ) { let mut identity = client.identities.lock(); - let ids = unsafe { slice::from_raw_parts(ids, id_length) }; + let ids = slice::from_raw_parts(ids, id_length); for &id in ids { identity.select(id.backend()).adapters.free(id) } diff --git a/wgpu-remote/src/server.rs b/wgpu-remote/src/server.rs index abe7a6947a..ccabb93b87 100644 --- a/wgpu-remote/src/server.rs +++ b/wgpu-remote/src/server.rs @@ -14,10 +14,15 @@ pub extern "C" fn wgpu_server_new() -> *mut Global { Box::into_raw(Box::new(Global::new("wgpu"))) } +/// # Safety +/// +/// This function is unsafe because improper use may lead to memory +/// problems. For example, a double-free may occur if the function is called +/// twice on the same raw pointer. #[no_mangle] -pub extern "C" fn wgpu_server_delete(global: *mut Global) { +pub unsafe extern "C" fn wgpu_server_delete(global: *mut Global) { log::info!("Terminating WGPU server"); - unsafe { Box::from_raw(global) }.delete(); + Box::from_raw(global).delete(); log::info!("\t...done"); } @@ -25,14 +30,19 @@ pub extern "C" fn wgpu_server_delete(global: *mut Global) { /// Provide the list of IDs to pick from. /// /// Returns the index in this list, or -1 if unable to pick. +/// +/// # Safety +/// +/// This function is unsafe as there is no guarantee that the given pointer is +/// valid for `id_length` elements. #[no_mangle] -pub extern "C" fn wgpu_server_instance_request_adapter( +pub unsafe extern "C" fn wgpu_server_instance_request_adapter( global: &Global, desc: &core::instance::RequestAdapterOptions, ids: *const id::AdapterId, id_length: usize, ) -> i8 { - let ids = unsafe { slice::from_raw_parts(ids, id_length) }; + let ids = slice::from_raw_parts(ids, id_length); match global.pick_adapter( desc, core::instance::AdapterInputs::IdSet(ids, |i| i.backend()), @@ -67,8 +77,12 @@ pub extern "C" fn wgpu_server_device_create_buffer( gfx_select!(self_id => global.device_create_buffer(self_id, desc, new_id)); } +/// # Safety +/// +/// This function is unsafe as there is no guarantee that the given pointer is +/// valid for `size` elements. #[no_mangle] -pub extern "C" fn wgpu_server_device_set_buffer_sub_data( +pub unsafe extern "C" fn wgpu_server_device_set_buffer_sub_data( global: &Global, self_id: id::DeviceId, buffer_id: id::BufferId, @@ -76,14 +90,16 @@ pub extern "C" fn wgpu_server_device_set_buffer_sub_data( data: *const u8, size: core::BufferAddress, ) { - let slice = unsafe { - slice::from_raw_parts(data, size as usize) - }; + let slice = slice::from_raw_parts(data, size as usize); gfx_select!(self_id => global.device_set_buffer_sub_data(self_id, buffer_id, offset, slice)); } +/// # Safety +/// +/// This function is unsafe as there is no guarantee that the given pointer is +/// valid for `size` elements. #[no_mangle] -pub extern "C" fn wgpu_server_device_get_buffer_sub_data( +pub unsafe extern "C" fn wgpu_server_device_get_buffer_sub_data( global: &Global, self_id: id::DeviceId, buffer_id: id::BufferId, @@ -91,9 +107,7 @@ pub extern "C" fn wgpu_server_device_get_buffer_sub_data( data: *mut u8, size: core::BufferAddress, ) { - let slice = unsafe { - slice::from_raw_parts_mut(data, size as usize) - }; + let slice = slice::from_raw_parts_mut(data, size as usize); gfx_select!(self_id => global.device_get_buffer_sub_data(self_id, buffer_id, offset, slice)); } @@ -122,6 +136,10 @@ pub extern "C" fn wgpu_server_encoder_destroy( gfx_select!(self_id => global.command_encoder_destroy(self_id)); } +/// # Safety +/// +/// This function is unsafe as there is no guarantee that the given pointer is +/// valid for `byte_length` elements. #[no_mangle] pub unsafe extern "C" fn wgpu_server_encode_compute_pass( global: &Global, @@ -133,6 +151,11 @@ pub unsafe extern "C" fn wgpu_server_encode_compute_pass( gfx_select!(self_id => global.command_encoder_run_compute_pass(self_id, raw_data)); } +/// # Safety +/// +/// This function is unsafe as there is no guarantee that the given pointers are +/// valid for `color_attachments_length` and `command_length` elements, +/// respectively. #[no_mangle] pub unsafe extern "C" fn wgpu_server_encode_render_pass( global: &Global, @@ -148,6 +171,10 @@ pub unsafe extern "C" fn wgpu_server_encode_render_pass( gfx_select!(self_id => global.command_encoder_run_render_pass(self_id, color_attachments, depth_stencil_attachment, raw_pass)); } +/// # Safety +/// +/// This function is unsafe as there is no guarantee that the given pointer is +/// valid for `command_buffer_id_length` elements. #[no_mangle] pub unsafe extern "C" fn wgpu_server_queue_submit( global: &Global,