Skip to content

Selectively disable sanitizer instrumentation #68164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions src/doc/unstable-book/src/language-features/no-sanitize.md
Original file line number Diff line number Diff line change
@@ -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() {
// ...
}
```
8 changes: 8 additions & 0 deletions src/librustc/middle/codegen_fn_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ 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;
/// All `#[no_sanitize(...)]` attributes.
const NO_SANITIZE_ANY = Self::NO_SANITIZE_ADDRESS.bits | Self::NO_SANITIZE_MEMORY.bits | Self::NO_SANITIZE_THREAD.bits;
}
}

Expand Down
26 changes: 26 additions & 0 deletions src/librustc_codegen_llvm/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,31 @@ fn inline(cx: &CodegenCx<'ll, '_>, val: &'ll Value, inline: InlineAttr) {
};
}

/// Apply LLVM sanitize attributes.
#[inline]
pub fn sanitize(cx: &CodegenCx<'ll, '_>, codegen_fn_flags: CodegenFnAttrFlags, llfn: &'ll Value) {
if let Some(ref sanitizer) = cx.tcx.sess.opts.debugging_opts.sanitizer {
match *sanitizer {
Sanitizer::Address => {
if !codegen_fn_flags.contains(CodegenFnAttrFlags::NO_SANITIZE_ADDRESS) {
llvm::Attribute::SanitizeAddress.apply_llfn(Function, llfn);
}
}
Sanitizer::Memory => {
if !codegen_fn_flags.contains(CodegenFnAttrFlags::NO_SANITIZE_MEMORY) {
llvm::Attribute::SanitizeMemory.apply_llfn(Function, llfn);
}
}
Sanitizer::Thread => {
if !codegen_fn_flags.contains(CodegenFnAttrFlags::NO_SANITIZE_THREAD) {
llvm::Attribute::SanitizeThread.apply_llfn(Function, llfn);
}
}
Sanitizer::Leak => {}
}
}
}

/// Tell LLVM to emit or not emit the information necessary to unwind the stack for the function.
#[inline]
pub fn emit_uwtable(val: &'ll Value, emit: bool) {
Expand Down Expand Up @@ -288,6 +313,7 @@ pub fn from_fn_attrs(
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::ALLOCATOR) {
Attribute::NoAlias.apply_llfn(llvm::AttributePlace::ReturnValue, llfn);
}
sanitize(cx, codegen_fn_attrs.flags, llfn);

unwind(
llfn,
Expand Down
7 changes: 5 additions & 2 deletions src/librustc_codegen_llvm/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

use super::ModuleLlvm;

use crate::attributes;
use crate::builder::Builder;
use crate::common;
use crate::context::CodegenCx;
Expand All @@ -23,7 +24,7 @@ use crate::metadata;
use crate::value::Value;

use rustc::dep_graph;
use rustc::middle::codegen_fn_attrs::CodegenFnAttrs;
use rustc::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
use rustc::middle::cstore::EncodedMetadata;
use rustc::middle::exported_symbols;
use rustc::mir::mono::{Linkage, Visibility};
Expand Down Expand Up @@ -131,7 +132,9 @@ pub fn compile_codegen_unit(

// If this codegen unit contains the main function, also create the
// wrapper here
maybe_create_entry_wrapper::<Builder<'_, '_, '_>>(&cx);
if let Some(entry) = maybe_create_entry_wrapper::<Builder<'_, '_, '_>>(&cx) {
attributes::sanitize(&cx, CodegenFnAttrFlags::empty(), entry);
}

// Run replace-all-uses-with for statics that need it
for &(old_g, new_g) in cx.statics_to_rauw().borrow().iter() {
Expand Down
16 changes: 0 additions & 16 deletions src/librustc_codegen_llvm/declare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
22 changes: 12 additions & 10 deletions src/librustc_codegen_ssa/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,36 +391,36 @@ pub fn codegen_instance<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>>(

/// Creates the `main` function which will initialize the rust runtime and call
/// users main function.
pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(cx: &'a Bx::CodegenCx) {
pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
cx: &'a Bx::CodegenCx,
) -> Option<Bx::Function> {
let (main_def_id, span) = match cx.tcx().entry_fn(LOCAL_CRATE) {
Some((def_id, _)) => (def_id, cx.tcx().def_span(def_id)),
None => return,
None => return None,
};

let instance = Instance::mono(cx.tcx(), main_def_id);

if !cx.codegen_unit().contains_item(&MonoItem::Fn(instance)) {
// We want to create the wrapper in the same codegen unit as Rust's main
// function.
return;
return None;
}

let main_llfn = cx.get_fn_addr(instance);

let et = cx.tcx().entry_fn(LOCAL_CRATE).map(|e| e.1);
match et {
Some(EntryFnType::Main) => create_entry_fn::<Bx>(cx, span, main_llfn, main_def_id, true),
Some(EntryFnType::Start) => create_entry_fn::<Bx>(cx, span, main_llfn, main_def_id, false),
None => {} // Do nothing.
}
return cx.tcx().entry_fn(LOCAL_CRATE).map(|(_, et)| {
let use_start_lang_item = EntryFnType::Start != et;
create_entry_fn::<Bx>(cx, span, main_llfn, main_def_id, use_start_lang_item)
});

fn create_entry_fn<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
cx: &'a Bx::CodegenCx,
sp: Span,
rust_main: Bx::Value,
rust_main_def_id: DefId,
use_start_lang_item: bool,
) {
) -> Bx::Function {
// The entry function is either `int main(void)` or `int main(int argc, char **argv)`,
// depending on whether the target needs `argc` and `argv` to be passed in.
let llfty = if cx.sess().target.target.options.main_needs_argc_argv {
Expand Down Expand Up @@ -481,6 +481,8 @@ pub fn maybe_create_entry_wrapper<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(cx: &'
let result = bx.call(start_fn, &args, None);
let cast = bx.intcast(result, cx.type_int(), true);
bx.ret(cast);

llfn
}
}

Expand Down
3 changes: 3 additions & 0 deletions src/librustc_feature/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
// -------------------------------------------------------------------------
Expand Down
5 changes: 5 additions & 0 deletions src/librustc_feature/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")),
Expand Down
23 changes: 23 additions & 0 deletions src/librustc_mir/transform/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions src/librustc_session/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -537,5 +543,6 @@ declare_lint_pass! {
MUTABLE_BORROW_RESERVATION_CONFLICT,
INDIRECT_STRUCTURAL_MATCH,
SOFT_UNSTABLE,
INLINE_NO_SANITIZE,
]
}
4 changes: 4 additions & 0 deletions src/librustc_span/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ symbols! {
abi_vectorcall,
abi_x86_interrupt,
aborts,
address,
add_with_overflow,
advanced_slice_patterns,
adx_target_feature,
Expand Down Expand Up @@ -445,6 +446,7 @@ symbols! {
mem_uninitialized,
mem_zeroed,
member_constraints,
memory,
message,
meta,
min_align_of,
Expand Down Expand Up @@ -487,6 +489,7 @@ symbols! {
None,
non_exhaustive,
non_modrs_mods,
no_sanitize,
no_stack_check,
no_start,
no_std,
Expand Down Expand Up @@ -721,6 +724,7 @@ symbols! {
test_removed_feature,
test_runner,
then_with,
thread,
thread_local,
tool_attributes,
tool_lints,
Expand Down
38 changes: 37 additions & 1 deletion src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// ignore-tidy-filelength

//! "Collection" is the process of determining the type and other external
//! details of each item in Rust. Collection is specifically concerned
//! with *inter-procedural* things -- for example, for a function
Expand Down Expand Up @@ -2743,6 +2745,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;
Expand Down Expand Up @@ -2832,6 +2835,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();
}
}
}
Comment on lines +2840 to +2855
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to extract this to a function.

}
}

Expand Down Expand Up @@ -2911,7 +2932,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 {
Expand All @@ -2924,6 +2944,22 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs {
}
}

if codegen_fn_attrs.flags.intersects(CodegenFnAttrFlags::NO_SANITIZE_ANY) {
if codegen_fn_attrs.inline == InlineAttr::Always {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We issue this lint for #[inline(always)], but not just a plain #[inline]?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rustc and LLVM go out of the way to respect #[inline(always)] attribute.
Even ignoring restrictions that would otherwise prevent inlining.
I didn't want to change that behaviour just for no-sanitize.

On the other hand the #[inline] hint is much weaker. In the case that
no-sanitize attribute is present and sanitizer enabled, it takes the
precedence over inlining.

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.
Expand Down
Loading