lint on more incorrect usages of core::ffi::c_void

[Jules-Bertholet]

View all comments

core::ffi::c_void has size 1 and a highly non-trivial validity invariant. It should only ever be used behind a raw pointer. However, people mess this up all the time.

• c_void_references: Lint against & or &mut references to c_void. This is a widespread mistake, e.g. here in Miri, here across GitHub.
• c_void_statics: Lint against statics and consts of type c_void. Examples of this mistake across GitHub
• c_void_parameters: Lint against usage of c_void as a function parameter type. Examples of this mistake across GitHub
• c_void_values: Lint against usage of c_void by value in MIR.

Also fixes false negatives in the pre-exisiting c_void_returns. Like that lint, these new lints are warn-by-default, but should eventually be upgraded to deny.

See also #159935

@rustbot label T-lang needs-fcp A-FFI

[rustbot]

miri is developed in its own repository. If the Miri part of this change can be broken out, consider making this change to rust-lang/miri instead. However, if Miri needs adjusting for rustc changes, just ignore this message.

cc @rust-lang/miri

[rustbot]

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 74 candidates
• Random selection from 17 candidates

[RalfJung]

The lint should probably also complain about statics of type c_void.

[Jules-Bertholet]

The lint should probably also complain about statics of type c_void.

Added another lint for this

[Jules-Bertholet]

Added a third lint, against c_void function parameters.

[theemathas]

Idea (not sure if this is feasible): If we do anything that asks for the size of c_void, emit a warning unless it's for a pointer offset operation

[mejrs]

Idea (not sure if this is feasible): If we do anything that asks for the size of c_void, emit a warning unless it's for a pointer offset operation

We can emit a warning regardless, with a note to just use byte_add for offset operations. (Is that what people are using size_of for..?)

[rustbot]

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

[Jules-Bertholet]

Added a fourth lint to check for incorrect usage of c_void in MIR.

[tgross35]

Perhaps also:

let x: c_void;

View changes since the review

[tgross35]

Apologies if missed, but do any of these catch patterns like struct S(core::ffi::c_void);? This is rejected by C compilers.

[Jules-Bertholet]

Apologies if missed, but do any of these catch patterns like struct S(core::ffi::c_void);? This is rejected by C compilers.

No. For something like a #[repr(transparent)] pub struct Foo(c_void); that is only used behind a raw pointer, I'm not confident we want to lint, so I decided to start with the most clearly wrong stuff first. But if the lang team is fine with it, sure we can lint on that too

[tgross35]

Assuming this needs lang to take a look and that it is about ready for it:

@rustbot label +I-lang-nominated

[Jules-Bertholet]

One thing I would like lang's input on: should we merge the lints? We could have a single lint for all incorrect c_void usage, or one lint for by-ref and one for by-value. Note that the pre-existing c_void_returns hits stable in 3 weeks.

[traviscross]

Thanks @Jules-Bertholet.

@rfcbot reviewed

[scottmcm]

Yes, we should ship them. Agreed they should start at warn, but also agreed that in 2..=12 months we should move them to deny.

@rfcbot reviewed

No strong feelings about one lint vs multiple lints or whether there should be a group. I don't think people should be allowing them as a group, so not having a single allow for them is probably fine?

[rust-rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rust-rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.