Replace unsound PyByteArray::data with PyByteArray::to_vec

[konstin]

Thanks to @ExpHP's comment in #373 pointing out the existing unsoundness of PyByteArray::data I've replace the function with to_vec, which copies the bytearray to a Vec. In the long run we likely want something like ReadOnlyCell for an as_slice, but this at least fixes the UB for now.

[kngwyu]

I'm not against this PR, but to think about what kind of safety we can provide under the current design, we should keep in our mind that we can do mutate operation to all &Py~ types.
It means we cannot follow the Rust's covariant rule about &'a types.
So, to get surface safety, one possible approach we(and rust-cpython team) can do is to use cell/pointer types when exposing data underlying PyObect pointer to Rust world, which makes mutate operation unsafe.
Yeah, I think this approach is fine, but I also feel it makes our codes more complicated and not offers many benefits...

[birkenfeld]

In the end, to be taken seriously by the Rust community, and to deserve the name of a safe wrapper, PyO3 cannot leave UB possible in safe code. This is the benefit that outweighs any drawback of more complicated code.

[konstin]

For all I can tell pyo3 is in its general design sound. The known exceptions are:

• &mut self and setters without RefCell(Use RefCell to enforce aliasing rules #342). The solution is relatively straight forward (wrap all rust objects in RefCells before making them part of a PyObject), but the implementation requires changing a lot of code. A workaround would be feature gating &mut self and setter with a "you-can-write-unsound-code-this-way" feature.
• Panics are propagated through ffi boundaries (Panics should be caught at the FFI boundary and propagated into python some other way #492). The body of all extern "C" fn that are implemented by pyo3 needs to be wrapped in catch_unwind. I think panic = "abort" would also solve this, but catch_unwind is the proper solution.
• Subclassing is segfaulty (Subclassing a #[pyclass] in Python causes segfault during interpreter finalization #220, Fix compilation with -Z minimal versions #470). I think this isn't general unsoundness but some problem with the gc. This will either get fixed by either Disable segfaulty subclassing by default #540 or a proper solution.
• A missing null check (SIGSEGV when PyIterator is given an integer #494). Will get fixed by Handle null in PyIterator::from_object to fix #494 #544 once figure out why the this one CI job keeps failing.
• The issue this PR fixes obviously.

I'd be happy about pull requests for those issues and I'm willing to help with questions arising when implementing those.

Given the size of pyo3's codebase and the cpython ffi, there are surely more bugs that can produce UB in the code, but I don't think that there are fundamental problems with pyo3's current design.

I'm not against this PR, but to think about what kind of safety we can provide under the current design, we should keep in our mind that we can do mutate operation to all &Py~ types. It means we cannot follow the Rust's covariant rule about &'a types.

Could you explain that a bit more? For my understanding what we're doing is well defined because every modification happens through a call to a c function with a raw pointer, which are both opaque to rustc.

[kngwyu]

Could you explain that a bit more?

Sorry for my poor English and explanation, ...
In Rust, fn(&A)->&B is generally safe(so &~ types are covariant), but in our case, it is sometimes not safe since &Py~ types are actually mutable(e.g., the case A is PyByteArray and B is [u8]).
I mean this fact makes it difficult to define what kind of safety we can provide.
We don't explicitly follow Rust's rule about references.
To provide safe interfaces, one realistic way is to wrap internal data(say, &[u8]) by *Cell when we expose it Rust types, since *Cell types are considered as invariant(≒ we can retrieve &mut from &) in Rust's manner.

For my understanding what we're doing is well defined because every modification happens through a call to a c function with a raw pointer, which are both opaque to rustc.

Yeah, I think this is a good interpretation of what kind of safety we can provide.
But there still remain unsafety in that we can do mutate opretation via py.run without unsafe block.