Tell emscripten to remove exception handling code when the panic runtime is used #36900

Closed
brson opened this Issue Oct 1, 2016 · 6 comments

Comments

Projects
None yet
6 participants
@brson
Contributor

brson commented Oct 1, 2016

emcc can remove invoke instructions during final translation. So when compiling with -C panic-runtime=abort, in order to strip the unwinding code out of std, we should pass the appropriate flags to emcc. This can probably be done be setting the link_args from the panic_abort crate.

@japaric

This comment has been minimized.

Show comment
Hide comment
@japaric

japaric Oct 1, 2016

Member

This can probably be done be setting the link_args from the panic_abort crate.

If you mean #[link_args], that doesn't carry over from dependency to the top crate so it wouldn't work. And, Cargo can only inject the -l and -L variety of linker arguments. So, I think, this approach would require adding a feature to (Cargo) build scripts that lets them inject arbitrary linker arguments. (I've wanted this feature before but I've heard second-hand that @alexcrichton doesn't like the idea of injecting arbitrary linker arguments and there are some very valid resons to not do allow them but one can't deny it's an useful feature)

I think a, perhaps, more feasible alternative would be special casing this target to have rustc pass the extra, needed flags to emcc when the panic=abort profile is selected. At least this trick can be kept private to rustc.

Member

japaric commented Oct 1, 2016

This can probably be done be setting the link_args from the panic_abort crate.

If you mean #[link_args], that doesn't carry over from dependency to the top crate so it wouldn't work. And, Cargo can only inject the -l and -L variety of linker arguments. So, I think, this approach would require adding a feature to (Cargo) build scripts that lets them inject arbitrary linker arguments. (I've wanted this feature before but I've heard second-hand that @alexcrichton doesn't like the idea of injecting arbitrary linker arguments and there are some very valid resons to not do allow them but one can't deny it's an useful feature)

I think a, perhaps, more feasible alternative would be special casing this target to have rustc pass the extra, needed flags to emcc when the panic=abort profile is selected. At least this trick can be kept private to rustc.

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton Oct 3, 2016

Member

Agreed with @japaric, I don't think #[link_args] is what we want here. I'd be fine adding special knowledge for emscripten in the compiler, however.

Member

alexcrichton commented Oct 3, 2016

Agreed with @japaric, I don't think #[link_args] is what we want here. I'd be fine adding special knowledge for emscripten in the compiler, however.

@aidanhs

This comment has been minimized.

Show comment
Hide comment
@aidanhs

aidanhs Oct 13, 2016

Member

As I mentioned in the discuss thread, it'd also be nice to consider the possibility of making abort the default for emscripten.

Member

aidanhs commented Oct 13, 2016

As I mentioned in the discuss thread, it'd also be nice to consider the possibility of making abort the default for emscripten.

@DemiMarie

This comment has been minimized.

Show comment
Hide comment
@DemiMarie

DemiMarie Oct 30, 2016

Contributor

I think abort SHOULD be the default for emscripten! Exceptions in emscripten absolutely trash performance, even if no exception is ever thrown.

Contributor

DemiMarie commented Oct 30, 2016

I think abort SHOULD be the default for emscripten! Exceptions in emscripten absolutely trash performance, even if no exception is ever thrown.

@geppy

This comment has been minimized.

Show comment
Hide comment
@geppy

geppy Nov 20, 2016

I'd be interested in contributing a PR for this, using @japaric's approach. This'd be my first Rust PR, so should I look for an -E-easy issue instead?

geppy commented Nov 20, 2016

I'd be interested in contributing a PR for this, using @japaric's approach. This'd be my first Rust PR, so should I look for an -E-easy issue instead?

@brson

This comment has been minimized.

Show comment
Hide comment
@brson

brson Dec 31, 2016

Contributor

@geppy sorry for not getting back to you earlier.

It seems like we can't do @japaric's approach but need to teach the compiler how to add the arguments without #[link_args].

It looks like the logic belongs in rustc_trans::back::link::link_natively. From existing precedent I'd say that target specs should get an is_like_emscripten flag. and link_natively should do some logic when that is set and the panic strategy is abort.

Contributor

brson commented Dec 31, 2016

@geppy sorry for not getting back to you earlier.

It seems like we can't do @japaric's approach but need to teach the compiler how to add the arguments without #[link_args].

It looks like the logic belongs in rustc_trans::back::link::link_natively. From existing precedent I'd say that target specs should get an is_like_emscripten flag. and link_natively should do some logic when that is set and the panic strategy is abort.

@brson brson referenced this issue Jan 3, 2017

Closed

Tracking issue for wasm support #38805

11 of 15 tasks complete

frewsxcv added a commit to frewsxcv/rust that referenced this issue Feb 5, 2017

Rollup merge of #39193 - pepyakin:emcc-strip-panic-rt, r=alexcrichton
Tell emscripten to remove exception handling code when panic=abort

Fixes #36900

frewsxcv added a commit to frewsxcv/rust that referenced this issue Feb 5, 2017

Rollup merge of #39193 - pepyakin:emcc-strip-panic-rt, r=alexcrichton
Tell emscripten to remove exception handling code when panic=abort

Fixes #36900

@bors bors closed this in #39193 Feb 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment