From 1fb1643129f9ac6a63bddf04d69e28632615c0df Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Tue, 10 Aug 2021 11:50:33 +0100 Subject: [PATCH] Implement `black_box` using intrinsic The new implementation allows some `memcpy`s to be optimized away, so the uninit value in ui/sanitize/memory.rs is constructed directly onto the return place. Therefore the sanitizer now says that the value is allocated by `main` rather than `random`. --- .../src/intrinsics/mod.rs | 5 ++++ compiler/rustc_codegen_llvm/src/asm.rs | 2 +- compiler/rustc_codegen_llvm/src/intrinsic.rs | 26 +++++++++++++++++++ .../rustc_mir/src/interpret/intrinsics.rs | 2 +- compiler/rustc_span/src/symbol.rs | 1 + compiler/rustc_typeck/src/check/intrinsic.rs | 3 +++ library/core/src/hint.rs | 20 ++++++-------- library/core/src/intrinsics.rs | 6 +++++ src/test/ui/sanitize/memory.rs | 2 +- 9 files changed, 52 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs b/compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs index 86698460747..1c4d307fc50 100644 --- a/compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs +++ b/compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs @@ -1136,6 +1136,11 @@ fn type_by_size(size: Size) -> Option { }; ret.write_cvalue(fx, CValue::by_val(is_eq_value, ret.layout())); }; + + black_box, (c a) { + // FIXME implement black_box semantics + ret.write_cvalue(fx, a); + }; } if let Some((_, dest)) = destination { diff --git a/compiler/rustc_codegen_llvm/src/asm.rs b/compiler/rustc_codegen_llvm/src/asm.rs index ebc3773df57..4790b44bd19 100644 --- a/compiler/rustc_codegen_llvm/src/asm.rs +++ b/compiler/rustc_codegen_llvm/src/asm.rs @@ -425,7 +425,7 @@ fn codegen_global_asm( } } -fn inline_asm_call( +pub(crate) fn inline_asm_call( bx: &mut Builder<'a, 'll, 'tcx>, asm: &str, cons: &str, diff --git a/compiler/rustc_codegen_llvm/src/intrinsic.rs b/compiler/rustc_codegen_llvm/src/intrinsic.rs index ed484185865..fe2ed21c1e3 100644 --- a/compiler/rustc_codegen_llvm/src/intrinsic.rs +++ b/compiler/rustc_codegen_llvm/src/intrinsic.rs @@ -7,6 +7,7 @@ use crate::va_arg::emit_va_arg; use crate::value::Value; +use rustc_ast as ast; use rustc_codegen_ssa::base::{compare_simd_types, wants_msvc_seh}; use rustc_codegen_ssa::common::span_invalid_monomorphization_error; use rustc_codegen_ssa::common::{IntPredicate, TypeKind}; @@ -327,6 +328,31 @@ fn codegen_intrinsic_call( } } + sym::black_box => { + args[0].val.store(self, result); + + // We need to "use" the argument in some way LLVM can't introspect, and on + // targets that support it we can typically leverage inline assembly to do + // this. LLVM's interpretation of inline assembly is that it's, well, a black + // box. This isn't the greatest implementation since it probably deoptimizes + // more than we want, but it's so far good enough. + crate::asm::inline_asm_call( + self, + "", + "r,~{memory}", + &[result.llval], + self.type_void(), + true, + false, + ast::LlvmAsmDialect::Att, + &[span], + ) + .unwrap_or_else(|| bug!("failed to generate inline asm call for `black_box`")); + + // We have copied the value to `result` already. + return; + } + _ if name_str.starts_with("simd_") => { match generic_simd_intrinsic(self, name, callee_ty, args, ret_ty, llret_ty, span) { Ok(llval) => llval, diff --git a/compiler/rustc_mir/src/interpret/intrinsics.rs b/compiler/rustc_mir/src/interpret/intrinsics.rs index dc1f9053b61..bfab886b6ee 100644 --- a/compiler/rustc_mir/src/interpret/intrinsics.rs +++ b/compiler/rustc_mir/src/interpret/intrinsics.rs @@ -465,7 +465,7 @@ pub fn emulate_intrinsic( ); self.copy_op(&self.operand_index(&args[0], index)?, dest)?; } - sym::likely | sym::unlikely => { + sym::likely | sym::unlikely | sym::black_box => { // These just return their argument self.copy_op(&args[0], dest)?; } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 3f5d8273b38..6d03f1a3732 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -335,6 +335,7 @@ bitreverse, bitxor, bitxor_assign, + black_box, block, bool, borrowck_graphviz_format, diff --git a/compiler/rustc_typeck/src/check/intrinsic.rs b/compiler/rustc_typeck/src/check/intrinsic.rs index 6661df21ed9..664954b0eb7 100644 --- a/compiler/rustc_typeck/src/check/intrinsic.rs +++ b/compiler/rustc_typeck/src/check/intrinsic.rs @@ -102,6 +102,7 @@ pub fn intrinsic_operation_unsafety(intrinsic: Symbol) -> hir::Unsafety { | sym::maxnumf64 | sym::type_name | sym::forget + | sym::black_box | sym::variant_count => hir::Unsafety::Normal, _ => hir::Unsafety::Unsafe, } @@ -387,6 +388,8 @@ pub fn check_intrinsic_type(tcx: TyCtxt<'_>, it: &hir::ForeignItem<'_>) { (1, vec![param_ty; 2], tcx.types.bool) } + sym::black_box => (1, vec![param(0)], param(0)), + other => { tcx.sess.emit_err(UnrecognizedIntrinsicFunction { span: it.span, name: other }); return; diff --git a/library/core/src/hint.rs b/library/core/src/hint.rs index a0b65399da2..a4924554919 100644 --- a/library/core/src/hint.rs +++ b/library/core/src/hint.rs @@ -152,23 +152,19 @@ pub fn spin_loop() { /// backend used. Programs cannot rely on `black_box` for *correctness* in any way. /// /// [`std::convert::identity`]: crate::convert::identity -#[cfg_attr(not(miri), inline)] -#[cfg_attr(miri, inline(never))] +#[inline] #[unstable(feature = "bench_black_box", issue = "64102")] -#[cfg_attr(miri, allow(unused_mut))] +#[cfg_attr(not(bootstrap), allow(unused_mut))] pub fn black_box(mut dummy: T) -> T { - // We need to "use" the argument in some way LLVM can't introspect, and on - // targets that support it we can typically leverage inline assembly to do - // this. LLVM's interpretation of inline assembly is that it's, well, a black - // box. This isn't the greatest implementation since it probably deoptimizes - // more than we want, but it's so far good enough. - - #[cfg(not(miri))] // This is just a hint, so it is fine to skip in Miri. + #[cfg(bootstrap)] // SAFETY: the inline assembly is a no-op. unsafe { - // FIXME: Cannot use `asm!` because it doesn't support MIPS and other architectures. llvm_asm!("" : : "r"(&mut dummy) : "memory" : "volatile"); + dummy } - dummy + #[cfg(not(bootstrap))] + { + crate::intrinsics::black_box(dummy) + } } diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index d15ac89668f..272b1e3a1d7 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -1933,6 +1933,12 @@ pub unsafe fn drop_in_place(to_drop: *mut T) { /// which is UB if any of their inputs are `undef`.) #[rustc_const_unstable(feature = "const_intrinsic_raw_eq", issue = "none")] pub fn raw_eq(a: &T, b: &T) -> bool; + + /// See documentation of [`std::hint::black_box`] for details. + /// + /// [`std::hint::black_box`]: crate::hint::black_box + #[cfg(not(bootstrap))] + pub fn black_box(dummy: T) -> T; } // Some functions are defined here because they accidentally got made diff --git a/src/test/ui/sanitize/memory.rs b/src/test/ui/sanitize/memory.rs index 48a482a13aa..b53f19a5b01 100644 --- a/src/test/ui/sanitize/memory.rs +++ b/src/test/ui/sanitize/memory.rs @@ -6,7 +6,7 @@ // run-fail // error-pattern: MemorySanitizer: use-of-uninitialized-value // error-pattern: Uninitialized value was created by an allocation -// error-pattern: in the stack frame of function 'random' +// error-pattern: in the stack frame of function 'main' // // This test case intentionally limits the usage of the std, // since it will be linked with an uninstrumented version of it.