Improve inline asm error diagnostics

src/libfmt_macros/lib.rs

@@ -191,6 +191,11 @@ pub struct Parser<'a> {
     append_newline: bool,
     /// Whether this formatting string is a literal or it comes from a macro.
     is_literal: bool,
+    /// Start position of the current line.
+    cur_line_start: usize,
+    /// Start and end byte offset of every line of the format string. Excludes
+    /// newline characters and leading whitespace.
+    pub line_spans: Vec<InnerSpan>,
 }
 
 impl<'a> Iterator for Parser<'a> {
@@ -235,10 +240,15 @@ impl<'a> Iterator for Parser<'a> {
                         None
                     }
                 }
-                '\n' => Some(String(self.string(pos))),
                 _ => Some(String(self.string(pos))),
             }
         } else {
+            if self.is_literal && self.cur_line_start != self.input.len() {
+                let start = self.to_span_index(self.cur_line_start);
+                let end = self.to_span_index(self.input.len());
+                self.line_spans.push(start.to(end));
+                self.cur_line_start = self.input.len();
+            }
             None
         }
     }
@@ -266,6 +276,8 @@ impl<'a> Parser<'a> {
             last_opening_brace: None,
             append_newline,
             is_literal,
+            cur_line_start: 0,
+            line_spans: vec![],
         }
     }
 
@@ -433,7 +445,17 @@ impl<'a> Parser<'a> {
                 '{' | '}' => {
                     return &self.input[start..pos];
                 }
+                '\n' if self.is_literal => {
+                    let start = self.to_span_index(self.cur_line_start);
+                    let end = self.to_span_index(pos);
+                    self.line_spans.push(start.to(end));
+                    self.cur_line_start = pos + 1;
+                    self.cur.next();
+                }
                 _ => {
+                    if self.is_literal && pos == self.cur_line_start && c.is_whitespace() {
+                        self.cur_line_start = pos + c.len_utf8();
+                    }
                     self.cur.next();
                 }
             }

src/librustc_ast/ast.rs

@@ -1252,7 +1252,7 @@ pub enum ExprKind {
     Ret(Option<P<Expr>>),
 
     /// Output of the `asm!()` macro.
-    InlineAsm(InlineAsm),
+    InlineAsm(P<InlineAsm>),
     /// Output of the `llvm_asm!()` macro.
     LlvmInlineAsm(P<LlvmInlineAsm>),
 
@@ -1971,6 +1971,7 @@ pub struct InlineAsm {
     pub template: Vec<InlineAsmTemplatePiece>,
     pub operands: Vec<(InlineAsmOperand, Span)>,
     pub options: InlineAsmOptions,
+    pub line_spans: Vec<Span>,
 }
 
 /// Inline assembly dialect.

src/librustc_ast_lowering/expr.rs

@@ -1265,7 +1265,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
 
         let operands = self.arena.alloc_from_iter(operands);
         let template = self.arena.alloc_from_iter(asm.template.iter().cloned());
-        let hir_asm = hir::InlineAsm { template, operands, options: asm.options };
+        let line_spans = self.arena.alloc_slice(&asm.line_spans[..]);
+        let hir_asm = hir::InlineAsm { template, operands, options: asm.options, line_spans };
         hir::ExprKind::InlineAsm(self.arena.alloc(hir_asm))
     }
 

src/librustc_builtin_macros/asm.rs

@@ -513,10 +513,16 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, sp: Span, args: AsmArgs) -> P<ast
         }
     }
 
-    let inline_asm = ast::InlineAsm { template, operands, options: args.options };
+    let line_spans = if parser.line_spans.is_empty() {
+        vec![template_sp]
+    } else {
+        parser.line_spans.iter().map(|span| template_span.from_inner(*span)).collect()
+    };
+
+    let inline_asm = ast::InlineAsm { template, operands, options: args.options, line_spans };
     P(ast::Expr {
         id: ast::DUMMY_NODE_ID,
-        kind: ast::ExprKind::InlineAsm(inline_asm),
+        kind: ast::ExprKind::InlineAsm(P(inline_asm)),
         span: sp,
         attrs: ast::AttrVec::new(),
         tokens: None,

src/librustc_codegen_llvm/asm.rs

@@ -14,7 +14,7 @@ use rustc_data_structures::fx::FxHashMap;
 use rustc_hir as hir;
 use rustc_middle::span_bug;
 use rustc_middle::ty::layout::TyAndLayout;
-use rustc_span::Span;
+use rustc_span::{Pos, Span};
 use rustc_target::abi::*;
 use rustc_target::asm::*;
 
@@ -97,7 +97,7 @@ impl AsmBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> {
             ia.volatile,
             ia.alignstack,
             ia.dialect,
-            span,
+            &[span],
         );
         if r.is_none() {
             return false;
@@ -119,7 +119,7 @@ impl AsmBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> {
         template: &[InlineAsmTemplatePiece],
         operands: &[InlineAsmOperandRef<'tcx, Self>],
         options: InlineAsmOptions,
-        span: Span,
+        line_spans: &[Span],
     ) {
         let asm_arch = self.tcx.sess.asm_arch.unwrap();
 
@@ -286,9 +286,9 @@ impl AsmBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> {
             volatile,
             alignstack,
             dialect,
-            span,
+            line_spans,
         )
-        .unwrap_or_else(|| span_bug!(span, "LLVM asm constraint validation failed"));
+        .unwrap_or_else(|| span_bug!(line_spans[0], "LLVM asm constraint validation failed"));
 
         if options.contains(InlineAsmOptions::PURE) {
             if options.contains(InlineAsmOptions::NOMEM) {
@@ -340,7 +340,7 @@ fn inline_asm_call(
     volatile: bool,
     alignstack: bool,
     dia: LlvmAsmDialect,
-    span: Span,
+    line_spans: &[Span],
 ) -> Option<&'ll Value> {
     let volatile = if volatile { llvm::True } else { llvm::False };
     let alignstack = if alignstack { llvm::True } else { llvm::False };
@@ -381,8 +381,24 @@ fn inline_asm_call(
                 key.len() as c_uint,
             );
 
-            let val: &'ll Value = bx.const_i32(span.ctxt().outer_expn().as_u32() as i32);
-            llvm::LLVMSetMetadata(call, kind, llvm::LLVMMDNodeInContext(bx.llcx, &val, 1));
+            // srcloc contains one integer for each line of assembly code.
+            // Unfortunately this isn't enough to encode a full span so instead
+            // we just encode the start position of each line.
+            // FIXME: Figure out a way to pass the entire line spans.
+            let mut srcloc = vec![];
+            if dia == LlvmAsmDialect::Intel && line_spans.len() > 1 {
+                // LLVM inserts an extra line to add the ".intel_syntax", so add
+                // a dummy srcloc entry for it.
+                //
+                // Don't do this if we only have 1 line span since that may be
+                // due to the asm template string coming from a macro. LLVM will
+                // default to the first srcloc for lines that don't have an
+                // associated srcloc.
+                srcloc.push(bx.const_i32(0));
+            }
+            srcloc.extend(line_spans.iter().map(|span| bx.const_i32(span.lo().to_u32() as i32)));
+            let md = llvm::LLVMMDNodeInContext(bx.llcx, srcloc.as_ptr(), srcloc.len() as u32);
+            llvm::LLVMSetMetadata(call, kind, md);
 
             Some(call)
         } else {

src/librustc_codegen_llvm/back/write.rs

@@ -23,6 +23,7 @@ use rustc_middle::bug;
 use rustc_middle::ty::TyCtxt;
 use rustc_session::config::{self, Lto, OutputType, Passes, Sanitizer, SwitchWithOptPath};
 use rustc_session::Session;
+use rustc_span::InnerSpan;
 use rustc_target::spec::{CodeModel, RelocModel};
 
 use libc::{c_char, c_int, c_uint, c_void, size_t};
@@ -238,12 +239,19 @@ impl<'a> Drop for DiagnosticHandlers<'a> {
     }
 }
 
-unsafe extern "C" fn report_inline_asm(
+fn report_inline_asm(
     cgcx: &CodegenContext<LlvmCodegenBackend>,
-    msg: &str,
-    cookie: c_uint,
+    msg: String,
+    mut cookie: c_uint,
+    source: Option<(String, Vec<InnerSpan>)>,
 ) {
-    cgcx.diag_emitter.inline_asm_error(cookie as u32, msg.to_owned());
+    // In LTO build we may get srcloc values from other crates which are invalid
+    // since they use a different source map. To be safe we just suppress these
+    // in LTO builds.
+    if matches!(cgcx.lto, Lto::Fat | Lto::Thin) {
+        cookie = 0;
+    }
+    cgcx.diag_emitter.inline_asm_error(cookie as u32, msg, source);
 }
 
 unsafe extern "C" fn inline_asm_handler(diag: &SMDiagnostic, user: *const c_void, cookie: c_uint) {
@@ -252,10 +260,37 @@ unsafe extern "C" fn inline_asm_handler(diag: &SMDiagnostic, user: *const c_void
     }
     let (cgcx, _) = *(user as *const (&CodegenContext<LlvmCodegenBackend>, &Handler));
 
-    let msg = llvm::build_string(|s| llvm::LLVMRustWriteSMDiagnosticToString(diag, s))
-        .expect("non-UTF8 SMDiagnostic");
+    // Recover the post-substitution assembly code from LLVM for better
+    // diagnostics.
+    let mut have_source = false;
+    let mut buffer = String::new();
+    let mut loc = 0;
+    let mut ranges = [0; 8];
+    let mut num_ranges = ranges.len() / 2;
+    let msg = llvm::build_string(|msg| {
+        buffer = llvm::build_string(|buffer| {
+            have_source = llvm::LLVMRustUnpackSMDiagnostic(
+                diag,
+                msg,
+                buffer,
+                &mut loc,
+                ranges.as_mut_ptr(),
+                &mut num_ranges,
+            );
+        })
+        .expect("non-UTF8 inline asm");
+    })
+    .expect("non-UTF8 SMDiagnostic");
+
+    let source = have_source.then(|| {
+        let mut spans = vec![InnerSpan::new(loc as usize, loc as usize)];
+        for i in 0..num_ranges {
+            spans.push(InnerSpan::new(ranges[i * 2] as usize, ranges[i * 2 + 1] as usize));
+        }
+        (buffer, spans)
+    });
 
-    report_inline_asm(cgcx, &msg, cookie);
+    report_inline_asm(cgcx, msg, cookie, source);
 }
 
 unsafe extern "C" fn diagnostic_handler(info: &DiagnosticInfo, user: *mut c_void) {
@@ -266,7 +301,7 @@ unsafe extern "C" fn diagnostic_handler(info: &DiagnosticInfo, user: *mut c_void
 
     match llvm::diagnostic::Diagnostic::unpack(info) {
         llvm::diagnostic::InlineAsm(inline) => {
-            report_inline_asm(cgcx, &llvm::twine_to_string(inline.message), inline.cookie);
+            report_inline_asm(cgcx, llvm::twine_to_string(inline.message), inline.cookie, None);
         }
 
         llvm::diagnostic::Optimization(opt) => {

src/librustc_codegen_llvm/llvm/ffi.rs

@@ -2070,7 +2070,14 @@ extern "C" {
     );
 
     #[allow(improper_ctypes)]
-    pub fn LLVMRustWriteSMDiagnosticToString(d: &SMDiagnostic, s: &RustString);
+    pub fn LLVMRustUnpackSMDiagnostic(
+        d: &SMDiagnostic,
+        message_out: &RustString,
+        buffer_out: &RustString,
+        loc_out: &mut c_uint,
+        ranges_out: *mut c_uint,
+        num_ranges: &mut usize,
+    ) -> bool;
 
     pub fn LLVMRustWriteArchive(
         Dst: *const c_char,

src/librustc_codegen_ssa/back/write.rs

@@ -31,9 +31,9 @@ use rustc_session::cgu_reuse_tracker::CguReuseTracker;
 use rustc_session::config::{self, CrateType, Lto, OutputFilenames, OutputType};
 use rustc_session::config::{Passes, Sanitizer, SwitchWithOptPath};
 use rustc_session::Session;
-use rustc_span::hygiene::ExpnId;
 use rustc_span::source_map::SourceMap;
 use rustc_span::symbol::{sym, Symbol};
+use rustc_span::{BytePos, FileName, InnerSpan, Pos, Span};
 use rustc_target::spec::{MergeFunctions, PanicStrategy};
 
 use std::any::Any;
@@ -1551,7 +1551,7 @@ fn spawn_work<B: ExtraBackendMethods>(cgcx: CodegenContext<B>, work: WorkItem<B>
 
 enum SharedEmitterMessage {
     Diagnostic(Diagnostic),
-    InlineAsmError(u32, String),
+    InlineAsmError(u32, String, Option<(String, Vec<InnerSpan>)>),
     AbortIfErrors,
     Fatal(String),
 }
@@ -1572,8 +1572,13 @@ impl SharedEmitter {
         (SharedEmitter { sender }, SharedEmitterMain { receiver })
     }
 
-    pub fn inline_asm_error(&self, cookie: u32, msg: String) {
-        drop(self.sender.send(SharedEmitterMessage::InlineAsmError(cookie, msg)));
+    pub fn inline_asm_error(
+        &self,
+        cookie: u32,
+        msg: String,
+        source: Option<(String, Vec<InnerSpan>)>,
+    ) {
+        drop(self.sender.send(SharedEmitterMessage::InlineAsmError(cookie, msg, source)));
     }
 
     pub fn fatal(&self, msg: &str) {
@@ -1626,8 +1631,30 @@ impl SharedEmitterMain {
                     }
                     handler.emit_diagnostic(&d);
                 }
-                Ok(SharedEmitterMessage::InlineAsmError(cookie, msg)) => {
-                    sess.span_err(ExpnId::from_u32(cookie).expn_data().call_site, &msg)
+                Ok(SharedEmitterMessage::InlineAsmError(cookie, msg, source)) => {
+                    let msg = msg.strip_prefix("error: ").unwrap_or(&msg);
+
+                    // If the cookie is 0 then we don't have span information.
+                    let mut err = if cookie == 0 {
+                        sess.struct_err(&msg)
+                    } else {
+                        let pos = BytePos::from_u32(cookie);
+                        let span = Span::with_root_ctxt(pos, pos);
+                        sess.struct_span_err(span, &msg)
+                    };
+
+                    // Point to the generated assembly if it is available.
+                    if let Some((buffer, spans)) = source {
+                        let source = sess
+                            .source_map()
+                            .new_source_file(FileName::inline_asm_source_code(&buffer), buffer);
+                        let source_span = Span::with_root_ctxt(source.start_pos, source.end_pos);
+                        let spans: Vec<_> =
+                            spans.iter().map(|sp| source_span.from_inner(*sp)).collect();
+                        err.span_note(spans, "instantiated into assembly here");
+                    }
+
+                    err.emit();
                 }
                 Ok(SharedEmitterMessage::AbortIfErrors) => {
                     sess.abort_if_errors();

src/librustc_codegen_ssa/mir/block.rs

@@ -831,6 +831,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         template: &[ast::InlineAsmTemplatePiece],
         operands: &[mir::InlineAsmOperand<'tcx>],
         options: ast::InlineAsmOptions,
+        line_spans: &[Span],
         destination: Option<mir::BasicBlock>,
     ) {
         let span = terminator.source_info.span;
@@ -931,7 +932,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             })
             .collect();
 
-        bx.codegen_inline_asm(template, &operands, options, span);
+        bx.codegen_inline_asm(template, &operands, options, line_spans);
 
         if let Some(target) = destination {
             helper.funclet_br(self, &mut bx, target);
@@ -1034,14 +1035,21 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                 bug!("borrowck false edges in codegen")
             }
 
-            mir::TerminatorKind::InlineAsm { template, ref operands, options, destination } => {
+            mir::TerminatorKind::InlineAsm {
+                template,
+                ref operands,
+                options,
+                line_spans,
+                destination,
+            } => {
                 self.codegen_asm_terminator(
                     helper,
                     bx,
                     terminator,
                     template,
                     operands,
                     options,
+                    line_spans,
                     destination,
                 );
             }

src/librustc_codegen_ssa/traits/asm.rs

@@ -52,7 +52,7 @@ pub trait AsmBuilderMethods<'tcx>: BackendTypes {
         template: &[InlineAsmTemplatePiece],
         operands: &[InlineAsmOperandRef<'tcx, Self>],
         options: InlineAsmOptions,
-        span: Span,
+        line_spans: &[Span],
     );
 }