logo

Rust crates reviews

Cryptographically verifiable, distributed dependency reviews

reviewer: cad97

https://lib.rs/cad97

$ cargo crev repo fetch url https://github.com/cad97/crev-proofs
$ cargo crev id trust e13vqC78LDCc-IiVPs-XkOHWd1j6PJUcYrugasS77hQ

repo: https://github.com/cad97/crev-proofs

crate version
rating
date
reviewer
thoroughness, understanding
positive
2020-02-10
cad97
high, high

I am the author of this crate. As such, I obviously trust it and believe it is useful.
So this review will instead point at the trickiest parts and try to rationalize them.

ErasedPtr is a trivial typedef.
Erasable is an unsafe trait, but an obvious one.

ErasablePtr is a tricky trait, as it encodes the requirements to make Thin work properly.
The methods functionality it provides is self-evident, but the Thin requirements are subtle.
Specifically, ErasablePtr requires Deref and DerefMut to "behave" to make Thin::with(_mut) sound.
It needs to be sound to take an erased pointer, convert it back to the real pointer temporarily,
dereference the real pointer, trash the temporary real pointer, and then use the resulting ref.
This is the requirement laid out by usage with Thin and described generally by the documentation.

Because of the simplicity of the actual functionality, the actual implementations of ErasablePtr
(as well as Erasable) are almost trivially correct, just type erasing and casting the type back.

Thin is the real scary unsafe involved. It relies on the guarantees promised by ErasablePtr
to treat an ErasedPtr as if it were the real, invariant-holding pointer.
The unsafe points are Thin::with, Thin::with_mut, Thin::deref, and Thin::deref_mut.
Thin::with is sound because of the no-shared-mutability-before-indirection requirement,
meaning that the created undropped temporary is a fine substitue for the real pointer.
Thin::with_mut adds the possiblity of mutability-before-indirection, and uses a scope guard
to ensure that the potentially mutated pointer is re-erased and re-stored as the thin pointer.
The Deref and DerefMut impls then rely on the address-independent-deref semantics to tie the
lifetime of the deref'd reference to the erased pointer rather than the temporary real pointer.
The only chance for mutability-before-indirection is in the pointer's own DerefMut impl,
and the use of Thin::with_mut to deref the real pointer ensures that this is properly respected.
The remaining impls on Thin just forward trait impls, as is done for pointers in std.

Note, however, that Thin actually provides little/no actual benefits without an additional
library (slice-dst), because all erasable pointers provided by this crate are already thin.

Smoke tests are run under miri, but the crate could potentially do with some more examples
as well as some more tests to ensure that all of the functionality works as advertised.

Some completely dead code was left in, but has been patched out of the git head,
and a patch version will be released that removes the unneeded dead code.

I've only put this review as positive rather than strong because of a lack of real-world use.
As of yet, the library is just of theoretical use, and has yet to be stress tested for real.
However, I feel perfectly fine recommending it for any/all use case that wants to make working
with type-erased pointers simpler, or uses slice-dst and wants thin pointers to those structures.

positive
2020-02-10
cad97
high, high

I am the author of this crate. As such, I obviously trust it and believe it is useful.
So this review will instead point at the trickiest parts and try to rationalize them.

Don't believe cargo-geiger for this crate. The majority of the API is macro-generated,
and cargo-geiger misses unsafe invocations emitted by the complex macro.

This crate makes liberal usage of unreachable_unchecked,
but for cases where the case is trivially actually unreachable.

However, using this crate will decrease the required unsafe for your crate,
as it gives you alignment-tagged pointer unions for only one line of unsafe per union type.
The unsafe burden you take on is to guarantee the alignment required to store the tag within.

Smoke tests are run under miri, but the crate could potentially do with some more examples
as well as some more tests to ensure that all of the functionality works as advertised.

I've only put this review as positive rather than strong because of a lack of real-world use.
As of yet, the library is just of theoretical use, and has yet to be stress tested for real.

positive
2020-02-10
cad97
high, high
alternative:
triomphe

I am the author of this crate. As such, I obviously trust it and believe it is useful.
So this review will instead point at the trickiest parts and try to rationalize them.

Do not believe cargo-geiger for this crate. The implementation is primarily macro-generated,
and cargo-geiger does not see into unsafe generated in macros.

That said, this crate is fairly simple and self-evident.
The main tricky bit is around pointer provenance when reconstructing the (A)Rc.
See https://internals.rust-lang.org/t/_/11463/11 for some context.
The standard library currently does this wrong, even!
Getting the "raw" reference as &**arc gives shared immutable provenance,
and (A)Rc requires "raw mutable" provenance, because of get_mut.

If you're reading this and you ever upgrade &T to (A)Rc, replace it with this crate!
The crate uses autocfg to automatically probe for the "fix" methods of std providing a
(A)Rc::as_raw, so will automatically upgrade to the purely sound version when it's available.

Smoke tests are run under miri, but the crate could potentially do with some more examples
as well as some more tests to ensure that all of the functionality works as advertised.

I've only put this review as positive rather than strong because of a lack of real-world use.
As of yet, the library is just of theoretical use, and has yet to be stress tested for real.

positive
2020-02-10
cad97
high, high

I am the author of this crate. As such, I obviously trust it and believe it is useful.
So this review will instead point at the trickiest parts and try to rationalize them.

Do not believe cargo-geiger for this crate. The implementation is primarily macro-generated,
and cargo-geiger does not see into unsafe generated in macros.

That said, this crate is fairly simple and self-evident.

Smoke tests are run under miri, but the crate could potentially do with some more examples
as well as some more tests to ensure that all of the functionality works as advertised.

I've only put this review as positive rather than strong because of a lack of real-world use.
As of yet, the library is just of theoretical use, and has yet to be stress tested for real.

positive
2020-02-10
cad97
high, high
alternative:
triomphe

I am the author of this crate. As such, I obviously trust it and believe it is useful.
So this review will instead point at the trickiest parts and try to rationalize them.

First and foremost, a caution: YOLO_RC_HEAP_LAYOUT_KNOWN is unsound, don't use it.
It isn't public API nor documented API, but it does create "working" code that only
happens to work for the current compiler and relies on too many implementation details.

This crate provides a completely safe API through SliceWithHeader.
Prefer using it to the unsafe APIs that power it.

It should be noted that this crate's unsafety is purely on slice dst construction.
Once the slice dst has been manually allocated into a Box, everything becomes safe again.
Rust actually supports these trailing-slice DSTs perfectly well,
it is only construction that is unsafe.

Triomphe also provides a HeaderSlice<HeaderWithLength, [Item]> equivalent to slice-dst's
SliceWithHeader<Header, Item>. Slice-dst's is usable with std smart pointers, wheras triomphe's
is only usable with triomphe::Arc (and variants). Triomphe, however, is more battle tested.

Smoke tests are run under miri, but the crate could potentially do with some more examples
as well as some more tests to ensure that all of the functionality works as advertised.

I've only put this review as positive rather than strong because of a lack of real-world use.
As of yet, the library is just of theoretical use, and has yet to be stress tested for real.

© bestia.dev 2023, MIT License, Version: 2023.608.1636

Open source repository for this web app: https://github.com/bestia-dev/cargo_crev_web/