diff --git a/doc/src/config-lints.md b/doc/src/config-lints.md index 7fd7cc67..182419db 100644 --- a/doc/src/config-lints.md +++ b/doc/src/config-lints.md @@ -217,6 +217,23 @@ std::io::stderr().write_all(b"foobar").unwrap(); let _stdin_is_forbidden_too = std::io::stdin(); ``` +### `plrust_static_impls` + +This lint forbids certain `impl` blocks for types containing `&'static` references. The precise details are somewhat obscure, but can usually be avoided by making a custom struct to contain your static reference, which avoids the particular soundness hole we're concerned with. For example: + +```rust +// This is forbidden: +impl SomeTrait for (&'static Foo, Bar) { + // ... +} + +// Instead, do this: +struct MyType(&'static Foo, Bar); +impl SomeTrait for MyType { + // ... +} +``` + ### `plrust_autotrait_impls` This lint forbids explicit implementations of the safe auto traits, as a workaround for various soundness holes around these. It may be relaxed in the future if those are fixed. diff --git a/plrust/src/lib.rs b/plrust/src/lib.rs index 4380c153..88f46741 100644 --- a/plrust/src/lib.rs +++ b/plrust/src/lib.rs @@ -70,6 +70,7 @@ const DEFAULT_LINTS: &'static str = "\ plrust_lifetime_parameterized_traits, \ implied_bounds_entailment, \ plrust_autotrait_impls, \ + plrust_static_impls, \ plrust_fn_pointers, \ plrust_filesystem_macros, \ plrust_env_macros, \ diff --git a/plrustc/plrustc/src/lints.rs b/plrustc/plrustc/src/lints.rs index 43fa6794..cd6400f1 100644 --- a/plrustc/plrustc/src/lints.rs +++ b/plrustc/plrustc/src/lints.rs @@ -62,6 +62,96 @@ impl<'tcx> LateLintPass<'tcx> for NoExternBlockPass { } } +declare_plrust_lint!( + pub(crate) PLRUST_STATIC_IMPLS, + "Disallow impl blocks for types containing `'static` references" +); + +declare_lint_pass!(PlrustStaticImpls => [PLRUST_STATIC_IMPLS]); + +impl<'tcx> LateLintPass<'tcx> for PlrustStaticImpls { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) { + let hir::ItemKind::Impl(imp) = &item.kind else { + return; + }; + if self.has_static(imp.self_ty) { + cx.lint( + PLRUST_STATIC_IMPLS, + "`impl` blocks for types containing `'static` references are not allowed", + |b| b.set_span(imp.self_ty.span), + ) + } + } +} + +impl PlrustStaticImpls { + /// This is pretty naïve and designed to match the specific patterns that + /// trip up https://github.com/rust-lang/rust/issues/104005... + /// + /// Also, I feel like I should be able to use `hir::intravisit::walk_ty` + /// here instead, but it doesn't seem to let be know the lifetime of refs, + /// so... not sure... + /// + /// It's a method on `self` mostly to discourage use in other contexts, + /// since it's probably wrong for them. + fn has_static(&self, t: &hir::Ty) -> bool { + use hir::{Lifetime, LifetimeName::Static, MutTy, TyKind}; + match &t.kind { + TyKind::Infer + | TyKind::Err + // Doesn't exist + | TyKind::Typeof(..) + // Not strictly correct but we forbid this elsewhere anyway. + | TyKind::BareFn(..) + // TAIT stuff, still unstable at the moment, seems very hard to + // prevent this for... + | TyKind::OpaqueDef(..) + | TyKind::Never => false, + // Found one! + TyKind::Rptr(Lifetime { res: Static, .. }, _) | TyKind::TraitObject(_, Lifetime { res: Static, .. }, _) => true, + // Need to keep looking. + TyKind::Rptr(_, MutTy { ty, .. }) + | TyKind::Ptr(MutTy { ty, .. }) + | TyKind::Array(ty, _) + | TyKind::Slice(ty) => self.has_static(*ty), + + TyKind::Tup(types) => types.iter().any(|t| self.has_static(t)), + + TyKind::TraitObject(polytrait, ..) => { + polytrait.iter().any(|poly| { + self.segments_have_static(poly.trait_ref.path.segments) + }) + } + // Something like `Vec` or `Option`. Need to look inside... + TyKind::Path(qpath) => { + let segs = match qpath { + hir::QPath::Resolved(Some(maybe_ty), _) if self.has_static(maybe_ty) => return true, + hir::QPath::TypeRelative(t, _) if self.has_static(*t) => return true, + hir::QPath::LangItem(..) => return false, + hir::QPath::Resolved(_, path) => path.segments, + hir::QPath::TypeRelative(_, seg) => std::slice::from_ref(*seg), + }; + self.segments_have_static(segs) + } + } + } + + fn segments_have_static(&self, segs: &[hir::PathSegment]) -> bool { + segs.iter().any(|seg| { + seg.args().args.iter().any(|arg| match arg { + hir::GenericArg::Lifetime(hir::Lifetime { + res: hir::LifetimeName::Static, + .. + }) => true, + hir::GenericArg::Type(t) => self.has_static(t), + hir::GenericArg::Const(_) | hir::GenericArg::Infer(_) + // Wasn't static + | hir::GenericArg::Lifetime(_) => false, + }) + }) + } +} + declare_plrust_lint!( pub(crate) PLRUST_AUTOTRAIT_IMPLS, "Disallow impls of auto traits", @@ -535,6 +625,7 @@ static PLRUST_LINTS: Lazy> = Lazy::new(|| { let mut v = vec![ PLRUST_ASYNC, PLRUST_AUTOTRAIT_IMPLS, + PLRUST_STATIC_IMPLS, PLRUST_EXTERN_BLOCKS, PLRUST_EXTERNAL_MOD, PLRUST_FILESYSTEM_MACROS, @@ -580,6 +671,7 @@ pub fn register(store: &mut LintStore, _sess: &Session) { store.register_early_pass(move || Box::new(PlrustAsync)); store.register_early_pass(move || Box::new(PlrustExternalMod)); store.register_late_pass(move |_| Box::new(PlrustAutoTraitImpls)); + store.register_late_pass(move |_| Box::new(PlrustStaticImpls)); store.register_late_pass(move |_| Box::new(PlrustFnPointer)); store.register_late_pass(move |_| Box::new(PlrustLeaky)); store.register_late_pass(move |_| Box::new(PlrustBuiltinMacros)); diff --git a/plrustc/plrustc/uitests/static_impl_etc.rs b/plrustc/plrustc/uitests/static_impl_etc.rs new file mode 100644 index 00000000..d3a1aed2 --- /dev/null +++ b/plrustc/plrustc/uitests/static_impl_etc.rs @@ -0,0 +1,18 @@ +#![crate_type = "lib"] + +trait Foo {} +struct Bar(core::marker::PhantomData<(A, B)>); +struct Baz<'a, A>(core::marker::PhantomData<&'a A>); +trait Quux {} + +impl Foo for &'static T {} +impl Foo for &'static mut T {} +impl Foo for [&'static T] {} +impl Foo for &[&'static T] {} +impl Foo for (i32, [&'static T]) {} +impl Foo for (i32, [&'static T; 1]) {} +impl Foo for *const &'static T {} +impl Foo for Bar {} +impl Foo for Baz<'static, T> {} +impl Foo for dyn Quux<&'static T> {} +impl Foo for &'static dyn Quux {} diff --git a/plrustc/plrustc/uitests/static_impl_etc.stderr b/plrustc/plrustc/uitests/static_impl_etc.stderr new file mode 100644 index 00000000..e0f376f7 --- /dev/null +++ b/plrustc/plrustc/uitests/static_impl_etc.stderr @@ -0,0 +1,70 @@ +error: `impl` blocks for types containing `'static` references are not allowed + --> $DIR/static_impl_etc.rs:8:17 + | +LL | impl Foo for &'static T {} + | ^^^^^^^^^^ + | + = note: `-F plrust-static-impls` implied by `-F plrust-lints` + +error: `impl` blocks for types containing `'static` references are not allowed + --> $DIR/static_impl_etc.rs:9:17 + | +LL | impl Foo for &'static mut T {} + | ^^^^^^^^^^^^^^ + +error: `impl` blocks for types containing `'static` references are not allowed + --> $DIR/static_impl_etc.rs:10:17 + | +LL | impl Foo for [&'static T] {} + | ^^^^^^^^^^^^ + +error: `impl` blocks for types containing `'static` references are not allowed + --> $DIR/static_impl_etc.rs:11:17 + | +LL | impl Foo for &[&'static T] {} + | ^^^^^^^^^^^^^ + +error: `impl` blocks for types containing `'static` references are not allowed + --> $DIR/static_impl_etc.rs:12:17 + | +LL | impl Foo for (i32, [&'static T]) {} + | ^^^^^^^^^^^^^^^^^^^ + +error: `impl` blocks for types containing `'static` references are not allowed + --> $DIR/static_impl_etc.rs:13:17 + | +LL | impl Foo for (i32, [&'static T; 1]) {} + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: `impl` blocks for types containing `'static` references are not allowed + --> $DIR/static_impl_etc.rs:14:17 + | +LL | impl Foo for *const &'static T {} + | ^^^^^^^^^^^^^^^^^ + +error: `impl` blocks for types containing `'static` references are not allowed + --> $DIR/static_impl_etc.rs:15:17 + | +LL | impl Foo for Bar {} + | ^^^^^^^^^^^^^^^^^^^^ + +error: `impl` blocks for types containing `'static` references are not allowed + --> $DIR/static_impl_etc.rs:16:17 + | +LL | impl Foo for Baz<'static, T> {} + | ^^^^^^^^^^^^^^^ + +error: `impl` blocks for types containing `'static` references are not allowed + --> $DIR/static_impl_etc.rs:17:17 + | +LL | impl Foo for dyn Quux<&'static T> {} + | ^^^^^^^^^^^^^^^^^^^^ + +error: `impl` blocks for types containing `'static` references are not allowed + --> $DIR/static_impl_etc.rs:18:17 + | +LL | impl Foo for &'static dyn Quux {} + | ^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 11 previous errors + diff --git a/plrustc/plrustc/uitests/static_impl_unsound.rs b/plrustc/plrustc/uitests/static_impl_unsound.rs new file mode 100644 index 00000000..352ff108 --- /dev/null +++ b/plrustc/plrustc/uitests/static_impl_unsound.rs @@ -0,0 +1,27 @@ +#![crate_type = "lib"] + +use std::fmt::Display; + +trait Displayable { + fn display(self) -> Box; +} + +// This is more complex than the one in the issue, to make sure the `Box`'s +// lang_item status doesn't bite us. +impl Displayable for (T, Box>) { + fn display(self) -> Box { + Box::new(self.0) + } +} + +fn extend_lt(val: T) -> Box +where + (T, Box>): Displayable, +{ + Displayable::display((val, Box::new(None))) +} + +pub fn get_garbage(s: &str) -> String { + let val = extend_lt(&String::from(s)); + val.to_string() +} diff --git a/plrustc/plrustc/uitests/static_impl_unsound.stderr b/plrustc/plrustc/uitests/static_impl_unsound.stderr new file mode 100644 index 00000000..b37d996b --- /dev/null +++ b/plrustc/plrustc/uitests/static_impl_unsound.stderr @@ -0,0 +1,10 @@ +error: `impl` blocks for types containing `'static` references are not allowed + --> $DIR/static_impl_unsound.rs:11:34 + | +LL | impl Displayable for (T, Box>) { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-F plrust-static-impls` implied by `-F plrust-lints` + +error: aborting due to previous error +