From a9d996a7c5f962b4ac914c86f5005eb22128f511 Mon Sep 17 00:00:00 2001 From: Francesco Zardi Date: Wed, 21 Oct 2020 09:06:05 +0200 Subject: [PATCH 1/4] Add whitelist of safe intrinsics --- crates/hir_def/src/item_tree/lower.rs | 44 +++++++++++++++++++++++++-- crates/hir_def/src/item_tree/tests.rs | 20 ++++++++++++ crates/hir_expand/src/name.rs | 36 ++++++++++++++++++++++ 3 files changed, 98 insertions(+), 2 deletions(-) diff --git a/crates/hir_def/src/item_tree/lower.rs b/crates/hir_def/src/item_tree/lower.rs index 3328639cfe3..1a29081c5b9 100644 --- a/crates/hir_def/src/item_tree/lower.rs +++ b/crates/hir_def/src/item_tree/lower.rs @@ -3,7 +3,7 @@ use std::{collections::hash_map::Entry, mem, sync::Arc}; use arena::map::ArenaMap; -use hir_expand::{ast_id_map::AstIdMap, hygiene::Hygiene, HirFileId}; +use hir_expand::{ast_id_map::AstIdMap, hygiene::Hygiene, name::known, HirFileId}; use smallvec::SmallVec; use syntax::{ ast::{self, ModuleItemOwner}, @@ -42,6 +42,45 @@ pub(super) struct Ctx { forced_visibility: Option, } +/// Returns `true` if the given intrinsic is unsafe to call or not. +pub fn is_intrinsic_fn_unsafe(name: &Name) -> bool { + // Should be kept in sync with https://github.com/rust-lang/rust/blob/c6e4db620a7d2f569f11dcab627430921ea8aacf/compiler/rustc_typeck/src/check/intrinsic.rs#L68 + *name != known::abort + && *name != known::min_align_of + && *name != known::needs_drop + && *name != known::caller_location + && *name != known::size_of_val + && *name != known::min_align_of_val + && *name != known::add_with_overflow + && *name != known::sub_with_overflow + && *name != known::mul_with_overflow + && *name != known::wrapping_add + && *name != known::wrapping_sub + && *name != known::wrapping_mul + && *name != known::saturating_add + && *name != known::saturating_sub + && *name != known::rotate_left + && *name != known::rotate_right + && *name != known::ctpop + && *name != known::ctlz + && *name != known::cttz + && *name != known::bswap + && *name != known::bitreverse + && *name != known::discriminant_value + && *name != known::type_id + && *name != known::likely + && *name != known::unlikely + && *name != known::ptr_guaranteed_eq + && *name != known::ptr_guaranteed_ne + && *name != known::minnumf32 + && *name != known::minnumf64 + && *name != known::maxnumf32 + && *name != known::rustc_peek + && *name != known::maxnumf64 + && *name != known::type_name + && *name != known::variant_count +} + impl Ctx { pub(super) fn new(db: &dyn DefDatabase, hygiene: Hygiene, file: HirFileId) -> Self { Self { @@ -555,7 +594,8 @@ impl Ctx { let id: ModItem = match item { ast::ExternItem::Fn(ast) => { let func = self.lower_function(&ast)?; - self.data().functions[func.index].is_unsafe = true; + self.data().functions[func.index].is_unsafe = + is_intrinsic_fn_unsafe(&self.data().functions[func.index].name); func.into() } ast::ExternItem::Static(ast) => { diff --git a/crates/hir_def/src/item_tree/tests.rs b/crates/hir_def/src/item_tree/tests.rs index 4b354c4c145..495c5a7e9fa 100644 --- a/crates/hir_def/src/item_tree/tests.rs +++ b/crates/hir_def/src/item_tree/tests.rs @@ -437,3 +437,23 @@ fn assoc_item_macros() { "#]], ); } + +#[test] +fn safe_intrinsic() { + check( + r" + #![feature(core_intrinsics)] + + fn reverse(input: u32) -> u32 { + std::intrinsics::bitreverse(input) + } + ", + expect![[r#" + inner attrs: Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("feature"))] }, input: Some(TokenTree(SUBTREE () 0 + IDENT core_intrinsics 1)) }]) } + + top-level items: + Function { name: Name(Text("reverse")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: false, has_body: true, is_unsafe: false, params: [Path(Path { type_anchor: None, mod_path: ModPath { kind: Plain, segments: [Name(Text("u32"))] }, generic_args: [None] })], is_varargs: false, ret_type: Path(Path { type_anchor: None, mod_path: ModPath { kind: Plain, segments: [Name(Text("u32"))] }, generic_args: [None] }), ast_id: FileAstId::(0) } + "#]], + ); +} diff --git a/crates/hir_expand/src/name.rs b/crates/hir_expand/src/name.rs index 63f8287079d..0d5b566815d 100644 --- a/crates/hir_expand/src/name.rs +++ b/crates/hir_expand/src/name.rs @@ -208,6 +208,42 @@ pub mod known { PartialOrd, Eq, PartialEq, + // Safe primitives + abort, + size_of, + min_align_of, + needs_drop, + caller_location, + size_of_val, + min_align_of_val, + add_with_overflow, + sub_with_overflow, + mul_with_overflow, + wrapping_add, + wrapping_sub, + wrapping_mul, + saturating_add, + saturating_sub, + rotate_left, + rotate_right, + ctpop, + ctlz, + cttz, + bswap, + bitreverse, + discriminant_value, + type_id, + likely, + unlikely, + ptr_guaranteed_eq, + ptr_guaranteed_ne, + minnumf32, + minnumf64, + maxnumf32, + rustc_peek, + maxnumf64, + type_name, + variant_count, ); // self/Self cannot be used as an identifier From f3aa44b01df7dd679adfe3afe55283cfed611508 Mon Sep 17 00:00:00 2001 From: Francesco Zardi Date: Wed, 21 Oct 2020 21:51:53 +0200 Subject: [PATCH 2/4] Fix typo in comment --- crates/hir_expand/src/name.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/hir_expand/src/name.rs b/crates/hir_expand/src/name.rs index 0d5b566815d..b26ffa1ef2e 100644 --- a/crates/hir_expand/src/name.rs +++ b/crates/hir_expand/src/name.rs @@ -208,7 +208,7 @@ pub mod known { PartialOrd, Eq, PartialEq, - // Safe primitives + // Safe intrinsics abort, size_of, min_align_of, From 0be21b05d6811936a22d491f2cea4c7fe244ce2f Mon Sep 17 00:00:00 2001 From: Francesco Zardi Date: Wed, 21 Oct 2020 21:53:05 +0200 Subject: [PATCH 3/4] Move safe intrinsic tests --- crates/hir_def/src/item_tree/tests.rs | 20 ------------------- crates/hir_ty/src/diagnostics/unsafe_check.rs | 18 +++++++++++++++++ 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/crates/hir_def/src/item_tree/tests.rs b/crates/hir_def/src/item_tree/tests.rs index 495c5a7e9fa..4b354c4c145 100644 --- a/crates/hir_def/src/item_tree/tests.rs +++ b/crates/hir_def/src/item_tree/tests.rs @@ -437,23 +437,3 @@ fn assoc_item_macros() { "#]], ); } - -#[test] -fn safe_intrinsic() { - check( - r" - #![feature(core_intrinsics)] - - fn reverse(input: u32) -> u32 { - std::intrinsics::bitreverse(input) - } - ", - expect![[r#" - inner attrs: Attrs { entries: Some([Attr { path: ModPath { kind: Plain, segments: [Name(Text("feature"))] }, input: Some(TokenTree(SUBTREE () 0 - IDENT core_intrinsics 1)) }]) } - - top-level items: - Function { name: Name(Text("reverse")), visibility: RawVisibilityId("pub(self)"), generic_params: GenericParamsId(4294967295), has_self_param: false, has_body: true, is_unsafe: false, params: [Path(Path { type_anchor: None, mod_path: ModPath { kind: Plain, segments: [Name(Text("u32"))] }, generic_args: [None] })], is_varargs: false, ret_type: Path(Path { type_anchor: None, mod_path: ModPath { kind: Plain, segments: [Name(Text("u32"))] }, generic_args: [None] }), ast_id: FileAstId::(0) } - "#]], - ); -} diff --git a/crates/hir_ty/src/diagnostics/unsafe_check.rs b/crates/hir_ty/src/diagnostics/unsafe_check.rs index 21a121aad7e..2da9688cab0 100644 --- a/crates/hir_ty/src/diagnostics/unsafe_check.rs +++ b/crates/hir_ty/src/diagnostics/unsafe_check.rs @@ -199,6 +199,24 @@ fn main() { let x = STATIC_MUT.a; } } +"#, + ); + } + + #[test] + fn no_missing_unsafe_diagnostic_with_safe_intrinsic() { + check_diagnostics( + r#" +extern "rust-intrinsic" { + pub fn bitreverse(x: u32) -> u32; // Safe intrinsic + pub fn floorf32(x: f32) -> f32; // Unsafe intrinsic +} + +fn main() { + let _ = bitreverse(12); + let _ = floorf32(12.0); + //^^^^^^^^^^^^^^ This operation is unsafe and requires an unsafe function or block +} "#, ); } From aff04d81ba6a334c1ba20ea4e6e04ffc88221aee Mon Sep 17 00:00:00 2001 From: Francesco Zardi Date: Wed, 21 Oct 2020 21:53:37 +0200 Subject: [PATCH 4/4] Refactor is_intrinsic_fn_unsafe() and make it private --- crates/hir_def/src/item_tree/lower.rs | 81 ++++++++++++++------------- 1 file changed, 42 insertions(+), 39 deletions(-) diff --git a/crates/hir_def/src/item_tree/lower.rs b/crates/hir_def/src/item_tree/lower.rs index 1a29081c5b9..ca7fb4a43db 100644 --- a/crates/hir_def/src/item_tree/lower.rs +++ b/crates/hir_def/src/item_tree/lower.rs @@ -42,45 +42,6 @@ pub(super) struct Ctx { forced_visibility: Option, } -/// Returns `true` if the given intrinsic is unsafe to call or not. -pub fn is_intrinsic_fn_unsafe(name: &Name) -> bool { - // Should be kept in sync with https://github.com/rust-lang/rust/blob/c6e4db620a7d2f569f11dcab627430921ea8aacf/compiler/rustc_typeck/src/check/intrinsic.rs#L68 - *name != known::abort - && *name != known::min_align_of - && *name != known::needs_drop - && *name != known::caller_location - && *name != known::size_of_val - && *name != known::min_align_of_val - && *name != known::add_with_overflow - && *name != known::sub_with_overflow - && *name != known::mul_with_overflow - && *name != known::wrapping_add - && *name != known::wrapping_sub - && *name != known::wrapping_mul - && *name != known::saturating_add - && *name != known::saturating_sub - && *name != known::rotate_left - && *name != known::rotate_right - && *name != known::ctpop - && *name != known::ctlz - && *name != known::cttz - && *name != known::bswap - && *name != known::bitreverse - && *name != known::discriminant_value - && *name != known::type_id - && *name != known::likely - && *name != known::unlikely - && *name != known::ptr_guaranteed_eq - && *name != known::ptr_guaranteed_ne - && *name != known::minnumf32 - && *name != known::minnumf64 - && *name != known::maxnumf32 - && *name != known::rustc_peek - && *name != known::maxnumf64 - && *name != known::type_name - && *name != known::variant_count -} - impl Ctx { pub(super) fn new(db: &dyn DefDatabase, hygiene: Hygiene, file: HirFileId) -> Self { Self { @@ -753,3 +714,45 @@ enum GenericsOwner<'a> { TypeAlias, Impl, } + +/// Returns `true` if the given intrinsic is unsafe to call, or false otherwise. +fn is_intrinsic_fn_unsafe(name: &Name) -> bool { + // Should be kept in sync with https://github.com/rust-lang/rust/blob/c6e4db620a7d2f569f11dcab627430921ea8aacf/compiler/rustc_typeck/src/check/intrinsic.rs#L68 + ![ + known::abort, + known::min_align_of, + known::needs_drop, + known::caller_location, + known::size_of_val, + known::min_align_of_val, + known::add_with_overflow, + known::sub_with_overflow, + known::mul_with_overflow, + known::wrapping_add, + known::wrapping_sub, + known::wrapping_mul, + known::saturating_add, + known::saturating_sub, + known::rotate_left, + known::rotate_right, + known::ctpop, + known::ctlz, + known::cttz, + known::bswap, + known::bitreverse, + known::discriminant_value, + known::type_id, + known::likely, + known::unlikely, + known::ptr_guaranteed_eq, + known::ptr_guaranteed_ne, + known::minnumf32, + known::minnumf64, + known::maxnumf32, + known::rustc_peek, + known::maxnumf64, + known::type_name, + known::variant_count, + ] + .contains(&name) +}