Skip to content

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
Contributor

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
Collaborator

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
Collaborator

bors commented Jun 8, 2017

⌛ Testing commit 28682bc with merge 4946653...

@bors
Copy link
Collaborator

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
Collaborator

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
Collaborator

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
Collaborator

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