From abed45ff9fa3e68f2a32ca12e012f95b9153f4df Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Mon, 17 Feb 2020 21:36:01 +0000 Subject: Implement asm! codegen --- src/rustllvm/RustWrapper.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/rustllvm/RustWrapper.cpp') diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index 28efc8bf5dd..b988d06871b 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -203,6 +203,10 @@ static Attribute::AttrKind fromRust(LLVMRustAttribute Kind) { return Attribute::OptimizeNone; case ReturnsTwice: return Attribute::ReturnsTwice; + case ReadNone: + return Attribute::ReadNone; + case InaccessibleMemOnly: + return Attribute::InaccessibleMemOnly; } report_fatal_error("bad AttributeKind"); } -- cgit 1.4.1-3-g733a5 From d0a48d19f5e10869ea4a137d4bb3b84d62966e31 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 21 May 2020 20:53:41 +0300 Subject: rustllvm: Fix warnings about unused function parameters --- src/librustc_codegen_llvm/back/write.rs | 3 --- src/librustc_codegen_llvm/context.rs | 20 +++++--------------- src/librustc_codegen_llvm/debuginfo/metadata.rs | 12 ++---------- src/librustc_codegen_llvm/debuginfo/mod.rs | 9 ++------- src/librustc_codegen_llvm/llvm/ffi.rs | 6 ------ src/rustllvm/PassWrapper.cpp | 2 +- src/rustllvm/RustWrapper.cpp | 7 ++----- 7 files changed, 12 insertions(+), 47 deletions(-) (limited to 'src/rustllvm/RustWrapper.cpp') diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index 6ed9cd69738..57e018bba6a 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -6,7 +6,6 @@ use crate::back::profiling::{ use crate::base; use crate::common; use crate::consts; -use crate::context::all_outputs_are_pic_executables; use crate::llvm::{self, DiagnosticInfo, PassManager, SMDiagnostic}; use crate::llvm_util; use crate::type_::Type; @@ -150,7 +149,6 @@ pub fn target_machine_factory( let features = features.join(","); let features = CString::new(features).unwrap(); let abi = SmallCStr::new(&sess.target.target.options.llvm_abiname); - let pic_is_pie = all_outputs_are_pic_executables(sess); let trap_unreachable = sess.target.target.options.trap_unreachable; let emit_stack_size_section = sess.opts.debugging_opts.emit_stack_sizes; @@ -174,7 +172,6 @@ pub fn target_machine_factory( reloc_model, opt_level, use_softfp, - pic_is_pie, ffunction_sections, fdata_sections, trap_unreachable, diff --git a/src/librustc_codegen_llvm/context.rs b/src/librustc_codegen_llvm/context.rs index 01f90cae7a5..3192d4fc157 100644 --- a/src/librustc_codegen_llvm/context.rs +++ b/src/librustc_codegen_llvm/context.rs @@ -97,17 +97,6 @@ fn to_llvm_tls_model(tls_model: TlsModel) -> llvm::ThreadLocalMode { } } -/// PIE is potentially more effective than PIC, but can only be used in executables. -/// If all our outputs are executables, then we can relax PIC to PIE when producing object code. -/// If the list of crate types is not yet known we conservatively return `false`. -pub fn all_outputs_are_pic_executables(sess: &Session) -> bool { - sess.relocation_model() == RelocModel::Pic - && sess - .crate_types - .try_get() - .map_or(false, |crate_types| crate_types.iter().all(|ty| *ty == CrateType::Executable)) -} - fn strip_function_ptr_alignment(data_layout: String) -> String { // FIXME: Make this more general. data_layout.replace("-Fi8-", "-") @@ -183,10 +172,11 @@ pub unsafe fn create_module( if sess.relocation_model() == RelocModel::Pic { llvm::LLVMRustSetModulePICLevel(llmod); - } - - if all_outputs_are_pic_executables(sess) { - llvm::LLVMRustSetModulePIELevel(llmod); + // PIE is potentially more effective than PIC, but can only be used in executables. + // If all our outputs are executables, then we can relax PIC to PIE. + if sess.crate_types.get().iter().all(|ty| *ty == CrateType::Executable) { + llvm::LLVMRustSetModulePIELevel(llmod); + } } // If skipping the PLT is enabled, we need to add some module metadata diff --git a/src/librustc_codegen_llvm/debuginfo/metadata.rs b/src/librustc_codegen_llvm/debuginfo/metadata.rs index fb9a27ed001..0cce0b25e58 100644 --- a/src/librustc_codegen_llvm/debuginfo/metadata.rs +++ b/src/librustc_codegen_llvm/debuginfo/metadata.rs @@ -447,7 +447,6 @@ fn subroutine_type_metadata( unsafe { llvm::LLVMRustDIBuilderCreateSubroutineType( DIB(cx), - unknown_file_metadata(cx), create_DIArray(DIB(cx), &signature_metadata[..]), ) }, @@ -635,14 +634,12 @@ pub fn type_metadata(cx: &CodegenCx<'ll, 'tcx>, t: Ty<'tcx>, usage_site_span: Sp // anything reading the debuginfo for a recursive // type is going to see *something* weird - the only // question is what exactly it will see. - let (size, align) = cx.size_and_align_of(t); let name = ""; llvm::LLVMRustDIBuilderCreateBasicType( DIB(cx), name.as_ptr().cast(), name.len(), - size.bits(), - align.bits() as u32, + cx.size_of(t).bits(), DW_ATE_unsigned, ) } @@ -841,14 +838,12 @@ fn basic_type_metadata(cx: &CodegenCx<'ll, 'tcx>, t: Ty<'tcx>) -> &'ll DIType { _ => bug!("debuginfo::basic_type_metadata - `t` is invalid type"), }; - let (size, align) = cx.size_and_align_of(t); let ty_metadata = unsafe { llvm::LLVMRustDIBuilderCreateBasicType( DIB(cx), name.as_ptr().cast(), name.len(), - size.bits(), - align.bits() as u32, + cx.size_of(t).bits(), encoding, ) }; @@ -2187,9 +2182,6 @@ fn compute_type_parameters(cx: &CodegenCx<'ll, 'tcx>, ty: Ty<'tcx>) -> Option<&' name.as_ptr().cast(), name.len(), actual_type_metadata, - unknown_file_metadata(cx), - 0, - 0, )) }) } else { diff --git a/src/librustc_codegen_llvm/debuginfo/mod.rs b/src/librustc_codegen_llvm/debuginfo/mod.rs index 8c9a2c09c27..8c580847ef8 100644 --- a/src/librustc_codegen_llvm/debuginfo/mod.rs +++ b/src/librustc_codegen_llvm/debuginfo/mod.rs @@ -252,7 +252,7 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { let function_type_metadata = unsafe { let fn_signature = get_function_signature(self, fn_abi); - llvm::LLVMRustDIBuilderCreateSubroutineType(DIB(self), file_metadata, fn_signature) + llvm::LLVMRustDIBuilderCreateSubroutineType(DIB(self), fn_signature) }; // Find the enclosing function, in case this is a closure. @@ -265,8 +265,7 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { // name if necessary. let generics = self.tcx().generics_of(enclosing_fn_def_id); let substs = instance.substs.truncate_to(self.tcx(), generics); - let template_parameters = - get_template_parameters(self, &generics, substs, file_metadata, &mut name); + let template_parameters = get_template_parameters(self, &generics, substs, &mut name); // Get the linkage_name, which is just the symbol name let linkage_name = mangled_name_of_instance(self, instance); @@ -388,7 +387,6 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { cx: &CodegenCx<'ll, 'tcx>, generics: &ty::Generics, substs: SubstsRef<'tcx>, - file_metadata: &'ll DIFile, name_to_append_suffix_to: &mut String, ) -> &'ll DIArray { if substs.types().next().is_none() { @@ -429,9 +427,6 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { name.as_ptr().cast(), name.len(), actual_type_metadata, - file_metadata, - 0, - 0, )) }) } else { diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs index 9cb0f0e0c2e..3fb7ff3cb8d 100644 --- a/src/librustc_codegen_llvm/llvm/ffi.rs +++ b/src/librustc_codegen_llvm/llvm/ffi.rs @@ -1655,7 +1655,6 @@ extern "C" { pub fn LLVMRustDIBuilderCreateSubroutineType( Builder: &DIBuilder<'a>, - File: &'a DIFile, ParameterTypes: &'a DIArray, ) -> &'a DICompositeType; @@ -1682,7 +1681,6 @@ extern "C" { Name: *const c_char, NameLen: size_t, SizeInBits: u64, - AlignInBits: u32, Encoding: c_uint, ) -> &'a DIBasicType; @@ -1880,9 +1878,6 @@ extern "C" { Name: *const c_char, NameLen: size_t, Ty: &'a DIType, - File: &'a DIFile, - LineNo: c_uint, - ColumnNo: c_uint, ) -> &'a DITemplateTypeParameter; pub fn LLVMRustDIBuilderCreateNameSpace( @@ -1948,7 +1943,6 @@ extern "C" { Reloc: RelocModel, Level: CodeGenOptLevel, UseSoftFP: bool, - PositionIndependentExecutable: bool, FunctionSections: bool, DataSections: bool, TrapUnreachable: bool, diff --git a/src/rustllvm/PassWrapper.cpp b/src/rustllvm/PassWrapper.cpp index 6c638c5453a..02dcfb8e829 100644 --- a/src/rustllvm/PassWrapper.cpp +++ b/src/rustllvm/PassWrapper.cpp @@ -445,7 +445,7 @@ extern "C" LLVMTargetMachineRef LLVMRustCreateTargetMachine( const char *TripleStr, const char *CPU, const char *Feature, const char *ABIStr, LLVMRustCodeModel RustCM, LLVMRustRelocModel RustReloc, LLVMRustCodeGenOptLevel RustOptLevel, bool UseSoftFloat, - bool PositionIndependentExecutable, bool FunctionSections, + bool FunctionSections, bool DataSections, bool TrapUnreachable, bool Singlethread, diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index b988d06871b..24f35627d10 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -720,7 +720,6 @@ extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateFile( extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateSubroutineType(LLVMRustDIBuilderRef Builder, - LLVMMetadataRef File, LLVMMetadataRef ParameterTypes) { return wrap(Builder->createSubroutineType( DITypeRefArray(unwrap(ParameterTypes)))); @@ -755,7 +754,7 @@ extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateFunction( extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateBasicType( LLVMRustDIBuilderRef Builder, const char *Name, size_t NameLen, - uint64_t SizeInBits, uint32_t AlignInBits, unsigned Encoding) { + uint64_t SizeInBits, unsigned Encoding) { return wrap(Builder->createBasicType(StringRef(Name, NameLen), SizeInBits, Encoding)); } @@ -964,9 +963,7 @@ extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateUnionType( extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateTemplateTypeParameter( LLVMRustDIBuilderRef Builder, LLVMMetadataRef Scope, - const char *Name, size_t NameLen, - LLVMMetadataRef Ty, LLVMMetadataRef File, unsigned LineNo, - unsigned ColumnNo) { + const char *Name, size_t NameLen, LLVMMetadataRef Ty) { return wrap(Builder->createTemplateTypeParameter( unwrapDI(Scope), StringRef(Name, NameLen), unwrapDI(Ty))); } -- cgit 1.4.1-3-g733a5 From b78b15665b622cc37b25e9bd971537296403b83d Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Tue, 26 May 2020 20:07:59 +0100 Subject: Improve inline asm error diagnostics --- src/libfmt_macros/lib.rs | 24 +++++++- src/librustc_ast/ast.rs | 3 +- src/librustc_ast_lowering/expr.rs | 3 +- src/librustc_builtin_macros/asm.rs | 10 +++- src/librustc_codegen_llvm/asm.rs | 32 +++++++--- src/librustc_codegen_llvm/back/write.rs | 51 +++++++++++++--- src/librustc_codegen_llvm/llvm/ffi.rs | 9 ++- src/librustc_codegen_ssa/back/write.rs | 39 +++++++++++-- src/librustc_codegen_ssa/mir/block.rs | 12 +++- src/librustc_codegen_ssa/traits/asm.rs | 2 +- src/librustc_hir/hir.rs | 1 + src/librustc_middle/mir/mod.rs | 6 +- src/librustc_middle/mir/type_foldable.rs | 10 +++- src/librustc_middle/mir/visit.rs | 1 + src/librustc_mir/borrow_check/invalidation.rs | 8 ++- src/librustc_mir/borrow_check/mod.rs | 8 ++- src/librustc_mir/dataflow/framework/direction.rs | 2 +- src/librustc_mir/dataflow/move_paths/builder.rs | 8 ++- src/librustc_mir_build/build/expr/into.rs | 3 +- src/librustc_mir_build/hair/cx/expr.rs | 1 + src/librustc_mir_build/hair/mod.rs | 1 + src/librustc_span/lib.rs | 12 +++- src/rustllvm/RustWrapper.cpp | 31 ++++++++-- src/test/ui/asm/srcloc.rs | 41 +++++++++++++ src/test/ui/asm/srcloc.stderr | 74 ++++++++++++++++++++++++ src/test/ui/issues/issue-23458.stderr | 15 +++-- src/test/ui/llvm-asm/issue-69092.rs | 2 +- src/test/ui/llvm-asm/issue-69092.stderr | 13 +++-- 28 files changed, 365 insertions(+), 57 deletions(-) create mode 100644 src/test/ui/asm/srcloc.rs create mode 100644 src/test/ui/asm/srcloc.stderr (limited to 'src/rustllvm/RustWrapper.cpp') diff --git a/src/libfmt_macros/lib.rs b/src/libfmt_macros/lib.rs index 677c027f17b..23bf7b35419 100644 --- a/src/libfmt_macros/lib.rs +++ b/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, } 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(); } } diff --git a/src/librustc_ast/ast.rs b/src/librustc_ast/ast.rs index 30bb5c0bffa..efcf95ec706 100644 --- a/src/librustc_ast/ast.rs +++ b/src/librustc_ast/ast.rs @@ -1252,7 +1252,7 @@ pub enum ExprKind { Ret(Option>), /// Output of the `asm!()` macro. - InlineAsm(InlineAsm), + InlineAsm(P), /// Output of the `llvm_asm!()` macro. LlvmInlineAsm(P), @@ -1971,6 +1971,7 @@ pub struct InlineAsm { pub template: Vec, pub operands: Vec<(InlineAsmOperand, Span)>, pub options: InlineAsmOptions, + pub line_spans: Vec, } /// Inline assembly dialect. diff --git a/src/librustc_ast_lowering/expr.rs b/src/librustc_ast_lowering/expr.rs index 5bcd111706f..fd69f5c1e5f 100644 --- a/src/librustc_ast_lowering/expr.rs +++ b/src/librustc_ast_lowering/expr.rs @@ -1265,7 +1265,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)) } diff --git a/src/librustc_builtin_macros/asm.rs b/src/librustc_builtin_macros/asm.rs index 224b52b239f..19fae635572 100644 --- a/src/librustc_builtin_macros/asm.rs +++ b/src/librustc_builtin_macros/asm.rs @@ -513,10 +513,16 @@ fn expand_preparsed_asm(ecx: &mut ExtCtxt<'_>, sp: Span, args: AsmArgs) -> P 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(); @@ -286,9 +286,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) { @@ -340,7 +340,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 }; @@ -381,8 +381,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 { diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index 57e018bba6a..02a9294930d 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/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, - msg: &str, - cookie: c_uint, + msg: String, + mut cookie: c_uint, + source: Option<(String, Vec)>, ) { - 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, &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) => { diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs index 3fb7ff3cb8d..759c2bf1b85 100644 --- a/src/librustc_codegen_llvm/llvm/ffi.rs +++ b/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, diff --git a/src/librustc_codegen_ssa/back/write.rs b/src/librustc_codegen_ssa/back/write.rs index 9e03c283cfb..cb5c95c11fa 100644 --- a/src/librustc_codegen_ssa/back/write.rs +++ b/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(cgcx: CodegenContext, work: WorkItem enum SharedEmitterMessage { Diagnostic(Diagnostic), - InlineAsmError(u32, String), + InlineAsmError(u32, String, Option<(String, Vec)>), 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)>, + ) { + 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(); diff --git a/src/librustc_codegen_ssa/mir/block.rs b/src/librustc_codegen_ssa/mir/block.rs index b487ed8dea8..e0c6fb451fc 100644 --- a/src/librustc_codegen_ssa/mir/block.rs +++ b/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, ) { let span = terminator.source_info.span; @@ -931,7 +932,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); @@ -1034,7 +1035,13 @@ 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, @@ -1042,6 +1049,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { template, operands, options, + line_spans, destination, ); } diff --git a/src/librustc_codegen_ssa/traits/asm.rs b/src/librustc_codegen_ssa/traits/asm.rs index 0abfdfde780..b6b57744f95 100644 --- a/src/librustc_codegen_ssa/traits/asm.rs +++ b/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], ); } diff --git a/src/librustc_hir/hir.rs b/src/librustc_hir/hir.rs index 35cff668581..0194dc9f90b 100644 --- a/src/librustc_hir/hir.rs +++ b/src/librustc_hir/hir.rs @@ -2106,6 +2106,7 @@ pub struct InlineAsm<'hir> { pub template: &'hir [InlineAsmTemplatePiece], pub operands: &'hir [InlineAsmOperand<'hir>], pub options: InlineAsmOptions, + pub line_spans: &'hir [Span], } #[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug, HashStable_Generic, PartialEq)] diff --git a/src/librustc_middle/mir/mod.rs b/src/librustc_middle/mir/mod.rs index 47cfa62abb1..f6a236d38ec 100644 --- a/src/librustc_middle/mir/mod.rs +++ b/src/librustc_middle/mir/mod.rs @@ -1193,6 +1193,10 @@ pub enum TerminatorKind<'tcx> { /// Miscellaneous options for the inline assembly. options: InlineAsmOptions, + /// Source spans for each line of the inline assembly code. These are + /// used to map assembler errors back to the line in the source code. + line_spans: &'tcx [Span], + /// Destination block after the inline assembly returns, unless it is /// diverging (InlineAsmOptions::NORETURN). destination: Option, @@ -1595,7 +1599,7 @@ impl<'tcx> TerminatorKind<'tcx> { } FalseEdges { .. } => write!(fmt, "falseEdges"), FalseUnwind { .. } => write!(fmt, "falseUnwind"), - InlineAsm { template, ref operands, options, destination: _ } => { + InlineAsm { template, ref operands, options, .. } => { write!(fmt, "asm!(\"{}\"", InlineAsmTemplatePiece::to_string(template))?; for op in operands { write!(fmt, ", ")?; diff --git a/src/librustc_middle/mir/type_foldable.rs b/src/librustc_middle/mir/type_foldable.rs index bb7001c1207..b0207b469fa 100644 --- a/src/librustc_middle/mir/type_foldable.rs +++ b/src/librustc_middle/mir/type_foldable.rs @@ -78,9 +78,13 @@ impl<'tcx> TypeFoldable<'tcx> for Terminator<'tcx> { FalseEdges { real_target, imaginary_target } } FalseUnwind { real_target, unwind } => FalseUnwind { real_target, unwind }, - InlineAsm { template, ref operands, options, destination } => { - InlineAsm { template, operands: operands.fold_with(folder), options, destination } - } + InlineAsm { template, ref operands, options, line_spans, destination } => InlineAsm { + template, + operands: operands.fold_with(folder), + options, + line_spans, + destination, + }, }; Terminator { source_info: self.source_info, kind } } diff --git a/src/librustc_middle/mir/visit.rs b/src/librustc_middle/mir/visit.rs index a29b7b75294..035e6e55a97 100644 --- a/src/librustc_middle/mir/visit.rs +++ b/src/librustc_middle/mir/visit.rs @@ -535,6 +535,7 @@ macro_rules! make_mir_visitor { template: _, operands, options: _, + line_spans: _, destination: _, } => { for op in operands { diff --git a/src/librustc_mir/borrow_check/invalidation.rs b/src/librustc_mir/borrow_check/invalidation.rs index 178e3db17cd..0b59e29b66c 100644 --- a/src/librustc_mir/borrow_check/invalidation.rs +++ b/src/librustc_mir/borrow_check/invalidation.rs @@ -183,7 +183,13 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { } } } - TerminatorKind::InlineAsm { template: _, ref operands, options: _, destination: _ } => { + TerminatorKind::InlineAsm { + template: _, + ref operands, + options: _, + line_spans: _, + destination: _, + } => { for op in operands { match *op { InlineAsmOperand::In { reg: _, ref value } diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index a0c1d96bb47..525c054a766 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -724,7 +724,13 @@ impl<'cx, 'tcx> dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtxt<'cx, 'tc self.mutate_place(loc, (resume_arg, span), Deep, JustWrite, flow_state); } - TerminatorKind::InlineAsm { template: _, ref operands, options: _, destination: _ } => { + TerminatorKind::InlineAsm { + template: _, + ref operands, + options: _, + line_spans: _, + destination: _, + } => { for op in operands { match *op { InlineAsmOperand::In { reg: _, ref value } diff --git a/src/librustc_mir/dataflow/framework/direction.rs b/src/librustc_mir/dataflow/framework/direction.rs index 97b14ea771b..9e2a28853e1 100644 --- a/src/librustc_mir/dataflow/framework/direction.rs +++ b/src/librustc_mir/dataflow/framework/direction.rs @@ -482,7 +482,7 @@ impl Direction for Forward { } } - InlineAsm { template: _, operands: _, options: _, destination } => { + InlineAsm { template: _, operands: _, options: _, line_spans: _, destination } => { if let Some(target) = destination { propagate(target, exit_state); } diff --git a/src/librustc_mir/dataflow/move_paths/builder.rs b/src/librustc_mir/dataflow/move_paths/builder.rs index 427ab1ca5cd..e35d853c928 100644 --- a/src/librustc_mir/dataflow/move_paths/builder.rs +++ b/src/librustc_mir/dataflow/move_paths/builder.rs @@ -411,7 +411,13 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { self.gather_init(destination.as_ref(), InitKind::NonPanicPathOnly); } } - TerminatorKind::InlineAsm { template: _, ref operands, options: _, destination: _ } => { + TerminatorKind::InlineAsm { + template: _, + ref operands, + options: _, + line_spans: _, + destination: _ + } => { for op in operands { match *op { InlineAsmOperand::In { reg: _, ref value } diff --git a/src/librustc_mir_build/build/expr/into.rs b/src/librustc_mir_build/build/expr/into.rs index ff3c7ee3ee8..e7733deee4d 100644 --- a/src/librustc_mir_build/build/expr/into.rs +++ b/src/librustc_mir_build/build/expr/into.rs @@ -310,7 +310,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ); block.unit() } - ExprKind::InlineAsm { template, operands, options } => { + ExprKind::InlineAsm { template, operands, options, line_spans } => { use crate::hair; use rustc_middle::mir; let operands = operands @@ -368,6 +368,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { template, operands, options, + line_spans, destination: if options.contains(InlineAsmOptions::NORETURN) { None } else { diff --git a/src/librustc_mir_build/hair/cx/expr.rs b/src/librustc_mir_build/hair/cx/expr.rs index 114bf571040..056cb9d98ce 100644 --- a/src/librustc_mir_build/hair/cx/expr.rs +++ b/src/librustc_mir_build/hair/cx/expr.rs @@ -513,6 +513,7 @@ fn make_mirror_unadjusted<'a, 'tcx>( }) .collect(), options: asm.options, + line_spans: asm.line_spans, }, hir::ExprKind::LlvmInlineAsm(ref asm) => ExprKind::LlvmInlineAsm { diff --git a/src/librustc_mir_build/hair/mod.rs b/src/librustc_mir_build/hair/mod.rs index aba7a7a1b42..0a1c68e83a9 100644 --- a/src/librustc_mir_build/hair/mod.rs +++ b/src/librustc_mir_build/hair/mod.rs @@ -283,6 +283,7 @@ crate enum ExprKind<'tcx> { template: &'tcx [InlineAsmTemplatePiece], operands: Vec>, options: InlineAsmOptions, + line_spans: &'tcx [Span], }, LlvmInlineAsm { asm: &'tcx hir::LlvmInlineAsmInner, diff --git a/src/librustc_span/lib.rs b/src/librustc_span/lib.rs index 58cdb87158a..616876d4b02 100644 --- a/src/librustc_span/lib.rs +++ b/src/librustc_span/lib.rs @@ -101,6 +101,8 @@ pub enum FileName { /// Custom sources for explicit parser calls from plugins and drivers. Custom(String), DocTest(PathBuf, isize), + /// Post-substitution inline assembly from LLVM + InlineAsm(u64), } impl std::fmt::Display for FileName { @@ -116,6 +118,7 @@ impl std::fmt::Display for FileName { CliCrateAttr(_) => write!(fmt, ""), Custom(ref s) => write!(fmt, "<{}>", s), DocTest(ref path, _) => write!(fmt, "{}", path.display()), + InlineAsm(_) => write!(fmt, ""), } } } @@ -139,7 +142,8 @@ impl FileName { | CliCrateAttr(_) | Custom(_) | QuoteExpansion(_) - | DocTest(_, _) => false, + | DocTest(_, _) + | InlineAsm(_) => false, } } @@ -182,6 +186,12 @@ impl FileName { pub fn doc_test_source_code(path: PathBuf, line: isize) -> FileName { FileName::DocTest(path, line) } + + pub fn inline_asm_source_code(src: &str) -> FileName { + let mut hasher = StableHasher::new(); + src.hash(&mut hasher); + FileName::InlineAsm(hasher.finish()) + } } /// Spans represent a region of code, used for error reporting. Positions in spans diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index 24f35627d10..6fac2662506 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -1216,10 +1216,33 @@ extern "C" void LLVMRustSetInlineAsmDiagnosticHandler( unwrap(C)->setInlineAsmDiagnosticHandler(H, CX); } -extern "C" void LLVMRustWriteSMDiagnosticToString(LLVMSMDiagnosticRef D, - RustStringRef Str) { - RawRustStringOstream OS(Str); - unwrap(D)->print("", OS); +extern "C" bool LLVMRustUnpackSMDiagnostic(LLVMSMDiagnosticRef DRef, + RustStringRef MessageOut, + RustStringRef BufferOut, + unsigned* LocOut, + unsigned* RangesOut, + size_t* NumRanges) { + SMDiagnostic& D = *unwrap(DRef); + RawRustStringOstream MessageOS(MessageOut); + MessageOS << D.getMessage(); + + if (D.getLoc() == SMLoc()) + return false; + + const SourceMgr &LSM = *D.getSourceMgr(); + const MemoryBuffer *LBuf = LSM.getMemoryBuffer(LSM.FindBufferContainingLoc(D.getLoc())); + LLVMRustStringWriteImpl(BufferOut, LBuf->getBufferStart(), LBuf->getBufferSize()); + + *LocOut = D.getLoc().getPointer() - LBuf->getBufferStart(); + + *NumRanges = std::min(*NumRanges, D.getRanges().size()); + size_t LineStart = *LocOut - (size_t)D.getColumnNo(); + for (size_t i = 0; i < *NumRanges; i++) { + RangesOut[i * 2] = LineStart + D.getRanges()[i].first; + RangesOut[i * 2 + 1] = LineStart + D.getRanges()[i].second; + } + + return true; } extern "C" LLVMValueRef LLVMRustBuildCleanupPad(LLVMBuilderRef B, diff --git a/src/test/ui/asm/srcloc.rs b/src/test/ui/asm/srcloc.rs new file mode 100644 index 00000000000..7af6f620a98 --- /dev/null +++ b/src/test/ui/asm/srcloc.rs @@ -0,0 +1,41 @@ +// no-system-llvm +// only-x86_64 +// build-fail + +#![feature(asm)] + +// Checks that inline asm errors are mapped to the correct line in the source code. + +fn main() { + unsafe { + asm!("invalid_instruction"); + //~^ ERROR: invalid instruction mnemonic 'invalid_instruction' + + asm!(" + invalid_instruction + "); + //~^^ ERROR: invalid instruction mnemonic 'invalid_instruction' + + asm!(r#" + invalid_instruction + "#); + //~^^ ERROR: invalid instruction mnemonic 'invalid_instruction' + + asm!(" + mov eax, eax + invalid_instruction + mov eax, eax + "); + //~^^^ ERROR: invalid instruction mnemonic 'invalid_instruction' + + asm!(r#" + mov eax, eax + invalid_instruction + mov eax, eax + "#); + //~^^^ ERROR: invalid instruction mnemonic 'invalid_instruction' + + asm!(concat!("invalid", "_", "instruction")); + //~^ ERROR: invalid instruction mnemonic 'invalid_instruction' + } +} diff --git a/src/test/ui/asm/srcloc.stderr b/src/test/ui/asm/srcloc.stderr new file mode 100644 index 00000000000..57a4fbb9742 --- /dev/null +++ b/src/test/ui/asm/srcloc.stderr @@ -0,0 +1,74 @@ +error: invalid instruction mnemonic 'invalid_instruction' + --> $DIR/srcloc.rs:11:15 + | +LL | asm!("invalid_instruction"); + | ^ + | +note: instantiated into assembly here + --> :2:2 + | +LL | invalid_instruction + | ^^^^^^^^^^^^^^^^^^^ + +error: invalid instruction mnemonic 'invalid_instruction' + --> $DIR/srcloc.rs:15:13 + | +LL | invalid_instruction + | ^ + | +note: instantiated into assembly here + --> :3:13 + | +LL | invalid_instruction + | ^^^^^^^^^^^^^^^^^^^ + +error: invalid instruction mnemonic 'invalid_instruction' + --> $DIR/srcloc.rs:20:13 + | +LL | invalid_instruction + | ^ + | +note: instantiated into assembly here + --> :3:13 + | +LL | invalid_instruction + | ^^^^^^^^^^^^^^^^^^^ + +error: invalid instruction mnemonic 'invalid_instruction' + --> $DIR/srcloc.rs:26:13 + | +LL | invalid_instruction + | ^ + | +note: instantiated into assembly here + --> :4:13 + | +LL | invalid_instruction + | ^^^^^^^^^^^^^^^^^^^ + +error: invalid instruction mnemonic 'invalid_instruction' + --> $DIR/srcloc.rs:33:13 + | +LL | invalid_instruction + | ^ + | +note: instantiated into assembly here + --> :4:13 + | +LL | invalid_instruction + | ^^^^^^^^^^^^^^^^^^^ + +error: invalid instruction mnemonic 'invalid_instruction' + --> $DIR/srcloc.rs:38:14 + | +LL | asm!(concat!("invalid", "_", "instruction")); + | ^ + | +note: instantiated into assembly here + --> :2:2 + | +LL | invalid_instruction + | ^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 6 previous errors + diff --git a/src/test/ui/issues/issue-23458.stderr b/src/test/ui/issues/issue-23458.stderr index 81f06e63975..a6500b9bb4c 100644 --- a/src/test/ui/issues/issue-23458.stderr +++ b/src/test/ui/issues/issue-23458.stderr @@ -2,16 +2,19 @@ error: invalid operand in inline asm: 'int $3' --> $DIR/issue-23458.rs:8:9 | LL | llvm_asm!("int $3"); - | ^^^^^^^^^^^^^^^^^^^^ - -error: :1:2: error: too few operands for instruction - int - ^ + | ^ +error: too few operands for instruction --> $DIR/issue-23458.rs:8:9 | LL | llvm_asm!("int $3"); - | ^^^^^^^^^^^^^^^^^^^^ + | ^ + | +note: instantiated into assembly here + --> :1:2 + | +LL | int + | ^ error: aborting due to 2 previous errors diff --git a/src/test/ui/llvm-asm/issue-69092.rs b/src/test/ui/llvm-asm/issue-69092.rs index ecce7bfdf5b..96c019b760e 100644 --- a/src/test/ui/llvm-asm/issue-69092.rs +++ b/src/test/ui/llvm-asm/issue-69092.rs @@ -6,5 +6,5 @@ fn main() { unsafe { llvm_asm!(".ascii \"Xen\0\""); } - //~^ ERROR: :1:9: error: expected string in '.ascii' directive + //~^ ERROR: expected string in '.ascii' directive } diff --git a/src/test/ui/llvm-asm/issue-69092.stderr b/src/test/ui/llvm-asm/issue-69092.stderr index 35f77edc3c4..2ca86cf7c1b 100644 --- a/src/test/ui/llvm-asm/issue-69092.stderr +++ b/src/test/ui/llvm-asm/issue-69092.stderr @@ -1,11 +1,14 @@ -error: :1:9: error: expected string in '.ascii' directive - .ascii "Xen - ^ - +error: expected string in '.ascii' directive --> $DIR/issue-69092.rs:8:14 | LL | unsafe { llvm_asm!(".ascii \"Xen\0\""); } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^ + | +note: instantiated into assembly here + --> :1:9 + | +LL | .ascii "Xen + | ^ error: aborting due to previous error -- cgit 1.4.1-3-g733a5 From 5541f689e9cfc6c1ccdb2277a308bc2e8541ab5e Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Tue, 9 Jun 2020 14:37:59 +0100 Subject: Handle assembler warnings properly --- src/librustc_codegen_llvm/back/write.rs | 22 ++++++++++--- src/librustc_codegen_llvm/llvm/diagnostic.rs | 12 +++++-- src/librustc_codegen_llvm/llvm/ffi.rs | 13 ++++++++ src/librustc_codegen_ssa/back/write.rs | 20 +++++++----- src/librustc_errors/lib.rs | 5 +++ src/librustc_session/session.rs | 3 ++ src/rustllvm/RustWrapper.cpp | 47 +++++++++++++++++++++++++++- src/test/ui/asm/srcloc.rs | 3 ++ src/test/ui/asm/srcloc.stderr | 14 ++++++++- 9 files changed, 124 insertions(+), 15 deletions(-) (limited to 'src/rustllvm/RustWrapper.cpp') diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index 02a9294930d..26f5334668b 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -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; @@ -242,6 +242,7 @@ impl<'a> Drop for DiagnosticHandlers<'a> { fn report_inline_asm( cgcx: &CodegenContext, msg: String, + level: llvm::DiagnosticLevel, mut cookie: c_uint, source: Option<(String, Vec)>, ) { @@ -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) { @@ -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; @@ -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, @@ -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) { @@ -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) => { diff --git a/src/librustc_codegen_llvm/llvm/diagnostic.rs b/src/librustc_codegen_llvm/llvm/diagnostic.rs index 4347cd06532..47f5c94e70c 100644 --- a/src/librustc_codegen_llvm/llvm/diagnostic.rs +++ b/src/librustc_codegen_llvm/llvm/diagnostic.rs @@ -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>, @@ -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 } } } diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs index 759c2bf1b85..3c981af73ab 100644 --- a/src/librustc_codegen_llvm/llvm/ffi.rs +++ b/src/librustc_codegen_llvm/llvm/ffi.rs @@ -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)] @@ -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>, @@ -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, diff --git a/src/librustc_codegen_ssa/back/write.rs b/src/librustc_codegen_ssa/back/write.rs index cb5c95c11fa..c118e5ebdb7 100644 --- a/src/librustc_codegen_ssa/back/write.rs +++ b/src/librustc_codegen_ssa/back/write.rs @@ -1551,7 +1551,7 @@ fn spawn_work(cgcx: CodegenContext, work: WorkItem enum SharedEmitterMessage { Diagnostic(Diagnostic), - InlineAsmError(u32, String, Option<(String, Vec)>), + InlineAsmError(u32, String, Level, Option<(String, Vec)>), AbortIfErrors, Fatal(String), } @@ -1576,9 +1576,10 @@ impl SharedEmitter { &self, cookie: u32, msg: String, + level: Level, source: Option<(String, Vec)>, ) { - 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) { @@ -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. diff --git a/src/librustc_errors/lib.rs b/src/librustc_errors/lib.rs index e4a560e434a..7261c638ce0 100644 --- a/src/librustc_errors/lib.rs +++ b/src/librustc_errors/lib.rs @@ -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, msg: &str) -> FatalError { self.emit_diag_at_span(Diagnostic::new(Fatal, msg), span); FatalError diff --git a/src/librustc_session/session.rs b/src/librustc_session/session.rs index 048033846a1..06d7d4f14d8 100644 --- a/src/librustc_session/session.rs +++ b/src/librustc_session/session.rs @@ -441,6 +441,9 @@ impl Session { pub fn span_note_without_error>(&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 diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index 6fac2662506..4704622922a 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -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! @@ -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, @@ -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. @@ -1219,6 +1246,7 @@ extern "C" void LLVMRustSetInlineAsmDiagnosticHandler( extern "C" bool LLVMRustUnpackSMDiagnostic(LLVMSMDiagnosticRef DRef, RustStringRef MessageOut, RustStringRef BufferOut, + LLVMRustDiagnosticLevel* LevelOut, unsigned* LocOut, unsigned* RangesOut, size_t* NumRanges) { @@ -1226,6 +1254,23 @@ extern "C" bool LLVMRustUnpackSMDiagnostic(LLVMSMDiagnosticRef 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; diff --git a/src/test/ui/asm/srcloc.rs b/src/test/ui/asm/srcloc.rs index 7af6f620a98..402adc50d5b 100644 --- a/src/test/ui/asm/srcloc.rs +++ b/src/test/ui/asm/srcloc.rs @@ -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 } } diff --git a/src/test/ui/asm/srcloc.stderr b/src/test/ui/asm/srcloc.stderr index 57a4fbb9742..d5d12b00481 100644 --- a/src/test/ui/asm/srcloc.stderr +++ b/src/test/ui/asm/srcloc.stderr @@ -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 + --> :1:23 + | +LL | movaps %xmm3, (%esi, 2) + | ^ + +error: aborting due to 6 previous errors; 1 warning emitted -- cgit 1.4.1-3-g733a5 From 5068ae1ca05b2be0c2a98206a58d894aa620b312 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Wed, 3 Jun 2020 21:19:34 -0700 Subject: [WIP] injects llvm intrinsic instrprof.increment for coverage reports This initial version only injects counters at the top of each function. Rust Coverage will require injecting additional counters at each conditional code branch. --- src/libcore/intrinsics.rs | 9 + src/librustc_codegen_llvm/builder.rs | 27 ++ src/librustc_codegen_llvm/context.rs | 2 + src/librustc_codegen_llvm/intrinsic.rs | 21 ++ src/librustc_codegen_llvm/llvm/ffi.rs | 1 + src/librustc_codegen_ssa/back/write.rs | 6 + src/librustc_codegen_ssa/mir/block.rs | 6 +- src/librustc_codegen_ssa/traits/builder.rs | 8 + src/librustc_codegen_ssa/traits/intrinsic.rs | 1 + src/librustc_hir/lang_items.rs | 2 + src/librustc_interface/tests.rs | 1 + src/librustc_middle/mir/mono.rs | 1 + src/librustc_middle/ty/instance.rs | 7 + src/librustc_middle/ty/mod.rs | 1 + src/librustc_middle/ty/structural_impls.rs | 11 +- src/librustc_mir/interpret/terminator.rs | 3 + src/librustc_mir/monomorphize/collector.rs | 5 +- src/librustc_mir/monomorphize/partitioning.rs | 2 + src/librustc_mir/shim.rs | 3 + src/librustc_mir/transform/instrument_coverage.rs | 100 ++++++ src/librustc_mir/transform/mod.rs | 3 + src/librustc_session/options.rs | 3 + src/librustc_span/symbol.rs | 1 + src/librustc_ty/instance.rs | 4 + src/rustllvm/RustWrapper.cpp | 6 + src/test/codegen/coverage-experiments/Cargo.lock | 5 + src/test/codegen/coverage-experiments/Cargo.toml | 103 ++++++ .../README-THIS-IS-TEMPORARY.md | 157 +++++++++ .../src/coverage_injection_test.rs | 335 +++++++++++++++++++ .../src/coverage_injection_test2.rs | 320 ++++++++++++++++++ .../src/coverage_injection_test_alt.rs | 362 +++++++++++++++++++++ .../codegen/coverage-experiments/src/drop_trait.rs | 25 ++ .../src/drop_trait_with_comments_prints.rs | 53 +++ src/test/codegen/coverage-experiments/src/for.rs | 41 +++ .../coverage-experiments/src/for_with_comments.rs | 24 ++ src/test/codegen/coverage-experiments/src/if.rs | 80 +++++ .../coverage-experiments/src/if_with_comments.rs | 39 +++ .../src/increment_intrinsic.rs | 11 + .../codegen/coverage-experiments/src/just_main.rs | 3 + .../coverage-experiments/src/lazy_boolean.rs | 17 + .../coverage-experiments/src/loop_break_value.rs | 15 + src/test/codegen/coverage-experiments/src/match.rs | 22 ++ .../src/match_with_increment.rs | 305 +++++++++++++++++ .../src/match_with_increment_alt.rs | 296 +++++++++++++++++ .../src/match_without_increment.mir | 0 .../src/match_without_increment.rs | 5 + .../src/match_without_increment_alt.mir | 0 ...stion_mark_err_status_handling_with_comments.rs | 24 ++ src/test/codegen/coverage-experiments/src/while.rs | 23 ++ .../coverage-experiments/src/while_clean.rs | 6 + .../coverage-experiments/src/while_early_return.rs | 10 + .../src/while_with_comments.rs | 51 +++ 52 files changed, 2561 insertions(+), 5 deletions(-) create mode 100644 src/librustc_mir/transform/instrument_coverage.rs create mode 100644 src/test/codegen/coverage-experiments/Cargo.lock create mode 100644 src/test/codegen/coverage-experiments/Cargo.toml create mode 100644 src/test/codegen/coverage-experiments/README-THIS-IS-TEMPORARY.md create mode 100644 src/test/codegen/coverage-experiments/src/coverage_injection_test.rs create mode 100644 src/test/codegen/coverage-experiments/src/coverage_injection_test2.rs create mode 100644 src/test/codegen/coverage-experiments/src/coverage_injection_test_alt.rs create mode 100644 src/test/codegen/coverage-experiments/src/drop_trait.rs create mode 100644 src/test/codegen/coverage-experiments/src/drop_trait_with_comments_prints.rs create mode 100644 src/test/codegen/coverage-experiments/src/for.rs create mode 100644 src/test/codegen/coverage-experiments/src/for_with_comments.rs create mode 100644 src/test/codegen/coverage-experiments/src/if.rs create mode 100644 src/test/codegen/coverage-experiments/src/if_with_comments.rs create mode 100644 src/test/codegen/coverage-experiments/src/increment_intrinsic.rs create mode 100644 src/test/codegen/coverage-experiments/src/just_main.rs create mode 100644 src/test/codegen/coverage-experiments/src/lazy_boolean.rs create mode 100644 src/test/codegen/coverage-experiments/src/loop_break_value.rs create mode 100644 src/test/codegen/coverage-experiments/src/match.rs create mode 100644 src/test/codegen/coverage-experiments/src/match_with_increment.rs create mode 100644 src/test/codegen/coverage-experiments/src/match_with_increment_alt.rs create mode 100644 src/test/codegen/coverage-experiments/src/match_without_increment.mir create mode 100644 src/test/codegen/coverage-experiments/src/match_without_increment.rs create mode 100644 src/test/codegen/coverage-experiments/src/match_without_increment_alt.mir create mode 100644 src/test/codegen/coverage-experiments/src/question_mark_err_status_handling_with_comments.rs create mode 100644 src/test/codegen/coverage-experiments/src/while.rs create mode 100644 src/test/codegen/coverage-experiments/src/while_clean.rs create mode 100644 src/test/codegen/coverage-experiments/src/while_early_return.rs create mode 100644 src/test/codegen/coverage-experiments/src/while_with_comments.rs (limited to 'src/rustllvm/RustWrapper.cpp') diff --git a/src/libcore/intrinsics.rs b/src/libcore/intrinsics.rs index 85076a573b5..abb35e838ea 100644 --- a/src/libcore/intrinsics.rs +++ b/src/libcore/intrinsics.rs @@ -1943,6 +1943,15 @@ extern "rust-intrinsic" { pub fn miri_start_panic(payload: *mut u8) -> !; } +#[cfg(not(bootstrap))] +#[cfg_attr(not(bootstrap), lang = "count_code_region")] +pub fn count_code_region(_index: u32) { + #[cfg_attr(not(bootstrap), allow(unused_unsafe))] // remove `unsafe` on bootstrap bump + unsafe { + abort() + } +} + // Some functions are defined here because they accidentally got made // available in this module on stable. See . // (`transmute` also falls into this category, but it cannot be wrapped due to the diff --git a/src/librustc_codegen_llvm/builder.rs b/src/librustc_codegen_llvm/builder.rs index f5ae9824df8..ba285b5ef38 100644 --- a/src/librustc_codegen_llvm/builder.rs +++ b/src/librustc_codegen_llvm/builder.rs @@ -997,6 +997,33 @@ impl BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { self.call_lifetime_intrinsic("llvm.lifetime.end.p0i8", ptr, size); } + fn instrprof_increment( + &mut self, + fn_name: &'ll Value, + hash: &'ll Value, + num_counters: &'ll Value, + index: &'ll Value, + ) -> &'ll Value { + debug!( + "instrprof_increment() with args ({:?}, {:?}, {:?}, {:?})", + fn_name, hash, num_counters, index + ); + + let llfn = unsafe { llvm::LLVMRustGetInstrprofIncrementIntrinsic(self.cx().llmod) }; + let args = &[fn_name, hash, num_counters, index]; + let args = self.check_call("call", llfn, args); + + unsafe { + llvm::LLVMRustBuildCall( + self.llbuilder, + llfn, + args.as_ptr() as *const &llvm::Value, + args.len() as c_uint, + None, + ) + } + } + fn call( &mut self, llfn: &'ll Value, diff --git a/src/librustc_codegen_llvm/context.rs b/src/librustc_codegen_llvm/context.rs index 4c810a37d41..7ff5ac5cbdc 100644 --- a/src/librustc_codegen_llvm/context.rs +++ b/src/librustc_codegen_llvm/context.rs @@ -749,6 +749,8 @@ impl CodegenCx<'b, 'tcx> { ifn!("llvm.lifetime.start.p0i8", fn(t_i64, i8p) -> void); ifn!("llvm.lifetime.end.p0i8", fn(t_i64, i8p) -> void); + ifn!("llvm.instrprof.increment", fn(i8p, t_i64, t_i32, t_i32) -> void); + ifn!("llvm.expect.i1", fn(i1, i1) -> i1); ifn!("llvm.eh.typeid.for", fn(i8p) -> t_i32); ifn!("llvm.localescape", fn(...) -> void); diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index 1e6d2e3dbb7..7fddda99185 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -7,6 +7,8 @@ use crate::type_of::LayoutLlvmExt; use crate::va_arg::emit_va_arg; use crate::value::Value; +use log::debug; + use rustc_ast::ast; use rustc_codegen_ssa::base::{compare_simd_types, to_immediate, wants_msvc_seh}; use rustc_codegen_ssa::common::span_invalid_monomorphization_error; @@ -21,6 +23,7 @@ use rustc_middle::ty::layout::{FnAbiExt, HasTyCtxt}; use rustc_middle::ty::{self, Ty}; use rustc_middle::{bug, span_bug}; use rustc_span::Span; +use rustc_span::Symbol; use rustc_target::abi::{self, HasDataLayout, LayoutOf, Primitive}; use rustc_target::spec::PanicStrategy; @@ -86,6 +89,7 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { args: &[OperandRef<'tcx, &'ll Value>], llresult: &'ll Value, span: Span, + caller_instance: ty::Instance<'tcx>, ) { let tcx = self.tcx; let callee_ty = instance.monomorphic_ty(tcx); @@ -136,6 +140,23 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { let llfn = self.get_intrinsic(&("llvm.debugtrap")); self.call(llfn, &[], None) } + "count_code_region" => { + if let ty::InstanceDef::Item(fn_def_id) = caller_instance.def { + let caller_fn_path = tcx.def_path_str(fn_def_id); + debug!( + "count_code_region to llvm.instrprof.increment(fn_name={})", + caller_fn_path + ); + + let (fn_name, _len_val) = self.const_str(Symbol::intern(&caller_fn_path)); + let index = args[0].immediate(); + let hash = self.const_u64(1234); + let num_counters = self.const_u32(1); + self.instrprof_increment(fn_name, hash, num_counters, index) + } else { + bug!("intrinsic count_code_region: no src.instance"); + } + } "va_start" => self.va_start(args[0].immediate()), "va_end" => self.va_end(args[0].immediate()), "va_copy" => { diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs index 54cf99e1c6d..372fb17573a 100644 --- a/src/librustc_codegen_llvm/llvm/ffi.rs +++ b/src/librustc_codegen_llvm/llvm/ffi.rs @@ -1360,6 +1360,7 @@ extern "C" { // Miscellaneous instructions pub fn LLVMBuildPhi(B: &Builder<'a>, Ty: &'a Type, Name: *const c_char) -> &'a Value; + pub fn LLVMRustGetInstrprofIncrementIntrinsic(M: &Module) -> &'a Value; pub fn LLVMRustBuildCall( B: &Builder<'a>, Fn: &'a Value, diff --git a/src/librustc_codegen_ssa/back/write.rs b/src/librustc_codegen_ssa/back/write.rs index c118e5ebdb7..49054765b9d 100644 --- a/src/librustc_codegen_ssa/back/write.rs +++ b/src/librustc_codegen_ssa/back/write.rs @@ -175,6 +175,12 @@ impl ModuleConfig { if sess.opts.debugging_opts.profile && !is_compiler_builtins { passes.push("insert-gcov-profiling".to_owned()); } + + // The rustc option `-Zinstrument_coverage` injects intrinsic calls to + // `llvm.instrprof.increment()`, which requires the LLVM `instrprof` pass. + if sess.opts.debugging_opts.instrument_coverage { + passes.push("instrprof".to_owned()); + } passes }, vec![] diff --git a/src/librustc_codegen_ssa/mir/block.rs b/src/librustc_codegen_ssa/mir/block.rs index ef59ad486ee..d7db6571549 100644 --- a/src/librustc_codegen_ssa/mir/block.rs +++ b/src/librustc_codegen_ssa/mir/block.rs @@ -566,7 +566,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { // Handle intrinsics old codegen wants Expr's for, ourselves. let intrinsic = match def { - Some(ty::InstanceDef::Intrinsic(def_id)) => Some(bx.tcx().item_name(def_id).as_str()), + Some(ty::InstanceDef::Intrinsic(def_id)) + | Some(ty::InstanceDef::InjectedCode(def_id)) => { + Some(bx.tcx().item_name(def_id).as_str()) + } _ => None, }; let intrinsic = intrinsic.as_ref().map(|s| &s[..]); @@ -693,6 +696,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { &args, dest, terminator.source_info.span, + self.instance, ); if let ReturnDest::IndirectOperand(dst, _) = ret_dest { diff --git a/src/librustc_codegen_ssa/traits/builder.rs b/src/librustc_codegen_ssa/traits/builder.rs index caba7ebef59..7ffc9f15bff 100644 --- a/src/librustc_codegen_ssa/traits/builder.rs +++ b/src/librustc_codegen_ssa/traits/builder.rs @@ -260,6 +260,14 @@ pub trait BuilderMethods<'a, 'tcx>: /// Called for `StorageDead` fn lifetime_end(&mut self, ptr: Self::Value, size: Size); + fn instrprof_increment( + &mut self, + fn_name: Self::Value, + hash: Self::Value, + num_counters: Self::Value, + index: Self::Value, + ) -> Self::Value; + fn call( &mut self, llfn: Self::Value, diff --git a/src/librustc_codegen_ssa/traits/intrinsic.rs b/src/librustc_codegen_ssa/traits/intrinsic.rs index 9d48e233de6..f6201949851 100644 --- a/src/librustc_codegen_ssa/traits/intrinsic.rs +++ b/src/librustc_codegen_ssa/traits/intrinsic.rs @@ -15,6 +15,7 @@ pub trait IntrinsicCallMethods<'tcx>: BackendTypes { args: &[OperandRef<'tcx, Self::Value>], llresult: Self::Value, span: Span, + caller_instance: ty::Instance<'tcx>, ); fn abort(&mut self); diff --git a/src/librustc_hir/lang_items.rs b/src/librustc_hir/lang_items.rs index 83bada40419..091ded6d74d 100644 --- a/src/librustc_hir/lang_items.rs +++ b/src/librustc_hir/lang_items.rs @@ -242,6 +242,8 @@ language_item_table! { StartFnLangItem, "start", start_fn, Target::Fn; + CountCodeRegionFnLangItem, "count_code_region", count_code_region_fn, Target::Fn; + EhPersonalityLangItem, "eh_personality", eh_personality, Target::Fn; EhCatchTypeinfoLangItem, "eh_catch_typeinfo", eh_catch_typeinfo, Target::Static; diff --git a/src/librustc_interface/tests.rs b/src/librustc_interface/tests.rs index 87647f3b0b0..c2a7d1a4a61 100644 --- a/src/librustc_interface/tests.rs +++ b/src/librustc_interface/tests.rs @@ -548,6 +548,7 @@ fn test_debugging_options_tracking_hash() { tracked!(human_readable_cgu_names, true); tracked!(inline_in_all_cgus, Some(true)); tracked!(insert_sideeffect, true); + tracked!(instrument_coverage, true); tracked!(instrument_mcount, true); tracked!(link_only, true); tracked!(merge_functions, Some(MergeFunctions::Disabled)); diff --git a/src/librustc_middle/mir/mono.rs b/src/librustc_middle/mir/mono.rs index c889dbc0a44..b2c00849d9f 100644 --- a/src/librustc_middle/mir/mono.rs +++ b/src/librustc_middle/mir/mono.rs @@ -352,6 +352,7 @@ impl<'tcx> CodegenUnit<'tcx> { InstanceDef::VtableShim(..) | InstanceDef::ReifyShim(..) | InstanceDef::Intrinsic(..) + | InstanceDef::InjectedCode(..) | InstanceDef::FnPtrShim(..) | InstanceDef::Virtual(..) | InstanceDef::ClosureOnceShim { .. } diff --git a/src/librustc_middle/ty/instance.rs b/src/librustc_middle/ty/instance.rs index 1ce079821a2..4f88e64c503 100644 --- a/src/librustc_middle/ty/instance.rs +++ b/src/librustc_middle/ty/instance.rs @@ -21,6 +21,10 @@ pub enum InstanceDef<'tcx> { Item(DefId), Intrinsic(DefId), + /// Injected call to a placeholder function that is replaced with + /// For example: `core::intrinsic::count_code_region()` for code coverage. + InjectedCode(DefId), + /// `::method` where `method` receives unsizeable `self: Self`. VtableShim(DefId), @@ -149,6 +153,7 @@ impl<'tcx> InstanceDef<'tcx> { | InstanceDef::FnPtrShim(def_id, _) | InstanceDef::Virtual(def_id, _) | InstanceDef::Intrinsic(def_id) + | InstanceDef::InjectedCode(def_id) | InstanceDef::ClosureOnceShim { call_once: def_id } | InstanceDef::DropGlue(def_id, _) | InstanceDef::CloneShim(def_id, _) => def_id, @@ -236,6 +241,7 @@ impl<'tcx> fmt::Display for Instance<'tcx> { InstanceDef::VtableShim(_) => write!(f, " - shim(vtable)"), InstanceDef::ReifyShim(_) => write!(f, " - shim(reify)"), InstanceDef::Intrinsic(_) => write!(f, " - intrinsic"), + InstanceDef::InjectedCode(_) => write!(f, " - injected-code"), InstanceDef::Virtual(_, num) => write!(f, " - virtual#{}", num), InstanceDef::FnPtrShim(_, ty) => write!(f, " - shim({:?})", ty), InstanceDef::ClosureOnceShim { .. } => write!(f, " - shim"), @@ -415,6 +421,7 @@ impl<'tcx> Instance<'tcx> { | InstanceDef::FnPtrShim(..) | InstanceDef::Item(_) | InstanceDef::Intrinsic(..) + | InstanceDef::InjectedCode(..) | InstanceDef::ReifyShim(..) | InstanceDef::Virtual(..) | InstanceDef::VtableShim(..) => Some(self.substs), diff --git a/src/librustc_middle/ty/mod.rs b/src/librustc_middle/ty/mod.rs index 93ef7317199..9b1e717731e 100644 --- a/src/librustc_middle/ty/mod.rs +++ b/src/librustc_middle/ty/mod.rs @@ -2717,6 +2717,7 @@ impl<'tcx> TyCtxt<'tcx> { ty::InstanceDef::VtableShim(..) | ty::InstanceDef::ReifyShim(..) | ty::InstanceDef::Intrinsic(..) + | ty::InstanceDef::InjectedCode(..) | ty::InstanceDef::FnPtrShim(..) | ty::InstanceDef::Virtual(..) | ty::InstanceDef::ClosureOnceShim { .. } diff --git a/src/librustc_middle/ty/structural_impls.rs b/src/librustc_middle/ty/structural_impls.rs index f6f5dfd6516..b6cbd2082a5 100644 --- a/src/librustc_middle/ty/structural_impls.rs +++ b/src/librustc_middle/ty/structural_impls.rs @@ -674,6 +674,7 @@ impl<'a, 'tcx> Lift<'tcx> for ty::InstanceDef<'a> { ty::InstanceDef::VtableShim(def_id) => Some(ty::InstanceDef::VtableShim(def_id)), ty::InstanceDef::ReifyShim(def_id) => Some(ty::InstanceDef::ReifyShim(def_id)), ty::InstanceDef::Intrinsic(def_id) => Some(ty::InstanceDef::Intrinsic(def_id)), + ty::InstanceDef::InjectedCode(def_id) => Some(ty::InstanceDef::Intrinsic(def_id)), ty::InstanceDef::FnPtrShim(def_id, ref ty) => { Some(ty::InstanceDef::FnPtrShim(def_id, tcx.lift(ty)?)) } @@ -846,6 +847,7 @@ impl<'tcx> TypeFoldable<'tcx> for ty::instance::Instance<'tcx> { VtableShim(did) => VtableShim(did.fold_with(folder)), ReifyShim(did) => ReifyShim(did.fold_with(folder)), Intrinsic(did) => Intrinsic(did.fold_with(folder)), + InjectedCode(did) => InjectedCode(did.fold_with(folder)), FnPtrShim(did, ty) => FnPtrShim(did.fold_with(folder), ty.fold_with(folder)), Virtual(did, i) => Virtual(did.fold_with(folder), i), ClosureOnceShim { call_once } => { @@ -861,9 +863,12 @@ impl<'tcx> TypeFoldable<'tcx> for ty::instance::Instance<'tcx> { use crate::ty::InstanceDef::*; self.substs.visit_with(visitor) || match self.def { - Item(did) | VtableShim(did) | ReifyShim(did) | Intrinsic(did) | Virtual(did, _) => { - did.visit_with(visitor) - } + Item(did) + | VtableShim(did) + | ReifyShim(did) + | Intrinsic(did) + | InjectedCode(did) + | Virtual(did, _) => did.visit_with(visitor), FnPtrShim(did, ty) | CloneShim(did, ty) => { did.visit_with(visitor) || ty.visit_with(visitor) } diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index cd7621ea975..82fa471b54d 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -257,6 +257,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { assert!(caller_abi == Abi::RustIntrinsic || caller_abi == Abi::PlatformIntrinsic); M::call_intrinsic(self, instance, args, ret, unwind) } + ty::InstanceDef::InjectedCode(..) => { + M::call_intrinsic(self, instance, args, ret, unwind) + } ty::InstanceDef::VtableShim(..) | ty::InstanceDef::ReifyShim(..) | ty::InstanceDef::ClosureOnceShim { .. } diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index 994d1e69f2e..24c4226bb4e 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -714,7 +714,9 @@ fn visit_instance_use<'tcx>( } match instance.def { - ty::InstanceDef::Virtual(..) | ty::InstanceDef::Intrinsic(_) => { + ty::InstanceDef::Virtual(..) + | ty::InstanceDef::Intrinsic(_) + | ty::InstanceDef::InjectedCode(_) => { if !is_direct_call { bug!("{:?} being reified", instance); } @@ -751,6 +753,7 @@ fn should_monomorphize_locally<'tcx>(tcx: TyCtxt<'tcx>, instance: &Instance<'tcx | ty::InstanceDef::FnPtrShim(..) | ty::InstanceDef::DropGlue(..) | ty::InstanceDef::Intrinsic(_) + | ty::InstanceDef::InjectedCode(_) | ty::InstanceDef::CloneShim(..) => return true, }; diff --git a/src/librustc_mir/monomorphize/partitioning.rs b/src/librustc_mir/monomorphize/partitioning.rs index db1ea72c0a5..7c97b9d611e 100644 --- a/src/librustc_mir/monomorphize/partitioning.rs +++ b/src/librustc_mir/monomorphize/partitioning.rs @@ -322,6 +322,7 @@ fn mono_item_visibility( | InstanceDef::FnPtrShim(..) | InstanceDef::Virtual(..) | InstanceDef::Intrinsic(..) + | InstanceDef::InjectedCode(..) | InstanceDef::ClosureOnceShim { .. } | InstanceDef::DropGlue(..) | InstanceDef::CloneShim(..) => return Visibility::Hidden, @@ -717,6 +718,7 @@ fn characteristic_def_id_of_mono_item<'tcx>( | ty::InstanceDef::FnPtrShim(..) | ty::InstanceDef::ClosureOnceShim { .. } | ty::InstanceDef::Intrinsic(..) + | ty::InstanceDef::InjectedCode(..) | ty::InstanceDef::DropGlue(..) | ty::InstanceDef::Virtual(..) | ty::InstanceDef::CloneShim(..) => return None, diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs index f95fd9b9e90..b4477d9c86d 100644 --- a/src/librustc_mir/shim.rs +++ b/src/librustc_mir/shim.rs @@ -109,6 +109,9 @@ fn make_shim<'tcx>(tcx: TyCtxt<'tcx>, instance: ty::InstanceDef<'tcx>) -> Body<' ty::InstanceDef::Intrinsic(_) => { bug!("creating shims from intrinsics ({:?}) is unsupported", instance) } + ty::InstanceDef::InjectedCode(_) => { + bug!("creating shims from injected code ({:?}) is unsupported", instance) + } }; debug!("make_shim({:?}) = untransformed {:?}", instance, result); diff --git a/src/librustc_mir/transform/instrument_coverage.rs b/src/librustc_mir/transform/instrument_coverage.rs new file mode 100644 index 00000000000..045cd03d1f7 --- /dev/null +++ b/src/librustc_mir/transform/instrument_coverage.rs @@ -0,0 +1,100 @@ +use crate::transform::{MirPass, MirSource}; +use rustc_index::vec::Idx; +use rustc_middle::mir::interpret::Scalar; +use rustc_middle::mir::*; +use rustc_middle::mir::{Local, LocalDecl}; +use rustc_middle::ty; +use rustc_middle::ty::Ty; +use rustc_middle::ty::TyCtxt; +use rustc_span::def_id::DefId; +use rustc_span::Span; + +pub struct InstrumentCoverage; + +/** + * Inserts call to count_code_region() as a placeholder to be replaced during code generation with + * the intrinsic llvm.instrprof.increment. + */ + +// FIXME(richkadel): As a first step, counters are only injected at the top of each function. +// The complete solution will inject counters at each conditional code branch. + +impl<'tcx> MirPass<'tcx> for InstrumentCoverage { + fn run_pass(&self, tcx: TyCtxt<'tcx>, src: MirSource<'tcx>, body: &mut Body<'tcx>) { + if tcx.sess.opts.debugging_opts.instrument_coverage { + if let Some(callee_fn_def_id) = tcx.lang_items().count_code_region_fn() { + debug!("instrumenting {:?}", src.def_id()); + instrument_coverage(tcx, callee_fn_def_id, body); + } + } + } +} + +pub fn instrument_coverage<'tcx>( + tcx: TyCtxt<'tcx>, + callee_fn_def_id: DefId, + body: &mut Body<'tcx>, +) { + let span = body.span.shrink_to_lo(); + + let ret_ty = tcx.fn_sig(callee_fn_def_id).output(); + let ret_ty = ret_ty.no_bound_vars().unwrap(); + let substs = tcx.mk_substs(::std::iter::once(ty::subst::GenericArg::from(ret_ty))); + + let count_code_region_fn: Operand<'_> = + Operand::function_handle(tcx, callee_fn_def_id, substs, span); + + let index = const_int_operand(tcx, span.clone(), tcx.types.u32, 0); + + let args = vec![index]; + + let source_info = SourceInfo { span: span, scope: OUTERMOST_SOURCE_SCOPE }; + + let new_block = START_BLOCK + body.basic_blocks().len(); + + let next_local = body.local_decls.len(); + let new_temp = Local::new(next_local); + let unit_temp = Place::from(new_temp); + + let storage_live = Statement { source_info, kind: StatementKind::StorageLive(new_temp) }; + let storage_dead = Statement { source_info, kind: StatementKind::StorageDead(new_temp) }; + + let count_code_region_call = TerminatorKind::Call { + func: count_code_region_fn, + args, + destination: Some((unit_temp, new_block)), + cleanup: None, + from_hir_call: false, + }; + + body.local_decls.push(LocalDecl::new(tcx.mk_unit(), body.span)); + body.basic_blocks_mut().push(BasicBlockData { + statements: vec![storage_live], + is_cleanup: false, + terminator: Some(Terminator { source_info, kind: count_code_region_call }), + }); + + body.basic_blocks_mut().swap(START_BLOCK, new_block); + body[new_block].statements.push(storage_dead); + + // FIXME(richkadel): ALSO add each computed Span for each conditional branch to the coverage map + // and provide that map to LLVM to encode in the final binary. +} + +fn const_int_operand<'tcx>( + tcx: TyCtxt<'tcx>, + span: Span, + ty: Ty<'tcx>, + val: u128, +) -> Operand<'tcx> { + let param_env_and_ty = ty::ParamEnv::empty().and(ty); + let size = tcx + .layout_of(param_env_and_ty) + .unwrap_or_else(|e| panic!("could not compute layout for {:?}: {:?}", ty, e)) + .size; + Operand::Constant(box Constant { + span, + user_ty: None, + literal: ty::Const::from_scalar(tcx, Scalar::from_uint(val, size), ty), + }) +} diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index 4240b528a61..e03ef48f748 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -28,6 +28,7 @@ pub mod elaborate_drops; pub mod generator; pub mod inline; pub mod instcombine; +pub mod instrument_coverage; pub mod no_landing_pads; pub mod nrvo; pub mod promote_consts; @@ -287,6 +288,8 @@ fn mir_validated( &[&[ // What we need to run borrowck etc. &promote_pass, + // FIXME(richkadel): is this the best place for the InstrumentCoverage pass? + &instrument_coverage::InstrumentCoverage, &simplify::SimplifyCfg::new("qualify-consts"), ]], ); diff --git a/src/librustc_session/options.rs b/src/librustc_session/options.rs index d22c6ec9d7d..599ce595e13 100644 --- a/src/librustc_session/options.rs +++ b/src/librustc_session/options.rs @@ -876,6 +876,9 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, "fix undefined behavior when a thread doesn't eventually make progress \ (such as entering an empty infinite loop) by inserting llvm.sideeffect \ (default: no)"), + instrument_coverage: bool = (false, parse_bool, [TRACKED], + "instrument the generated code with LLVM code region counters for \ + generating coverage reports (default: no)"), instrument_mcount: bool = (false, parse_bool, [TRACKED], "insert function instrument code for mcount-based tracing (default: no)"), keep_hygiene_data: bool = (false, parse_bool, [UNTRACKED], diff --git a/src/librustc_span/symbol.rs b/src/librustc_span/symbol.rs index fdeb58b7b7a..623c2797347 100644 --- a/src/librustc_span/symbol.rs +++ b/src/librustc_span/symbol.rs @@ -240,6 +240,7 @@ symbols! { copy_closures, core, core_intrinsics, + count_code_region, crate_id, crate_in_paths, crate_local, diff --git a/src/librustc_ty/instance.rs b/src/librustc_ty/instance.rs index 0acf7691681..d4ceeff3244 100644 --- a/src/librustc_ty/instance.rs +++ b/src/librustc_ty/instance.rs @@ -35,6 +35,10 @@ fn resolve_instance<'tcx>( debug!(" => intrinsic"); ty::InstanceDef::Intrinsic(def_id) } + ty::FnDef(def_id, _) if Some(def_id) == tcx.lang_items().count_code_region_fn() => { + debug!(" => injected placeholder function to be replaced"); + ty::InstanceDef::InjectedCode(def_id) + } ty::FnDef(def_id, substs) if Some(def_id) == tcx.lang_items().drop_in_place_fn() => { let ty = substs.type_at(0); diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index 4704622922a..cdb3a157eab 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -5,6 +5,7 @@ #include "llvm/IR/DiagnosticPrinter.h" #include "llvm/IR/GlobalVariable.h" #include "llvm/IR/Instructions.h" +#include "llvm/IR/Intrinsics.h" #include "llvm/Object/Archive.h" #include "llvm/Object/ObjectFile.h" #include "llvm/Bitcode/BitcodeWriterPass.h" @@ -1364,6 +1365,11 @@ extern "C" LLVMValueRef LLVMRustBuildCall(LLVMBuilderRef B, LLVMValueRef Fn, unwrap(Fn), makeArrayRef(unwrap(Args), NumArgs), Bundles)); } +extern "C" LLVMValueRef LLVMRustGetInstrprofIncrementIntrinsic(LLVMModuleRef M) { + return wrap(llvm::Intrinsic::getDeclaration(unwrap(M), + (llvm::Intrinsic::ID)llvm::Intrinsic::instrprof_increment)); +} + extern "C" LLVMValueRef LLVMRustBuildMemCpy(LLVMBuilderRef B, LLVMValueRef Dst, unsigned DstAlign, LLVMValueRef Src, unsigned SrcAlign, diff --git a/src/test/codegen/coverage-experiments/Cargo.lock b/src/test/codegen/coverage-experiments/Cargo.lock new file mode 100644 index 00000000000..132469cbb18 --- /dev/null +++ b/src/test/codegen/coverage-experiments/Cargo.lock @@ -0,0 +1,5 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +[[package]] +name = "coverage_experiments" +version = "0.1.0" diff --git a/src/test/codegen/coverage-experiments/Cargo.toml b/src/test/codegen/coverage-experiments/Cargo.toml new file mode 100644 index 00000000000..296a8d5c9af --- /dev/null +++ b/src/test/codegen/coverage-experiments/Cargo.toml @@ -0,0 +1,103 @@ +[workspace] + +[package] +name = "coverage_experiments" +version = "0.1.0" +license = "BSD-3-Clause" +authors = ["rust-fuchsia@fuchsia.com"] +edition = "2018" + +[[bin]] + +name = "coverage_injection_test" +path = "src/coverage_injection_test.rs" + +[[bin]] + +name = "coverage_injection_test2" +path = "src/coverage_injection_test2.rs" + +[[bin]] + +name = "while" +path = "src/while.rs" + +[[bin]] + +name = "while_clean" +path = "src/while_clean.rs" + +[[bin]] + +name = "while_early_return" +path = "src/while_early_return.rs" + +[[bin]] + +name = "if_with_comments" +path = "src/if_with_comments.rs" + +[[bin]] + +name = "if" +path = "src/if.rs" + +[[bin]] + +name = "increment_intrinsic" +path = "src/increment_intrinsic.rs" + +[[bin]] + +name = "just_main" +path = "src/just_main.rs" + +[[bin]] + +name = "lazy_boolean" +path = "src/lazy_boolean.rs" + +[[bin]] + +name = "match" +path = "src/match.rs" + +[[bin]] + +name = "match_without_increment" +path = "src/match_without_increment.rs" # identical to -Zunpretty=hir output + +[[bin]] + +name = "match_with_increment" +path = "src/match_with_increment.rs" + +[[bin]] + +name = "match_with_increment_alt" +path = "src/match_with_increment_alt.rs" + +[[bin]] + +name = "loop_break_value" +path = "src/loop_break_value.rs" + +[[bin]] + +name = "for_with_comments" +path = "src/for_with_comments.rs" + +[[bin]] + +name = "for" +path = "src/for.rs" + +[[bin]] + +name = "drop_trait" +path = "src/drop_trait.rs" + +#[dependencies] # Should not need to manually add coverage dependencies +#version = "0.1.0" +#path = "../__builtin" # for mod __builtin::coverage + diff --git a/src/test/codegen/coverage-experiments/README-THIS-IS-TEMPORARY.md b/src/test/codegen/coverage-experiments/README-THIS-IS-TEMPORARY.md new file mode 100644 index 00000000000..3b69c0a4065 --- /dev/null +++ b/src/test/codegen/coverage-experiments/README-THIS-IS-TEMPORARY.md @@ -0,0 +1,157 @@ +# codegen/coverage-experiments +*

THIS DIRECTORY IS TEMPORARY

* + +This directory contains some work-in-progress (WIP) code used for experimental development and +testing of Rust Coverage feature development. + +The code in this directory will be removed, or migrated into product tests, when the Rust +Coverage feature is complete. + +[TOC] + +## Development Notes + +### config.toml + +config.toml probably requires (I should verify that intrinsic `llvm.instrprof.increment` +code generation ONLY works with this config option): + + profiler = true + +## First build + +```shell +./x.py clean +./x.py build -i --stage 1 src/libstd +``` + +## Incremental builds *IF POSSIBLE!* + +```shell +./x.py build -i --stage 1 src/libstd --keep-stage 1 +``` + +*Note: Some changes made for Rust Coverage required the full build (without `--keep-stage 1`), and in some cases, required `./x.py clean` first!. Occassionally I would get errors when building or when compiling a test program with `--Zinstrument-coverage` that work correctly only after a full clean and build.* + +## Compile a test program with LLVM coverage instrumentation + +*Note: This PR is still a work in progress. At the time of this writing, the `llvm.instrprof.increment` intrinsic is injected, and recognized by the LLVM code generation stage, but it does not appear to be included in the final binary. This is not surprising since other steps are still to be implemented, such as generating the coverage map. See the suggested additional `llvm` flags for ways to verify the `llvm` passes at least get the right intrinsic.* + +Suggested debug configuration to confirm Rust coverage features: +```shell +$ export RUSTC_LOG=rustc_codegen_llvm::intrinsic,rustc_mir::transform::instrument_coverage=debug +``` + +Ensure the new compiled `rustc` is used (the path below, relative to the `rust` code repository root, is an example only): + +```shell +$ build/x86_64-unknown-linux-gnu/stage1/bin/rustc \ + src/test/codegen/coverage-experiments/just_main.rs \ + -Zinstrument-coverage +``` + +### About the test programs in coverage-experiments/src/ + +The `coverage-experiments/src/` directory contains some sample (and very simple) Rust programs used to analyze Rust compiler output at various stages, with or without the Rust code coverage compiler option. For now, these are only used for the in-progress development and will be removed at a future date. (These are *not* formal test programs.) + +The src director may also contain some snapshots of mir output from experimentation, particularly if the saved snapshots highlight results that are important to the future development, individually or when compared with other output files. + +Be aware that some of the files and/or comments may be outdated. + +### Additional `llvm` flags (append to the `rustc` command) + +These optional flags generate additional files and/or terminal output. LLVM's `-print-before=all` should show the `instrprof.increment` intrinsic with arguments computed by the experimental Rust coverage feature code: + +```shell + --emit llvm-ir \ + -Zverify-llvm-ir \ + -Zprint-llvm-passes \ + -Csave-temps \ + -Cllvm-args=-print-before-all +``` + +### Additional flags for MIR analysis and transforms + +These optional flags generate a directory with many files representing the MIR as text (`.mir` files) and as a visual graph (`.dot` files) rendered by `graphviz`. (**Some IDEs, such as `VSCode` have `graphviz` extensions.**) + +```shell + -Zdump-mir=main \ + -Zdump-mir-graphviz +``` + +### Flags I've used but appear to be irrelvant to `-Zinstrument-coverage` after all: +```shell + # -Zprofile + # -Ccodegen-units=1 + # -Cinline-threshold=0 + # -Clink-dead-code + # -Coverflow-checks=off +``` + +## Run the test program compiled with code coverage instrumentation (maybe): + +As stated above, at the time of this writing, this work-in-progress seems to generate `llvm.instrprof.increment` intrinsic calls correctly, and are visibile in early `llvm` code generation passes, but are eventually stripped. + +The test program should run as expected, currently does not generate any coverage output. + +*Example:* + +```shell + $ src/test/codegen/coverage-experiments/just_main + hello world! (should be covered) +``` + +### Running the coverage-enabled `rustc` compiler in the `lldb` debugger: + +For example, to verify the intrinsic is codegen'ed, set breakpoint in `lldb` where it validates a certain instruction is the `llvm.instrprof.increment` instruction. + +First, update config.toml for debugging: + +```toml + [llvm] + optimize = false + release-debuginfo = true + + [rust] + debug = true + debuginfo-level = 2 +``` + +*(Note, in case this is relevant after all, I also have the following changes; but I don't think I need them:)* + +```toml + # Add and uncomment these if relevant/useful: + # codegen-units = 0 + # python = '/usr/bin/python3.6' +``` + +Run the compiler with additional flags as needed: + +```shell +lldb \ + build/x86_64-unknown-linux-gnu/stage1/bin/rustc \ + -- \ + src/test/codegen/coverage-experiments/just_main.rs \ + -Zinstrument-coverage \ + -Zdump-mir=main \ + -Zdump-mir-graphviz +``` + +Note the specific line numbers may be different: + +```c++ +(lldb) b lib/Transforms/Instrumentation/InstrProfiling.cpp:418 +(lldb) r + +Process 93855 stopped +* thread #6, name = 'rustc', stop reason = breakpoint 2.1 + frame #0: 0x00007fffedff7738 librustc_driver-5a0990d8d18fb2b4.so`llvm::InstrProfiling::lowerIntrinsics(this=0x00007fffcc001d40, F=0x00007fffe4552198) at InstrProfiling.cpp:418:23 + 415 auto Instr = I++; + 416 InstrProfIncrementInst *Inc = castToIncrementInst(&*Instr); + 417 if (Inc) { +-> 418 lowerIncrement(Inc); + 419 MadeChange = true; + 420 } else if (auto *Ind = dyn_cast(Instr)) { + 421 lowerValueProfileInst(Ind); +(lldb) +``` \ No newline at end of file diff --git a/src/test/codegen/coverage-experiments/src/coverage_injection_test.rs b/src/test/codegen/coverage-experiments/src/coverage_injection_test.rs new file mode 100644 index 00000000000..231da1dc1a6 --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/coverage_injection_test.rs @@ -0,0 +1,335 @@ +/* */ use std::io::Error; +/* */ use std::io::ErrorKind; +/* */ +/* */ /// Align Rust counter increment with with: +/* */ /// [‘llvm.instrprof.increment’ Intrinsic](https://llvm.org/docs/LangRef.html#llvm-instrprof-increment-intrinsic) +/* */ /// +/* */ /// declare void @llvm.instrprof.increment(i8* , i64 , i32 , i32 ) +/* */ /// +/* */ /// The first argument is a pointer to a global variable containing the name of the entity +/* */ /// being instrumented. This should generally be the (mangled) function name for a set of +/* */ /// counters. +/* */ /// +/* */ /// The second argument is a hash value that can be used by the consumer of the profile data +/* */ /// to detect changes to the instrumented source, and the third is the number of counters +/* */ /// associated with name. It is an error if hash or num-counters differ between two +/* */ /// instances of instrprof.increment that refer to the same name. +/* */ /// +/* */ /// The last argument refers to which of the counters for name should be incremented. It +/* */ /// should be a value between 0 and num-counters. +/* */ /// +/* */ /// # Arguments +/* */ /// +/* */ /// `mangled_fn_name` - &'static ref to computed and injected static str, using: +/* */ /// +/* */ /// ``` +/* */ /// fn rustc_symbol_mangling::compute_symbol_name( +/* */ /// tcx: TyCtxt<'tcx>, +/* */ /// instance: Instance<'tcx>, +/* */ /// compute_instantiating_crate: impl FnOnce() -> CrateNum, +/* */ /// ) -> String +/* */ /// ``` +/* */ /// +/* */ /// `source_version_hash` - Compute hash based that only changes if there are "significant" +/* */ /// to control-flow inside the function. +/* */ /// +/* */ /// `num_counters` - The total number of counter calls [MAX(counter_index) + 1] within the +/* */ /// function. +/* */ /// +/* */ /// `counter_index` - zero-based counter index scoped by the function. (Ordering of +/* */ /// counters, relative to the source code location, is apparently not expected.) +/* */ /// +/* */ /// # Notes +/* */ /// +/* */ /// * The mangled_fn_name may not be computable until generics are monomorphized (see +/* */ /// parameters required by rustc_symbol_mangling::compute_symbol_name). +/* */ /// * The version hash may be computable from AST analysis, and may not benefit from further +/* */ /// lowering. +/* */ /// * num_counters depends on having already identified all counter insertion locations. +/* */ /// * counter_index can be computed at time of counter insertion (incrementally). +/* */ /// * Numeric parameters are signed to match the llvm increment intrinsic parameter types. +/* */ fn __lower_incr_cov(_mangled_fn_name: &'static str, _fn_version_hash: i64, _num_counters: i32, _counter_index: i32) { +/* */ } +/* */ +/* */ /// A coverage counter implementation that will work as both an intermediate coverage +/* */ /// counting and reporting implementation at the AST-level only--for debugging and +/* */ /// development--but also serves as a "marker" to be replaced by calls to LLVM +/* */ /// intrinsic coverage counter APIs during the lowering process. +/* */ /// +/* */ /// Calls to this function will be injected automatically into the AST. When LLVM intrinsics +/* */ /// are enabled, the counter function calls that were injected into the AST serve as +/* */ /// placeholders, to be replaced by an alternative, such as: +/* */ /// +/* */ /// * direct invocation of the `llvm.instrprof.increment()` intrinsic; or +/* */ /// * the `__lower_incr_cov()` function, defined above, that would invoke the +/* */ /// `llvm.instrprof.increment()` intrinsic; or +/* */ /// * a similar expression wrapper, with the additional parameters (as defined above +/* */ /// for `__lower_incr_cov()`, that invokes `llvm.instrprof.increment()` and returns the +/* */ /// result of the wrapped expression) +/* */ /// +/* */ /// The first two options would require replacing the inlined wrapper call with something +/* */ /// like: +/* */ /// +/* */ /// ``` +/* */ /// { let result = {expr}; __inlined_incr_cov(context, counter); result } +/* */ /// ``` +/* */ /// +/* */ /// But if the lowering process is already unwrapping the inlined call to `__incr_cov()`, then +/* */ /// it may be a perfect opportunity to replace the function with one of these more +/* */ /// direct methods. +/* */ /// +/* */ #[inline(always)] +/* */ pub fn __incr_cov(region_loc: &str, /*index: u32,*/ result: T) -> T { +/* */ // Either call the intermediate non-llvm coverage counter API or +/* */ // replace the call to this function with the expanded `__lower_incr_cov()` call. +/* */ +/* */ // let _lock = increment_counter(counter); +/* */ println!("{}", region_loc); +/* */ +/* */ result +/* */ } +/* */ +/* */ /// Write a report identifying each incremented counter and the number of times each counter +/* */ /// was incremented. +/* */ fn __report() { +/* */ println!("WRITE REPORT!"); +/* */ } +/* */ +/* */ /// Increment the counter after evaluating the wrapped expression (see `__incr_cov()`), then +/* */ /// write a report identifying each incremented counter and the number of times each counter +/* */ /// was incremented. +/* */ #[inline(always)] +/* */ pub fn __incr_cov_and_report(region_loc: &str, /*counter: u32,*/ result: T) -> T { +/* */ __incr_cov(region_loc, /*counter,*/ ()); +/* */ __report(); +/* */ result +/* */ } +/* */ +/* */ macro_rules! from { +/* */ ($from:expr) => { &format!("from: {}\n to: {}:{}:{}", $from, file!(), line!(), column!()) }; +/* */ } +/* */ +/* */ #[derive(Debug)] +/* */ enum TestEnum { +/* */ Red, +/* */ Green, +/* */ Blue, +/* */ } +/* */ +/* */ struct TestStruct { +/* */ field: i32, +/* */ } +/* */ +/* */ // IMPORTANT! IS WRAPPING main() ENOUGH? OR DO I ALSO NEED TO WRAP THREAD FUNCTIONS, ASSUMING +/* */ // THEY ARE STILL RUNNING WITH MAIN EXITS? (IF THEY CAN). NOT SURE HOW RUST HANDLES THAT. +/* */ +/* */ // I SUSPECT USING THREAD_LOCAL COUNTERS MAY NOT ACTUALLY BE AN OPTIMIZATION OVER MUTEX LOCKS, +/* */ // BUT MAYBE I SHOULD ASK. +/* */ +/* */ impl TestStruct { +/* - */ fn new() -> Self { +/* ┃ */ __incr_cov(from!("fn new()"),Self::new_with_value(31415)) // function-scoped counter index = 0 +/* - */ } +/* */ +/* - */ fn new_with_value(field: i32) -> Self { +/* ┃ */ __incr_cov(from!("fn new_with_value()"),Self { +/* ┃ */ field, +/* ┃ */ }) // function-scoped counter index = 0 +/* - */ } +/* */ +/* */ fn call_closure(&self, closure: F) -> bool +/* */ where +/* */ F: FnOnce( +/* */ i32, +/* */ ) -> bool, +/* - */ { +/* ┃ */ __incr_cov(from!("fn call_closure()"),closure(123)) // function-scoped counter index = 0 +/* - */ } +/* */ +/* - */ fn various(&self) -> Result<(),Error> { +/* ┃ */ use TestEnum::*; +/* ┃ */ let mut color = Red; +/* ┃ */ let _ = color; +/* ┃ */ color = Blue; +/* ┃ */ let _ = color; +/* ┃ */ color = Green; +/* ┃ */ match __incr_cov(from!("fn various"),color) { // function-scoped counter index = 0 +/* : */ +/* : */ // !!! RECORD SPAN FROM START OF INNERMOST CONTAINING BLOCK (THE FUNCTION IN THIS CASE) TO END OF MATCH EXPRESSION +/* : */ // If `match`, `while`, `loop`, `for`, `if`, etc. expression has a `return`, `break`, or `continue` +/* : */ // (if legal), then RECORD SPAN FROM START OF INNERMOST CONTAINING BLOCK TO END OF `return` EXPRESSION +/* : */ // If the expression includes lazy booleans, nest calls to `__incr_cov()`. +/* : I */ Red => __incr_cov(from!("Red => or end of MatchArmGuard expression inside pattern, if any"),println!("roses")), +/* : - */ Green => { +/* : ┃ */ let spidey = 100; +/* : ┃ */ let goblin = 50; +/* : ┃ */ // if spidey > goblin {__incr_cov(from!(""),{ +/* : ┃ */ // println!("what ev"); +/* : ┃ */ // })} +/* : ┃ */ // ACTUALLY, WRAPPING THE ENTIRE IF BLOCK IN `__incr_cov` IS NOT A GREAT GENERAL RULE. +/* : ┃ */ // JUST INSERTING A `return`, `break`, or `continue` IN THAT BLOCK (without an intermediate condition) +/* : ┃ */ // MAKES THE `__incr_cov()` CALL UNREACHABLE! +/* : ┃ */ // MY ORIGINAL SOLUTION WORKS BETTER (WRAP LAST EXPRESSION OR AFTER LAST SEMICOLON STATEMENT IN BLOCK) +/* : ┃ */ // UNLESS THE EXPRESSION IS NOT A BLOCK. +/* : ┃ - */ if __incr_cov(from!("Green => or end of MatchArmGuard expression inside pattern, if any"),spidey > goblin) { +/* : : ┃ */ println!("spidey beats goblin"); +/* : : ┃ */ __incr_cov(from!("block start"),()); +/* : ┃ - */ } else if __incr_cov(from!("`else if` on this line"),spidey == goblin) { +/* : : ┃ */ // COVERAGE NOTE: Do we mark only the expression span (that may be trivial, as in this case), +/* : : ┃ */ // or associate it with the outer block, similar to how the `if` expression is associated with +/* : : ┃ */ // the outer block? (Although it is a continuation, in a sense, it is discontiguous in this case, +/* : : ┃ */ // so I think simpler to just make it its own coverage region.) +/* : : ┃ */ println!("it's a draw"); +/* : : ┃ */ __incr_cov(from!("block start"),()); +/* : ┃ - - - */ } else if if __incr_cov(from!("`else if` on this line"),true) { +/* : : : ┃ */ // return __incr_cov(from!("after `if true`"),Ok(())); +/* : : : ┃ */ // ACTUALLY, BECAUSE OF `return`, WE DO NOT RECORD THE `if true` EVEN THOUGH WE COVERED IT. +/* : : : ┃ */ // IN FACT, IF THIS NESTED CONDITIONAL IN A CONDITIONAL EXPRESSION WAS AN `if` (WITHOUT PRECEDING ELSE) +/* : : : ┃ */ // WE WOULD NOT HAVE RECORDED THE COVERAGE OF STATEMENTS LEADING UP TO THE `if`, SO +/* : : : ┃ */ // IT SHOULD BE: +/* ┏-:---:-------:---< */ return __incr_cov(from!(""),Ok(())); +/* V : : : : */ // NOTE THE `from` STRING IS SAME FOR THE `else if`s `__incr_cov` AND THIS `return`. +/* : : : : */ // ONLY ONE OF THESE WILL EXECUTE, TO RECORD COVERAGE FROM THAT SPOT. +/* : : ┃ - */ } else { +/* : : : I */ __incr_cov(from!("`else`"),false) +/* : : - - */ } { +/* : : ┃ */ println!("wierd science"); +/* : : ┃ */ __incr_cov(from!("block start"),()); +/* : ┃ - */ } else { +/* : : ┃ */ println!("goblin wins"); +/* ┏-:---:---< */ return __incr_cov(from!("`else`"),Ok(())); // THIS COUNTS LAST STATEMENT IN `else` BLOCK +/* V : : : */ // COVERAGE NOTE: When counting the span for `return`, +/* : : : */ // `break`, or `continue`, also report the outer spans +/* : : : */ // got this far--including this `else` block. Record +/* : : : */ // The start positions for those outer blocks, but: +/* : : : */ // * For the block containing the `return`, `break`, or +/* : : : */ // `continue`, end report the end position is the +/* : : : */ // start of the `return` span (or 1 char before it). +/* : : : */ // * Anything else? +/* : ┃ - */ } +/* : ┃ - */ // __incr_cov(from!(""),()); // DO NOT COUNT HERE IF NO STATEMENTS AFTER LAST `if` or `match` +/* : - */ }, +/* : I */ Blue => __incr_cov(from!("Blue => or end of MatchArmGuard expression inside pattern, if any"),println!("violets")), +/* ┃ */ } +/* ┃ */ +/* ┃ */ let condition1 = true; +/* ┃ */ let condition2 = false; +/* ┃ */ let condition3 = true; +/* ┃ */ +/* ┃ */ println!("Called `various()` for TestStruct with field={}", self.field); +/* ┃ */ +/* ┃ - */ if __incr_cov(from!("after block end of prior `match` (or `if-else if-else`)"),condition1) { +/* : ┃ */ println!("before while loop"); +/* : ┃ */ let mut countdown = 10; +/* : ┃ */ __incr_cov(from!("block start"),()); // Must increment before repeated while text expression +/* : : I */ while __incr_cov(from!("while test"), countdown > 0) { // span is just the while test expression +/* : : ┃ */ println!("top of `while` loop"); +/* : : ┃ */ countdown -= 1; +/* : : ┃ */ // __incr_cov(from!("while loop"),()); // Counter not needed, but span is computed as "while test" minus "block start" +/* : : ┃ */ // If test expression is 11, and the outer block runs only once, 11-1 = 10 +/* : ┃ - */ } +/* : ┃ */ println!("before for loop"); +/* : ┃ - */ for index in __incr_cov(from!("end of while"),0..10) { +/* : : ┃ */ println!("top of `for` loop"); +/* : : ┃ - */ if __incr_cov(from!("block start"),index == 8) { +/* : : : ┃ */ println!("before break"); +/* : : : ┃ */ // note the following is not legal here: +/* : : : ┃ */ // "can only break with a value inside `loop` or breakable block" +/* : : : ┃ */ // break __incr_cov(from!(""),()); +/* : : : ┃ */ __incr_cov(from!("block start"),()); +/* : : ┏-----< */ break; +/* : : V : : */ +/* : : : : */ // FIXME(richkadel): add examples with loop labels, breaking out of inner and outer loop to outer loop label, with expression. +/* : : : : */ // May want to record both the span and the start position after the broken out block depdnding on label +/* : : ┃ - */ } +/* : : ┃ */ println!("after `break` test"); +/* : : ┃ - */ if __incr_cov(from!("block end of `if index == 8`"),condition2) { +/* ┏-:---:---:---< */ return __incr_cov(from!("block start"),Ok(())); +/* V : : ┃ - */ } +/* : : ┃ */ +/* : : ┃ */ // BECAUSE THE PREVIOUS COVERAGE REGION HAS A `return`, THEN +/* : : ┃ */ // IF PREVIOUS COVERAGE REGION IS NOT COUNTED THEN OUTER REGION REACHED HERE. +/* : : ┃ */ // ADD A COVERAGE REGION FOR THE SPAN FROM JUST AFTER PREVIOUS REGION TO END +/* : : ┃ */ // OF OUTER SPAN, THEN TRUNCATE TO NEXT REGION NOT REACHED. +/* : : ┃ - */ if index % 3 == 2 { // NO __incr_cov() HERE BECAUSE NO STATEMENTS BETWEEN LAST CONDITIONAL BLOCK AND START OF THIS ONE +/* : : Λ : ┃ */ __incr_cov(from!("block end of `if condition2`"),()); +/* : : ┗-----< */ continue; +/* : : ┃ - */ } +/* : : ┃ */ println!("after `continue` test"); +/* : : ┃ */ // maybe add a runtime flag for a possible `return` here? +/* : : ┃ */ __incr_cov(from!("for loop"),()); +/* : ┃ - */ } +/* : ┃ */ println!("after for loop"); +/* : ┃ */ let result = if { // START OF NEW CONDITIONAL EXPRESSION. NEXT "GUARANTEED" COUNTER SHOULD COUNT FROM END OF LAST CONDITIONAL EXPRESSION +/* : ┃ */ // A "GUARANTEED" COUNTER CALL IS ONE THAT WILL BE CALLED REGARDLESS OF OTHER CONDITIONS. THIS INCLUDES: +/* : ┃ */ // * A CONDITIONAL EXPRESSION THAT IS NOT A BLOCK (OR ANOTHER CONDITIONAL STATEMENT, WHICH WOULD CONTAIN A BLOCK) +/* : ┃ */ // * OR IF THE NEXT CONDITIONAL EXPRESSION IS A BLOCK OR CONDITIONAL STATEMENT, THEN THE FIRST "GUARANTEED" COUNTER IN THAT BLOCK +/* : ┃ */ // * END OF BLOCK IF THE BLOCK DOES NOT HAVE INNER CONDITIONAL EXPRESSIONS +/* : ┃ */ // * BRANCHING STATEMENTS (`return`, `break`, `continue`) BY EITHER WRAPPING THE BRANCH STATEMENT NON-BLOCK EXPRESSION, +/* : ┃ */ // OR PREPENDING A COUNTER WITH EMPTY TUPLE IF NO EXPRESSION, OR IF EXPRESSION IS A BLOCK, THEN THE NEXT "GUARANTEED" +/* : ┃ */ // COUNTER CALL WITHIN THAT BLOCK. +/* : ┃ */ // BASICALLY, CARRY THE START OF COVERAGE SPAN FORWARD UNTIL THE GUARANTEED COUNTER IS FOUND +/* : ┃ */ println!("after result = if ..."); +/* : ┃ - */ if __incr_cov(from!("block end of `for` loop"),condition2) { +/* : : ┃ */ println!("before first return"); +/* ┏-:---:-------< */ return __incr_cov(from!("block start"),Ok(())); +/* V : : - */ } else if __incr_cov(from!("`else`"),condition3) { +/* : : ┃ */ // THE ABOVE COUNTER IS _NOT_ REALLY NECESSARY IF EXPRESSION IS GUARANTEED TO EXECUTE. +/* : : ┃ */ // IF WE GET COUNTER IN `else if` BLOCK WE COVERED EXPRESSION. +/* : : ┃ */ // IF WE GET TO ANY REMAINING `else` or `else if` BLOCK WE KNOW WE EVALUATED THIS CONDITION +/* : : ┃ */ // AND ALL OTHERS UP TO THE EXECUTED BLOCK. BUT THE SPAN WOULD HAVE "HOLES" FOR UNEXECUTED BLOCKS. +/* : : ┃ */ println!("not second return"); +/* ┏-:---:-------< */ return __incr_cov(from!("block start"),Ok(())); +/* V : : - */ } else { +/* : : ┃ */ println!("not returning"); +/* : : ┃ */ __incr_cov(from!("block start"),false) +/* : : - */ } +/* : ┃ */ // NO COUNTER HERE BECAUSE NO STATEMENTS AFTER CONDITIONAL BLOCK +/* : ┃ - */ } { +/* : : ┃ */ println!("branched condition returned true"); +/* : : ┃ */ __incr_cov(from!(""),Ok(())) +/* : ┃ - */ } else if self.call_closure( +/* : : - */ |closure_param| __incr_cov(from!(""), +/* : : ┃ - */ if condition3 { +/* : : : ┃ */ println!("in closure, captured condition said to print the param {}", closure_param); +/* : : : ┃ */ __incr_cov(from!(""),false) +/* : : ┃ - */ } else { +/* : : : ┃ */ println!("in closure, captured condition was false"); +/* : : : ┃ */ __incr_cov(from!(""),true) +/* : : ┃ - */ } +/* : : - */ ) +/* : : - */ ) { +/* : : ┃ */ println!("closure returned true"); +/* : : ┃ */ __incr_cov(from!(""),Err(Error::new(ErrorKind::Other, "Result is error if closure returned true"))) +/* : ┃ - */ } else { +/* : : ┃ */ println!("closure returned false"); +/* : : ┃ */ __incr_cov(from!(""),Err(Error::new(ErrorKind::Other, "Result is error if closure returned false"))) +/* : ┃ - */ }; +/* : ┃ */ println!("bottom of function might be skipped if early `return`"); +/* : ┃ */ __incr_cov(from!("if condition1"),result) +/* ┃ - */ } else { +/* : ┃ */ println!("skipping everything in `various()`"); +/* : ┃ */ __incr_cov(from!(""),Ok(())) +/* ┃ - */ } +/* ┃ - */ // __incr_cov(from!(""),0) // DO NOT COUNT IF NO STATEMENTS AFTER CONDITIONAL BLOCK. ALL COVERAGE IS ALREADY COUNTED +/* - */ } +/* */ } +/* */ +/* - */ fn main() -> Result<(), std::io::Error> { +/* ┃ */ //let mut status: u8 = 2; +/* ┃ */ let mut status: u8 = 1; +/* : - */ let result = if status < 2 && +/* : ┃ */ __incr_cov(from!(""),{ +/* : ┃ */ status -= 1; +/* : ┃ */ status == 0 +/* : - - */ }) { +/* : ┃ */ let test_struct = TestStruct::new_with_value(100); +/* : ┃ */ let _ = test_struct.various(); +/* ┏-:---< */ return __incr_cov_and_report(from!(""),Err(Error::new(ErrorKind::Other, format!("Error status {}", status)))) +/* V : - */ } else { +/* : ┃ */ let test_struct = TestStruct::new(); +/* : ┃ */ __incr_cov(from!(""),test_struct.various()) +/* : - */ }; +/* ┃ */ println!("done"); +/* ┃ */ __incr_cov_and_report(from!(""),result) // function-scoped counter index = 0 +/* - */ } \ No newline at end of file diff --git a/src/test/codegen/coverage-experiments/src/coverage_injection_test2.rs b/src/test/codegen/coverage-experiments/src/coverage_injection_test2.rs new file mode 100644 index 00000000000..8f4399ab51d --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/coverage_injection_test2.rs @@ -0,0 +1,320 @@ +/* */ use std::io::Error; +/* */ use std::io::ErrorKind; +/* */ +/* */ /// Align Rust counter increment with with: +/* */ /// [‘llvm.instrprof.increment’ Intrinsic](https://llvm.org/docs/LangRef.html#llvm-instrprof-increment-intrinsic) +/* */ /// +/* */ /// declare void @llvm.instrprof.increment(i8* , i64 , i32 , i32 ) +/* */ /// +/* */ /// The first argument is a pointer to a global variable containing the name of the entity +/* */ /// being instrumented. This should generally be the (mangled) function name for a set of +/* */ /// counters. +/* */ /// +/* */ /// The second argument is a hash value that can be used by the consumer of the profile data +/* */ /// to detect changes to the instrumented source, and the third is the number of counters +/* */ /// associated with name. It is an error if hash or num-counters differ between two +/* */ /// instances of instrprof.increment that refer to the same name. +/* */ /// +/* */ /// The last argument refers to which of the counters for name should be incremented. It +/* */ /// should be a value between 0 and num-counters. +/* */ /// +/* */ /// # Arguments +/* */ /// +/* */ /// `mangled_fn_name` - &'static ref to computed and injected static str, using: +/* */ /// +/* */ /// ``` +/* */ /// fn rustc_symbol_mangling::compute_symbol_name( +/* */ /// tcx: TyCtxt<'tcx>, +/* */ /// instance: Instance<'tcx>, +/* */ /// compute_instantiating_crate: impl FnOnce() -> CrateNum, +/* */ /// ) -> String +/* */ /// ``` +/* */ /// +/* */ /// `source_version_hash` - Compute hash based that only changes if there are "significant" +/* */ /// to control-flow inside the function. +/* */ /// +/* */ /// `num_counters` - The total number of counter calls [MAX(counter_index) + 1] within the +/* */ /// function. +/* */ /// +/* */ /// `counter_index` - zero-based counter index scoped by the function. (Ordering of +/* */ /// counters, relative to the source code location, is apparently not expected.) +/* */ /// +/* */ /// # Notes +/* */ /// +/* */ /// * The mangled_fn_name may not be computable until generics are monomorphized (see +/* */ /// parameters required by rustc_symbol_mangling::compute_symbol_name). +/* */ /// * The version hash may be computable from AST analysis, and may not benefit from further +/* */ /// lowering. +/* */ /// * num_counters depends on having already identified all counter insertion locations. +/* */ /// * counter_index can be computed at time of counter insertion (incrementally). +/* */ /// * Numeric parameters are signed to match the llvm increment intrinsic parameter types. +/* */ fn __lower_incr_cov(_mangled_fn_name: &'static str, _fn_version_hash: i64, _num_counters: i32, _counter_index: i32) { +/* */ } +/* */ +/* */ /// A coverage counter implementation that will work as both an intermediate coverage +/* */ /// counting and reporting implementation at the AST-level only--for debugging and +/* */ /// development--but also serves as a "marker" to be replaced by calls to LLVM +/* */ /// intrinsic coverage counter APIs during the lowering process. +/* */ /// +/* */ /// Calls to this function will be injected automatically into the AST. When LLVM intrinsics +/* */ /// are enabled, the counter function calls that were injected into the AST serve as +/* */ /// placeholders, to be replaced by an alternative, such as: +/* */ /// +/* */ /// * direct invocation of the `llvm.instrprof.increment()` intrinsic; or +/* */ /// * the `__lower_incr_cov()` function, defined above, that would invoke the +/* */ /// `llvm.instrprof.increment()` intrinsic; or +/* */ /// * a similar expression wrapper, with the additional parameters (as defined above +/* */ /// for `__lower_incr_cov()`, that invokes `llvm.instrprof.increment()` and returns the +/* */ /// result of the wrapped expression) +/* */ /// +/* */ /// The first two options would require replacing the inlined wrapper call with something +/* */ /// like: +/* */ /// +/* */ /// ``` +/* */ /// { let result = {expr}; __inlined_incr_cov(context, counter); result } +/* */ /// ``` +/* */ /// +/* */ /// But if the lowering process is already unwrapping the inlined call to `__incr_cov()`, then +/* */ /// it may be a perfect opportunity to replace the function with one of these more +/* */ /// direct methods. +/* */ /// +/* */ #[inline(always)] +/* */ pub fn __incr_cov(region_loc: &str) { +/* */ // Either call the intermediate non-llvm coverage counter API or +/* */ // replace the call to this function with the expanded `__lower_incr_cov()` call. +/* */ +/* */ // let _lock = increment_counter(counter); +/* */ println!("{}", region_loc); +/* */ } +/* */ +/* */ /// Write a report identifying each incremented counter and the number of times each counter +/* */ /// was incremented. +/* */ fn __report() { +/* */ println!("WRITE REPORT!"); +/* */ } +/* */ +/* */ macro_rules! from { +/* */ ($from:expr) => { &format!("from: {}\n to: {}:{}:{}", $from, file!(), line!(), column!()) }; +/* */ } +/* */ +/* */ #[derive(Debug)] +/* */ enum TestEnum { +/* */ Red, +/* */ Green, +/* */ Blue, +/* */ } +/* */ +/* */ struct TestStruct { +/* */ field: i32, +/* */ } +/* */ +/* */ // IMPORTANT! IS WRAPPING main() ENOUGH? OR DO I ALSO NEED TO WRAP THREAD FUNCTIONS, ASSUMING +/* */ // THEY ARE STILL RUNNING WITH MAIN EXITS? (IF THEY CAN). NOT SURE HOW RUST HANDLES THAT. +/* */ +/* */ // I SUSPECT USING THREAD_LOCAL COUNTERS MAY NOT ACTUALLY BE AN OPTIMIZATION OVER MUTEX LOCKS, +/* */ // BUT MAYBE I SHOULD ASK. +/* */ +/* */ impl TestStruct { +/* - */ fn new() -> Self { +/* ┃ */ let __result = Self::new_with_value(31415); // function-scoped counter index = 0 +/* ┃ */ __incr_cov(from!("fn new()")); +/* ┃ */ __result +/* - */ } +/* */ +/* - */ fn new_with_value(field: i32) -> Self { +/* ┃ */ let __result = Self { +/* ┃ */ field, +/* ┃ */ }; +/* ┃ */ __incr_cov(from!("fn new_with_value()")); // function-scoped counter index = 0 +/* ┃ */ __result +/* - */ } +/* */ +/* */ fn call_closure(&self, closure: F) -> bool +/* */ where +/* */ F: FnOnce( +/* */ i32, +/* */ ) -> bool, +/* - */ { +/* ┃ */ let __result = closure(123); +/* ┃ */ __incr_cov(from!("fn call_closure()")); // function-scoped counter index = 0 +/* ┃ */ __result +/* - */ } +/* */ +/* - */ fn various(&self) -> Result<(),Error> { +/* ┃ */ use TestEnum::*; +/* ┃ */ let mut color = Red; +/* ┃ */ let _ = color; +/* ┃ */ color = Blue; +/* ┃ */ let _ = color; +/* ┃ */ color = Green; +/* ┃ */ match { let __result = color; __incr_cov(from!("fn various")); __result } { // function-scoped counter index = 0 +/* : */ +/* : */ // !!! RECORD SPAN FROM START OF INNERMOST CONTAINING BLOCK (THE FUNCTION IN THIS CASE) TO END OF MATCH EXPRESSION +/* : */ // If `match`, `while`, `loop`, `for`, `if`, etc. expression has a `return`, `break`, or `continue` +/* : */ // (if legal), then RECORD SPAN FROM START OF INNERMOST CONTAINING BLOCK TO END OF `return` EXPRESSION +/* : */ // If the expression includes lazy booleans, nest calls to `__incr_cov()`. +/* : I */ Red => {println!("roses"); __incr_cov(from!("Red => or end of MatchArmGuard expression inside pattern, if any"));} +/* : - */ Green => { +/* : ┃ */ let spidey = 100; +/* : ┃ */ let goblin = 50; +/* : ┃ */ // if spidey > goblin {__incr_cov(from!(""),{ +/* : ┃ */ // println!("what ev"); +/* : ┃ */ // })} +/* : ┃ */ // ACTUALLY, WRAPPING THE ENTIRE IF BLOCK IN `__incr_cov` IS NOT A GREAT GENERAL RULE. +/* : ┃ */ // JUST INSERTING A `return`, `break`, or `continue` IN THAT BLOCK (without an intermediate condition) +/* : ┃ */ // MAKES THE `__incr_cov()` CALL UNREACHABLE! +/* : ┃ */ // MY ORIGINAL SOLUTION WORKS BETTER (WRAP LAST EXPRESSION OR AFTER LAST SEMICOLON STATEMENT IN BLOCK) +/* : ┃ */ // UNLESS THE EXPRESSION IS NOT A BLOCK. +/* : ┃ - */ if { let __result = spidey > goblin; __incr_cov(from!("Green => or end of MatchArmGuard expression inside pattern, if any")); __result } { +/* : : ┃ */ println!("spidey beats goblin"); +/* : : ┃ */ __incr_cov(from!("block start")); +/* : ┃ - */ } else if { let __result = spidey == goblin; __incr_cov(from!("`else if` on this line")); __result } { +/* : : ┃ */ // COVERAGE NOTE: Do we mark only the expression span (that may be trivial, as in this case), +/* : : ┃ */ // or associate it with the outer block, similar to how the `if` expression is associated with +/* : : ┃ */ // the outer block? (Although it is a continuation, in a sense, it is discontiguous in this case, +/* : : ┃ */ // so I think simpler to just make it its own coverage region.) +/* : : ┃ */ println!("it's a draw"); +/* : : ┃ */ __incr_cov(from!("block start")); +/* : ┃ - - - */ } else if if { let __result = true; __incr_cov(from!("`else if` on this line")); __result } { +/* : : : ┃ */ // return __incr_cov(from!("after `if true`"),Ok(())); +/* : : : ┃ */ // ACTUALLY, BECAUSE OF `return`, WE DO NOT RECORD THE `if true` EVEN THOUGH WE COVERED IT. +/* : : : ┃ */ // IN FACT, IF THIS NESTED CONDITIONAL IN A CONDITIONAL EXPRESSION WAS AN `if` (WITHOUT PRECEDING ELSE) +/* : : : ┃ */ // WE WOULD NOT HAVE RECORDED THE COVERAGE OF STATEMENTS LEADING UP TO THE `if`, SO +/* : : : ┃ */ // IT SHOULD BE: +/* ┏-:---:-------:---< */ return { let __result = Ok(()); __incr_cov(from!("")); __result }; +/* V : : : : */ // NOTE THE `from` STRING IS SAME FOR THE `else if`s `__incr_cov` AND THIS `return`. +/* : : : : */ // ONLY ONE OF THESE WILL EXECUTE, TO RECORD COVERAGE FROM THAT SPOT. +/* : : ┃ - */ } else { +/* : : : I */ { let __result = false; __incr_cov(from!("`else`")); __result } +/* : : - - */ } { +/* : : ┃ */ println!("wierd science"); +/* : : ┃ */ __incr_cov(from!("block start")); +/* : ┃ - */ } else { +/* : : ┃ */ println!("goblin wins"); +/* ┏-:---:---< */ return { let __result = Ok(()); __incr_cov(from!("`else`")); __result }; // THIS COUNTS LAST STATEMENT IN `else` BLOCK +/* V : : : */ // COVERAGE NOTE: When counting the span for `return`, +/* : : : */ // `break`, or `continue`, also report the outer spans +/* : : : */ // got this far--including this `else` block. Record +/* : : : */ // The start positions for those outer blocks, but: +/* : : : */ // * For the block containing the `return`, `break`, or +/* : : : */ // `continue`, end report the end position is the +/* : : : */ // start of the `return` span (or 1 char before it). +/* : : : */ // * Anything else? +/* : ┃ - */ } +/* : ┃ - */ // __incr_cov(from!("")); // DO NOT COUNT HERE IF NO STATEMENTS AFTER LAST `if` or `match` +/* : - */ }, +/* : I */ Blue => { println!("violets"); __incr_cov(from!("Blue => or end of MatchArmGuard expression inside pattern, if any")); } +/* ┃ */ } +/* ┃ */ +/* ┃ */ let condition1 = true; +/* ┃ */ let condition2 = false; +/* ┃ */ let condition3 = true; +/* ┃ */ +/* ┃ */ println!("Called `various()` for TestStruct with field={}", self.field); +/* ┃ */ +/* ┃ - */ if { let __result = condition1; __incr_cov(from!("after block end of prior `match` (or `if-else if-else`)")); __result } { +/* : ┃ */ println!("before for loop"); +/* : ┃ - */ for index in { let __result = 0..10; __incr_cov(from!("block start")); __result } { +/* : : ┃ */ println!("top of `for` loop"); +/* : : ┃ - */ if { let __result = index == 8; __incr_cov(from!("block start")); __result } { +/* : : : ┃ */ println!("before break"); +/* : : : ┃ */ // note the following is not legal here: +/* : : : ┃ */ // "can only break with a value inside `loop` or breakable block" +/* : : : ┃ */ // break __incr_cov(from!("")); +/* : : : ┃ */ __incr_cov(from!("block start")); +/* : : ┏-----< */ break; +/* : : V : : */ +/* : : : : */ // FIXME(richkadel): add examples with loop labels, breaking out of inner and outer loop to outer loop label, with expression. +/* : : : : */ // May want to record both the span and the start position after the broken out block depdnding on label +/* : : ┃ - */ } +/* : : ┃ */ println!("after `break` test"); +/* : : ┃ - */ if { let __result = condition2; __incr_cov(from!("block end of `if index == 8`")); __result } { +/* ┏-:---:---:---< */ return { let __result = Ok(()); __incr_cov(from!("block start")); __result }; +/* V : : ┃ - */ } +/* : : ┃ */ +/* : : ┃ */ // BECAUSE THE PREVIOUS COVERAGE REGION HAS A `return`, THEN +/* : : ┃ */ // IF PREVIOUS COVERAGE REGION IS NOT COUNTED THEN OUTER REGION REACHED HERE. +/* : : ┃ */ // ADD A COVERAGE REGION FOR THE SPAN FROM JUST AFTER PREVIOUS REGION TO END +/* : : ┃ */ // OF OUTER SPAN, THEN TRUNCATE TO NEXT REGION NOT REACHED. +/* : : ┃ - */ if index % 3 == 2 { // NO __incr_cov() HERE BECAUSE NO STATEMENTS BETWEEN LAST CONDITIONAL BLOCK AND START OF THIS ONE +/* : : Λ : ┃ */ __incr_cov(from!("block end of `if condition2`")); +/* : : ┗-----< */ continue; +/* : : ┃ - */ } +/* : : ┃ */ println!("after `continue` test"); +/* : : ┃ */ // maybe add a runtime flag for a possible `return` here? +/* : : ┃ */ __incr_cov(from!("")); +/* : ┃ - */ } +/* : ┃ */ println!("after for loop"); +/* : ┃ */ let result = if { // START OF NEW CONDITIONAL EXPRESSION. NEXT "GUARANTEED" COUNTER SHOULD COUNT FROM END OF LAST CONDITIONAL EXPRESSION +/* : ┃ */ // A "GUARANTEED" COUNTER CALL IS ONE THAT WILL BE CALLED REGARDLESS OF OTHER CONDITIONS. THIS INCLUDES: +/* : ┃ */ // * A CONDITIONAL EXPRESSION THAT IS NOT A BLOCK (OR ANOTHER CONDITIONAL STATEMENT, WHICH WOULD CONTAIN A BLOCK) +/* : ┃ */ // * OR IF THE NEXT CONDITIONAL EXPRESSION IS A BLOCK OR CONDITIONAL STATEMENT, THEN THE FIRST "GUARANTEED" COUNTER IN THAT BLOCK +/* : ┃ */ // * END OF BLOCK IF THE BLOCK DOES NOT HAVE INNER CONDITIONAL EXPRESSIONS +/* : ┃ */ // * BRANCHING STATEMENTS (`return`, `break`, `continue`) BY EITHER WRAPPING THE BRANCH STATEMENT NON-BLOCK EXPRESSION, +/* : ┃ */ // OR PREPENDING A COUNTER WITH EMPTY TUPLE IF NO EXPRESSION, OR IF EXPRESSION IS A BLOCK, THEN THE NEXT "GUARANTEED" +/* : ┃ */ // COUNTER CALL WITHIN THAT BLOCK. +/* : ┃ */ // BASICALLY, CARRY THE START OF COVERAGE SPAN FORWARD UNTIL THE GUARANTEED COUNTER IS FOUND +/* : ┃ */ println!("after result = if ..."); +/* : ┃ - */ if { let __result = condition2; __incr_cov(from!("block end of `for` loop")); __result } { +/* : : ┃ */ println!("before first return"); +/* ┏-:---:-------< */ return { let __result = Ok(()); __incr_cov(from!("block start")); __result }; +/* V : : - */ } else if { let __result = condition3; __incr_cov(from!("`else`")); __result } { +/* : : ┃ */ // THE ABOVE COUNTER IS _NOT_ REALLY NECESSARY IF EXPRESSION IS GUARANTEED TO EXECUTE. +/* : : ┃ */ // IF WE GET COUNTER IN `else if` BLOCK WE COVERED EXPRESSION. +/* : : ┃ */ // IF WE GET TO ANY REMAINING `else` or `else if` BLOCK WE KNOW WE EVALUATED THIS CONDITION +/* : : ┃ */ // AND ALL OTHERS UP TO THE EXECUTED BLOCK. BUT THE SPAN WOULD HAVE "HOLES" FOR UNEXECUTED BLOCKS. +/* : : ┃ */ println!("not second return"); +/* ┏-:---:-------< */ return { let __result = Ok(()); __incr_cov(from!("block start")); __result }; +/* V : : - */ } else { +/* : : ┃ */ println!("not returning"); +/* : : ┃ */ { let __result = false; __incr_cov(from!("block start")); __result } +/* : : - */ } +/* : ┃ */ // NO COUNTER HERE BECAUSE NO STATEMENTS AFTER CONDITIONAL BLOCK +/* : ┃ - */ } { +/* : : ┃ */ println!("branched condition returned true"); +/* : : ┃ */ { let __result = Ok(()); __incr_cov(from!("")); __result } +/* : ┃ - */ } else if self.call_closure( +/* : : - */ |closure_param| { +/* : : ┃ - */ let __result = if condition3 { +/* : : : ┃ */ println!("in closure, captured condition said to print the param {}", closure_param); +/* : : : ┃ */ { let __result = false; __incr_cov(from!("")); __result } +/* : : ┃ - */ } else { +/* : : : ┃ */ println!("in closure, captured condition was false"); +/* : : : ┃ */ { let __result = true; __incr_cov(from!("")); __result } +/* : : ┃ - */ }; +/* : : - */ __incr_cov(from!("")); __result } +/* : : - */ ) { +/* : : ┃ */ println!("closure returned true"); +/* : : ┃ */ { let __result = Err(Error::new(ErrorKind::Other, "Result is error if closure returned true")); __incr_cov(from!("")); __result } +/* : ┃ - */ } else { +/* : : ┃ */ println!("closure returned false"); +/* : : ┃ */ { let __result = Err(Error::new(ErrorKind::Other, "Result is error if closure returned false")); __incr_cov(from!("")); __result } +/* : ┃ - */ }; +/* : ┃ */ println!("bottom of function might be skipped if early `return`"); +/* : ┃ */ { let __result = result; __incr_cov(from!("if condition1")); __result } +/* ┃ - */ } else { +/* : ┃ */ println!("skipping everything in `various()`"); +/* : ┃ */ { let __result = Ok(()); __incr_cov(from!("")); __result } +/* ┃ - */ } +/* ┃ - */ // __incr_cov(from!(""),0) // DO NOT COUNT IF NO STATEMENTS AFTER CONDITIONAL BLOCK. ALL COVERAGE IS ALREADY COUNTED +/* - */ } +/* */ } +/* */ +/* - */ fn main() -> Result<(), std::io::Error> { +/* ┃ */ //let mut status: u8 = 2; +/* ┃ */ let mut status: u8 = 1; +/* : - */ let result = if status < 2 && +/* : ┃ */ { let __result = { +/* : ┃ */ status -= 1; +/* : ┃ */ status == 0 +/* : - - */ }; __incr_cov(from!("")); __result } { +/* : ┃ */ let test_struct = TestStruct::new_with_value(100); +/* : ┃ */ let _ = test_struct.various(); +/* ┏-:---< */ return { let __result = Err(Error::new(ErrorKind::Other, format!("Error status {}", status))); __incr_cov(from!("")); __report(); __result } +/* V : - */ } else { +/* : ┃ */ let test_struct = TestStruct::new(); +/* : ┃ */ { let __result = test_struct.various(); __incr_cov(from!("")); __result } +/* : - */ }; +/* ┃ */ println!("done"); +/* ┃ */ { let __result = result; __incr_cov(from!("")); __report(); __result } +/* - */ } \ No newline at end of file diff --git a/src/test/codegen/coverage-experiments/src/coverage_injection_test_alt.rs b/src/test/codegen/coverage-experiments/src/coverage_injection_test_alt.rs new file mode 100644 index 00000000000..20c4835dd88 --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/coverage_injection_test_alt.rs @@ -0,0 +1,362 @@ +/* */ use std::io::Error; +/* */ use std::io::ErrorKind; +/* */ +/* */ /// Align Rust counter increment with with: +/* */ /// [‘llvm.instrprof.increment’ Intrinsic](https://llvm.org/docs/LangRef.html#llvm-instrprof-increment-intrinsic) +/* */ /// +/* */ /// declare void @llvm.instrprof.increment(i8* , i64 , i32 , i32 ) +/* */ /// +/* */ /// The first argument is a pointer to a global variable containing the name of the entity +/* */ /// being instrumented. This should generally be the (mangled) function name for a set of +/* */ /// counters. +/* */ /// +/* */ /// The second argument is a hash value that can be used by the consumer of the profile data +/* */ /// to detect changes to the instrumented source, and the third is the number of counters +/* */ /// associated with name. It is an error if hash or num-counters differ between two +/* */ /// instances of instrprof.increment that refer to the same name. +/* */ /// +/* */ /// The last argument refers to which of the counters for name should be incremented. It +/* */ /// should be a value between 0 and num-counters. +/* */ /// +/* */ /// # Arguments +/* */ /// +/* */ /// `mangled_fn_name` - &'static ref to computed and injected static str, using: +/* */ /// +/* */ /// ``` +/* */ /// fn rustc_symbol_mangling::compute_symbol_name( +/* */ /// tcx: TyCtxt<'tcx>, +/* */ /// instance: Instance<'tcx>, +/* */ /// compute_instantiating_crate: impl FnOnce() -> CrateNum, +/* */ /// ) -> String +/* */ /// ``` +/* */ /// +/* */ /// `source_version_hash` - Compute hash based that only changes if there are "significant" +/* */ /// to control-flow inside the function. +/* */ /// +/* */ /// `num_counters` - The total number of counter calls [MAX(counter_index) + 1] within the +/* */ /// function. +/* */ /// +/* */ /// `counter_index` - zero-based counter index scoped by the function. (Ordering of +/* */ /// counters, relative to the source code location, is apparently not expected.) +/* */ /// +/* */ /// # Notes +/* */ /// +/* */ /// * The mangled_fn_name may not be computable until generics are monomorphized (see +/* */ /// parameters required by rustc_symbol_mangling::compute_symbol_name). +/* */ /// * The version hash may be computable from AST analysis, and may not benefit from further +/* */ /// lowering. +/* */ /// * num_counters depends on having already identified all counter insertion locations. +/* */ /// * counter_index can be computed at time of counter insertion (incrementally). +/* */ /// * Numeric parameters are signed to match the llvm increment intrinsic parameter types. +/* */ fn __lower_incr_cov(_mangled_fn_name: &'static str, _fn_version_hash: i64, _num_counters: i32, _counter_index: i32) { +/* */ } +/* */ +/* */ /// A coverage counter implementation that will work as both an intermediate coverage +/* */ /// counting and reporting implementation at the AST-level only--for debugging and +/* */ /// development--but also serves as a "marker" to be replaced by calls to LLVM +/* */ /// intrinsic coverage counter APIs during the lowering process. +/* */ /// +/* */ /// Calls to this function will be injected automatically into the AST. When LLVM intrinsics +/* */ /// are enabled, the counter function calls that were injected into the AST serve as +/* */ /// placeholders, to be replaced by an alternative, such as: +/* */ /// +/* */ /// * direct invocation of the `llvm.instrprof.increment()` intrinsic; or +/* */ /// * the `__lower_incr_cov()` function, defined above, that would invoke the +/* */ /// `llvm.instrprof.increment()` intrinsic; or +/* */ /// * a similar expression wrapper, with the additional parameters (as defined above +/* */ /// for `__lower_incr_cov()`, that invokes `llvm.instrprof.increment()` and returns the +/* */ /// result of the wrapped expression) +/* */ /// +/* */ /// The first two options would require replacing the inlined wrapper call with something +/* */ /// like: +/* */ /// +/* */ /// ``` +/* */ /// { let result = {expr}; __inlined_incr_cov(context, counter); result } +/* */ /// ``` +/* */ /// +/* */ /// But if the lowering process is already unwrapping the inlined call to `__incr_cov()`, then +/* */ /// it may be a perfect opportunity to replace the function with one of these more +/* */ /// direct methods. +/* */ /// +/* */ #[inline(always)] +/* */ pub fn __incr_cov(region_loc: &str, /*index: u32,*/) { +/* */ // Either call the intermediate non-llvm coverage counter API or +/* */ // replace the call to this function with the expanded `__lower_incr_cov()` call. +/* */ +/* */ // let _lock = increment_counter(counter); +/* */ println!("{}", region_loc); +/* */ } +/* */ +/* */ /// Write a report identifying each incremented counter and the number of times each counter +/* */ /// was incremented. +/* */ fn __report() { +/* */ println!("WRITE REPORT!"); +/* */ } +/* */ +/* */ /// Increment the counter after evaluating the wrapped expression (see `__incr_cov()`), then +/* */ /// write a report identifying each incremented counter and the number of times each counter +/* */ /// was incremented. +/* */ #[inline(always)] +/* */ pub fn __incr_cov_and_report(region_loc: &str, /*counter: u32,*/ result: T) -> T { +/* */ __incr_cov(region_loc, /*counter,*/); +/* */ __report(); +/* */ result +/* */ } +/* */ +/* */ macro_rules! from { +/* */ ($from:expr) => { &format!("from: {}\n to: {}:{}:{}", $from, file!(), line!(), column!()) }; +/* */ } +/* */ +/* */ macro_rules! to { +/* */ ($to:expr) => { &format!("to: {}\n to: {}:{}:{}", $to, file!(), line!(), column!()) }; +/* */ } +/* */ +/* */ #[derive(Debug)] +/* */ enum TestEnum { +/* */ Red, +/* */ Green, +/* */ Blue, +/* */ } +/* */ +/* */ struct TestStruct { +/* */ field: i32, +/* */ } +/* */ +/* */ // IMPORTANT! IS WRAPPING main() ENOUGH? OR DO I ALSO NEED TO WRAP THREAD FUNCTIONS, ASSUMING +/* */ // THEY ARE STILL RUNNING WITH MAIN EXITS? (IF THEY CAN). NOT SURE HOW RUST HANDLES THAT. +/* */ +/* */ // I SUSPECT USING THREAD_LOCAL COUNTERS MAY NOT ACTUALLY BE AN OPTIMIZATION OVER MUTEX LOCKS, +/* */ // BUT MAYBE I SHOULD ASK. +/* */ +/* */ impl TestStruct { +/* - */ fn new() -> Self { +/* ┃ */ __incr_cov(to!("end of fn new()")); // function-scoped counter index = 0 +/* ┃ */ Self::new_with_value(31415) +/* - */ } +/* */ +/* - */ fn new_with_value(field: i32) -> Self { +/* ┃ */ __incr_cov(to!("end of fn new_with_value()")); // function-scoped counter index = 0 +/* ┃ */ Self { +/* ┃ */ field, +/* ┃ */ } +/* - */ } +/* */ +/* */ fn call_closure(&self, closure: F) -> bool +/* */ where +/* */ F: FnOnce( +/* */ i32, +/* */ ) -> bool, +/* - */ { +/* ┃ */ __incr_cov(to!("end of fn call_closure()")); // function-scoped counter index = 0 +/* ┃ */ closure(123) +/* - */ } +/* */ +/* - */ fn various(&self) -> Result<(),Error> { +/* ┃ */ __incr_cov(to!("just before next branch: after `match color`: pattern selection")); +/* ┃ */ use TestEnum::*; +/* ┃ */ let mut color = Red; +/* ┃ */ let _ = color; +/* ┃ */ color = Blue; +/* ┃ */ let _ = color; +/* ┃ */ color = Green; +/* ┃ */ match color { // function-scoped counter index = 0 +/* : */ +/* : */ // !!! RECORD SPAN FROM START OF INNERMOST CONTAINING BLOCK (THE FUNCTION IN THIS CASE) TO END OF MATCH EXPRESSION +/* : */ // If `match`, `while`, `loop`, `for`, `if`, etc. expression has a `return`, `break`, or `continue` +/* : */ // (if legal), then RECORD SPAN FROM START OF INNERMOST CONTAINING BLOCK TO END OF `return` EXPRESSION +/* : */ // If the expression includes lazy booleans, nest calls to `__incr_cov()`. +/* : - */ Red => { +/* : ┃ */ __incr_cov(to!("end of matched Red")); +/* : ┃ */ println!("roses"); +/* : - */ } +/* : - */ Green => { +/* : ┃ */ __incr_cov(to!("just before next branch: after `if spidey > goblin`")); +/* : ┃ */ let spidey = 100; +/* : ┃ */ let goblin = 50; +/* : ┃ */ // if spidey > goblin {__incr_cov(from!(""),{ +/* : ┃ */ // println!("what ev"); +/* : ┃ */ // })} +/* : ┃ */ // ACTUALLY, WRAPPING THE ENTIRE IF BLOCK IN `__incr_cov` IS NOT A GREAT GENERAL RULE. +/* : ┃ */ // JUST INSERTING A `return`, `break`, or `continue` IN THAT BLOCK (without an intermediate condition) +/* : ┃ */ // MAKES THE `__incr_cov()` CALL UNREACHABLE! +/* : ┃ */ // MY ORIGINAL SOLUTION WORKS BETTER (WRAP LAST EXPRESSION OR AFTER LAST SEMICOLON STATEMENT IN BLOCK) +/* : ┃ */ // UNLESS THE EXPRESSION IS NOT A BLOCK. +/* : ┃ - */ if spidey > goblin { +/* : : ┃ */ __incr_cov(to!("end of if block, if no earlier branch in this scope")); +/* : : ┃ */ println!("spidey beats goblin"); +/* : : ┃ */ +/* : ┃ - */ } else if { +/* : : : */ // Make sure we can't compute the coverage count here. +/* : : : */ // We know the expression executed if the previous if block DID NOT +/* : : : */ // execute, and either this `else if` block does execute OR any subsequent +/* : : : */ // `else if` or `else` blocks execute, OR none of the blocks in the +/* : : : */ // `if`, `else if` or `else` blocks execute. +/* : : : */ // `if`, `else if` or `else` blocks execute. +/* : : ┃ */ __incr_cov(to!("end of `else if spidey == goblin` expression")); +/* : : ┃ */ spidey == goblin +/* : ┃ - */ } { +/* : : ┃ */ __incr_cov(to!("end of if block, if no earlier branch in this scope")); +/* : : ┃ */ // COVERAGE NOTE: Do we mark only the expression span (that may be trivial, as in this case), +/* : : ┃ */ // or associate it with the outer block, similar to how the `if` expression is associated with +/* : : ┃ */ // the outer block? (Although it is a continuation, in a sense, it is discontiguous in this case, +/* : : ┃ */ // so I think simpler to just make it its own coverage region.) +/* : : ┃ */ println!("it's a draw"); +/* : : ┃ */ +/* : ┃ - - - */ } else if { +/* : : ┃ */ __incr_cov(to!("end of `if true`")); +/* : ┃ - - - */ if true { +/* : : : ┃ */ __incr_cov(to!("end of `return Ok(())`")); +/* ┏-:---:-------:---< */ return Ok(()); +/* V : : ┃ - */ } else { +/* : : : ┃ */ // __incr_cov(to!("end of else block")); +/* : : : ┃ */ // computed counter expression +/* : : : ┃ */ false +/* : : : - */ } +/* : : - - - */ } { +/* : : ┃ */ __incr_cov(to!("end of if block")); +/* : : ┃ */ println!("wierd science"); +/* : ┃ - */ } else { +/* : : ┃ */ // __incr_cov(to!("end of `return Ok(())")); +/* : : ┃ */ // counter expression: (start of Green match arm) - (if spidey > goblin) - (previous `} else if {`) +/* : : ┃ */ println!("goblin wins"); +/* ┏-:---:---< */ return Ok(()); // THIS COUNTS LAST STATEMENT IN `else` BLOCK +/* V : : : */ // COVERAGE NOTE: When counting the span for `return`, +/* : : : */ // `break`, or `continue`, also report the outer spans +/* : : : */ // got this far--including this `else` block. Record +/* : : : */ // The start positions for those outer blocks, but: +/* : : : */ // * For the block containing the `return`, `break`, or +/* : : : */ // `continue`, end report the end position is the +/* : : : */ // start of the `return` span (or 1 char before it). +/* : : : */ // * Anything else? +/* : ┃ - */ } +/* : : */ // __incr_cov(to!("end of matched Green")); +/* : : */ // // DO NOT COUNT HERE IF NO STATEMENTS AFTER LAST `if` or `match` +/* : - */ }, +/* : - */ Blue => { +/* : ┃ */ __incr_cov(to!("end of matched Blue")); +/* : ┃ */ println!("violets"); +/* : - */ } +/* ┃ */ } +/* ┃ */ __incr_cov(to!("just before next branch: after `if condition1` (HIR: 'match condition1')")); +/* ┃ */ +/* ┃ */ let condition1 = true; +/* ┃ */ let condition2 = false; +/* ┃ */ let condition3 = true; +/* ┃ */ +/* ┃ */ println!("Called `various()` for TestStruct with field={}", self.field); +/* ┃ */ +/* ┃ - */ if condition1 { +/* : ┃ */ println!("before while loop"); +/* : ┃ */ let mut countdown = 10; +/* : ┃ */ // Must increment before repeated while text expression +/* : : I */ while countdown > 0 { // span is just the while test expression +/* : : ┃ */ println!("top of `while` loop"); +/* : : ┃ */ countdown -= 1; +/* : : ┃ */ // // Counter not needed, but span is computed as "while test" minus "block start" +/* : : ┃ */ // If test expression is 11, and the outer block runs only once, 11-1 = 10 +/* : ┃ - */ } +/* : ┃ */ println!("before for loop"); +/* : ┃ - */ for index in 0..10 { +/* : : ┃ */ println!("top of `for` loop"); +/* : : ┃ - */ if index == 8 { +/* : : : ┃ */ println!("before break"); +/* : : : ┃ */ // note the following is not legal here: +/* : : : ┃ */ // "can only break with a value inside `loop` or breakable block" +/* : : : ┃ */ // break +/* : : : ┃ */ +/* : : ┏-----< */ break; +/* : : V : : */ +/* : : : : */ // FIXME(richkadel): add examples with loop labels, breaking out of inner and outer loop to outer loop label, with expression. +/* : : : : */ // May want to record both the span and the start position after the broken out block depdnding on label +/* : : ┃ - */ } +/* : : ┃ */ println!("after `break` test"); +/* : : ┃ - */ if condition2 { +/* ┏-:---:---:---< */ return Ok(()); +/* V : : ┃ - */ } +/* : : ┃ */ +/* : : ┃ */ // BECAUSE THE PREVIOUS COVERAGE REGION HAS A `return`, THEN +/* : : ┃ */ // IF PREVIOUS COVERAGE REGION IS NOT COUNTED THEN OUTER REGION REACHED HERE. +/* : : ┃ */ // ADD A COVERAGE REGION FOR THE SPAN FROM JUST AFTER PREVIOUS REGION TO END +/* : : ┃ */ // OF OUTER SPAN, THEN TRUNCATE TO NEXT REGION NOT REACHED. +/* : : ┃ - */ if index % 3 == 2 { // NO __incr_cov() HERE BECAUSE NO STATEMENTS BETWEEN LAST CONDITIONAL BLOCK AND START OF THIS ONE +/* : : Λ : ┃ */ +/* : : ┗-----< */ continue; +/* : : ┃ - */ } +/* : : ┃ */ println!("after `continue` test"); +/* : : ┃ */ // maybe add a runtime flag for a possible `return` here? +/* : : ┃ */ +/* : ┃ - */ } +/* : ┃ */ println!("after for loop"); +/* : ┃ */ let result = if { // START OF NEW CONDITIONAL EXPRESSION. NEXT "GUARANTEED" COUNTER SHOULD COUNT FROM END OF LAST CONDITIONAL EXPRESSION +/* : ┃ */ // A "GUARANTEED" COUNTER CALL IS ONE THAT WILL BE CALLED REGARDLESS OF OTHER CONDITIONS. THIS INCLUDES: +/* : ┃ */ // * A CONDITIONAL EXPRESSION THAT IS NOT A BLOCK (OR ANOTHER CONDITIONAL STATEMENT, WHICH WOULD CONTAIN A BLOCK) +/* : ┃ */ // * OR IF THE NEXT CONDITIONAL EXPRESSION IS A BLOCK OR CONDITIONAL STATEMENT, THEN THE FIRST "GUARANTEED" COUNTER IN THAT BLOCK +/* : ┃ */ // * END OF BLOCK IF THE BLOCK DOES NOT HAVE INNER CONDITIONAL EXPRESSIONS +/* : ┃ */ // * BRANCHING STATEMENTS (`return`, `break`, `continue`) BY EITHER WRAPPING THE BRANCH STATEMENT NON-BLOCK EXPRESSION, +/* : ┃ */ // OR PREPENDING A COUNTER WITH EMPTY TUPLE IF NO EXPRESSION, OR IF EXPRESSION IS A BLOCK, THEN THE NEXT "GUARANTEED" +/* : ┃ */ // COUNTER CALL WITHIN THAT BLOCK. +/* : ┃ */ // BASICALLY, CARRY THE START OF COVERAGE SPAN FORWARD UNTIL THE GUARANTEED COUNTER IS FOUND +/* : ┃ */ println!("after result = if ..."); +/* : ┃ - */ if condition2 { +/* : : ┃ */ println!("before first return"); +/* ┏-:---:-------< */ return Ok(()); +/* V : : - */ } else if condition3 { +/* : : ┃ */ // THE ABOVE COUNTER IS _NOT_ REALLY NECESSARY IF EXPRESSION IS GUARANTEED TO EXECUTE. +/* : : ┃ */ // IF WE GET COUNTER IN `else if` BLOCK WE COVERED EXPRESSION. +/* : : ┃ */ // IF WE GET TO ANY REMAINING `else` or `else if` BLOCK WE KNOW WE EVALUATED THIS CONDITION +/* : : ┃ */ // AND ALL OTHERS UP TO THE EXECUTED BLOCK. BUT THE SPAN WOULD HAVE "HOLES" FOR UNEXECUTED BLOCKS. +/* : : ┃ */ println!("not second return"); +/* ┏-:---:-------< */ return Ok(()); +/* V : : - */ } else { +/* : : ┃ */ println!("not returning"); +/* : : ┃ */ false +/* : : - */ } +/* : ┃ */ // NO COUNTER HERE BECAUSE NO STATEMENTS AFTER CONDITIONAL BLOCK +/* : ┃ - */ } { +/* : : ┃ */ println!("branched condition returned true"); +/* : : ┃ */ Ok(()) +/* : ┃ - */ } else if self.call_closure( +/* : : - */ |closure_param| +/* : : ┃ - */ if condition3 { +/* : : : ┃ */ println!("in closure, captured condition said to print the param {}", closure_param); +/* : : : ┃ */ false +/* : : ┃ - */ } else { +/* : : : ┃ */ println!("in closure, captured condition was false"); +/* : : : ┃ */ true +/* : : ┃ - */ } +/* : : - */ +/* : : - */ ) { +/* : : ┃ */ println!("closure returned true"); +/* : : ┃ */ Err(Error::new(ErrorKind::Other, "Result is error if closure returned true")) +/* : ┃ - */ } else { +/* : : ┃ */ println!("closure returned false"); +/* : : ┃ */ Err(Error::new(ErrorKind::Other, "Result is error if closure returned false")) +/* : ┃ - */ }; +/* : ┃ */ println!("bottom of function might be skipped if early `return`"); +/* : ┃ */ result +/* ┃ - */ } else { +/* : ┃ */ println!("skipping everything in `various()`"); +/* : ┃ */ Ok(()) +/* ┃ - */ } +/* ┃ - */ // 0 // DO NOT COUNT IF NO STATEMENTS AFTER CONDITIONAL BLOCK. ALL COVERAGE IS ALREADY COUNTED +/* - */ } +/* */ } +/* */ +/* - */ fn main() -> Result<(), std::io::Error> { +/* ┃ */ //let mut status: u8 = 2; +/* ┃ */ let mut status: u8 = 1; +/* : - */ let result = if status < 2 && +/* : ┃ */ { +/* : ┃ */ status -= 1; +/* : ┃ */ status == 0 +/* : - - */ } { +/* : ┃ */ let test_struct = TestStruct::new_with_value(100); +/* : ┃ */ let _ = test_struct.various(); +/* ┏-:---< */ return __incr_cov_and_report(from!(""),Err(Error::new(ErrorKind::Other, format!("Error status {}", status)))) +/* V : - */ } else { +/* : ┃ */ let test_struct = TestStruct::new(); +/* : ┃ */ test_struct.various() +/* : - */ }; +/* ┃ */ println!("done"); +/* ┃ */ __incr_cov_and_report(from!(""),result) // function-scoped counter index = 0 +/* - */ } \ No newline at end of file diff --git a/src/test/codegen/coverage-experiments/src/drop_trait.rs b/src/test/codegen/coverage-experiments/src/drop_trait.rs new file mode 100644 index 00000000000..75400e037e9 --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/drop_trait.rs @@ -0,0 +1,25 @@ +#[inline(always)] +pub fn __incr_cov(_region_loc: &str, result: T) -> T { + result +} + +struct Firework { + _strength: i32, +} + +impl Drop for Firework { + fn drop(&mut self) { + __incr_cov("start of drop()", ()); + } +} + +fn main() -> Result<(),u8> { + let _firecracker = Firework { _strength: 1 }; + + if __incr_cov("start of main()", true) { + return __incr_cov("if true", { let _t = Err(1); _t }); + } + + let _tnt = Firework { _strength: 100 }; + Ok(()) +} \ No newline at end of file diff --git a/src/test/codegen/coverage-experiments/src/drop_trait_with_comments_prints.rs b/src/test/codegen/coverage-experiments/src/drop_trait_with_comments_prints.rs new file mode 100644 index 00000000000..de9f5d5cb46 --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/drop_trait_with_comments_prints.rs @@ -0,0 +1,53 @@ +// +// +// +// It's interesting to speculate if there is a way to leverage the Drop trait functionality +// to increment counters when a scope is closed, but I don't think it would help "out of the box". +// +// A `return` or `break` with expression might not need a temp value expression wrapper +// such as `return { let _t = result_expression; __incr_counter(...); _t };` +// +// ... **if** the __incr_counter() was somehow called from a "drop()" trait function. +// +// The problem is, since the drop call is automatic, there is no way to have argument variants +// depending on where the drop() occurs (e.g., from a `return` statement vs. from the end of +// the function). We need 2 different code regions though. +// +// +// +// + +#[inline(always)] +pub fn __incr_cov(_region_loc: &str, /*index: u32,*/ result: T) -> T { + // println!("from: {}", _region_loc); + result +} + +struct Firework { + strength: i32, +} + +impl Drop for Firework { + fn drop(&mut self) { + println!("BOOM times {}!!!", self.strength); + __incr_cov("start of drop()", ()); + } +} + +fn main() -> Result<(),u8> { + let _firecracker = Firework { strength: 1 }; + + if __incr_cov("start of main()", true) { + return __incr_cov("if true", { let _t = Err(1); println!("computing return value"); _t }); + } + + let _tnt = Firework { strength: 100 }; + // __incr_cov("after if block", Ok(())) // CAN USE COUNTER EXPRESSION: "start of drop()" - "if true" + Ok(()) +} + +// OUTPUT WHEN RUNNING THIS PROGRAM IS AS EXPECTED: + +// computing return value +// BOOM times 1!!! +// Error: 1 diff --git a/src/test/codegen/coverage-experiments/src/for.rs b/src/test/codegen/coverage-experiments/src/for.rs new file mode 100644 index 00000000000..3f44c382a1e --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/for.rs @@ -0,0 +1,41 @@ +#[inline(always)] +pub fn __incr_cov(_region_loc: &str, /*index: u32,*/ result: T) -> T { + result +} + +fn main() { + for countdown in __incr_cov("start", 10..0) { + let _ = countdown; + __incr_cov("top of for", ()); + } +} + +// LOWERED TO HIR: +// +// fn main() { +// { +// let _t = +// match ::std::iter::IntoIterator::into_iter(__incr_cov("start", +// ::std::ops::Range{start: +// 10, +// end: +// 0,})) +// { +// mut iter => +// loop { +// let mut __next; +// match ::std::iter::Iterator::next(&mut iter) { +// ::std::option::Option::Some(val) => +// __next = val, +// ::std::option::Option::None => break , +// } +// let countdown = __next; +// { +// let _ = countdown; +// __incr_cov("top of for", ()); +// } +// }, +// }; +// _t +// } +// } \ No newline at end of file diff --git a/src/test/codegen/coverage-experiments/src/for_with_comments.rs b/src/test/codegen/coverage-experiments/src/for_with_comments.rs new file mode 100644 index 00000000000..03d11b2c230 --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/for_with_comments.rs @@ -0,0 +1,24 @@ +/* */ #[inline(always)] +/* */ pub fn __incr_cov(_region_loc: &str, /*index: u32,*/ result: T) -> T { +/* */ result +/* */ } +/* */ +/* - */ fn main() { +/* : I */ for countdown in __incr_cov("start", 10..0) { // span is just the while test expression +/* : ┃ */ let _ = countdown; +/* : ┃ */ __incr_cov("top of for", ()); +/* ┃ - */ } +/* - */ } + + +// -Z unpretty=val -- present the input source, unstable (and less-pretty) variants; +// valid types are any of the types for `--pretty`, as well as: +// `expanded`, `expanded,identified`, +// `expanded,hygiene` (with internal representations), +// `everybody_loops` (all function bodies replaced with `loop {}`), +// `hir` (the HIR), `hir,identified`, +// `hir,typed` (HIR with types for each node), +// `hir-tree` (dump the raw HIR), +// `mir` (the MIR), or `mir-cfg` (graphviz formatted MIR) + +// argument to `pretty` must be one of `normal`, `expanded`, `identified`, or `expanded,identified` diff --git a/src/test/codegen/coverage-experiments/src/if.rs b/src/test/codegen/coverage-experiments/src/if.rs new file mode 100644 index 00000000000..ad50f6be190 --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/if.rs @@ -0,0 +1,80 @@ +#![feature(core_intrinsics)] + +pub fn __llvm_incr_counter(_region_loc: &str) { +} + +#[inline(always)] +pub fn __incr_cov(region_loc: &str, result: T) -> T { + __llvm_incr_counter(region_loc); + result +} + +static TEST_FUNC_NAME: &'static [u8; 6] = b"main()"; + +fn main() { + let mut countdown = 10; + if __incr_cov("start", countdown > 0) { + + + // // TEST CALLING INTRINSIC: + unsafe { core::intrinsics::instrprof_increment(TEST_FUNC_NAME as *const u8, 1234 as u64, 314 as u32, 31 as u32) }; + // // Results in: + // // LLVM ERROR: Cannot select: intrinsic %llvm.instrprof.increment + // // I may need to pass one or more of the following flags (or equivalent opts) to LLVM to enable this: + // // -fprofile-instr-generate -fcoverage-mapping + + + countdown -= 1; + __incr_cov("if block",()); + } else if countdown > 5 { + countdown -= 2; + __incr_cov("else if block",()); + } else { + countdown -= 3; + } + + let mut countdown = 10; + if { let _tcov = countdown > 0; __llvm_incr_counter("start", ); _tcov } { + countdown -= 1; + __incr_cov("if block",()); + } else if countdown > 5 { + countdown -= 2; + __incr_cov("else if block",()); + } else { + countdown -= 3; + } +} + +// NOTE: hir REDUNDANTLY lowers the manually inlined counter in the second if block to: +// +// match { +// let _t = +// { +// let _tcov = countdown > 0; +// __llvm_incr_counter("start"); +// _tcov +// }; +// _t +// } { + +// I don't know if optimization phases will fix this or not. +// Otherwise, a more optimal (but definitely special case) way to handle this would be +// to inject the counter between the hir-introduced temp `_t` assignment and the block result +// line returning `_t`: +// +// match { +// let _t = countdown > 0; +// __llvm_incr_counter("start"); // <-- the only thing inserted for coverage here +// _t +// } +// +// UNFORTUNATELY THIS IS NOT A PATTERN WE CAN ALWAYS LEVERAGE, FOR EXPRESSIONS THAT HAVE VALUES +// WHERE WE NEED TO INJECT THE COUNTER AFTER THE EXPRESSION BUT BEFORE IT IS USED. +// +// IT DOES APPEAR TO BE THE CASE FOR WHILE EXPRESSIONS, (BECOMES loop { match { let _t = condition; _t} { true => {...} _ => break, }}) +// AND IS TRUE FOR IF EXPRESSIONS AS NOTED +// BUT NOT FOR RETURN STATEMENT (and I'm guessing not for loop { break value; } ? ) +// +// AND NOT FOR LAZY BOOLEAN EXPRESSIONS! +// +// AND NOT FOR MATCH EXPRESSIONS IN THE ORIGINAL SOURCE! \ No newline at end of file diff --git a/src/test/codegen/coverage-experiments/src/if_with_comments.rs b/src/test/codegen/coverage-experiments/src/if_with_comments.rs new file mode 100644 index 00000000000..267e7bca2c5 --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/if_with_comments.rs @@ -0,0 +1,39 @@ +/* */ #[inline(always)] +/* */ pub fn __incr_cov(_region_loc: &str, /*index: u32,*/ result: T) -> T { +/* */ result +/* */ } +/* */ +/* - */ fn main() { +/* ┃ */ let mut countdown = 10; +/* : I */ if __incr_cov("start", countdown > 0) { // span is from start of main() +/* : ┃ */ countdown -= 1; +/* : ┃ */ __incr_cov("if block",()); +/* ┃ - */ } + + let mut countdown = 10; + if __incr_cov("start", countdown > 0) { + countdown -= 1; + __incr_cov("if block",()); + } else if countdown > 5 { // counter expression "start" - "if block" + countdown -= 2; + __incr_cov("else if block",()); + } else { + countdown -= 3; + // __incr_cov("else block",()); // counter expression (countdown > 5 counter expression) - "else if block" + // PLACED AT END OF ELSE BLOCK OR START OF FIRST CONDITIONAL BLOCK, IF ANY (PRESUMING POSSIBLE EARLY EXIT). + // IF WE CAN GUARANTEE NO EARLY EXIT IN THIS BLOCK, THEN AT THE END IS FINE EVEN IF ELSE BLOCK CONTAINS OTHER CONDITIONS. + } + +/* - */ } + +// -Z unpretty=val -- present the input source, unstable (and less-pretty) variants; +// valid types are any of the types for `--pretty`, as well as: +// `expanded`, `expanded,identified`, +// `expanded,hygiene` (with internal representations), +// `everybody_loops` (all function bodies replaced with `loop {}`), +// `hir` (the HIR), `hir,identified`, +// `hir,typed` (HIR with types for each node), +// `hir-tree` (dump the raw HIR), +// `mir` (the MIR), or `mir-cfg` (graphviz formatted MIR) + +// argument to `pretty` must be one of `normal`, `expanded`, `identified`, or `expanded,identified` diff --git a/src/test/codegen/coverage-experiments/src/increment_intrinsic.rs b/src/test/codegen/coverage-experiments/src/increment_intrinsic.rs new file mode 100644 index 00000000000..d4708cd367f --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/increment_intrinsic.rs @@ -0,0 +1,11 @@ +#![feature(core_intrinsics)] + +pub fn not_instrprof_increment(_hash: u64, _num_counters: u32, _index: u32) { +} + +fn main() { + // COMPARE THIS WITH INTRINSIC INSERTION + //not_instrprof_increment(1234 as u64, 314 as u32, 31 as u32); + + unsafe { core::intrinsics::instrprof_increment(1234 as u64, 314 as u32, 31 as u32) }; +} \ No newline at end of file diff --git a/src/test/codegen/coverage-experiments/src/just_main.rs b/src/test/codegen/coverage-experiments/src/just_main.rs new file mode 100644 index 00000000000..081e5d72a6e --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/just_main.rs @@ -0,0 +1,3 @@ +fn main() { + println!("hello world! (should be covered)"); +} diff --git a/src/test/codegen/coverage-experiments/src/lazy_boolean.rs b/src/test/codegen/coverage-experiments/src/lazy_boolean.rs new file mode 100644 index 00000000000..263277c7cdc --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/lazy_boolean.rs @@ -0,0 +1,17 @@ +pub fn __llvm_incr_counter(_region_loc: &str) { +} + +#[inline(always)] +pub fn __incr_cov(region_loc: &str, result: T) -> T { + __llvm_incr_counter(region_loc); + result +} + +fn main() { + let a = 1; + let b = 10; + let c = 100; + let _result = __incr_cov("start", a < b) || __incr_cov("or", b < c); + + let _result = { let _t = a < b; __llvm_incr_counter("start"); _t } || { let _t = b < c; __llvm_incr_counter("start"); _t }; +} \ No newline at end of file diff --git a/src/test/codegen/coverage-experiments/src/loop_break_value.rs b/src/test/codegen/coverage-experiments/src/loop_break_value.rs new file mode 100644 index 00000000000..76caa833ec4 --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/loop_break_value.rs @@ -0,0 +1,15 @@ +pub fn __llvm_incr_counter(_region_loc: &str) { +} + +#[inline(always)] +pub fn __incr_cov(region_loc: &str, result: T) -> T { + __llvm_incr_counter(region_loc); + result +} + +fn main() { + __incr_cov("start", ()); + let _result = loop { + break __incr_cov("top of loop", true); + }; +} \ No newline at end of file diff --git a/src/test/codegen/coverage-experiments/src/match.rs b/src/test/codegen/coverage-experiments/src/match.rs new file mode 100644 index 00000000000..afbb20888ea --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/match.rs @@ -0,0 +1,22 @@ +pub fn __llvm_incr_counter(_region_loc: &str) { +} + +#[inline(always)] +pub fn __incr_cov(region_loc: &str, result: T) -> T { + __llvm_incr_counter(region_loc); + result +} + +fn main() { + let a = 1; + let b = 10; + let _result = match a < b { + true => true, + _ => false, + }; + + let _result = match __incr_cov("end of first match", a < b) { + true => __incr_cov("matched true", true), + _ => false, // counter expression "end of first match" - "matched true" + }; +} \ No newline at end of file diff --git a/src/test/codegen/coverage-experiments/src/match_with_increment.rs b/src/test/codegen/coverage-experiments/src/match_with_increment.rs new file mode 100644 index 00000000000..f618b37ed52 --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/match_with_increment.rs @@ -0,0 +1,305 @@ +#![feature(core_intrinsics)] +//static TEST_FUNC_NAME: &'static [u8; 7] = b"main()\0"; + static TEST_FUNC_NAME: &'static [u8; 6] = b"main()"; +fn main() { + let a = 1; + let b = 10; + let _result = match { + let _t = a < b; + unsafe { core::intrinsics::instrprof_increment(TEST_FUNC_NAME as *const u8, 1234 as u64, 3 as u32, 0 as u32) }; + _t + } { + true => { + let _t = true; + unsafe { core::intrinsics::instrprof_increment(TEST_FUNC_NAME as *const u8, 1234 as u64, 3 as u32, 1 as u32) }; + _t + } + _ => false, + }; +} + +/* + +I NEED TO INSERT THE instrprof_increment() CALL: + + 1. JUST BEFORE THE switchInt(_4) (because we haven't counted entering the function main() yet, deferring that to "JUST BEFORE FIRST BRANCH") + 2. SOME TIME AFTER THE switchInt(_4), AND JUST BEFORE ANOTHER BRANCH (in this case, before "goto") + 2.a. NOT BEFORE BOTH GOTO'S AFTER switchInt(_4) (because one can be calculated by counter expression), BUT PERHAPS INSERT A noop PLACEHOLDER + AS A MARKER TO INCLUDE THE COVERAGE REGION AND REFERENCE THE COUNTERS TO BE SUBTRACTED (AND/OR SUMMED)? + + WHY DEFER INSERTING COUNTERS TO "JUST BEFORE FIRST BRANCH"? We can ignore panic/unwind() and only count if the coverage region ACTUALLY + executed in entirety. BUT IS THAT NECESSARY? IS IT MUCH EASIER TO INSERT COUNTERS AT THE TOP OF A REGION THAT MUST EXECUTE IN ENTIRETY IF + PANIC DOES NOT OCCUR? AND WHAT IF WE ADD SUPPORT FOR PANIC UNWIND (later)? + + IS THERE A BENEFIT OF THE DEFERRED APPROACH WHEN CONSIDERING EXPRESSIONS MAY HAVE EARLY RETURNS? (BECAUSE, WE STILL NEED TO COUNT THE REGION + LEADING UP TO THE EXPRESSION ANYWAY) + +================================================= +================================================= + +To inject an intrinsic after computing a final expression value of a coverage region: + +Replace the following basic block end (last statement plus terminator): + +... ... +StorageLive(_4) +StorageLive(_5) +_5 = _1 +StorageLive(_6) +_6 = _2 +_4 = Lt(move _5, move _6) +StorageDead(_6) +StorageDead(_5) + <------ to insert instrprof_increment() here +FakeRead(ForMatchedPlace, _4) +-------------------------------------------------------------------------------------- +switchInt(_4) + + +================================================= +Insert call to intrinsic with: + +StorageLive(_4) # _4 is now meant for deferred FakeRead(ForMatchdPlace, _4) in BasicBlock after increment() call +StorageLive(_5) # Unchanged except _4 is now _5 +StorageLive(_6) # Unchanged except _5 is now _6 +_6 = _1 # Unchanged except _5 is now _6 +StorageLive(_7) # Unchanged except _6 is now _7 +_7 = _2 # Unchanged except _6 is now _7 +_5 = Lt(move _6, move _7) # Unchanged except _4, _5, _6 is now _5, _6, _7 +StorageDead(_7) # Unchanged except _6 is now _7 +StorageDead(_6) # Unchanged except _5 is now _6 + +FakeRead(ForLet, _5) # CHANGED ForMatchedPlace to ForLet + +> # ALL NEW AND NECESSARY TO CALL instrprof_increment() +> StorageLive(_8) # ?? stores function pointer to instrprof_increment function? +> StorageLive(_9) +> StorageLive(_10) +> StorageLive(_11) +> _11 = const {alloc1+0: &&[u8; 6]} +> _10 = &raw const (*(*_11)) +> _9 = move _10 as *const u8 (Pointer(ArrayToPointer)) +> StorageDead(_10) +> StorageLive(_12) +> _12 = const 1234u64 +> StorageLive(_13) +> _13 = const 3u32 +> StorageLive(_14) +> _14 = const 0u32 +> -------------------------------------------------------------------------------------- +> _8 = const std::intrinsics::instrprof_increment(move _9, move _12, move _13, move _14) +> +> -> return +> +> StorageDead(_14) +> StorageDead(_13) +> StorageDead(_12) +> StorageDead(_9) +> StorageDead(_11) +> StorageDead(_8) + +_4 = _5 # ARE THESE LINES REDUNDANT? CAN I JUST PASS _5 DIRECTLY TO FakeRead()? +StorageDead(_5) # DROP "_t" temp result of `let _t = a < b` + # (NOTE THAT IF SO, I CAN REMOVE _5 altogether, and use _4, which coincidentally makes less changes) + # SEE BELOW + +FakeRead(ForMatchedPlace, _4) # Unchanged +-------------------------------------------------------------------------------------- +switchInt(_4) # Unchanged + + +================================================= +Can I skip the extra variable and insert call to intrinsic with: + +StorageLive(_4) # Unchanged +StorageLive(_5) # Unchanged +_5 = _1 # Unchanged +StorageLive(_6) # Unchanged +_6 = _2 # Unchanged +_4 = Lt(move _5, move _6) # Unchanged +StorageDead(_6) # Unchanged +StorageDead(_5) # Unchanged + +> # ALL NEW AND NECESSARY TO CALL instrprof_increment() +> FakeRead(ForLet, _4) # Save the post-increment result in temp "_t" +> StorageLive(_8) # ?? stores function pointer to instrprof_increment function? +> StorageLive(_9) +> StorageLive(_10) +> StorageLive(_11) +> _11 = const {alloc1+0: &&[u8; 6]} +> _10 = &raw const (*(*_11)) +> _9 = move _10 as *const u8 (Pointer(ArrayToPointer)) +> StorageDead(_10) +> StorageLive(_12) +> _12 = const 1234u64 +> StorageLive(_13) +> _13 = const 3u32 +> StorageLive(_14) +> _14 = const 0u32 +> -------------------------------------------------------------------------------------- +> _8 = const std::intrinsics::instrprof_increment(move _9, move _12, move _13, move _14) +> +> -> return +> +> StorageDead(_14) +> StorageDead(_13) +> StorageDead(_12) +> StorageDead(_9) +> StorageDead(_11) +> StorageDead(_8) + +FakeRead(ForMatchedPlace, _4) # Unchanged (PREVIOUSLY USED IN FakeRead(ForLet), is that OK?) +-------------------------------------------------------------------------------------- +switchInt(_4) # Unchanged + + + + + +================================================= +================================================= + +For the second inserted call to instrprof_increment, without that call we have: + +-------------------------------------------------------------------------------------- +switchInt(_4) # From above + +-> otherwise # that is, "NOT false" + +_3 = const true + <------ to insert instrprof_increment() here +-------------------------------------------------------------------------------------- +goto + +-> # No label. No condition, and not a "return" + +FakeRead(ForLet, _3) # NOTE: Unused result +StorageDead(_4) +_0 = () +StorageDead(_3) +StorageDead(_2) +StorageDead(_1) +-------------------------------------------------------------------------------------- +goto + +-> # No label. No condition, and not a "return" + +return # from main() + + +================================================= +With the call to increment(): + +-------------------------------------------------------------------------------------- +switchInt(_4) # From above + +-> otherwise # "NOT false" # UNCHANGED + +StorageLive(_15) # CHANGED! Allocated new storage (_15) for the result of match, if true. +_15 = const true # UNCHANGED except _3 is now _15 +FakeRead(ForLet, _15) # CHANGED! Assign value to temporary (to be assigned to _3 later) ... Do I need to do this? + +> # ALL NEW AND NECESSARY TO CALL instrprof_increment() +> StorageLive(_16) # pointer to instrprof_increment() function ? +> StorageLive(_17) +> StorageLive(_18) +> StorageLive(_19) +> _19 = const {alloc1+0: &&[u8; 6]} +> _18 = &raw const (*(*_19)) +> _17 = move _18 as *const u8 (Pointer(ArrayToPointer)) +> StorageDead(_18) +> StorageLive(_20) +> _20 = const 1234u64 +> StorageLive(_21) +> _21 = const 3u32 +> StorageLive(_22) +> _22 = const 1u32 +> -------------------------------------------------------------------------------------- +> _16 = const std::intrinsics::instrprof_increment(move _17, move _20, move _21, move _22) +> +> -> return +> +> StorageDead(_22) +> StorageDead(_21) +> StorageDead(_20) +> StorageDead(_17) +> StorageDead(_19) +> StorageDead(_16) +> _3 = _15 +> StorageDead(_15) + +--------------------------------# UNCHANGED------------------------------------------- +goto # UNCHANGED + +-> # UNCHANGED + +FakeRead(ForLet, _3) # UNCHANGED +StorageDead(_4) # UNCHANGED +_0 = () # UNCHANGED +StorageDead(_3) # UNCHANGED +StorageDead(_2) # UNCHANGED +StorageDead(_1) # UNCHANGED +-------------------------------------------------------------------------------------- +goto # UNCHANGED + +-> # UNCHANGED + +return # from main() # UNCHANGED + +================================================= +As before, can I skip the extra variable (_15) and insert the call to intrinsic with _3 directly?: + + +-------------------------------------------------------------------------------------- +switchInt(_4) # From above + +-> otherwise # "NOT false" # UNCHANGED + +_3 = const true # UNCHANGED? + +> # ALL NEW AND NECESSARY TO CALL instrprof_increment() +> StorageLive(_16) # pointer to instrprof_increment() function ? +> StorageLive(_17) +> StorageLive(_18) +> StorageLive(_19) +> _19 = const {alloc1+0: &&[u8; 6]} +> _18 = &raw const (*(*_19)) +> _17 = move _18 as *const u8 (Pointer(ArrayToPointer)) +> StorageDead(_18) +> StorageLive(_20) +> _20 = const 1234u64 +> StorageLive(_21) +> _21 = const 3u32 +> StorageLive(_22) +> _22 = const 1u32 +> -------------------------------------------------------------------------------------- +> _16 = const std::intrinsics::instrprof_increment(move _17, move _20, move _21, move _22) +> +> -> return +> +> StorageDead(_22) +> StorageDead(_21) +> StorageDead(_20) +> StorageDead(_17) +> StorageDead(_19) +> StorageDead(_16) + +--------------------------------# UNCHANGED------------------------------------------- +goto # UNCHANGED + +-> # UNCHANGED + +FakeRead(ForLet, _3) # UNCHANGED +StorageDead(_4) # UNCHANGED +_0 = () # UNCHANGED +StorageDead(_3) # UNCHANGED +StorageDead(_2) # UNCHANGED +StorageDead(_1) # UNCHANGED +-------------------------------------------------------------------------------------- +goto # UNCHANGED + +-> # UNCHANGED + +return # from main() # UNCHANGED + +*/ \ No newline at end of file diff --git a/src/test/codegen/coverage-experiments/src/match_with_increment_alt.rs b/src/test/codegen/coverage-experiments/src/match_with_increment_alt.rs new file mode 100644 index 00000000000..60586967920 --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/match_with_increment_alt.rs @@ -0,0 +1,296 @@ +#![feature(core_intrinsics)] +//static TEST_FUNC_NAME: &'static [u8; 7] = b"main()\0"; + static TEST_FUNC_NAME: &'static [u8; 6] = b"main()"; +fn main() { + unsafe { core::intrinsics::instrprof_increment(TEST_FUNC_NAME as *const u8, 1234 as u64, 3 as u32, 0 as u32) }; + let a = 1; + let b = 10; + let _result = match a < b { + true => { + unsafe { core::intrinsics::instrprof_increment(TEST_FUNC_NAME as *const u8, 1234 as u64, 3 as u32, 1 as u32) }; + true + } + _ => false, + }; +} + +/* + +ALTERNATE APPROACH: + + IS IT MUCH EASIER TO INSERT COUNTERS AT THE TOP OF A REGION THAT MUST EXECUTE IN ENTIRETY IF + PANIC DOES NOT OCCUR? AND WHAT IF WE ADD SUPPORT FOR PANIC UNWIND (later)? + + IS THERE A DETRACTOR COMPARED TO THE DEFERRED APPROACH WHEN CONSIDERING EXPRESSIONS MAY HAVE EARLY RETURNS? + + (BECAUSE, WE STILL NEED TO COUNT THE REGION LEADING UP TO THE EXPRESSION ANYWAY) + +================================================= +================================================= + +To inject an intrinsic after computing a final expression value of a coverage region: + +Replace the following basic block end (last statement plus terminator): + +... ... +StorageLive(_4) +StorageLive(_5) +_5 = _1 +StorageLive(_6) +_6 = _2 +_4 = Lt(move _5, move _6) +StorageDead(_6) +StorageDead(_5) + <------ to insert instrprof_increment() here +FakeRead(ForMatchedPlace, _4) +-------------------------------------------------------------------------------------- +switchInt(_4) + + +================================================= +Insert call to intrinsic with: + +StorageLive(_4) # _4 is now meant for deferred FakeRead(ForMatchdPlace, _4) in BasicBlock after increment() call +StorageLive(_5) # Unchanged except _4 is now _5 +StorageLive(_6) # Unchanged except _5 is now _6 +_6 = _1 # Unchanged except _5 is now _6 +StorageLive(_7) # Unchanged except _6 is now _7 +_7 = _2 # Unchanged except _6 is now _7 +_5 = Lt(move _6, move _7) # Unchanged except _4, _5, _6 is now _5, _6, _7 +StorageDead(_7) # Unchanged except _6 is now _7 +StorageDead(_6) # Unchanged except _5 is now _6 + +FakeRead(ForLet, _5) # CHANGED ForMatchedPlace to ForLet + +> # ALL NEW AND NECESSARY TO CALL instrprof_increment() +> StorageLive(_8) # ?? stores function pointer to instrprof_increment function? +> StorageLive(_9) +> StorageLive(_10) +> StorageLive(_11) +> _11 = const {alloc1+0: &&[u8; 6]} +> _10 = &raw const (*(*_11)) +> _9 = move _10 as *const u8 (Pointer(ArrayToPointer)) +> StorageDead(_10) +> StorageLive(_12) +> _12 = const 1234u64 +> StorageLive(_13) +> _13 = const 3u32 +> StorageLive(_14) +> _14 = const 0u32 +> -------------------------------------------------------------------------------------- +> _8 = const std::intrinsics::instrprof_increment(move _9, move _12, move _13, move _14) +> +> -> return +> +> StorageDead(_14) +> StorageDead(_13) +> StorageDead(_12) +> StorageDead(_9) +> StorageDead(_11) +> StorageDead(_8) + +_4 = _5 # ARE THESE LINES REDUNDANT? CAN I JUST PASS _5 DIRECTLY TO FakeRead()? +StorageDead(_5) # DROP "_t" temp result of `let _t = a < b` + # (NOTE THAT IF SO, I CAN REMOVE _5 altogether, and use _4, which coincidentally makes less changes) + # SEE BELOW + +FakeRead(ForMatchedPlace, _4) # Unchanged +-------------------------------------------------------------------------------------- +switchInt(_4) # Unchanged + + +================================================= +Can I skip the extra variable and insert call to intrinsic with: + +StorageLive(_4) # Unchanged +StorageLive(_5) # Unchanged +_5 = _1 # Unchanged +StorageLive(_6) # Unchanged +_6 = _2 # Unchanged +_4 = Lt(move _5, move _6) # Unchanged +StorageDead(_6) # Unchanged +StorageDead(_5) # Unchanged + +> # ALL NEW AND NECESSARY TO CALL instrprof_increment() +> FakeRead(ForLet, _4) # Save the post-increment result in temp "_t" +> StorageLive(_8) # ?? stores function pointer to instrprof_increment function? +> StorageLive(_9) +> StorageLive(_10) +> StorageLive(_11) +> _11 = const {alloc1+0: &&[u8; 6]} +> _10 = &raw const (*(*_11)) +> _9 = move _10 as *const u8 (Pointer(ArrayToPointer)) +> StorageDead(_10) +> StorageLive(_12) +> _12 = const 1234u64 +> StorageLive(_13) +> _13 = const 3u32 +> StorageLive(_14) +> _14 = const 0u32 +> -------------------------------------------------------------------------------------- +> _8 = const std::intrinsics::instrprof_increment(move _9, move _12, move _13, move _14) +> +> -> return +> +> StorageDead(_14) +> StorageDead(_13) +> StorageDead(_12) +> StorageDead(_9) +> StorageDead(_11) +> StorageDead(_8) + +FakeRead(ForMatchedPlace, _4) # Unchanged (PREVIOUSLY USED IN FakeRead(ForLet), is that OK?) +-------------------------------------------------------------------------------------- +switchInt(_4) # Unchanged + + + + + +================================================= +================================================= + +For the second inserted call to instrprof_increment, without that call we have: + +-------------------------------------------------------------------------------------- +switchInt(_4) # From above + +-> otherwise # that is, "NOT false" + +_3 = const true + <------ to insert instrprof_increment() here +-------------------------------------------------------------------------------------- +goto + +-> # No label. No condition, and not a "return" + +FakeRead(ForLet, _3) # NOTE: Unused result +StorageDead(_4) +_0 = () +StorageDead(_3) +StorageDead(_2) +StorageDead(_1) +-------------------------------------------------------------------------------------- +goto + +-> # No label. No condition, and not a "return" + +return # from main() + + +================================================= +With the call to increment(): + +-------------------------------------------------------------------------------------- +switchInt(_4) # From above + +-> otherwise # "NOT false" # UNCHANGED + +StorageLive(_15) # CHANGED! Allocated new storage (_15) for the result of match, if true. +_15 = const true # UNCHANGED except _3 is now _15 +FakeRead(ForLet, _15) # CHANGED! Assign value to temporary (to be assigned to _3 later) ... Do I need to do this? + +> # ALL NEW AND NECESSARY TO CALL instrprof_increment() +> StorageLive(_16) # pointer to instrprof_increment() function ? +> StorageLive(_17) +> StorageLive(_18) +> StorageLive(_19) +> _19 = const {alloc1+0: &&[u8; 6]} +> _18 = &raw const (*(*_19)) +> _17 = move _18 as *const u8 (Pointer(ArrayToPointer)) +> StorageDead(_18) +> StorageLive(_20) +> _20 = const 1234u64 +> StorageLive(_21) +> _21 = const 3u32 +> StorageLive(_22) +> _22 = const 1u32 +> -------------------------------------------------------------------------------------- +> _16 = const std::intrinsics::instrprof_increment(move _17, move _20, move _21, move _22) +> +> -> return +> +> StorageDead(_22) +> StorageDead(_21) +> StorageDead(_20) +> StorageDead(_17) +> StorageDead(_19) +> StorageDead(_16) +> _3 = _15 +> StorageDead(_15) + +--------------------------------# UNCHANGED------------------------------------------- +goto # UNCHANGED + +-> # UNCHANGED + +FakeRead(ForLet, _3) # UNCHANGED +StorageDead(_4) # UNCHANGED +_0 = () # UNCHANGED +StorageDead(_3) # UNCHANGED +StorageDead(_2) # UNCHANGED +StorageDead(_1) # UNCHANGED +-------------------------------------------------------------------------------------- +goto # UNCHANGED + +-> # UNCHANGED + +return # from main() # UNCHANGED + +================================================= +As before, can I skip the extra variable (_15) and insert the call to intrinsic with _3 directly?: + + +-------------------------------------------------------------------------------------- +switchInt(_4) # From above + +-> otherwise # "NOT false" # UNCHANGED + +_3 = const true # UNCHANGED? + +> # ALL NEW AND NECESSARY TO CALL instrprof_increment() +> StorageLive(_16) # pointer to instrprof_increment() function ? +> StorageLive(_17) +> StorageLive(_18) +> StorageLive(_19) +> _19 = const {alloc1+0: &&[u8; 6]} +> _18 = &raw const (*(*_19)) +> _17 = move _18 as *const u8 (Pointer(ArrayToPointer)) +> StorageDead(_18) +> StorageLive(_20) +> _20 = const 1234u64 +> StorageLive(_21) +> _21 = const 3u32 +> StorageLive(_22) +> _22 = const 1u32 +> -------------------------------------------------------------------------------------- +> _16 = const std::intrinsics::instrprof_increment(move _17, move _20, move _21, move _22) +> +> -> return +> +> StorageDead(_22) +> StorageDead(_21) +> StorageDead(_20) +> StorageDead(_17) +> StorageDead(_19) +> StorageDead(_16) + +--------------------------------# UNCHANGED------------------------------------------- +goto # UNCHANGED + +-> # UNCHANGED + +FakeRead(ForLet, _3) # UNCHANGED +StorageDead(_4) # UNCHANGED +_0 = () # UNCHANGED +StorageDead(_3) # UNCHANGED +StorageDead(_2) # UNCHANGED +StorageDead(_1) # UNCHANGED +-------------------------------------------------------------------------------------- +goto # UNCHANGED + +-> # UNCHANGED + +return # from main() # UNCHANGED + +*/ \ No newline at end of file diff --git a/src/test/codegen/coverage-experiments/src/match_without_increment.mir b/src/test/codegen/coverage-experiments/src/match_without_increment.mir new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/test/codegen/coverage-experiments/src/match_without_increment.rs b/src/test/codegen/coverage-experiments/src/match_without_increment.rs new file mode 100644 index 00000000000..fa85833e054 --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/match_without_increment.rs @@ -0,0 +1,5 @@ +fn main() { + let a = 1; + let b = 10; + let _result = match a < b { true => true, _ => false, }; +} \ No newline at end of file diff --git a/src/test/codegen/coverage-experiments/src/match_without_increment_alt.mir b/src/test/codegen/coverage-experiments/src/match_without_increment_alt.mir new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/test/codegen/coverage-experiments/src/question_mark_err_status_handling_with_comments.rs b/src/test/codegen/coverage-experiments/src/question_mark_err_status_handling_with_comments.rs new file mode 100644 index 00000000000..03d11b2c230 --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/question_mark_err_status_handling_with_comments.rs @@ -0,0 +1,24 @@ +/* */ #[inline(always)] +/* */ pub fn __incr_cov(_region_loc: &str, /*index: u32,*/ result: T) -> T { +/* */ result +/* */ } +/* */ +/* - */ fn main() { +/* : I */ for countdown in __incr_cov("start", 10..0) { // span is just the while test expression +/* : ┃ */ let _ = countdown; +/* : ┃ */ __incr_cov("top of for", ()); +/* ┃ - */ } +/* - */ } + + +// -Z unpretty=val -- present the input source, unstable (and less-pretty) variants; +// valid types are any of the types for `--pretty`, as well as: +// `expanded`, `expanded,identified`, +// `expanded,hygiene` (with internal representations), +// `everybody_loops` (all function bodies replaced with `loop {}`), +// `hir` (the HIR), `hir,identified`, +// `hir,typed` (HIR with types for each node), +// `hir-tree` (dump the raw HIR), +// `mir` (the MIR), or `mir-cfg` (graphviz formatted MIR) + +// argument to `pretty` must be one of `normal`, `expanded`, `identified`, or `expanded,identified` diff --git a/src/test/codegen/coverage-experiments/src/while.rs b/src/test/codegen/coverage-experiments/src/while.rs new file mode 100644 index 00000000000..3cb185eda54 --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/while.rs @@ -0,0 +1,23 @@ +#[inline(always)] +pub fn __incr_cov(_region_loc: &str, result: T) -> T { + result +} + +fn main() { + let mut countdown = 10; + __incr_cov("block start",()); + while __incr_cov("while test", countdown > 0) { + countdown -= 1; + } + + let mut countdown = 10; + __incr_cov("after first while loop",()); + while __incr_cov("while test", countdown > 0) { + countdown -= 1; + if countdown < 5 { + __incr_cov("top of if countdown < 5",()); + break; + } + countdown -= 2; + } +} \ No newline at end of file diff --git a/src/test/codegen/coverage-experiments/src/while_clean.rs b/src/test/codegen/coverage-experiments/src/while_clean.rs new file mode 100644 index 00000000000..e9ed1efc220 --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/while_clean.rs @@ -0,0 +1,6 @@ +fn main() { + let mut countdown = 10; + while countdown > 0 { + countdown -= 1; + } +} \ No newline at end of file diff --git a/src/test/codegen/coverage-experiments/src/while_early_return.rs b/src/test/codegen/coverage-experiments/src/while_early_return.rs new file mode 100644 index 00000000000..35709ffba3a --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/while_early_return.rs @@ -0,0 +1,10 @@ +fn main() -> u8 { // this will lower to HIR but will not compile: `main` can only return types that implement `std::process::Termination` + let mut countdown = 10; + while countdown > 0 { + if false { + return if countdown > 8 { 1 } else { return 2; }; + } + countdown -= 1; + } + 0 +} \ No newline at end of file diff --git a/src/test/codegen/coverage-experiments/src/while_with_comments.rs b/src/test/codegen/coverage-experiments/src/while_with_comments.rs new file mode 100644 index 00000000000..56417fedf00 --- /dev/null +++ b/src/test/codegen/coverage-experiments/src/while_with_comments.rs @@ -0,0 +1,51 @@ +/* */ #[inline(always)] +/* */ pub fn __incr_cov(_region_loc: &str, /*index: u32,*/ result: T) -> T { +/* */ result +/* */ } +/* */ +/* - */ fn main() { +/* ┃ */ let mut countdown = 10; +/* ┃ */ __incr_cov("block start",()); // Must increment before repeated while text expression +/* : I */ while __incr_cov("while test", countdown > 0) { // span is just the while test expression +/* : ┃ */ countdown -= 1; +/* : ┃ */ // __incr_cov("while loop",()); // Counter not needed, but span is computed as "while test" minus "block start" +/* : ┃ */ // If while criteria is tested 11 times, and the outer block runs only once, 11-1 = 10 +/* : ┃ */ // REMOVING COUNTER ASSUMES NO EARLY RETURN THOUGH. +/* : ┃ */ // I THINK WE CAN ONLY USE THE COUNTER EXPRESSION UP TO FIRST CONDITIONAL BLOCK, IF ANY (if, match, maybe any loop) +/* ┃ - */ } + + let mut countdown = 10; + __incr_cov("after first while loop",()); + while __incr_cov("while test", countdown > 0) { + countdown -= 1; + // if __incr_cov("top of while loop", countdown < 5) { + if countdown < 5 { // "top of while loop" = counter expression "while test" - "after first while loop" + __incr_cov("top of if countdown < 5",()); + break; + } + countdown -= 2; + // __incr_cov("after if countdown < 5 block", ()); + // "after if countdown < 5 block" = counter expression "top of while loop" - "top of if countdown < 5" + // HOWEVER, WE CAN ONLY REMOVE THE COUNTER AND USE COUNTER EXPRESSION IF WE **KNOW** THAT THE BODY OF THE IF + // WILL **ALWAYS** BREAK (OR RETURN, OR CONTINUE?) + // AND THUS WE TREAT THE STATEMENTS FOLLOWING THE IF BLOCK AS IF THEY WERE AN ELSE BLOCK. + // THAT'S A LOT TO ASK. + + // PERHAPS TREAT EARLY RETURNS AS A SPECIAL KIND OF COUNTER AND IF ANY ARE INVOKED BEFORE STATEMENTS AFTER THE BLOCK THAT CONTAINS THEM, + // THEN SUBTRACT THOSE COUNTS FROM THE COUNT BEFORE THE BLOCK (AS WE DO HERE)? (SO ONE SET OF EXPRESSIONS MUST SUM ALL OF THE EARLY + // RETURNS) + } +/* - */ } + + +// -Z unpretty=val -- present the input source, unstable (and less-pretty) variants; +// valid types are any of the types for `--pretty`, as well as: +// `expanded`, `expanded,identified`, +// `expanded,hygiene` (with internal representations), +// `everybody_loops` (all function bodies replaced with `loop {}`), +// `hir` (the HIR), `hir,identified`, +// `hir,typed` (HIR with types for each node), +// `hir-tree` (dump the raw HIR), +// `mir` (the MIR), or `mir-cfg` (graphviz formatted MIR) + +// argument to `pretty` must be one of `normal`, `expanded`, `identified`, or `expanded,identified` -- cgit 1.4.1-3-g733a5 From 24a728a8eb4832568509eb757c2374934a76cb98 Mon Sep 17 00:00:00 2001 From: MaulingMonkey Date: Wed, 24 Jun 2020 23:28:00 -0700 Subject: debuginfo: Define int/float types in terms of MSVC-recognized types. PDB debug information doesn't appear to be emitted for basic types. By defining u32 as a typedef for unsigned __int32 when targeting MSVC, we allow CDB and other debuggers to recognize "u32" as a type/expression. This in turn unblocks rust-lang#70052 "Update hashbrown to 0.8.0" by allowing $T1 ..= $T3 to resolve, which would otherwise fail to resolve when builtin types fail to parse. --- src/librustc_codegen_llvm/debuginfo/metadata.rs | 72 ++++++++++++++++++++++++- src/librustc_codegen_llvm/llvm/ffi.rs | 10 ++++ src/rustllvm/RustWrapper.cpp | 8 +++ 3 files changed, 89 insertions(+), 1 deletion(-) (limited to 'src/rustllvm/RustWrapper.cpp') diff --git a/src/librustc_codegen_llvm/debuginfo/metadata.rs b/src/librustc_codegen_llvm/debuginfo/metadata.rs index 8a957a729fb..ac8f28e1609 100644 --- a/src/librustc_codegen_llvm/debuginfo/metadata.rs +++ b/src/librustc_codegen_llvm/debuginfo/metadata.rs @@ -19,6 +19,7 @@ use crate::llvm::debuginfo::{ use crate::value::Value; use log::debug; +use rustc_ast::ast; use rustc_codegen_ssa::traits::*; use rustc_data_structures::const_cstr; use rustc_data_structures::fingerprint::Fingerprint; @@ -824,14 +825,60 @@ fn file_metadata_raw( } } +trait MsvcBasicName { + fn msvc_basic_name(self) -> &'static str; +} + +impl MsvcBasicName for ast::IntTy { + fn msvc_basic_name(self) -> &'static str { + match self { + ast::IntTy::Isize => "ptrdiff_t", + ast::IntTy::I8 => "__int8", + ast::IntTy::I16 => "__int16", + ast::IntTy::I32 => "__int32", + ast::IntTy::I64 => "__int64", + ast::IntTy::I128 => "__int128", + } + } +} + +impl MsvcBasicName for ast::UintTy { + fn msvc_basic_name(self) -> &'static str { + match self { + ast::UintTy::Usize => "size_t", + ast::UintTy::U8 => "unsigned __int8", + ast::UintTy::U16 => "unsigned __int16", + ast::UintTy::U32 => "unsigned __int32", + ast::UintTy::U64 => "unsigned __int64", + ast::UintTy::U128 => "unsigned __int128", + } + } +} + +impl MsvcBasicName for ast::FloatTy { + fn msvc_basic_name(self) -> &'static str { + match self { + ast::FloatTy::F32 => "float", + ast::FloatTy::F64 => "double", + } + } +} + fn basic_type_metadata(cx: &CodegenCx<'ll, 'tcx>, t: Ty<'tcx>) -> &'ll DIType { debug!("basic_type_metadata: {:?}", t); + // When targeting MSVC, emit MSVC style type names for compatibility with + // .natvis visualizers (and perhaps other existing native debuggers?) + let msvc_like_names = cx.tcx.sess.target.target.options.is_like_msvc; + let (name, encoding) = match t.kind { ty::Never => ("!", DW_ATE_unsigned), ty::Tuple(ref elements) if elements.is_empty() => ("()", DW_ATE_unsigned), ty::Bool => ("bool", DW_ATE_boolean), ty::Char => ("char", DW_ATE_unsigned_char), + ty::Int(int_ty) if msvc_like_names => (int_ty.msvc_basic_name(), DW_ATE_signed), + ty::Uint(uint_ty) if msvc_like_names => (uint_ty.msvc_basic_name(), DW_ATE_unsigned), + ty::Float(float_ty) if msvc_like_names => (float_ty.msvc_basic_name(), DW_ATE_float), ty::Int(int_ty) => (int_ty.name_str(), DW_ATE_signed), ty::Uint(uint_ty) => (uint_ty.name_str(), DW_ATE_unsigned), ty::Float(float_ty) => (float_ty.name_str(), DW_ATE_float), @@ -848,7 +895,30 @@ fn basic_type_metadata(cx: &CodegenCx<'ll, 'tcx>, t: Ty<'tcx>) -> &'ll DIType { ) }; - ty_metadata + if !msvc_like_names { + return ty_metadata; + } + + let typedef_name = match t.kind { + ty::Int(int_ty) => int_ty.name_str(), + ty::Uint(uint_ty) => uint_ty.name_str(), + ty::Float(float_ty) => float_ty.name_str(), + _ => return ty_metadata, + }; + + let typedef_metadata = unsafe { + llvm::LLVMRustDIBuilderCreateTypedef( + DIB(cx), + ty_metadata, + typedef_name.as_ptr().cast(), + typedef_name.len(), + unknown_file_metadata(cx), + 0, + None, + ) + }; + + typedef_metadata } fn foreign_type_metadata( diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs index 8063d97aa73..61b1cae698d 100644 --- a/src/librustc_codegen_llvm/llvm/ffi.rs +++ b/src/librustc_codegen_llvm/llvm/ffi.rs @@ -1699,6 +1699,16 @@ extern "C" { Encoding: c_uint, ) -> &'a DIBasicType; + pub fn LLVMRustDIBuilderCreateTypedef( + Builder: &DIBuilder<'a>, + Type: &'a DIBasicType, + Name: *const c_char, + NameLen: size_t, + File: &'a DIFile, + LineNo: c_uint, + Scope: Option<&'a DIScope>, + ) -> &'a DIDerivedType; + pub fn LLVMRustDIBuilderCreatePointerType( Builder: &DIBuilder<'a>, PointeeTy: &'a DIType, diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index cdb3a157eab..d78fb63fd4e 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -759,6 +759,14 @@ extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateBasicType( return wrap(Builder->createBasicType(StringRef(Name, NameLen), SizeInBits, Encoding)); } +extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateTypedef( + LLVMRustDIBuilderRef Builder, LLVMMetadataRef Type, const char *Name, size_t NameLen, + LLVMMetadataRef File, unsigned LineNo, LLVMMetadataRef Scope) { + return wrap(Builder->createTypedef( + unwrap(Type), StringRef(Name, NameLen), unwrap(File), + LineNo, unwrap(Scope))); +} + extern "C" LLVMMetadataRef LLVMRustDIBuilderCreatePointerType( LLVMRustDIBuilderRef Builder, LLVMMetadataRef PointeeTy, uint64_t SizeInBits, uint32_t AlignInBits, unsigned AddressSpace, -- cgit 1.4.1-3-g733a5 From 49f6166ef7825a39e980c0ba0904073379bb01e6 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Thu, 25 Jun 2020 18:52:41 -0700 Subject: Prepare for LLVM 11 --- src/libprofiler_builtins/build.rs | 16 +++-- src/librustc_codegen_llvm/back/lto.rs | 5 +- src/librustc_codegen_llvm/llvm/ffi.rs | 16 ++++- src/librustc_codegen_ssa/common.rs | 2 + src/rustllvm/PassWrapper.cpp | 119 ++++++++++++++++++++++++++-------- src/rustllvm/RustWrapper.cpp | 58 ++++++++++++----- src/test/codegen/sanitizer-recover.rs | 6 +- src/tools/compiletest/src/header.rs | 2 +- 8 files changed, 167 insertions(+), 57 deletions(-) (limited to 'src/rustllvm/RustWrapper.cpp') diff --git a/src/libprofiler_builtins/build.rs b/src/libprofiler_builtins/build.rs index e23e2f2c130..bb7d59e113c 100644 --- a/src/libprofiler_builtins/build.rs +++ b/src/libprofiler_builtins/build.rs @@ -24,6 +24,12 @@ fn main() { "InstrProfilingUtil.c", "InstrProfilingValue.c", "InstrProfilingWriter.c", + // This file was renamed in LLVM 10. + "InstrProfilingRuntime.cc", + "InstrProfilingRuntime.cpp", + // These files were added in LLVM 11. + "InstrProfilingInternal.c", + "InstrProfilingBiasVar.c", ]; if target.contains("msvc") { @@ -69,14 +75,12 @@ fn main() { let src_root = root.join("lib").join("profile"); for src in profile_sources { - cfg.file(src_root.join(src)); + let path = src_root.join(src); + if path.exists() { + cfg.file(path); + } } - // The file was renamed in LLVM 10. - let old_runtime_path = src_root.join("InstrProfilingRuntime.cc"); - let new_runtime_path = src_root.join("InstrProfilingRuntime.cpp"); - cfg.file(if old_runtime_path.exists() { old_runtime_path } else { new_runtime_path }); - cfg.include(root.join("include")); cfg.warnings(false); cfg.compile("profiler-rt"); diff --git a/src/librustc_codegen_llvm/back/lto.rs b/src/librustc_codegen_llvm/back/lto.rs index d3e3441b087..9764c9a102e 100644 --- a/src/librustc_codegen_llvm/back/lto.rs +++ b/src/librustc_codegen_llvm/back/lto.rs @@ -797,6 +797,7 @@ pub unsafe fn optimize_thin_module( kind: ModuleKind::Regular, }; { + let target = &*module.module_llvm.tm; let llmod = module.module_llvm.llmod(); save_temp_bitcode(&cgcx, &module, "thin-lto-input"); @@ -833,7 +834,7 @@ pub unsafe fn optimize_thin_module( { let _timer = cgcx.prof.generic_activity_with_arg("LLVM_thin_lto_rename", thin_module.name()); - if !llvm::LLVMRustPrepareThinLTORename(thin_module.shared.data.0, llmod) { + if !llvm::LLVMRustPrepareThinLTORename(thin_module.shared.data.0, llmod, target) { let msg = "failed to prepare thin LTO module"; return Err(write::llvm_err(&diag_handler, msg)); } @@ -865,7 +866,7 @@ pub unsafe fn optimize_thin_module( { let _timer = cgcx.prof.generic_activity_with_arg("LLVM_thin_lto_import", thin_module.name()); - if !llvm::LLVMRustPrepareThinLTOImport(thin_module.shared.data.0, llmod) { + if !llvm::LLVMRustPrepareThinLTOImport(thin_module.shared.data.0, llmod, target) { let msg = "failed to prepare thin LTO module"; return Err(write::llvm_err(&diag_handler, msg)); } diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs index 8063d97aa73..7beb4fc8974 100644 --- a/src/librustc_codegen_llvm/llvm/ffi.rs +++ b/src/librustc_codegen_llvm/llvm/ffi.rs @@ -233,6 +233,8 @@ pub enum TypeKind { Metadata = 14, X86_MMX = 15, Token = 16, + ScalableVector = 17, + BFloat = 18, } impl TypeKind { @@ -255,6 +257,8 @@ impl TypeKind { TypeKind::Metadata => rustc_codegen_ssa::common::TypeKind::Metadata, TypeKind::X86_MMX => rustc_codegen_ssa::common::TypeKind::X86_MMX, TypeKind::Token => rustc_codegen_ssa::common::TypeKind::Token, + TypeKind::ScalableVector => rustc_codegen_ssa::common::TypeKind::ScalableVector, + TypeKind::BFloat => rustc_codegen_ssa::common::TypeKind::BFloat, } } } @@ -2141,10 +2145,18 @@ extern "C" { PreservedSymbols: *const *const c_char, PreservedSymbolsLen: c_uint, ) -> Option<&'static mut ThinLTOData>; - pub fn LLVMRustPrepareThinLTORename(Data: &ThinLTOData, Module: &Module) -> bool; + pub fn LLVMRustPrepareThinLTORename( + Data: &ThinLTOData, + Module: &Module, + Target: &TargetMachine, + ) -> bool; pub fn LLVMRustPrepareThinLTOResolveWeak(Data: &ThinLTOData, Module: &Module) -> bool; pub fn LLVMRustPrepareThinLTOInternalize(Data: &ThinLTOData, Module: &Module) -> bool; - pub fn LLVMRustPrepareThinLTOImport(Data: &ThinLTOData, Module: &Module) -> bool; + pub fn LLVMRustPrepareThinLTOImport( + Data: &ThinLTOData, + Module: &Module, + Target: &TargetMachine, + ) -> bool; pub fn LLVMRustGetThinLTOModuleImports( Data: *const ThinLTOData, ModuleNameCallback: ThinLTOModuleNameCallback, diff --git a/src/librustc_codegen_ssa/common.rs b/src/librustc_codegen_ssa/common.rs index 0d0321ec4ae..432b2f3bdc3 100644 --- a/src/librustc_codegen_ssa/common.rs +++ b/src/librustc_codegen_ssa/common.rs @@ -98,6 +98,8 @@ pub enum TypeKind { Metadata, X86_MMX, Token, + ScalableVector, + BFloat, } // FIXME(mw): Anything that is produced via DepGraph::with_task() must implement diff --git a/src/rustllvm/PassWrapper.cpp b/src/rustllvm/PassWrapper.cpp index 9bc111c26ba..41b14714842 100644 --- a/src/rustllvm/PassWrapper.cpp +++ b/src/rustllvm/PassWrapper.cpp @@ -49,8 +49,10 @@ typedef struct LLVMOpaqueTargetMachine *LLVMTargetMachineRef; DEFINE_STDCXX_CONVERSION_FUNCTIONS(Pass, LLVMPassRef) DEFINE_STDCXX_CONVERSION_FUNCTIONS(TargetMachine, LLVMTargetMachineRef) +#if LLVM_VERSION_LT(11, 0) DEFINE_STDCXX_CONVERSION_FUNCTIONS(PassManagerBuilder, LLVMPassManagerBuilderRef) +#endif extern "C" void LLVMInitializePasses() { PassRegistry &Registry = *PassRegistry::getPassRegistry(); @@ -343,17 +345,17 @@ enum class LLVMRustPassBuilderOptLevel { static PassBuilder::OptimizationLevel fromRust(LLVMRustPassBuilderOptLevel Level) { switch (Level) { case LLVMRustPassBuilderOptLevel::O0: - return PassBuilder::O0; + return PassBuilder::OptimizationLevel::O0; case LLVMRustPassBuilderOptLevel::O1: - return PassBuilder::O1; + return PassBuilder::OptimizationLevel::O1; case LLVMRustPassBuilderOptLevel::O2: - return PassBuilder::O2; + return PassBuilder::OptimizationLevel::O2; case LLVMRustPassBuilderOptLevel::O3: - return PassBuilder::O3; + return PassBuilder::OptimizationLevel::O3; case LLVMRustPassBuilderOptLevel::Os: - return PassBuilder::Os; + return PassBuilder::OptimizationLevel::Os; case LLVMRustPassBuilderOptLevel::Oz: - return PassBuilder::Oz; + return PassBuilder::OptimizationLevel::Oz; default: report_fatal_error("Bad PassBuilderOptLevel."); } @@ -796,8 +798,13 @@ LLVMRustOptimizeWithNewPassManager( // We manually collect pipeline callbacks so we can apply them at O0, where the // PassBuilder does not create a pipeline. std::vector> PipelineStartEPCallbacks; +#if LLVM_VERSION_GE(11, 0) + std::vector> + OptimizerLastEPCallbacks; +#else std::vector> OptimizerLastEPCallbacks; +#endif if (VerifyIR) { PipelineStartEPCallbacks.push_back([VerifyIR](ModulePassManager &MPM) { @@ -811,6 +818,14 @@ LLVMRustOptimizeWithNewPassManager( SanitizerOptions->SanitizeMemoryTrackOrigins, SanitizerOptions->SanitizeMemoryRecover, /*CompileKernel=*/false); +#if LLVM_VERSION_GE(11, 0) + OptimizerLastEPCallbacks.push_back( + [Options](ModulePassManager &MPM, PassBuilder::OptimizationLevel Level) { + MPM.addPass(MemorySanitizerPass(Options)); + MPM.addPass(createModuleToFunctionPassAdaptor(MemorySanitizerPass(Options))); + } + ); +#else #if LLVM_VERSION_GE(10, 0) PipelineStartEPCallbacks.push_back([Options](ModulePassManager &MPM) { MPM.addPass(MemorySanitizerPass(Options)); @@ -821,9 +836,18 @@ LLVMRustOptimizeWithNewPassManager( FPM.addPass(MemorySanitizerPass(Options)); } ); +#endif } if (SanitizerOptions->SanitizeThread) { +#if LLVM_VERSION_GE(11, 0) + OptimizerLastEPCallbacks.push_back( + [](ModulePassManager &MPM, PassBuilder::OptimizationLevel Level) { + MPM.addPass(ThreadSanitizerPass()); + MPM.addPass(createModuleToFunctionPassAdaptor(ThreadSanitizerPass())); + } + ); +#else #if LLVM_VERSION_GE(10, 0) PipelineStartEPCallbacks.push_back([](ModulePassManager &MPM) { MPM.addPass(ThreadSanitizerPass()); @@ -834,9 +858,22 @@ LLVMRustOptimizeWithNewPassManager( FPM.addPass(ThreadSanitizerPass()); } ); +#endif } if (SanitizerOptions->SanitizeAddress) { +#if LLVM_VERSION_GE(11, 0) + OptimizerLastEPCallbacks.push_back( + [SanitizerOptions](ModulePassManager &MPM, PassBuilder::OptimizationLevel Level) { + MPM.addPass(RequireAnalysisPass()); + MPM.addPass(ModuleAddressSanitizerPass( + /*CompileKernel=*/false, SanitizerOptions->SanitizeAddressRecover)); + MPM.addPass(createModuleToFunctionPassAdaptor(AddressSanitizerPass( + /*CompileKernel=*/false, SanitizerOptions->SanitizeAddressRecover, + /*UseAfterScope=*/true))); + } + ); +#else PipelineStartEPCallbacks.push_back([&](ModulePassManager &MPM) { MPM.addPass(RequireAnalysisPass()); }); @@ -853,21 +890,27 @@ LLVMRustOptimizeWithNewPassManager( /*CompileKernel=*/false, SanitizerOptions->SanitizeAddressRecover)); } ); +#endif } } ModulePassManager MPM(DebugPassManager); if (!NoPrepopulatePasses) { - if (OptLevel == PassBuilder::O0) { + if (OptLevel == PassBuilder::OptimizationLevel::O0) { for (const auto &C : PipelineStartEPCallbacks) C(MPM); +#if LLVM_VERSION_GE(11, 0) + for (const auto &C : OptimizerLastEPCallbacks) + C(MPM, OptLevel); +#else if (!OptimizerLastEPCallbacks.empty()) { FunctionPassManager FPM(DebugPassManager); for (const auto &C : OptimizerLastEPCallbacks) C(FPM, OptLevel); MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM))); } +#endif MPM.addPass(AlwaysInlinerPass(EmitLifetimeMarkers)); @@ -892,12 +935,17 @@ LLVMRustOptimizeWithNewPassManager( break; case LLVMRustOptStage::PreLinkThinLTO: MPM = PB.buildThinLTOPreLinkDefaultPipeline(OptLevel, DebugPassManager); +#if LLVM_VERSION_GE(11, 0) + for (const auto &C : OptimizerLastEPCallbacks) + C(MPM, OptLevel); +#else if (!OptimizerLastEPCallbacks.empty()) { FunctionPassManager FPM(DebugPassManager); for (const auto &C : OptimizerLastEPCallbacks) C(FPM, OptLevel); MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM))); } +#endif break; case LLVMRustOptStage::PreLinkFatLTO: MPM = PB.buildLTOPreLinkDefaultPipeline(OptLevel, DebugPassManager); @@ -994,10 +1042,10 @@ public: const Value *Value; if (const CallInst *CI = dyn_cast(I)) { Name = "call"; - Value = CI->getCalledValue(); + Value = CI->getCalledOperand(); } else if (const InvokeInst* II = dyn_cast(I)) { Name = "invoke"; - Value = II->getCalledValue(); + Value = II->getCalledOperand(); } else { // Could demangle more operations, e. g. // `store %place, @function`. @@ -1335,10 +1383,33 @@ LLVMRustFreeThinLTOData(LLVMRustThinLTOData *Data) { // `ProcessThinLTOModule` function. Here they're split up into separate steps // so rustc can save off the intermediate bytecode between each step. +#if LLVM_VERSION_GE(11, 0) +static bool +clearDSOLocalOnDeclarations(Module &Mod, TargetMachine &TM) { + // When linking an ELF shared object, dso_local should be dropped. We + // conservatively do this for -fpic. + bool ClearDSOLocalOnDeclarations = + TM.getTargetTriple().isOSBinFormatELF() && + TM.getRelocationModel() != Reloc::Static && + Mod.getPIELevel() == PIELevel::Default; + return ClearDSOLocalOnDeclarations; +} +#endif + extern "C" bool -LLVMRustPrepareThinLTORename(const LLVMRustThinLTOData *Data, LLVMModuleRef M) { +LLVMRustPrepareThinLTORename(const LLVMRustThinLTOData *Data, LLVMModuleRef M, + LLVMTargetMachineRef TM) { Module &Mod = *unwrap(M); - if (renameModuleForThinLTO(Mod, Data->Index)) { + TargetMachine &Target = *unwrap(TM); + +#if LLVM_VERSION_GE(11, 0) + bool ClearDSOLocal = clearDSOLocalOnDeclarations(Mod, Target); + bool error = renameModuleForThinLTO(Mod, Data->Index, ClearDSOLocal); +#else + bool error = renameModuleForThinLTO(Mod, Data->Index); +#endif + + if (error) { LLVMRustSetLastError("renameModuleForThinLTO failed"); return false; } @@ -1362,8 +1433,10 @@ LLVMRustPrepareThinLTOInternalize(const LLVMRustThinLTOData *Data, LLVMModuleRef } extern "C" bool -LLVMRustPrepareThinLTOImport(const LLVMRustThinLTOData *Data, LLVMModuleRef M) { +LLVMRustPrepareThinLTOImport(const LLVMRustThinLTOData *Data, LLVMModuleRef M, + LLVMTargetMachineRef TM) { Module &Mod = *unwrap(M); + TargetMachine &Target = *unwrap(TM); const auto &ImportList = Data->ImportLists.lookup(Mod.getModuleIdentifier()); auto Loader = [&](StringRef Identifier) { @@ -1399,7 +1472,12 @@ LLVMRustPrepareThinLTOImport(const LLVMRustThinLTOData *Data, LLVMModuleRef M) { return MOrErr; }; +#if LLVM_VERSION_GE(11, 0) + bool ClearDSOLocal = clearDSOLocalOnDeclarations(Mod, Target); + FunctionImporter Importer(Data->Index, Loader, ClearDSOLocal); +#else FunctionImporter Importer(Data->Index, Loader); +#endif Expected Result = Importer.importFunctions(Mod, ImportList); if (!Result) { LLVMRustSetLastError(toString(Result.takeError()).c_str()); @@ -1558,22 +1636,11 @@ LLVMRustThinLTOPatchDICompileUnit(LLVMModuleRef Mod, DICompileUnit *Unit) { } // Use LLVM's built-in `DebugInfoFinder` to find a bunch of debuginfo and - // process it recursively. Note that we specifically iterate over instructions - // to ensure we feed everything into it. + // process it recursively. Note that we used to specifically iterate over + // instructions to ensure we feed everything into it, but `processModule` + // started doing this the same way in LLVM 7 (commit d769eb36ab2b8). DebugInfoFinder Finder; Finder.processModule(*M); - for (Function &F : M->functions()) { - for (auto &FI : F) { - for (Instruction &BI : FI) { - if (auto Loc = BI.getDebugLoc()) - Finder.processLocation(*M, Loc); - if (auto DVI = dyn_cast(&BI)) - Finder.processValue(*M, DVI); - if (auto DDI = dyn_cast(&BI)) - Finder.processDeclare(*M, DDI); - } - } - } // After we've found all our debuginfo, rewrite all subprograms to point to // the same `DICompileUnit`. diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index cdb3a157eab..063b6acc604 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -1,5 +1,4 @@ #include "rustllvm.h" -#include "llvm/IR/CallSite.h" #include "llvm/IR/DebugInfoMetadata.h" #include "llvm/IR/DiagnosticInfo.h" #include "llvm/IR/DiagnosticPrinter.h" @@ -214,50 +213,50 @@ static Attribute::AttrKind fromRust(LLVMRustAttribute Kind) { extern "C" void LLVMRustAddCallSiteAttribute(LLVMValueRef Instr, unsigned Index, LLVMRustAttribute RustAttr) { - CallSite Call = CallSite(unwrap(Instr)); + CallBase *Call = unwrap(Instr); Attribute Attr = Attribute::get(Call->getContext(), fromRust(RustAttr)); - Call.addAttribute(Index, Attr); + Call->addAttribute(Index, Attr); } extern "C" void LLVMRustAddAlignmentCallSiteAttr(LLVMValueRef Instr, unsigned Index, uint32_t Bytes) { - CallSite Call = CallSite(unwrap(Instr)); + CallBase *Call = unwrap(Instr); AttrBuilder B; B.addAlignmentAttr(Bytes); - Call.setAttributes(Call.getAttributes().addAttributes( + Call->setAttributes(Call->getAttributes().addAttributes( Call->getContext(), Index, B)); } extern "C" void LLVMRustAddDereferenceableCallSiteAttr(LLVMValueRef Instr, unsigned Index, uint64_t Bytes) { - CallSite Call = CallSite(unwrap(Instr)); + CallBase *Call = unwrap(Instr); AttrBuilder B; B.addDereferenceableAttr(Bytes); - Call.setAttributes(Call.getAttributes().addAttributes( + Call->setAttributes(Call->getAttributes().addAttributes( Call->getContext(), Index, B)); } extern "C" void LLVMRustAddDereferenceableOrNullCallSiteAttr(LLVMValueRef Instr, unsigned Index, uint64_t Bytes) { - CallSite Call = CallSite(unwrap(Instr)); + CallBase *Call = unwrap(Instr); AttrBuilder B; B.addDereferenceableOrNullAttr(Bytes); - Call.setAttributes(Call.getAttributes().addAttributes( + Call->setAttributes(Call->getAttributes().addAttributes( Call->getContext(), Index, B)); } extern "C" void LLVMRustAddByValCallSiteAttr(LLVMValueRef Instr, unsigned Index, LLVMTypeRef Ty) { - CallSite Call = CallSite(unwrap(Instr)); + CallBase *Call = unwrap(Instr); #if LLVM_VERSION_GE(9, 0) Attribute Attr = Attribute::getWithByValType(Call->getContext(), unwrap(Ty)); #else Attribute Attr = Attribute::get(Call->getContext(), Attribute::ByVal); #endif - Call.addAttribute(Index, Attr); + Call->addAttribute(Index, Attr); } extern "C" void LLVMRustAddFunctionAttribute(LLVMValueRef Fn, unsigned Index, @@ -336,20 +335,24 @@ extern "C" void LLVMRustSetHasUnsafeAlgebra(LLVMValueRef V) { extern "C" LLVMValueRef LLVMRustBuildAtomicLoad(LLVMBuilderRef B, LLVMValueRef Source, const char *Name, LLVMAtomicOrdering Order) { - LoadInst *LI = new LoadInst(unwrap(Source)); + Value *Ptr = unwrap(Source); + Type *Ty = Ptr->getType()->getPointerElementType(); + LoadInst *LI = unwrap(B)->CreateLoad(Ty, Ptr, Name); LI->setAtomic(fromRust(Order)); - return wrap(unwrap(B)->Insert(LI, Name)); + return wrap(LI); } extern "C" LLVMValueRef LLVMRustBuildAtomicStore(LLVMBuilderRef B, LLVMValueRef V, LLVMValueRef Target, LLVMAtomicOrdering Order) { - StoreInst *SI = new StoreInst(unwrap(V), unwrap(Target)); + StoreInst *SI = unwrap(B)->CreateStore(unwrap(V), unwrap(Target)); SI->setAtomic(fromRust(Order)); - return wrap(unwrap(B)->Insert(SI)); + return wrap(SI); } +// FIXME: Use the C-API LLVMBuildAtomicCmpXchg and LLVMSetWeak +// once we raise our minimum support to LLVM 10. extern "C" LLVMValueRef LLVMRustBuildAtomicCmpXchg(LLVMBuilderRef B, LLVMValueRef Target, LLVMValueRef Old, LLVMValueRef Source, @@ -965,8 +968,14 @@ extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateUnionType( extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateTemplateTypeParameter( LLVMRustDIBuilderRef Builder, LLVMMetadataRef Scope, const char *Name, size_t NameLen, LLVMMetadataRef Ty) { +#if LLVM_VERSION_GE(11, 0) + bool IsDefault = false; // FIXME: should we ever set this true? + return wrap(Builder->createTemplateTypeParameter( + unwrapDI(Scope), StringRef(Name, NameLen), unwrapDI(Ty), IsDefault)); +#else return wrap(Builder->createTemplateTypeParameter( unwrapDI(Scope), StringRef(Name, NameLen), unwrapDI(Ty))); +#endif } extern "C" LLVMMetadataRef LLVMRustDIBuilderCreateNameSpace( @@ -1227,12 +1236,23 @@ extern "C" LLVMTypeKind LLVMRustGetTypeKind(LLVMTypeRef Ty) { return LLVMArrayTypeKind; case Type::PointerTyID: return LLVMPointerTypeKind; +#if LLVM_VERSION_GE(11, 0) + case Type::FixedVectorTyID: + return LLVMVectorTypeKind; +#else case Type::VectorTyID: return LLVMVectorTypeKind; +#endif case Type::X86_MMXTyID: return LLVMX86_MMXTypeKind; case Type::TokenTyID: return LLVMTokenTypeKind; +#if LLVM_VERSION_GE(11, 0) + case Type::ScalableVectorTyID: + return LLVMScalableVectorTypeKind; + case Type::BFloatTyID: + return LLVMBFloatTypeKind; +#endif } report_fatal_error("Unhandled TypeID."); } @@ -1359,10 +1379,12 @@ extern "C" void LLVMRustFreeOperandBundleDef(OperandBundleDef *Bundle) { extern "C" LLVMValueRef LLVMRustBuildCall(LLVMBuilderRef B, LLVMValueRef Fn, LLVMValueRef *Args, unsigned NumArgs, OperandBundleDef *Bundle) { + Value *Callee = unwrap(Fn); + FunctionType *FTy = cast(Callee->getType()->getPointerElementType()); unsigned Len = Bundle ? 1 : 0; ArrayRef Bundles = makeArrayRef(Bundle, Len); return wrap(unwrap(B)->CreateCall( - unwrap(Fn), makeArrayRef(unwrap(Args), NumArgs), Bundles)); + FTy, Callee, makeArrayRef(unwrap(Args), NumArgs), Bundles)); } extern "C" LLVMValueRef LLVMRustGetInstrprofIncrementIntrinsic(LLVMModuleRef M) { @@ -1422,9 +1444,11 @@ LLVMRustBuildInvoke(LLVMBuilderRef B, LLVMValueRef Fn, LLVMValueRef *Args, unsigned NumArgs, LLVMBasicBlockRef Then, LLVMBasicBlockRef Catch, OperandBundleDef *Bundle, const char *Name) { + Value *Callee = unwrap(Fn); + FunctionType *FTy = cast(Callee->getType()->getPointerElementType()); unsigned Len = Bundle ? 1 : 0; ArrayRef Bundles = makeArrayRef(Bundle, Len); - return wrap(unwrap(B)->CreateInvoke(unwrap(Fn), unwrap(Then), unwrap(Catch), + return wrap(unwrap(B)->CreateInvoke(FTy, Callee, unwrap(Then), unwrap(Catch), makeArrayRef(unwrap(Args), NumArgs), Bundles, Name)); } diff --git a/src/test/codegen/sanitizer-recover.rs b/src/test/codegen/sanitizer-recover.rs index 719f219ce4e..433d32abd37 100644 --- a/src/test/codegen/sanitizer-recover.rs +++ b/src/test/codegen/sanitizer-recover.rs @@ -27,17 +27,17 @@ // ASAN: } // // MSAN-LABEL: define i32 @penguin( -// MSAN: call void @__msan_warning_noreturn() +// MSAN: call void @__msan_warning{{(_with_origin_noreturn\(i32 0\)|_noreturn\(\))}} // MSAN: unreachable // MSAN: } // // MSAN-RECOVER-LABEL: define i32 @penguin( -// MSAN-RECOVER: call void @__msan_warning() +// MSAN-RECOVER: call void @__msan_warning{{(_with_origin\(i32 0\)|\(\))}} // MSAN-RECOVER-NOT: unreachable // MSAN-RECOVER: } // // MSAN-RECOVER-LTO-LABEL: define i32 @penguin( -// MSAN-RECOVER-LTO: call void @__msan_warning() +// MSAN-RECOVER-LTO: call void @__msan_warning{{(_with_origin\(i32 0\)|\(\))}} // MSAN-RECOVER-LTO-NOT: unreachable // MSAN-RECOVER-LTO: } // diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 7d2c83881d1..571e7a59113 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -263,7 +263,7 @@ impl EarlyProps { } fn version_to_int(version: &str) -> u32 { - let version_without_suffix = version.split('-').next().unwrap(); + let version_without_suffix = version.trim_end_matches("git").split('-').next().unwrap(); let components: Vec = version_without_suffix .split('.') .map(|s| s.parse().expect("Malformed version component")) -- cgit 1.4.1-3-g733a5