Skip to content

Better sanitize attr diagnostic #145182

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

Closed
wants to merge 1 commit into from
Closed
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
5 changes: 4 additions & 1 deletion compiler/rustc_codegen_ssa/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -171,9 +171,12 @@ codegen_ssa_invalid_monomorphization_unsupported_symbol = invalid monomorphizati

codegen_ssa_invalid_monomorphization_unsupported_symbol_of_size = invalid monomorphization of `{$name}` intrinsic: unsupported {$symbol} from `{$in_ty}` with element `{$in_elem}` of size `{$size}` to `{$ret_ty}`

codegen_ssa_invalid_sanitize = invalid argument for `sanitize`
codegen_ssa_invalid_sanitizer = invalid argument for `sanitize`
.note = expected one of: `address`, `kernel_address`, `cfi`, `hwaddress`, `kcfi`, `memory`, `memtag`, `shadow_call_stack`, or `thread`

codegen_ssa_invalid_sanitizer_setting = invalid setting for `{$sanitizer}`
.note = expected one of: `on`, or `off`

codegen_ssa_invalid_windows_subsystem = invalid windows subsystem `{$subsystem}`, only `windows` and `console` are allowed

codegen_ssa_ld64_unimplemented_modifier = `as-needed` modifier not implemented yet for ld64
Expand Down
130 changes: 87 additions & 43 deletions compiler/rustc_codegen_ssa/src/codegen_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -569,57 +569,101 @@ fn parse_sanitize_attr(
if let Some(list) = attr.meta_item_list() {
for item in list.iter() {
let MetaItemInner::MetaItem(set) = item else {
tcx.dcx().emit_err(errors::InvalidSanitize { span: attr.span() });
tcx.dcx().emit_err(errors::InvalidSanitizer { span: attr.span() });
break;
};
let segments = set.path.segments.iter().map(|x| x.ident.name).collect::<Vec<_>>();
match segments.as_slice() {
// Similar to clang, sanitize(address = ..) and
// sanitize(kernel_address = ..) control both ASan and KASan
// Source: https://reviews.llvm.org/D44981.
[sym::address] | [sym::kernel_address] if set.value_str() == Some(sym::off) => {
result |= SanitizerSet::ADDRESS | SanitizerSet::KERNELADDRESS
}
[sym::address] | [sym::kernel_address] if set.value_str() == Some(sym::on) => {
result &= !SanitizerSet::ADDRESS;
result &= !SanitizerSet::KERNELADDRESS;
}
[sym::cfi] if set.value_str() == Some(sym::off) => result |= SanitizerSet::CFI,
[sym::cfi] if set.value_str() == Some(sym::on) => result &= !SanitizerSet::CFI,
[sym::kcfi] if set.value_str() == Some(sym::off) => result |= SanitizerSet::KCFI,
[sym::kcfi] if set.value_str() == Some(sym::on) => result &= !SanitizerSet::KCFI,
[sym::memory] if set.value_str() == Some(sym::off) => {
result |= SanitizerSet::MEMORY
}
[sym::memory] if set.value_str() == Some(sym::on) => {
result &= !SanitizerSet::MEMORY
}
[sym::memtag] if set.value_str() == Some(sym::off) => {
result |= SanitizerSet::MEMTAG
}
[sym::memtag] if set.value_str() == Some(sym::on) => {
result &= !SanitizerSet::MEMTAG
}
[sym::shadow_call_stack] if set.value_str() == Some(sym::off) => {
result |= SanitizerSet::SHADOWCALLSTACK
}
[sym::shadow_call_stack] if set.value_str() == Some(sym::on) => {
result &= !SanitizerSet::SHADOWCALLSTACK
}
[sym::thread] if set.value_str() == Some(sym::off) => {
result |= SanitizerSet::THREAD
}
[sym::thread] if set.value_str() == Some(sym::on) => {
result &= !SanitizerSet::THREAD
}
[sym::hwaddress] if set.value_str() == Some(sym::off) => {
result |= SanitizerSet::HWADDRESS
}
[sym::hwaddress] if set.value_str() == Some(sym::on) => {
result &= !SanitizerSet::HWADDRESS
}
[sym::address] | [sym::kernel_address] => match set.value_str() {
Some(sym::off) => result |= SanitizerSet::ADDRESS | SanitizerSet::KERNELADDRESS,
Some(sym::on) => {
result &= !SanitizerSet::ADDRESS;
result &= !SanitizerSet::KERNELADDRESS;
}
_ => {
let sanitizer = segments.as_slice()[0];
tcx.dcx().emit_err(errors::InvalidSanitizerSetting {
span: set.span,
sanitizer,
});
}
},
[sym::cfi] => match set.value_str() {
Some(sym::off) => result |= SanitizerSet::CFI,
Some(sym::on) => result &= !SanitizerSet::CFI,
_ => {
tcx.dcx().emit_err(errors::InvalidSanitizerSetting {
span: set.span,
sanitizer: sym::cfi,
});
}
},
[sym::kcfi] => match set.value_str() {
Some(sym::off) => result |= SanitizerSet::KCFI,
Some(sym::on) => result &= !SanitizerSet::KCFI,
_ => {
tcx.dcx().emit_err(errors::InvalidSanitizerSetting {
span: set.span,
sanitizer: sym::kcfi,
});
}
},
[sym::memory] => match set.value_str() {
Some(sym::off) => result |= SanitizerSet::MEMORY,
Some(sym::on) => result &= !SanitizerSet::MEMORY,
_ => {
tcx.dcx().emit_err(errors::InvalidSanitizerSetting {
span: set.span,
sanitizer: sym::memory,
});
}
},
[sym::memtag] => match set.value_str() {
Some(sym::off) => result |= SanitizerSet::MEMTAG,
Some(sym::on) => result &= !SanitizerSet::MEMTAG,
_ => {
tcx.dcx().emit_err(errors::InvalidSanitizerSetting {
span: set.span,
sanitizer: sym::memtag,
});
}
},
[sym::shadow_call_stack] => match set.value_str() {
Some(sym::off) => result |= SanitizerSet::SHADOWCALLSTACK,
Some(sym::on) => result &= !SanitizerSet::SHADOWCALLSTACK,
_ => {
tcx.dcx().emit_err(errors::InvalidSanitizerSetting {
span: set.span,
sanitizer: sym::shadow_call_stack,
});
}
},
[sym::thread] => match set.value_str() {
Some(sym::off) => result |= SanitizerSet::THREAD,
Some(sym::on) => result &= !SanitizerSet::THREAD,
_ => {
tcx.dcx().emit_err(errors::InvalidSanitizerSetting {
span: set.span,
sanitizer: sym::thread,
});
}
},

[sym::hwaddress] => match set.value_str() {
Some(sym::off) => result |= SanitizerSet::HWADDRESS,
Some(sym::on) => result &= !SanitizerSet::HWADDRESS,
_ => {
tcx.dcx().emit_err(errors::InvalidSanitizerSetting {
span: set.span,
sanitizer: sym::hwaddress,
});
}
},
_ => {
tcx.dcx().emit_err(errors::InvalidSanitize { span: attr.span() });
tcx.dcx().emit_err(errors::InvalidSanitizer { span: attr.span() });
}
}
}
Expand Down
13 changes: 11 additions & 2 deletions compiler/rustc_codegen_ssa/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1121,13 +1121,22 @@ impl IntoDiagArg for ExpectedPointerMutability {
}

#[derive(Diagnostic)]
#[diag(codegen_ssa_invalid_sanitize)]
#[diag(codegen_ssa_invalid_sanitizer)]
#[note]
pub(crate) struct InvalidSanitize {
pub(crate) struct InvalidSanitizer {
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(codegen_ssa_invalid_sanitizer_setting)]
#[note]
pub(crate) struct InvalidSanitizerSetting {
#[primary_span]
pub span: Span,
pub sanitizer: Symbol,
}

#[derive(Diagnostic)]
#[diag(codegen_ssa_target_feature_safe_trait)]
pub(crate) struct TargetFeatureSafeTrait {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/sanitize-attr/invalid-sanitize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ fn multiple_consistent() {}
#[sanitize(address = "off")]
fn multiple_inconsistent() {}

#[sanitize(address = "bogus")] //~ ERROR invalid argument for `sanitize`
#[sanitize(address = "bogus")] //~ ERROR invalid setting for `address`
fn wrong_value() {}

#[sanitize = "off"] //~ ERROR malformed `sanitize` attribute input
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/sanitize-attr/invalid-sanitize.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,13 @@ LL | #[sanitize(brontosaurus = "off")]
|
= note: expected one of: `address`, `kernel_address`, `cfi`, `hwaddress`, `kcfi`, `memory`, `memtag`, `shadow_call_stack`, or `thread`

error: invalid argument for `sanitize`
--> $DIR/invalid-sanitize.rs:15:1
error: invalid setting for `address`
--> $DIR/invalid-sanitize.rs:15:12
|
LL | #[sanitize(address = "bogus")]
Copy link
Author

Choose a reason for hiding this comment

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

I would like for this to only mark the "bogus" part. I couldn't figure out how to get the correct span for that.

| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^
|
= note: expected one of: `address`, `kernel_address`, `cfi`, `hwaddress`, `kcfi`, `memory`, `memtag`, `shadow_call_stack`, or `thread`
= note: expected one of: `on`, or `off`

error: aborting due to 6 previous errors

Loading