From 470c244c8a8274e9969e873148f13fe23e64a2c4 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 10 Jul 2023 08:25:50 +0000 Subject: Remove the library/term exception in tidy's pal checker code This crate doesn't exist anymore. --- src/tools/tidy/src/pal.rs | 1 - 1 file changed, 1 deletion(-) (limited to 'src/tools') diff --git a/src/tools/tidy/src/pal.rs b/src/tools/tidy/src/pal.rs index 62c070221dc..b386b000bef 100644 --- a/src/tools/tidy/src/pal.rs +++ b/src/tools/tidy/src/pal.rs @@ -39,7 +39,6 @@ const EXCEPTION_PATHS: &[&str] = &[ "library/panic_unwind", "library/unwind", "library/rtstartup", // Not sure what to do about this. magic stuff for mingw - "library/term", // Not sure how to make this crate portable, but test crate needs it. "library/test", // Probably should defer to unstable `std::sys` APIs. // The `VaList` implementation must have platform specific code. // The Windows implementation of a `va_list` is always a character -- cgit 1.4.1-3-g733a5 From c0fbeeab1697d52b47fad4fff73ac3f334b85562 Mon Sep 17 00:00:00 2001 From: Urgau Date: Wed, 17 May 2023 20:01:36 +0200 Subject: Drop uplifted `clippy::fn_null_check` --- .../clippy/clippy_lints/src/declared_lints.rs | 1 - src/tools/clippy/clippy_lints/src/fn_null_check.rs | 102 ------------------- src/tools/clippy/clippy_lints/src/lib.rs | 2 - src/tools/clippy/clippy_lints/src/renamed_lints.rs | 1 + src/tools/clippy/tests/ui/fn_null_check.rs | 22 ---- src/tools/clippy/tests/ui/fn_null_check.stderr | 43 -------- src/tools/clippy/tests/ui/rename.fixed | 2 + src/tools/clippy/tests/ui/rename.rs | 2 + src/tools/clippy/tests/ui/rename.stderr | 112 +++++++++++---------- 9 files changed, 64 insertions(+), 223 deletions(-) delete mode 100644 src/tools/clippy/clippy_lints/src/fn_null_check.rs delete mode 100644 src/tools/clippy/tests/ui/fn_null_check.rs delete mode 100644 src/tools/clippy/tests/ui/fn_null_check.stderr (limited to 'src/tools') diff --git a/src/tools/clippy/clippy_lints/src/declared_lints.rs b/src/tools/clippy/clippy_lints/src/declared_lints.rs index 9d9ee6ba307..b60f0738f64 100644 --- a/src/tools/clippy/clippy_lints/src/declared_lints.rs +++ b/src/tools/clippy/clippy_lints/src/declared_lints.rs @@ -171,7 +171,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::float_literal::LOSSY_FLOAT_LITERAL_INFO, crate::floating_point_arithmetic::IMPRECISE_FLOPS_INFO, crate::floating_point_arithmetic::SUBOPTIMAL_FLOPS_INFO, - crate::fn_null_check::FN_NULL_CHECK_INFO, crate::format::USELESS_FORMAT_INFO, crate::format_args::FORMAT_IN_FORMAT_ARGS_INFO, crate::format_args::TO_STRING_IN_FORMAT_ARGS_INFO, diff --git a/src/tools/clippy/clippy_lints/src/fn_null_check.rs b/src/tools/clippy/clippy_lints/src/fn_null_check.rs deleted file mode 100644 index 521045a9fed..00000000000 --- a/src/tools/clippy/clippy_lints/src/fn_null_check.rs +++ /dev/null @@ -1,102 +0,0 @@ -use clippy_utils::consts::{constant, Constant}; -use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::{is_integer_literal, is_path_diagnostic_item}; -use rustc_hir::{BinOpKind, Expr, ExprKind, TyKind}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::sym; - -declare_clippy_lint! { - /// ### What it does - /// Checks for comparing a function pointer to null. - /// - /// ### Why is this bad? - /// Function pointers are assumed to not be null. - /// - /// ### Example - /// ```rust,ignore - /// let fn_ptr: fn() = /* somehow obtained nullable function pointer */ - /// - /// if (fn_ptr as *const ()).is_null() { ... } - /// ``` - /// Use instead: - /// ```rust,ignore - /// let fn_ptr: Option = /* somehow obtained nullable function pointer */ - /// - /// if fn_ptr.is_none() { ... } - /// ``` - #[clippy::version = "1.68.0"] - pub FN_NULL_CHECK, - correctness, - "`fn()` type assumed to be nullable" -} -declare_lint_pass!(FnNullCheck => [FN_NULL_CHECK]); - -fn lint_expr(cx: &LateContext<'_>, expr: &Expr<'_>) { - span_lint_and_help( - cx, - FN_NULL_CHECK, - expr.span, - "function pointer assumed to be nullable, even though it isn't", - None, - "try wrapping your function pointer type in `Option` instead, and using `is_none` to check for null pointer value", - ); -} - -fn is_fn_ptr_cast(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - if let ExprKind::Cast(cast_expr, cast_ty) = expr.kind - && let TyKind::Ptr(_) = cast_ty.kind - { - cx.typeck_results().expr_ty_adjusted(cast_expr).is_fn() - } else { - false - } -} - -impl<'tcx> LateLintPass<'tcx> for FnNullCheck { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - match expr.kind { - // Catching: - // (fn_ptr as * ).is_null() - ExprKind::MethodCall(method_name, receiver, _, _) - if method_name.ident.as_str() == "is_null" && is_fn_ptr_cast(cx, receiver) => - { - lint_expr(cx, expr); - }, - - ExprKind::Binary(op, left, right) if matches!(op.node, BinOpKind::Eq) => { - let to_check: &Expr<'_>; - if is_fn_ptr_cast(cx, left) { - to_check = right; - } else if is_fn_ptr_cast(cx, right) { - to_check = left; - } else { - return; - } - - match to_check.kind { - // Catching: - // (fn_ptr as * ) == (0 as ) - ExprKind::Cast(cast_expr, _) if is_integer_literal(cast_expr, 0) => { - lint_expr(cx, expr); - }, - - // Catching: - // (fn_ptr as * ) == std::ptr::null() - ExprKind::Call(func, []) if is_path_diagnostic_item(cx, func, sym::ptr_null) => { - lint_expr(cx, expr); - }, - - // Catching: - // (fn_ptr as * ) == - _ if matches!(constant(cx, cx.typeck_results(), to_check), Some(Constant::RawPtr(0))) => { - lint_expr(cx, expr); - }, - - _ => {}, - } - }, - _ => {}, - } - } -} diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index 87329ee5e14..365cc4c59ad 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/clippy_lints/src/lib.rs @@ -131,7 +131,6 @@ mod extra_unused_type_parameters; mod fallible_impl_from; mod float_literal; mod floating_point_arithmetic; -mod fn_null_check; mod format; mod format_args; mod format_impl; @@ -1003,7 +1002,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: semicolon_outside_block_ignore_multiline, )) }); - store.register_late_pass(|_| Box::new(fn_null_check::FnNullCheck)); store.register_late_pass(|_| Box::new(permissions_set_readonly_false::PermissionsSetReadonlyFalse)); store.register_late_pass(|_| Box::new(size_of_ref::SizeOfRef)); store.register_late_pass(|_| Box::new(multiple_unsafe_ops_per_block::MultipleUnsafeOpsPerBlock)); diff --git a/src/tools/clippy/clippy_lints/src/renamed_lints.rs b/src/tools/clippy/clippy_lints/src/renamed_lints.rs index cbcd11debfd..d24215c2292 100644 --- a/src/tools/clippy/clippy_lints/src/renamed_lints.rs +++ b/src/tools/clippy/clippy_lints/src/renamed_lints.rs @@ -42,6 +42,7 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[ ("clippy::for_loops_over_fallibles", "for_loops_over_fallibles"), ("clippy::forget_copy", "forgetting_copy_types"), ("clippy::forget_ref", "forgetting_references"), + ("clippy::fn_null_check", "incorrect_fn_null_checks"), ("clippy::into_iter_on_array", "array_into_iter"), ("clippy::invalid_atomic_ordering", "invalid_atomic_ordering"), ("clippy::invalid_ref", "invalid_value"), diff --git a/src/tools/clippy/tests/ui/fn_null_check.rs b/src/tools/clippy/tests/ui/fn_null_check.rs deleted file mode 100644 index dfdea100c8f..00000000000 --- a/src/tools/clippy/tests/ui/fn_null_check.rs +++ /dev/null @@ -1,22 +0,0 @@ -#![allow(unused)] -#![warn(clippy::fn_null_check)] -#![allow(clippy::cmp_null)] -#![allow(clippy::needless_if)] -#![allow(clippy::ptr_eq)] -#![allow(clippy::zero_ptr)] - -pub const ZPTR: *const () = 0 as *const _; -pub const NOT_ZPTR: *const () = 1 as *const _; - -fn main() { - let fn_ptr = main; - - if (fn_ptr as *mut ()).is_null() {} - if (fn_ptr as *const u8).is_null() {} - if (fn_ptr as *const ()) == std::ptr::null() {} - if (fn_ptr as *const ()) == (0 as *const ()) {} - if (fn_ptr as *const ()) == ZPTR {} - - // no lint - if (fn_ptr as *const ()) == NOT_ZPTR {} -} diff --git a/src/tools/clippy/tests/ui/fn_null_check.stderr b/src/tools/clippy/tests/ui/fn_null_check.stderr deleted file mode 100644 index 5b9f48a961c..00000000000 --- a/src/tools/clippy/tests/ui/fn_null_check.stderr +++ /dev/null @@ -1,43 +0,0 @@ -error: function pointer assumed to be nullable, even though it isn't - --> $DIR/fn_null_check.rs:14:8 - | -LL | if (fn_ptr as *mut ()).is_null() {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: try wrapping your function pointer type in `Option` instead, and using `is_none` to check for null pointer value - = note: `-D clippy::fn-null-check` implied by `-D warnings` - -error: function pointer assumed to be nullable, even though it isn't - --> $DIR/fn_null_check.rs:15:8 - | -LL | if (fn_ptr as *const u8).is_null() {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: try wrapping your function pointer type in `Option` instead, and using `is_none` to check for null pointer value - -error: function pointer assumed to be nullable, even though it isn't - --> $DIR/fn_null_check.rs:16:8 - | -LL | if (fn_ptr as *const ()) == std::ptr::null() {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: try wrapping your function pointer type in `Option` instead, and using `is_none` to check for null pointer value - -error: function pointer assumed to be nullable, even though it isn't - --> $DIR/fn_null_check.rs:17:8 - | -LL | if (fn_ptr as *const ()) == (0 as *const ()) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: try wrapping your function pointer type in `Option` instead, and using `is_none` to check for null pointer value - -error: function pointer assumed to be nullable, even though it isn't - --> $DIR/fn_null_check.rs:18:8 - | -LL | if (fn_ptr as *const ()) == ZPTR {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = help: try wrapping your function pointer type in `Option` instead, and using `is_none` to check for null pointer value - -error: aborting due to 5 previous errors - diff --git a/src/tools/clippy/tests/ui/rename.fixed b/src/tools/clippy/tests/ui/rename.fixed index b24c83d9a0d..cab02bb93c9 100644 --- a/src/tools/clippy/tests/ui/rename.fixed +++ b/src/tools/clippy/tests/ui/rename.fixed @@ -37,6 +37,7 @@ #![allow(for_loops_over_fallibles)] #![allow(forgetting_copy_types)] #![allow(forgetting_references)] +#![allow(incorrect_fn_null_checks)] #![allow(array_into_iter)] #![allow(invalid_atomic_ordering)] #![allow(invalid_value)] @@ -89,6 +90,7 @@ #![warn(for_loops_over_fallibles)] #![warn(forgetting_copy_types)] #![warn(forgetting_references)] +#![warn(incorrect_fn_null_checks)] #![warn(array_into_iter)] #![warn(invalid_atomic_ordering)] #![warn(invalid_value)] diff --git a/src/tools/clippy/tests/ui/rename.rs b/src/tools/clippy/tests/ui/rename.rs index baa6345a64f..e5e31452149 100644 --- a/src/tools/clippy/tests/ui/rename.rs +++ b/src/tools/clippy/tests/ui/rename.rs @@ -37,6 +37,7 @@ #![allow(for_loops_over_fallibles)] #![allow(forgetting_copy_types)] #![allow(forgetting_references)] +#![allow(incorrect_fn_null_checks)] #![allow(array_into_iter)] #![allow(invalid_atomic_ordering)] #![allow(invalid_value)] @@ -89,6 +90,7 @@ #![warn(clippy::for_loops_over_fallibles)] #![warn(clippy::forget_copy)] #![warn(clippy::forget_ref)] +#![warn(clippy::fn_null_check)] #![warn(clippy::into_iter_on_array)] #![warn(clippy::invalid_atomic_ordering)] #![warn(clippy::invalid_ref)] diff --git a/src/tools/clippy/tests/ui/rename.stderr b/src/tools/clippy/tests/ui/rename.stderr index ae25c3b46bd..783608a0841 100644 --- a/src/tools/clippy/tests/ui/rename.stderr +++ b/src/tools/clippy/tests/ui/rename.stderr @@ -1,5 +1,5 @@ error: lint `clippy::almost_complete_letter_range` has been renamed to `clippy::almost_complete_range` - --> $DIR/rename.rs:52:9 + --> $DIR/rename.rs:53:9 | LL | #![warn(clippy::almost_complete_letter_range)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::almost_complete_range` @@ -7,310 +7,316 @@ LL | #![warn(clippy::almost_complete_letter_range)] = note: `-D renamed-and-removed-lints` implied by `-D warnings` error: lint `clippy::blacklisted_name` has been renamed to `clippy::disallowed_names` - --> $DIR/rename.rs:53:9 + --> $DIR/rename.rs:54:9 | LL | #![warn(clippy::blacklisted_name)] | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::disallowed_names` error: lint `clippy::block_in_if_condition_expr` has been renamed to `clippy::blocks_in_if_conditions` - --> $DIR/rename.rs:54:9 + --> $DIR/rename.rs:55:9 | LL | #![warn(clippy::block_in_if_condition_expr)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::blocks_in_if_conditions` error: lint `clippy::block_in_if_condition_stmt` has been renamed to `clippy::blocks_in_if_conditions` - --> $DIR/rename.rs:55:9 + --> $DIR/rename.rs:56:9 | LL | #![warn(clippy::block_in_if_condition_stmt)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::blocks_in_if_conditions` error: lint `clippy::box_vec` has been renamed to `clippy::box_collection` - --> $DIR/rename.rs:56:9 + --> $DIR/rename.rs:57:9 | LL | #![warn(clippy::box_vec)] | ^^^^^^^^^^^^^^^ help: use the new name: `clippy::box_collection` error: lint `clippy::const_static_lifetime` has been renamed to `clippy::redundant_static_lifetimes` - --> $DIR/rename.rs:57:9 + --> $DIR/rename.rs:58:9 | LL | #![warn(clippy::const_static_lifetime)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::redundant_static_lifetimes` error: lint `clippy::cyclomatic_complexity` has been renamed to `clippy::cognitive_complexity` - --> $DIR/rename.rs:58:9 + --> $DIR/rename.rs:59:9 | LL | #![warn(clippy::cyclomatic_complexity)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::cognitive_complexity` error: lint `clippy::derive_hash_xor_eq` has been renamed to `clippy::derived_hash_with_manual_eq` - --> $DIR/rename.rs:59:9 + --> $DIR/rename.rs:60:9 | LL | #![warn(clippy::derive_hash_xor_eq)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::derived_hash_with_manual_eq` error: lint `clippy::disallowed_method` has been renamed to `clippy::disallowed_methods` - --> $DIR/rename.rs:60:9 + --> $DIR/rename.rs:61:9 | LL | #![warn(clippy::disallowed_method)] | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::disallowed_methods` error: lint `clippy::disallowed_type` has been renamed to `clippy::disallowed_types` - --> $DIR/rename.rs:61:9 + --> $DIR/rename.rs:62:9 | LL | #![warn(clippy::disallowed_type)] | ^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::disallowed_types` error: lint `clippy::eval_order_dependence` has been renamed to `clippy::mixed_read_write_in_expression` - --> $DIR/rename.rs:62:9 + --> $DIR/rename.rs:63:9 | LL | #![warn(clippy::eval_order_dependence)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::mixed_read_write_in_expression` error: lint `clippy::identity_conversion` has been renamed to `clippy::useless_conversion` - --> $DIR/rename.rs:63:9 + --> $DIR/rename.rs:64:9 | LL | #![warn(clippy::identity_conversion)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::useless_conversion` error: lint `clippy::if_let_some_result` has been renamed to `clippy::match_result_ok` - --> $DIR/rename.rs:64:9 + --> $DIR/rename.rs:65:9 | LL | #![warn(clippy::if_let_some_result)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::match_result_ok` error: lint `clippy::integer_arithmetic` has been renamed to `clippy::arithmetic_side_effects` - --> $DIR/rename.rs:65:9 + --> $DIR/rename.rs:66:9 | LL | #![warn(clippy::integer_arithmetic)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::arithmetic_side_effects` error: lint `clippy::logic_bug` has been renamed to `clippy::overly_complex_bool_expr` - --> $DIR/rename.rs:66:9 + --> $DIR/rename.rs:67:9 | LL | #![warn(clippy::logic_bug)] | ^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::overly_complex_bool_expr` error: lint `clippy::new_without_default_derive` has been renamed to `clippy::new_without_default` - --> $DIR/rename.rs:67:9 + --> $DIR/rename.rs:68:9 | LL | #![warn(clippy::new_without_default_derive)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::new_without_default` error: lint `clippy::option_and_then_some` has been renamed to `clippy::bind_instead_of_map` - --> $DIR/rename.rs:68:9 + --> $DIR/rename.rs:69:9 | LL | #![warn(clippy::option_and_then_some)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::bind_instead_of_map` error: lint `clippy::option_expect_used` has been renamed to `clippy::expect_used` - --> $DIR/rename.rs:69:9 + --> $DIR/rename.rs:70:9 | LL | #![warn(clippy::option_expect_used)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::expect_used` error: lint `clippy::option_map_unwrap_or` has been renamed to `clippy::map_unwrap_or` - --> $DIR/rename.rs:70:9 + --> $DIR/rename.rs:71:9 | LL | #![warn(clippy::option_map_unwrap_or)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::map_unwrap_or` error: lint `clippy::option_map_unwrap_or_else` has been renamed to `clippy::map_unwrap_or` - --> $DIR/rename.rs:71:9 + --> $DIR/rename.rs:72:9 | LL | #![warn(clippy::option_map_unwrap_or_else)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::map_unwrap_or` error: lint `clippy::option_unwrap_used` has been renamed to `clippy::unwrap_used` - --> $DIR/rename.rs:72:9 + --> $DIR/rename.rs:73:9 | LL | #![warn(clippy::option_unwrap_used)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::unwrap_used` error: lint `clippy::ref_in_deref` has been renamed to `clippy::needless_borrow` - --> $DIR/rename.rs:73:9 + --> $DIR/rename.rs:74:9 | LL | #![warn(clippy::ref_in_deref)] | ^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::needless_borrow` error: lint `clippy::result_expect_used` has been renamed to `clippy::expect_used` - --> $DIR/rename.rs:74:9 + --> $DIR/rename.rs:75:9 | LL | #![warn(clippy::result_expect_used)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::expect_used` error: lint `clippy::result_map_unwrap_or_else` has been renamed to `clippy::map_unwrap_or` - --> $DIR/rename.rs:75:9 + --> $DIR/rename.rs:76:9 | LL | #![warn(clippy::result_map_unwrap_or_else)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::map_unwrap_or` error: lint `clippy::result_unwrap_used` has been renamed to `clippy::unwrap_used` - --> $DIR/rename.rs:76:9 + --> $DIR/rename.rs:77:9 | LL | #![warn(clippy::result_unwrap_used)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::unwrap_used` error: lint `clippy::single_char_push_str` has been renamed to `clippy::single_char_add_str` - --> $DIR/rename.rs:77:9 + --> $DIR/rename.rs:78:9 | LL | #![warn(clippy::single_char_push_str)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::single_char_add_str` error: lint `clippy::stutter` has been renamed to `clippy::module_name_repetitions` - --> $DIR/rename.rs:78:9 + --> $DIR/rename.rs:79:9 | LL | #![warn(clippy::stutter)] | ^^^^^^^^^^^^^^^ help: use the new name: `clippy::module_name_repetitions` error: lint `clippy::to_string_in_display` has been renamed to `clippy::recursive_format_impl` - --> $DIR/rename.rs:79:9 + --> $DIR/rename.rs:80:9 | LL | #![warn(clippy::to_string_in_display)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::recursive_format_impl` error: lint `clippy::zero_width_space` has been renamed to `clippy::invisible_characters` - --> $DIR/rename.rs:80:9 + --> $DIR/rename.rs:81:9 | LL | #![warn(clippy::zero_width_space)] | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `clippy::invisible_characters` error: lint `clippy::cast_ref_to_mut` has been renamed to `cast_ref_to_mut` - --> $DIR/rename.rs:81:9 + --> $DIR/rename.rs:82:9 | LL | #![warn(clippy::cast_ref_to_mut)] | ^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `cast_ref_to_mut` error: lint `clippy::clone_double_ref` has been renamed to `suspicious_double_ref_op` - --> $DIR/rename.rs:82:9 + --> $DIR/rename.rs:83:9 | LL | #![warn(clippy::clone_double_ref)] | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `suspicious_double_ref_op` error: lint `clippy::cmp_nan` has been renamed to `invalid_nan_comparisons` - --> $DIR/rename.rs:83:9 + --> $DIR/rename.rs:84:9 | LL | #![warn(clippy::cmp_nan)] | ^^^^^^^^^^^^^^^ help: use the new name: `invalid_nan_comparisons` error: lint `clippy::drop_bounds` has been renamed to `drop_bounds` - --> $DIR/rename.rs:84:9 + --> $DIR/rename.rs:85:9 | LL | #![warn(clippy::drop_bounds)] | ^^^^^^^^^^^^^^^^^^^ help: use the new name: `drop_bounds` error: lint `clippy::drop_copy` has been renamed to `dropping_copy_types` - --> $DIR/rename.rs:85:9 + --> $DIR/rename.rs:86:9 | LL | #![warn(clippy::drop_copy)] | ^^^^^^^^^^^^^^^^^ help: use the new name: `dropping_copy_types` error: lint `clippy::drop_ref` has been renamed to `dropping_references` - --> $DIR/rename.rs:86:9 + --> $DIR/rename.rs:87:9 | LL | #![warn(clippy::drop_ref)] | ^^^^^^^^^^^^^^^^ help: use the new name: `dropping_references` error: lint `clippy::for_loop_over_option` has been renamed to `for_loops_over_fallibles` - --> $DIR/rename.rs:87:9 + --> $DIR/rename.rs:88:9 | LL | #![warn(clippy::for_loop_over_option)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `for_loops_over_fallibles` error: lint `clippy::for_loop_over_result` has been renamed to `for_loops_over_fallibles` - --> $DIR/rename.rs:88:9 + --> $DIR/rename.rs:89:9 | LL | #![warn(clippy::for_loop_over_result)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `for_loops_over_fallibles` error: lint `clippy::for_loops_over_fallibles` has been renamed to `for_loops_over_fallibles` - --> $DIR/rename.rs:89:9 + --> $DIR/rename.rs:90:9 | LL | #![warn(clippy::for_loops_over_fallibles)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `for_loops_over_fallibles` error: lint `clippy::forget_copy` has been renamed to `forgetting_copy_types` - --> $DIR/rename.rs:90:9 + --> $DIR/rename.rs:91:9 | LL | #![warn(clippy::forget_copy)] | ^^^^^^^^^^^^^^^^^^^ help: use the new name: `forgetting_copy_types` error: lint `clippy::forget_ref` has been renamed to `forgetting_references` - --> $DIR/rename.rs:91:9 + --> $DIR/rename.rs:92:9 | LL | #![warn(clippy::forget_ref)] | ^^^^^^^^^^^^^^^^^^ help: use the new name: `forgetting_references` +error: lint `clippy::fn_null_check` has been renamed to `incorrect_fn_null_checks` + --> $DIR/rename.rs:93:9 + | +LL | #![warn(clippy::fn_null_check)] + | ^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `incorrect_fn_null_checks` + error: lint `clippy::into_iter_on_array` has been renamed to `array_into_iter` - --> $DIR/rename.rs:92:9 + --> $DIR/rename.rs:94:9 | LL | #![warn(clippy::into_iter_on_array)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `array_into_iter` error: lint `clippy::invalid_atomic_ordering` has been renamed to `invalid_atomic_ordering` - --> $DIR/rename.rs:93:9 + --> $DIR/rename.rs:95:9 | LL | #![warn(clippy::invalid_atomic_ordering)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `invalid_atomic_ordering` error: lint `clippy::invalid_ref` has been renamed to `invalid_value` - --> $DIR/rename.rs:94:9 + --> $DIR/rename.rs:96:9 | LL | #![warn(clippy::invalid_ref)] | ^^^^^^^^^^^^^^^^^^^ help: use the new name: `invalid_value` error: lint `clippy::invalid_utf8_in_unchecked` has been renamed to `invalid_from_utf8_unchecked` - --> $DIR/rename.rs:95:9 + --> $DIR/rename.rs:97:9 | LL | #![warn(clippy::invalid_utf8_in_unchecked)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `invalid_from_utf8_unchecked` error: lint `clippy::let_underscore_drop` has been renamed to `let_underscore_drop` - --> $DIR/rename.rs:96:9 + --> $DIR/rename.rs:98:9 | LL | #![warn(clippy::let_underscore_drop)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `let_underscore_drop` error: lint `clippy::mem_discriminant_non_enum` has been renamed to `enum_intrinsics_non_enums` - --> $DIR/rename.rs:97:9 + --> $DIR/rename.rs:99:9 | LL | #![warn(clippy::mem_discriminant_non_enum)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `enum_intrinsics_non_enums` error: lint `clippy::panic_params` has been renamed to `non_fmt_panics` - --> $DIR/rename.rs:98:9 + --> $DIR/rename.rs:100:9 | LL | #![warn(clippy::panic_params)] | ^^^^^^^^^^^^^^^^^^^^ help: use the new name: `non_fmt_panics` error: lint `clippy::positional_named_format_parameters` has been renamed to `named_arguments_used_positionally` - --> $DIR/rename.rs:99:9 + --> $DIR/rename.rs:101:9 | LL | #![warn(clippy::positional_named_format_parameters)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `named_arguments_used_positionally` error: lint `clippy::temporary_cstring_as_ptr` has been renamed to `temporary_cstring_as_ptr` - --> $DIR/rename.rs:100:9 + --> $DIR/rename.rs:102:9 | LL | #![warn(clippy::temporary_cstring_as_ptr)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `temporary_cstring_as_ptr` error: lint `clippy::undropped_manually_drops` has been renamed to `undropped_manually_drops` - --> $DIR/rename.rs:101:9 + --> $DIR/rename.rs:103:9 | LL | #![warn(clippy::undropped_manually_drops)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `undropped_manually_drops` error: lint `clippy::unknown_clippy_lints` has been renamed to `unknown_lints` - --> $DIR/rename.rs:102:9 + --> $DIR/rename.rs:104:9 | LL | #![warn(clippy::unknown_clippy_lints)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `unknown_lints` error: lint `clippy::unused_label` has been renamed to `unused_labels` - --> $DIR/rename.rs:103:9 + --> $DIR/rename.rs:105:9 | LL | #![warn(clippy::unused_label)] | ^^^^^^^^^^^^^^^^^^^^ help: use the new name: `unused_labels` -error: aborting due to 52 previous errors +error: aborting due to 53 previous errors -- cgit 1.4.1-3-g733a5 From dd5fa7d9b3176f2d922bf52a34a86847adaaab4c Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Mon, 10 Jul 2023 14:53:57 -0700 Subject: Don't use serde-derive in the rls shim The already-small RLS shim can get a little smaller, and faster to build, if we drop the serde-derive dependency and decode the one "method" field it needs manually from `serde_json::Value`. --- Cargo.lock | 1 - src/tools/rls/Cargo.toml | 1 - src/tools/rls/src/main.rs | 9 +++++---- 3 files changed, 5 insertions(+), 6 deletions(-) (limited to 'src/tools') diff --git a/Cargo.lock b/Cargo.lock index 96eda77abb2..a1bbc1dca76 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3021,7 +3021,6 @@ dependencies = [ name = "rls" version = "2.0.0" dependencies = [ - "serde", "serde_json", ] diff --git a/src/tools/rls/Cargo.toml b/src/tools/rls/Cargo.toml index b84647eb332..b7aa659c25a 100644 --- a/src/tools/rls/Cargo.toml +++ b/src/tools/rls/Cargo.toml @@ -5,5 +5,4 @@ edition = "2021" license = "Apache-2.0/MIT" [dependencies] -serde = { version = "1.0.143", features = ["derive"] } serde_json = "1.0.83" diff --git a/src/tools/rls/src/main.rs b/src/tools/rls/src/main.rs index f96f1325d96..d7be108e89c 100644 --- a/src/tools/rls/src/main.rs +++ b/src/tools/rls/src/main.rs @@ -3,7 +3,7 @@ //! This is a small stub that replaces RLS to alert the user that RLS is no //! longer available. -use serde::Deserialize; +use serde_json::Value; use std::error::Error; use std::io::BufRead; use std::io::Write; @@ -21,7 +21,6 @@ fn main() { } } -#[derive(Deserialize)] struct Message { method: Option, } @@ -88,8 +87,10 @@ fn read_message_raw(reader: &mut R) -> Result fn read_message(reader: &mut R) -> Result> { let m = read_message_raw(reader)?; - match serde_json::from_str(&m) { - Ok(m) => Ok(m), + match serde_json::from_str::(&m) { + Ok(message) => Ok(Message { + method: message.get("method").and_then(|value| value.as_str().map(String::from)), + }), Err(e) => Err(format!("failed to parse message {m}\n{e}").into()), } } -- cgit 1.4.1-3-g733a5 From dd453a6a9961a5e6377add8980cf4c257ab5a395 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 10 Jul 2023 22:07:07 +0200 Subject: miri: protect Move() function arguments during the call --- .../rustc_const_eval/src/const_eval/machine.rs | 12 +- .../rustc_const_eval/src/interpret/eval_context.rs | 8 +- compiler/rustc_const_eval/src/interpret/intern.rs | 2 +- compiler/rustc_const_eval/src/interpret/machine.rs | 30 +++- compiler/rustc_const_eval/src/interpret/mod.rs | 1 + compiler/rustc_const_eval/src/interpret/operand.rs | 8 -- compiler/rustc_const_eval/src/interpret/place.rs | 22 +-- .../rustc_const_eval/src/interpret/terminator.rs | 157 +++++++++++++++++---- compiler/rustc_const_eval/src/interpret/visitor.rs | 4 +- compiler/rustc_mir_transform/src/const_prop.rs | 6 +- .../rustc_mir_transform/src/dataflow_const_prop.rs | 4 +- src/tools/miri/src/borrow_tracker/mod.rs | 6 +- .../borrow_tracker/stacked_borrows/diagnostics.rs | 4 +- .../miri/src/borrow_tracker/stacked_borrows/mod.rs | 32 ++--- .../miri/src/borrow_tracker/tree_borrows/mod.rs | 33 ++--- src/tools/miri/src/lib.rs | 1 + src/tools/miri/src/machine.rs | 53 +++++-- src/tools/miri/src/shims/mod.rs | 8 +- .../fail/both_borrows/return_pointer_aliasing.rs | 32 ----- .../return_pointer_aliasing.stack.stderr | 42 ------ .../return_pointer_aliasing.tree.stderr | 44 ------ .../fail/function_calls/arg_inplace_mutate.rs | 34 +++++ .../function_calls/arg_inplace_mutate.stack.stderr | 37 +++++ .../function_calls/arg_inplace_mutate.tree.stderr | 39 +++++ .../function_calls/arg_inplace_observe_after.rs | 27 ++++ .../arg_inplace_observe_after.stderr | 22 +++ .../arg_inplace_observe_during.none.stderr | 20 +++ .../function_calls/arg_inplace_observe_during.rs | 34 +++++ .../arg_inplace_observe_during.stack.stderr | 37 +++++ .../arg_inplace_observe_during.tree.stderr | 39 +++++ .../return_pointer_aliasing.none.stderr | 20 +++ .../fail/function_calls/return_pointer_aliasing.rs | 34 +++++ .../return_pointer_aliasing.stack.stderr | 37 +++++ .../return_pointer_aliasing.tree.stderr | 39 +++++ .../pass/function_calls/return_place_on_heap.rs | 25 ++++ 35 files changed, 703 insertions(+), 250 deletions(-) delete mode 100644 src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.rs delete mode 100644 src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.stack.stderr delete mode 100644 src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.tree.stderr create mode 100644 src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.rs create mode 100644 src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.stack.stderr create mode 100644 src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.tree.stderr create mode 100644 src/tools/miri/tests/fail/function_calls/arg_inplace_observe_after.rs create mode 100644 src/tools/miri/tests/fail/function_calls/arg_inplace_observe_after.stderr create mode 100644 src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.none.stderr create mode 100644 src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.rs create mode 100644 src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.stack.stderr create mode 100644 src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.tree.stderr create mode 100644 src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.none.stderr create mode 100644 src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.rs create mode 100644 src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.stack.stderr create mode 100644 src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.tree.stderr create mode 100644 src/tools/miri/tests/pass/function_calls/return_place_on_heap.rs (limited to 'src/tools') diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs index f9f645af41f..a20f0276b3e 100644 --- a/compiler/rustc_const_eval/src/const_eval/machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/machine.rs @@ -22,7 +22,7 @@ use rustc_target::spec::abi::Abi as CallAbi; use crate::errors::{LongRunning, LongRunningWarn}; use crate::interpret::{ - self, compile_time_machine, AllocId, ConstAllocation, FnVal, Frame, ImmTy, InterpCx, + self, compile_time_machine, AllocId, ConstAllocation, FnArg, FnVal, Frame, ImmTy, InterpCx, InterpResult, OpTy, PlaceTy, Pointer, Scalar, }; use crate::{errors, fluent_generated as fluent}; @@ -201,7 +201,7 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> { fn hook_special_const_fn( &mut self, instance: ty::Instance<'tcx>, - args: &[OpTy<'tcx>], + args: &[FnArg<'tcx>], dest: &PlaceTy<'tcx>, ret: Option, ) -> InterpResult<'tcx, Option>> { @@ -210,6 +210,7 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> { if Some(def_id) == self.tcx.lang_items().panic_display() || Some(def_id) == self.tcx.lang_items().begin_panic_fn() { + let args = self.copy_fn_args(args)?; // &str or &&str assert!(args.len() == 1); @@ -236,8 +237,9 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> { return Ok(Some(new_instance)); } else if Some(def_id) == self.tcx.lang_items().align_offset_fn() { + let args = self.copy_fn_args(args)?; // For align_offset, we replace the function call if the pointer has no address. - match self.align_offset(instance, args, dest, ret)? { + match self.align_offset(instance, &args, dest, ret)? { ControlFlow::Continue(()) => return Ok(Some(instance)), ControlFlow::Break(()) => return Ok(None), } @@ -293,7 +295,7 @@ impl<'mir, 'tcx: 'mir> CompileTimeEvalContext<'mir, 'tcx> { self.eval_fn_call( FnVal::Instance(instance), (CallAbi::Rust, fn_abi), - &[addr, align], + &[FnArg::Copy(addr), FnArg::Copy(align)], /* with_caller_location = */ false, dest, ret, @@ -427,7 +429,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, ecx: &mut InterpCx<'mir, 'tcx, Self>, instance: ty::Instance<'tcx>, _abi: CallAbi, - args: &[OpTy<'tcx>], + args: &[FnArg<'tcx>], dest: &PlaceTy<'tcx>, ret: Option, _unwind: mir::UnwindAction, // unwinding is not supported in consts diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index 36606ff69a6..5e5ad31d8c2 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -682,11 +682,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { return_to_block: StackPopCleanup, ) -> InterpResult<'tcx> { trace!("body: {:#?}", body); - // Clobber previous return place contents, nobody is supposed to be able to see them any more - // This also checks dereferenceable, but not align. We rely on all constructed places being - // sufficiently aligned (in particular we rely on `deref_operand` checking alignment). - self.write_uninit(return_place)?; - // first push a stack frame so we have access to the local substs + // First push a stack frame so we have access to the local substs let pre_frame = Frame { body, loc: Right(body.span), // Span used for errors caused during preamble. @@ -805,6 +801,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { throw_ub_custom!(fluent::const_eval_unwind_past_top); } + M::before_stack_pop(self, self.frame())?; + // Copy return value. Must of course happen *before* we deallocate the locals. let copy_ret_result = if !unwinding { let op = self diff --git a/compiler/rustc_const_eval/src/interpret/intern.rs b/compiler/rustc_const_eval/src/interpret/intern.rs index 7b11ad33091..107e5bec614 100644 --- a/compiler/rustc_const_eval/src/interpret/intern.rs +++ b/compiler/rustc_const_eval/src/interpret/intern.rs @@ -30,7 +30,7 @@ use super::{ use crate::const_eval; use crate::errors::{DanglingPtrInFinal, UnsupportedUntypedPointer}; -pub trait CompileTimeMachine<'mir, 'tcx, T> = Machine< +pub trait CompileTimeMachine<'mir, 'tcx: 'mir, T> = Machine< 'mir, 'tcx, MemoryKind = T, diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs index b448e3a24c6..e101785b6e2 100644 --- a/compiler/rustc_const_eval/src/interpret/machine.rs +++ b/compiler/rustc_const_eval/src/interpret/machine.rs @@ -17,7 +17,7 @@ use rustc_target::spec::abi::Abi as CallAbi; use crate::const_eval::CheckAlignment; use super::{ - AllocBytes, AllocId, AllocRange, Allocation, ConstAllocation, Frame, ImmTy, InterpCx, + AllocBytes, AllocId, AllocRange, Allocation, ConstAllocation, FnArg, Frame, ImmTy, InterpCx, InterpResult, MemoryKind, OpTy, Operand, PlaceTy, Pointer, Provenance, Scalar, }; @@ -84,7 +84,7 @@ pub trait AllocMap { /// Methods of this trait signifies a point where CTFE evaluation would fail /// and some use case dependent behaviour can instead be applied. -pub trait Machine<'mir, 'tcx>: Sized { +pub trait Machine<'mir, 'tcx: 'mir>: Sized { /// Additional memory kinds a machine wishes to distinguish from the builtin ones type MemoryKind: Debug + std::fmt::Display + MayLeak + Eq + 'static; @@ -182,7 +182,7 @@ pub trait Machine<'mir, 'tcx>: Sized { ecx: &mut InterpCx<'mir, 'tcx, Self>, instance: ty::Instance<'tcx>, abi: CallAbi, - args: &[OpTy<'tcx, Self::Provenance>], + args: &[FnArg<'tcx, Self::Provenance>], destination: &PlaceTy<'tcx, Self::Provenance>, target: Option, unwind: mir::UnwindAction, @@ -194,7 +194,7 @@ pub trait Machine<'mir, 'tcx>: Sized { ecx: &mut InterpCx<'mir, 'tcx, Self>, fn_val: Self::ExtraFnVal, abi: CallAbi, - args: &[OpTy<'tcx, Self::Provenance>], + args: &[FnArg<'tcx, Self::Provenance>], destination: &PlaceTy<'tcx, Self::Provenance>, target: Option, unwind: mir::UnwindAction, @@ -418,6 +418,18 @@ pub trait Machine<'mir, 'tcx>: Sized { Ok(()) } + /// Called on places used for in-place function argument and return value handling. + /// + /// These places need to be protected to make sure the program cannot tell whether the + /// argument/return value was actually copied or passed in-place.. + fn protect_in_place_function_argument( + ecx: &mut InterpCx<'mir, 'tcx, Self>, + place: &PlaceTy<'tcx, Self::Provenance>, + ) -> InterpResult<'tcx> { + // Without an aliasing model, all we can do is put `Uninit` into the place. + ecx.write_uninit(place) + } + /// Called immediately before a new stack frame gets pushed. fn init_frame_extra( ecx: &mut InterpCx<'mir, 'tcx, Self>, @@ -439,6 +451,14 @@ pub trait Machine<'mir, 'tcx>: Sized { Ok(()) } + /// Called just before the return value is copied to the caller-provided return place. + fn before_stack_pop( + _ecx: &InterpCx<'mir, 'tcx, Self>, + _frame: &Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>, + ) -> InterpResult<'tcx> { + Ok(()) + } + /// Called immediately after a stack frame got popped, but before jumping back to the caller. /// The `locals` have already been destroyed! fn after_stack_pop( @@ -484,7 +504,7 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) { _ecx: &mut InterpCx<$mir, $tcx, Self>, fn_val: !, _abi: CallAbi, - _args: &[OpTy<$tcx>], + _args: &[FnArg<$tcx>], _destination: &PlaceTy<$tcx, Self::Provenance>, _target: Option, _unwind: mir::UnwindAction, diff --git a/compiler/rustc_const_eval/src/interpret/mod.rs b/compiler/rustc_const_eval/src/interpret/mod.rs index 898d62361ab..f657f954f9c 100644 --- a/compiler/rustc_const_eval/src/interpret/mod.rs +++ b/compiler/rustc_const_eval/src/interpret/mod.rs @@ -26,6 +26,7 @@ pub use self::machine::{compile_time_machine, AllocMap, Machine, MayLeak, StackP pub use self::memory::{AllocKind, AllocRef, AllocRefMut, FnVal, Memory, MemoryKind}; pub use self::operand::{ImmTy, Immediate, OpTy, Operand}; pub use self::place::{MPlaceTy, MemPlace, MemPlaceMeta, Place, PlaceTy}; +pub use self::terminator::FnArg; pub use self::validity::{CtfeValidationMode, RefTracking}; pub use self::visitor::{MutValueVisitor, Value, ValueVisitor}; diff --git a/compiler/rustc_const_eval/src/interpret/operand.rs b/compiler/rustc_const_eval/src/interpret/operand.rs index 5f89d652fab..6727937363e 100644 --- a/compiler/rustc_const_eval/src/interpret/operand.rs +++ b/compiler/rustc_const_eval/src/interpret/operand.rs @@ -575,14 +575,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(op) } - /// Evaluate a bunch of operands at once - pub(super) fn eval_operands( - &self, - ops: &[mir::Operand<'tcx>], - ) -> InterpResult<'tcx, Vec>> { - ops.iter().map(|op| self.eval_operand(op, None)).collect() - } - fn eval_ty_constant( &self, val: ty::Const<'tcx>, diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs index ca1106384fd..f1c777a077b 100644 --- a/compiler/rustc_const_eval/src/interpret/place.rs +++ b/compiler/rustc_const_eval/src/interpret/place.rs @@ -354,25 +354,27 @@ where #[inline] pub(super) fn get_place_alloc( &self, - place: &MPlaceTy<'tcx, M::Provenance>, + mplace: &MPlaceTy<'tcx, M::Provenance>, ) -> InterpResult<'tcx, Option>> { - assert!(place.layout.is_sized()); - assert!(!place.meta.has_meta()); - let size = place.layout.size; - self.get_ptr_alloc(place.ptr, size, place.align) + let (size, _align) = self + .size_and_align_of_mplace(&mplace)? + .unwrap_or((mplace.layout.size, mplace.layout.align.abi)); + // Due to packed places, only `mplace.align` matters. + self.get_ptr_alloc(mplace.ptr, size, mplace.align) } #[inline] pub(super) fn get_place_alloc_mut( &mut self, - place: &MPlaceTy<'tcx, M::Provenance>, + mplace: &MPlaceTy<'tcx, M::Provenance>, ) -> InterpResult<'tcx, Option>> { - assert!(place.layout.is_sized()); - assert!(!place.meta.has_meta()); - let size = place.layout.size; - self.get_ptr_alloc_mut(place.ptr, size, place.align) + let (size, _align) = self + .size_and_align_of_mplace(&mplace)? + .unwrap_or((mplace.layout.size, mplace.layout.align.abi)); + // Due to packed places, only `mplace.align` matters. + self.get_ptr_alloc_mut(mplace.ptr, size, mplace.align) } /// Check if this mplace is dereferenceable and sufficiently aligned. diff --git a/compiler/rustc_const_eval/src/interpret/terminator.rs b/compiler/rustc_const_eval/src/interpret/terminator.rs index 15823a5975e..97d7a68e190 100644 --- a/compiler/rustc_const_eval/src/interpret/terminator.rs +++ b/compiler/rustc_const_eval/src/interpret/terminator.rs @@ -1,7 +1,8 @@ use std::borrow::Cow; +use either::Either; use rustc_ast::ast::InlineAsmOptions; -use rustc_middle::ty::layout::{FnAbiOf, LayoutOf}; +use rustc_middle::ty::layout::{FnAbiOf, LayoutOf, TyAndLayout}; use rustc_middle::ty::Instance; use rustc_middle::{ mir, @@ -12,12 +13,63 @@ use rustc_target::abi::call::{ArgAbi, ArgAttribute, ArgAttributes, FnAbi, PassMo use rustc_target::spec::abi::Abi; use super::{ - FnVal, ImmTy, Immediate, InterpCx, InterpResult, MPlaceTy, Machine, MemoryKind, OpTy, Operand, - PlaceTy, Scalar, StackPopCleanup, + AllocId, FnVal, ImmTy, Immediate, InterpCx, InterpResult, MPlaceTy, Machine, MemoryKind, OpTy, + Operand, PlaceTy, Provenance, Scalar, StackPopCleanup, }; use crate::fluent_generated as fluent; +/// An argment passed to a function. +#[derive(Clone, Debug)] +pub enum FnArg<'tcx, Prov: Provenance = AllocId> { + /// Pass a copy of the given operand. + Copy(OpTy<'tcx, Prov>), + /// Allow for the argument to be passed in-place: destroy the value originally stored at that place and + /// make the place inaccessible for the duration of the function call. + InPlace(PlaceTy<'tcx, Prov>), +} + +impl<'tcx, Prov: Provenance> FnArg<'tcx, Prov> { + pub fn layout(&self) -> &TyAndLayout<'tcx> { + match self { + FnArg::Copy(op) => &op.layout, + FnArg::InPlace(place) => &place.layout, + } + } +} + impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { + /// Make a copy of the given fn_arg. Any `InPlace` are degenerated to copies, no protection of the + /// original memory occurs. + pub fn copy_fn_arg( + &self, + arg: &FnArg<'tcx, M::Provenance>, + ) -> InterpResult<'tcx, OpTy<'tcx, M::Provenance>> { + match arg { + FnArg::Copy(op) => Ok(op.clone()), + FnArg::InPlace(place) => self.place_to_op(&place), + } + } + + /// Make a copy of the given fn_args. Any `InPlace` are degenerated to copies, no protection of the + /// original memory occurs. + pub fn copy_fn_args( + &self, + args: &[FnArg<'tcx, M::Provenance>], + ) -> InterpResult<'tcx, Vec>> { + args.iter().map(|fn_arg| self.copy_fn_arg(fn_arg)).collect() + } + + pub fn fn_arg_field( + &mut self, + arg: &FnArg<'tcx, M::Provenance>, + field: usize, + ) -> InterpResult<'tcx, FnArg<'tcx, M::Provenance>> { + Ok(match arg { + FnArg::Copy(op) => FnArg::Copy(self.operand_field(op, field)?), + FnArg::InPlace(place) => FnArg::InPlace(self.place_field(place, field)?), + }) + } + pub(super) fn eval_terminator( &mut self, terminator: &mir::Terminator<'tcx>, @@ -68,14 +120,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let old_stack = self.frame_idx(); let old_loc = self.frame().loc; let func = self.eval_operand(func, None)?; - let args = self.eval_operands(args)?; + let args = self.eval_fn_call_arguments(args)?; let fn_sig_binder = func.layout.ty.fn_sig(*self.tcx); let fn_sig = self.tcx.normalize_erasing_late_bound_regions(self.param_env, fn_sig_binder); let extra_args = &args[fn_sig.inputs().len()..]; let extra_args = - self.tcx.mk_type_list_from_iter(extra_args.iter().map(|arg| arg.layout.ty)); + self.tcx.mk_type_list_from_iter(extra_args.iter().map(|arg| arg.layout().ty)); let (fn_val, fn_abi, with_caller_location) = match *func.layout.ty.kind() { ty::FnPtr(_sig) => { @@ -185,6 +237,21 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(()) } + /// Evaluate the arguments of a function call + pub(super) fn eval_fn_call_arguments( + &mut self, + ops: &[mir::Operand<'tcx>], + ) -> InterpResult<'tcx, Vec>> { + ops.iter() + .map(|op| { + Ok(match op { + mir::Operand::Move(place) => FnArg::InPlace(self.eval_place(*place)?), + _ => FnArg::Copy(self.eval_operand(op, None)?), + }) + }) + .collect() + } + fn check_argument_compat( caller_abi: &ArgAbi<'tcx, Ty<'tcx>>, callee_abi: &ArgAbi<'tcx, Ty<'tcx>>, @@ -275,7 +342,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { fn pass_argument<'x, 'y>( &mut self, caller_args: &mut impl Iterator< - Item = (&'x OpTy<'tcx, M::Provenance>, &'y ArgAbi<'tcx, Ty<'tcx>>), + Item = (&'x FnArg<'tcx, M::Provenance>, &'y ArgAbi<'tcx, Ty<'tcx>>), >, callee_abi: &ArgAbi<'tcx, Ty<'tcx>>, callee_arg: &PlaceTy<'tcx, M::Provenance>, @@ -295,21 +362,25 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // Now, check if !Self::check_argument_compat(caller_abi, callee_abi) { let callee_ty = format!("{}", callee_arg.layout.ty); - let caller_ty = format!("{}", caller_arg.layout.ty); + let caller_ty = format!("{}", caller_arg.layout().ty); throw_ub_custom!( fluent::const_eval_incompatible_types, callee_ty = callee_ty, caller_ty = caller_ty, ) } + // We work with a copy of the argument for now; if this is in-place argument passing, we + // will later protect the source it comes from. This means the callee cannot observe if we + // did in-place of by-copy argument passing, except for pointer equality tests. + let caller_arg_copy = self.copy_fn_arg(&caller_arg)?; // Special handling for unsized parameters. - if caller_arg.layout.is_unsized() { + if caller_arg_copy.layout.is_unsized() { // `check_argument_compat` ensures that both have the same type, so we know they will use the metadata the same way. - assert_eq!(caller_arg.layout.ty, callee_arg.layout.ty); + assert_eq!(caller_arg_copy.layout.ty, callee_arg.layout.ty); // We have to properly pre-allocate the memory for the callee. - // So let's tear down some wrappers. + // So let's tear down some abstractions. // This all has to be in memory, there are no immediate unsized values. - let src = caller_arg.assert_mem_place(); + let src = caller_arg_copy.assert_mem_place(); // The destination cannot be one of these "spread args". let (dest_frame, dest_local) = callee_arg.assert_local(); // We are just initializing things, so there can't be anything here yet. @@ -331,7 +402,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // FIXME: Depending on the PassMode, this should reset some padding to uninitialized. (This // is true for all `copy_op`, but there are a lot of special cases for argument passing // specifically.) - self.copy_op(&caller_arg, callee_arg, /*allow_transmute*/ true) + self.copy_op(&caller_arg_copy, callee_arg, /*allow_transmute*/ true)?; + // If this was an in-place pass, protect the place it comes from for the duration of the call. + if let FnArg::InPlace(place) = caller_arg { + M::protect_in_place_function_argument(self, place)?; + } + Ok(()) } /// Call this function -- pushing the stack frame and initializing the arguments. @@ -346,7 +422,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { &mut self, fn_val: FnVal<'tcx, M::ExtraFnVal>, (caller_abi, caller_fn_abi): (Abi, &FnAbi<'tcx, Ty<'tcx>>), - args: &[OpTy<'tcx, M::Provenance>], + args: &[FnArg<'tcx, M::Provenance>], with_caller_location: bool, destination: &PlaceTy<'tcx, M::Provenance>, target: Option, @@ -372,8 +448,15 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { match instance.def { ty::InstanceDef::Intrinsic(def_id) => { assert!(self.tcx.is_intrinsic(def_id)); - // caller_fn_abi is not relevant here, we interpret the arguments directly for each intrinsic. - M::call_intrinsic(self, instance, args, destination, target, unwind) + // FIXME: Should `InPlace` arguments be reset to uninit? + M::call_intrinsic( + self, + instance, + &self.copy_fn_args(args)?, + destination, + target, + unwind, + ) } ty::InstanceDef::VTableShim(..) | ty::InstanceDef::ReifyShim(..) @@ -428,7 +511,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { "caller ABI: {:?}, args: {:#?}", caller_abi, args.iter() - .map(|arg| (arg.layout.ty, format!("{:?}", **arg))) + .map(|arg| ( + arg.layout().ty, + match arg { + FnArg::Copy(op) => format!("copy({:?})", *op), + FnArg::InPlace(place) => format!("in-place({:?})", *place), + } + )) .collect::>() ); trace!( @@ -449,7 +538,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // last incoming argument. These two iterators do not have the same type, // so to keep the code paths uniform we accept an allocation // (for RustCall ABI only). - let caller_args: Cow<'_, [OpTy<'tcx, M::Provenance>]> = + let caller_args: Cow<'_, [FnArg<'tcx, M::Provenance>]> = if caller_abi == Abi::RustCall && !args.is_empty() { // Untuple let (untuple_arg, args) = args.split_last().unwrap(); @@ -458,11 +547,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { args.iter() .map(|a| Ok(a.clone())) .chain( - (0..untuple_arg.layout.fields.count()) - .map(|i| self.operand_field(untuple_arg, i)), + (0..untuple_arg.layout().fields.count()) + .map(|i| self.fn_arg_field(untuple_arg, i)), ) - .collect::>>>( - )?, + .collect::>>()?, ) } else { // Plain arg passing @@ -523,6 +611,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { caller_ty = caller_ty, ) } + // Ensure the return place is aligned and dereferenceable, and protect it for + // in-place return value passing. + if let Either::Left(mplace) = destination.as_mplace_or_local() { + self.check_mplace(mplace)?; + } else { + // Nothing to do for locals, they are always properly allocated and aligned. + } + M::protect_in_place_function_argument(self, destination)?; }; match res { Err(err) => { @@ -538,7 +634,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // We have to implement all "object safe receivers". So we have to go search for a // pointer or `dyn Trait` type, but it could be wrapped in newtypes. So recursively // unwrap those newtypes until we are there. - let mut receiver = args[0].clone(); + // An `InPlace` does nothing here, we keep the original receiver intact. We can't + // really pass the argument in-place anyway, and we are constructing a new + // `Immediate` receiver. + let mut receiver = self.copy_fn_arg(&args[0])?; let receiver_place = loop { match receiver.layout.ty.kind() { ty::Ref(..) | ty::RawPtr(..) => { @@ -648,11 +747,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } // Adjust receiver argument. Layout can be any (thin) ptr. - args[0] = ImmTy::from_immediate( - Scalar::from_maybe_pointer(adjusted_receiver, self).into(), - self.layout_of(Ty::new_mut_ptr(self.tcx.tcx, dyn_ty))?, - ) - .into(); + args[0] = FnArg::Copy( + ImmTy::from_immediate( + Scalar::from_maybe_pointer(adjusted_receiver, self).into(), + self.layout_of(Ty::new_mut_ptr(self.tcx.tcx, dyn_ty))?, + ) + .into(), + ); trace!("Patched receiver operand to {:#?}", args[0]); // recurse with concrete function self.eval_fn_call( @@ -710,7 +811,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.eval_fn_call( FnVal::Instance(instance), (Abi::Rust, fn_abi), - &[arg.into()], + &[FnArg::Copy(arg.into())], false, &ret.into(), Some(target), diff --git a/compiler/rustc_const_eval/src/interpret/visitor.rs b/compiler/rustc_const_eval/src/interpret/visitor.rs index 7a14459399c..879ae198f7e 100644 --- a/compiler/rustc_const_eval/src/interpret/visitor.rs +++ b/compiler/rustc_const_eval/src/interpret/visitor.rs @@ -13,7 +13,7 @@ use super::{InterpCx, MPlaceTy, Machine, OpTy, PlaceTy}; /// A thing that we can project into, and that has a layout. /// This wouldn't have to depend on `Machine` but with the current type inference, /// that's just more convenient to work with (avoids repeating all the `Machine` bounds). -pub trait Value<'mir, 'tcx, M: Machine<'mir, 'tcx>>: Sized { +pub trait Value<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>>: Sized { /// Gets this value's layout. fn layout(&self) -> TyAndLayout<'tcx>; @@ -54,7 +54,7 @@ pub trait Value<'mir, 'tcx, M: Machine<'mir, 'tcx>>: Sized { /// A thing that we can project into given *mutable* access to `ecx`, and that has a layout. /// This wouldn't have to depend on `Machine` but with the current type inference, /// that's just more convenient to work with (avoids repeating all the `Machine` bounds). -pub trait ValueMut<'mir, 'tcx, M: Machine<'mir, 'tcx>>: Sized { +pub trait ValueMut<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>>: Sized { /// Gets this value's layout. fn layout(&self) -> TyAndLayout<'tcx>; diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index 2f2c7357b00..f3ac679f97b 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -22,8 +22,8 @@ use rustc_target::spec::abi::Abi as CallAbi; use crate::MirPass; use rustc_const_eval::interpret::{ - self, compile_time_machine, AllocId, ConstAllocation, ConstValue, Frame, ImmTy, Immediate, - InterpCx, InterpResult, LocalValue, MemoryKind, OpTy, PlaceTy, Pointer, Scalar, + self, compile_time_machine, AllocId, ConstAllocation, ConstValue, FnArg, Frame, ImmTy, + Immediate, InterpCx, InterpResult, LocalValue, MemoryKind, OpTy, PlaceTy, Pointer, Scalar, StackPopCleanup, }; @@ -185,7 +185,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx> _ecx: &mut InterpCx<'mir, 'tcx, Self>, _instance: ty::Instance<'tcx>, _abi: CallAbi, - _args: &[OpTy<'tcx>], + _args: &[FnArg<'tcx>], _destination: &PlaceTy<'tcx>, _target: Option, _unwind: UnwindAction, diff --git a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs index 78fb196358f..5b6cbb5577c 100644 --- a/compiler/rustc_mir_transform/src/dataflow_const_prop.rs +++ b/compiler/rustc_mir_transform/src/dataflow_const_prop.rs @@ -532,7 +532,7 @@ impl<'tcx, 'map, 'a> Visitor<'tcx> for OperandCollector<'tcx, 'map, 'a> { struct DummyMachine; -impl<'mir, 'tcx> rustc_const_eval::interpret::Machine<'mir, 'tcx> for DummyMachine { +impl<'mir, 'tcx: 'mir> rustc_const_eval::interpret::Machine<'mir, 'tcx> for DummyMachine { rustc_const_eval::interpret::compile_time_machine!(<'mir, 'tcx>); type MemoryKind = !; const PANIC_ON_ALLOC_FAIL: bool = true; @@ -557,7 +557,7 @@ impl<'mir, 'tcx> rustc_const_eval::interpret::Machine<'mir, 'tcx> for DummyMachi _ecx: &mut InterpCx<'mir, 'tcx, Self>, _instance: ty::Instance<'tcx>, _abi: rustc_target::spec::abi::Abi, - _args: &[rustc_const_eval::interpret::OpTy<'tcx, Self::Provenance>], + _args: &[rustc_const_eval::interpret::FnArg<'tcx, Self::Provenance>], _destination: &rustc_const_eval::interpret::PlaceTy<'tcx, Self::Provenance>, _target: Option, _unwind: UnwindAction, diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs index faa23fd2620..a2cf7c80950 100644 --- a/src/tools/miri/src/borrow_tracker/mod.rs +++ b/src/tools/miri/src/borrow_tracker/mod.rs @@ -302,12 +302,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } } - fn retag_return_place(&mut self) -> InterpResult<'tcx> { + fn protect_place(&mut self, place: &MPlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { let this = self.eval_context_mut(); let method = this.machine.borrow_tracker.as_ref().unwrap().borrow().borrow_tracker_method; match method { - BorrowTrackerMethod::StackedBorrows => this.sb_retag_return_place(), - BorrowTrackerMethod::TreeBorrows => this.tb_retag_return_place(), + BorrowTrackerMethod::StackedBorrows => this.sb_protect_place(place), + BorrowTrackerMethod::TreeBorrows => this.tb_protect_place(place), } } diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs index de307a3c5f5..5ec8d80fb32 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs @@ -189,7 +189,7 @@ struct RetagOp { #[derive(Debug, Clone, Copy, PartialEq)] pub enum RetagCause { Normal, - FnReturnPlace, + InPlaceFnPassing, FnEntry, TwoPhase, } @@ -501,7 +501,7 @@ impl RetagCause { match self { RetagCause::Normal => "retag", RetagCause::FnEntry => "function-entry retag", - RetagCause::FnReturnPlace => "return-place retag", + RetagCause::InPlaceFnPassing => "in-place function argument/return passing protection", RetagCause::TwoPhase => "two-phase retag", } .to_string() diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index ca0f69450c9..e22b352e740 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -994,35 +994,25 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } } - /// After a stack frame got pushed, retag the return place so that we are sure - /// it does not alias with anything. - /// - /// This is a HACK because there is nothing in MIR that would make the retag - /// explicit. Also see . - fn sb_retag_return_place(&mut self) -> InterpResult<'tcx> { + /// Protect a place so that it cannot be used any more for the duration of the current function + /// call. + /// + /// This is used to ensure soundness of in-place function argument/return passing. + fn sb_protect_place(&mut self, place: &MPlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - let return_place = &this.frame().return_place; - if return_place.layout.is_zst() { - // There may not be any memory here, nothing to do. - return Ok(()); - } - // We need this to be in-memory to use tagged pointers. - let return_place = this.force_allocation(&return_place.clone())?; - // We have to turn the place into a pointer to use the existing code. + // We have to turn the place into a pointer to use the usual retagging logic. // (The pointer type does not matter, so we use a raw pointer.) - let ptr_layout = this.layout_of(Ty::new_mut_ptr(this.tcx.tcx, return_place.layout.ty))?; - let val = ImmTy::from_immediate(return_place.to_ref(this), ptr_layout); - // Reborrow it. With protection! That is part of the point. + let ptr_layout = this.layout_of(Ty::new_mut_ptr(this.tcx.tcx, place.layout.ty))?; + let ptr = ImmTy::from_immediate(place.to_ref(this), ptr_layout); + // Reborrow it. With protection! That is the entire point. let new_perm = NewPermission::Uniform { perm: Permission::Unique, access: Some(AccessKind::Write), protector: Some(ProtectorKind::StrongProtector), }; - let val = this.sb_retag_reference(&val, new_perm, RetagCause::FnReturnPlace)?; - // And use reborrowed pointer for return place. - let return_place = this.ref_to_mplace(&val)?; - this.frame_mut().return_place = return_place.into(); + let _new_ptr = this.sb_retag_reference(&ptr, new_perm, RetagCause::InPlaceFnPassing)?; + // We just throw away `new_ptr`, so nobody can access this memory while it is protected. Ok(()) } diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index 3bb38a249ff..a3f0310f1df 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -493,36 +493,25 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } } - /// After a stack frame got pushed, retag the return place so that we are sure - /// it does not alias with anything. - /// - /// This is a HACK because there is nothing in MIR that would make the retag - /// explicit. Also see . - fn tb_retag_return_place(&mut self) -> InterpResult<'tcx> { + /// Protect a place so that it cannot be used any more for the duration of the current function + /// call. + /// + /// This is used to ensure soundness of in-place function argument/return passing. + fn tb_protect_place(&mut self, place: &MPlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { let this = self.eval_context_mut(); - //this.debug_hint_location(); - let return_place = &this.frame().return_place; - if return_place.layout.is_zst() { - // There may not be any memory here, nothing to do. - return Ok(()); - } - // We need this to be in-memory to use tagged pointers. - let return_place = this.force_allocation(&return_place.clone())?; - // We have to turn the place into a pointer to use the existing code. + // We have to turn the place into a pointer to use the usual retagging logic. // (The pointer type does not matter, so we use a raw pointer.) - let ptr_layout = this.layout_of(Ty::new_mut_ptr(this.tcx.tcx, return_place.layout.ty))?; - let val = ImmTy::from_immediate(return_place.to_ref(this), ptr_layout); - // Reborrow it. With protection! That is part of the point. + let ptr_layout = this.layout_of(Ty::new_mut_ptr(this.tcx.tcx, place.layout.ty))?; + let ptr = ImmTy::from_immediate(place.to_ref(this), ptr_layout); + // Reborrow it. With protection! That is the entire point. let new_perm = Some(NewPermission { initial_state: Permission::new_active(), zero_size: false, protector: Some(ProtectorKind::StrongProtector), }); - let val = this.tb_retag_reference(&val, new_perm)?; - // And use reborrowed pointer for return place. - let return_place = this.ref_to_mplace(&val)?; - this.frame_mut().return_place = return_place.into(); + let _new_ptr = this.tb_retag_reference(&ptr, new_perm)?; + // We just throw away `new_ptr`, so nobody can access this memory while it is protected. Ok(()) } diff --git a/src/tools/miri/src/lib.rs b/src/tools/miri/src/lib.rs index 7e92dc7a0c7..4a093d7bcc6 100644 --- a/src/tools/miri/src/lib.rs +++ b/src/tools/miri/src/lib.rs @@ -55,6 +55,7 @@ extern crate rustc_index; extern crate rustc_session; extern crate rustc_span; extern crate rustc_target; +extern crate either; // the one from rustc // Necessary to pull in object code as the rest of the rustc crates are shipped only as rmeta // files. diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index ac2bad22119..5510e3f94b7 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -7,6 +7,7 @@ use std::fmt; use std::path::Path; use std::process; +use either::Either; use rand::rngs::StdRng; use rand::SeedableRng; @@ -533,7 +534,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { let target = &tcx.sess.target; match target.arch.as_ref() { "wasm32" | "wasm64" => 64 * 1024, // https://webassembly.github.io/spec/core/exec/runtime.html#memory-instances - "aarch64" => + "aarch64" => { if target.options.vendor.as_ref() == "apple" { // No "definitive" source, but see: // https://www.wwdcnotes.com/notes/wwdc20/10214/ @@ -541,7 +542,8 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { 16 * 1024 } else { 4 * 1024 - }, + } + } _ => 4 * 1024, } }; @@ -892,7 +894,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { ecx: &mut MiriInterpCx<'mir, 'tcx>, instance: ty::Instance<'tcx>, abi: Abi, - args: &[OpTy<'tcx, Provenance>], + args: &[FnArg<'tcx, Provenance>], dest: &PlaceTy<'tcx, Provenance>, ret: Option, unwind: mir::UnwindAction, @@ -905,12 +907,13 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { ecx: &mut MiriInterpCx<'mir, 'tcx>, fn_val: Dlsym, abi: Abi, - args: &[OpTy<'tcx, Provenance>], + args: &[FnArg<'tcx, Provenance>], dest: &PlaceTy<'tcx, Provenance>, ret: Option, _unwind: mir::UnwindAction, ) -> InterpResult<'tcx> { - ecx.call_dlsym(fn_val, abi, args, dest, ret) + let args = ecx.copy_fn_args(args)?; // FIXME: Should `InPlace` arguments be reset to uninit? + ecx.call_dlsym(fn_val, abi, &args, dest, ret) } #[inline(always)] @@ -1094,8 +1097,9 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { ptr: Pointer, ) -> InterpResult<'tcx> { match ptr.provenance { - Provenance::Concrete { alloc_id, tag } => - intptrcast::GlobalStateInner::expose_ptr(ecx, alloc_id, tag), + Provenance::Concrete { alloc_id, tag } => { + intptrcast::GlobalStateInner::expose_ptr(ecx, alloc_id, tag) + } Provenance::Wildcard => { // No need to do anything for wildcard pointers as // their provenances have already been previously exposed. @@ -1206,6 +1210,25 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { Ok(()) } + fn protect_in_place_function_argument( + ecx: &mut InterpCx<'mir, 'tcx, Self>, + place: &PlaceTy<'tcx, Provenance>, + ) -> InterpResult<'tcx> { + // We do need to write `uninit` so that even after the call ends, the former contents of + // this place cannot be observed any more. + ecx.write_uninit(place)?; + // If we have a borrow tracker, we also have it set up protection so that all reads *and + // writes* during this call are insta-UB. + if ecx.machine.borrow_tracker.is_some() { + if let Either::Left(place) = place.as_mplace_or_local() { + ecx.protect_place(&place)?; + } else { + // Locals that don't have their address taken are as protected as they can ever be. + } + } + Ok(()) + } + #[inline(always)] fn init_frame_extra( ecx: &mut InterpCx<'mir, 'tcx, Self>, @@ -1288,8 +1311,17 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { let stack_len = ecx.active_thread_stack().len(); ecx.active_thread_mut().set_top_user_relevant_frame(stack_len - 1); } - if ecx.machine.borrow_tracker.is_some() { - ecx.retag_return_place()?; + Ok(()) + } + + fn before_stack_pop( + ecx: &InterpCx<'mir, 'tcx, Self>, + frame: &Frame<'mir, 'tcx, Self::Provenance, Self::FrameExtra>, + ) -> InterpResult<'tcx> { + // We want this *before* the return value copy, because the return place itself is protected + // until we do `end_call` here. + if let Some(borrow_tracker) = &ecx.machine.borrow_tracker { + borrow_tracker.borrow_mut().end_call(&frame.extra); } Ok(()) } @@ -1308,9 +1340,6 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { ecx.active_thread_mut().recompute_top_user_relevant_frame(); } let timing = frame.extra.timing.take(); - if let Some(borrow_tracker) = &ecx.machine.borrow_tracker { - borrow_tracker.borrow_mut().end_call(&frame.extra); - } let res = ecx.handle_stack_pop_unwind(frame.extra, unwinding); if let Some(profiler) = ecx.machine.profiler.as_ref() { profiler.finish_recording_interval_event(timing.unwrap()); diff --git a/src/tools/miri/src/shims/mod.rs b/src/tools/miri/src/shims/mod.rs index a423a0786b7..1027b24e301 100644 --- a/src/tools/miri/src/shims/mod.rs +++ b/src/tools/miri/src/shims/mod.rs @@ -31,7 +31,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { &mut self, instance: ty::Instance<'tcx>, abi: Abi, - args: &[OpTy<'tcx, Provenance>], + args: &[FnArg<'tcx, Provenance>], dest: &PlaceTy<'tcx, Provenance>, ret: Option, unwind: mir::UnwindAction, @@ -41,7 +41,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // There are some more lang items we want to hook that CTFE does not hook (yet). if this.tcx.lang_items().align_offset_fn() == Some(instance.def.def_id()) { - let [ptr, align] = check_arg_count(args)?; + let args = this.copy_fn_args(args)?; + let [ptr, align] = check_arg_count(&args)?; if this.align_offset(ptr, align, dest, ret, unwind)? { return Ok(None); } @@ -55,7 +56,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // to run extra MIR), and Ok(Some(body)) if we found MIR to run for the // foreign function // Any needed call to `goto_block` will be performed by `emulate_foreign_item`. - return this.emulate_foreign_item(instance.def_id(), abi, args, dest, ret, unwind); + let args = this.copy_fn_args(args)?; // FIXME: Should `InPlace` arguments be reset to uninit? + return this.emulate_foreign_item(instance.def_id(), abi, &args, dest, ret, unwind); } // Otherwise, load the MIR. diff --git a/src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.rs b/src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.rs deleted file mode 100644 index 03886e7874d..00000000000 --- a/src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.rs +++ /dev/null @@ -1,32 +0,0 @@ -//@revisions: stack tree -//@[tree]compile-flags: -Zmiri-tree-borrows -#![feature(raw_ref_op)] -#![feature(core_intrinsics)] -#![feature(custom_mir)] - -use std::intrinsics::mir::*; -use std::mem::MaybeUninit; - -#[custom_mir(dialect = "runtime", phase = "optimized")] -pub fn main() { - mir! { - { - let x = 0; - let ptr = &raw mut x; - // We arrange for `myfun` to have a pointer that aliases - // its return place. Even just reading from that pointer is UB. - Call(*ptr, after_call, myfun(ptr)) - } - - after_call = { - Return() - } - } -} - -fn myfun(ptr: *mut i32) -> i32 { - unsafe { ptr.cast::>().read() }; - //~[stack]^ ERROR: /not granting access/ - //~[tree]| ERROR: /read access .* forbidden/ - 13 -} diff --git a/src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.stack.stderr b/src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.stack.stderr deleted file mode 100644 index 5bdddf5e49c..00000000000 --- a/src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.stack.stderr +++ /dev/null @@ -1,42 +0,0 @@ -error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID - --> $DIR/return_pointer_aliasing.rs:LL:CC - | -LL | unsafe { ptr.cast::>().read() }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID - | - = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental - = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information -help: was created by a SharedReadWrite retag at offsets [0x0..0x4] - --> $DIR/return_pointer_aliasing.rs:LL:CC - | -LL | / mir! { -LL | | { -LL | | let x = 0; -LL | | let ptr = &raw mut x; -... | -LL | | } -LL | | } - | |_____^ -help: is this argument - --> $DIR/return_pointer_aliasing.rs:LL:CC - | -LL | / fn myfun(ptr: *mut i32) -> i32 { -LL | | unsafe { ptr.cast::>().read() }; -LL | | -LL | | -LL | | 13 -LL | | } - | |_^ - = note: BACKTRACE (of the first span): - = note: inside `myfun` at $DIR/return_pointer_aliasing.rs:LL:CC -note: inside `main` - --> $DIR/return_pointer_aliasing.rs:LL:CC - | -LL | Call(*ptr, after_call, myfun(ptr)) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: this error originates in the macro `::core::intrinsics::mir::__internal_remove_let` which comes from the expansion of the macro `mir` (in Nightly builds, run with -Z macro-backtrace for more info) - -note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace - -error: aborting due to previous error - diff --git a/src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.tree.stderr b/src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.tree.stderr deleted file mode 100644 index dd76d65c7ca..00000000000 --- a/src/tools/miri/tests/fail/both_borrows/return_pointer_aliasing.tree.stderr +++ /dev/null @@ -1,44 +0,0 @@ -error: Undefined Behavior: read access through (root of the allocation) is forbidden - --> $DIR/return_pointer_aliasing.rs:LL:CC - | -LL | unsafe { ptr.cast::>().read() }; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ read access through (root of the allocation) is forbidden - | - = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = help: the accessed tag (root of the allocation) is foreign to the protected tag (i.e., it is not a child) - = help: this foreign read access would cause the protected tag to transition from Active to Frozen - = help: this transition would be a loss of write permissions, which is not allowed for protected tags -help: the accessed tag was created here - --> $DIR/return_pointer_aliasing.rs:LL:CC - | -LL | / mir! { -LL | | { -LL | | let x = 0; -LL | | let ptr = &raw mut x; -... | -LL | | } -LL | | } - | |_____^ -help: the protected tag was created here, in the initial state Active - --> $DIR/return_pointer_aliasing.rs:LL:CC - | -LL | / fn myfun(ptr: *mut i32) -> i32 { -LL | | unsafe { ptr.cast::>().read() }; -LL | | -LL | | -LL | | 13 -LL | | } - | |_^ - = note: BACKTRACE (of the first span): - = note: inside `myfun` at $DIR/return_pointer_aliasing.rs:LL:CC -note: inside `main` - --> $DIR/return_pointer_aliasing.rs:LL:CC - | -LL | Call(*ptr, after_call, myfun(ptr)) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: this error originates in the macro `::core::intrinsics::mir::__internal_remove_let` which comes from the expansion of the macro `mir` (in Nightly builds, run with -Z macro-backtrace for more info) - -note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace - -error: aborting due to previous error - diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.rs b/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.rs new file mode 100644 index 00000000000..625a8bda8af --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.rs @@ -0,0 +1,34 @@ +//@revisions: stack tree +//@[tree]compile-flags: -Zmiri-tree-borrows +#![feature(custom_mir, core_intrinsics)] +use std::intrinsics::mir::*; + +pub struct S(i32); + +#[custom_mir(dialect = "runtime", phase = "optimized")] +fn main() { + mir! { + let unit: (); + { + let non_copy = S(42); + let ptr = std::ptr::addr_of_mut!(non_copy); + // Inside `callee`, the first argument and `*ptr` are basically + // aliasing places! + Call(unit, after_call, callee(Move(*ptr), ptr)) + } + after_call = { + Return() + } + + } +} + +pub fn callee(x: S, ptr: *mut S) { + // With the setup above, if `x` is indeed moved in + // (i.e. we actually just get a pointer to the underlying storage), + // then writing to `ptr` will change the value stored in `x`! + unsafe { ptr.write(S(0)) }; + //~[stack]^ ERROR: not granting access + //~[tree]| ERROR: /write access .* forbidden/ + assert_eq!(x.0, 42); +} diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.stack.stderr b/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.stack.stderr new file mode 100644 index 00000000000..471dc1dd6dd --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.stack.stderr @@ -0,0 +1,37 @@ +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID + --> $DIR/arg_inplace_mutate.rs:LL:CC + | +LL | unsafe { ptr.write(S(0)) }; + | ^^^^^^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental + = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information +help: was created by a SharedReadWrite retag at offsets [0x0..0x4] + --> $DIR/arg_inplace_mutate.rs:LL:CC + | +LL | / mir! { +LL | | let unit: (); +LL | | { +LL | | let non_copy = S(42); +... | +LL | | +LL | | } + | |_____^ +help: is this argument + --> $DIR/arg_inplace_mutate.rs:LL:CC + | +LL | unsafe { ptr.write(S(0)) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: BACKTRACE (of the first span): + = note: inside `callee` at $DIR/arg_inplace_mutate.rs:LL:CC +note: inside `main` + --> $DIR/arg_inplace_mutate.rs:LL:CC + | +LL | Call(unit, after_call, callee(Move(*ptr), ptr)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in the macro `::core::intrinsics::mir::__internal_remove_let` which comes from the expansion of the macro `mir` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.tree.stderr b/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.tree.stderr new file mode 100644 index 00000000000..35c02cc2ebd --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_mutate.tree.stderr @@ -0,0 +1,39 @@ +error: Undefined Behavior: write access through (root of the allocation) is forbidden + --> $DIR/arg_inplace_mutate.rs:LL:CC + | +LL | unsafe { ptr.write(S(0)) }; + | ^^^^^^^^^^^^^^^ write access through (root of the allocation) is forbidden + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental + = help: the accessed tag (root of the allocation) is foreign to the protected tag (i.e., it is not a child) + = help: this foreign write access would cause the protected tag to transition from Active to Disabled + = help: this transition would be a loss of read and write permissions, which is not allowed for protected tags +help: the accessed tag was created here + --> $DIR/arg_inplace_mutate.rs:LL:CC + | +LL | / mir! { +LL | | let unit: (); +LL | | { +LL | | let non_copy = S(42); +... | +LL | | +LL | | } + | |_____^ +help: the protected tag was created here, in the initial state Active + --> $DIR/arg_inplace_mutate.rs:LL:CC + | +LL | unsafe { ptr.write(S(0)) }; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: BACKTRACE (of the first span): + = note: inside `callee` at $DIR/arg_inplace_mutate.rs:LL:CC +note: inside `main` + --> $DIR/arg_inplace_mutate.rs:LL:CC + | +LL | Call(unit, after_call, callee(Move(*ptr), ptr)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in the macro `::core::intrinsics::mir::__internal_remove_let` which comes from the expansion of the macro `mir` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_after.rs b/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_after.rs new file mode 100644 index 00000000000..8eda913feb4 --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_after.rs @@ -0,0 +1,27 @@ +#![feature(custom_mir, core_intrinsics)] +use std::intrinsics::mir::*; + +pub struct S(i32); + +#[custom_mir(dialect = "runtime", phase = "optimized")] +fn main() { + // FIXME: the span is not great (probably caused by custom MIR) + mir! { //~ERROR: uninitialized + let unit: (); + { + let non_copy = S(42); + // This could change `non_copy` in-place + Call(unit, after_call, change_arg(Move(non_copy))) + } + after_call = { + // So now we must not be allowed to observe non-copy again. + let _observe = non_copy.0; + Return() + } + + } +} + +pub fn change_arg(mut x: S) { + x.0 = 0; +} diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_after.stderr b/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_after.stderr new file mode 100644 index 00000000000..3ff7976c70b --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_after.stderr @@ -0,0 +1,22 @@ +error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory + --> $DIR/arg_inplace_observe_after.rs:LL:CC + | +LL | / mir! { +LL | | let unit: (); +LL | | { +LL | | let non_copy = S(42); +... | +LL | | +LL | | } + | |_____^ using uninitialized data, but this operation requires initialized memory + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE: + = note: inside `main` at RUSTLIB/core/src/intrinsics/mir.rs:LL:CC + = note: this error originates in the macro `::core::intrinsics::mir::__internal_remove_let` which comes from the expansion of the macro `mir` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.none.stderr b/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.none.stderr new file mode 100644 index 00000000000..baa91484793 --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.none.stderr @@ -0,0 +1,20 @@ +error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory + --> $DIR/arg_inplace_observe_during.rs:LL:CC + | +LL | unsafe { ptr.read() }; + | ^^^^^^^^^^ using uninitialized data, but this operation requires initialized memory + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE: + = note: inside `change_arg` at $DIR/arg_inplace_observe_during.rs:LL:CC +note: inside `main` + --> $DIR/arg_inplace_observe_during.rs:LL:CC + | +LL | Call(unit, after_call, change_arg(Move(*ptr), ptr)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.rs b/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.rs new file mode 100644 index 00000000000..2e57872db96 --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.rs @@ -0,0 +1,34 @@ +//@revisions: stack tree none +//@[tree]compile-flags: -Zmiri-tree-borrows +//@[none]compile-flags: -Zmiri-disable-stacked-borrows +#![feature(custom_mir, core_intrinsics)] +use std::intrinsics::mir::*; + +pub struct S(i32); + +#[custom_mir(dialect = "runtime", phase = "optimized")] +fn main() { + mir! { + let unit: (); + { + let non_copy = S(42); + let ptr = std::ptr::addr_of_mut!(non_copy); + // This could change `non_copy` in-place + Call(unit, after_call, change_arg(Move(*ptr), ptr)) + } + after_call = { + Return() + } + + } +} + +pub fn change_arg(mut x: S, ptr: *mut S) { + x.0 = 0; + // If `x` got passed in-place, we'd see the write through `ptr`! + // Make sure we are not allowed to do that read. + unsafe { ptr.read() }; + //~[stack]^ ERROR: not granting access + //~[tree]| ERROR: /read access .* forbidden/ + //~[none]| ERROR: uninitialized +} diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.stack.stderr b/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.stack.stderr new file mode 100644 index 00000000000..a842d3a8044 --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.stack.stderr @@ -0,0 +1,37 @@ +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID + --> $DIR/arg_inplace_observe_during.rs:LL:CC + | +LL | unsafe { ptr.read() }; + | ^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental + = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information +help: was created by a SharedReadWrite retag at offsets [0x0..0x4] + --> $DIR/arg_inplace_observe_during.rs:LL:CC + | +LL | / mir! { +LL | | let unit: (); +LL | | { +LL | | let non_copy = S(42); +... | +LL | | +LL | | } + | |_____^ +help: is this argument + --> $DIR/arg_inplace_observe_during.rs:LL:CC + | +LL | x.0 = 0; + | ^^^^^^^ + = note: BACKTRACE (of the first span): + = note: inside `change_arg` at $DIR/arg_inplace_observe_during.rs:LL:CC +note: inside `main` + --> $DIR/arg_inplace_observe_during.rs:LL:CC + | +LL | Call(unit, after_call, change_arg(Move(*ptr), ptr)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in the macro `::core::intrinsics::mir::__internal_remove_let` which comes from the expansion of the macro `mir` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.tree.stderr b/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.tree.stderr new file mode 100644 index 00000000000..cbd76c38f62 --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/arg_inplace_observe_during.tree.stderr @@ -0,0 +1,39 @@ +error: Undefined Behavior: read access through (root of the allocation) is forbidden + --> $DIR/arg_inplace_observe_during.rs:LL:CC + | +LL | unsafe { ptr.read() }; + | ^^^^^^^^^^ read access through (root of the allocation) is forbidden + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental + = help: the accessed tag (root of the allocation) is foreign to the protected tag (i.e., it is not a child) + = help: this foreign read access would cause the protected tag to transition from Active to Frozen + = help: this transition would be a loss of write permissions, which is not allowed for protected tags +help: the accessed tag was created here + --> $DIR/arg_inplace_observe_during.rs:LL:CC + | +LL | / mir! { +LL | | let unit: (); +LL | | { +LL | | let non_copy = S(42); +... | +LL | | +LL | | } + | |_____^ +help: the protected tag was created here, in the initial state Active + --> $DIR/arg_inplace_observe_during.rs:LL:CC + | +LL | x.0 = 0; + | ^^^^^^^ + = note: BACKTRACE (of the first span): + = note: inside `change_arg` at $DIR/arg_inplace_observe_during.rs:LL:CC +note: inside `main` + --> $DIR/arg_inplace_observe_during.rs:LL:CC + | +LL | Call(unit, after_call, change_arg(Move(*ptr), ptr)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in the macro `::core::intrinsics::mir::__internal_remove_let` which comes from the expansion of the macro `mir` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.none.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.none.stderr new file mode 100644 index 00000000000..9d9dfc89f89 --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.none.stderr @@ -0,0 +1,20 @@ +error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory + --> $DIR/return_pointer_aliasing.rs:LL:CC + | +LL | unsafe { ptr.read() }; + | ^^^^^^^^^^ using uninitialized data, but this operation requires initialized memory + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE: + = note: inside `myfun` at $DIR/return_pointer_aliasing.rs:LL:CC +note: inside `main` + --> $DIR/return_pointer_aliasing.rs:LL:CC + | +LL | Call(*ptr, after_call, myfun(ptr)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.rs b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.rs new file mode 100644 index 00000000000..829809102fa --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.rs @@ -0,0 +1,34 @@ +//@revisions: stack tree none +//@[tree]compile-flags: -Zmiri-tree-borrows +//@[none]compile-flags: -Zmiri-disable-stacked-borrows +#![feature(raw_ref_op)] +#![feature(core_intrinsics)] +#![feature(custom_mir)] + +use std::intrinsics::mir::*; + +#[custom_mir(dialect = "runtime", phase = "optimized")] +pub fn main() { + mir! { + { + let x = 0; + let ptr = &raw mut x; + // We arrange for `myfun` to have a pointer that aliases + // its return place. Even just reading from that pointer is UB. + Call(*ptr, after_call, myfun(ptr)) + } + + after_call = { + Return() + } + } +} + +fn myfun(ptr: *mut i32) -> i32 { + unsafe { ptr.read() }; + //~[stack]^ ERROR: not granting access + //~[tree]| ERROR: /read access .* forbidden/ + //~[none]| ERROR: uninitialized + // Without an aliasing model, reads are "fine" but at least they return uninit data. + 13 +} diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.stack.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.stack.stderr new file mode 100644 index 00000000000..d486dcb95df --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.stack.stderr @@ -0,0 +1,37 @@ +error: Undefined Behavior: not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID + --> $DIR/return_pointer_aliasing.rs:LL:CC + | +LL | unsafe { ptr.read() }; + | ^^^^^^^^^^ not granting access to tag because that would remove [Unique for ] which is strongly protected because it is an argument of call ID + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental + = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information +help: was created by a SharedReadWrite retag at offsets [0x0..0x4] + --> $DIR/return_pointer_aliasing.rs:LL:CC + | +LL | / mir! { +LL | | { +LL | | let x = 0; +LL | | let ptr = &raw mut x; +... | +LL | | } +LL | | } + | |_____^ +help: is this argument + --> $DIR/return_pointer_aliasing.rs:LL:CC + | +LL | unsafe { ptr.read() }; + | ^^^^^^^^^^^^^^^^^^^^^ + = note: BACKTRACE (of the first span): + = note: inside `myfun` at $DIR/return_pointer_aliasing.rs:LL:CC +note: inside `main` + --> $DIR/return_pointer_aliasing.rs:LL:CC + | +LL | Call(*ptr, after_call, myfun(ptr)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in the macro `::core::intrinsics::mir::__internal_remove_let` which comes from the expansion of the macro `mir` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.tree.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.tree.stderr new file mode 100644 index 00000000000..c2c9de3f4ee --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing.tree.stderr @@ -0,0 +1,39 @@ +error: Undefined Behavior: read access through (root of the allocation) is forbidden + --> $DIR/return_pointer_aliasing.rs:LL:CC + | +LL | unsafe { ptr.read() }; + | ^^^^^^^^^^ read access through (root of the allocation) is forbidden + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental + = help: the accessed tag (root of the allocation) is foreign to the protected tag (i.e., it is not a child) + = help: this foreign read access would cause the protected tag to transition from Active to Frozen + = help: this transition would be a loss of write permissions, which is not allowed for protected tags +help: the accessed tag was created here + --> $DIR/return_pointer_aliasing.rs:LL:CC + | +LL | / mir! { +LL | | { +LL | | let x = 0; +LL | | let ptr = &raw mut x; +... | +LL | | } +LL | | } + | |_____^ +help: the protected tag was created here, in the initial state Active + --> $DIR/return_pointer_aliasing.rs:LL:CC + | +LL | unsafe { ptr.read() }; + | ^^^^^^^^^^^^^^^^^^^^^ + = note: BACKTRACE (of the first span): + = note: inside `myfun` at $DIR/return_pointer_aliasing.rs:LL:CC +note: inside `main` + --> $DIR/return_pointer_aliasing.rs:LL:CC + | +LL | Call(*ptr, after_call, myfun(ptr)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in the macro `::core::intrinsics::mir::__internal_remove_let` which comes from the expansion of the macro `mir` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/pass/function_calls/return_place_on_heap.rs b/src/tools/miri/tests/pass/function_calls/return_place_on_heap.rs new file mode 100644 index 00000000000..dcfebd0f82b --- /dev/null +++ b/src/tools/miri/tests/pass/function_calls/return_place_on_heap.rs @@ -0,0 +1,25 @@ +#![feature(raw_ref_op)] +#![feature(core_intrinsics)] +#![feature(custom_mir)] + +use std::intrinsics::mir::*; + +// Make sure calls with the return place "on the heap" work. +#[custom_mir(dialect = "runtime", phase = "optimized")] +pub fn main() { + mir! { + { + let x = 0; + let ptr = &raw mut x; + Call(*ptr, after_call, myfun()) + } + + after_call = { + Return() + } + } +} + +fn myfun() -> i32 { + 13 +} -- cgit 1.4.1-3-g733a5 From 95392ef0c92dabdb2baf7b91f7f50e9866444275 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 10 Jul 2023 22:14:28 +0200 Subject: miri tree borrows: skip retag_reference early if there is no NewPermission --- .../miri/src/borrow_tracker/tree_borrows/mod.rs | 28 ++++++++++++---------- 1 file changed, 15 insertions(+), 13 deletions(-) (limited to 'src/tools') diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index a3f0310f1df..274a4a0aaba 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -178,7 +178,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' &mut self, place: &MPlaceTy<'tcx, Provenance>, // parent tag extracted from here ptr_size: Size, - new_perm: Option, + new_perm: NewPermission, new_tag: BorTag, ) -> InterpResult<'tcx, Option<(AllocId, BorTag)>> { let this = self.eval_context_mut(); @@ -256,10 +256,6 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' ptr_size.bytes() ); - let Some(new_perm) = new_perm else { - return Ok(Some((alloc_id, orig_tag))); - }; - if let Some(protect) = new_perm.protector { // We register the protection in two different places. // This makes creating a protector slower, but checking whether a tag @@ -305,7 +301,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' fn tb_retag_reference( &mut self, val: &ImmTy<'tcx, Provenance>, - new_perm: Option, + new_perm: NewPermission, ) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> { let this = self.eval_context_mut(); // We want a place for where the ptr *points to*, so we get one. @@ -317,7 +313,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' // - if the pointer is not reborrowed (raw pointer) or if `zero_size` is set // then we override the size to do a zero-length reborrow. let reborrow_size = match new_perm { - Some(NewPermission { zero_size: false, .. }) => + NewPermission { zero_size: false, .. } => this.size_and_align_of_mplace(&place)? .map(|(size, _)| size) .unwrap_or(place.layout.size), @@ -374,7 +370,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { NewPermission::from_ref_ty(pointee, mutability, kind, this), _ => None, }; - this.tb_retag_reference(val, new_perm) + if let Some(new_perm) = new_perm { + this.tb_retag_reference(val, new_perm) + } else { + Ok(val.clone()) + } } /// Retag all pointers that are stored in this place. @@ -405,9 +405,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { place: &PlaceTy<'tcx, Provenance>, new_perm: Option, ) -> InterpResult<'tcx> { - let val = self.ecx.read_immediate(&self.ecx.place_to_op(place)?)?; - let val = self.ecx.tb_retag_reference(&val, new_perm)?; - self.ecx.write_immediate(*val, place)?; + if let Some(new_perm) = new_perm { + let val = self.ecx.read_immediate(&self.ecx.place_to_op(place)?)?; + let val = self.ecx.tb_retag_reference(&val, new_perm)?; + self.ecx.write_immediate(*val, place)?; + } Ok(()) } } @@ -505,11 +507,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let ptr_layout = this.layout_of(Ty::new_mut_ptr(this.tcx.tcx, place.layout.ty))?; let ptr = ImmTy::from_immediate(place.to_ref(this), ptr_layout); // Reborrow it. With protection! That is the entire point. - let new_perm = Some(NewPermission { + let new_perm = NewPermission { initial_state: Permission::new_active(), zero_size: false, protector: Some(ProtectorKind::StrongProtector), - }); + }; let _new_ptr = this.tb_retag_reference(&ptr, new_perm)?; // We just throw away `new_ptr`, so nobody can access this memory while it is protected. -- cgit 1.4.1-3-g733a5 From e7c6db7d44b8627b9b3b1106fc08413c52fbabaa Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 11 Jul 2023 15:50:11 +0200 Subject: fix handling of alignment for dyn-sized places --- compiler/rustc_const_eval/src/interpret/place.rs | 10 ++++++---- src/tools/miri/tests/fail/unaligned_pointers/dyn_alignment.rs | 7 ++++--- .../miri/tests/fail/unaligned_pointers/dyn_alignment.stderr | 4 ++-- 3 files changed, 12 insertions(+), 9 deletions(-) (limited to 'src/tools') diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs index f1c777a077b..c2fb61753fe 100644 --- a/compiler/rustc_const_eval/src/interpret/place.rs +++ b/compiler/rustc_const_eval/src/interpret/place.rs @@ -328,7 +328,8 @@ where }; let mplace = MemPlace { ptr: ptr.to_pointer(self)?, meta }; - // When deref'ing a pointer, the *static* alignment given by the type is what matters. + // `ref_to_mplace` is called on raw pointers even if they don't actually get dereferenced; + // we hence can't call `size_and_align_of` since that asserts more validity than we want. let align = layout.align.abi; Ok(MPlaceTy { mplace, layout, align }) } @@ -379,11 +380,12 @@ where /// Check if this mplace is dereferenceable and sufficiently aligned. pub fn check_mplace(&self, mplace: MPlaceTy<'tcx, M::Provenance>) -> InterpResult<'tcx> { - let (size, align) = self + let (size, _align) = self .size_and_align_of_mplace(&mplace)? .unwrap_or((mplace.layout.size, mplace.layout.align.abi)); - assert!(mplace.align <= align, "dynamic alignment less strict than static one?"); - let align = if M::enforce_alignment(self).should_check() { align } else { Align::ONE }; + // Due to packed places, only `mplace.align` matters. + let align = + if M::enforce_alignment(self).should_check() { mplace.align } else { Align::ONE }; self.check_ptr_access_align(mplace.ptr, size, align, CheckInAllocMsg::DerefTest)?; Ok(()) } diff --git a/src/tools/miri/tests/fail/unaligned_pointers/dyn_alignment.rs b/src/tools/miri/tests/fail/unaligned_pointers/dyn_alignment.rs index 6d31ded75c6..555aa57de30 100644 --- a/src/tools/miri/tests/fail/unaligned_pointers/dyn_alignment.rs +++ b/src/tools/miri/tests/fail/unaligned_pointers/dyn_alignment.rs @@ -1,5 +1,6 @@ -// should find the bug even without validation and stacked borrows, but gets masked by optimizations -//@compile-flags: -Zmiri-disable-validation -Zmiri-disable-stacked-borrows -Zmir-opt-level=0 -Cdebug-assertions=no +// should find the bug even without, but gets masked by optimizations +//@compile-flags: -Zmiri-disable-stacked-borrows -Zmir-opt-level=0 -Cdebug-assertions=no +//@normalize-stderr-test: "but found [0-9]+" -> "but found $$ALIGN" #[repr(align(256))] #[derive(Debug)] @@ -19,6 +20,6 @@ fn main() { (&mut ptr as *mut _ as *mut *const u8).write(&buf as *const _ as *const u8); } // Re-borrow that. This should be UB. - let _ptr = &*ptr; //~ERROR: alignment 256 is required + let _ptr = &*ptr; //~ERROR: required 256 byte alignment } } diff --git a/src/tools/miri/tests/fail/unaligned_pointers/dyn_alignment.stderr b/src/tools/miri/tests/fail/unaligned_pointers/dyn_alignment.stderr index a900b46612b..503721b9551 100644 --- a/src/tools/miri/tests/fail/unaligned_pointers/dyn_alignment.stderr +++ b/src/tools/miri/tests/fail/unaligned_pointers/dyn_alignment.stderr @@ -1,8 +1,8 @@ -error: Undefined Behavior: accessing memory with alignment ALIGN, but alignment ALIGN is required +error: Undefined Behavior: constructing invalid value: encountered an unaligned reference (required 256 byte alignment but found $ALIGN) --> $DIR/dyn_alignment.rs:LL:CC | LL | let _ptr = &*ptr; - | ^^^^^ accessing memory with alignment ALIGN, but alignment ALIGN is required + | ^^^^^ constructing invalid value: encountered an unaligned reference (required 256 byte alignment but found $ALIGN) | = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information -- cgit 1.4.1-3-g733a5 From 67b5990472e3cac643d8cf90f45fe42201ddec3c Mon Sep 17 00:00:00 2001 From: jyn Date: Mon, 10 Jul 2023 15:59:30 -0500 Subject: Revert "Fix `x test lint-docs` when download-rustc is enabled" This was not the correct fix. The problem was two-fold: - `download-rustc` didn't respect `llvm.assertions` - `rust-dev` was missing a bump to `download-ci-llvm-stamp` The first is fixed in this PR and the latter was fixed a while ago. Revert this change to avoid breaking `rpath = false`. --- src/tools/lint-docs/src/groups.rs | 1 - src/tools/lint-docs/src/lib.rs | 6 ------ 2 files changed, 7 deletions(-) (limited to 'src/tools') diff --git a/src/tools/lint-docs/src/groups.rs b/src/tools/lint-docs/src/groups.rs index b11fb287cf4..5be8ef7996b 100644 --- a/src/tools/lint-docs/src/groups.rs +++ b/src/tools/lint-docs/src/groups.rs @@ -39,7 +39,6 @@ impl<'a> LintExtractor<'a> { fn collect_groups(&self) -> Result> { let mut result = BTreeMap::new(); let mut cmd = Command::new(self.rustc_path); - cmd.env_remove("LD_LIBRARY_PATH"); cmd.arg("-Whelp"); let output = cmd.output().map_err(|e| format!("failed to run command {:?}\n{}", cmd, e))?; if !output.status.success() { diff --git a/src/tools/lint-docs/src/lib.rs b/src/tools/lint-docs/src/lib.rs index fe29b9abda3..b7c8b9ed2e3 100644 --- a/src/tools/lint-docs/src/lib.rs +++ b/src/tools/lint-docs/src/lib.rs @@ -403,12 +403,6 @@ impl<'a> LintExtractor<'a> { fs::write(&tempfile, source) .map_err(|e| format!("failed to write {}: {}", tempfile.display(), e))?; let mut cmd = Command::new(self.rustc_path); - // NOTE: bootstrap sets `LD_LIBRARY_PATH` for building lint-docs itself. - // Unfortunately, lint-docs is a bootstrap tool while rustc is built from source, - // and sometimes the paths conflict. In particular, when using `download-rustc`, - // the LLVM versions can differ between `ci-llvm` and `ci-rustc-sysroot`. - // Unset LD_LIBRARY_PATH here so it doesn't interfere with running the compiler. - cmd.env_remove("LD_LIBRARY_PATH"); if options.contains(&"edition2015") { cmd.arg("--edition=2015"); } else { -- cgit 1.4.1-3-g733a5 From fb5efd7008c698a821e9c50c3f3a04d48abd8295 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Wed, 12 Jul 2023 06:59:57 -0700 Subject: Ignore flaky clippy tests. --- src/tools/clippy/tests/ui/empty_line_after_doc_comments.rs | 3 +++ src/tools/clippy/tests/ui/empty_line_after_outer_attribute.rs | 3 +++ src/tools/clippy/tests/ui/needless_arbitrary_self_type_unfixable.rs | 3 +++ 3 files changed, 9 insertions(+) (limited to 'src/tools') diff --git a/src/tools/clippy/tests/ui/empty_line_after_doc_comments.rs b/src/tools/clippy/tests/ui/empty_line_after_doc_comments.rs index 83db2a07d33..cc36ce5f487 100644 --- a/src/tools/clippy/tests/ui/empty_line_after_doc_comments.rs +++ b/src/tools/clippy/tests/ui/empty_line_after_doc_comments.rs @@ -1,4 +1,7 @@ //@aux-build:proc_macro_attr.rs:proc-macro +// Flaky test, see https://github.com/rust-lang/rust/issues/113585. +//@ignore-32bit +//@ignore-64bit #![warn(clippy::empty_line_after_doc_comments)] #![allow(clippy::assertions_on_constants)] #![feature(custom_inner_attributes)] diff --git a/src/tools/clippy/tests/ui/empty_line_after_outer_attribute.rs b/src/tools/clippy/tests/ui/empty_line_after_outer_attribute.rs index b2d7ddae427..bc54e0fd2de 100644 --- a/src/tools/clippy/tests/ui/empty_line_after_outer_attribute.rs +++ b/src/tools/clippy/tests/ui/empty_line_after_outer_attribute.rs @@ -1,4 +1,7 @@ //@aux-build:proc_macro_attr.rs:proc-macro +// Flaky test, see https://github.com/rust-lang/rust/issues/113585. +//@ignore-32bit +//@ignore-64bit #![warn(clippy::empty_line_after_outer_attr)] #![allow(clippy::assertions_on_constants)] #![feature(custom_inner_attributes)] diff --git a/src/tools/clippy/tests/ui/needless_arbitrary_self_type_unfixable.rs b/src/tools/clippy/tests/ui/needless_arbitrary_self_type_unfixable.rs index 876f16a3854..321aa69a1a5 100644 --- a/src/tools/clippy/tests/ui/needless_arbitrary_self_type_unfixable.rs +++ b/src/tools/clippy/tests/ui/needless_arbitrary_self_type_unfixable.rs @@ -1,4 +1,7 @@ //@aux-build:proc_macro_attr.rs:proc-macro +// Flaky test, see https://github.com/rust-lang/rust/issues/113585. +//@ignore-32bit +//@ignore-64bit #![warn(clippy::needless_arbitrary_self_type)] -- cgit 1.4.1-3-g733a5