Rollup merge of #72625 - Amanieu:asm-srcloc, r=petrochenkov

Improve inline asm error diagnostics

Previously we were just using the raw LLVM error output (with line, caret, etc) as the diagnostic message, which ends up looking rather out of place with our existing diagnostics.

The new diagnostics properly format the diagnostics and also take advantage of LLVM's per-line `srcloc` attribute to map an error in inline assembly directly to the relevant line of source code.

Incidentally also fixes #71639 by disabling `srcloc` metadata during LTO builds since we don't know what crate it might have come from. We can only resolve `srcloc`s from the currently crate since it indexes into the source map for the current crate.

Fixes #72664
Fixes #71639

r? @petrochenkov

### Old style

```rust
#![feature(llvm_asm)]

fn main() {
    unsafe {
        let _x: i32;
        llvm_asm!(
            "mov $0, $1
             invalid_instruction $0, $1
             mov $0, $1"
             : "=&r" (_x)
             : "r" (0)
             :: "intel"
        );
    }
}
```

```
error: <inline asm>:3:14: error: invalid instruction mnemonic 'invalid_instruction'
             invalid_instruction ecx, eax
             ^~~~~~~~~~~~~~~~~~~

  --> src/main.rs:6:9
   |
6  | /         llvm_asm!(
7  | |             "mov $0, $1
8  | |              invalid_instruction $0, $1
9  | |              mov $0, $1"
...  |
12 | |              :: "intel"
13 | |         );
   | |__________^
```

### New style

```rust
#![feature(asm)]

fn main() {
    unsafe {
        asm!(
            "mov {0}, {1}
             invalid_instruction {0}, {1}
             mov {0}, {1}",
            out(reg) _,
            in(reg) 0i64,
        );
    }
}
```

```
error: invalid instruction mnemonic 'invalid_instruction'
 --> test.rs:7:14
  |
7 |              invalid_instruction {0}, {1}
  |              ^
  |
note: instantiated into assembly here
 --> <inline asm>:3:14
  |
3 |              invalid_instruction rax, rcx
  |              ^^^^^^^^^^^^^^^^^^^
```

Author: RalfJung

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

@@ -1267,7 +1267,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();
 
@@ -287,9 +287,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) {
@@ -341,7 +341,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 };
@@ -382,8 +382,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;
@@ -930,7 +931,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);
@@ -1033,14 +1034,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],
     );
 }