Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should we try to be more formal about safety? #124

Open
llogiq opened this issue Aug 27, 2018 · 4 comments
Open

Should we try to be more formal about safety? #124

llogiq opened this issue Aug 27, 2018 · 4 comments

Comments

@llogiq
Copy link
Contributor

llogiq commented Aug 27, 2018

e.g. I could envision a fn upholds_invariants() -> bool method that could be debug_assert!ed on all methods that use unsafe, thereby allowing us to fuzz-check for mistakes we may have made w.r.t. memory safety. Alas, there are some invariants we cannot check for now (namely around uninitialized memory), but we could at least document them.

We could also add more comments describing why what we do really should be safe, so others can understand – and try to poke holes into – our thinking.

@dpc
Copy link
Contributor

dpc commented Jul 3, 2019

I was just filling a cargo-crev advisory, following RustSec one, and I've noticed that a while ago I was briefly reviewing this crate and what was striking to me, that it is in dear need of fuzzing

You could follow examples from eg. rust-bitcoin project fuzzing.

@Shnatsel
Copy link

Shnatsel commented Sep 3, 2019

https://github.com/jakubadamw/arbitrary-model-tests should make fuzzing a breeze, since you already have std::vec as a model to check against.

dpc added a commit to dpc/rust-smallvec that referenced this issue Oct 13, 2019
dpc added a commit to dpc/rust-smallvec that referenced this issue Oct 13, 2019
Add infrastructure to automatically run fuzzers in CI,
and implement a simple fuzzing test based on triggering all (most)
public APIs in a randimized way.

As far as I was able to try it catches the previous unsoundness issues
in a matter of seconds. This can be tried by changing the `path = "../"` dependency to
`version = "=0.6.3"` etc. and running the fuzzer manually. (Note: You'll need
to tweak the `Cargo.lock` to allow downloading the yanked versions).

Related to servo#124
dpc added a commit to dpc/rust-smallvec that referenced this issue Oct 29, 2019
Add infrastructure to automatically run fuzzers in CI,
and implement a simple fuzzing test based on triggering all (most)
public APIs in a randimized way.

As far as I was able to try it catches the previous unsoundness issues
in a matter of seconds. This can be tried by changing the `path = "../"` dependency to
`version = "=0.6.3"` etc. and running the fuzzer manually. (Note: You'll need
to tweak the `Cargo.lock` to allow downloading the yanked versions).

Related to servo#124
dpc added a commit to dpc/rust-smallvec that referenced this issue Oct 30, 2019
Add infrastructure to automatically run fuzzers in CI,
and implement a simple fuzzing test based on triggering all (most)
public APIs in a randimized way.

As far as I was able to try it catches the previous unsoundness issues
in a matter of seconds. This can be tried by changing the `path = "../"` dependency to
`version = "=0.6.3"` etc. and running the fuzzer manually. (Note: You'll need
to tweak the `Cargo.lock` to allow downloading the yanked versions).

Related to servo#124
dpc added a commit to dpc/rust-smallvec that referenced this issue Oct 30, 2019
Add infrastructure to automatically run fuzzers in CI,
and implement a simple fuzzing test based on triggering all (most)
public APIs in a randimized way.

As far as I was able to try it catches the previous unsoundness issues
in a matter of seconds. This can be tried by changing the `path = "../"` dependency to
`version = "=0.6.3"` etc. and running the fuzzer manually. (Note: You'll need
to tweak the `Cargo.lock` to allow downloading the yanked versions).

Related to servo#124
@aticu
Copy link

aticu commented Jul 21, 2020

I've developed a library for my bachelor's thesis which has the goal to aid developers in being more formal about safety.

It works by annotating unsafe functions with the preconditions that need to be upheld for their safety.
A call to an annotated function will fail to compile, unless it is assured at the call site that the preconditions hold.
Assuring this uses another annotation at the call site.

This has two main advantages:

  • It is documented in the code why a piece of unsafe code is safe.
  • This documentation is partially checked by the compiler. If the conditions that need to be upheld change, it can be noticed right away.

Such annotations could be added behind a cfg as a dev-dependency or also - optionally - behind a feature flag as part of the public API for the unsafe functions.

If you'd be interested in such annotations, I would be happy to prepare a pull request adding some of them to smallvec. If so, which part of the code would benefit most from such annotations?

If you're not interested, I'd appreciate it, if you could briefly let me know why, as that would be valuable feedback for my thesis.

@Shnatsel
Copy link

Shnatsel commented Jul 21, 2020

Meanwhile arbitrary-model-tests was renamed to rutenspitz and since yesterday correctly handles panics. It should make fuzzing very easy, and also easily allow verifying that behavior of SmallVec is identical to that of Vec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants