Skip to content

Commit

Permalink
Rollup merge of #73169 - Amanieu:asm-warnings, r=petrochenkov
Browse files Browse the repository at this point in the history
Handle assembler warnings properly

Previously all inline asm diagnostics were treated as errors, but LLVM sometimes emits warnings and notes as well.

Fixes #73160

r? @petrochenkov
  • Loading branch information
Dylan-DPC authored Jun 11, 2020
2 parents 8d62099 + 5541f68 commit b341115
Show file tree
Hide file tree
Showing 9 changed files with 124 additions and 15 deletions.
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

0 comments on commit b341115

Please sign in to comment.