Code review for unsafe code in diesel

[Yandros]

@weiznich

Now in my understanding this results into having a owned value + a const * to the underlying buffer at the same time, which would violate rusts aliasing rules, right? This means I would need to wrap certain parts of the code (likely the binds_to_free and _query field of StatementUse ) in a UnsafeCell to communicate that to the compiler, or I'm wrong here?

UnsafeCell wouldn't be the right tool here, since that one interacts with &-access that wants to temporarily feature mutable, and what you are mentioning is &mut or by-value access to the owned value (which is an exclusive access to that value, and, if high-level Rust types (≠ raw pointers) are used, such assumption from the compiler could be transitive through those.

I've detailed this problem in the following post (when talking about owning_ref problems).

How can we teach people about self-referential types?

My own remarks: For those coming with an engineering "design issue" that aren't ready yet to hear a "this is an XY problem" kind of answer, I often tell them about reference counting with weak refs. It can be a bit cumbersome, but at least it is correct and safe (the worse thing that can happen being memory leaks if they get their cycles wrong). What I like about suggesting this approach is that smart pointers / reference-counting is way closer to what most programmers from gc languages ha…

Indeed, your code technically suffers from the same issues, but:

• this is a Stacked Borrows issue which may not be applied to Rust (precisely because of how problematic in can be in cases such as yours);

• in your case, the code dereferencing those pointers would be FFI code (sqlite3), so, in practice, it is unexploitable by the compiler. Although cross-language LTO could change that .

• and the main argument: there is currently no general-purpose tool to opt out of the aforementioned aliasing requirements / assumptions. In that post, I do showcase a hand-rolled AliasableBox definition. In general, that would be the only true tool to opt out of "pointer-transitive lack-of-aliasing assumptions", so you'd need to do an AliasableVec definition as well, whereby wrapping the Vec raw parts (an example).

  • EDIT: I just found out about the ::aliasable crate, which does feature both an AliasableBox and an AliasableVec!

  There is currently a partially-hacky general-purpose tool for these things, though: ManuallyDrop and MaybeUninit. Since both may be carrying stale data, their being valid to assume usable is a runtime property through which the alias analysis ought not be able to see. This means that ManuallyDrop<Box<…>> may have the same properties as AliasableBox<…> (or rather, lack thereof), as well as for Vec. Given that I've already recommended that the Vec be wrapped in a ManuallyDrop, it's all the more reason to do so, and do it for the Box as well (just don't forget to ManuallyDrop::drop(…) it after un-bind-ding the values).