Skip to content

Commit

Permalink
Auto merge of #53640 - alexcrichton:more-symbol-tweaks, r=michaelwoer…
Browse files Browse the repository at this point in the history
…ister

rustc: Continue to tweak "std internal symbols"

In investigating [an issue][1] with `panic_implementation` defined in an
executable that's optimized I once again got to rethinking a bit about the
`rustc_std_internal_symbol` attribute as well as weak lang items. We've sort of
been non-stop tweaking these items ever since their inception, and this
continues to the trend.

The crux of the bug was that in the reachability we have a [different branch][2]
for non-library builds which meant that weak lang items (and std internal
symbols) weren't considered reachable, causing them to get eliminiated by
ThinLTO passes. The fix was to basically tweak that branch to consider these
symbols to ensure that they're propagated all the way to the linker.

Along the way I've attempted to erode the distinction between std internal
symbols and weak lang items by having weak lang items automatically configure
fields of `CodegenFnAttrs`. That way most code no longer even considers weak
lang items and they're simply considered normal functions with attributes about
the ABI.

In the end this fixes the final comment of #51342

[1]: #51342 (comment)
[2]: https://github.com/rust-lang/rust/blob/35bf1ae25799a4e62131159f052e0a3cbd27c960/src/librustc/middle/reachable.rs#L225-L238
  • Loading branch information
bors committed Aug 27, 2018
2 parents 51777b1 + 0a2282e commit 3a2c603
Show file tree
Hide file tree
Showing 15 changed files with 151 additions and 35 deletions.
8 changes: 4 additions & 4 deletions src/liballoc_jemalloc/lib.rs
Expand Up @@ -89,25 +89,25 @@ mod contents {
// linkage directives are provided as part of the current compiler allocator
// ABI

#[no_mangle]
#[rustc_std_internal_symbol]
#[cfg_attr(stage0, no_mangle)]
pub unsafe extern fn __rde_alloc(size: usize, align: usize) -> *mut u8 {
let flags = align_to_flags(align, size);
let ptr = mallocx(size as size_t, flags) as *mut u8;
ptr
}

#[no_mangle]
#[rustc_std_internal_symbol]
#[cfg_attr(stage0, no_mangle)]
pub unsafe extern fn __rde_dealloc(ptr: *mut u8,
size: usize,
align: usize) {
let flags = align_to_flags(align, size);
sdallocx(ptr as *mut c_void, size, flags);
}

#[no_mangle]
#[rustc_std_internal_symbol]
#[cfg_attr(stage0, no_mangle)]
pub unsafe extern fn __rde_realloc(ptr: *mut u8,
_old_size: usize,
align: usize,
Expand All @@ -117,8 +117,8 @@ mod contents {
ptr
}

#[no_mangle]
#[rustc_std_internal_symbol]
#[cfg_attr(stage0, no_mangle)]
pub unsafe extern fn __rde_alloc_zeroed(size: usize, align: usize) -> *mut u8 {
let ptr = if align <= MIN_ALIGN && align <= size {
calloc(size as size_t, 1) as *mut u8
Expand Down
4 changes: 2 additions & 2 deletions src/libpanic_abort/lib.rs
Expand Up @@ -31,7 +31,7 @@

// Rust's "try" function, but if we're aborting on panics we just call the
// function as there's nothing else we need to do here.
#[no_mangle]
#[cfg_attr(stage0, no_mangle)]
#[rustc_std_internal_symbol]
pub unsafe extern fn __rust_maybe_catch_panic(f: fn(*mut u8),
data: *mut u8,
Expand All @@ -51,7 +51,7 @@ pub unsafe extern fn __rust_maybe_catch_panic(f: fn(*mut u8),
// which would break compat with XP. For now just use `intrinsics::abort` which
// will kill us with an illegal instruction, which will do a good enough job for
// now hopefully.
#[no_mangle]
#[cfg_attr(stage0, no_mangle)]
#[rustc_std_internal_symbol]
pub unsafe extern fn __rust_start_panic(_payload: usize) -> u32 {
abort();
Expand Down
35 changes: 35 additions & 0 deletions src/librustc/hir/mod.rs
Expand Up @@ -2287,25 +2287,59 @@ pub fn provide(providers: &mut Providers) {
#[derive(Clone, RustcEncodable, RustcDecodable)]
pub struct CodegenFnAttrs {
pub flags: CodegenFnAttrFlags,
/// Parsed representation of the `#[inline]` attribute
pub inline: InlineAttr,
/// The `#[export_name = "..."]` attribute, indicating a custom symbol a
/// function should be exported under
pub export_name: Option<Symbol>,
/// The `#[link_name = "..."]` attribute, indicating a custom symbol an
/// imported function should be imported as. Note that `export_name`
/// probably isn't set when this is set, this is for foreign items while
/// `#[export_name]` is for Rust-defined functions.
pub link_name: Option<Symbol>,
/// The `#[target_feature(enable = "...")]` attribute and the enabled
/// features (only enabled features are supported right now).
pub target_features: Vec<Symbol>,
/// The `#[linkage = "..."]` attribute and the value we found.
pub linkage: Option<Linkage>,
/// The `#[link_section = "..."]` attribute, or what executable section this
/// should be placed in.
pub link_section: Option<Symbol>,
}

bitflags! {
#[derive(RustcEncodable, RustcDecodable)]
pub struct CodegenFnAttrFlags: u32 {
/// #[cold], a hint to LLVM that this function, when called, is never on
/// the hot path
const COLD = 1 << 0;
/// #[allocator], a hint to LLVM that the pointer returned from this
/// function is never null
const ALLOCATOR = 1 << 1;
/// #[unwind], an indicator that this function may unwind despite what
/// its ABI signature may otherwise imply
const UNWIND = 1 << 2;
/// #[rust_allocator_nounwind], an indicator that an imported FFI
/// function will never unwind. Probably obsolete by recent changes with
/// #[unwind], but hasn't been removed/migrated yet
const RUSTC_ALLOCATOR_NOUNWIND = 1 << 3;
/// #[naked], indicates to LLVM that no function prologue/epilogue
/// should be generated
const NAKED = 1 << 4;
/// #[no_mangle], the function's name should be the same as its symbol
const NO_MANGLE = 1 << 5;
/// #[rustc_std_internal_symbol], and indicator that this symbol is a
/// "weird symbol" for the standard library in that it has slightly
/// different linkage, visibility, and reachability rules.
const RUSTC_STD_INTERNAL_SYMBOL = 1 << 6;
/// #[no_debug], indicates that no debugging information should be
/// generated for this function by LLVM
const NO_DEBUG = 1 << 7;
/// #[thread_local], indicates a static is actually a thread local
/// piece of memory
const THREAD_LOCAL = 1 << 8;
/// #[used], indicates that LLVM can't eliminate this function (but the
/// linker can!)
const USED = 1 << 9;
}
}
Expand All @@ -2316,6 +2350,7 @@ impl CodegenFnAttrs {
flags: CodegenFnAttrFlags::empty(),
inline: InlineAttr::None,
export_name: None,
link_name: None,
target_features: vec![],
linkage: None,
link_section: None,
Expand Down
1 change: 1 addition & 0 deletions src/librustc/ich/impls_hir.rs
Expand Up @@ -1136,6 +1136,7 @@ impl_stable_hash_for!(struct hir::CodegenFnAttrs {
flags,
inline,
export_name,
link_name,
target_features,
linkage,
link_section,
Expand Down
7 changes: 5 additions & 2 deletions src/librustc/middle/reachable.rs
Expand Up @@ -231,8 +231,11 @@ impl<'a, 'tcx> ReachableContext<'a, 'tcx> {
false
};
let def_id = self.tcx.hir.local_def_id(item.id);
let is_extern = self.tcx.codegen_fn_attrs(def_id).contains_extern_indicator();
if reachable || is_extern {
let codegen_attrs = self.tcx.codegen_fn_attrs(def_id);
let is_extern = codegen_attrs.contains_extern_indicator();
let std_internal = codegen_attrs.flags.contains(
CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL);
if reachable || is_extern || std_internal {
self.reachable_symbols.insert(search_item);
}
}
Expand Down
8 changes: 1 addition & 7 deletions src/librustc_allocator/expand.rs
Expand Up @@ -236,15 +236,9 @@ impl<'a> AllocFnFactory<'a> {
}

fn attrs(&self) -> Vec<Attribute> {
let no_mangle = Symbol::intern("no_mangle");
let no_mangle = self.cx.meta_word(self.span, no_mangle);

let special = Symbol::intern("rustc_std_internal_symbol");
let special = self.cx.meta_word(self.span, special);
vec![
self.cx.attribute(self.span, no_mangle),
self.cx.attribute(self.span, special),
]
vec![self.cx.attribute(self.span, special)]
}

fn arg_ty(
Expand Down
15 changes: 5 additions & 10 deletions src/librustc_codegen_utils/symbol_names.rs
Expand Up @@ -99,9 +99,9 @@

use rustc::hir::def_id::{DefId, LOCAL_CRATE};
use rustc::hir::map as hir_map;
use rustc::hir::CodegenFnAttrFlags;
use rustc::hir::map::definitions::DefPathData;
use rustc::ich::NodeIdHashingMode;
use rustc::middle::weak_lang_items;
use rustc::ty::item_path::{self, ItemPathBuffer, RootMode};
use rustc::ty::query::Providers;
use rustc::ty::subst::Substs;
Expand All @@ -111,7 +111,6 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use rustc_mir::monomorphize::item::{InstantiationMode, MonoItem, MonoItemExt};
use rustc_mir::monomorphize::Instance;

use syntax::attr;
use syntax_pos::symbol::Symbol;

use std::fmt::Write;
Expand Down Expand Up @@ -260,7 +259,6 @@ fn compute_symbol_name<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, instance: Instance
}

// FIXME(eddyb) Precompute a custom symbol name based on attributes.
let attrs = tcx.get_attrs(def_id);
let is_foreign = if let Some(id) = node_id {
match tcx.hir.get(id) {
hir_map::NodeForeignItem(_) => true,
Expand All @@ -270,24 +268,21 @@ fn compute_symbol_name<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, instance: Instance
tcx.is_foreign_item(def_id)
};

if let Some(name) = weak_lang_items::link_name(&attrs) {
return name.to_string();
}

let attrs = tcx.codegen_fn_attrs(def_id);
if is_foreign {
if let Some(name) = attr::first_attr_value_str_by_name(&attrs, "link_name") {
if let Some(name) = attrs.link_name {
return name.to_string();
}
// Don't mangle foreign items.
return tcx.item_name(def_id).to_string();
}

if let Some(name) = tcx.codegen_fn_attrs(def_id).export_name {
if let Some(name) = &attrs.export_name {
// Use provided name
return name.to_string();
}

if attr::contains_name(&attrs, "no_mangle") {
if attrs.flags.contains(CodegenFnAttrFlags::NO_MANGLE) {
// Don't mangle
return tcx.item_name(def_id).to_string();
}
Expand Down
1 change: 0 additions & 1 deletion src/librustc_mir/monomorphize/collector.rs
Expand Up @@ -1023,7 +1023,6 @@ impl<'b, 'a, 'v> RootCollector<'b, 'a, 'v> {
MonoItemCollectionMode::Lazy => {
self.entry_fn == Some(def_id) ||
self.tcx.is_reachable_non_generic(def_id) ||
self.tcx.is_weak_lang_item(def_id) ||
self.tcx.codegen_fn_attrs(def_id).flags.contains(
CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL)
}
Expand Down
6 changes: 2 additions & 4 deletions src/librustc_mir/monomorphize/partitioning.rs
Expand Up @@ -516,10 +516,8 @@ fn mono_item_visibility(
// visibility below. Like the weak lang items, though, we can't let
// LLVM internalize them as this decision is left up to the linker to
// omit them, so prevent them from being internalized.
let codegen_fn_attrs = tcx.codegen_fn_attrs(def_id);
let std_internal_symbol = codegen_fn_attrs.flags
.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL);
if tcx.is_weak_lang_item(def_id) || std_internal_symbol {
let attrs = tcx.codegen_fn_attrs(def_id);
if attrs.flags.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL) {
*can_be_internalized = false;
}

Expand Down
23 changes: 23 additions & 0 deletions src/librustc_typeck/collect.rs
Expand Up @@ -29,6 +29,7 @@ use constrained_type_params as ctp;
use lint;
use middle::lang_items::SizedTraitLangItem;
use middle::resolve_lifetime as rl;
use middle::weak_lang_items;
use rustc::mir::mono::Linkage;
use rustc::ty::query::Providers;
use rustc::ty::subst::Substs;
Expand Down Expand Up @@ -2281,6 +2282,8 @@ fn codegen_fn_attrs<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, id: DefId) -> Codegen
codegen_fn_attrs.link_section = Some(val);
}
}
} else if attr.check_name("link_name") {
codegen_fn_attrs.link_name = attr.value_str();
}
}

Expand All @@ -2300,5 +2303,25 @@ fn codegen_fn_attrs<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, id: DefId) -> Codegen
}
}

// Weak lang items have the same semantics as "std internal" symbols in the
// sense that they're preserved through all our LTO passes and only
// strippable by the linker.
//
// Additionally weak lang items have predetermined symbol names.
if tcx.is_weak_lang_item(id) {
codegen_fn_attrs.flags |= CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL;
}
if let Some(name) = weak_lang_items::link_name(&attrs) {
codegen_fn_attrs.export_name = Some(name);
codegen_fn_attrs.link_name = Some(name);
}

// Internal symbols to the standard library all have no_mangle semantics in
// that they have defined symbol names present in the function name. This
// also applies to weak symbols where they all have known symbol names.
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL) {
codegen_fn_attrs.flags |= CodegenFnAttrFlags::NO_MANGLE;
}

codegen_fn_attrs
}
8 changes: 4 additions & 4 deletions src/libstd/alloc.rs
Expand Up @@ -150,23 +150,23 @@ pub mod __default_lib_allocator {
// linkage directives are provided as part of the current compiler allocator
// ABI

#[no_mangle]
#[rustc_std_internal_symbol]
#[cfg_attr(stage0, no_mangle)]
pub unsafe extern fn __rdl_alloc(size: usize, align: usize) -> *mut u8 {
let layout = Layout::from_size_align_unchecked(size, align);
System.alloc(layout)
}

#[no_mangle]
#[rustc_std_internal_symbol]
#[cfg_attr(stage0, no_mangle)]
pub unsafe extern fn __rdl_dealloc(ptr: *mut u8,
size: usize,
align: usize) {
System.dealloc(ptr, Layout::from_size_align_unchecked(size, align))
}

#[no_mangle]
#[rustc_std_internal_symbol]
#[cfg_attr(stage0, no_mangle)]
pub unsafe extern fn __rdl_realloc(ptr: *mut u8,
old_size: usize,
align: usize,
Expand All @@ -175,8 +175,8 @@ pub mod __default_lib_allocator {
System.realloc(ptr, old_layout, new_size)
}

#[no_mangle]
#[rustc_std_internal_symbol]
#[cfg_attr(stage0, no_mangle)]
pub unsafe extern fn __rdl_alloc_zeroed(size: usize, align: usize) -> *mut u8 {
let layout = Layout::from_size_align_unchecked(size, align);
System.alloc_zeroed(layout)
Expand Down
16 changes: 16 additions & 0 deletions src/test/run-make/wasm-symbols-not-imported/Makefile
@@ -0,0 +1,16 @@
-include ../../run-make-fulldeps/tools.mk

ifeq ($(TARGET),wasm32-unknown-unknown)
all:
$(RUSTC) foo.rs --target wasm32-unknown-unknown
$(NODE) verify-no-imports.js $(TMPDIR)/foo.wasm
$(RUSTC) foo.rs --target wasm32-unknown-unknown -C lto
$(NODE) verify-no-imports.js $(TMPDIR)/foo.wasm
$(RUSTC) foo.rs --target wasm32-unknown-unknown -O
$(NODE) verify-no-imports.js $(TMPDIR)/foo.wasm
$(RUSTC) foo.rs --target wasm32-unknown-unknown -O -C lto
$(NODE) verify-no-imports.js $(TMPDIR)/foo.wasm
else
all:
endif

26 changes: 26 additions & 0 deletions src/test/run-make/wasm-symbols-not-imported/foo.rs
@@ -0,0 +1,26 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![crate_type = "cdylib"]

#![feature(panic_implementation)]
#![no_std]

use core::panic::PanicInfo;

#[no_mangle]
pub extern fn foo() {
panic!()
}

#[panic_implementation]
fn panic(_info: &PanicInfo) -> ! {
loop {}
}
20 changes: 20 additions & 0 deletions src/test/run-make/wasm-symbols-not-imported/verify-no-imports.js
@@ -0,0 +1,20 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

const fs = require('fs');
const process = require('process');
const assert = require('assert');
const buffer = fs.readFileSync(process.argv[2]);

let m = new WebAssembly.Module(buffer);
let list = WebAssembly.Module.imports(m);
console.log('imports', list);
if (list.length !== 0)
throw new Error("there are some imports");
8 changes: 7 additions & 1 deletion src/test/ui/panic-handler/panic-handler-std.stderr
Expand Up @@ -8,6 +8,12 @@ LL | | }
|
= note: first defined in crate `std`.

error: aborting due to previous error
error: argument should be `&PanicInfo`
--> $DIR/panic-handler-std.rs:18:16
|
LL | fn panic(info: PanicInfo) -> ! {
| ^^^^^^^^^

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0152`.

0 comments on commit 3a2c603

Please sign in to comment.