Skip to content

Annotate all unsafe blocks with a safety comment and prevent regressions #429

Open
@joshlf

Description

As part of #61, we need to make sure that all unsafe code is proven to be sound. Currently, all unsafe code is either documented with a safety comment (// SAFETY: ...), or is marked with a TODO that references this issue. Our goal is to reach 100% safety comment coverage, and to not regress once we've reached 100%. To that end, we enforce the clippy::undocumented_unsafe_blocks lint to prevent regressions.

A note on linting: It'd be nice to be able to replace the top-level #![deny(clippy::undocumented_unsafe_blocks)] with a forbid once all TODOs are burned down, but unfortunately our safety_comment! macro relies on the ability to use #[allow(clippy::undocumented_unsafe_blocks)], so we have to settle for a deny.

In order to ensure that our soundness is forwards-compatible, safety comments must satisfy the following criteria:

  • Safety comments must constitute a (possibly informal) proof that all of Rust's soundness rules are upheld
  • Safety comments must not rely on any statements which do not appear in the stable versions of the Rust reference or standard library documentation (ie, the docs for core, alloc, and std); arguments which rely on text from the beta or nightly versions of these documents are not considered complete
  • All statements from the reference or standard library documentation which are relied upon for soundness must be quoted in the safety comment. This ensures that there is no ambiguity as to what aspect of the text is being cited. This is especially important in cases where the text of these documents changes in the future. Such changes are of course required to be backwards-compatible, but may change the manner in which a particular guarantee is explained.

Mentoring instructions

  • Easy tasks
    • Find any safety comment which links to a version of the Rust reference or stdlib docs that uses a beta or nightly link, and replace it with a link to the stable version of the same docs
  • Medium tasks
    • Find any safety comment which cites the reference or stdlib docs without quoting the text it is citing. Fix this by quoting any text on which the safety comment relies for soundness.
  • Hard tasks
    • See the list of suggested TODOs at the bottom of this issue, or find any instance of TODO(#429) in a comment; leave a GitHub comment on this issue to claim that instance to avoid duplicated work. Write a safety comment that abides by the requirements listed above.

Feel free to ask for help here if you're stuck or have questions!

List of suggested safety comments

List

This list contains safety comments which are good starter safety comments if you're not already familiar with writing them.

  • zerocopy/src/lib.rs

    Lines 757 to 767 in f001cf2

    // TODO(#429): Add a "SAFETY" comment and remove this `allow`.
    #[allow(clippy::undocumented_unsafe_blocks)]
    let ptr = unsafe { alloc::alloc::alloc_zeroed(layout).cast::<Self>() };
    if ptr.is_null() {
    alloc::alloc::handle_alloc_error(layout);
    }
    // TODO(#429): Add a "SAFETY" comment and remove this `allow`.
    #[allow(clippy::undocumented_unsafe_blocks)]
    unsafe {
    Box::from_raw(ptr)
    }
  • zerocopy/src/lib.rs

    Lines 757 to 767 in f001cf2

    // TODO(#429): Add a "SAFETY" comment and remove this `allow`.
    #[allow(clippy::undocumented_unsafe_blocks)]
    let ptr = unsafe { alloc::alloc::alloc_zeroed(layout).cast::<Self>() };
    if ptr.is_null() {
    alloc::alloc::handle_alloc_error(layout);
    }
    // TODO(#429): Add a "SAFETY" comment and remove this `allow`.
    #[allow(clippy::undocumented_unsafe_blocks)]
    unsafe {
    Box::from_raw(ptr)
    }

Metadata

Assignees

No one assigned

    Labels

    compatibility-nonbreakingChanges that are (likely to be) non-breakingexperience-easyThis issue is easy, and shouldn't require much experienceexperience-hardThis issue is hard, and requires a lot of experienceexperience-mediumThis issue is of medium difficulty, and requires some experiencehelp wantedExtra attention is needed

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions