From e6aa92c4325e3da9a698010bb88e3cdb240ce142 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 21 Oct 2016 12:41:20 -0400 Subject: [PATCH 1/3] trans: Make names of internal symbols independent of CGU translation order. Every codegen unit gets its own local counter for generating new symbol names. This makes bitcode and object files reproducible at the binary level even when incremental compilation is used. --- src/librustc_trans/common.rs | 4 +--- src/librustc_trans/consts.rs | 6 +----- src/librustc_trans/context.rs | 14 ++++++++++++++ 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/librustc_trans/common.rs b/src/librustc_trans/common.rs index 6ae5fc1657a..76b778fb61f 100644 --- a/src/librustc_trans/common.rs +++ b/src/librustc_trans/common.rs @@ -799,9 +799,7 @@ pub fn C_cstr(cx: &CrateContext, s: InternedString, null_terminated: bool) -> Va s.as_ptr() as *const c_char, s.len() as c_uint, !null_terminated as Bool); - - let gsym = token::gensym("str"); - let sym = format!("str{}", gsym.0); + let sym = cx.generate_local_symbol_name("str"); let g = declare::define_global(cx, &sym[..], val_ty(sc)).unwrap_or_else(||{ bug!("symbol `{}` is already defined", sym); }); diff --git a/src/librustc_trans/consts.rs b/src/librustc_trans/consts.rs index 15f7132e52d..0dc10aa7759 100644 --- a/src/librustc_trans/consts.rs +++ b/src/librustc_trans/consts.rs @@ -30,7 +30,6 @@ use rustc::hir; use std::ffi::{CStr, CString}; use syntax::ast; use syntax::attr; -use syntax::parse::token; pub fn ptrcast(val: ValueRef, ty: Type) -> ValueRef { unsafe { @@ -44,10 +43,7 @@ pub fn addr_of_mut(ccx: &CrateContext, kind: &str) -> ValueRef { unsafe { - // FIXME: this totally needs a better name generation scheme, perhaps a simple global - // counter? Also most other uses of gensym in trans. - let gsym = token::gensym("_"); - let name = format!("{}{}", kind, gsym.0); + let name = ccx.generate_local_symbol_name(kind); let gv = declare::define_global(ccx, &name[..], val_ty(cv)).unwrap_or_else(||{ bug!("symbol `{}` is already defined", name); }); diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index 1b67516a9e6..2a72d42296d 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -166,6 +166,9 @@ pub struct LocalCrateContext<'tcx> { type_of_depth: Cell, symbol_map: Rc>, + + /// A counter that is used for generating local symbol names + local_gen_sym_counter: Cell, } // Implement DepTrackingMapConfig for `trait_cache` @@ -688,6 +691,7 @@ impl<'tcx> LocalCrateContext<'tcx> { n_llvm_insns: Cell::new(0), type_of_depth: Cell::new(0), symbol_map: symbol_map, + local_gen_sym_counter: Cell::new(0), }; let (int_type, opaque_vec_type, str_slice_ty, mut local_ccx) = { @@ -1021,6 +1025,16 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> { pub fn empty_substs_for_def_id(&self, item_def_id: DefId) -> &'tcx Substs<'tcx> { self.shared().empty_substs_for_def_id(item_def_id) } + + /// Generate a new symbol name with the given prefix. This symbol name must + /// only be used for definitions with `internal` or `private` linkage. + pub fn generate_local_symbol_name(&self, prefix: &str) -> String { + let idx = self.local().local_gen_sym_counter.get(); + self.local().local_gen_sym_counter.set(idx + 1); + // Include a '.' character, so there can be no accidental conflicts with + // user defined names + format!("{}.{}", prefix, idx) + } } pub struct TypeOfDepthLock<'a, 'tcx: 'a>(&'a LocalCrateContext<'tcx>); From e46c1ad9c94fcc75d62805a9b0f17333b67e99c1 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Fri, 21 Oct 2016 18:29:25 -0400 Subject: [PATCH 2/3] Adapt codegen test to new naming scheme for generated symbols. --- src/test/codegen/consts.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/codegen/consts.rs b/src/test/codegen/consts.rs index 36a582ca737..33b4221b733 100644 --- a/src/test/codegen/consts.rs +++ b/src/test/codegen/consts.rs @@ -19,12 +19,12 @@ // CHECK: @STATIC = {{.*}}, align 4 // This checks the constants from inline_enum_const -// CHECK: @ref{{[0-9]+}} = {{.*}}, align 2 +// CHECK: @ref.{{[0-9]+}} = {{.*}}, align 2 // This checks the constants from {low,high}_align_const, they share the same // constant, but the alignment differs, so the higher one should be used -// CHECK: [[LOW_HIGH:@ref[0-9]+]] = {{.*}}, align 4 -// CHECK: [[LOW_HIGH_REF:@const[0-9]+]] = {{.*}} [[LOW_HIGH]] +// CHECK: [[LOW_HIGH:@ref.[0-9]+]] = {{.*}}, align 4 +// CHECK: [[LOW_HIGH_REF:@const.[0-9]+]] = {{.*}} [[LOW_HIGH]] #[derive(Copy, Clone)] From 992203b9764c7e4e48ca454aa3162ee7d73cb87c Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Mon, 24 Oct 2016 11:07:45 -0400 Subject: [PATCH 3/3] Adapt rmake-test to new naming scheme for internal symbols. --- src/test/run-make/symbols-are-reasonable/Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/run-make/symbols-are-reasonable/Makefile b/src/test/run-make/symbols-are-reasonable/Makefile index c668ffc5832..1c117cf0c9e 100644 --- a/src/test/run-make/symbols-are-reasonable/Makefile +++ b/src/test/run-make/symbols-are-reasonable/Makefile @@ -1,7 +1,7 @@ -include ../tools.mk # check that the compile generated symbols for strings, binaries, -# vtables, etc. have semisane names (e.g. `str1234`); it's relatively +# vtables, etc. have semisane names (e.g. `str.1234`); it's relatively # easy to accidentally modify the compiler internals to make them # become things like `str"str"(1234)`. @@ -10,6 +10,6 @@ OUT=$(TMPDIR)/lib.s all: $(RUSTC) lib.rs --emit=asm --crate-type=staticlib # just check for symbol declarations with the names we're expecting. - grep 'str[0-9][0-9]*:' $(OUT) - grep 'byte_str[0-9][0-9]*:' $(OUT) - grep 'vtable[0-9][0-9]*' $(OUT) + grep 'str.[0-9][0-9]*:' $(OUT) + grep 'byte_str.[0-9][0-9]*:' $(OUT) + grep 'vtable.[0-9][0-9]*' $(OUT)