diff options
author | Samson <16504129+sagudev@users.noreply.github.com> | 2023-12-01 16:50:52 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-12-01 15:50:52 +0000 |
commit | 604979e367faa6aa09805e8fa0223b8883ea009d (patch) | |
tree | b61c5ffefbd4c43aa3e81bd89b70dad41d7c2626 /support | |
parent | 20a73721de2f1a8a0b29905617783148bd3cfaff (diff) | |
download | servo-604979e367faa6aa09805e8fa0223b8883ea009d.tar.gz servo-604979e367faa6aa09805e8fa0223b8883ea009d.zip |
Replace script_plugins with a clippy like rustc driver (named crown) (#30508)
* Remove script_plugins
* Use crown instead of script_plugins
* crown_is_not_used
* Use crown in command base
* bootstrap crown
* tidy happy
* disable sccache
* Bring crown in tree
* Install crown from tree
* fix windows ci
* fix warning
* fix mac
libscript_plugins.dylib is not available anymore
* Update components/script/lib.rs
Co-authored-by: Martin Robinson <mrobinson@igalia.com>
* Update for nightly-2023-03-18
Mostly just based off https://github.com/servo/servo/pull/30630
* Always install crown
it's slow only when there is new version
* Run crown test with `mach test-unit`
* Small fixups; better trace_in_no_trace tests
* Better doc
* crown in config.toml
* Fix tidy for real
* no sccache on rustc_wrapper
* document rustc overrides
* fixup of compiletest
* Make a few minor comment adjustments
* Fix a typo in python/servo/platform/base.py
Co-authored-by: Samson <16504129+sagudev@users.noreply.github.com>
* Proper test types
* Ignore tidy on crown/tests
---------
Co-authored-by: Martin Robinson <mrobinson@igalia.com>
Diffstat (limited to 'support')
17 files changed, 1235 insertions, 0 deletions
diff --git a/support/crown/Cargo.toml b/support/crown/Cargo.toml new file mode 100644 index 00000000000..19a60cc59e8 --- /dev/null +++ b/support/crown/Cargo.toml @@ -0,0 +1,19 @@ +[package] +name = "crown" +authors = ["The Servo Project Developers"] +version = "0.1.0" +edition = "2021" +license = "MPL-2.0" + +[dev-dependencies] +compiletest_rs = { version = "0.10", features = ["tmp"] } +once_cell = "1" + +[features] +default = ["unrooted_must_root_lint", "trace_in_no_trace_lint"] +unrooted_must_root_lint = [] +trace_in_no_trace_lint = [] + +[package.metadata.rust-analyzer] +# This crate uses #![feature(rustc_private)] +rustc_private = true diff --git a/support/crown/src/common.rs b/support/crown/src/common.rs new file mode 100644 index 00000000000..d360b05069e --- /dev/null +++ b/support/crown/src/common.rs @@ -0,0 +1,346 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +use rustc_ast::Mutability; +use rustc_hir::def::{DefKind, Res}; +use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, LOCAL_CRATE}; +use rustc_hir::{ImplItemRef, ItemKind, Node, OwnerId, PrimTy, TraitItemRef}; +use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; +use rustc_infer::infer::TyCtxtInferExt; +use rustc_lint::LateContext; +use rustc_middle::ty::{self, GenericArg, ParamEnv, Ty, TyCtxt, TypeVisitableExt}; +use rustc_span::source_map::{ExpnKind, MacroKind, Span}; +use rustc_span::symbol::{Ident, Symbol}; +use rustc_span::DUMMY_SP; +use rustc_trait_selection::infer::InferCtxtExt; +use rustc_type_ir::{FloatTy, IntTy, UintTy}; + +/// check if a DefId's path matches the given absolute type path +/// usage e.g. with +/// `match_def_path(cx, id, &["core", "option", "Option"])` +pub fn match_def_path(cx: &LateContext, def_id: DefId, path: &[Symbol]) -> bool { + let def_path = cx.tcx.def_path(def_id); + let krate = &cx.tcx.crate_name(def_path.krate); + if krate != &path[0] { + return false; + } + + let path = &path[1..]; + let other = def_path.data; + + if other.len() != path.len() { + return false; + } + + other + .into_iter() + .zip(path) + .all(|(e, p)| e.data.get_opt_name().as_ref() == Some(p)) +} + +pub fn in_derive_expn(span: Span) -> bool { + matches!( + span.ctxt().outer_expn_data().kind, + ExpnKind::Macro(MacroKind::Derive, ..) + ) +} + +#[macro_export] +macro_rules! symbols { + ($($s: ident)+) => { + #[derive(Clone)] + #[allow(non_snake_case)] + pub(crate) struct Symbols { + $( $s: Symbol, )+ + } + + impl Symbols { + fn new() -> Self { + Symbols { + $( $s: Symbol::intern(stringify!($s)), )+ + } + } + } + } +} + +/* +Stuff copied from clippy: +*/ + +fn find_primitive_impls<'tcx>(tcx: TyCtxt<'tcx>, name: &str) -> impl Iterator<Item = DefId> + 'tcx { + use rustc_middle::ty::fast_reject::SimplifiedType::*; + let ty = match name { + "bool" => BoolSimplifiedType, + "char" => CharSimplifiedType, + "str" => StrSimplifiedType, + "array" => ArraySimplifiedType, + "slice" => SliceSimplifiedType, + // FIXME: rustdoc documents these two using just `pointer`. + // + // Maybe this is something we should do here too. + "const_ptr" => PtrSimplifiedType(Mutability::Not), + "mut_ptr" => PtrSimplifiedType(Mutability::Mut), + "isize" => IntSimplifiedType(IntTy::Isize), + "i8" => IntSimplifiedType(IntTy::I8), + "i16" => IntSimplifiedType(IntTy::I16), + "i32" => IntSimplifiedType(IntTy::I32), + "i64" => IntSimplifiedType(IntTy::I64), + "i128" => IntSimplifiedType(IntTy::I128), + "usize" => UintSimplifiedType(UintTy::Usize), + "u8" => UintSimplifiedType(UintTy::U8), + "u16" => UintSimplifiedType(UintTy::U16), + "u32" => UintSimplifiedType(UintTy::U32), + "u64" => UintSimplifiedType(UintTy::U64), + "u128" => UintSimplifiedType(UintTy::U128), + "f32" => FloatSimplifiedType(FloatTy::F32), + "f64" => FloatSimplifiedType(FloatTy::F64), + _ => return [].iter().copied(), + }; + + tcx.incoherent_impls(ty).iter().copied() +} + +fn non_local_item_children_by_name(tcx: TyCtxt<'_>, def_id: DefId, name: Symbol) -> Vec<Res> { + match tcx.def_kind(def_id) { + DefKind::Mod | DefKind::Enum | DefKind::Trait => tcx + .module_children(def_id) + .iter() + .filter(|item| item.ident.name == name) + .map(|child| child.res.expect_non_local()) + .collect(), + DefKind::Impl { .. } => tcx + .associated_item_def_ids(def_id) + .iter() + .copied() + .filter(|assoc_def_id| tcx.item_name(*assoc_def_id) == name) + .map(|assoc_def_id| Res::Def(tcx.def_kind(assoc_def_id), assoc_def_id)) + .collect(), + _ => Vec::new(), + } +} + +fn local_item_children_by_name(tcx: TyCtxt<'_>, local_id: LocalDefId, name: Symbol) -> Vec<Res> { + let hir = tcx.hir(); + + let root_mod; + let item_kind = match hir.find_by_def_id(local_id) { + Some(Node::Crate(r#mod)) => { + root_mod = ItemKind::Mod(r#mod); + &root_mod + }, + Some(Node::Item(item)) => &item.kind, + _ => return Vec::new(), + }; + + let res = |ident: Ident, owner_id: OwnerId| { + if ident.name == name { + let def_id = owner_id.to_def_id(); + Some(Res::Def(tcx.def_kind(def_id), def_id)) + } else { + None + } + }; + + match item_kind { + ItemKind::Mod(r#mod) => r#mod + .item_ids + .iter() + .filter_map(|&item_id| res(hir.item(item_id).ident, item_id.owner_id)) + .collect(), + ItemKind::Impl(r#impl) => r#impl + .items + .iter() + .filter_map(|&ImplItemRef { ident, id, .. }| res(ident, id.owner_id)) + .collect(), + ItemKind::Trait(.., trait_item_refs) => trait_item_refs + .iter() + .filter_map(|&TraitItemRef { ident, id, .. }| res(ident, id.owner_id)) + .collect(), + _ => Vec::new(), + } +} + +fn item_children_by_name(tcx: TyCtxt<'_>, def_id: DefId, name: Symbol) -> Vec<Res> { + if let Some(local_id) = def_id.as_local() { + local_item_children_by_name(tcx, local_id, name) + } else { + non_local_item_children_by_name(tcx, def_id, name) + } +} + +/// Resolves a def path like `std::vec::Vec`. +/// +/// Can return multiple resolutions when there are multiple versions of the same crate, e.g. +/// `memchr::memchr` could return the functions from both memchr 1.0 and memchr 2.0. +/// +/// Also returns multiple results when there are multiple paths under the same name e.g. `std::vec` +/// would have both a [`DefKind::Mod`] and [`DefKind::Macro`]. +/// +/// This function is expensive and should be used sparingly. +pub fn def_path_res(cx: &LateContext<'_>, path: &[&str]) -> Vec<Res> { + fn find_crates(tcx: TyCtxt<'_>, name: Symbol) -> impl Iterator<Item = DefId> + '_ { + tcx.crates(()) + .iter() + .copied() + .filter(move |&num| tcx.crate_name(num) == name) + .map(CrateNum::as_def_id) + } + + let tcx = cx.tcx; + + let (base, mut path) = match *path { + [primitive] => { + return vec![PrimTy::from_name(Symbol::intern(primitive)).map_or(Res::Err, Res::PrimTy)]; + }, + [base, ref path @ ..] => (base, path), + _ => return Vec::new(), + }; + + let base_sym = Symbol::intern(base); + + let local_crate = if tcx.crate_name(LOCAL_CRATE) == base_sym { + Some(LOCAL_CRATE.as_def_id()) + } else { + None + }; + + let starts = find_primitive_impls(tcx, base) + .chain(find_crates(tcx, base_sym)) + .chain(local_crate) + .map(|id| Res::Def(tcx.def_kind(id), id)); + + let mut resolutions: Vec<Res> = starts.collect(); + + while let [segment, rest @ ..] = path { + path = rest; + let segment = Symbol::intern(segment); + + resolutions = resolutions + .into_iter() + .filter_map(|res| res.opt_def_id()) + .flat_map(|def_id| { + // When the current def_id is e.g. `struct S`, check the impl items in + // `impl S { ... }` + let inherent_impl_children = tcx + .inherent_impls(def_id) + .iter() + .flat_map(|&impl_def_id| item_children_by_name(tcx, impl_def_id, segment)); + + let direct_children = item_children_by_name(tcx, def_id, segment); + + inherent_impl_children.chain(direct_children) + }) + .collect(); + } + + resolutions +} + +/// Resolves a def path like `std::vec::Vec`, but searches only local crate +/// +/// Also returns multiple results when there are multiple paths under the same name e.g. `std::vec` +/// would have both a [`DefKind::Mod`] and [`DefKind::Macro`]. +/// +/// This function is less expensive than `def_path_res` and should be used sparingly. +pub fn def_local_res(cx: &LateContext<'_>, path: &str) -> Vec<Res> { + let tcx = cx.tcx; + let local_crate = LOCAL_CRATE.as_def_id(); + let starts = Res::Def(tcx.def_kind(local_crate), local_crate); + let mut resolutions: Vec<Res> = vec![starts]; + let segment = Symbol::intern(path); + + resolutions = resolutions + .into_iter() + .filter_map(|res| res.opt_def_id()) + .flat_map(|def_id| { + // When the current def_id is e.g. `struct S`, check the impl items in + // `impl S { ... }` + let inherent_impl_children = tcx + .inherent_impls(def_id) + .iter() + .flat_map(|&impl_def_id| item_children_by_name(tcx, impl_def_id, segment)); + + let direct_children = item_children_by_name(tcx, def_id, segment); + + inherent_impl_children.chain(direct_children) + }) + .collect(); + + resolutions +} + +pub fn get_trait_def_id(cx: &LateContext<'_>, path: &[&str]) -> Option<DefId> { + def_path_res(cx, path) + .into_iter() + .find_map(|res| match res { + Res::Def(DefKind::Trait | DefKind::TraitAlias, trait_id) => Some(trait_id), + _ => None, + }) +} + +pub fn get_local_trait_def_id(cx: &LateContext<'_>, path: &str) -> Option<DefId> { + def_local_res(cx, path) + .into_iter() + .find_map(|res| match res { + Res::Def(DefKind::Trait | DefKind::TraitAlias, trait_id) => Some(trait_id), + _ => None, + }) +} + +/// Checks whether a type implements a trait. +/// The function returns false in case the type contains an inference variable. +/// +/// See: +/// * [`get_trait_def_id`](super::get_trait_def_id) to get a trait [`DefId`]. +/// * [Common tools for writing lints] for an example how to use this function and other options. +/// +/// [Common tools for writing lints]: https://github.com/rust-lang/rust-clippy/blob/master/book/src/development/common_tools_writing_lints.md#checking-if-a-type-implements-a-specific-trait +pub fn implements_trait<'tcx>( + cx: &LateContext<'tcx>, + ty: Ty<'tcx>, + trait_id: DefId, + ty_params: &[GenericArg<'tcx>], +) -> bool { + implements_trait_with_env( + cx.tcx, + cx.param_env, + ty, + trait_id, + ty_params.iter().map(|&arg| Some(arg)), + ) +} + +/// Same as `implements_trait` but allows using a `ParamEnv` different from the lint context. +pub fn implements_trait_with_env<'tcx>( + tcx: TyCtxt<'tcx>, + param_env: ParamEnv<'tcx>, + ty: ty::Ty<'tcx>, + trait_id: DefId, + ty_params: impl IntoIterator<Item = Option<GenericArg<'tcx>>>, +) -> bool { + let ty = tcx.erase_regions(ty); + if ty.has_escaping_bound_vars() { + return false; + } + let infcx = tcx.infer_ctxt().build(); + let orig = TypeVariableOrigin { + kind: TypeVariableOriginKind::MiscVariable, + span: DUMMY_SP, + }; + let ty_params = tcx.mk_substs_from_iter( + ty_params + .into_iter() + .map(|arg| arg.unwrap_or_else(|| infcx.next_ty_var(orig).into())), + ); + infcx + .type_implements_trait( + trait_id, + // for some unknown reason we need to have vec here + // clippy has array + vec![ty.into()].into_iter().chain(ty_params), + param_env, + ) + .must_apply_modulo_regions() +} diff --git a/support/crown/src/crown_is_not_used.rs b/support/crown/src/crown_is_not_used.rs new file mode 100644 index 00000000000..66593a31e05 --- /dev/null +++ b/support/crown/src/crown_is_not_used.rs @@ -0,0 +1,16 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +use rustc_lint::LintStore; +use rustc_session::declare_lint; + +declare_lint! { + CROWN_IS_NOT_USED, + Deny, + "Issues a rustc warning if crown is not used in compilation" +} + +pub fn register(lint_store: &mut LintStore) { + lint_store.register_lints(&[&CROWN_IS_NOT_USED]); +} diff --git a/support/crown/src/main.rs b/support/crown/src/main.rs new file mode 100644 index 00000000000..fa2a16868b1 --- /dev/null +++ b/support/crown/src/main.rs @@ -0,0 +1,68 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#![feature(rustc_private)] +#![warn(rustc::internal)] +#![allow(rustc::potential_query_instability)] + +// This rustc crates are private so they must be manually imported. +extern crate rustc_ast; +extern crate rustc_driver; +extern crate rustc_error_messages; +extern crate rustc_hir; +extern crate rustc_infer; +extern crate rustc_interface; +extern crate rustc_lint; +extern crate rustc_middle; +extern crate rustc_session; +extern crate rustc_span; +extern crate rustc_trait_selection; +extern crate rustc_type_ir; + +use std::process::ExitCode; + +use rustc_driver::Callbacks; +use rustc_interface::interface::Config; + +mod common; + +#[cfg(feature = "unrooted_must_root_lint")] +mod unrooted_must_root; + +#[cfg(feature = "trace_in_no_trace_lint")] +mod trace_in_no_trace; + +mod crown_is_not_used; + +struct MyCallbacks; + +impl Callbacks for MyCallbacks { + fn config(&mut self, config: &mut Config) { + config.register_lints = Some(Box::new(move |sess, lint_store| { + // Skip checks for proc-macro crates. + if sess + .crate_types() + .contains(&rustc_session::config::CrateType::ProcMacro) + { + return; + } + + crown_is_not_used::register(lint_store); + #[cfg(feature = "unrooted_must_root_lint")] + unrooted_must_root::register(lint_store); + #[cfg(feature = "trace_in_no_trace_lint")] + trace_in_no_trace::register(lint_store); + })); + } +} + +fn main() -> ExitCode { + rustc_driver::init_env_logger("CROWN_LOG"); + let args: Vec<_> = std::env::args().collect(); + + match rustc_driver::RunCompiler::new(&args, &mut MyCallbacks).run() { + Ok(_) => ExitCode::SUCCESS, + Err(_) => ExitCode::FAILURE, + } +} diff --git a/support/crown/src/trace_in_no_trace.rs b/support/crown/src/trace_in_no_trace.rs new file mode 100644 index 00000000000..0e685c242a0 --- /dev/null +++ b/support/crown/src/trace_in_no_trace.rs @@ -0,0 +1,196 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +use rustc_ast::ast::{AttrKind, Attribute}; +use rustc_ast::token::TokenKind; +use rustc_ast::tokenstream::TokenTree; +use rustc_ast::AttrArgs; +use rustc_error_messages::MultiSpan; +use rustc_hir::{self as hir}; +use rustc_lint::{LateContext, LateLintPass, LintContext, LintPass, LintStore}; +use rustc_middle::ty; +use rustc_session::declare_tool_lint; +use rustc_span::symbol::Symbol; + +use crate::common::{get_local_trait_def_id, get_trait_def_id, implements_trait}; +use crate::symbols; + +declare_tool_lint! { + pub crown::TRACE_IN_NO_TRACE, + Deny, + "Warn and report incorrect usage of Traceable (jsmanaged) objects in must_not_have_traceable marked wrappers" +} + +declare_tool_lint! { + pub crown::EMPTY_TRACE_IN_NO_TRACE, + Warn, + "Warn about usage of empty Traceable objects in must_not_have_traceable marked wrappers" +} + +const EMPTY_TRACE_IN_NO_TRACE_MSG: &str = + "must_not_have_traceable marked wrapper is not needed for types that implements \ +empty Traceable (like primitive types). Consider removing the wrapper."; + +pub fn register(lint_store: &mut LintStore) { + let symbols = Symbols::new(); + lint_store.register_lints(&[&TRACE_IN_NO_TRACE, &EMPTY_TRACE_IN_NO_TRACE]); + lint_store.register_late_pass(move |_| Box::new(NotracePass::new(symbols.clone()))); +} + +/// Lint for ensuring safe usage of NoTrace wrappers +/// +/// This lint (disable with `-A trace-in-no-trace`/`#[allow(trace_in_no_trace)]`) ensures that +/// wrappers marked with must_not_have_traceable(i: usize) only stores +/// non-jsmanaged (DOES NOT implement JSTraceble) type in i-th generic +/// +/// For example usage look at the tests +pub(crate) struct NotracePass { + symbols: Symbols, +} + +impl NotracePass { + pub(crate) fn new(symbols: Symbols) -> Self { + Self { symbols } + } +} + +impl LintPass for NotracePass { + fn name(&self) -> &'static str { + "ServoNotracePass" + } +} + +fn get_must_not_have_traceable(sym: &Symbols, attrs: &[Attribute]) -> Option<usize> { + attrs + .iter() + .find(|attr| { + matches!( + &attr.kind, + AttrKind::Normal(normal) + if normal.item.path.segments.len() == 3 && + normal.item.path.segments[0].ident.name == sym.crown && + normal.item.path.segments[1].ident.name == sym.trace_in_no_trace_lint && + normal.item.path.segments[2].ident.name == sym.must_not_have_traceable + ) + }) + .map(|x| match &x.get_normal_item().args { + AttrArgs::Empty => 0, + AttrArgs::Delimited(a) => match a + .tokens + .trees() + .next() + .expect("Arguments not found for must_not_have_traceable") + { + TokenTree::Token(tok, _) => match tok.kind { + TokenKind::Literal(lit) => lit.symbol.as_str().parse().unwrap(), + _ => panic!("must_not_have_traceable expected integer literal here"), + }, + TokenTree::Delimited(_, _, _) => { + todo!("must_not_have_traceable does not support multiple notraceable positions") + }, + }, + _ => { + panic!("must_not_have_traceable does not support key-value arguments") + }, + }) +} + +fn is_jstraceable<'tcx>(cx: &LateContext<'tcx>, ty: ty::Ty<'tcx>) -> bool { + // TODO(sagudev): get_trait_def_id is expensive, use lazy and cache it for whole pass + if let Some(trait_id) = get_trait_def_id(cx, &["mozjs", "gc", "Traceable"]) { + return implements_trait(cx, ty, trait_id, &[]); + } + // when running tests + if let Some(trait_id) = get_local_trait_def_id(cx, "JSTraceable") { + return implements_trait(cx, ty, trait_id, &[]); + } + panic!("JSTraceable not found"); +} + +/// Gives warrning or errors for incorect usage of NoTrace like `NoTrace<impl Traceable>`. +fn incorrect_no_trace<'tcx, I: Into<MultiSpan> + Copy>( + sym: &'_ Symbols, + cx: &LateContext<'tcx>, + ty: ty::Ty<'tcx>, + span: I, +) { + let mut walker = ty.walk(); + while let Some(generic_arg) = walker.next() { + let t = match generic_arg.unpack() { + rustc_middle::ty::subst::GenericArgKind::Type(t) => t, + _ => { + walker.skip_current_subtree(); + continue; + }, + }; + let recur_into_subtree = match t.kind() { + ty::Adt(did, substs) => { + if let Some(pos) = + get_must_not_have_traceable(sym, &cx.tcx.get_attrs_unchecked(did.did())) + { + let inner = substs.type_at(pos); + if inner.is_primitive_ty() { + cx.lint( + EMPTY_TRACE_IN_NO_TRACE, + EMPTY_TRACE_IN_NO_TRACE_MSG, + |lint| lint.set_span(span), + ) + } else if is_jstraceable(cx, inner) { + cx.lint( + TRACE_IN_NO_TRACE, + format!( + "must_not_have_traceable marked wrapper must not have \ +jsmanaged inside on {pos}-th position. Consider removing the wrapper." + ), + |lint| lint.set_span(span), + ) + } + false + } else { + true + } + }, + _ => !t.is_primitive_ty(), + }; + if !recur_into_subtree { + walker.skip_current_subtree(); + } + } +} + +// NoTrace correct usage of NoTrace must only be checked on Struct (item) and Enums (variants) +// as these are the only ones that are actually traced +impl<'tcx> LateLintPass<'tcx> for NotracePass { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item) { + // TODO: better performance if we limit with lint attr??? + /*let attrs = cx.tcx.hir().attrs(item.hir_id()); + if has_lint_attr(&self.symbols, &attrs, self.symbols.must_root) { + return; + }*/ + if let hir::ItemKind::Struct(def, ..) = &item.kind { + for ref field in def.fields() { + let field_type = cx.tcx.type_of(field.def_id); + incorrect_no_trace(&self.symbols, cx, field_type.0, field.span); + } + } + } + + fn check_variant(&mut self, cx: &LateContext, var: &hir::Variant) { + match var.data { + hir::VariantData::Tuple(fields, ..) => { + for field in fields { + let field_type = cx.tcx.type_of(field.def_id); + incorrect_no_trace(&self.symbols, cx, field_type.0, field.ty.span); + } + }, + _ => (), // Struct variants already caught by check_struct_def + } + } +} + +symbols! { + crown + trace_in_no_trace_lint + must_not_have_traceable +} diff --git a/support/crown/src/unrooted_must_root.rs b/support/crown/src/unrooted_must_root.rs new file mode 100644 index 00000000000..36c22c3170b --- /dev/null +++ b/support/crown/src/unrooted_must_root.rs @@ -0,0 +1,377 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +use rustc_ast::ast::{AttrKind, Attribute}; +use rustc_hir::{self as hir, intravisit as visit, ExprKind}; +use rustc_lint::{LateContext, LateLintPass, LintContext, LintPass, LintStore}; +use rustc_middle::ty; +use rustc_session::declare_tool_lint; +use rustc_span::def_id::LocalDefId; +use rustc_span::source_map; +use rustc_span::symbol::{sym, Symbol}; + +use crate::common::{in_derive_expn, match_def_path}; +use crate::symbols; + +declare_tool_lint! { + pub crown::UNROOTED_MUST_ROOT, + Deny, + "Warn and report usage of unrooted jsmanaged objects" +} + +pub fn register(lint_store: &mut LintStore) { + let symbols = Symbols::new(); + lint_store.register_lints(&[&UNROOTED_MUST_ROOT]); + lint_store.register_late_pass(move |_| Box::new(UnrootedPass::new(symbols.clone()))); +} + +/// Lint for ensuring safe usage of unrooted pointers +/// +/// This lint (disable with `-A unrooted-must-root`/`#[allow(unrooted_must_root)]`) ensures that +/// `#[crown::unrooted_must_root_lint::must_root]` values are used correctly. +/// +/// "Incorrect" usage includes: +/// +/// - Not being used in a struct/enum field which is not `#[crown::unrooted_must_root_lint::must_root]` itself +/// - Not being used as an argument to a function (Except onces named `new` and `new_inherited`) +/// - Not being bound locally in a `let` statement, assignment, `for` loop, or `match` statement. +/// +/// This helps catch most situations where pointers like `JS<T>` are used in a way that they can be invalidated by a +/// GC pass. +/// +/// Structs which have their own mechanism of rooting their unrooted contents (e.g. `ScriptThread`) +/// can be marked as `#[allow(unrooted_must_root)]`. Smart pointers which root their interior type +/// can be marked as `#[crown::unrooted_must_root_lint::allow_unrooted_interior]` +pub(crate) struct UnrootedPass { + symbols: Symbols, +} + +impl UnrootedPass { + pub(crate) fn new(symbols: Symbols) -> UnrootedPass { + UnrootedPass { symbols } + } +} + +fn has_lint_attr(sym: &Symbols, attrs: &[Attribute], name: Symbol) -> bool { + attrs.iter().any(|attr| { + matches!( + &attr.kind, + AttrKind::Normal(normal) + if normal.item.path.segments.len() == 3 && + normal.item.path.segments[0].ident.name == sym.crown && + normal.item.path.segments[1].ident.name == sym.unrooted_must_root_lint && + normal.item.path.segments[2].ident.name == name + ) + }) +} + +/// Checks if a type is unrooted or contains any owned unrooted types +fn is_unrooted_ty<'tcx>( + sym: &'_ Symbols, + cx: &LateContext<'tcx>, + ty: ty::Ty<'tcx>, + in_new_function: bool, +) -> bool { + let mut ret = false; + let mut walker = ty.walk(); + while let Some(generic_arg) = walker.next() { + let t = match generic_arg.unpack() { + rustc_middle::ty::subst::GenericArgKind::Type(t) => t, + _ => { + walker.skip_current_subtree(); + continue; + }, + }; + let recur_into_subtree = match t.kind() { + ty::Adt(did, substs) => { + let has_attr = + |did, name| has_lint_attr(sym, &cx.tcx.get_attrs_unchecked(did), name); + if has_attr(did.did(), sym.must_root) { + ret = true; + false + } else if has_attr(did.did(), sym.allow_unrooted_interior) { + false + } else if match_def_path(cx, did.did(), &[sym.alloc, sym.rc, sym.Rc]) { + // Rc<Promise> is okay + let inner = substs.type_at(0); + if let ty::Adt(did, _) = inner.kind() { + if has_attr(did.did(), sym.allow_unrooted_in_rc) { + false + } else { + true + } + } else { + true + } + } else if match_def_path(cx, did.did(), &[sym::core, sym.cell, sym.Ref]) || + match_def_path(cx, did.did(), &[sym::core, sym.cell, sym.RefMut]) || + match_def_path(cx, did.did(), &[sym::core, sym::slice, sym::iter, sym.Iter]) || + match_def_path( + cx, + did.did(), + &[sym::core, sym::slice, sym::iter, sym.IterMut], + ) || + match_def_path(cx, did.did(), &[sym.accountable_refcell, sym.Ref]) || + match_def_path(cx, did.did(), &[sym.accountable_refcell, sym.RefMut]) || + match_def_path( + cx, + did.did(), + &[sym::std, sym.collections, sym.hash, sym.map, sym.Entry], + ) || + match_def_path( + cx, + did.did(), + &[ + sym::std, + sym.collections, + sym.hash, + sym.map, + sym.OccupiedEntry, + ], + ) || + match_def_path( + cx, + did.did(), + &[ + sym::std, + sym.collections, + sym.hash, + sym.map, + sym.VacantEntry, + ], + ) || + match_def_path( + cx, + did.did(), + &[sym::std, sym.collections, sym.hash, sym.map, sym.Iter], + ) || + match_def_path( + cx, + did.did(), + &[sym::std, sym.collections, sym.hash, sym.set, sym.Iter], + ) + { + // Structures which are semantically similar to an &ptr. + false + } else if did.is_box() && in_new_function { + // box in new() is okay + false + } else { + true + } + }, + ty::Ref(..) => false, // don't recurse down &ptrs + ty::RawPtr(..) => false, // don't recurse down *ptrs + ty::FnDef(..) | ty::FnPtr(_) => false, + + _ => true, + }; + if !recur_into_subtree { + walker.skip_current_subtree(); + } + } + ret +} + +impl LintPass for UnrootedPass { + fn name(&self) -> &'static str { + "ServoUnrootedPass" + } +} + +impl<'tcx> LateLintPass<'tcx> for UnrootedPass { + /// All structs containing #[crown::unrooted_must_root_lint::must_root] types + /// must be #[crown::unrooted_must_root_lint::must_root] themselves + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item) { + let attrs = cx.tcx.hir().attrs(item.hir_id()); + if has_lint_attr(&self.symbols, &attrs, self.symbols.must_root) { + return; + } + if let hir::ItemKind::Struct(def, ..) = &item.kind { + for ref field in def.fields() { + let field_type = cx.tcx.type_of(field.def_id); + if is_unrooted_ty(&self.symbols, cx, field_type.0, false) { + cx.lint( + UNROOTED_MUST_ROOT, + "Type must be rooted, use #[crown::unrooted_must_root_lint::must_root] \ + on the struct definition to propagate", + |lint| lint.set_span(field.span), + ) + } + } + } + } + + /// All enums containing #[crown::unrooted_must_root_lint::must_root] types + /// must be #[crown::unrooted_must_root_lint::must_root] themselves + fn check_variant(&mut self, cx: &LateContext, var: &hir::Variant) { + let ref map = cx.tcx.hir(); + let parent_item = map.expect_item(map.get_parent_item(var.hir_id).def_id); + let attrs = cx.tcx.hir().attrs(parent_item.hir_id()); + if !has_lint_attr(&self.symbols, &attrs, self.symbols.must_root) { + match var.data { + hir::VariantData::Tuple(fields, ..) => { + for field in fields { + let field_type = cx.tcx.type_of(field.def_id); + if is_unrooted_ty(&self.symbols, cx, field_type.0, false) { + cx.lint( + UNROOTED_MUST_ROOT, + "Type must be rooted, \ + use #[crown::unrooted_must_root_lint::must_root] \ + on the enum definition to propagate", + |lint| lint.set_span(field.ty.span), + ) + } + } + }, + _ => (), // Struct variants already caught by check_struct_def + } + } + } + /// Function arguments that are #[crown::unrooted_must_root_lint::must_root] types are not allowed + fn check_fn( + &mut self, + cx: &LateContext<'tcx>, + kind: visit::FnKind<'tcx>, + decl: &'tcx hir::FnDecl, + body: &'tcx hir::Body, + span: source_map::Span, + def_id: LocalDefId, + ) { + let in_new_function = match kind { + visit::FnKind::ItemFn(n, _, _) | visit::FnKind::Method(n, _) => { + &*n.as_str() == "new" || n.as_str().starts_with("new_") + }, + visit::FnKind::Closure => return, + }; + + if !in_derive_expn(span) { + let sig = cx.tcx.type_of(def_id).0.fn_sig(cx.tcx); + + for (arg, ty) in decl.inputs.iter().zip(sig.inputs().skip_binder().iter()) { + if is_unrooted_ty(&self.symbols, cx, *ty, false) { + cx.lint(UNROOTED_MUST_ROOT, "Type must be rooted", |lint| { + lint.set_span(arg.span) + }) + } + } + + if !in_new_function && + is_unrooted_ty(&self.symbols, cx, sig.output().skip_binder(), false) + { + cx.lint(UNROOTED_MUST_ROOT, "Type must be rooted", |lint| { + lint.set_span(decl.output.span()) + }) + } + } + + let mut visitor = FnDefVisitor { + symbols: &self.symbols, + cx, + in_new_function, + }; + visit::walk_expr(&mut visitor, &body.value); + } +} + +struct FnDefVisitor<'a, 'tcx: 'a> { + symbols: &'a Symbols, + cx: &'a LateContext<'tcx>, + in_new_function: bool, +} + +impl<'a, 'tcx> visit::Visitor<'tcx> for FnDefVisitor<'a, 'tcx> { + type Map = rustc_middle::hir::map::Map<'tcx>; + + fn visit_expr(&mut self, expr: &'tcx hir::Expr) { + let cx = self.cx; + + let require_rooted = |cx: &LateContext, in_new_function: bool, subexpr: &hir::Expr| { + let ty = cx.typeck_results().expr_ty(&subexpr); + if is_unrooted_ty(&self.symbols, cx, ty, in_new_function) { + cx.lint( + UNROOTED_MUST_ROOT, + format!("Expression of type {:?} must be rooted", ty), + |lint| lint.set_span(subexpr.span), + ) + } + }; + + match expr.kind { + // Trait casts from #[crown::unrooted_must_root_lint::must_root] types are not allowed + ExprKind::Cast(subexpr, _) => require_rooted(cx, self.in_new_function, &subexpr), + // This catches assignments... the main point of this would be to catch mutable + // references to `JS<T>`. + // FIXME: Enable this? Triggers on certain kinds of uses of DomRefCell. + // hir::ExprAssign(_, ref rhs) => require_rooted(cx, self.in_new_function, &*rhs), + // This catches calls; basically, this enforces the constraint that only constructors + // can call other constructors. + // FIXME: Enable this? Currently triggers with constructs involving DomRefCell, and + // constructs like Vec<JS<T>> and RootedVec<JS<T>>. + // hir::ExprCall(..) if !self.in_new_function => { + // require_rooted(cx, self.in_new_function, expr); + // } + _ => { + // TODO(pcwalton): Check generics with a whitelist of allowed generics. + }, + } + + visit::walk_expr(self, expr); + } + + fn visit_pat(&mut self, pat: &'tcx hir::Pat) { + let cx = self.cx; + + // We want to detect pattern bindings that move a value onto the stack. + // When "default binding modes" https://github.com/rust-lang/rust/issues/42640 + // are implemented, the `Unannotated` case could cause false-positives. + // These should be fixable by adding an explicit `ref`. + match pat.kind { + hir::PatKind::Binding(hir::BindingAnnotation::NONE, ..) | + hir::PatKind::Binding(hir::BindingAnnotation::MUT, ..) => { + let ty = cx.typeck_results().pat_ty(pat); + if is_unrooted_ty(self.symbols, cx, ty, self.in_new_function) { + cx.lint( + UNROOTED_MUST_ROOT, + format!("Expression of type {:?} must be rooted", ty), + |lint| lint.set_span(pat.span), + ) + } + }, + _ => {}, + } + + visit::walk_pat(self, pat); + } + + fn visit_ty(&mut self, _: &'tcx hir::Ty) {} + + fn nested_visit_map(&mut self) -> Self::Map { + self.cx.tcx.hir() + } +} + +symbols! { + crown + unrooted_must_root_lint + allow_unrooted_interior + allow_unrooted_in_rc + must_root + alloc + rc + Rc + cell + accountable_refcell + Ref + RefMut + Iter + IterMut + collections + hash + map + set + Entry + OccupiedEntry + VacantEntry +} diff --git a/support/crown/tests/compile-fail/trace_in_no_trace_composable_works.rs b/support/crown/tests/compile-fail/trace_in_no_trace_composable_works.rs new file mode 100644 index 00000000000..8b8ed813052 --- /dev/null +++ b/support/crown/tests/compile-fail/trace_in_no_trace_composable_works.rs @@ -0,0 +1,25 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +// compile-flags: --error-format=human +/// Mock `JSTraceable` +pub trait JSTraceable {} + +struct TraceableStruct; +impl JSTraceable for TraceableStruct {} + +struct NotTraceableStruct; + +// `must_not_have_traceable(1)` indicates that the second generic argument should +// not be traceable and this test verifies that this lint passes enforces this. +#[crown::trace_in_no_trace_lint::must_not_have_traceable(1)] +struct NoTraceComposable<Traceable, NoTraceable> { + t: Traceable, + n: NoTraceable, +} + +// The lint should fail because TraceableStruct is traceable +struct Foo(NoTraceComposable<TraceableStruct, TraceableStruct>); +//~^ ERROR: must_not_have_traceable marked wrapper must not have jsmanaged inside on 1-th position. Consider removing the wrapper. + +fn main() {} diff --git a/support/crown/tests/compile-fail/trace_in_no_trace_works.rs b/support/crown/tests/compile-fail/trace_in_no_trace_works.rs new file mode 100644 index 00000000000..e45964e56f1 --- /dev/null +++ b/support/crown/tests/compile-fail/trace_in_no_trace_works.rs @@ -0,0 +1,19 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +/// Mock `JSTraceable` +pub trait JSTraceable {} + +struct TraceableStruct; +impl JSTraceable for TraceableStruct {} + +struct NotTraceableStruct; + +#[crown::trace_in_no_trace_lint::must_not_have_traceable] +struct NoTrace<T>(T); + +struct Foo(NoTrace<TraceableStruct>); +//~^ ERROR: must_not_have_traceable marked wrapper must not have jsmanaged inside on 0-th position. Consider removing the wrapper. + +fn main() {} diff --git a/support/crown/tests/compile-fail/unrooted_must_root_parameter.rs b/support/crown/tests/compile-fail/unrooted_must_root_parameter.rs new file mode 100644 index 00000000000..626832cd567 --- /dev/null +++ b/support/crown/tests/compile-fail/unrooted_must_root_parameter.rs @@ -0,0 +1,10 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#[crown::unrooted_must_root_lint::must_root] +struct Foo(i32); + +fn foo1(_: Foo) {} //~ ERROR: Type must be rooted + +fn main() {} diff --git a/support/crown/tests/compile-fail/unrooted_must_root_return_type.rs b/support/crown/tests/compile-fail/unrooted_must_root_return_type.rs new file mode 100644 index 00000000000..768e56b258c --- /dev/null +++ b/support/crown/tests/compile-fail/unrooted_must_root_return_type.rs @@ -0,0 +1,13 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#[crown::unrooted_must_root_lint::must_root] +struct Foo(i32); + +fn foo2() -> Foo { + //~^ ERROR: Type must be rooted + unimplemented!() +} + +fn main() {} diff --git a/support/crown/tests/compile-fail/unrooted_must_root_struct_field.rs b/support/crown/tests/compile-fail/unrooted_must_root_struct_field.rs new file mode 100644 index 00000000000..5c132577c32 --- /dev/null +++ b/support/crown/tests/compile-fail/unrooted_must_root_struct_field.rs @@ -0,0 +1,10 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#[crown::unrooted_must_root_lint::must_root] +struct Foo(i32); +struct Bar(Foo); +//~^ ERROR: Type must be rooted, use #[crown::unrooted_must_root_lint::must_root] on the struct definition to propagate + +fn main() {} diff --git a/support/crown/tests/compile_test.rs b/support/crown/tests/compile_test.rs new file mode 100644 index 00000000000..604fcf1d876 --- /dev/null +++ b/support/crown/tests/compile_test.rs @@ -0,0 +1,52 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +use std::env; +use std::path::PathBuf; + +use compiletest_rs as compiletest; +use once_cell::sync::Lazy; + +static PROFILE_PATH: Lazy<PathBuf> = Lazy::new(|| { + let current_exe_path = env::current_exe().unwrap(); + let deps_path = current_exe_path.parent().unwrap(); + let profile_path = deps_path.parent().unwrap(); + profile_path.into() +}); + +fn run_mode(mode: &'static str, bless: bool) { + let mut config = compiletest::Config { + bless, + edition: Some("2021".into()), + ..Default::default() + } + .tempdir(); + + let cfg_mode = mode.parse().expect("Invalid mode"); + config.mode = cfg_mode; + config.src_base = PathBuf::from("tests").join(mode); + config.rustc_path = PROFILE_PATH.join("crown"); + config.target_rustcflags = Some(format!( + "-L {} -L {} -Zcrate-attr=feature(register_tool) -Zcrate-attr=register_tool(crown)", + PROFILE_PATH.display(), + PROFILE_PATH.join("deps").display() + )); + // Does not work reliably: https://github.com/servo/servo/pull/30508#issuecomment-1834542203 + //config.link_deps(); + config.clean_rmeta(); + config.strict_headers = true; + + compiletest::run_tests(&config); +} + +#[test] +fn compile_test() { + let bless = env::var("BLESS").map_or(false, |x| !x.trim().is_empty()); + run_mode("compile-fail", bless); + run_mode("run-pass", bless); + // UI test fails on windows + if !cfg!(windows) { + run_mode("ui", bless); + } +} diff --git a/support/crown/tests/run-pass/trace_in_no_trace_composable.rs b/support/crown/tests/run-pass/trace_in_no_trace_composable.rs new file mode 100644 index 00000000000..1d642ad85e1 --- /dev/null +++ b/support/crown/tests/run-pass/trace_in_no_trace_composable.rs @@ -0,0 +1,24 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +/// Mock `JSTraceable` +pub trait JSTraceable {} + +struct TraceableStruct; +impl JSTraceable for TraceableStruct {} + +struct NotTraceableStruct; + +// `must_not_have_traceable(1)` indicates that the second generic argument should +// not be traceable and this test verifies that this lint passes. +#[crown::trace_in_no_trace_lint::must_not_have_traceable(1)] +struct NoTraceComposable<Traceable, NoTraceable> { + t: Traceable, + n: NoTraceable, +} + +// this is ok: TraceableStruct is traceable +struct Foo(NoTraceComposable<TraceableStruct, NotTraceableStruct>); + +fn main() {} diff --git a/support/crown/tests/run-pass/trace_in_no_trace_ok.rs b/support/crown/tests/run-pass/trace_in_no_trace_ok.rs new file mode 100644 index 00000000000..6c573c2f105 --- /dev/null +++ b/support/crown/tests/run-pass/trace_in_no_trace_ok.rs @@ -0,0 +1,18 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +/// Mock `JSTraceable` +pub trait JSTraceable {} + +struct TraceableStruct; +impl JSTraceable for TraceableStruct {} + +struct NotTraceableStruct; + +#[crown::trace_in_no_trace_lint::must_not_have_traceable] +struct NoTrace<T>(T); + +struct Foo(NoTrace<NotTraceableStruct>); + +fn main() {} diff --git a/support/crown/tests/run-pass/unrooted_must_root_ok.rs b/support/crown/tests/run-pass/unrooted_must_root_ok.rs new file mode 100644 index 00000000000..e556143d8a2 --- /dev/null +++ b/support/crown/tests/run-pass/unrooted_must_root_ok.rs @@ -0,0 +1,15 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +// compile-flags: --error-format=human +#[crown::unrooted_must_root_lint::must_root] +struct Foo(i32); +#[crown::unrooted_must_root_lint::must_root] +struct Bar(Foo); + +fn foo1(_: &Foo) {} +fn foo2(_: &()) -> &Foo { + unimplemented!() +} + +fn main() {} diff --git a/support/crown/tests/ui/trace_in_no_trace_primitive.rs b/support/crown/tests/ui/trace_in_no_trace_primitive.rs new file mode 100644 index 00000000000..d1f765a3afd --- /dev/null +++ b/support/crown/tests/ui/trace_in_no_trace_primitive.rs @@ -0,0 +1,17 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +/// Mock `JSTraceable` +pub trait JSTraceable {} +impl JSTraceable for i32 {} + +// second generic argument must not be traceable +#[crown::trace_in_no_trace_lint::must_not_have_traceable(0)] +struct NoTrace<NoTraceable> { + n: NoTraceable, +} + +struct Foo(NoTrace<i32>); + +fn main() {} diff --git a/support/crown/tests/ui/trace_in_no_trace_primitive.stderr b/support/crown/tests/ui/trace_in_no_trace_primitive.stderr new file mode 100644 index 00000000000..b3e8178289a --- /dev/null +++ b/support/crown/tests/ui/trace_in_no_trace_primitive.stderr @@ -0,0 +1,10 @@ +warning: must_not_have_traceable marked wrapper is not needed for types that implements empty Traceable (like primitive types). Consider removing the wrapper. + --> $DIR/trace_in_no_trace_primitive.rs:15:12 + | +15 | struct Foo(NoTrace<i32>); + | ^^^^^^^^^^^^ + | + = note: `#[warn(crown::empty_trace_in_no_trace)]` on by default + +warning: 1 warning emitted + |