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.

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

Handle assembler warnings properly #73169

Merged
merged 1 commit into from
Jun 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 18 additions & 4 deletions src/librustc_codegen_llvm/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use rustc_codegen_ssa::back::write::{BitcodeSection, CodegenContext, EmitObj, Mo
use rustc_codegen_ssa::traits::*;
use rustc_codegen_ssa::{CompiledModule, ModuleCodegen};
use rustc_data_structures::small_c_str::SmallCStr;
use rustc_errors::{FatalError, Handler};
use rustc_errors::{FatalError, Handler, Level};
use rustc_fs_util::{link_or_copy, path_to_c_string};
use rustc_hir::def_id::LOCAL_CRATE;
use rustc_middle::bug;
Expand Down Expand Up @@ -242,6 +242,7 @@ impl<'a> Drop for DiagnosticHandlers<'a> {
fn report_inline_asm(
cgcx: &CodegenContext<LlvmCodegenBackend>,
msg: String,
level: llvm::DiagnosticLevel,
mut cookie: c_uint,
source: Option<(String, Vec<InnerSpan>)>,
) {
Expand All @@ -251,7 +252,12 @@ fn report_inline_asm(
if matches!(cgcx.lto, Lto::Fat | Lto::Thin) {
cookie = 0;
}
cgcx.diag_emitter.inline_asm_error(cookie as u32, msg, source);
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 as u32, msg, level, source);
}

unsafe extern "C" fn inline_asm_handler(diag: &SMDiagnostic, user: *const c_void, cookie: c_uint) {
Expand All @@ -264,6 +270,7 @@ unsafe extern "C" fn inline_asm_handler(diag: &SMDiagnostic, user: *const c_void
// diagnostics.
let mut have_source = false;
let mut buffer = String::new();
let mut level = llvm::DiagnosticLevel::Error;
let mut loc = 0;
let mut ranges = [0; 8];
let mut num_ranges = ranges.len() / 2;
Expand All @@ -273,6 +280,7 @@ unsafe extern "C" fn inline_asm_handler(diag: &SMDiagnostic, user: *const c_void
diag,
msg,
buffer,
&mut level,
&mut loc,
ranges.as_mut_ptr(),
&mut num_ranges,
Expand All @@ -290,7 +298,7 @@ unsafe extern "C" fn inline_asm_handler(diag: &SMDiagnostic, user: *const c_void
(buffer, spans)
});

report_inline_asm(cgcx, msg, cookie, source);
report_inline_asm(cgcx, msg, level, cookie, source);
}

unsafe extern "C" fn diagnostic_handler(info: &DiagnosticInfo, user: *mut c_void) {
Expand All @@ -301,7 +309,13 @@ 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, None);
report_inline_asm(
cgcx,
llvm::twine_to_string(inline.message),
inline.level,
inline.cookie,
None,
);
}

llvm::diagnostic::Optimization(opt) => {
Expand Down
12 changes: 10 additions & 2 deletions src/librustc_codegen_llvm/llvm/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ impl OptimizationDiagnostic<'ll> {

#[derive(Copy, Clone)]
pub struct InlineAsmDiagnostic<'ll> {
pub level: super::DiagnosticLevel,
pub cookie: c_uint,
pub message: &'ll Twine,
pub instruction: Option<&'ll Value>,
Expand All @@ -98,10 +99,17 @@ impl InlineAsmDiagnostic<'ll> {
let mut cookie = 0;
let mut message = None;
let mut instruction = None;
let mut level = super::DiagnosticLevel::Error;

super::LLVMRustUnpackInlineAsmDiagnostic(di, &mut cookie, &mut message, &mut instruction);
super::LLVMRustUnpackInlineAsmDiagnostic(
di,
&mut level,
&mut cookie,
&mut message,
&mut instruction,
);

InlineAsmDiagnostic { cookie, message: message.unwrap(), instruction }
InlineAsmDiagnostic { level, cookie, message: message.unwrap(), instruction }
}
}

Expand Down
13 changes: 13 additions & 0 deletions src/librustc_codegen_llvm/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,17 @@ pub enum DiagnosticKind {
Linker,
}

/// LLVMRustDiagnosticLevel
#[derive(Copy, Clone)]
#[repr(C)]
#[allow(dead_code)] // Variants constructed by C++.
pub enum DiagnosticLevel {
Error,
Warning,
Note,
Remark,
}

/// LLVMRustArchiveKind
#[derive(Copy, Clone)]
#[repr(C)]
Expand Down Expand Up @@ -2054,6 +2065,7 @@ extern "C" {

pub fn LLVMRustUnpackInlineAsmDiagnostic(
DI: &'a DiagnosticInfo,
level_out: &mut DiagnosticLevel,
cookie_out: &mut c_uint,
message_out: &mut Option<&'a Twine>,
instruction_out: &mut Option<&'a Value>,
Expand All @@ -2074,6 +2086,7 @@ extern "C" {
d: &SMDiagnostic,
message_out: &RustString,
buffer_out: &RustString,
level_out: &mut DiagnosticLevel,
loc_out: &mut c_uint,
ranges_out: *mut c_uint,
num_ranges: &mut usize,
Expand Down
20 changes: 13 additions & 7 deletions src/librustc_codegen_ssa/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1551,7 +1551,7 @@ fn spawn_work<B: ExtraBackendMethods>(cgcx: CodegenContext<B>, work: WorkItem<B>

enum SharedEmitterMessage {
Diagnostic(Diagnostic),
InlineAsmError(u32, String, Option<(String, Vec<InnerSpan>)>),
InlineAsmError(u32, String, Level, Option<(String, Vec<InnerSpan>)>),
AbortIfErrors,
Fatal(String),
}
Expand All @@ -1576,9 +1576,10 @@ impl SharedEmitter {
&self,
cookie: u32,
msg: String,
level: Level,
source: Option<(String, Vec<InnerSpan>)>,
) {
drop(self.sender.send(SharedEmitterMessage::InlineAsmError(cookie, msg, source)));
drop(self.sender.send(SharedEmitterMessage::InlineAsmError(cookie, msg, level, source)));
}

pub fn fatal(&self, msg: &str) {
Expand Down Expand Up @@ -1631,16 +1632,21 @@ impl SharedEmitterMain {
}
handler.emit_diagnostic(&d);
}
Ok(SharedEmitterMessage::InlineAsmError(cookie, msg, source)) => {
Ok(SharedEmitterMessage::InlineAsmError(cookie, msg, level, source)) => {
let msg = msg.strip_prefix("error: ").unwrap_or(&msg);

let mut err = match level {
Level::Error => sess.struct_err(&msg),
Level::Warning => sess.struct_warn(&msg),
Level::Note => sess.struct_note_without_error(&msg),
_ => bug!("Invalid inline asm diagnostic level"),
};

// If the cookie is 0 then we don't have span information.
let mut err = if cookie == 0 {
sess.struct_err(&msg)
} else {
if cookie != 0 {
let pos = BytePos::from_u32(cookie);
let span = Span::with_root_ctxt(pos, pos);
sess.struct_span_err(span, &msg)
err.set_span(span);
};

// Point to the generated assembly if it is available.
Expand Down
5 changes: 5 additions & 0 deletions src/librustc_errors/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,11 @@ impl Handler {
DiagnosticBuilder::new(self, Level::Help, msg)
}

/// Construct a builder at the `Note` level with the `msg`.
pub fn struct_note_without_error(&self, msg: &str) -> DiagnosticBuilder<'_> {
DiagnosticBuilder::new(self, Level::Note, msg)
}

pub fn span_fatal(&self, span: impl Into<MultiSpan>, msg: &str) -> FatalError {
self.emit_diag_at_span(Diagnostic::new(Fatal, msg), span);
FatalError
Expand Down
3 changes: 3 additions & 0 deletions src/librustc_session/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,9 @@ impl Session {
pub fn span_note_without_error<S: Into<MultiSpan>>(&self, sp: S, msg: &str) {
self.diagnostic().span_note_without_error(sp, msg)
}
pub fn struct_note_without_error(&self, msg: &str) -> DiagnosticBuilder<'_> {
self.diagnostic().struct_note_without_error(msg)
}

pub fn diagnostic(&self) -> &rustc_errors::Handler {
&self.parse_sess.span_diagnostic
Expand Down
47 changes: 46 additions & 1 deletion src/rustllvm/RustWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1094,8 +1094,17 @@ extern "C" void LLVMRustUnpackOptimizationDiagnostic(
MessageOS << Opt->getMsg();
}

enum class LLVMRustDiagnosticLevel {
Error,
Warning,
Note,
Remark,
};

extern "C" void
LLVMRustUnpackInlineAsmDiagnostic(LLVMDiagnosticInfoRef DI, unsigned *CookieOut,
LLVMRustUnpackInlineAsmDiagnostic(LLVMDiagnosticInfoRef DI,
LLVMRustDiagnosticLevel *LevelOut,
unsigned *CookieOut,
LLVMTwineRef *MessageOut,
LLVMValueRef *InstructionOut) {
// Undefined to call this not on an inline assembly diagnostic!
Expand All @@ -1105,6 +1114,23 @@ LLVMRustUnpackInlineAsmDiagnostic(LLVMDiagnosticInfoRef DI, unsigned *CookieOut,
*CookieOut = IA->getLocCookie();
*MessageOut = wrap(&IA->getMsgStr());
*InstructionOut = wrap(IA->getInstruction());

switch (IA->getSeverity()) {
case DS_Error:
*LevelOut = LLVMRustDiagnosticLevel::Error;
break;
case DS_Warning:
*LevelOut = LLVMRustDiagnosticLevel::Warning;
break;
case DS_Note:
*LevelOut = LLVMRustDiagnosticLevel::Note;
break;
case DS_Remark:
*LevelOut = LLVMRustDiagnosticLevel::Remark;
break;
default:
report_fatal_error("Invalid LLVMRustDiagnosticLevel value!");
}
}

extern "C" void LLVMRustWriteDiagnosticInfoToString(LLVMDiagnosticInfoRef DI,
Expand Down Expand Up @@ -1166,6 +1192,7 @@ extern "C" LLVMRustDiagnosticKind
LLVMRustGetDiagInfoKind(LLVMDiagnosticInfoRef DI) {
return toRust((DiagnosticKind)unwrap(DI)->getKind());
}

// This is kept distinct from LLVMGetTypeKind, because when
// a new type kind is added, the Rust-side enum must be
// updated or UB will result.
Expand Down Expand Up @@ -1219,13 +1246,31 @@ extern "C" void LLVMRustSetInlineAsmDiagnosticHandler(
extern "C" bool LLVMRustUnpackSMDiagnostic(LLVMSMDiagnosticRef DRef,
RustStringRef MessageOut,
RustStringRef BufferOut,
LLVMRustDiagnosticLevel* LevelOut,
unsigned* LocOut,
unsigned* RangesOut,
size_t* NumRanges) {
SMDiagnostic& D = *unwrap(DRef);
RawRustStringOstream MessageOS(MessageOut);
MessageOS << D.getMessage();

switch (D.getKind()) {
case SourceMgr::DK_Error:
*LevelOut = LLVMRustDiagnosticLevel::Error;
break;
case SourceMgr::DK_Warning:
*LevelOut = LLVMRustDiagnosticLevel::Warning;
break;
case SourceMgr::DK_Note:
*LevelOut = LLVMRustDiagnosticLevel::Note;
break;
case SourceMgr::DK_Remark:
*LevelOut = LLVMRustDiagnosticLevel::Remark;
break;
default:
report_fatal_error("Invalid LLVMRustDiagnosticLevel value!");
}

if (D.getLoc() == SMLoc())
return false;

Expand Down
3 changes: 3 additions & 0 deletions src/test/ui/asm/srcloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,8 @@ fn main() {

asm!(concat!("invalid", "_", "instruction"));
//~^ ERROR: invalid instruction mnemonic 'invalid_instruction'

asm!("movaps %xmm3, (%esi, 2)", options(att_syntax));
//~^ WARN: scale factor without index register is ignored
}
}
14 changes: 13 additions & 1 deletion src/test/ui/asm/srcloc.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,17 @@ note: instantiated into assembly here
LL | invalid_instruction
| ^^^^^^^^^^^^^^^^^^^

error: aborting due to 6 previous errors
warning: scale factor without index register is ignored
--> $DIR/srcloc.rs:41:15
|
LL | asm!("movaps %xmm3, (%esi, 2)", options(att_syntax));
| ^
|
note: instantiated into assembly here
--> <inline asm>:1:23
|
LL | movaps %xmm3, (%esi, 2)
| ^

error: aborting due to 6 previous errors; 1 warning emitted