Skip to content

Commit

Permalink
Add a lint for certain suspicious trait object usage (#264)
Browse files Browse the repository at this point in the history
* Add a lint for certain suspicious trait object usage
* Add some comments on ignored match items
  • Loading branch information
thomcc authored Mar 23, 2023
1 parent d8c6d4a commit 9641933
Show file tree
Hide file tree
Showing 7 changed files with 192 additions and 0 deletions.
11 changes: 11 additions & 0 deletions doc/src/config-lints.md
Original file line number Diff line number Diff line change
Expand Up @@ -253,3 +253,14 @@ impl std::marker::Unpin for Foo {}
```

As a workaround, in most cases, you should be able to use [`std::panic::AssertUnwindSafe`](https://doc.rust-lang.org/nightly/std/panic/struct.AssertUnwindSafe.html) instead of implementing one of the `UnwindSafe` traits, and Boxing your type can usually work around the need for `Unpin` (which should be rare in non-`async` code anyway).

### `plrust_suspicious_trait_object`

This lint forbids trait object use in turbofish and generic defaults. This is an effort to fix [certain soundness holes](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=764d78856996e1985ee88819b013c645) in the Rust language. More simply, the following patterns are disallowed:

```rs
// Trait object in turbofish
foo::<dyn SomeTrait>();
// Trait object in type default (enum, union, trait, and so on are all also forbidden)
struct SomeStruct<T = dyn SomeTrait>(...);
```
1 change: 1 addition & 0 deletions plrust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ const DEFAULT_LINTS: &'static str = "\
plrust_external_mod, \
plrust_print_macros, \
plrust_stdio, \
plrust_suspicious_trait_object, \
unsafe_code, \
deprecated, \
suspicious_auto_trait_impls, \
Expand Down
65 changes: 65 additions & 0 deletions plrustc/plrustc/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,69 @@ impl<'tcx> LateLintPass<'tcx> for PlrustAutoTraitImpls {
}
}

declare_plrust_lint!(
pub(crate) PLRUST_SUSPICIOUS_TRAIT_OBJECT,
"Disallow suspicious generic use of trait objects",
);

declare_lint_pass!(PlrustSuspiciousTraitObject => [PLRUST_SUSPICIOUS_TRAIT_OBJECT]);

impl<'tcx> LateLintPass<'tcx> for PlrustSuspiciousTraitObject {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &hir::Expr) {
let path_segments = match &expr.kind {
hir::ExprKind::Path(hir::QPath::Resolved(_, path)) => path.segments,
hir::ExprKind::Path(hir::QPath::TypeRelative(_, segment, ..))
| hir::ExprKind::MethodCall(segment, ..) => std::slice::from_ref(*segment),
// We're looking for expressions that (directly, since `check_expr`
// will visit stuff that contains them through `Expr`) contain
// paths, and there's nothing else.
_ => return,
};
for segment in path_segments {
let Some(args) = segment.args else {
continue;
};
for arg in args.args {
let hir::GenericArg::Type(ty) = arg else {
continue;
};
if let hir::TyKind::TraitObject(..) = &ty.kind {
cx.lint(
PLRUST_SUSPICIOUS_TRAIT_OBJECT,
"trait objects in turbofish are forbidden",
|b| b.set_span(expr.span),
);
}
}
}
}

fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) {
let generics = match &item.kind {
hir::ItemKind::TyAlias(_, generics) => *generics,
hir::ItemKind::Enum(_, generics) => *generics,
hir::ItemKind::Struct(_, generics) => *generics,
hir::ItemKind::Union(_, generics) => *generics,
hir::ItemKind::Trait(_, _, generics, ..) => *generics,
hir::ItemKind::Fn(_, generics, ..) => *generics,
// Nothing else is stable and has `Generics`.
_ => return,
};
for param in generics.params {
let hir::GenericParamKind::Type { default: Some(ty), .. } = &param.kind else {
continue;
};
if let hir::TyKind::TraitObject(..) = &ty.kind {
cx.lint(
PLRUST_SUSPICIOUS_TRAIT_OBJECT,
"trait objects in generic defaults are forbidden",
|b| b.set_span(item.span),
);
}
}
}
}

declare_plrust_lint!(
pub(crate) PLRUST_LIFETIME_PARAMETERIZED_TRAITS,
"Disallow lifetime parameterized traits"
Expand Down Expand Up @@ -635,6 +698,7 @@ static PLRUST_LINTS: Lazy<Vec<&'static Lint>> = Lazy::new(|| {
PLRUST_LIFETIME_PARAMETERIZED_TRAITS,
PLRUST_PRINT_MACROS,
PLRUST_STDIO,
PLRUST_SUSPICIOUS_TRAIT_OBJECT,
];
if *INCLUDE_TEST_ONLY_LINTS {
let test_only_lints = [PLRUST_TEST_ONLY_FORCE_ICE];
Expand Down Expand Up @@ -670,6 +734,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(PlrustSuspiciousTraitObject));
store.register_late_pass(move |_| Box::new(PlrustAutoTraitImpls));
store.register_late_pass(move |_| Box::new(PlrustStaticImpls));
store.register_late_pass(move |_| Box::new(PlrustFnPointer));
Expand Down
34 changes: 34 additions & 0 deletions plrustc/plrustc/uitests/sus_trait_obj_items.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#![crate_type = "lib"]

pub trait Foo {}

pub trait Bar<T = dyn Foo>
where
T: ?Sized,
{
}

#[allow(invalid_type_param_default)] // not the lint we're interested in testing
pub fn sus_fn<T = dyn Foo>()
where
T: ?Sized,
{
}

pub struct SusStruct<T = dyn Foo>(pub Box<T>)
where
T: ?Sized;

pub enum SusEnum<T = dyn Foo>
where
T: ?Sized,
{
Something(Box<T>),
}

pub union SusUnion<T = dyn Foo>
where
T: ?Sized,
{
pub something: *const T,
}
54 changes: 54 additions & 0 deletions plrustc/plrustc/uitests/sus_trait_obj_items.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
error: trait objects in generic defaults are forbidden
--> $DIR/sus_trait_obj_items.rs:5:1
|
LL | / pub trait Bar<T = dyn Foo>
LL | | where
LL | | T: ?Sized,
LL | | {
LL | | }
| |_^
|
= note: `-F plrust-suspicious-trait-object` implied by `-F plrust-lints`

error: trait objects in generic defaults are forbidden
--> $DIR/sus_trait_obj_items.rs:12:1
|
LL | / pub fn sus_fn<T = dyn Foo>()
LL | | where
LL | | T: ?Sized,
LL | | {
LL | | }
| |_^

error: trait objects in generic defaults are forbidden
--> $DIR/sus_trait_obj_items.rs:18:1
|
LL | / pub struct SusStruct<T = dyn Foo>(pub Box<T>)
LL | | where
LL | | T: ?Sized;
| |______________^

error: trait objects in generic defaults are forbidden
--> $DIR/sus_trait_obj_items.rs:22:1
|
LL | / pub enum SusEnum<T = dyn Foo>
LL | | where
LL | | T: ?Sized,
LL | | {
LL | | Something(Box<T>),
LL | | }
| |_^

error: trait objects in generic defaults are forbidden
--> $DIR/sus_trait_obj_items.rs:29:1
|
LL | / pub union SusUnion<T = dyn Foo>
LL | | where
LL | | T: ?Sized,
LL | | {
LL | | pub something: *const T,
LL | | }
| |_^

error: aborting due to 5 previous errors

17 changes: 17 additions & 0 deletions plrustc/plrustc/uitests/sus_trait_obj_transmute.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#![crate_type = "lib"]

trait Object<U> {
type Output;
}

impl<T: ?Sized, U> Object<U> for T {
type Output = U;
}

fn foo<T: ?Sized, U>(x: <T as Object<U>>::Output) -> U {
x
}

pub fn transmute<T, U>(x: T) -> U {
foo::<dyn Object<U, Output = T>, U>(x)
}
10 changes: 10 additions & 0 deletions plrustc/plrustc/uitests/sus_trait_obj_transmute.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: trait objects in turbofish are forbidden
--> $DIR/sus_trait_obj_transmute.rs:16:5
|
LL | foo::<dyn Object<U, Output = T>, U>(x)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-F plrust-suspicious-trait-object` implied by `-F plrust-lints`

error: aborting due to previous error

0 comments on commit 9641933

Please sign in to comment.