]> git.lizzy.rs Git - rust.git/commitdiff
debuginfo: Unified namespace generation approach for crate-local and external items...
authorMichael Woerister <michaelwoerister@gmail>
Tue, 1 Oct 2013 10:24:50 +0000 (12:24 +0200)
committerMichael Woerister <michaelwoerister@gmail>
Tue, 8 Oct 2013 08:35:24 +0000 (10:35 +0200)
src/librustc/lib/llvm.rs
src/librustc/middle/trans/base.rs
src/librustc/middle/trans/debuginfo.rs
src/rustllvm/RustWrapper.cpp

index 6fe8d65cc39d9b3f4670f868655d768d43840318..1c56f699c130d78f615f4dba098c597247b61333 100644 (file)
@@ -1968,7 +1968,8 @@ pub fn LLVMDIBuilderCreateStructType(Builder: DIBuilderRef,
                                              DerivedFrom: DIType,
                                              Elements: DIArray,
                                              RunTimeLang: c_uint,
-                                             VTableHolder: ValueRef)
+                                             VTableHolder: ValueRef,
+                                             UniqueId: *c_char)
                                              -> DICompositeType;
 
         #[fast_ffi]
index 73b197ec3619909700d2c084772d701dab4118c6..9b6db062233a5f1aff37d00926b87aac0b611052 100644 (file)
@@ -3119,11 +3119,6 @@ pub fn trans_crate(sess: session::Session,
                                      symbol_hasher,
                                      link_meta,
                                      analysis.reachable);
-
-    if ccx.sess.opts.debuginfo {
-        debuginfo::initialize(ccx, &crate);
-    }
-
     {
         let _icx = push_ctxt("text");
         trans_mod(ccx, &crate.module);
index 52695100b3ee755baef9aaf4a422640fc78086da..a982a4767fdcf7c743dd67875b05f818b775f8d0 100644 (file)
@@ -104,11 +104,13 @@ struct List {
 
 use std::c_str::ToCStr;
 use std::hashmap::HashMap;
+use std::hashmap::HashSet;
 use std::libc::{c_uint, c_ulonglong, c_longlong};
 use std::ptr;
+use std::unstable::atomics;
 use std::vec;
 use syntax::codemap::{Span, Pos};
-use syntax::{ast, codemap, ast_util, ast_map, opt_vec, visit};
+use syntax::{ast, codemap, ast_util, ast_map, opt_vec};
 use syntax::parse::token;
 use syntax::parse::token::special_idents;
 
@@ -136,8 +138,10 @@ pub struct CrateDebugContext {
     priv current_debug_location: DebugLocation,
     priv created_files: HashMap<~str, DIFile>,
     priv created_types: HashMap<uint, DIType>,
-    priv local_namespace_map: HashMap<ast::NodeId, @NamespaceTreeNode>,
-    priv extern_namespaces: HashMap<~[ast::Ident], @NamespaceTreeNode>,
+    priv namespace_map: HashMap<~[ast::Ident], @NamespaceTreeNode>,
+    // This collection is used to assert that composite types (structs, enums, ...) have their
+    // members only set once:
+    priv composite_types_completed: HashSet<DIType>,
 }
 
 impl CrateDebugContext {
@@ -153,8 +157,8 @@ pub fn new(llmod: ModuleRef, crate: ~str) -> CrateDebugContext {
             current_debug_location: UnknownLocation,
             created_files: HashMap::new(),
             created_types: HashMap::new(),
-            local_namespace_map: HashMap::new(),
-            extern_namespaces: HashMap::new(),
+            namespace_map: HashMap::new(),
+            composite_types_completed: HashSet::new(),
         };
     }
 }
@@ -224,16 +228,6 @@ enum VariableKind {
     CapturedVariable,
 }
 
-pub fn initialize(cx: &mut CrateContext, crate: &ast::Crate) {
-    if cx.dbg_cx.is_none() {
-        return;
-    }
-
-    let crate_namespace_ident = token::str_to_ident(cx.link_meta.name);
-    let mut visitor = NamespaceVisitor::new_crate_visitor(cx, crate_namespace_ident);
-    visit::walk_crate(&mut visitor, crate, ());
-}
-
 /// Create any deferred debug metadata nodes
 pub fn finalize(cx: @mut CrateContext) {
     if cx.dbg_cx.is_none() {
@@ -548,11 +542,11 @@ pub fn create_function_debug_context(cx: &mut CrateContext,
     let empty_generics = ast::Generics { lifetimes: opt_vec::Empty, ty_params: opt_vec::Empty };
 
     let fnitem = cx.tcx.items.get_copy(&fn_ast_id);
-    let (ident, fn_decl, generics, top_level_block, span) = match fnitem {
+    let (ident, fn_decl, generics, top_level_block, span, has_path) = match fnitem {
         ast_map::node_item(ref item, _) => {
             match item.node {
                 ast::item_fn(ref fn_decl, _, _, ref generics, ref top_level_block) => {
-                    (item.ident, fn_decl, generics, top_level_block, item.span)
+                    (item.ident, fn_decl, generics, top_level_block, item.span, true)
                 }
                 _ => {
                     cx.sess.span_bug(item.span,
@@ -571,7 +565,7 @@ pub fn create_function_debug_context(cx: &mut CrateContext,
             },
             _,
             _) => {
-            (ident, fn_decl, generics, top_level_block, span)
+            (ident, fn_decl, generics, top_level_block, span, true)
         }
         ast_map::node_expr(ref expr) => {
             match expr.node {
@@ -583,7 +577,9 @@ pub fn create_function_debug_context(cx: &mut CrateContext,
                         // enclosing function.
                         &empty_generics,
                         top_level_block,
-                        expr.span)
+                        expr.span,
+                        // Don't try to lookup the item path:
+                        false)
                 }
                 _ => cx.sess.span_bug(expr.span,
                         "create_function_debug_context: expected an expr_fn_block here")
@@ -601,7 +597,7 @@ pub fn create_function_debug_context(cx: &mut CrateContext,
                 }),
             _,
             _) => {
-            (ident, fn_decl, generics, top_level_block, span)
+            (ident, fn_decl, generics, top_level_block, span, true)
         }
         ast_map::node_foreign_item(@ast::foreign_item { _ }, _, _, _) |
         ast_map::node_variant(*) |
@@ -633,18 +629,16 @@ pub fn create_function_debug_context(cx: &mut CrateContext,
                                                       file_metadata,
                                                       &mut function_name);
 
-    let namespace_node = debug_context(cx).local_namespace_map.find_copy(&fn_ast_id);
-    let (linkage_name, containing_scope) = match namespace_node {
-        Some(namespace_node) => {
-            (namespace_node.mangled_name_of_contained_item(function_name), namespace_node.scope)
-        }
-        None => {
-            // This branch is only hit when there is a bug in the NamespaceVisitor.
-            cx.sess.span_warn(span, format!("debuginfo: Could not find namespace node for function
-                                          with name {}. This is a bug! Please report this to
-                                          github.com/mozilla/rust/issues", function_name));
-            (function_name.clone(), file_metadata)
-        }
+    // There is no ast_map::path for ast::ExprFnBlock-type functions. For now, just don't put them
+    // into a namespace. In the future this could be improved somehow (storing a path in the
+    // ast_map, or construct a path using the enclosing function).
+    let (linkage_name, containing_scope) = if has_path {
+        let namespace_node = namespace_for_item(cx, ast_util::local_def(fn_ast_id), span);
+        let linkage_name = namespace_node.mangled_name_of_contained_item(function_name);
+        let containing_scope = namespace_node.scope;
+        (linkage_name, containing_scope)
+    } else {
+        (function_name.clone(), file_metadata)
     };
 
     let scope_line = get_scope_line(cx, top_level_block, loc.line);
@@ -682,16 +676,6 @@ pub fn create_function_debug_context(cx: &mut CrateContext,
     let arg_pats = do fn_decl.inputs.map |arg_ref| { arg_ref.pat };
     populate_scope_map(cx, arg_pats, top_level_block, fn_metadata, &mut fn_debug_context.scope_map);
 
-    // Create namespaces for the interior of this function
-    {
-        let mut namespace_visitor = NamespaceVisitor::new_function_visitor(cx,
-                                                                           function_name,
-                                                                           namespace_node,
-                                                                           file_metadata,
-                                                                           span);
-        visit::walk_block(&mut namespace_visitor, top_level_block, ());
-    }
-
     return FunctionDebugContext(fn_debug_context);
 
     fn get_function_signature(cx: &mut CrateContext,
@@ -1584,6 +1568,17 @@ fn set_members_of_composite_type(cx: &mut CrateContext,
                                  member_descriptions: &[MemberDescription],
                                  file_metadata: DIFile,
                                  definition_span: Span) {
+    // In some rare cases LLVM metadata uniquing would lead to an existing type description being
+    // used instead of a new one created in create_struct_stub. This would cause a hard to trace
+    // assertion in DICompositeType::SetTypeArray(). The following check makes sure that we get a
+    // better error message if this should happen again due to some regression.
+    if debug_context(cx).composite_types_completed.contains(&composite_type_metadata) {
+        cx.sess.span_bug(definition_span, "debuginfo::set_members_of_composite_type() - Already \
+                                           completed forward declaration re-encountered.");
+    } else {
+        debug_context(cx).composite_types_completed.insert(composite_type_metadata);
+    }
+
     let loc = span_start(cx, definition_span);
 
     let member_metadata: ~[DIDescriptor] = member_descriptions
@@ -1632,8 +1627,16 @@ fn create_struct_stub(cx: &mut CrateContext,
     let loc = span_start(cx, definition_span);
     let (struct_size, struct_align) = size_and_align_of(cx, struct_llvm_type);
 
-    return do struct_type_name.with_c_str |name| {
-        unsafe {
+    // We assign unique IDs to the type stubs so LLVM metadata uniquing does not reuse instances
+    // where we don't want it.
+    let unique_id = unsafe {
+        static mut unique_id_counter: atomics::AtomicUint = atomics::INIT_ATOMIC_UINT;
+        format!("DiStructStub{}", unique_id_counter.fetch_add(1, atomics::SeqCst))
+    };
+
+    return unsafe {
+        do struct_type_name.with_c_str |name| {
+        do unique_id.with_c_str |unique_id| {
             // LLVMDIBuilderCreateStructType() wants an empty array. A null pointer will lead to
             // hard to trace and debug LLVM assertions later on in llvm/lib/IR/Value.cpp
             let empty_array = create_DIArray(DIB(cx), []);
@@ -1650,8 +1653,9 @@ fn create_struct_stub(cx: &mut CrateContext,
                 ptr::null(),
                 empty_array,
                 0,
-                ptr::null())
-    }};
+                ptr::null(),
+                unique_id)
+    }}};
 }
 
 fn boxed_type_metadata(cx: &mut CrateContext,
@@ -2199,8 +2203,8 @@ fn get_namespace_and_span_for_item(cx: &mut CrateContext,
                                    def_id: ast::DefId,
                                    warning_span: Span)
                                 -> (DIScope, Span) {
-    if def_id.crate == ast::LOCAL_CRATE {
-        let containing_scope = debug_context(cx).local_namespace_map.get_copy(&def_id.node).scope;
+    let containing_scope = namespace_for_item(cx, def_id, warning_span).scope;
+    let definition_span = if def_id.crate == ast::LOCAL_CRATE {
         let definition_span = match cx.tcx.items.find(&def_id.node) {
             Some(&ast_map::node_item(@ast::item { span, _ }, _)) => span,
             ref node => {
@@ -2210,12 +2214,13 @@ fn get_namespace_and_span_for_item(cx: &mut CrateContext,
                 codemap::dummy_sp()
             }
         };
-        (containing_scope, definition_span)
+        definition_span
     } else {
-        let item_path = ty::item_path(cx.tcx, def_id);
         // For external items there is no span information
-        (namespace_for_external_item(cx, &item_path).scope, codemap::dummy_sp())
-    }
+        codemap::dummy_sp()
+    };
+
+    (containing_scope, definition_span)
 }
 
 // This procedure builds the *scope map* for a given function, which maps any given ast::NodeId in
@@ -2703,29 +2708,41 @@ fn fill_nested(node: &NamespaceTreeNode, output: &mut ~str) {
     }
 }
 
-fn namespace_for_external_item(cx: &mut CrateContext,
-                               item_path: &ast_map::path)
-                            -> @NamespaceTreeNode {
-    if item_path.len() < 2 {
-        cx.sess.bug(format!("debuginfo::namespace_for_external_item() - Invalid item_path: {}",
-            ast_map::path_to_str(*item_path, token::get_ident_interner())));
-    }
+fn namespace_for_item(cx: &mut CrateContext,
+                      def_id: ast::DefId,
+                      warning_span: Span)
+                   -> @NamespaceTreeNode {
+    let namespace_path = {
+        let mut item_path = ty::item_path(cx.tcx, def_id);
 
-    let path_excluding_item = item_path.slice_to(item_path.len() - 1);
-    let mut current_key = vec::with_capacity(path_excluding_item.len());
-    let mut parent_node: Option<@NamespaceTreeNode> = None;
-    let last_index = path_excluding_item.len() - 1;
+        if (def_id.crate == ast::LOCAL_CRATE && item_path.len() < 1) ||
+           (def_id.crate != ast::LOCAL_CRATE && item_path.len() < 2) {
+            cx.sess.bug(format!("debuginfo::namespace_for_item() - Item path too short: {}",
+                ast_map::path_to_str(item_path, token::get_ident_interner())));
+        }
 
-    for (i, &path_element) in path_excluding_item.iter().enumerate() {
-        let ident = match path_element {
-            ast_map::path_mod(ident)            |
-            ast_map::path_name(ident)           |
-            ast_map::path_pretty_name(ident, _) => ident
-        };
+        // remove the name of the item
+        item_path.pop();
+
+        if def_id.crate == ast::LOCAL_CRATE {
+            // prepend crate name if not already present
+            let crate_namespace_ident = token::str_to_ident(cx.link_meta.name);
+            item_path.insert(0, ast_map::path_mod(crate_namespace_ident));
+        }
 
+        item_path
+    };
+
+    let mut current_key = vec::with_capacity(namespace_path.len());
+    let mut parent_node: Option<@NamespaceTreeNode> = None;
+    let last_index = namespace_path.len() - 1;
+
+    // Create/Lookup namespace for each element of the path.
+    for (i, &path_element) in namespace_path.iter().enumerate() {
+        let ident = path_element.ident();
         current_key.push(ident);
 
-        let existing_node = debug_context(cx).extern_namespaces.find_copy(&current_key);
+        let existing_node = debug_context(cx).namespace_map.find_copy(&current_key);
         let current_node = match existing_node {
             Some(existing_node) => existing_node,
             None => {
@@ -2743,7 +2760,7 @@ fn namespace_for_external_item(cx: &mut CrateContext,
                             parent_scope,
                             namespace_name,
                             ptr::null(), // cannot reconstruct file ...
-                            0)           // ... or line information
+                            0)           // ... or line information, but that's not so important.
                     }
                 };
 
@@ -2753,7 +2770,7 @@ fn namespace_for_external_item(cx: &mut CrateContext,
                     parent: parent_node,
                 };
 
-                debug_context(cx).extern_namespaces.insert(current_key.clone(), node);
+                debug_context(cx).namespace_map.insert(current_key.clone(), node);
 
                 node
             }
@@ -2766,142 +2783,9 @@ fn namespace_for_external_item(cx: &mut CrateContext,
         }
     }
 
-    cx.sess.bug("debuginfo::namespace_for_external_item() - Code path should be unreachable");
-}
-
-struct NamespaceVisitor<'self> {
-    module_ident: ast::Ident,
-    scope_stack: ~[@NamespaceTreeNode],
-    crate_context: &'self mut CrateContext,
-}
-
-impl<'self> NamespaceVisitor<'self> {
-
-    fn new_crate_visitor<'a>(cx: &'a mut CrateContext,
-                             crate_ident: ast::Ident)
-                          -> NamespaceVisitor<'a> {
-        NamespaceVisitor {
-            module_ident: crate_ident,
-            scope_stack: ~[],
-            crate_context: cx,
-        }
-    }
-
-    fn new_function_visitor<'a>(cx: &'a mut CrateContext,
-                                function_name: &str,
-                                parent_node: Option<@NamespaceTreeNode>,
-                                file_metadata: DIFile,
-                                span: Span)
-                             -> NamespaceVisitor<'a> {
-        let companion_name = function_name + "()";
-        let companion_ident = token::str_to_ident(companion_name);
-        let parent_scope = match parent_node {
-            Some(parent_node) => parent_node.scope,
-            None => ptr::null()
-        };
-        let line = span_start(cx, span).line as c_uint;
-
-        let namespace_metadata = unsafe {
-            do companion_name.with_c_str |companion_name| {
-                llvm::LLVMDIBuilderCreateNameSpace(
-                    DIB(cx),
-                    parent_scope,
-                    companion_name,
-                    file_metadata,
-                    line)
-            }
-        };
-
-        let function_node = @NamespaceTreeNode {
-            scope: namespace_metadata,
-            ident: companion_ident,
-            parent: parent_node,
-        };
-
-        return NamespaceVisitor {
-            module_ident: special_idents::invalid,
-            scope_stack: ~[function_node],
-            crate_context: cx,
-        };
-    }
-}
-
-// Possible optimization: Only recurse if needed.
-impl<'self> visit::Visitor<()> for NamespaceVisitor<'self> {
-
-    fn visit_mod(&mut self,
-                 module: &ast::_mod,
-                 span: Span,
-                 _: ast::NodeId,
-                 _: ()) {
-        let module_name = token::ident_to_str(&self.module_ident);
-
-        let (parent_node, parent_scope) = if self.scope_stack.len() > 0 {
-            let parent_node = *self.scope_stack.last();
-            (Some(parent_node), parent_node.scope)
-        } else {
-            (None, ptr::null())
-        };
-
-        let loc = span_start(self.crate_context, span);
-        let file_metadata = file_metadata(self.crate_context, loc.file.name);
-
-        let namespace_metadata = unsafe {
-            do module_name.with_c_str |module_name| {
-                llvm::LLVMDIBuilderCreateNameSpace(
-                    DIB(self.crate_context),
-                    parent_scope,
-                    module_name,
-                    file_metadata,
-                    loc.line as c_uint)
-            }
-        };
-
-        let this_node = @NamespaceTreeNode {
-            scope: namespace_metadata,
-            ident: self.module_ident,
-            parent: parent_node,
-        };
-
-        self.scope_stack.push(this_node);
-
-        visit::walk_mod(self, module, ());
-
-        self.scope_stack.pop();
-    }
-
-    fn visit_item(&mut self, item: @ast::item, _: ()) {
-        match item.node {
-            ast::item_mod(*) => {
-                // always store the last module ident so visit_mod() has it available
-                self.module_ident = item.ident;
-            }
-            ast::item_fn(*) => { /* handled by visit_fn */ }
-            _ => {
-                debug_context(self.crate_context)
-                    .local_namespace_map
-                    .insert(item.id, *self.scope_stack.last());
-            }
-        }
-
-        visit::walk_item(self, item, ());
-    }
-
-    fn visit_foreign_item(&mut self, item: @ast::foreign_item, _: ()) {
-        debug_context(self.crate_context)
-            .local_namespace_map
-            .insert(item.id, *self.scope_stack.last());
-    }
-
-    fn visit_fn(&mut self,
-                _: &visit::fn_kind,
-                _: &ast::fn_decl,
-                _: &ast::Block,
-                _: Span,
-                node_id: ast::NodeId,
-                _: ()) {
-        debug_context(self.crate_context)
-            .local_namespace_map
-            .insert(node_id, *self.scope_stack.last());
-    }
+    // Should be unreachable:
+    let error_message = format!("debuginfo::namespace_for_item() - Code path should be \
+                                 unreachable. namespace_path was {}",
+                                 ast_map::path_to_str(namespace_path, token::get_ident_interner()));
+    cx.sess.span_bug(warning_span, error_message);
 }
index 63d42816207cd79dae5cd0713cd6233e2d0d4882..31a02dceb1c09e17b2cbdcf8f25085c3afe5a12b 100644 (file)
@@ -548,14 +548,21 @@ extern "C" LLVMValueRef LLVMDIBuilderCreateStructType(
     LLVMValueRef DerivedFrom,
     LLVMValueRef Elements,
     unsigned RunTimeLang,
-    LLVMValueRef VTableHolder) {
+    LLVMValueRef VTableHolder,
+    const char *UniqueId) {
     return wrap(Builder->createStructType(
-        unwrapDI<DIDescriptor>(Scope), Name,
-        unwrapDI<DIFile>(File), LineNumber,
-        SizeInBits, AlignInBits, Flags,
+        unwrapDI<DIDescriptor>(Scope),
+        Name,
+        unwrapDI<DIFile>(File),
+        LineNumber,
+        SizeInBits,
+        AlignInBits,
+        Flags,
         unwrapDI<DIType>(DerivedFrom),
-        unwrapDI<DIArray>(Elements), RunTimeLang,
-        unwrapDI<MDNode*>(VTableHolder)));
+        unwrapDI<DIArray>(Elements),
+        RunTimeLang,
+        unwrapDI<MDNode*>(VTableHolder),
+        UniqueId));
 }
 
 extern "C" LLVMValueRef LLVMDIBuilderCreateMemberType(