From 9d60de93e2c5af1b69201b5e3bcf8943ae5df664 Mon Sep 17 00:00:00 2001 From: Keegan McAllister Date: Sat, 27 Sep 2014 01:33:36 -0700 Subject: [PATCH 1/2] Translate inline assembly errors back to source locations Fixes #17552. --- src/librustc/back/write.rs | 43 ++++++++++++++++--- src/librustc/driver/driver.rs | 2 + src/librustc/middle/trans/asm.rs | 14 ++++++ src/librustc_llvm/lib.rs | 9 ++++ src/libsyntax/ast.rs | 3 +- src/libsyntax/codemap.rs | 2 +- src/libsyntax/ext/asm.rs | 13 +++++- src/libsyntax/fold.rs | 6 ++- src/rustllvm/RustWrapper.cpp | 15 +++++++ src/rustllvm/rustllvm.h | 1 + .../compile-fail/asm-src-loc-codegen-units.rs | 20 +++++++++ src/test/compile-fail/asm-src-loc.rs | 17 ++++++++ 12 files changed, 133 insertions(+), 12 deletions(-) create mode 100644 src/test/compile-fail/asm-src-loc-codegen-units.rs create mode 100644 src/test/compile-fail/asm-src-loc.rs diff --git a/src/librustc/back/write.rs b/src/librustc/back/write.rs index 7242c12ae0c7a..8e703d954f300 100644 --- a/src/librustc/back/write.rs +++ b/src/librustc/back/write.rs @@ -16,6 +16,7 @@ use driver::session::Session; use driver::config; use llvm; use llvm::{ModuleRef, TargetMachineRef, PassManagerRef, DiagnosticInfoRef, ContextRef}; +use llvm::SMDiagnosticRef; use util::common::time; use syntax::abi; use syntax::codemap; @@ -326,14 +327,40 @@ impl<'a> CodegenContext<'a> { } } -struct DiagHandlerFreeVars<'a> { +struct HandlerFreeVars<'a> { llcx: ContextRef, cgcx: &'a CodegenContext<'a>, } +unsafe extern "C" fn inline_asm_handler(diag: SMDiagnosticRef, + user: *const c_void, + cookie: c_uint) { + use syntax::codemap::ExpnId; + + let HandlerFreeVars { cgcx, .. } + = *mem::transmute::<_, *const HandlerFreeVars>(user); + + let msg = llvm::build_string(|s| llvm::LLVMWriteSMDiagnosticToString(diag, s)) + .expect("non-UTF8 SMDiagnostic"); + + match cgcx.lto_ctxt { + Some((sess, _)) => { + sess.codemap().with_expn_info(ExpnId(cookie as u32), |info| match info { + Some(ei) => sess.span_err(ei.call_site, msg.as_slice()), + None => sess.err(msg.as_slice()), + }); + } + + None => { + cgcx.handler.err(msg.as_slice()); + cgcx.handler.note("build without -C codegen-units for more exact errors"); + } + } +} + unsafe extern "C" fn diagnostic_handler(info: DiagnosticInfoRef, user: *mut c_void) { - let DiagHandlerFreeVars { llcx, cgcx } - = *mem::transmute::<_, *const DiagHandlerFreeVars>(user); + let HandlerFreeVars { llcx, cgcx } + = *mem::transmute::<_, *const HandlerFreeVars>(user); match llvm::diagnostic::Diagnostic::unpack(info) { llvm::diagnostic::Optimization(opt) => { @@ -368,14 +395,16 @@ unsafe fn optimize_and_codegen(cgcx: &CodegenContext, let tm = config.tm; // llcx doesn't outlive this function, so we can put this on the stack. - let fv = DiagHandlerFreeVars { + let fv = HandlerFreeVars { llcx: llcx, cgcx: cgcx, }; + let fv = &fv as *const HandlerFreeVars as *mut c_void; + + llvm::LLVMSetInlineAsmDiagnosticHandler(llcx, inline_asm_handler, fv); + if !cgcx.remark.is_empty() { - llvm::LLVMContextSetDiagnosticHandler(llcx, diagnostic_handler, - &fv as *const DiagHandlerFreeVars - as *mut c_void); + llvm::LLVMContextSetDiagnosticHandler(llcx, diagnostic_handler, fv); } if config.emit_no_opt_bc { diff --git a/src/librustc/driver/driver.rs b/src/librustc/driver/driver.rs index 33e6579fb87dc..b0b445b590ca9 100644 --- a/src/librustc/driver/driver.rs +++ b/src/librustc/driver/driver.rs @@ -556,6 +556,8 @@ pub fn phase_5_run_llvm_passes(sess: &Session, sess.opts.output_types.as_slice(), outputs)); } + + sess.abort_if_errors(); } /// Run the linker on any artifacts that resulted from the LLVM run. diff --git a/src/librustc/middle/trans/asm.rs b/src/librustc/middle/trans/asm.rs index a5e6d606d7bc1..b4c10c78db874 100644 --- a/src/librustc/middle/trans/asm.rs +++ b/src/librustc/middle/trans/asm.rs @@ -25,6 +25,7 @@ use middle::trans::type_::Type; use std::c_str::ToCStr; use std::string::String; use syntax::ast; +use libc::{c_uint, c_char}; // Take an inline assembly expression and splat it out via LLVM pub fn trans_inline_asm<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, ia: &ast::InlineAsm) @@ -141,6 +142,19 @@ pub fn trans_inline_asm<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, ia: &ast::InlineAsm) } } + // Store expn_id in a metadata node so we can map LLVM errors + // back to source locations. See #17552. + unsafe { + let key = "srcloc"; + let kind = llvm::LLVMGetMDKindIDInContext(bcx.ccx().llcx(), + key.as_ptr() as *const c_char, key.len() as c_uint); + + let val: llvm::ValueRef = C_i32(bcx.ccx(), ia.expn_id as i32); + + llvm::LLVMSetMetadata(r, kind, + llvm::LLVMMDNodeInContext(bcx.ccx().llcx(), &val, 1)); + } + return bcx; } diff --git a/src/librustc_llvm/lib.rs b/src/librustc_llvm/lib.rs index c25ef2ca63a0b..401933d705849 100644 --- a/src/librustc_llvm/lib.rs +++ b/src/librustc_llvm/lib.rs @@ -424,8 +424,11 @@ pub enum DiagnosticInfo_opaque {} pub type DiagnosticInfoRef = *mut DiagnosticInfo_opaque; pub enum DebugLoc_opaque {} pub type DebugLocRef = *mut DebugLoc_opaque; +pub enum SMDiagnostic_opaque {} +pub type SMDiagnosticRef = *mut SMDiagnostic_opaque; pub type DiagnosticHandler = unsafe extern "C" fn(DiagnosticInfoRef, *mut c_void); +pub type InlineAsmDiagHandler = unsafe extern "C" fn(SMDiagnosticRef, *const c_void, c_uint); pub mod debuginfo { use super::{ValueRef}; @@ -1967,6 +1970,12 @@ extern { pub fn LLVMGetDiagInfoKind(DI: DiagnosticInfoRef) -> DiagnosticKind; pub fn LLVMWriteDebugLocToString(C: ContextRef, DL: DebugLocRef, s: RustStringRef); + + pub fn LLVMSetInlineAsmDiagnosticHandler(C: ContextRef, + H: InlineAsmDiagHandler, + CX: *mut c_void); + + pub fn LLVMWriteSMDiagnosticToString(d: SMDiagnosticRef, s: RustStringRef); } pub fn SetInstructionCallConv(instr: ValueRef, cc: CallConv) { diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 38d8136c1a13c..43d6b9b9e905a 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -983,7 +983,8 @@ pub struct InlineAsm { pub clobbers: InternedString, pub volatile: bool, pub alignstack: bool, - pub dialect: AsmDialect + pub dialect: AsmDialect, + pub expn_id: u32, } /// represents an argument in a function header diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index 9072889463cd6..d44de7862a33a 100644 --- a/src/libsyntax/codemap.rs +++ b/src/libsyntax/codemap.rs @@ -224,7 +224,7 @@ pub struct ExpnInfo { } #[deriving(PartialEq, Eq, Clone, Show, Hash)] -pub struct ExpnId(u32); +pub struct ExpnId(pub u32); pub static NO_EXPANSION: ExpnId = ExpnId(-1); diff --git a/src/libsyntax/ext/asm.rs b/src/libsyntax/ext/asm.rs index 4b8c3376cad2e..f82fe4b13a212 100644 --- a/src/libsyntax/ext/asm.rs +++ b/src/libsyntax/ext/asm.rs @@ -13,6 +13,7 @@ */ use ast; +use codemap; use codemap::Span; use ext::base; use ext::base::*; @@ -198,6 +199,15 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt, sp: Span, tts: &[ast::TokenTree]) } } + let codemap::ExpnId(expn_id) = cx.codemap().record_expansion(codemap::ExpnInfo { + call_site: sp, + callee: codemap::NameAndSpan { + name: "asm".to_string(), + format: codemap::MacroBang, + span: None, + }, + }); + MacExpr::new(P(ast::Expr { id: ast::DUMMY_NODE_ID, node: ast::ExprInlineAsm(ast::InlineAsm { @@ -208,7 +218,8 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt, sp: Span, tts: &[ast::TokenTree]) clobbers: token::intern_and_get_ident(cons.as_slice()), volatile: volatile, alignstack: alignstack, - dialect: dialect + dialect: dialect, + expn_id: expn_id, }), span: sp })) diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index 91a339a73f7af..53be7f2c20c4e 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -1279,7 +1279,8 @@ pub fn noop_fold_expr(Expr {id, node, span}: Expr, folder: &mut T) -> clobbers, volatile, alignstack, - dialect + dialect, + expn_id, }) => ExprInlineAsm(InlineAsm { inputs: inputs.move_map(|(c, input)| { (c, folder.fold_expr(input)) @@ -1292,7 +1293,8 @@ pub fn noop_fold_expr(Expr {id, node, span}: Expr, folder: &mut T) -> clobbers: clobbers, volatile: volatile, alignstack: alignstack, - dialect: dialect + dialect: dialect, + expn_id: expn_id, }), ExprMac(mac) => ExprMac(folder.fold_mac(mac)), ExprStruct(path, fields, maybe_expr) => { diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index 7896ce2ba761a..1fdaa548ebe6e 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -871,3 +871,18 @@ extern "C" void LLVMWriteDebugLocToString( raw_rust_string_ostream os(str); unwrap(dl)->print(*unwrap(C), os); } + +DEFINE_SIMPLE_CONVERSION_FUNCTIONS(SMDiagnostic, LLVMSMDiagnosticRef) + +extern "C" void LLVMSetInlineAsmDiagnosticHandler( + LLVMContextRef C, + LLVMContext::InlineAsmDiagHandlerTy H, + void *CX) +{ + unwrap(C)->setInlineAsmDiagnosticHandler(H, CX); +} + +extern "C" void LLVMWriteSMDiagnosticToString(LLVMSMDiagnosticRef d, RustStringRef str) { + raw_rust_string_ostream os(str); + unwrap(d)->print("", os); +} diff --git a/src/rustllvm/rustllvm.h b/src/rustllvm/rustllvm.h index 54b0c2506c76b..5469531c54188 100644 --- a/src/rustllvm/rustllvm.h +++ b/src/rustllvm/rustllvm.h @@ -73,6 +73,7 @@ void LLVMRustSetLastError(const char*); typedef struct OpaqueRustString *RustStringRef; typedef struct LLVMOpaqueTwine *LLVMTwineRef; typedef struct LLVMOpaqueDebugLoc *LLVMDebugLocRef; +typedef struct LLVMOpaqueSMDiagnostic *LLVMSMDiagnosticRef; extern "C" void rust_llvm_string_write_impl(RustStringRef str, const char *ptr, size_t size); diff --git a/src/test/compile-fail/asm-src-loc-codegen-units.rs b/src/test/compile-fail/asm-src-loc-codegen-units.rs new file mode 100644 index 0000000000000..1b8fb32a808dc --- /dev/null +++ b/src/test/compile-fail/asm-src-loc-codegen-units.rs @@ -0,0 +1,20 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. +// +// compile-flags: -C codegen-units=2 +// error-pattern: build without -C codegen-units for more exact errors + +#![feature(asm)] + +fn main() { + unsafe { + asm!("nowayisthisavalidinstruction"); + } +} diff --git a/src/test/compile-fail/asm-src-loc.rs b/src/test/compile-fail/asm-src-loc.rs new file mode 100644 index 0000000000000..8da6cca77cc4c --- /dev/null +++ b/src/test/compile-fail/asm-src-loc.rs @@ -0,0 +1,17 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![feature(asm)] + +fn main() { + unsafe { + asm!("nowayisthisavalidinstruction"); //~ ERROR invalid instruction mnemonic + } +} From 8826fdfe37a7cbf901ddced1d7e2b4320e117461 Mon Sep 17 00:00:00 2001 From: Keegan McAllister Date: Sun, 28 Sep 2014 09:25:48 -0700 Subject: [PATCH 2/2] Keep ExpnId abstract by providing conversions --- mk/crates.mk | 2 +- src/librustc/back/write.rs | 2 +- src/librustc/middle/trans/asm.rs | 2 +- src/libsyntax/ast.rs | 4 ++-- src/libsyntax/codemap.rs | 16 ++++++++++++++-- src/libsyntax/ext/asm.rs | 2 +- src/libsyntax/lib.rs | 1 + 7 files changed, 21 insertions(+), 8 deletions(-) diff --git a/mk/crates.mk b/mk/crates.mk index ed3fce775f31e..9f01ff23c7fc9 100644 --- a/mk/crates.mk +++ b/mk/crates.mk @@ -71,7 +71,7 @@ DEPS_graphviz := std DEPS_green := std native:context_switch DEPS_rustuv := std native:uv native:uv_support DEPS_native := std -DEPS_syntax := std term serialize log fmt_macros debug arena +DEPS_syntax := std term serialize log fmt_macros debug arena libc DEPS_rustc := syntax flate arena serialize getopts rbml \ time log graphviz debug rustc_llvm rustc_back DEPS_rustc_llvm := native:rustllvm libc std diff --git a/src/librustc/back/write.rs b/src/librustc/back/write.rs index 8e703d954f300..7b4d1780ccd69 100644 --- a/src/librustc/back/write.rs +++ b/src/librustc/back/write.rs @@ -345,7 +345,7 @@ unsafe extern "C" fn inline_asm_handler(diag: SMDiagnosticRef, match cgcx.lto_ctxt { Some((sess, _)) => { - sess.codemap().with_expn_info(ExpnId(cookie as u32), |info| match info { + sess.codemap().with_expn_info(ExpnId::from_llvm_cookie(cookie), |info| match info { Some(ei) => sess.span_err(ei.call_site, msg.as_slice()), None => sess.err(msg.as_slice()), }); diff --git a/src/librustc/middle/trans/asm.rs b/src/librustc/middle/trans/asm.rs index b4c10c78db874..c51e242026241 100644 --- a/src/librustc/middle/trans/asm.rs +++ b/src/librustc/middle/trans/asm.rs @@ -149,7 +149,7 @@ pub fn trans_inline_asm<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, ia: &ast::InlineAsm) let kind = llvm::LLVMGetMDKindIDInContext(bcx.ccx().llcx(), key.as_ptr() as *const c_char, key.len() as c_uint); - let val: llvm::ValueRef = C_i32(bcx.ccx(), ia.expn_id as i32); + let val: llvm::ValueRef = C_i32(bcx.ccx(), ia.expn_id.to_llvm_cookie()); llvm::LLVMSetMetadata(r, kind, llvm::LLVMMDNodeInContext(bcx.ccx().llcx(), &val, 1)); diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index 43d6b9b9e905a..0fee3ff321850 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -10,7 +10,7 @@ // The Rust abstract syntax tree. -use codemap::{Span, Spanned, DUMMY_SP}; +use codemap::{Span, Spanned, DUMMY_SP, ExpnId}; use abi::Abi; use ast_util; use owned_slice::OwnedSlice; @@ -984,7 +984,7 @@ pub struct InlineAsm { pub volatile: bool, pub alignstack: bool, pub dialect: AsmDialect, - pub expn_id: u32, + pub expn_id: ExpnId, } /// represents an argument in a function header diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index d44de7862a33a..e9b2556c53e25 100644 --- a/src/libsyntax/codemap.rs +++ b/src/libsyntax/codemap.rs @@ -26,6 +26,7 @@ source code snippets, etc. use serialize::{Encodable, Decodable, Encoder, Decoder}; use std::cell::RefCell; use std::rc::Rc; +use libc::c_uint; pub trait Pos { fn from_uint(n: uint) -> Self; @@ -223,11 +224,22 @@ pub struct ExpnInfo { pub callee: NameAndSpan } -#[deriving(PartialEq, Eq, Clone, Show, Hash)] -pub struct ExpnId(pub u32); +#[deriving(PartialEq, Eq, Clone, Show, Hash, Encodable, Decodable)] +pub struct ExpnId(u32); pub static NO_EXPANSION: ExpnId = ExpnId(-1); +impl ExpnId { + pub fn from_llvm_cookie(cookie: c_uint) -> ExpnId { + ExpnId(cookie as u32) + } + + pub fn to_llvm_cookie(self) -> i32 { + let ExpnId(cookie) = self; + cookie as i32 + } +} + pub type FileName = String; pub struct FileLines { diff --git a/src/libsyntax/ext/asm.rs b/src/libsyntax/ext/asm.rs index f82fe4b13a212..702be0c0eeede 100644 --- a/src/libsyntax/ext/asm.rs +++ b/src/libsyntax/ext/asm.rs @@ -199,7 +199,7 @@ pub fn expand_asm<'cx>(cx: &'cx mut ExtCtxt, sp: Span, tts: &[ast::TokenTree]) } } - let codemap::ExpnId(expn_id) = cx.codemap().record_expansion(codemap::ExpnInfo { + let expn_id = cx.codemap().record_expansion(codemap::ExpnInfo { call_site: sp, callee: codemap::NameAndSpan { name: "asm".to_string(), diff --git a/src/libsyntax/lib.rs b/src/libsyntax/lib.rs index 7a504d22c1e9e..a427154414654 100644 --- a/src/libsyntax/lib.rs +++ b/src/libsyntax/lib.rs @@ -33,6 +33,7 @@ extern crate debug; #[phase(plugin, link)] extern crate log; extern crate serialize; extern crate term; +extern crate libc; pub mod util { pub mod interner;