Skip to content

Commit

Permalink
Rollup merge of rust-lang#129181 - beetrees:asm-spans, r=pnkfelix
Browse files Browse the repository at this point in the history
Pass end position of span through inline ASM cookie

Before this PR, only the start position of the span was passed though the inline ASM cookie to diagnostics. LLVM 19 has full support for 64-bit inline ASM cookies; this PR uses that to pass the end position of the span in the upper 32 bits, meaning inline ASM diagnostics now point at the entire line the error occurred on, not just the first character of it.
  • Loading branch information
Urgau authored Oct 16, 2024
2 parents 0037048 + 29ae617 commit c9795bd
Show file tree
Hide file tree
Showing 17 changed files with 874 additions and 103 deletions.
21 changes: 10 additions & 11 deletions compiler/rustc_codegen_llvm/src/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,14 +487,13 @@ pub(crate) fn inline_asm_call<'ll>(
let key = "srcloc";
let kind = llvm::LLVMGetMDKindIDInContext(
bx.llcx,
key.as_ptr() as *const c_char,
key.as_ptr().cast::<c_char>(),
key.len() as c_uint,
);

// 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.
// `srcloc` contains one 64-bit integer for each line of assembly code,
// where the lower 32 bits hold the lo byte position and the upper 32 bits
// hold the hi byte position.
let mut srcloc = vec![];
if dia == llvm::AsmDialect::Intel && line_spans.len() > 1 {
// LLVM inserts an extra line to add the ".intel_syntax", so add
Expand All @@ -504,13 +503,13 @@ pub(crate) fn inline_asm_call<'ll>(
// 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(llvm::LLVMValueAsMetadata(bx.const_i32(0)));
srcloc.push(llvm::LLVMValueAsMetadata(bx.const_u64(0)));
}
srcloc.extend(
line_spans
.iter()
.map(|span| llvm::LLVMValueAsMetadata(bx.const_i32(span.lo().to_u32() as i32))),
);
srcloc.extend(line_spans.iter().map(|span| {
llvm::LLVMValueAsMetadata(bx.const_u64(
u64::from(span.lo().to_u32()) | (u64::from(span.hi().to_u32()) << 32),
))
}));
let md = llvm::LLVMMDNodeInContext2(bx.llcx, srcloc.as_ptr(), srcloc.len());
let md = llvm::LLVMMetadataAsValue(&bx.llcx, md);
llvm::LLVMSetMetadata(call, kind, md);
Expand Down
23 changes: 17 additions & 6 deletions compiler/rustc_codegen_llvm/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ use rustc_session::Session;
use rustc_session::config::{
self, Lto, OutputType, Passes, RemapPathScopeComponents, SplitDwarfKind, SwitchWithOptPath,
};
use rustc_span::InnerSpan;
use rustc_span::symbol::sym;
use rustc_span::{BytePos, InnerSpan, Pos, SpanData, SyntaxContext};
use rustc_target::spec::{CodeModel, RelocModel, SanitizerSet, SplitDebuginfo, TlsModel};
use tracing::debug;

Expand Down Expand Up @@ -411,21 +411,32 @@ fn report_inline_asm(
cgcx: &CodegenContext<LlvmCodegenBackend>,
msg: String,
level: llvm::DiagnosticLevel,
mut cookie: u64,
cookie: u64,
source: Option<(String, Vec<InnerSpan>)>,
) {
// 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;
}
let span = if cookie == 0 || matches!(cgcx.lto, Lto::Fat | Lto::Thin) {
SpanData::default()
} else {
let lo = BytePos::from_u32(cookie as u32);
let hi = BytePos::from_u32((cookie >> 32) as u32);
SpanData {
lo,
// LLVM version < 19 silently truncates the cookie to 32 bits in some situations.
hi: if hi.to_u32() != 0 { hi } else { lo },
ctxt: SyntaxContext::root(),
parent: None,
}
};
let level = match level {
llvm::DiagnosticLevel::Error => Level::Error,
llvm::DiagnosticLevel::Warning => Level::Warning,
llvm::DiagnosticLevel::Note | llvm::DiagnosticLevel::Remark => Level::Note,
};
cgcx.diag_emitter.inline_asm_error(cookie.try_into().unwrap(), msg, level, source);
let msg = msg.strip_prefix("error: ").unwrap_or(&msg).to_string();
cgcx.diag_emitter.inline_asm_error(span, msg, level, source);
}

unsafe extern "C" fn diagnostic_handler(info: &DiagnosticInfo, user: *mut c_void) {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/llvm/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ impl InlineAsmDiagnostic {
unsafe { SrcMgrDiagnostic::unpack(super::LLVMRustGetSMDiagnostic(di, &mut cookie)) };
InlineAsmDiagnostic {
level: smdiag.level,
cookie: cookie.into(),
cookie,
message: smdiag.message,
source: smdiag.source,
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2271,7 +2271,7 @@ unsafe extern "C" {

pub fn LLVMRustGetSMDiagnostic<'a>(
DI: &'a DiagnosticInfo,
cookie_out: &mut c_uint,
cookie_out: &mut u64,
) -> &'a SMDiagnostic;

#[allow(improper_ctypes)]
Expand Down
21 changes: 8 additions & 13 deletions compiler/rustc_codegen_ssa/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use rustc_session::config::{
};
use rustc_span::source_map::SourceMap;
use rustc_span::symbol::sym;
use rustc_span::{BytePos, FileName, InnerSpan, Pos, Span};
use rustc_span::{FileName, InnerSpan, Span, SpanData};
use rustc_target::spec::{MergeFunctions, SanitizerSet};
use tracing::debug;

Expand Down Expand Up @@ -1852,7 +1852,7 @@ fn spawn_work<'a, B: ExtraBackendMethods>(

enum SharedEmitterMessage {
Diagnostic(Diagnostic),
InlineAsmError(u32, String, Level, Option<(String, Vec<InnerSpan>)>),
InlineAsmError(SpanData, String, Level, Option<(String, Vec<InnerSpan>)>),
Fatal(String),
}

Expand All @@ -1874,12 +1874,12 @@ impl SharedEmitter {

pub fn inline_asm_error(
&self,
cookie: u32,
span: SpanData,
msg: String,
level: Level,
source: Option<(String, Vec<InnerSpan>)>,
) {
drop(self.sender.send(SharedEmitterMessage::InlineAsmError(cookie, msg, level, source)));
drop(self.sender.send(SharedEmitterMessage::InlineAsmError(span, msg, level, source)));
}

fn fatal(&self, msg: &str) {
Expand Down Expand Up @@ -1964,17 +1964,12 @@ impl SharedEmitterMain {
dcx.emit_diagnostic(d);
sess.dcx().abort_if_errors();
}
Ok(SharedEmitterMessage::InlineAsmError(cookie, msg, level, source)) => {
Ok(SharedEmitterMessage::InlineAsmError(span, msg, level, source)) => {
assert_matches!(level, Level::Error | Level::Warning | Level::Note);
let msg = msg.strip_prefix("error: ").unwrap_or(&msg).to_string();
let mut err = Diag::<()>::new(sess.dcx(), level, msg);

// If the cookie is 0 then we don't have span information.
if cookie != 0 {
let pos = BytePos::from_u32(cookie);
let span = Span::with_root_ctxt(pos, pos);
err.span(span);
};
if !span.is_dummy() {
err.span(span.span());
}

// Point to the generated assembly if it is available.
if let Some((buffer, spans)) = source {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1445,7 +1445,7 @@ extern "C" LLVMTypeKind LLVMRustGetTypeKind(LLVMTypeRef Ty) {
DEFINE_SIMPLE_CONVERSION_FUNCTIONS(SMDiagnostic, LLVMSMDiagnosticRef)

extern "C" LLVMSMDiagnosticRef LLVMRustGetSMDiagnostic(LLVMDiagnosticInfoRef DI,
unsigned *Cookie) {
uint64_t *Cookie) {
llvm::DiagnosticInfoSrcMgr *SM =
static_cast<llvm::DiagnosticInfoSrcMgr *>(unwrap(DI));
*Cookie = SM->getLocCookie();
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_span/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,12 @@ impl SpanData {
}
}

impl Default for SpanData {
fn default() -> Self {
Self { lo: BytePos(0), hi: BytePos(0), ctxt: SyntaxContext::root(), parent: None }
}
}

// The interner is pointed to by a thread local value which is only set on the main thread
// with parallelization is disabled. So we don't allow `Span` to transfer between threads
// to avoid panics and other errors, even though it would be memory safe to do so.
Expand Down
Loading

0 comments on commit c9795bd

Please sign in to comment.