]> git.lizzy.rs Git - rust.git/commitdiff
Auto merge of #23011 - nagisa:the-war-of-symbol-and-symbol, r=pnkfelix
authorbors <bors@rust-lang.org>
Sun, 12 Apr 2015 01:26:53 +0000 (01:26 +0000)
committerbors <bors@rust-lang.org>
Sun, 12 Apr 2015 01:26:53 +0000 (01:26 +0000)
We provide tools to tell what exact symbols to emit for any fn or static, but
don’t quite check if that won’t cause any issues later on. Some of the issues
include LLVM mangling our names again and our names pointing to wrong locations,
us generating dumb foreign call wrappers, linker errors, extern functions
resolving to different symbols altogether (`extern {fn fail();} fail();` in some
cases calling `fail1()`), etc.

Before the commit we had a function called `note_unique_llvm_symbol`, so it is
clear somebody was aware of the issue at some point, but the function was barely
used, mostly in irrelevant locations.

Along with working on it I took liberty to start refactoring trans/base into
a few smaller modules. The refactoring is incomplete and I hope I will find some
motivation to carry on with it.

This is possibly a [breaking-change] because it makes dumbly written code
properly invalid.

This fixes all those issues about incorrect use of #[no_mangle] being not reported/misreported/ICEd by the compiler.

NB. This PR does not attempt to tackle the parallel codegen issue that was mentioned in https://github.com/rust-lang/rust/pull/22811, but I believe it should be very straightforward in a follow up PR by modifying `trans::declare::get_defined_value` to look at all the contexts.

cc @alexcrichton @huonw @nrc because you commented on the original RFC issue.

EDIT: wow, this became much bigger than I initially intended.

1  2 
src/librustc_trans/trans/common.rs
src/librustc_trans/trans/controlflow.rs
src/librustc_trans/trans/debuginfo.rs
src/libsyntax/attr.rs

index 2c8da2dc31c5df353887b97bf80a0c73318af71c,153242353d49f5e5c9ac2cfefb482df90fc77861..168a294159d4fe7a8d85d34eb9842267f9fa382c
@@@ -31,6 -31,7 +31,7 @@@ use trans::cleanup
  use trans::consts;
  use trans::datum;
  use trans::debuginfo::{self, DebugLoc};
+ use trans::declare;
  use trans::machine;
  use trans::monomorphize;
  use trans::type_::Type;
@@@ -48,6 -49,7 +49,6 @@@ use std::ffi::CString
  use std::cell::{Cell, RefCell};
  use std::result::Result as StdResult;
  use std::vec::Vec;
 -use syntax::ast::Ident;
  use syntax::ast;
  use syntax::ast_map::{PathElem, PathName};
  use syntax::codemap::{DUMMY_SP, Span};
@@@ -621,8 -623,8 +622,8 @@@ impl<'blk, 'tcx> BlockS<'blk, 'tcx> 
      }
      pub fn sess(&self) -> &'blk Session { self.fcx.ccx.sess() }
  
 -    pub fn ident(&self, ident: Ident) -> String {
 -        token::get_ident(ident).to_string()
 +    pub fn name(&self, name: ast::Name) -> String {
 +        token::get_name(name).to_string()
      }
  
      pub fn node_id_to_string(&self, id: ast::NodeId) -> String {
@@@ -871,9 -873,10 +872,10 @@@ pub fn C_cstr(cx: &CrateContext, s: Int
                                                  !null_terminated as Bool);
  
          let gsym = token::gensym("str");
-         let buf = CString::new(format!("str{}", gsym.usize()));
-         let buf = buf.unwrap();
-         let g = llvm::LLVMAddGlobal(cx.llmod(), val_ty(sc).to_ref(), buf.as_ptr());
+         let sym = format!("str{}", gsym.usize());
+         let g = declare::define_global(cx, &sym[..], val_ty(sc)).unwrap_or_else(||{
+             cx.sess().bug(&format!("symbol `{}` is already defined", sym));
+         });
          llvm::LLVMSetInitializer(g, sc);
          llvm::LLVMSetGlobalConstant(g, True);
          llvm::SetLinkage(g, llvm::InternalLinkage);
index d1ecc1d6ddfb6913e9c44515afc4a8f4432c7c42,3ce883af07f6a56e94296a3d238bd2a286e5c024..f544efe7401c4dc00a90f1ed0c7fe591dcf55650
@@@ -27,6 -27,7 +27,6 @@@ use middle::ty
  use util::ppaux::Repr;
  
  use syntax::ast;
 -use syntax::ast::Ident;
  use syntax::ast_util;
  use syntax::parse::token::InternedString;
  use syntax::parse::token;
@@@ -320,7 -321,7 +320,7 @@@ pub fn trans_loop<'blk, 'tcx>(bcx: Bloc
  
  pub fn trans_break_cont<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
                                      expr: &ast::Expr,
 -                                    opt_label: Option<Ident>,
 +                                    opt_label: Option<ast::Ident>,
                                      exit: usize)
                                      -> Block<'blk, 'tcx> {
      let _icx = push_ctxt("trans_break_cont");
  
  pub fn trans_break<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
                                 expr: &ast::Expr,
 -                               label_opt: Option<Ident>)
 +                               label_opt: Option<ast::Ident>)
                                 -> Block<'blk, 'tcx> {
      return trans_break_cont(bcx, expr, label_opt, cleanup::EXIT_BREAK);
  }
  
  pub fn trans_cont<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
                                expr: &ast::Expr,
 -                              label_opt: Option<Ident>)
 +                              label_opt: Option<ast::Ident>)
                                -> Block<'blk, 'tcx> {
      return trans_break_cont(bcx, expr, label_opt, cleanup::EXIT_LOOP);
  }
@@@ -416,8 -417,7 +416,7 @@@ pub fn trans_fail<'blk, 'tcx>(bcx: Bloc
      let filename = C_str_slice(ccx, filename);
      let line = C_u32(ccx, loc.line as u32);
      let expr_file_line_const = C_struct(ccx, &[v_str, filename, line], false);
-     let expr_file_line = consts::addr_of(ccx, expr_file_line_const,
-                                          "panic_loc", call_info.id);
+     let expr_file_line = consts::addr_of(ccx, expr_file_line_const, "panic_loc");
      let args = vec!(expr_file_line);
      let did = langcall(bcx, Some(call_info.span), "", PanicFnLangItem);
      let bcx = callee::trans_lang_call(bcx,
@@@ -449,8 -449,7 +448,7 @@@ pub fn trans_fail_bounds_check<'blk, 't
      let filename = C_str_slice(ccx,  filename);
      let line = C_u32(ccx, loc.line as u32);
      let file_line_const = C_struct(ccx, &[filename, line], false);
-     let file_line = consts::addr_of(ccx, file_line_const,
-                                     "panic_bounds_check_loc", call_info.id);
+     let file_line = consts::addr_of(ccx, file_line_const, "panic_bounds_check_loc");
      let args = vec!(file_line, index, len);
      let did = langcall(bcx, Some(call_info.span), "", PanicBoundsCheckFnLangItem);
      let bcx = callee::trans_lang_call(bcx,
index 9efb91e0c1d736f5c7fb50864e0b06f775e794b6,a32b40dc2b640b673e70a371c580d856831b24e1..a59616d09b1c1b6a7501bef06cc40d079da98481
@@@ -196,8 -196,9 +196,9 @@@ use llvm::debuginfo::*
  use metadata::csearch;
  use middle::subst::{self, Substs};
  use trans::{self, adt, machine, type_of};
- use trans::common::{self, NodeIdAndSpan, CrateContext, FunctionContext, Block,
-                     C_bytes, NormalizingClosureTyper};
+ use trans::common::{self, NodeIdAndSpan, CrateContext, FunctionContext, Block, C_bytes,
+                     NormalizingClosureTyper};
+ use trans::declare;
  use trans::_match::{BindingInfo, TrByCopy, TrByMove, TrByRef};
  use trans::monomorphize;
  use trans::type_::Type;
@@@ -773,11 -774,11 +774,11 @@@ pub fn create_global_var_metadata(cx: &
  
      let var_item = cx.tcx().map.get(node_id);
  
 -    let (ident, span) = match var_item {
 +    let (name, span) = match var_item {
          ast_map::NodeItem(item) => {
              match item.node {
 -                ast::ItemStatic(..) => (item.ident, item.span),
 -                ast::ItemConst(..) => (item.ident, item.span),
 +                ast::ItemStatic(..) => (item.ident.name, item.span),
 +                ast::ItemConst(..) => (item.ident.name, item.span),
                  _ => {
                      cx.sess()
                        .span_bug(item.span,
      let variable_type = ty::node_id_to_type(cx.tcx(), node_id);
      let type_metadata = type_metadata(cx, variable_type, span);
      let namespace_node = namespace_for_item(cx, ast_util::local_def(node_id));
 -    let var_name = token::get_ident(ident).to_string();
 +    let var_name = token::get_name(name).to_string();
      let linkage_name =
          namespace_node.mangled_name_of_contained_item(&var_name[..]);
      let var_scope = namespace_node.scope;
@@@ -861,7 -862,7 +862,7 @@@ pub fn create_local_var_metadata(bcx: B
          let scope_metadata = scope_metadata(bcx.fcx, node_id, span);
  
          declare_local(bcx,
 -                      var_ident.node,
 +                      var_ident.node.name,
                        datum.ty,
                        scope_metadata,
                        DirectVariable { alloca: datum.val },
@@@ -889,14 -890,14 +890,14 @@@ pub fn create_captured_var_metadata<'bl
  
      let ast_item = cx.tcx().map.find(node_id);
  
 -    let variable_ident = match ast_item {
 +    let variable_name = match ast_item {
          None => {
              cx.sess().span_bug(span, "debuginfo::create_captured_var_metadata: node not found");
          }
          Some(ast_map::NodeLocal(pat)) | Some(ast_map::NodeArg(pat)) => {
              match pat.node {
                  ast::PatIdent(_, ref path1, _) => {
 -                    path1.node
 +                    path1.node.name
                  }
                  _ => {
                      cx.sess()
      };
  
      declare_local(bcx,
 -                  variable_ident,
 +                  variable_name,
                    variable_type,
                    scope_metadata,
                    variable_access,
  ///
  /// Adds the created metadata nodes directly to the crate's IR.
  pub fn create_match_binding_metadata<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
 -                                                 variable_ident: ast::Ident,
 +                                                 variable_name: ast::Name,
                                                   binding: BindingInfo<'tcx>) {
      if bcx.unreachable.get() ||
         fn_should_be_ignored(bcx.fcx) ||
      };
  
      declare_local(bcx,
 -                  variable_ident,
 +                  variable_name,
                    binding.ty,
                    scope_metadata,
                    var_access,
@@@ -1048,7 -1049,7 +1049,7 @@@ pub fn create_argument_metadata(bcx: Bl
          };
  
          declare_local(bcx,
 -                      var_ident.node,
 +                      var_ident.node.name,
                        datum.ty,
                        scope_metadata,
                        DirectVariable { alloca: datum.val },
@@@ -1283,7 -1284,7 +1284,7 @@@ pub fn create_function_debug_context<'a
  
      let fnitem = cx.tcx().map.get(fn_ast_id);
  
 -    let (ident, fn_decl, generics, top_level_block, span, has_path) = match fnitem {
 +    let (name, fn_decl, generics, top_level_block, span, has_path) = match fnitem {
          ast_map::NodeItem(ref item) => {
              if contains_nodebug_attribute(&item.attrs) {
                  return FunctionDebugContext::FunctionWithoutDebugInfo;
  
              match item.node {
                  ast::ItemFn(ref fn_decl, _, _, ref generics, ref top_level_block) => {
 -                    (item.ident, fn_decl, generics, top_level_block, item.span, true)
 +                    (item.ident.name, fn_decl, generics, top_level_block, item.span, true)
                  }
                  _ => {
                      cx.sess().span_bug(item.span,
                          return FunctionDebugContext::FunctionWithoutDebugInfo;
                      }
  
 -                    (impl_item.ident,
 +                    (impl_item.ident.name,
                       &sig.decl,
                       &sig.generics,
                       body,
              match expr.node {
                  ast::ExprClosure(_, ref fn_decl, ref top_level_block) => {
                      let name = format!("fn{}", token::gensym("fn"));
 -                    let name = token::str_to_ident(&name[..]);
 +                    let name = token::intern(&name[..]);
                      (name, fn_decl,
                          // This is not quite right. It should actually inherit
                          // the generics of the enclosing function.
                          return FunctionDebugContext::FunctionWithoutDebugInfo;
                      }
  
 -                    (trait_item.ident,
 +                    (trait_item.ident.name,
                       &sig.decl,
                       &sig.generics,
                       body,
  
      // Get_template_parameters() will append a `<...>` clause to the function
      // name if necessary.
 -    let mut function_name = String::from_str(&token::get_ident(ident));
 +    let mut function_name = String::from_str(&token::get_name(name));
      let template_parameters = get_template_parameters(cx,
                                                        generics,
                                                        param_substs,
                                                                actual_self_type,
                                                                codemap::DUMMY_SP);
  
 -                let ident = special_idents::type_self;
 +                let name = token::get_name(special_idents::type_self.name);
  
 -                let ident = token::get_ident(ident);
 -                let name = CString::new(ident.as_bytes()).unwrap();
 +                let name = CString::new(name.as_bytes()).unwrap();
                  let param_metadata = unsafe {
                      llvm::LLVMDIBuilderCreateTemplateTypeParameter(
                          DIB(cx),
@@@ -1672,7 -1674,7 +1673,7 @@@ fn compile_unit_metadata(cx: &CrateCont
  }
  
  fn declare_local<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
 -                             variable_ident: ast::Ident,
 +                             variable_name: ast::Name,
                               variable_type: Ty<'tcx>,
                               scope_metadata: DIScope,
                               variable_access: VariableAccess,
      let filename = span_start(cx, span).file.name.clone();
      let file_metadata = file_metadata(cx, &filename[..]);
  
 -    let name = token::get_ident(variable_ident);
 +    let name = token::get_name(variable_name);
      let loc = span_start(cx, span);
      let type_metadata = type_metadata(cx, variable_type, span);
  
@@@ -2260,7 -2262,7 +2261,7 @@@ impl<'tcx> EnumMemberDescriptionFactory
                  // MemberDescription of the struct's single field.
                  let sole_struct_member_description = MemberDescription {
                      name: match non_null_variant.arg_names {
 -                        Some(ref names) => token::get_ident(names[0]).to_string(),
 +                        Some(ref names) => token::get_name(names[0]).to_string(),
                          None => "".to_string()
                      },
                      llvm_type: non_null_llvm_type,
@@@ -2428,8 -2430,9 +2429,8 @@@ fn describe_enum_variant<'a, 'tcx>(cx: 
      let mut arg_names: Vec<_> = match variant_info.arg_names {
          Some(ref names) => {
              names.iter()
 -                 .map(|ident| {
 -                     token::get_ident(*ident).to_string()
 -                 }).collect()
 +                 .map(|&name| token::get_name(name).to_string())
 +                 .collect()
          }
          None => variant_info.args.iter().map(|_| "".to_string()).collect()
      };
@@@ -3243,10 -3246,11 +3244,10 @@@ fn create_scope_map(cx: &CrateContext
  
      struct ScopeStackEntry {
          scope_metadata: DIScope,
 -        ident: Option<ast::Ident>
 +        name: Option<ast::Name>
      }
  
 -    let mut scope_stack = vec!(ScopeStackEntry { scope_metadata: fn_metadata,
 -                                                 ident: None });
 +    let mut scope_stack = vec!(ScopeStackEntry { scope_metadata: fn_metadata, name: None });
      scope_map.insert(fn_ast_id, fn_metadata);
  
      // Push argument identifiers onto the stack so arguments integrate nicely
      for arg in args {
          pat_util::pat_bindings(def_map, &*arg.pat, |_, node_id, _, path1| {
              scope_stack.push(ScopeStackEntry { scope_metadata: fn_metadata,
 -                                               ident: Some(path1.node) });
 +                                               name: Some(path1.node.name) });
              scope_map.insert(node_id, fn_metadata);
          })
      }
                  loc.col.to_usize() as c_uint)
          };
  
 -        scope_stack.push(ScopeStackEntry { scope_metadata: scope_metadata,
 -                                           ident: None });
 +        scope_stack.push(ScopeStackEntry { scope_metadata: scope_metadata, name: None });
  
          inner_walk(cx, scope_stack, scope_map);
  
          // pop artificial scopes
 -        while scope_stack.last().unwrap().ident.is_some() {
 +        while scope_stack.last().unwrap().name.is_some() {
              scope_stack.pop();
          }
  
                  // scope stack and maybe introduce an artificial scope
                  if pat_util::pat_is_binding(def_map, &*pat) {
  
 -                    let ident = path1.node;
 +                    let name = path1.node.name;
  
                      // LLVM does not properly generate 'DW_AT_start_scope' fields
                      // for variable DIEs. For this reason we have to introduce
                      // variables with the same name will cause the problem.
                      let need_new_scope = scope_stack
                          .iter()
 -                        .any(|entry| entry.ident.iter().any(|i| i.name == ident.name));
 +                        .any(|entry| entry.name == Some(name));
  
                      if need_new_scope {
                          // Create a new lexical scope and push it onto the stack
  
                          scope_stack.push(ScopeStackEntry {
                              scope_metadata: scope_metadata,
 -                            ident: Some(ident)
 +                            name: Some(name)
                          });
  
                      } else {
                          let prev_metadata = scope_stack.last().unwrap().scope_metadata;
                          scope_stack.push(ScopeStackEntry {
                              scope_metadata: prev_metadata,
 -                            ident: Some(ident)
 +                            name: Some(name)
                          });
                      }
                  }
@@@ -3966,8 -3971,8 +3967,8 @@@ fn namespace_for_item(cx: &CrateContext
      ty::with_path(cx.tcx(), def_id, |path| {
          // prepend crate name if not already present
          let krate = if def_id.krate == ast::LOCAL_CRATE {
 -            let crate_namespace_ident = token::str_to_ident(crate_root_namespace(cx));
 -            Some(ast_map::PathMod(crate_namespace_ident.name))
 +            let crate_namespace_name = token::intern(crate_root_namespace(cx));
 +            Some(ast_map::PathMod(crate_namespace_name))
          } else {
              None
          };
@@@ -4067,7 -4072,7 +4068,7 @@@ pub fn insert_reference_to_gdb_debug_sc
  /// section.
  fn get_or_insert_gdb_debug_scripts_section_global(ccx: &CrateContext)
                                                    -> llvm::ValueRef {
-     let section_var_name = b"__rustc_debug_gdb_scripts_section__\0";
+     let section_var_name = "__rustc_debug_gdb_scripts_section__";
  
      let section_var = unsafe {
          llvm::LLVMGetNamedGlobal(ccx.llmod(),
          unsafe {
              let llvm_type = Type::array(&Type::i8(ccx),
                                          section_contents.len() as u64);
-             let section_var = llvm::LLVMAddGlobal(ccx.llmod(),
-                                                   llvm_type.to_ref(),
-                                                   section_var_name.as_ptr()
-                                                     as *const _);
+             let section_var = declare::define_global(ccx, section_var_name,
+                                                      llvm_type).unwrap_or_else(||{
+                 ccx.sess().bug(&format!("symbol `{}` is already defined", section_var_name))
+             });
              llvm::LLVMSetSection(section_var, section_name.as_ptr() as *const _);
              llvm::LLVMSetInitializer(section_var, C_bytes(ccx, section_contents));
              llvm::LLVMSetGlobalConstant(section_var, llvm::True);
diff --combined src/libsyntax/attr.rs
index 29b0716c8174fbebdab5bf754813e0ccf1e504b0,b966fe2c6e5cd97163757142cc7dd7494b9112da..755dd3bb4586fd72dae9b893a04d04ce8b09234e
@@@ -282,6 -282,23 +282,23 @@@ pub fn find_crate_name(attrs: &[Attribu
      first_attr_value_str_by_name(attrs, "crate_name")
  }
  
+ /// Find the value of #[export_name=*] attribute and check its validity.
+ pub fn find_export_name_attr(diag: &SpanHandler, attrs: &[Attribute]) -> Option<InternedString> {
+     attrs.iter().fold(None, |ia,attr| {
+         if attr.check_name("export_name") {
+             if let s@Some(_) = attr.value_str() {
+                 s
+             } else {
+                 diag.span_err(attr.span, "export_name attribute has invalid format");
+                 diag.handler.help("use #[export_name=\"*\"]");
+                 None
+             }
+         } else {
+             ia
+         }
+     })
+ }
  #[derive(Copy, Clone, PartialEq)]
  pub enum InlineAttr {
      None,
@@@ -503,8 -520,8 +520,8 @@@ pub fn require_unique_names(diagnostic
          let name = meta.name();
  
          if !set.insert(name.clone()) {
 -            diagnostic.span_fatal(meta.span,
 -                                  &format!("duplicate meta item `{}`", name));
 +            panic!(diagnostic.span_fatal(meta.span,
 +                                  &format!("duplicate meta item `{}`", name)));
          }
      }
  }