Declare OpenCL functions that can lead to undefined behaviour as unsafe

[kenba]

As per @awused comments in kenba/opencl3#51 and kenba/opencl3#52, all OpenCL functions that can lead to undefined behaviour should be declared unsafe.

This issue is to capture and change all cl3 functions that should be declared unsafe.

[awused]

Honestly I'm not a huge user of OpenCL so I don't think I can audit the entire crate. I was porting existing code that made some use of unsafe for performance.

Off the top of my head:

• Anything that can free resources still in use
• Code that has extra requirements (host pointer requires length and alignment for pointers)
• Anything where you pass in a *mut or *const for a parameter where lifetimes can't be tracked
  • Anything short lived can use Option<&mut> instead
  • CL_BLOCKING operations can be safe, but CL_NONBLOCKING may be inherently unsafe since the user must ensure they wait for completion before freeing the memory.
• Any constructors or functions that require their arguments be associated with the same context/device/etc.
  • A lot of these can probably be replaced with appropriate assertions.
• Running a kernel
  • This one is annoying because it means any actual use of OpenCL requires at least one unsafe. I don't think a safe abstraction is possible because the user is supplying their own kernel code.

[kenba]

Thank you @awused. Your answer and @vmx ‘s comment from kenba/opencl3#51:

probably most of cl3 should be unsafe

have made me reconsider this issue. To my mind OpenCL is inherently unsafe.

Rust’s compile time memory safety guarantee is one of the greatest features of the language IMHO, but it is incompatible with a library like OpenCL that runs programs (kernels) at runtime on different devices with different memory models.

Your point about “running a kernel” @awused is particularly relevant because that’s the whole purpose of OpenCL and there are many ways to run a kernel incorrectly e.g., see kenba/opencl3#49 where incorrect parameter types were passed to a kernel.

I want this library and opencl3 to conform to the Rust memory safety guarantee but there are so many ways of causing erroneous or undefined behaviour using OpenCL, that I fear marking some functions unsafe will just lead users into a false sense of security.

I consider the most appropriate solution to this issue is to add warnings to the descriptions of this library and opencl3 highlighting this issue and encouraging all users to read and understand the OpenCL manual before attempting to use the libraries. Do you agree?

[awused]

As I said in the other bug, libraries tend towards either making everything unsafe and doing no validation at all or exposing a safe interface with some unsafe escape hatches. OpenCL may be inherently unsafe but that doesn't mean safe abstractions can't be built over it for the vast majority of use cases. Running a kernel involves running C code, so that is necessarily unsafe, but I don't think there's any getting around that and having that isn't likely to prevent people from using the library. But it is possible to reduce unsafe it to the point where running a kernel is the only unsafe needed most of the time.

I hesitate far more to use libraries that mark everything as safe out of a desire to be popular or "friendly" when they're unsafe rather than those that just mark everything as unsafe and firmly put the onus on the user to use it properly. Since you asked for my opinion I do think that's the wrong way to go about it.

[kenba]

@awused I have declared functions as unsafe following your guidelines above.

Please let me know of any functions that you think I have missed. @vmx that applies to you too :)

[awused]

The changes to this package, at least, look fine at first glance, though I've not gone through them in detail. While I did experiment with opencl3 the lack of Sync and Clone pushed me back to using ocl for the foreseeable future since I'd have to refactor my applications around a single worker thread, so I've not got skin in this game any longer. I submitted the bugs after reading the discussion on Send/Sync/Clone since you seemed to be taking safety seriously.

[vmx]

I also only had a quick look (and I also still lack quite a bit of OpenCL knowledge), but what changed looks good to me.

Thanks @awused for spending so much time on making this library better.

[kenba]

@vmx and @awused I've now published these and the opencl3 changes to crates.io at version 0.9.0 together with PR kenba/opencl3#53: "added sync for all threadsafe opencl objects".