diff --git a/Cargo.lock b/Cargo.lock index 9e19293..0cb1191 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -82,6 +82,12 @@ version = "0.7.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7c02d123df017efcdfbd739ef81735b36c5ba83ec3c59c80a9d7ecc718f92e50" +[[package]] +name = "autocfg" +version = "1.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ace50bade8e6234aa140d9a2f552bbee1db4d353f69b8217bc503490fc1a9f26" + [[package]] name = "bitflags" version = "2.9.0" @@ -204,6 +210,12 @@ dependencies = [ "libc", ] +[[package]] +name = "crossbeam-utils" +version = "0.8.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d0a5c400df2834b80a4c3327b3aad3a4c4cd4de0629063962b03235697506a28" + [[package]] name = "crypto-common" version = "0.1.6" @@ -214,11 +226,26 @@ dependencies = [ "typenum", ] +[[package]] +name = "dashmap" +version = "6.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5041cc499144891f3790297212f32a74fb938e5136a14943f338ef9e0ae276cf" +dependencies = [ + "cfg-if", + "crossbeam-utils", + "hashbrown 0.14.5", + "lock_api", + "once_cell", + "parking_lot_core", +] + [[package]] name = "dashmap-ref" version = "0.1.0" dependencies = [ "clippy_utils", + "dashmap", "dylint_linting", "dylint_testing", "hakari", @@ -525,6 +552,12 @@ dependencies = [ "syn", ] +[[package]] +name = "hashbrown" +version = "0.14.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1" + [[package]] name = "hashbrown" version = "0.15.2" @@ -704,7 +737,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8c9c992b02b5b4c94ea26e32fe5bccb7aa7d9f390ab5c1221ff895bc7ea8b652" dependencies = [ "equivalent", - "hashbrown", + "hashbrown 0.15.2", ] [[package]] @@ -812,6 +845,16 @@ version = "0.7.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "23fb14cb19457329c82206317a5663005a4d404783dc74f4252769b0d5f42856" +[[package]] +name = "lock_api" +version = "0.4.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "07af8b9cdd281b7915f413fa73f29ebd5d55d0d3f0155584dade1ff18cea1b17" +dependencies = [ + "autocfg", + "scopeguard", +] + [[package]] name = "log" version = "0.4.26" @@ -873,6 +916,19 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" +[[package]] +name = "parking_lot_core" +version = "0.9.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e401f977ab385c9e4e3ab30627d6f26d00e2c73eef317493c4ec6d468726cf8" +dependencies = [ + "cfg-if", + "libc", + "redox_syscall", + "smallvec", + "windows-targets 0.52.6", +] + [[package]] name = "paste" version = "1.0.15" @@ -1055,6 +1111,12 @@ dependencies = [ "winapi-util", ] +[[package]] +name = "scopeguard" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" + [[package]] name = "semver" version = "1.0.26" diff --git a/Cargo.toml b/Cargo.toml index 4aa0a1b..9c3ce8b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,6 +6,7 @@ resolver = "2" [workspace.package] version = "0.1.0" +edition = "2021" [workspace.metadata.crane] name = "dylints-various" diff --git a/flake.lock b/flake.lock index be43e8f..d3f6d4f 100644 --- a/flake.lock +++ b/flake.lock @@ -31,11 +31,11 @@ ] }, "locked": { - "lastModified": 1741084643, - "narHash": "sha256-pohp+9ekX2rBuoew8MNoqkZKaez6q4n+8j/K6SSfTrE=", + "lastModified": 1741089216, + "narHash": "sha256-5ZBaJ0XvjakoZwpHPj6oGzwCcCrcREQlNuDklQfZAxQ=", "owner": "TheNeikos", "repo": "nix-dylint", - "rev": "c88fde3de47277b3d0c1f16f740ff8ceb079e161", + "rev": "7ca821aada8b6fd82a5c422cd1eb821eb208f8ae", "type": "github" }, "original": { diff --git a/flake.nix b/flake.nix index 5822811..a9dfc68 100644 --- a/flake.nix +++ b/flake.nix @@ -109,6 +109,9 @@ pkgs.openssl pkgs.pkg-config ]; + + DYLINT_DRIVER_PATH = dylint.passthru.DYLINT_DRIVER_PATH; + RUSTUP_TOOLCHAIN = toolchain; }; } ); diff --git a/lints/dashmap-ref/Cargo.toml b/lints/dashmap-ref/Cargo.toml index 70c7ba3..097ce72 100644 --- a/lints/dashmap-ref/Cargo.toml +++ b/lints/dashmap-ref/Cargo.toml @@ -7,12 +7,17 @@ publish = false [lib] crate-type = ["cdylib"] +[[example]] +name = "ui" +path = "ui/main.rs" + [dependencies] clippy_utils = { git = "https://github.com/rust-lang/rust-clippy", rev = "19e305bb57a7595f2a8d81f521c0dd8bf854e739" } dylint_linting = "4.0.0" hakari = { version = "0.1", path = "../../hakari" } [dev-dependencies] +dashmap = "6" dylint_testing = "4.0.0" [package.metadata.rust-analyzer] diff --git a/lints/dashmap-ref/src/lib.rs b/lints/dashmap-ref/src/lib.rs index fce89f0..eafcde4 100644 --- a/lints/dashmap-ref/src/lib.rs +++ b/lints/dashmap-ref/src/lib.rs @@ -1,31 +1,29 @@ #![feature(rustc_private)] #![feature(let_chains)] -#![warn(unused_extern_crates)] -extern crate rustc_arena; -extern crate rustc_ast; -extern crate rustc_ast_pretty; -extern crate rustc_data_structures; -extern crate rustc_errors; extern crate rustc_hir; -extern crate rustc_hir_pretty; -extern crate rustc_index; -extern crate rustc_infer; -extern crate rustc_lexer; extern crate rustc_middle; -extern crate rustc_mir_dataflow; -extern crate rustc_parse; extern crate rustc_span; -extern crate rustc_target; -extern crate rustc_trait_selection; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::match_any_def_paths; +use rustc_hir as hir; +use rustc_hir::def_id::DefId; +use rustc_lint::LateContext; use rustc_lint::LateLintPass; +use rustc_middle::mir::CoroutineLayout; +use rustc_middle::ty::Adt; +use rustc_middle::ty::Ty; +use rustc_span::sym; dylint_linting::declare_late_lint! { /// ### What it does + /// Check whether a DashMap reference is held across an await point. /// /// ### Why is this bad? /// + /// This can cause issues as the dashmap may deadlock + /// /// ### Known problems /// /// Remove if none. @@ -43,15 +41,102 @@ dylint_linting::declare_late_lint! { /// ``` pub DASHMAP_REF, Warn, - "description goes here" + "Check whether a dashmap reference is held over an await point" } -impl<'tcx> LateLintPass<'tcx> for DashmapRef { - // A list of things you might check can be found here: - // https://doc.rust-lang.org/stable/nightly-rustc/rustc_lint/trait.LateLintPass.html +const DASHMAP_REF_GUARD: [&str; 4] = ["dashmap", "mapref", "one", "Ref"]; +const DASHMAP_REF_MUT_GUARD: [&str; 4] = ["dashmap", "mapref", "one", "RefMut"]; +const DASHMAP_ENTRY_GUARD: [&str; 4] = ["dashmap", "mapref", "entry", "Entry"]; +const DASHMAP_ITER: [&str; 3] = ["dashmap", "iter", "Iter"]; +const DASHMAP_ITER_MUT: [&str; 3] = ["dashmap", "iter", "IterMut"]; + +impl LateLintPass<'_> for DashmapRef { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ hir::Expr<'_>) { + if let hir::ExprKind::Closure(hir::Closure { + def_id, + kind: + hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared( + hir::CoroutineDesugaring::Async, + _, + )), + .. + }) = expr.kind + { + if let Some(coroutine_layout) = cx.tcx.mir_coroutine_witnesses(*def_id) { + check_interior_types(cx, coroutine_layout); + } + } + } +} + +fn check_interior_types(cx: &LateContext<'_>, coroutine: &CoroutineLayout<'_>) { + for (ty_index, ty_cause) in coroutine.field_tys.iter_enumerated() { + if let Adt(adt, _) = ty_cause.ty.kind() { + let await_points = || { + coroutine + .variant_source_info + .iter_enumerated() + .filter_map(|(variant, source_info)| { + coroutine.variant_fields[variant] + .raw + .contains(&ty_index) + .then_some(source_info.span) + }) + .collect::>() + }; + + if is_dashmap_ref(cx, adt.did(), &ty_cause.ty) { + span_lint_and_then( + cx, + DASHMAP_REF, + ty_cause.source_info.span, + "this dashmap reference is held \ + across an 'await' point. Either drop it before the 'await', \ + or read it again afterwards.", + |diag| { + diag.span_note( + await_points(), + "these are all the await points this ref is held through", + ); + }, + ); + } + } + } +} + +fn is_dashmap_ref(cx: &LateContext<'_>, def_id: DefId, ty: &Ty<'_>) -> bool { + if let Adt(adt, args) = ty.kind() { + if let Some(ty) = cx + .tcx + .is_diagnostic_item(sym::Option, adt.did()) + .then(|| args.type_at(0)) + { + if let Adt(adt, ..) = ty.kind() { + is_dashmap_ref(cx, adt.did(), &ty) + } else { + false + } + } else { + match_any_def_paths( + cx, + def_id, + &[ + &DASHMAP_REF_GUARD, + &DASHMAP_REF_MUT_GUARD, + &DASHMAP_ENTRY_GUARD, + &DASHMAP_ITER, + &DASHMAP_ITER_MUT, + ], + ) + .is_some() + } + } else { + false + } } #[test] fn ui() { - dylint_testing::ui_test(env!("CARGO_PKG_NAME"), "ui"); + dylint_testing::ui_test_example(env!("CARGO_PKG_NAME"), "ui"); } diff --git a/lints/dashmap-ref/ui/main.rs b/lints/dashmap-ref/ui/main.rs index f328e4d..87ff771 100644 --- a/lints/dashmap-ref/ui/main.rs +++ b/lints/dashmap-ref/ui/main.rs @@ -1 +1,50 @@ -fn main() {} +#[expect(unused_must_use)] +fn main() { + held_across(); + held_across_multiple(); + dropped_before(); + all_kinds(); +} + +async fn held_across() { + let map = dashmap::DashMap::::default(); + + let reference = map.get("Hello"); + + std::future::pending::<()>().await; + + println!("{reference:?}"); +} + +async fn held_across_multiple() { + let map = dashmap::DashMap::::default(); + + let _reference = map.get("Hello"); + + std::future::pending::<()>().await; + + std::future::pending::<()>().await; +} + +async fn dropped_before() { + let map = dashmap::DashMap::::default(); + + let _ = map.get("Hello"); + + std::future::pending::<()>().await; +} + +async fn all_kinds() { + let map = dashmap::DashMap::::default(); + + let _ref = map.get("Hello"); + let _ref_mut = map.get_mut("Hello"); + let _entry = map.entry("Hello".to_string()); + let _opt_entry = map.try_entry("Hello".to_string()); + let _iter_entry = map.iter(); + let _iter_mut_entry = map.iter_mut(); + let _direct_ref = map.get("Hello").unwrap(); + let _direct_ref_mut = map.get_mut("Hello").unwrap(); + + std::future::pending::<()>().await; +} diff --git a/lints/dashmap-ref/ui/main.stderr b/lints/dashmap-ref/ui/main.stderr index e69de29..2e3b566 100644 --- a/lints/dashmap-ref/ui/main.stderr +++ b/lints/dashmap-ref/ui/main.stderr @@ -0,0 +1,126 @@ +warning: this dashmap reference is held across an 'await' point. Either drop it before the 'await', or read it again afterwards. + --> $DIR/main.rs:12:9 + | +LL | let reference = map.get("Hello"); + | ^^^^^^^^^ + | +note: these are all the await points this ref is held through + --> $DIR/main.rs:14:34 + | +LL | std::future::pending::<()>().await; + | ^^^^^ + = note: `#[warn(dashmap_ref)]` on by default + +warning: this dashmap reference is held across an 'await' point. Either drop it before the 'await', or read it again afterwards. + --> $DIR/main.rs:22:9 + | +LL | let _reference = map.get("Hello"); + | ^^^^^^^^^^ + | +note: these are all the await points this ref is held through + --> $DIR/main.rs:24:34 + | +LL | std::future::pending::<()>().await; + | ^^^^^ +LL | +LL | std::future::pending::<()>().await; + | ^^^^^ + +warning: this dashmap reference is held across an 'await' point. Either drop it before the 'await', or read it again afterwards. + --> $DIR/main.rs:40:9 + | +LL | let _ref = map.get("Hello"); + | ^^^^ + | +note: these are all the await points this ref is held through + --> $DIR/main.rs:49:34 + | +LL | std::future::pending::<()>().await; + | ^^^^^ + +warning: this dashmap reference is held across an 'await' point. Either drop it before the 'await', or read it again afterwards. + --> $DIR/main.rs:41:9 + | +LL | let _ref_mut = map.get_mut("Hello"); + | ^^^^^^^^ + | +note: these are all the await points this ref is held through + --> $DIR/main.rs:49:34 + | +LL | std::future::pending::<()>().await; + | ^^^^^ + +warning: this dashmap reference is held across an 'await' point. Either drop it before the 'await', or read it again afterwards. + --> $DIR/main.rs:42:9 + | +LL | let _entry = map.entry("Hello".to_string()); + | ^^^^^^ + | +note: these are all the await points this ref is held through + --> $DIR/main.rs:49:34 + | +LL | std::future::pending::<()>().await; + | ^^^^^ + +warning: this dashmap reference is held across an 'await' point. Either drop it before the 'await', or read it again afterwards. + --> $DIR/main.rs:43:9 + | +LL | let _opt_entry = map.try_entry("Hello".to_string()); + | ^^^^^^^^^^ + | +note: these are all the await points this ref is held through + --> $DIR/main.rs:49:34 + | +LL | std::future::pending::<()>().await; + | ^^^^^ + +warning: this dashmap reference is held across an 'await' point. Either drop it before the 'await', or read it again afterwards. + --> $DIR/main.rs:44:9 + | +LL | let _iter_entry = map.iter(); + | ^^^^^^^^^^^ + | +note: these are all the await points this ref is held through + --> $DIR/main.rs:49:34 + | +LL | std::future::pending::<()>().await; + | ^^^^^ + +warning: this dashmap reference is held across an 'await' point. Either drop it before the 'await', or read it again afterwards. + --> $DIR/main.rs:45:9 + | +LL | let _iter_mut_entry = map.iter_mut(); + | ^^^^^^^^^^^^^^^ + | +note: these are all the await points this ref is held through + --> $DIR/main.rs:49:34 + | +LL | std::future::pending::<()>().await; + | ^^^^^ + +warning: this dashmap reference is held across an 'await' point. Either drop it before the 'await', or read it again afterwards. + --> $DIR/main.rs:46:9 + | +LL | let _direct_ref = map.get("Hello").unwrap(); + | ^^^^^^^^^^^ + | +note: these are all the await points this ref is held through + --> $DIR/main.rs:49:34 + | +LL | std::future::pending::<()>().await; + | ^^^^^ + +warning: this dashmap reference is held across an 'await' point. Either drop it before the 'await', or read it again afterwards. + --> $DIR/main.rs:47:9 + | +LL | let _direct_ref_mut = map.get_mut("Hello").unwrap(); + | ^^^^^^^^^^^^^^^ + | +note: these are all the await points this ref is held through + --> $DIR/main.rs:49:34 + | +LL | std::future::pending::<()>().await; + | ^^^^^ + +warning: 10 warnings emitted +