Merge pull request #1472 from rust-lang/debuginfo_improvements2

Add debuginfo for statics

Author: bjorn3

src/base.rs

@@ -95,7 +95,6 @@ pub(crate) fn codegen_fn<'tcx>(
         caller_location: None, // set by `codegen_fn_prelude`
 
         clif_comments,
-        last_source_file: None,
         next_ssa_var: 0,
     };
 

src/common.rs

@@ -1,12 +1,9 @@
 use cranelift_codegen::isa::TargetFrontendConfig;
-use gimli::write::FileId;
-use rustc_data_structures::sync::Lrc;
 use rustc_index::IndexVec;
 use rustc_middle::ty::layout::{
     FnAbiError, FnAbiOfHelpers, FnAbiRequest, LayoutError, LayoutOfHelpers,
 };
 use rustc_span::source_map::Spanned;
-use rustc_span::SourceFile;
 use rustc_target::abi::call::FnAbi;
 use rustc_target::abi::{Integer, Primitive};
 use rustc_target::spec::{HasTargetSpec, Target};
@@ -305,11 +302,6 @@ pub(crate) struct FunctionCx<'m, 'clif, 'tcx: 'm> {
 
     pub(crate) clif_comments: crate::pretty_clif::CommentWriter,
 
-    /// Last accessed source file and it's debuginfo file id.
-    ///
-    /// For optimization purposes only
-    pub(crate) last_source_file: Option<(Lrc<SourceFile>, FileId)>,
-
     /// This should only be accessed by `CPlace::new_var`.
     pub(crate) next_ssa_var: u32,
 }
@@ -419,25 +411,8 @@ impl<'tcx> FunctionCx<'_, '_, 'tcx> {
 
     pub(crate) fn set_debug_loc(&mut self, source_info: mir::SourceInfo) {
         if let Some(debug_context) = &mut self.cx.debug_context {
-            let (file, line, column) =
-                DebugContext::get_span_loc(self.tcx, self.mir.span, source_info.span);
-
-            // add_source_file is very slow.
-            // Optimize for the common case of the current file not being changed.
-            let mut cached_file_id = None;
-            if let Some((ref last_source_file, last_file_id)) = self.last_source_file {
-                // If the allocations are not equal, the files may still be equal, but that
-                // doesn't matter, as this is just an optimization.
-                if rustc_data_structures::sync::Lrc::ptr_eq(last_source_file, &file) {
-                    cached_file_id = Some(last_file_id);
-                }
-            }
-
-            let file_id = if let Some(file_id) = cached_file_id {
-                file_id
-            } else {
-                debug_context.add_source_file(&file)
-            };
+            let (file_id, line, column) =
+                debug_context.get_span_loc(self.tcx, self.mir.span, source_info.span);
 
             let source_loc =
                 self.func_debug_cx.as_mut().unwrap().add_dbg_loc(file_id, line, column);

src/constant.rs

@@ -31,10 +31,16 @@ impl ConstantCx {
     }
 }
 
-pub(crate) fn codegen_static(tcx: TyCtxt<'_>, module: &mut dyn Module, def_id: DefId) {
+pub(crate) fn codegen_static(tcx: TyCtxt<'_>, module: &mut dyn Module, def_id: DefId) -> DataId {
     let mut constants_cx = ConstantCx::new();
     constants_cx.todo.push(TodoItem::Static(def_id));
     constants_cx.finalize(tcx, module);
+
+    data_id_for_static(
+        tcx, module, def_id, false,
+        // For a declaration the stated mutability doesn't matter.
+        false,
+    )
 }
 
 pub(crate) fn codegen_tls_ref<'tcx>(

src/debuginfo/emit.rs

@@ -1,5 +1,6 @@
 //! Write the debuginfo into an object file.
 
+use cranelift_module::{DataId, FuncId};
 use cranelift_object::ObjectProduct;
 use gimli::write::{Address, AttributeValue, EndianVec, Result, Sections, Writer};
 use gimli::{RunTimeEndian, SectionId};
@@ -8,6 +9,18 @@ use rustc_data_structures::fx::FxHashMap;
 use super::object::WriteDebugInfo;
 use super::DebugContext;
 
+pub(super) fn address_for_func(func_id: FuncId) -> Address {
+    let symbol = func_id.as_u32();
+    assert!(symbol & 1 << 31 == 0);
+    Address::Symbol { symbol: symbol as usize, addend: 0 }
+}
+
+pub(super) fn address_for_data(data_id: DataId) -> Address {
+    let symbol = data_id.as_u32();
+    assert!(symbol & 1 << 31 == 0);
+    Address::Symbol { symbol: (symbol | 1 << 31) as usize, addend: 0 }
+}
+
 impl DebugContext {
     pub(crate) fn emit(&mut self, product: &mut ObjectProduct) {
         let unit_range_list_id = self.dwarf.unit.ranges.add(self.unit_range_list.clone());
@@ -171,6 +184,7 @@ impl Writer for WriterRelocate {
                 gimli::DW_EH_PE_pcrel => {
                     let size = match eh_pe.format() {
                         gimli::DW_EH_PE_sdata4 => 4,
+                        gimli::DW_EH_PE_sdata8 => 8,
                         _ => return Err(gimli::write::Error::UnsupportedPointerEncoding(eh_pe)),
                     };
                     self.relocs.push(DebugReloc {

src/debuginfo/line_info.rs

@@ -5,14 +5,12 @@ use std::path::{Component, Path};
 
 use cranelift_codegen::binemit::CodeOffset;
 use cranelift_codegen::MachSrcLoc;
-use gimli::write::{
-    Address, AttributeValue, FileId, FileInfo, LineProgram, LineString, LineStringTable,
-};
-use rustc_data_structures::sync::Lrc;
+use gimli::write::{AttributeValue, FileId, FileInfo, LineProgram, LineString, LineStringTable};
 use rustc_span::{
     FileName, Pos, SourceFile, SourceFileAndLine, SourceFileHash, SourceFileHashAlgorithm,
 };
 
+use crate::debuginfo::emit::address_for_func;
 use crate::debuginfo::FunctionDebugContext;
 use crate::prelude::*;
 
@@ -60,72 +58,78 @@ fn make_file_info(hash: SourceFileHash) -> Option<FileInfo> {
 
 impl DebugContext {
     pub(crate) fn get_span_loc(
+        &mut self,
         tcx: TyCtxt<'_>,
         function_span: Span,
         span: Span,
-    ) -> (Lrc<SourceFile>, u64, u64) {
+    ) -> (FileId, u64, u64) {
         // Based on https://github.com/rust-lang/rust/blob/e369d87b015a84653343032833d65d0545fd3f26/src/librustc_codegen_ssa/mir/mod.rs#L116-L131
         // In order to have a good line stepping behavior in debugger, we overwrite debug
         // locations of macro expansions with that of the outermost expansion site (when the macro is
         // annotated with `#[collapse_debuginfo]` or when `-Zdebug-macros` is provided).
         let span = tcx.collapsed_debuginfo(span, function_span);
         match tcx.sess.source_map().lookup_line(span.lo()) {
             Ok(SourceFileAndLine { sf: file, line }) => {
+                let file_id = self.add_source_file(&file);
                 let line_pos = file.lines()[line];
                 let col = file.relative_position(span.lo()) - line_pos;
 
-                (file, u64::try_from(line).unwrap() + 1, u64::from(col.to_u32()) + 1)
+                (file_id, u64::try_from(line).unwrap() + 1, u64::from(col.to_u32()) + 1)
             }
-            Err(file) => (file, 0, 0),
+            Err(file) => (self.add_source_file(&file), 0, 0),
         }
     }
 
     pub(crate) fn add_source_file(&mut self, source_file: &SourceFile) -> FileId {
-        let line_program: &mut LineProgram = &mut self.dwarf.unit.line_program;
-        let line_strings: &mut LineStringTable = &mut self.dwarf.line_strings;
-
-        match &source_file.name {
-            FileName::Real(path) => {
-                let (dir_path, file_name) =
-                    split_path_dir_and_file(if self.should_remap_filepaths {
-                        path.remapped_path_if_available()
-                    } else {
-                        path.local_path_if_available()
-                    });
-                let dir_name = osstr_as_utf8_bytes(dir_path.as_os_str());
-                let file_name = osstr_as_utf8_bytes(file_name);
-
-                let dir_id = if !dir_name.is_empty() {
-                    let dir_name = LineString::new(dir_name, line_program.encoding(), line_strings);
-                    line_program.add_directory(dir_name)
-                } else {
-                    line_program.default_directory()
-                };
-                let file_name = LineString::new(file_name, line_program.encoding(), line_strings);
-
-                let info = make_file_info(source_file.src_hash);
-
-                line_program.file_has_md5 &= info.is_some();
-                line_program.add_file(file_name, dir_id, info)
-            }
-            // FIXME give more appropriate file names
-            filename => {
-                let dir_id = line_program.default_directory();
-                let dummy_file_name = LineString::new(
-                    filename
-                        .display(if self.should_remap_filepaths {
-                            FileNameDisplayPreference::Remapped
+        let cache_key = (source_file.stable_id, source_file.src_hash);
+        *self.created_files.entry(cache_key).or_insert_with(|| {
+            let line_program: &mut LineProgram = &mut self.dwarf.unit.line_program;
+            let line_strings: &mut LineStringTable = &mut self.dwarf.line_strings;
+
+            match &source_file.name {
+                FileName::Real(path) => {
+                    let (dir_path, file_name) =
+                        split_path_dir_and_file(if self.should_remap_filepaths {
+                            path.remapped_path_if_available()
                         } else {
-                            FileNameDisplayPreference::Local
-                        })
-                        .to_string()
-                        .into_bytes(),
-                    line_program.encoding(),
-                    line_strings,
-                );
-                line_program.add_file(dummy_file_name, dir_id, None)
+                            path.local_path_if_available()
+                        });
+                    let dir_name = osstr_as_utf8_bytes(dir_path.as_os_str());
+                    let file_name = osstr_as_utf8_bytes(file_name);
+
+                    let dir_id = if !dir_name.is_empty() {
+                        let dir_name =
+                            LineString::new(dir_name, line_program.encoding(), line_strings);
+                        line_program.add_directory(dir_name)
+                    } else {
+                        line_program.default_directory()
+                    };
+                    let file_name =
+                        LineString::new(file_name, line_program.encoding(), line_strings);
+
+                    let info = make_file_info(source_file.src_hash);
+
+                    line_program.file_has_md5 &= info.is_some();
+                    line_program.add_file(file_name, dir_id, info)
+                }
+                filename => {
+                    let dir_id = line_program.default_directory();
+                    let dummy_file_name = LineString::new(
+                        filename
+                            .display(if self.should_remap_filepaths {
+                                FileNameDisplayPreference::Remapped
+                            } else {
+                                FileNameDisplayPreference::Local
+                            })
+                            .to_string()
+                            .into_bytes(),
+                        line_program.encoding(),
+                        line_strings,
+                    );
+                    line_program.add_file(dummy_file_name, dir_id, None)
+                }
             }
-        }
+        })
     }
 }
 
@@ -138,7 +142,7 @@ impl FunctionDebugContext {
     pub(super) fn create_debug_lines(
         &mut self,
         debug_context: &mut DebugContext,
-        symbol: usize,
+        func_id: FuncId,
         context: &Context,
     ) -> CodeOffset {
         let create_row_for_span =
@@ -151,11 +155,7 @@ impl FunctionDebugContext {
                 debug_context.dwarf.unit.line_program.generate_row();
             };
 
-        debug_context
-            .dwarf
-            .unit
-            .line_program
-            .begin_sequence(Some(Address::Symbol { symbol, addend: 0 }));
+        debug_context.dwarf.unit.line_program.begin_sequence(Some(address_for_func(func_id)));
 
         let mut func_end = 0;
 
@@ -178,10 +178,7 @@ impl FunctionDebugContext {
         assert_ne!(func_end, 0);
 
         let entry = debug_context.dwarf.unit.get_mut(self.entry_id);
-        entry.set(
-            gimli::DW_AT_low_pc,
-            AttributeValue::Address(Address::Symbol { symbol, addend: 0 }),
-        );
+        entry.set(gimli::DW_AT_low_pc, AttributeValue::Address(address_for_func(func_id)));
         entry.set(gimli::DW_AT_high_pc, AttributeValue::Udata(u64::from(func_end)));
 
         func_end