Rollup merge of #69357 - tmiasko:debuginfo-column, r=michaelwoerister

Emit 1-based column numbers in debuginfo

* Use byte offsets instead of char position offsets. Resolves #67360.
* Use 1-based offsets instead of 0-based ones. Resolves #65437.
* Consistently omit column information for msvc targets, matching clang behaviour (previously columns have been omitted from `DILocation`, but not from `DILexicalBlock`).

Author: Dylan-DPC

Diff for: src/librustc_codegen_llvm/debuginfo/create_scope_map.rs

@@ -1,16 +1,12 @@
-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;
 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,16 +66,16 @@ 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 {
         Some(llvm::LLVMRustDIBuilderCreateLexicalBlock(
             DIB(cx),
             parent_scope.scope_metadata.unwrap(),
             file_metadata,
-            loc.line as c_uint,
-            loc.col.to_usize() as c_uint,
+            loc.line.unwrap_or(UNKNOWN_LINE_NUMBER),
+            loc.col.unwrap_or(UNKNOWN_COLUMN_NUMBER),
         ))
     };
     debug_context.scopes[scope] = DebugScope {

Diff for: 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;
 
@@ -2309,10 +2309,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);
@@ -2339,7 +2339,7 @@ pub fn create_global_var_metadata(cx: &CodegenCx<'ll, '_>, def_id: DefId, global
             linkage_name.as_ptr().cast(),
             linkage_name.len(),
             file_metadata,
-            line_number,
+            line_number.unwrap_or(UNKNOWN_LINE_NUMBER),
             type_metadata,
             is_local_to_unit,
             global,

Diff for: 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::{
@@ -248,7 +248,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 {
@@ -304,9 +304,9 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
                 linkage_name.as_ptr().cast(),
                 linkage_name.len(),
                 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,
@@ -530,7 +530,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);
@@ -550,7 +550,7 @@ impl DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
                 name.as_ptr().cast(),
                 name.len(),
                 file_metadata,
-                loc.line as c_uint,
+                loc.line.unwrap_or(UNKNOWN_LINE_NUMBER),
                 type_metadata,
                 true,
                 DIFlags::FlagZero,

Diff for: src/librustc_codegen_llvm/debuginfo/source_loc.rs

@@ -1,32 +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<SourceFile>,
+    /// The (1-based) line number.
+    pub line: Option<u32>,
+    /// The (1-based) column number.
+    pub col: Option<u32>,
+}
 
 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.col.to_usize() as c_uint
-        };
+            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 for: 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 for: 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);
+}

Diff for: src/test/codegen/debug-column.rs

@@ -0,0 +1,24 @@
+// Verify that debuginfo column nubmers are 1-based byte offsets.
+//
+// ignore-windows
+// compile-flags: -C debuginfo=2
+
+fn main() {
+    unsafe {
+        // 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();
+}