rules_rust icon indicating copy to clipboard operation
rules_rust copied to clipboard

Support allow/forbid of unsafe code

Open buchgr opened this issue 1 year ago • 3 comments

Hello rules_rust 👋 ,

We have a use case where in our project we want to

  • forbid the use of unsafe in general, but enable specific targets to opt-in to unsafe. When I say unsafe I mean everything with the unsafe keyword: blocks, traits, impls, ...
  • Annotate a target with properties of unsafe code it contains e.g. FFI code, whether it's been reviewed for soundness, the UB risk and so. We want to eventually be able to query for a specific target which kind of unsafe code it and any dependencies have.

I would like to propose the following:

  • Add a unsafe_code attribute to the rust_toolchain and the rust_(library|binary|...) rules.
  • The unsafe_code attribute on the toolchain controls the default (allowed / forbid) and individual targets can override it by setting their own unsafe_code attribute. The attributes default to allowed.
  • If our logic decides that unsafe code should be forbidden we set the rustc flag -Funsafe_code.
  • I intentionally did not mention the type of the attribute so far. I propose to make it a label_list that takes an UnsafeCodeInfo provider. The idea here is to make the mechanism extensible. rules_rust would define the basic allowed and forbidden values and projects can add their own targets, that ultimately will also just allow / forbid unsafe code, but describe the use of unsafe in more detail.

Here's an example of what this could look like in BUILD files.

rust_toolchain(
  unsafe_code = ["@rules_rust//unsafe_code:forbidden"],
  ...
)

rust_library(
  name = "unsafe_foo",
  unsafe_code = ["@rules_rust//unsafe_code:allowed"],
)

rust_library(
  name = "unsafe_bar",
  unsafe_code = [
    "@my_project//unsafe_code:ffi",
    "@my_project//unsafe_code:concurrency",
  ],
)

I am curious to learn what you think.

buchgr avatar Jul 31 '24 08:07 buchgr

Sounds very reasonable to me!

illicitonion avatar Jul 31 '24 09:07 illicitonion

Why not handle this with aspects and a tag? E.g. any targets tagged with unsafe are allowed to use unsafe code and the aspect could recompile with that check? Or maybe this is done via the toolchain where you can pass a tag_flags attribute that maps certain flags to a list of additional rustc flags to enable/disable?

UebelAndre avatar Jul 31 '24 12:07 UebelAndre

Note: After thinking about your proposal a bit more, I edited my reply so as to stay focused on the main point.

I think the aspect solution would be a hard blocker for us, because it requires the code to be compiled twice. We wouldn't be able to have this for build speed & cost reasons.

buchgr avatar Jul 31 '24 13:07 buchgr