Selectively disable sanitizer instrumentation

tmiasko · tmiasko · commit b846b42c8dcf · 2020-02-05T23:30:38.000+01:00
Add `no_sanitize` attribute that allows to opt out from sanitizer
instrumentation in an annotated function.
diff --git a/src/doc/unstable-book/src/language-features/no-sanitize.md b/src/doc/unstable-book/src/language-features/no-sanitize.md
@@ -0,0 +1,29 @@
+# `no_sanitize`
+
+The tracking issue for this feature is: [#39699]
+
+[#39699]: https://github.com/rust-lang/rust/issues/39699
+
+------------------------
+
+The `no_sanitize` attribute can be used to selectively disable sanitizer
+instrumentation in an annotated function. This might be useful to: avoid
+instrumentation overhead in a performance critical function, or avoid
+instrumenting code that contains constructs unsupported by given sanitizer.
+
+The precise effect of this annotation depends on particular sanitizer in use.
+For example, with `no_sanitize(thread)`, the thread sanitizer will no longer
+instrument non-atomic store / load operations, but it will instrument atomic
+operations to avoid reporting false positives and provide meaning full stack
+traces.
+
+## Examples
+
+``` rust
+#![feature(no_sanitize)]
+
+#[no_sanitize(address)]
+fn foo() {
+  // ...
+}
+```
diff --git a/src/librustc/middle/codegen_fn_attrs.rs b/src/librustc/middle/codegen_fn_attrs.rs
@@ -72,6 +72,12 @@ bitflags! {
         const FFI_RETURNS_TWICE         = 1 << 10;
         /// `#[track_caller]`: allow access to the caller location
         const TRACK_CALLER              = 1 << 11;
+        /// `#[no_sanitize(address)]`: disables address sanitizer instrumentation
+        const NO_SANITIZE_ADDRESS = 1 << 12;
+        /// `#[no_sanitize(memory)]`: disables memory sanitizer instrumentation
+        const NO_SANITIZE_MEMORY  = 1 << 13;
+        /// `#[no_sanitize(thread)]`: disables thread sanitizer instrumentation
+        const NO_SANITIZE_THREAD  = 1 << 14;
     }
 }
 
diff --git a/src/librustc_codegen_llvm/attributes.rs b/src/librustc_codegen_llvm/attributes.rs
@@ -288,6 +288,26 @@ pub fn from_fn_attrs(
     if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::ALLOCATOR) {
         Attribute::NoAlias.apply_llfn(llvm::AttributePlace::ReturnValue, llfn);
     }
+    if let Some(ref sanitizer) = cx.tcx.sess.opts.debugging_opts.sanitizer {
+        match *sanitizer {
+            Sanitizer::Address => {
+                if !codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_SANITIZE_ADDRESS) {
+                    llvm::Attribute::SanitizeAddress.apply_llfn(Function, llfn);
+                }
+            }
+            Sanitizer::Memory => {
+                if !codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_SANITIZE_MEMORY) {
+                    llvm::Attribute::SanitizeMemory.apply_llfn(Function, llfn);
+                }
+            }
+            Sanitizer::Thread => {
+                if !codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_SANITIZE_THREAD) {
+                    llvm::Attribute::SanitizeThread.apply_llfn(Function, llfn);
+                }
+            }
+            Sanitizer::Leak => {}
+        }
+    }
 
     unwind(
         llfn,
diff --git a/src/librustc_codegen_llvm/declare.rs b/src/librustc_codegen_llvm/declare.rs
@@ -19,7 +19,6 @@ use crate::llvm::AttributePlace::Function;
 use crate::type_::Type;
 use crate::value::Value;
 use log::debug;
-use rustc::session::config::Sanitizer;
 use rustc::ty::Ty;
 use rustc_codegen_ssa::traits::*;
 use rustc_data_structures::small_c_str::SmallCStr;
@@ -47,21 +46,6 @@ fn declare_raw_fn(
         llvm::Attribute::NoRedZone.apply_llfn(Function, llfn);
     }
 
-    if let Some(ref sanitizer) = cx.tcx.sess.opts.debugging_opts.sanitizer {
-        match *sanitizer {
-            Sanitizer::Address => {
-                llvm::Attribute::SanitizeAddress.apply_llfn(Function, llfn);
-            }
-            Sanitizer::Memory => {
-                llvm::Attribute::SanitizeMemory.apply_llfn(Function, llfn);
-            }
-            Sanitizer::Thread => {
-                llvm::Attribute::SanitizeThread.apply_llfn(Function, llfn);
-            }
-            _ => {}
-        }
-    }
-
     attributes::default_optimisation_attrs(cx.tcx.sess, llfn);
     attributes::non_lazy_bind(cx.sess(), llfn);
     llfn
diff --git a/src/librustc_feature/active.rs b/src/librustc_feature/active.rs
@@ -541,6 +541,9 @@ declare_features! (
     /// Allows `T: ?const Trait` syntax in bounds.
     (active, const_trait_bound_opt_out, "1.42.0", Some(67794), None),
 
+    /// Allows the use of `no_sanitize` attribute.
+    (active, no_sanitize, "1.42.0", Some(39699), None),
+
     // -------------------------------------------------------------------------
     // feature-group-end: actual feature gates
     // -------------------------------------------------------------------------
diff --git a/src/librustc_feature/builtin_attrs.rs b/src/librustc_feature/builtin_attrs.rs
@@ -261,6 +261,11 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
     ungated!(cold, Whitelisted, template!(Word)),
     ungated!(no_builtins, Whitelisted, template!(Word)),
     ungated!(target_feature, Whitelisted, template!(List: r#"enable = "name""#)),
+    gated!(
+        no_sanitize, Whitelisted,
+        template!(List: "address, memory, thread"),
+        experimental!(no_sanitize)
+    ),
 
     // FIXME: #14408 whitelist docs since rustdoc looks at them
     ungated!(doc, Whitelisted, template!(List: "hidden|inline|...", NameValueStr: "string")),
diff --git a/src/librustc_mir/transform/inline.rs b/src/librustc_mir/transform/inline.rs
@@ -8,6 +8,7 @@ use rustc_index::vec::{Idx, IndexVec};
 use rustc::middle::codegen_fn_attrs::CodegenFnAttrFlags;
 use rustc::mir::visit::*;
 use rustc::mir::*;
+use rustc::session::config::Sanitizer;
 use rustc::ty::subst::{InternalSubsts, Subst, SubstsRef};
 use rustc::ty::{self, Instance, InstanceDef, ParamEnv, Ty, TyCtxt, TypeFoldable};
 
@@ -228,6 +229,28 @@ impl Inliner<'tcx> {
             return false;
         }
 
+        // Avoid inlining functions marked as no_sanitize if sanitizer is enabled,
+        // since instrumentation might be enabled and performed on the caller.
+        match self.tcx.sess.opts.debugging_opts.sanitizer {
+            Some(Sanitizer::Address) => {
+                if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_SANITIZE_ADDRESS) {
+                    return false;
+                }
+            }
+            Some(Sanitizer::Memory) => {
+                if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_SANITIZE_MEMORY) {
+                    return false;
+                }
+            }
+            Some(Sanitizer::Thread) => {
+                if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::NO_SANITIZE_THREAD) {
+                    return false;
+                }
+            }
+            Some(Sanitizer::Leak) => {}
+            None => {}
+        }
+
         let hinted = match codegen_fn_attrs.inline {
             // Just treat inline(always) as a hint for now,
             // there are cases that prevent inlining that we
diff --git a/src/librustc_session/lint/builtin.rs b/src/librustc_session/lint/builtin.rs
@@ -474,6 +474,12 @@ declare_lint! {
     };
 }
 
+declare_lint! {
+    pub INLINE_NO_SANITIZE,
+    Warn,
+    "detects incompatible use of `#[inline(always)]` and `#[no_sanitize(...)]`",
+}
+
 declare_lint_pass! {
     /// Does nothing as a lint pass, but registers some `Lint`s
     /// that are used by other parts of the compiler.
@@ -537,5 +543,6 @@ declare_lint_pass! {
         MUTABLE_BORROW_RESERVATION_CONFLICT,
         INDIRECT_STRUCTURAL_MATCH,
         SOFT_UNSTABLE,
+        INLINE_NO_SANITIZE,
     ]
 }
diff --git a/src/librustc_span/symbol.rs b/src/librustc_span/symbol.rs
@@ -120,6 +120,7 @@ symbols! {
         abi_vectorcall,
         abi_x86_interrupt,
         aborts,
+        address,
         add_with_overflow,
         advanced_slice_patterns,
         adx_target_feature,
@@ -445,6 +446,7 @@ symbols! {
         mem_uninitialized,
         mem_zeroed,
         member_constraints,
+        memory,
         message,
         meta,
         min_align_of,
@@ -487,6 +489,7 @@ symbols! {
         None,
         non_exhaustive,
         non_modrs_mods,
+        no_sanitize,
         no_stack_check,
         no_start,
         no_std,
@@ -721,6 +724,7 @@ symbols! {
         test_removed_feature,
         test_runner,
         then_with,
+        thread,
         thread_local,
         tool_attributes,
         tool_lints,
diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs
@@ -2743,6 +2743,7 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs {
 
     let mut inline_span = None;
     let mut link_ordinal_span = None;
+    let mut no_sanitize_span = None;
     for attr in attrs.iter() {
         if attr.check_name(sym::cold) {
             codegen_fn_attrs.flags |= CodegenFnAttrFlags::COLD;
@@ -2832,6 +2833,24 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs {
             if let ordinal @ Some(_) = check_link_ordinal(tcx, attr) {
                 codegen_fn_attrs.link_ordinal = ordinal;
             }
+        } else if attr.check_name(sym::no_sanitize) {
+            no_sanitize_span = Some(attr.span);
+            if let Some(list) = attr.meta_item_list() {
+                for item in list.iter() {
+                    if item.check_name(sym::address) {
+                        codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_SANITIZE_ADDRESS;
+                    } else if item.check_name(sym::memory) {
+                        codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_SANITIZE_MEMORY;
+                    } else if item.check_name(sym::thread) {
+                        codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_SANITIZE_THREAD;
+                    } else {
+                        tcx.sess
+                            .struct_span_err(item.span(), "invalid argument for `no_sanitize`")
+                            .note("expected one of: `address`, `memory` or `thread`")
+                            .emit();
+                    }
+                }
+            }
         }
     }
 
@@ -2911,7 +2930,6 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs {
     // purpose functions as they wouldn't have the right target features
     // enabled. For that reason we also forbid #[inline(always)] as it can't be
     // respected.
-
     if codegen_fn_attrs.target_features.len() > 0 {
         if codegen_fn_attrs.inline == InlineAttr::Always {
             if let Some(span) = inline_span {
@@ -2924,6 +2942,25 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs {
         }
     }
 
+    let no_sanitize_flags = CodegenFnAttrFlags::NO_SANITIZE_ADDRESS
+        | CodegenFnAttrFlags::NO_SANITIZE_MEMORY
+        | CodegenFnAttrFlags::NO_SANITIZE_THREAD;
+    if codegen_fn_attrs.flags.intersects(no_sanitize_flags) {
+        if codegen_fn_attrs.inline == InlineAttr::Always {
+            if let (Some(no_sanitize_span), Some(inline_span)) = (no_sanitize_span, inline_span) {
+                let hir_id = tcx.hir().as_local_hir_id(id).unwrap();
+                tcx.struct_span_lint_hir(
+                    lint::builtin::INLINE_NO_SANITIZE,
+                    hir_id,
+                    no_sanitize_span,
+                    "`no_sanitize` will have no effect after inlining",
+                )
+                .span_note(inline_span, "inlining requested here")
+                .emit();
+            }
+        }
+    }
+
     // Weak lang items have the same semantics as "std internal" symbols in the
     // sense that they're preserved through all our LTO passes and only
     // strippable by the linker.
diff --git a/src/test/codegen/sanitizer-no-sanitize-inlining.rs b/src/test/codegen/sanitizer-no-sanitize-inlining.rs
@@ -0,0 +1,32 @@
+// Verifies that no_sanitize attribute prevents inlining when
+// given sanitizer is enabled, but has no effect on inlining otherwise.
+//
+// needs-sanitizer-support
+// only-x86_64
+//
+// revisions: ASAN LSAN
+//
+//[ASAN] compile-flags: -Zsanitizer=address -C opt-level=3 -Z mir-opt-level=3
+//[LSAN] compile-flags: -Zsanitizer=leak    -C opt-level=3 -Z mir-opt-level=3
+
+#![crate_type="lib"]
+#![feature(no_sanitize)]
+
+// ASAN-LABEL: define void @test
+// ASAN:         tail call fastcc void @random_inline
+// ASAN:       }
+//
+// LSAN-LABEL: define void @test
+// LSAN-NO:      call
+// LSAN:       }
+#[no_mangle]
+pub fn test(n: &mut u32) {
+    random_inline(n);
+}
+
+#[no_sanitize(address)]
+#[inline]
+#[no_mangle]
+pub fn random_inline(n: &mut u32) {
+    *n = 42;
+}
diff --git a/src/test/codegen/sanitizer-no-sanitize.rs b/src/test/codegen/sanitizer-no-sanitize.rs
@@ -0,0 +1,29 @@
+// Verifies that no_sanitze attribute can be used to
+// selectively disable sanitizer instrumentation.
+//
+// needs-sanitizer-support
+// compile-flags: -Zsanitizer=address
+
+#![crate_type="lib"]
+#![feature(no_sanitize)]
+
+// CHECK-LABEL: ; sanitizer_no_sanitize::unsanitized
+// CHECK-NEXT:  ; Function Attrs:
+// CHECK-NOT:   sanitize_address
+// CHECK:       start:
+// CHECK-NOT:   call void @__asan_report_load
+// CHECK:       }
+#[no_sanitize(address)]
+pub fn unsanitized(b: &mut u8) -> u8 {
+    *b
+}
+
+// CHECK-LABEL: ; sanitizer_no_sanitize::sanitized
+// CHECK-NEXT:  ; Function Attrs:
+// CHECK:       sanitize_address
+// CHECK:       start:
+// CHECK:       call void @__asan_report_load
+// CHECK:       }
+pub fn sanitized(b: &mut u8) -> u8 {
+    *b
+}
diff --git a/src/test/ui/feature-gates/feature-gate-no_sanitize.rs b/src/test/ui/feature-gates/feature-gate-no_sanitize.rs
@@ -0,0 +1,4 @@
+#[no_sanitize(address)]
+//~^ the `#[no_sanitize]` attribute is an experimental feature
+fn main() {
+}
diff --git a/src/test/ui/feature-gates/feature-gate-no_sanitize.stderr b/src/test/ui/feature-gates/feature-gate-no_sanitize.stderr
@@ -0,0 +1,12 @@
+error[E0658]: the `#[no_sanitize]` attribute is an experimental feature
+  --> $DIR/feature-gate-no_sanitize.rs:1:1
+   |
+LL | #[no_sanitize(address)]
+   | ^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: for more information, see https://github.com/rust-lang/rust/issues/39699
+   = help: add `#![feature(no_sanitize)]` to the crate attributes to enable
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0658`.
diff --git a/src/test/ui/invalid/invalid-no-sanitize.rs b/src/test/ui/invalid/invalid-no-sanitize.rs
@@ -0,0 +1,5 @@
+#![feature(no_sanitize)]
+
+#[no_sanitize(brontosaurus)] //~ ERROR invalid argument
+fn main() {
+}
diff --git a/src/test/ui/invalid/invalid-no-sanitize.stderr b/src/test/ui/invalid/invalid-no-sanitize.stderr
@@ -0,0 +1,10 @@
+error: invalid argument for `no_sanitize`
+  --> $DIR/invalid-no-sanitize.rs:3:15
+   |
+LL | #[no_sanitize(brontosaurus)]
+   |               ^^^^^^^^^^^^
+   |
+   = note: expected one of: `address`, `memory` or `thread`
+
+error: aborting due to previous error
+
diff --git a/src/test/ui/sanitize-inline-always.rs b/src/test/ui/sanitize-inline-always.rs
@@ -0,0 +1,15 @@
+// check-pass
+
+#![feature(no_sanitize)]
+
+#[inline(always)]
+//~^ NOTE inlining requested here
+#[no_sanitize(address)]
+//~^ WARN will have no effect after inlining
+//~| NOTE on by default
+fn x() {
+}
+
+fn main() {
+    x()
+}
diff --git a/src/test/ui/sanitize-inline-always.stderr b/src/test/ui/sanitize-inline-always.stderr

Original file line number	Diff line number	Diff line change
`@@ -72,6 +72,12 @@ bitflags! {`
`72`	`72`	`const FFI_RETURNS_TWICE = 1 << 10;`
`73`	`73`	/// `#[track_caller]`: allow access to the caller location
`74`	`74`	`const TRACK_CALLER = 1 << 11;`
	`75`	+ /// `#[no_sanitize(address)]`: disables address sanitizer instrumentation
	`76`	`+ const NO_SANITIZE_ADDRESS = 1 << 12;`
	`77`	+ /// `#[no_sanitize(memory)]`: disables memory sanitizer instrumentation
	`78`	`+ const NO_SANITIZE_MEMORY = 1 << 13;`
	`79`	+ /// `#[no_sanitize(thread)]`: disables thread sanitizer instrumentation
	`80`	`+ const NO_SANITIZE_THREAD = 1 << 14;`
`75`	`81`	`}`
`76`	`82`	`}`
`77`	`83`
Original file line number	Diff line number	Diff line change
`@@ -474,6 +474,12 @@ declare_lint! {`
`474`	`474`	`};`
`475`	`475`	`}`
`476`	`476`
	`477`	`+declare_lint! {`
	`478`	`+ pub INLINE_NO_SANITIZE,`
	`479`	`+ Warn,`
	`480`	+ "detects incompatible use of `#[inline(always)]` and `#[no_sanitize(...)]`",
	`481`	`+}`
	`482`	`+`
`477`	`483`	`declare_lint_pass! {`
`478`	`484`	/// Does nothing as a lint pass, but registers some `Lint`s
`479`	`485`	`/// that are used by other parts of the compiler.`
`@@ -537,5 +543,6 @@ declare_lint_pass! {`
`537`	`543`	`MUTABLE_BORROW_RESERVATION_CONFLICT,`
`538`	`544`	`INDIRECT_STRUCTURAL_MATCH,`
`539`	`545`	`SOFT_UNSTABLE,`
	`546`	`+ INLINE_NO_SANITIZE,`
`540`	`547`	`]`
`541`	`548`	`}`