github/wgpu
1
1
Fork 0
mirror of https://github.com/gfx-rs/wgpu.git synced 2026-04-22 03:02:01 -04:00
Code Issues Packages Projects Releases Wiki Activity

Merge #461

Browse Source 
461: Add Clippy to CI, fix errors and most warnings r=kvark a=yanchith

A quick rundown of changes:

- Fixed instances of clippy error [not_unsafe_ptr_arg_deref](https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref): Some extern functions in wgpu-remote are now marked unsafe, as they accessed data behind raw pointer via `Box::from_raw` and `slice::from_raw_parts` (commit 741844cc2b)

- Fixed clippy warning [or_fun_call](https://rust-lang.github.io/rust-clippy/master/index.html#or_fun_call) by changing `unwrap_or` to `unwrap_or_else`

- Added `#[allow(clippy::range_plus_one)]` in places where ranges are constructed along with TODOs to fix upstream in gfx. The rule [range_plus_one](https://rust-lang.github.io/rust-clippy/master/index.html#range_plus_one) is great if you have everything in one crate, but the gfx hal structs expect `Range` instead of the more generic `RangeBounds` trait.

- Fixed quite a few clippy warnings [missing_safety_doc](https://rust-lang.github.io/rust-clippy/master/index.html#missing_safety_doc) in the trivial cases (`Box::from_raw` and `slice::from_raw_parts`).

There's still a few `missing_safety_doc` warnings left. They all have in common the usage of the unsafe functions `RawPass::encode` and `RawPass::encode_slice`. I think these could potentially be made safe - it seems like `RawPass` manages its invariants internally, but I might be missing something.

I didn't add CI code that posts the warnings to github PR comments, but if anyone is willing to pick that up, this could help: https://github.com/dpobel/damien.pobel.fr/pull/62/files

Fixes: #422

Co-authored-by: yanchith <yanchi.toth@gmail.com>
This commit is contained in:
bors[bot]
2020-01-18 16:08:33 +00:00
committed by GitHub
parent 3a41272cba cd114e780a
commit 7387bd1a4d
11 changed files with 142 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
.travis.yml
View File

@@ -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 &&

6
wgpu-core/src/command/compute.rs
View File

@@ -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,

7
wgpu-core/src/command/mod.rs
View File

@@ -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,

12
wgpu-core/src/command/render.rs
View File

@@ -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,

28
wgpu-core/src/command/transfer.rs
View File

@@ -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,
}
}
}
}

4
wgpu-core/src/device/life.rs
View File

@@ -195,7 +195,7 @@ impl<B: GfxBackend> LifetimeTracker<B> {
}
/// 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<B: GfxBackend> LifetimeTracker<B> {
.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 {

10
wgpu-core/src/track/texture.rs
View File

@@ -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,

10
wgpu-native/src/command.rs
View File

@@ -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();

11
wgpu-native/src/device.rs
View File

@@ -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,

25
wgpu-remote/src/lib.rs
View File

@@ -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)
}

51
wgpu-remote/src/server.rs
View File

@@ -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,