Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std: Avoid panics in rust_eh_personality #42487

Merged
merged 1 commit into from
Jun 8, 2017

Conversation

alexcrichton
Copy link
Member

This commit removes a few calls to panic and/or assert in rust_eh_personality.
This function definitely can't itself panic (that'd probably segfault or do
something else weird) and I was also noticing that a pub extern fn foo() {}
cdylib was abnormally large. Turns out all that size was the panicking machinery
brought in by the personality function!

The change here is to return a Result internally so we can bubble up the fatal
error, eventually translating to the appropriate error code for the libunwind
ABI.

@rust-highfive
Copy link
Collaborator

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member Author

r? @vadimcn

@rust-highfive rust-highfive assigned vadimcn and unassigned aturon Jun 6, 2017
@vadimcn
Copy link
Contributor

vadimcn commented Jun 6, 2017

Looks good!

@bors: r+

@bors
Copy link
Contributor

bors commented Jun 6, 2017

📌 Commit 28682bc has been approved by vadimcn

@aidanhs aidanhs added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 7, 2017
@bors
Copy link
Contributor

bors commented Jun 8, 2017

⌛ Testing commit 28682bc with merge 4946653...

@bors
Copy link
Contributor

bors commented Jun 8, 2017

💔 Test failed - status-appveyor

@kennytm
Copy link
Member

kennytm commented Jun 8, 2017

x86_64-pc-windows-gnu build failure.

error[E0308]: mismatched types
   --> src\libpanic_unwind\seh64_gnu.rs:131:9
    |
131 |         EHAction::None => None,
    |         ^^^^^^^^^^^^^^ expected enum `core::result::Result`, found enum `dwarf::eh::EHAction`
    |
    = note: expected type `core::result::Result<dwarf::eh::EHAction, ()>`
               found type `dwarf::eh::EHAction`

This commit removes a few calls to panic and/or assert in `rust_eh_personality`.
This function definitely can't itself panic (that'd probably segfault or do
something else weird) and I was also noticing that a `pub extern fn foo() {}`
cdylib was abnormally large. Turns out all that size was the panicking machinery
brought in by the personality function!

The change here is to return a `Result` internally so we can bubble up the fatal
error, eventually translating to the appropriate error code for the libunwind
ABI.
@alexcrichton alexcrichton force-pushed the smaller-personality branch from 28682bc to 52805d2 Compare June 8, 2017 14:06
@alexcrichton
Copy link
Member Author

@bors: r=vadimcn

@bors
Copy link
Contributor

bors commented Jun 8, 2017

📌 Commit 52805d2 has been approved by vadimcn

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Jun 8, 2017
…=vadimcn

std: Avoid panics in rust_eh_personality

This commit removes a few calls to panic and/or assert in `rust_eh_personality`.
This function definitely can't itself panic (that'd probably segfault or do
something else weird) and I was also noticing that a `pub extern fn foo() {}`
cdylib was abnormally large. Turns out all that size was the panicking machinery
brought in by the personality function!

The change here is to return a `Result` internally so we can bubble up the fatal
error, eventually translating to the appropriate error code for the libunwind
ABI.
bors added a commit that referenced this pull request Jun 8, 2017
Rollup of 6 pull requests

- Successful merges: #42307, #42385, #42487, #42491, #42521, #42531
- Failed merges:
@bors
Copy link
Contributor

bors commented Jun 8, 2017

⌛ Testing commit 52805d2 with merge 148e917...

bors added a commit that referenced this pull request Jun 8, 2017
std: Avoid panics in rust_eh_personality

This commit removes a few calls to panic and/or assert in `rust_eh_personality`.
This function definitely can't itself panic (that'd probably segfault or do
something else weird) and I was also noticing that a `pub extern fn foo() {}`
cdylib was abnormally large. Turns out all that size was the panicking machinery
brought in by the personality function!

The change here is to return a `Result` internally so we can bubble up the fatal
error, eventually translating to the appropriate error code for the libunwind
ABI.
@bors
Copy link
Contributor

bors commented Jun 8, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: vadimcn
Pushing 148e917 to master...

@bors bors merged commit 52805d2 into rust-lang:master Jun 8, 2017
@alexcrichton alexcrichton deleted the smaller-personality branch June 14, 2017 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants