Skip to content
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.

Sign up for GitHub

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

Exempt #[naked] fns from named_asm_labels lint #88169

Closed
wants to merge 1 commit into from
Closed

Exempt #[naked] fns from named_asm_labels lint #88169

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions compiler/rustc_ast/src/ast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2028,6 +2028,7 @@ pub enum InlineAsmOperand {
#[derive(Clone, Encodable, Decodable, Debug)]
pub struct InlineAsm {
pub template: Vec<InlineAsmTemplatePiece>,
pub template_strs: Box<[(Symbol, Option<Symbol>, Span)]>,
pub operands: Vec<(InlineAsmOperand, Span)>,
pub clobber_abi: Option<(Symbol, Span)>,
pub options: InlineAsmOptions,
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_ast/src/visit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,20 @@ use crate::token;
use rustc_span::symbol::{Ident, Symbol};
use rustc_span::Span;

#[derive(Copy, Clone, PartialEq)]
Copy link
Member

Choose a reason for hiding this comment

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

Leftover debug code?

#[derive(Copy, Clone, Debug, PartialEq)]
pub enum AssocCtxt {
Trait,
Impl,
}

#[derive(Copy, Clone, PartialEq)]
#[derive(Copy, Clone, Debug, PartialEq)]
pub enum FnCtxt {
Free,
Foreign,
Assoc(AssocCtxt),
}

#[derive(Copy, Clone)]
#[derive(Copy, Clone, Debug)]
pub enum FnKind<'a> {
/// E.g., `fn foo()`, `fn foo(&self)`, or `extern "Abi" fn foo()`.
Fn(FnCtxt, Ident, &'a FnSig, &'a Visibility, Option<&'a Block>),
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_ast_lowering/src/asm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,8 +392,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {

let operands = self.arena.alloc_from_iter(operands);
let template = self.arena.alloc_from_iter(asm.template.iter().cloned());
let template_strs = self.arena.alloc_from_iter(asm.template_strs.iter().cloned());
let line_spans = self.arena.alloc_slice(&asm.line_spans[..]);
let hir_asm = hir::InlineAsm { template, operands, options: asm.options, line_spans };
let hir_asm =
hir::InlineAsm { template, template_strs, operands, options: asm.options, line_spans };
self.arena.alloc(hir_asm)
}
}
90 changes: 14 additions & 76 deletions compiler/rustc_builtin_macros/src/asm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ use rustc_errors::{Applicability, DiagnosticBuilder};
use rustc_expand::base::{self, *};
use rustc_parse::parser::Parser;
use rustc_parse_format as parse;
use rustc_session::lint::{self, BuiltinLintDiagnostics};
use rustc_session::lint;
use rustc_span::symbol::Ident;
use rustc_span::symbol::{kw, sym, Symbol};
use rustc_span::{InnerSpan, MultiSpan, Span};
use rustc_span::{InnerSpan, Span};
use rustc_target::asm::InlineAsmArch;
use smallvec::smallvec;

Expand Down Expand Up @@ -484,11 +484,7 @@ fn parse_reg<'a>(
Ok(result)
}

fn expand_preparsed_asm(
ecx: &mut ExtCtxt<'_>,
args: AsmArgs,
is_local_asm: bool,
) -> Option<ast::InlineAsm> {
fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, args: AsmArgs) -> Option<ast::InlineAsm> {
let mut template = vec![];
// Register operands are implicitly used since they are not allowed to be
// referenced in the template string.
Expand All @@ -501,6 +497,8 @@ fn expand_preparsed_asm(
let mut line_spans = Vec::with_capacity(args.templates.len());
let mut curarg = 0;

let mut template_strs = Vec::with_capacity(args.templates.len());

for template_expr in args.templates.into_iter() {
if !template.is_empty() {
template.push(ast::InlineAsmTemplatePiece::String("\n".to_string()));
Expand All @@ -524,8 +522,13 @@ fn expand_preparsed_asm(
ast::StrStyle::Raw(raw) => Some(raw as usize),
};

let template_str = &template_str.as_str();
let template_snippet = ecx.source_map().span_to_snippet(template_sp).ok();
template_strs.push((
template_str,
template_snippet.as_ref().map(|s| Symbol::intern(s)),
template_sp,
));
let template_str = &template_str.as_str();

if let Some(InlineAsmArch::X86 | InlineAsmArch::X86_64) = ecx.sess.asm_arch {
let find_span = |needle: &str| -> Span {
Expand Down Expand Up @@ -560,72 +563,6 @@ fn expand_preparsed_asm(
}
}

// Lint against the use of named labels in inline `asm!` but not `global_asm!`
if is_local_asm {
let find_label_span = |needle: &str| -> Option<Span> {
if let Some(snippet) = &template_snippet {
if let Some(pos) = snippet.find(needle) {
let end = pos
+ &snippet[pos..]
.find(|c| c == ':')
.unwrap_or(snippet[pos..].len() - 1);
let inner = InnerSpan::new(pos, end);
return Some(template_sp.from_inner(inner));
}
}

None
};

let mut found_labels = Vec::new();

// A semicolon might not actually be specified as a separator for all targets, but it seems like LLVM accepts it always
let statements = template_str.split(|c| matches!(c, '\n' | ';'));
for statement in statements {
// If there's a comment, trim it from the statement
let statement = statement.find("//").map_or(statement, |idx| &statement[..idx]);
let mut start_idx = 0;
for (idx, _) in statement.match_indices(':') {
let possible_label = statement[start_idx..idx].trim();
let mut chars = possible_label.chars();
if let Some(c) = chars.next() {
// A label starts with an alphabetic character or . or _ and continues with alphanumeric characters, _, or $
if (c.is_alphabetic() || matches!(c, '.' | '_'))
&& chars.all(|c| c.is_alphanumeric() || matches!(c, '_' | '$'))
{
found_labels.push(possible_label);
} else {
// If we encounter a non-label, there cannot be any further labels, so stop checking
break;
}
} else {
// Empty string means a leading ':' in this section, which is not a label
break;
}

start_idx = idx + 1;
}
}

if found_labels.len() > 0 {
let spans =
found_labels.into_iter().filter_map(find_label_span).collect::<Vec<Span>>();
// If there were labels but we couldn't find a span, combine the warnings and use the template span
let target_spans: MultiSpan =
if spans.len() > 0 { spans.into() } else { template_sp.into() };
ecx.parse_sess().buffer_lint_with_diagnostic(
lint::builtin::NAMED_ASM_LABELS,
target_spans,
ecx.current_expansion.lint_node_id,
"avoid using named labels in inline assembly",
BuiltinLintDiagnostics::NamedAsmLabel(
"only local labels of the form `<number>:` should be used in inline asm"
.to_string(),
),
);
}
}

// Don't treat raw asm as a format string.
if args.options.contains(ast::InlineAsmOptions::RAW) {
template.push(ast::InlineAsmTemplatePiece::String(template_str.to_string()));
Expand Down Expand Up @@ -819,6 +756,7 @@ fn expand_preparsed_asm(

Some(ast::InlineAsm {
template,
template_strs: template_strs.into_boxed_slice(),
operands: args.operands,
clobber_abi: args.clobber_abi,
options: args.options,
Expand All @@ -833,7 +771,7 @@ pub fn expand_asm<'cx>(
) -> Box<dyn base::MacResult + 'cx> {
match parse_args(ecx, sp, tts, false) {
Ok(args) => {
let expr = if let Some(inline_asm) = expand_preparsed_asm(ecx, args, true) {
let expr = if let Some(inline_asm) = expand_preparsed_asm(ecx, args) {
P(ast::Expr {
id: ast::DUMMY_NODE_ID,
kind: ast::ExprKind::InlineAsm(P(inline_asm)),
Expand All @@ -860,7 +798,7 @@ pub fn expand_global_asm<'cx>(
) -> Box<dyn base::MacResult + 'cx> {
match parse_args(ecx, sp, tts, true) {
Ok(args) => {
if let Some(inline_asm) = expand_preparsed_asm(ecx, args, false) {
if let Some(inline_asm) = expand_preparsed_asm(ecx, args) {
MacEager::items(smallvec![P(ast::Item {
ident: Ident::invalid(),
attrs: Vec::new(),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/hir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2386,6 +2386,7 @@ impl<'hir> InlineAsmOperand<'hir> {
#[derive(Debug, HashStable_Generic)]
pub struct InlineAsm<'hir> {
pub template: &'hir [InlineAsmTemplatePiece],
pub template_strs: &'hir [(Symbol, Option<Symbol>, Span)],
pub operands: &'hir [(InlineAsmOperand<'hir>, Span)],
pub options: InlineAsmOptions,
pub line_spans: &'hir [Span],
Expand Down
137 changes: 134 additions & 3 deletions compiler/rustc_lint/src/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::subst::{GenericArgKind, Subst};
use rustc_middle::ty::Instance;
use rustc_middle::ty::{self, layout::LayoutError, Ty, TyCtxt};
use rustc_session::lint::FutureIncompatibilityReason;
use rustc_session::lint::{BuiltinLintDiagnostics, FutureIncompatibilityReason};
use rustc_session::Session;
use rustc_span::edition::Edition;
use rustc_span::source_map::Spanned;
use rustc_span::symbol::{kw, sym, Ident, Symbol};
use rustc_span::{edition::Edition, InnerSpan};
use rustc_span::{source_map::Spanned, MultiSpan};
use rustc_span::{BytePos, Span};
use rustc_target::abi::{LayoutOf, VariantIdx};
use rustc_trait_selection::traits::misc::can_type_implement_copy;
Expand Down Expand Up @@ -3140,3 +3140,134 @@ impl<'tcx> LateLintPass<'tcx> for DerefNullPtr {
}
}
}

declare_lint! {
/// The `named_asm_labels` lint detects the use of named labels in the
/// inline `asm!` macro.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![feature(asm)]
/// fn main() {
/// unsafe {
/// asm!("foo: bar");
/// }
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// LLVM is allowed to duplicate inline assembly blocks for any
/// reason, for example when it is in a function that gets inlined. Because
/// of this, GNU assembler [local labels] *must* be used instead of labels
/// with a name. Using named labels might cause assembler or linker errors.
///
/// See the [unstable book] for more details.
///
/// [local labels]: https://sourceware.org/binutils/docs/as/Symbol-Names.html#Local-Labels
/// [unstable book]: https://doc.rust-lang.org/nightly/unstable-book/library-features/asm.html#labels
pub NAMED_ASM_LABELS,
Deny,
"named labels in inline assembly",
}

declare_lint_pass!(NamedAsmLabels => [NAMED_ASM_LABELS]);

impl<'tcx> LateLintPass<'tcx> for NamedAsmLabels {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
if let hir::Expr {
kind: hir::ExprKind::InlineAsm(hir::InlineAsm { template_strs, .. }),
..
} = expr
{
let hir = cx.tcx.hir();
if let Some(enclosing_body) = cx.enclosing_body {
let fn_id = hir.get_parent_item(enclosing_body.hir_id);
let attrs = hir.attrs(fn_id);
// Process lint only if the containing fn does not have a `#[naked]` attribute
if attrs.iter().any(|attr| attr.has_name(sym::naked)) {
return;
}

for (template_sym, template_snippet, template_span) in template_strs.iter() {
let template_str = &template_sym.as_str();
let find_label_span = |needle: &str| -> Option<Span> {
if let Some(snippet) = template_snippet {
let snippet = snippet.as_str();
if let Some(pos) = snippet.find(needle) {
let end = pos
+ &snippet[pos..]
.find(|c| c == ':')
.unwrap_or(snippet[pos..].len() - 1);
let inner = InnerSpan::new(pos, end);
return Some(template_span.from_inner(inner));
}
}

None
};

let mut found_labels = Vec::new();

// A semicolon might not actually be specified as a separator for all targets, but it seems like LLVM accepts it always
let statements = template_str.split(|c| matches!(c, '\n' | ';'));
for statement in statements {
// If there's a comment, trim it from the statement
let statement =
statement.find("//").map_or(statement, |idx| &statement[..idx]);
let mut start_idx = 0;
for (idx, _) in statement.match_indices(':') {
let possible_label = statement[start_idx..idx].trim();
let mut chars = possible_label.chars();
if let Some(c) = chars.next() {
// A label starts with an alphabetic character or . or _ and continues with alphanumeric characters, _, or $
if (c.is_alphabetic() || matches!(c, '.' | '_'))
&& chars.all(|c| c.is_alphanumeric() || matches!(c, '_' | '$'))
{
found_labels.push(possible_label);
} else {
// If we encounter a non-label, there cannot be any further labels, so stop checking
break;
}
} else {
// Empty string means a leading ':' in this section, which is not a label
break;
}

start_idx = idx + 1;
}
}

debug!("NamedAsmLabels::check_expr(): found_labels: {:#?}", &found_labels);
Copy link
Member

Choose a reason for hiding this comment

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

More leftover debugging code.


if found_labels.len() > 0 {
let spans = found_labels
.into_iter()
.filter_map(|label| find_label_span(label))
.collect::<Vec<Span>>();
// If there were labels but we couldn't find a span, combine the warnings and use the template span
let target_spans: MultiSpan =
if spans.len() > 0 { spans.into() } else { (*template_span).into() };

cx.lookup_with_diagnostics(
NAMED_ASM_LABELS,
Some(target_spans),
|diag| {
let mut err =
diag.build("avoid using named labels in inline assembly");
err.emit();
},
BuiltinLintDiagnostics::NamedAsmLabel(
"only local labels of the form `<number>:` should be used in inline asm"
.to_string(),
),
);
}
}
}
}
}
}
1 change: 1 addition & 0 deletions compiler/rustc_lint/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ macro_rules! late_lint_passes {
NonPanicFmt: NonPanicFmt,
NoopMethodCall: NoopMethodCall,
InvalidAtomicOrdering: InvalidAtomicOrdering,
NamedAsmLabels: NamedAsmLabels,
]
);
};
Expand Down
Loading