From e1a54725081227a3866669c167bee4c0559f5362 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Fri, 21 Feb 2020 00:00:00 +0000 Subject: [PATCH 1/3] Emit 1-based column numbers in debuginfo The debuginfo column numbers are 1-based. The value 0 indicates that no column has been specified. Translate 0-based column numbers to 1-based when emitting debug information. --- .../debuginfo/create_scope_map.rs | 3 ++- .../debuginfo/source_loc.rs | 3 ++- src/test/codegen/debug-column.rs | 16 ++++++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 src/test/codegen/debug-column.rs diff --git a/src/librustc_codegen_llvm/debuginfo/create_scope_map.rs b/src/librustc_codegen_llvm/debuginfo/create_scope_map.rs index cdb9657e1ff3c..de029ce51b321 100644 --- a/src/librustc_codegen_llvm/debuginfo/create_scope_map.rs +++ b/src/librustc_codegen_llvm/debuginfo/create_scope_map.rs @@ -79,7 +79,8 @@ fn make_mir_scope( parent_scope.scope_metadata.unwrap(), file_metadata, loc.line as c_uint, - loc.col.to_usize() as c_uint, + // Loc column is 0-based while debug one is 1-based. + loc.col.to_usize() as c_uint + 1, )) }; debug_context.scopes[scope] = DebugScope { diff --git a/src/librustc_codegen_llvm/debuginfo/source_loc.rs b/src/librustc_codegen_llvm/debuginfo/source_loc.rs index 1f871c7d207a4..ae5d9a7c8d89d 100644 --- a/src/librustc_codegen_llvm/debuginfo/source_loc.rs +++ b/src/librustc_codegen_llvm/debuginfo/source_loc.rs @@ -19,7 +19,8 @@ impl CodegenCx<'ll, '_> { let col_used = if self.sess().target.target.options.is_like_msvc { UNKNOWN_COLUMN_NUMBER } else { - loc.col.to_usize() as c_uint + // Loc column is 0-based while debug one is 1-based. + loc.col.to_usize() as c_uint + 1 }; unsafe { diff --git a/src/test/codegen/debug-column.rs b/src/test/codegen/debug-column.rs new file mode 100644 index 0000000000000..4a7b4176f2cee --- /dev/null +++ b/src/test/codegen/debug-column.rs @@ -0,0 +1,16 @@ +// Verify that emitted debuginfo column nubmers are 1-based. Regression test for issue #65437. +// +// ignore-windows +// compile-flags: -C debuginfo=2 + +fn main() { + unsafe { + // CHECK: call void @giraffe(), !dbg [[DBG:!.*]] + // CHECK: [[DBG]] = !DILocation(line: 10, column: 9 + giraffe(); + } +} + +extern { + fn giraffe(); +} From 0c51f2f5a580a7e239accd0cc5f3da2e8cdf6f8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Tue, 25 Feb 2020 00:00:00 +0000 Subject: [PATCH 2/3] Use byte offsets when emitting debuginfo columns --- .../debuginfo/create_scope_map.rs | 17 +++--- .../debuginfo/metadata.rs | 10 ++-- src/librustc_codegen_llvm/debuginfo/mod.rs | 14 ++--- .../debuginfo/source_loc.rs | 53 ++++++++++++++----- src/librustc_codegen_llvm/debuginfo/utils.rs | 8 --- src/test/codegen/debug-column.rs | 14 +++-- 6 files changed, 68 insertions(+), 48 deletions(-) diff --git a/src/librustc_codegen_llvm/debuginfo/create_scope_map.rs b/src/librustc_codegen_llvm/debuginfo/create_scope_map.rs index de029ce51b321..09422f4ec3768 100644 --- a/src/librustc_codegen_llvm/debuginfo/create_scope_map.rs +++ b/src/librustc_codegen_llvm/debuginfo/create_scope_map.rs @@ -1,5 +1,5 @@ -use super::metadata::file_metadata; -use super::utils::{span_start, DIB}; +use super::metadata::{file_metadata, UNKNOWN_COLUMN_NUMBER, UNKNOWN_LINE_NUMBER}; +use super::utils::DIB; use rustc_codegen_ssa::mir::debuginfo::{DebugScope, FunctionDebugContext}; use crate::common::CodegenCx; @@ -7,10 +7,6 @@ use crate::llvm; use crate::llvm::debuginfo::{DIScope, DISubprogram}; use rustc::mir::{Body, SourceScope}; -use libc::c_uint; - -use rustc_span::Pos; - use rustc_index::bit_set::BitSet; use rustc_index::vec::Idx; @@ -54,7 +50,7 @@ fn make_mir_scope( debug_context.scopes[parent] } else { // The root is the function itself. - let loc = span_start(cx, mir.span); + let loc = cx.lookup_debug_loc(mir.span.lo()); debug_context.scopes[scope] = DebugScope { scope_metadata: Some(fn_metadata), file_start_pos: loc.file.start_pos, @@ -70,7 +66,7 @@ fn make_mir_scope( return; } - let loc = span_start(cx, scope_data.span); + let loc = cx.lookup_debug_loc(scope_data.span.lo()); let file_metadata = file_metadata(cx, &loc.file.name, debug_context.defining_crate); let scope_metadata = unsafe { @@ -78,9 +74,8 @@ fn make_mir_scope( DIB(cx), parent_scope.scope_metadata.unwrap(), file_metadata, - loc.line as c_uint, - // Loc column is 0-based while debug one is 1-based. - loc.col.to_usize() as c_uint + 1, + loc.line.unwrap_or(UNKNOWN_LINE_NUMBER), + loc.col.unwrap_or(UNKNOWN_COLUMN_NUMBER), )) }; debug_context.scopes[scope] = DebugScope { diff --git a/src/librustc_codegen_llvm/debuginfo/metadata.rs b/src/librustc_codegen_llvm/debuginfo/metadata.rs index d1f70ad43bd28..8dfc55e2854b4 100644 --- a/src/librustc_codegen_llvm/debuginfo/metadata.rs +++ b/src/librustc_codegen_llvm/debuginfo/metadata.rs @@ -5,7 +5,7 @@ use self::RecursiveTypeDescription::*; use super::namespace::mangled_name_of_instance; use super::type_names::compute_debuginfo_type_name; use super::utils::{ - create_DIArray, debug_context, get_namespace_for_item, is_node_local_to_unit, span_start, DIB, + create_DIArray, debug_context, get_namespace_for_item, is_node_local_to_unit, DIB, }; use super::CrateDebugContext; @@ -2280,10 +2280,10 @@ pub fn create_global_var_metadata(cx: &CodegenCx<'ll, '_>, def_id: DefId, global let span = tcx.def_span(def_id); let (file_metadata, line_number) = if !span.is_dummy() { - let loc = span_start(cx, span); - (file_metadata(cx, &loc.file.name, LOCAL_CRATE), loc.line as c_uint) + let loc = cx.lookup_debug_loc(span.lo()); + (file_metadata(cx, &loc.file.name, LOCAL_CRATE), loc.line) } else { - (unknown_file_metadata(cx), UNKNOWN_LINE_NUMBER) + (unknown_file_metadata(cx), None) }; let is_local_to_unit = is_node_local_to_unit(cx, def_id); @@ -2308,7 +2308,7 @@ pub fn create_global_var_metadata(cx: &CodegenCx<'ll, '_>, def_id: DefId, global // which is what we want for no_mangle statics linkage_name.as_ref().map_or(ptr::null(), |name| name.as_ptr()), file_metadata, - line_number, + line_number.unwrap_or(UNKNOWN_LINE_NUMBER), type_metadata, is_local_to_unit, global, diff --git a/src/librustc_codegen_llvm/debuginfo/mod.rs b/src/librustc_codegen_llvm/debuginfo/mod.rs index 22effb102fd67..73e993bb46b87 100644 --- a/src/librustc_codegen_llvm/debuginfo/mod.rs +++ b/src/librustc_codegen_llvm/debuginfo/mod.rs @@ -3,10 +3,10 @@ mod doc; use rustc_codegen_ssa::mir::debuginfo::VariableKind::*; -use self::metadata::{file_metadata, type_metadata, TypeMap}; +use self::metadata::{file_metadata, type_metadata, TypeMap, UNKNOWN_LINE_NUMBER}; use self::namespace::mangled_name_of_instance; use self::type_names::compute_debuginfo_type_name; -use self::utils::{create_DIArray, is_node_local_to_unit, span_start, DIB}; +use self::utils::{create_DIArray, is_node_local_to_unit, DIB}; use crate::llvm; use crate::llvm::debuginfo::{ @@ -257,7 +257,7 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { let def_id = instance.def_id(); let containing_scope = get_containing_scope(self, instance); - let loc = span_start(self, span); + let loc = self.lookup_debug_loc(span.lo()); let file_metadata = file_metadata(self, &loc.file.name, def_id.krate); let function_type_metadata = unsafe { @@ -313,9 +313,9 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { function_name.as_ptr(), linkage_name.as_ptr(), file_metadata, - loc.line as c_uint, + loc.line.unwrap_or(UNKNOWN_LINE_NUMBER), function_type_metadata, - scope_line as c_uint, + scope_line.unwrap_or(UNKNOWN_LINE_NUMBER), flags, spflags, llfn, @@ -538,7 +538,7 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { variable_kind: VariableKind, span: Span, ) -> &'ll DIVariable { - let loc = span_start(self, span); + let loc = self.lookup_debug_loc(span.lo()); let file_metadata = file_metadata(self, &loc.file.name, dbg_context.defining_crate); let type_metadata = type_metadata(self, variable_type, span); @@ -557,7 +557,7 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> { scope_metadata, name.as_ptr(), file_metadata, - loc.line as c_uint, + loc.line.unwrap_or(UNKNOWN_LINE_NUMBER), type_metadata, true, DIFlags::FlagZero, diff --git a/src/librustc_codegen_llvm/debuginfo/source_loc.rs b/src/librustc_codegen_llvm/debuginfo/source_loc.rs index ae5d9a7c8d89d..66ae9d72c3e51 100644 --- a/src/librustc_codegen_llvm/debuginfo/source_loc.rs +++ b/src/librustc_codegen_llvm/debuginfo/source_loc.rs @@ -1,33 +1,58 @@ -use super::metadata::UNKNOWN_COLUMN_NUMBER; -use super::utils::{debug_context, span_start}; +use super::metadata::{UNKNOWN_COLUMN_NUMBER, UNKNOWN_LINE_NUMBER}; +use super::utils::debug_context; use crate::common::CodegenCx; use crate::llvm::debuginfo::DIScope; use crate::llvm::{self, Value}; use rustc_codegen_ssa::traits::*; -use libc::c_uint; -use rustc_span::{Pos, Span}; +use rustc_data_structures::sync::Lrc; +use rustc_span::{BytePos, Pos, SourceFile, SourceFileAndLine, Span}; + +/// A source code location used to generate debug information. +pub struct DebugLoc { + /// Information about the original source file. + pub file: Lrc, + /// The (1-based) line number. + pub line: Option, + /// The (1-based) column number. + pub col: Option, +} impl CodegenCx<'ll, '_> { - pub fn create_debug_loc(&self, scope: &'ll DIScope, span: Span) -> &'ll Value { - let loc = span_start(self, span); + /// Looks up debug source information about a `BytePos`. + pub fn lookup_debug_loc(&self, pos: BytePos) -> DebugLoc { + let (file, line, col) = match self.sess().source_map().lookup_line(pos) { + Ok(SourceFileAndLine { sf: file, line }) => { + let line_pos = file.line_begin_pos(pos); + + // Use 1-based indexing. + let line = (line + 1) as u32; + let col = (pos - line_pos).to_u32() + 1; + + (file, Some(line), Some(col)) + } + Err(file) => (file, None, None), + }; - // For MSVC, set the column number to zero. + // For MSVC, omit the column number. // Otherwise, emit it. This mimics clang behaviour. // See discussion in https://github.com/rust-lang/rust/issues/42921 - let col_used = if self.sess().target.target.options.is_like_msvc { - UNKNOWN_COLUMN_NUMBER + if self.sess().target.target.options.is_like_msvc { + DebugLoc { file, line, col: None } } else { - // Loc column is 0-based while debug one is 1-based. - loc.col.to_usize() as c_uint + 1 - }; + DebugLoc { file, line, col } + } + } + + pub fn create_debug_loc(&self, scope: &'ll DIScope, span: Span) -> &'ll Value { + let DebugLoc { line, col, .. } = self.lookup_debug_loc(span.lo()); unsafe { llvm::LLVMRustDIBuilderCreateDebugLocation( debug_context(self).llcontext, - loc.line as c_uint, - col_used, + line.unwrap_or(UNKNOWN_LINE_NUMBER), + col.unwrap_or(UNKNOWN_COLUMN_NUMBER), scope, None, ) diff --git a/src/librustc_codegen_llvm/debuginfo/utils.rs b/src/librustc_codegen_llvm/debuginfo/utils.rs index 4e17387e057f9..bef40decdf3ab 100644 --- a/src/librustc_codegen_llvm/debuginfo/utils.rs +++ b/src/librustc_codegen_llvm/debuginfo/utils.rs @@ -9,9 +9,6 @@ use rustc_hir::def_id::DefId; use crate::common::CodegenCx; use crate::llvm; use crate::llvm::debuginfo::{DIArray, DIBuilder, DIDescriptor, DIScope}; -use rustc_codegen_ssa::traits::*; - -use rustc_span::Span; pub fn is_node_local_to_unit(cx: &CodegenCx<'_, '_>, def_id: DefId) -> bool { // The is_local_to_unit flag indicates whether a function is local to the @@ -32,11 +29,6 @@ pub fn create_DIArray(builder: &DIBuilder<'ll>, arr: &[Option<&'ll DIDescriptor> }; } -/// Returns rustc_span::Loc corresponding to the beginning of the span -pub fn span_start(cx: &CodegenCx<'_, '_>, span: Span) -> rustc_span::Loc { - cx.sess().source_map().lookup_char_pos(span.lo()) -} - #[inline] pub fn debug_context(cx: &'a CodegenCx<'ll, 'tcx>) -> &'a CrateDebugContext<'ll, 'tcx> { cx.dbg_cx.as_ref().unwrap() diff --git a/src/test/codegen/debug-column.rs b/src/test/codegen/debug-column.rs index 4a7b4176f2cee..f348c48566d51 100644 --- a/src/test/codegen/debug-column.rs +++ b/src/test/codegen/debug-column.rs @@ -1,16 +1,24 @@ -// Verify that emitted debuginfo column nubmers are 1-based. Regression test for issue #65437. +// Verify that debuginfo column nubmers are 1-based byte offsets. // // ignore-windows // compile-flags: -C debuginfo=2 fn main() { unsafe { - // CHECK: call void @giraffe(), !dbg [[DBG:!.*]] - // CHECK: [[DBG]] = !DILocation(line: 10, column: 9 + // Column numbers are 1-based. Regression test for #65437. + // CHECK: call void @giraffe(), !dbg [[A:!.*]] giraffe(); + + // Column numbers use byte offests. Regression test for #67360 + // CHECK: call void @turtle(), !dbg [[B:!.*]] +/* ż */ turtle(); + + // CHECK: [[A]] = !DILocation(line: 10, column: 9, + // CHECK: [[B]] = !DILocation(line: 14, column: 10, } } extern { fn giraffe(); + fn turtle(); } From 8e93a01824a2239f1c588c7c0b884c8a2662e929 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Tue, 25 Feb 2020 00:00:00 +0000 Subject: [PATCH 3/3] Test that column information is not emitted for MSVC targets --- src/test/codegen/debug-column-msvc.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 src/test/codegen/debug-column-msvc.rs diff --git a/src/test/codegen/debug-column-msvc.rs b/src/test/codegen/debug-column-msvc.rs new file mode 100644 index 0000000000000..aad8b372a8a92 --- /dev/null +++ b/src/test/codegen/debug-column-msvc.rs @@ -0,0 +1,16 @@ +// Verify that no column information is emitted for MSVC targets +// +// only-msvc +// compile-flags: -C debuginfo=2 + +// CHECK-NOT: !DILexicalBlock({{.*}}column: {{.*}}) +// CHECK-NOT: !DILocation({{.*}}column: {{.*}}) + +pub fn add(a: u32, b: u32) -> u32 { + a + b +} + +fn main() { + let c = add(1, 2); + println!("{}", c); +}