From 204b2281fa508cf512d343cb1631955a5dfa9e0e Mon Sep 17 00:00:00 2001 From: Daniel Bertalan Date: Mon, 8 Jul 2024 15:49:50 +0200 Subject: [PATCH] Allow disabling ASan instrumentation for globals AddressSanitizer adds instrumentation to global variables unless the [`no_sanitize_address`](https://llvm.org/docs/LangRef.html#global-attributes) attribute is set on them. This commit extends the existing `#[no_sanitize(address)]` attribute to set this; previously it only had the desired effect on functions. --- compiler/rustc_codegen_llvm/src/base.rs | 9 +++ compiler/rustc_codegen_llvm/src/consts.rs | 2 + compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 3 + .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 19 ++++++ compiler/rustc_passes/messages.ftl | 4 ++ compiler/rustc_passes/src/check_attr.rs | 37 +++++++++++- compiler/rustc_passes/src/errors.rs | 11 ++++ tests/codegen/sanitizer/no-sanitize.rs | 10 ++++ tests/ui/attributes/no-sanitize.rs | 18 ++++-- tests/ui/attributes/no-sanitize.stderr | 58 +++++++++++-------- 10 files changed, 137 insertions(+), 34 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/base.rs b/compiler/rustc_codegen_llvm/src/base.rs index 3279389479499..f62310bd94808 100644 --- a/compiler/rustc_codegen_llvm/src/base.rs +++ b/compiler/rustc_codegen_llvm/src/base.rs @@ -172,3 +172,12 @@ pub(crate) fn visibility_to_llvm(linkage: Visibility) -> llvm::Visibility { Visibility::Protected => llvm::Visibility::Protected, } } + +pub(crate) fn set_variable_sanitizer_attrs(llval: &Value, attrs: &CodegenFnAttrs) { + if attrs.no_sanitize.contains(SanitizerSet::ADDRESS) { + unsafe { llvm::LLVMRustSetNoSanitizeAddress(llval) }; + } + if attrs.no_sanitize.contains(SanitizerSet::HWADDRESS) { + unsafe { llvm::LLVMRustSetNoSanitizeHWAddress(llval) }; + } +} diff --git a/compiler/rustc_codegen_llvm/src/consts.rs b/compiler/rustc_codegen_llvm/src/consts.rs index 21d996ef460c0..ce9f8a2a9ea98 100644 --- a/compiler/rustc_codegen_llvm/src/consts.rs +++ b/compiler/rustc_codegen_llvm/src/consts.rs @@ -476,6 +476,8 @@ impl<'ll> CodegenCx<'ll, '_> { base::set_link_section(g, attrs); } + base::set_variable_sanitizer_attrs(g, attrs); + if attrs.flags.contains(CodegenFnAttrFlags::USED) { // `USED` and `USED_LINKER` can't be used together. assert!(!attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER)); diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 5fad7583e1aee..2679d8ba9fe8c 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -2474,4 +2474,7 @@ unsafe extern "C" { pub fn LLVMRustIs64BitSymbolicFile(buf_ptr: *const u8, buf_len: usize) -> bool; pub fn LLVMRustIsECObject(buf_ptr: *const u8, buf_len: usize) -> bool; + + pub fn LLVMRustSetNoSanitizeAddress(Global: &Value); + pub fn LLVMRustSetNoSanitizeHWAddress(Global: &Value); } diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index 645b4082be5de..b474a7547dc38 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -1999,6 +1999,25 @@ extern "C" bool LLVMRustLLVMHasZstdCompressionForDebugSymbols() { return llvm::compression::zstd::isAvailable(); } +extern "C" void LLVMRustSetNoSanitizeAddress(LLVMValueRef Global) { + GlobalValue &GV = *unwrap(Global); + GlobalValue::SanitizerMetadata MD; + if (GV.hasSanitizerMetadata()) + MD = GV.getSanitizerMetadata(); + MD.NoAddress = true; + MD.IsDynInit = false; + GV.setSanitizerMetadata(MD); +} + +extern "C" void LLVMRustSetNoSanitizeHWAddress(LLVMValueRef Global) { + GlobalValue &GV = *unwrap(Global); + GlobalValue::SanitizerMetadata MD; + if (GV.hasSanitizerMetadata()) + MD = GV.getSanitizerMetadata(); + MD.NoHWAddress = true; + GV.setSanitizerMetadata(MD); +} + // Operations on composite constants. // These are clones of LLVM api functions that will become available in future // releases. They can be removed once Rust's minimum supported LLVM version diff --git a/compiler/rustc_passes/messages.ftl b/compiler/rustc_passes/messages.ftl index e5e70ba203384..8cd82e6a372cb 100644 --- a/compiler/rustc_passes/messages.ftl +++ b/compiler/rustc_passes/messages.ftl @@ -555,6 +555,10 @@ passes_no_mangle_foreign = passes_no_patterns = patterns not allowed in naked function parameters +passes_no_sanitize = + `#[no_sanitize({$attr_str})]` should be applied to {$accepted_kind} + .label = not {$accepted_kind} + passes_non_exported_macro_invalid_attrs = attribute should be applied to function or closure .label = not a function or closure diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index 0a2926c040447..5edba2cf14742 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -126,9 +126,7 @@ impl<'tcx> CheckAttrVisitor<'tcx> { [sym::inline, ..] => self.check_inline(hir_id, attr, span, target), [sym::coverage, ..] => self.check_coverage(attr, span, target), [sym::optimize, ..] => self.check_optimize(hir_id, attr, span, target), - [sym::no_sanitize, ..] => { - self.check_applied_to_fn_or_method(hir_id, attr, span, target) - } + [sym::no_sanitize, ..] => self.check_no_sanitize(attr, span, target), [sym::non_exhaustive, ..] => self.check_non_exhaustive(hir_id, attr, span, target), [sym::marker, ..] => self.check_marker(hir_id, attr, span, target), [sym::target_feature, ..] => { @@ -450,6 +448,39 @@ impl<'tcx> CheckAttrVisitor<'tcx> { } } + fn check_no_sanitize(&self, attr: &Attribute, span: Span, target: Target) { + if let Some(list) = attr.meta_item_list() { + for item in list.iter() { + let sym = item.name_or_empty(); + match sym { + sym::address | sym::hwaddress => { + let is_valid = + matches!(target, Target::Fn | Target::Method(..) | Target::Static); + if !is_valid { + self.dcx().emit_err(errors::NoSanitize { + attr_span: item.span(), + defn_span: span, + accepted_kind: "a function or static", + attr_str: sym.as_str(), + }); + } + } + _ => { + let is_valid = matches!(target, Target::Fn | Target::Method(..)); + if !is_valid { + self.dcx().emit_err(errors::NoSanitize { + attr_span: item.span(), + defn_span: span, + accepted_kind: "a function", + attr_str: sym.as_str(), + }); + } + } + } + } + } + } + fn check_generic_attr( &self, hir_id: HirId, diff --git a/compiler/rustc_passes/src/errors.rs b/compiler/rustc_passes/src/errors.rs index 8bd767c1243d9..4425155787022 100644 --- a/compiler/rustc_passes/src/errors.rs +++ b/compiler/rustc_passes/src/errors.rs @@ -1848,3 +1848,14 @@ pub(crate) struct AttrCrateLevelOnlySugg { #[primary_span] pub attr: Span, } + +#[derive(Diagnostic)] +#[diag(passes_no_sanitize)] +pub(crate) struct NoSanitize<'a> { + #[primary_span] + pub attr_span: Span, + #[label] + pub defn_span: Span, + pub accepted_kind: &'a str, + pub attr_str: &'a str, +} diff --git a/tests/codegen/sanitizer/no-sanitize.rs b/tests/codegen/sanitizer/no-sanitize.rs index 47d3fd83f1127..2a309f6b9c696 100644 --- a/tests/codegen/sanitizer/no-sanitize.rs +++ b/tests/codegen/sanitizer/no-sanitize.rs @@ -7,6 +7,16 @@ #![crate_type = "lib"] #![feature(no_sanitize)] +// CHECK: @UNSANITIZED = constant{{.*}} no_sanitize_address +// CHECK-NOT: @__asan_global_UNSANITIZED +#[no_mangle] +#[no_sanitize(address)] +pub static UNSANITIZED: u32 = 0; + +// CHECK: @__asan_global_SANITIZED +#[no_mangle] +pub static SANITIZED: u32 = 0; + // CHECK-LABEL: ; no_sanitize::unsanitized // CHECK-NEXT: ; Function Attrs: // CHECK-NOT: sanitize_address diff --git a/tests/ui/attributes/no-sanitize.rs b/tests/ui/attributes/no-sanitize.rs index 82b7a22d5701e..8c79866d5aa31 100644 --- a/tests/ui/attributes/no-sanitize.rs +++ b/tests/ui/attributes/no-sanitize.rs @@ -4,31 +4,37 @@ #![allow(dead_code)] fn invalid() { - #[no_sanitize(memory)] //~ ERROR attribute should be applied to a function definition + #[no_sanitize(memory)] //~ ERROR `#[no_sanitize(memory)]` should be applied to a function { 1 }; } -#[no_sanitize(memory)] //~ ERROR attribute should be applied to a function definition +#[no_sanitize(memory)] //~ ERROR `#[no_sanitize(memory)]` should be applied to a function type InvalidTy = (); -#[no_sanitize(memory)] //~ ERROR attribute should be applied to a function definition +#[no_sanitize(memory)] //~ ERROR `#[no_sanitize(memory)]` should be applied to a function mod invalid_module {} fn main() { - let _ = #[no_sanitize(memory)] //~ ERROR attribute should be applied to a function definition + let _ = #[no_sanitize(memory)] //~ ERROR `#[no_sanitize(memory)]` should be applied to a function (|| 1); } -#[no_sanitize(memory)] //~ ERROR attribute should be applied to a function definition +#[no_sanitize(memory)] //~ ERROR `#[no_sanitize(memory)]` should be applied to a function struct F; -#[no_sanitize(memory)] //~ ERROR attribute should be applied to a function definition +#[no_sanitize(memory)] //~ ERROR `#[no_sanitize(memory)]` should be applied to a function impl F { #[no_sanitize(memory)] fn valid(&self) {} } +#[no_sanitize(address, memory)] //~ ERROR `#[no_sanitize(memory)]` should be applied to a function +static INVALID : i32 = 0; + #[no_sanitize(memory)] fn valid() {} + +#[no_sanitize(address)] +static VALID : i32 = 0; diff --git a/tests/ui/attributes/no-sanitize.stderr b/tests/ui/attributes/no-sanitize.stderr index f742ba0beed18..9b0b76e3f4eba 100644 --- a/tests/ui/attributes/no-sanitize.stderr +++ b/tests/ui/attributes/no-sanitize.stderr @@ -1,55 +1,63 @@ -error: attribute should be applied to a function definition - --> $DIR/no-sanitize.rs:7:5 +error: `#[no_sanitize(memory)]` should be applied to a function + --> $DIR/no-sanitize.rs:7:19 | LL | #[no_sanitize(memory)] - | ^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^ LL | / { LL | | 1 LL | | }; - | |_____- not a function definition + | |_____- not a function -error: attribute should be applied to a function definition - --> $DIR/no-sanitize.rs:13:1 +error: `#[no_sanitize(memory)]` should be applied to a function + --> $DIR/no-sanitize.rs:13:15 | LL | #[no_sanitize(memory)] - | ^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^ LL | type InvalidTy = (); - | -------------------- not a function definition + | -------------------- not a function -error: attribute should be applied to a function definition - --> $DIR/no-sanitize.rs:16:1 +error: `#[no_sanitize(memory)]` should be applied to a function + --> $DIR/no-sanitize.rs:16:15 | LL | #[no_sanitize(memory)] - | ^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^ LL | mod invalid_module {} - | --------------------- not a function definition + | --------------------- not a function -error: attribute should be applied to a function definition - --> $DIR/no-sanitize.rs:20:13 +error: `#[no_sanitize(memory)]` should be applied to a function + --> $DIR/no-sanitize.rs:20:27 | LL | let _ = #[no_sanitize(memory)] - | ^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^ LL | (|| 1); - | ------ not a function definition + | ------ not a function -error: attribute should be applied to a function definition - --> $DIR/no-sanitize.rs:24:1 +error: `#[no_sanitize(memory)]` should be applied to a function + --> $DIR/no-sanitize.rs:24:15 | LL | #[no_sanitize(memory)] - | ^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^ LL | struct F; - | --------- not a function definition + | --------- not a function -error: attribute should be applied to a function definition - --> $DIR/no-sanitize.rs:27:1 +error: `#[no_sanitize(memory)]` should be applied to a function + --> $DIR/no-sanitize.rs:27:15 | LL | #[no_sanitize(memory)] - | ^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^ LL | / impl F { LL | | #[no_sanitize(memory)] LL | | fn valid(&self) {} LL | | } - | |_- not a function definition + | |_- not a function -error: aborting due to 6 previous errors +error: `#[no_sanitize(memory)]` should be applied to a function + --> $DIR/no-sanitize.rs:33:24 + | +LL | #[no_sanitize(address, memory)] + | ^^^^^^ +LL | static INVALID : i32 = 0; + | ------------------------- not a function + +error: aborting due to 7 previous errors