Skip to content

Commit

Permalink
Stop using C++ exceptions for stack unwinding.
Browse files Browse the repository at this point in the history
  • Loading branch information
@vadimcn
vadimcn committed Dec 21, 2013
1 parent ba801d8 commit 5d53fd7
Show file tree
Hide file tree
Showing 14 changed files with 261 additions and 157 deletions.
22 changes: 13 additions & 9 deletions mk/rt.mk
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,16 @@ RUNTIME_CXXFLAGS_$(1)_$(2) = -D_RUST_STAGE1
endif
endif

RUNTIME_CXXS_$(1)_$(2) := \
rt/rust_cxx_glue.cpp

RUNTIME_CS_$(1)_$(2) := \
rt/rust_builtin.c \
rt/rust_upcall.c \
rt/miniz.c \
rt/rust_android_dummy.c \
rt/rust_test_helpers.c

RUNTIME_LL_$(1)_$(2) := \
rt/rust_try.ll

This comment has been minimized.

Sign in to view

Copy link
@alexcrichton

alexcrichton Dec 22, 2013

This is pretty slick.

# stage0 remove this after the next snapshot
%.cpp:
@touch tmp/foo.o
Expand All @@ -94,18 +94,17 @@ RT_BUILD_DIR_$(1)_$(2) := $$(RT_OUTPUT_DIR_$(1))/stage$(2)
RUNTIME_DEF_$(1)_$(2) := $$(RT_OUTPUT_DIR_$(1))/rustrt$$(CFG_DEF_SUFFIX_$(1))
RUNTIME_INCS_$(1)_$(2) := -I $$(S)src/rt -I $$(S)src/rt/isaac -I $$(S)src/rt/uthash \
-I $$(S)src/rt/arch/$$(HOST_$(1))
RUNTIME_OBJS_$(1)_$(2) := $$(RUNTIME_CXXS_$(1)_$(2):rt/%.cpp=$$(RT_BUILD_DIR_$(1)_$(2))/%.o) \
RUNTIME_OBJS_$(1)_$(2) := \
$$(RUNTIME_CS_$(1)_$(2):rt/%.c=$$(RT_BUILD_DIR_$(1)_$(2))/%.o) \
$$(RUNTIME_S_$(1)_$(2):rt/%.S=$$(RT_BUILD_DIR_$(1)_$(2))/%.o)
$$(RUNTIME_S_$(1)_$(2):rt/%.S=$$(RT_BUILD_DIR_$(1)_$(2))/%.o) \
$$(RUNTIME_LL_$(1)_$(2):rt/%.ll=$$(RT_BUILD_DIR_$(1)_$(2))/%.o)

ALL_OBJ_FILES += $$(RUNTIME_OBJS_$(1)_$(2))

MORESTACK_OBJS_$(1)_$(2) := $$(RT_BUILD_DIR_$(1)_$(2))/arch/$$(HOST_$(1))/morestack.o
ALL_OBJ_FILES += $$(MORESTACK_OBJS_$(1)_$(2))

$$(RT_BUILD_DIR_$(1)_$(2))/rust_cxx_glue.o: rt/rust_cxx_glue.cpp $$(MKFILE_DEPS)
@$$(call E, compile: $$@)
$$(Q)$$(call CFG_COMPILE_CXX_$(1), $$@, $$(RUNTIME_INCS_$(1)_$(2)) \
$$(SNAP_DEFINES) $$(RUNTIME_CXXFLAGS_$(1)_$(2))) $$<
LLVM_LLC = $(CFG_LLVM_INST_DIR_$(CFG_BUILD))/bin/llc -filetype=obj -o $$1 $$2

$$(RT_BUILD_DIR_$(1)_$(2))/%.o: rt/%.c $$(MKFILE_DEPS)
@$$(call E, compile: $$@)
Expand All @@ -117,6 +116,11 @@ $$(RT_BUILD_DIR_$(1)_$(2))/%.o: rt/%.S $$(MKFILE_DEPS) \
@$$(call E, compile: $$@)
$$(Q)$$(call CFG_ASSEMBLE_$(1),$$@,$$<)

$$(RT_BUILD_DIR_$(1)_$(2))/%.o: rt/%.ll $$(MKFILE_DEPS) \
$$(LLVM_CONFIG_$$(CFG_BUILD))
@$$(call E, compile: $$@)
$$(Q)$$(call LLVM_LLC,$$@,$$<)

This comment has been minimized.

Sign in to view

Copy link
@alexcrichton

alexcrichton Dec 22, 2013

This doesn't look like it would work with cross compilations. I agree that the llc used should always be the CFG_BUILD llc (so we're guaranteed that we can run it), but I think that this should pass through the target triple ($(1) I think) to llc.

This comment has been minimized.

Sign in to view

Copy link
@vadimcn

vadimcn Dec 22, 2013

Author Owner

Can you please elaborate a bit on what'd be the proper way of doing this? Rust's makefiles still baffle me sometimes.

This comment has been minimized.

Sign in to view

Copy link
@alexcrichton

alexcrichton Dec 22, 2013

I would make this rule something like this:

// put this in the "LLVM macros" section of Makefile.in
LLVM_LLC_$(1):=$$(CFG_LLVM_INST_DIR_$(1))/bin/llc$$(X_$(1))

// put this in mk/rt.mk
$$(...)/%.o: rt/%.ll $(...) $(...)
        $$(Q)$(LLVM_LLC_$(CFG_BUILD)) -filetype=obj -o $$@ $$< -target-triple=$(1)

Or at least something along the lines of that

$$(RT_BUILD_DIR_$(1)_$(2))/arch/$$(HOST_$(1))/libmorestack.a: $$(MORESTACK_OBJS_$(1)_$(2))
@$$(call E, link: $$@)
$$(Q)$(AR_$(1)) rcs $$@ $$^
Expand Down
7 changes: 7 additions & 0 deletions src/etc/mklldeps.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@
for lib in out.strip().split(' '):
lib = lib[2:] # chop of the leading '-l'
f.write("#[link(name = \"" + lib + "\", kind = \"static\")]\n")

# LLVM depends on C++ runtime, so we link it in (statically).
# Because our linker driver is gcc, not g++, it won't understand the -static-libstdc++ option,
# so we need to pass "-Bstatic -lstdc++ -Bdynamic" directly to ld.
f.write("#[link_args = \"-Xlinker -Bstatic -Xlinker -lstdc++ -Xlinker -Bdynamic\"]\n")

if os == 'win32':
f.write("#[link(name = \"imagehlp\")]\n")

f.write("extern {}\n")
13 changes: 8 additions & 5 deletions src/librustc/back/link.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -702,10 +702,7 @@ pub fn get_cc_prog(sess: Session) -> ~str {
// In the future, FreeBSD will use clang as default compiler.
// It would be flexible to use cc (system's default C compiler)
// instead of hard-coded gcc.
// For win32, there is no cc command, so we add a condition to make it use
// g++. We use g++ rather than gcc because it automatically adds linker
// options required for generation of dll modules that correctly register
// stack unwind tables.
// For win32, there is no cc command, so we add a condition to make it use gcc.
match sess.targ_cfg.os {
abi::OsAndroid => match sess.opts.android_cross_path {
Some(ref path) => format!("{}/bin/arm-linux-androideabi-gcc", *path),
Expand All @@ -714,7 +711,7 @@ pub fn get_cc_prog(sess: Session) -> ~str {
(--android-cross-path)")
}
},
abi::OsWin32 => ~"g++",
abi::OsWin32 => ~"gcc",
_ => ~"cc",
}
}
Expand Down Expand Up @@ -1023,6 +1020,12 @@ fn link_args(sess: Session,
}
}

if sess.targ_cfg.os == abi::OsWin32 {
// Make sure that we link to the dynamic libgcc, otherwise cross-module
// DWARF stack unwinding will not work.
args.push(~"-shared-libgcc");

This comment has been minimized.

Sign in to view

Copy link
@alexcrichton

alexcrichton Dec 22, 2013

Did you run into problems where libgcc was linked statically to some libraries? The only way that this can be a problem is if libgcc is linked statically to a dll and then that dll is later linked to a binary which has another copy of libgcc.

I'm not sure if there's a benefit, but we could special case and allow static executables to get a static libgcc (if ld decides to do so), but it's also probably too much work.

I guess my question here is what ld's behavior is on linking libgcc? Does it prefer static or dynamic or does it depend on what's being generated? Overall though, I think this is fine.

This comment has been minimized.

Sign in to view

Copy link
@vadimcn

vadimcn Dec 22, 2013

Author Owner

There should be only one copy of __register_frame_info function (which does unwind info registration) in the process, or cross-module unwinding will not work. Or, more precisely, it will work, but only between modules that invoked the same copy of it.

In some cases it might be OK to link libgcc statically,- if the rest of Rust runtime and libs are linked statically as well. However, even then it's possible to mess things up by dynamically loading modules that contain unwind info.

If you remember, we've discussed this issue on IRC a couple of days ago, but did not arrive at any definite conclusion. So I've decided not to tackle this problem in this PR.
}

add_local_native_libraries(&mut args, sess);
add_upstream_rust_crates(&mut args, sess, dylib, tmpdir);
add_upstream_native_libraries(&mut args, sess);
Expand Down
35 changes: 0 additions & 35 deletions src/librustc/back/upcall.rs
View file

This file was deleted.

3 changes: 1 addition & 2 deletions src/librustc/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
#[crate_type = "dylib"];
#[crate_type = "rlib"];

#[feature(macro_rules, globs, struct_variant, managed_boxes)];
#[feature(macro_rules, globs, struct_variant, managed_boxes, link_args)];

This comment has been minimized.

Sign in to view

Copy link
@alexcrichton

alexcrichton Dec 22, 2013

It seems a little odd to me that we're forcing windows to use a static libstdc++ when every other platform is still using a dynamic version. I can see the appeal of removing dependencies from the snapshot, but it looks like we're stuck with the snapshot dependency of libgcc_s and I'm not sure that there's much difference between 1 dependency and 2 dependencies.

Is there another reason to link libstdc++ statically? If not, I think I'd rather not opt in to using link_args and stick with a dynamic libstdc++ like we have today.

This comment has been minimized.

Sign in to view

Copy link
@vadimcn

vadimcn Dec 22, 2013

Author Owner

Sure, we can link dynamically, but I had thought you wanted to empty winnt_runtime_deps?

And, actually, my change to link sdtc++ statically applies to all platforms. This isn't specific to win32.

This comment has been minimized.

Sign in to view

Copy link
@alexcrichton

alexcrichton Dec 22, 2013

I would love to remove all the deps! Do you know of a path forward to remove libgcc_s though?

I'm a little hesitant to link libstdc++ statically everywhere, but it seems like it should be possible. For now though, let's stick to a dynamic libstdc++ for just librustc.

This comment has been minimized.

Sign in to view

Copy link
@vadimcn

vadimcn Dec 22, 2013

Author Owner

Yes, I can think of a couple of ways to get rid of libgcc_s. Unfortunately, they all have drawbacks, so I am not sure if they'd be worth it.
Ok, I'll switch back to dynamic libstdc++ for the time being.

extern mod extra;
extern mod syntax;
Expand Down Expand Up @@ -92,7 +92,6 @@ pub mod back {
pub mod link;
pub mod manifest;
pub mod abi;
pub mod upcall;
pub mod arm;
pub mod mips;
pub mod x86;
Expand Down
1 change: 0 additions & 1 deletion src/librustc/lib/llvm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,6 @@ pub mod llvm {
// automatically updated whenever LLVM is updated to include an up-to-date
// set of the libraries we need to link to LLVM for.
#[link(name = "rustllvm", kind = "static")]
#[link(name = "stdc++")]
extern {
/* Create and destroy contexts. */
pub fn LLVMContextCreate() -> ContextRef;
Expand Down
4 changes: 3 additions & 1 deletion src/librustc/middle/lang_items.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ pub fn collect_language_items(crate: &ast::Crate,
}

lets_do_this! {
There are 42 lang items.
There are 43 lang items.

// ID, Variant name, Name, Method name;
0, FreezeTraitLangItem, "freeze", freeze_trait;
Expand Down Expand Up @@ -261,5 +261,7 @@ lets_do_this! {
40, EventLoopFactoryLangItem, "event_loop_factory", event_loop_factory;

41, TypeIdLangItem, "type_id", type_id;

42, EhPersonalityLangItem, "eh_personality", eh_personality_fn;
}

10 changes: 6 additions & 4 deletions src/librustc/middle/trans/base.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use metadata::{csearch, cstore, encoder};
use middle::astencode;
use middle::lang_items::{LangItem, ExchangeMallocFnLangItem, StartFnLangItem};
use middle::lang_items::{MallocFnLangItem, ClosureExchangeMallocFnLangItem};
use middle::lang_items::{EhPersonalityLangItem};
use middle::trans::_match;
use middle::trans::adt;
use middle::trans::base;
Expand Down Expand Up @@ -1028,10 +1029,10 @@ pub fn get_landing_pad(bcx: @mut Block) -> BasicBlockRef {
// this represents but it's determined by the personality function and
// this is what the EH proposal example uses.
let llretty = Type::struct_([Type::i8p(), Type::i32()], false);
// The exception handling personality function. This is the C++
// personality function __gxx_personality_v0, wrapped in our naming
// convention.
let personality = bcx.ccx().upcalls.rust_personality;
// The exception handling personality function.
let personality = callee::trans_fn_ref(bcx,
langcall(bcx, None, "", EhPersonalityLangItem),
0).llfn;
// The only landing pad clause will be 'cleanup'
let llretval = LandingPad(pad_bcx, llretty, personality, 1u);
// The landing pad block is a cleanup
Expand Down Expand Up @@ -3197,6 +3198,7 @@ pub fn trans_crate(sess: session::Session,
reachable.push(ccx.crate_map_name.to_owned());
reachable.push(~"main");
reachable.push(~"rust_stack_exhausted");
reachable.push(~"eh_rust_personality_catch"); // referenced from rt/rust_try.ll

return CrateTranslation {
context: llcx,
Expand Down
3 changes: 0 additions & 3 deletions src/librustc/middle/trans/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
// except according to those terms.


use back::{upcall};
use driver::session;
use lib::llvm::{ContextRef, ModuleRef, ValueRef};
use lib::llvm::{llvm, TargetData, TypeNames};
Expand Down Expand Up @@ -105,7 +104,6 @@ pub struct CrateContext {
tcx: ty::ctxt,
maps: astencode::Maps,
stats: @mut Stats,
upcalls: @upcall::Upcalls,
tydesc_type: Type,
int_type: Type,
opaque_vec_type: Type,
Expand Down Expand Up @@ -233,7 +231,6 @@ impl CrateContext {
llvm_insns: HashMap::new(),
fn_stats: ~[]
},
upcalls: upcall::declare_upcalls(targ_cfg, llmod),
tydesc_type: tydesc_type,
int_type: int_type,
opaque_vec_type: opaque_vec_type,
Expand Down
3 changes: 3 additions & 0 deletions src/libstd/rt/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,9 @@ mod local_ptr;
/// Bindings to pthread/windows thread-local storage.
mod thread_local_storage;

/// Stack unwinding
pub mod unwind;

/// Just stuff
mod util;

Expand Down
68 changes: 2 additions & 66 deletions src/libstd/rt/task.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@ use super::local_heap::LocalHeap;
use prelude::*;

use borrow;
use cast::transmute;
use cleanup;
use io::Writer;
use libc::{c_void, uintptr_t, c_char, size_t};
use libc::{c_char, size_t};
use local_data;
use option::{Option, Some, None};
use rt::borrowck::BorrowRecord;
Expand All @@ -33,8 +32,8 @@ use rt::local::Local;
use rt::logging::StdErrLogger;
use rt::sched::{Scheduler, SchedHandle};
use rt::stack::{StackSegment, StackPool};
use rt::unwind::Unwinder;
use send_str::SendStr;
use task::TaskResult;
use unstable::finally::Finally;
use unstable::mutex::Mutex;

Expand Down Expand Up @@ -91,21 +90,6 @@ pub enum SchedHome {
pub struct GarbageCollector;
pub struct LocalStorage(Option<local_data::Map>);

pub struct Unwinder {
unwinding: bool,
cause: Option<~Any>
}

impl Unwinder {
fn result(&mut self) -> TaskResult {
if self.unwinding {
Err(self.cause.take().unwrap())
} else {
Ok(())
}
}
}

impl Task {

// A helper to build a new task using the dynamically found
Expand Down Expand Up @@ -452,54 +436,6 @@ impl Coroutine {

}


// Just a sanity check to make sure we are catching a Rust-thrown exception
static UNWIND_TOKEN: uintptr_t = 839147;

impl Unwinder {
pub fn try(&mut self, f: ||) {
use unstable::raw::Closure;

unsafe {
let closure: Closure = transmute(f);
let code = transmute(closure.code);
let env = transmute(closure.env);

let token = rust_try(try_fn, code, env);
assert!(token == 0 || token == UNWIND_TOKEN);
}

extern fn try_fn(code: *c_void, env: *c_void) {
unsafe {
let closure: Closure = Closure {
code: transmute(code),
env: transmute(env),
};
let closure: || = transmute(closure);
closure();
}
}

extern {
fn rust_try(f: extern "C" fn(*c_void, *c_void),
code: *c_void,
data: *c_void) -> uintptr_t;
}
}

pub fn begin_unwind(&mut self, cause: ~Any) -> ! {
self.unwinding = true;
self.cause = Some(cause);
unsafe {
rust_begin_unwind(UNWIND_TOKEN);
return transmute(());
}
extern {
fn rust_begin_unwind(token: uintptr_t);
}
}
}

/// This function is invoked from rust's current __morestack function. Segmented
/// stacks are currently not enabled as segmented stacks, but rather one giant
/// stack segment. This means that whenever we run out of stack, we want to
Expand Down
Loading

0 comments on commit 5d53fd7

Please sign in to comment.