Skip to content

Implement rust_eh_personality in Rust, remove rust_eh_personality_catch. - #34832

Merged
bors merged 1 commit into
rust-lang:masterfrom
vadimcn:rusty-personality
Jul 23, 2016
Merged

Implement rust_eh_personality in Rust, remove rust_eh_personality_catch.#34832
bors merged 1 commit into
rust-lang:masterfrom
vadimcn:rusty-personality

Conversation

@vadimcn

Copy link
Copy Markdown
Contributor

Well, not quite: ARM EHABI platforms still use the old scheme -- for now.

r? @alexcrichton

@vadimcn

Copy link
Copy Markdown
ContributorAuthor

cc #34786

@bors

bors commented Jul 15, 2016

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #34819) made this pull request unmergeable. Please resolve the merge conflicts.

@vadimcn

Copy link
Copy Markdown
ContributorAuthor

@vhbit: I hope I got SjLj stuff right.

Comment threadsrc/librustc_trans/intrinsic.rs Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to be looking at sess rather than cfg! (otherwise this'll always return false)

@alexcrichton

Copy link
Copy Markdown
Member

Awesome work @vadimcn! This looks quite solid.

@vadimcn

Copy link
Copy Markdown
ContributorAuthor

r?

@alexcrichton

Copy link
Copy Markdown
Member

Looks good to me, but the travis test failure looks worrisome?

@vadimcn

Copy link
Copy Markdown
ContributorAuthor

but the travis test failure looks worrisome?

It does, but all tests pass locally (Linux 64, OSX 64 and Windows 64). I'd have liked to run this PR on try...

@alexcrichton

Copy link
Copy Markdown
Member

Eh let's send it to bors and see what happens

@bors: r+

@bors

bors commented Jul 16, 2016

Copy link
Copy Markdown
Collaborator

📌 Commit a06e308 has been approved by alexcrichton

@alexcrichton

Copy link
Copy Markdown
Member

Good way to get coverage at least!

@vadimcn

Copy link
Copy Markdown
ContributorAuthor

Huh, since when you don't need to provide a commit hash with r+ ?

@alexcrichton

Copy link
Copy Markdown
Member

Oh I believe that's actually always been the case, I just personally prefer to put the hash most of the time.

@bors

bors commented Jul 17, 2016

Copy link
Copy Markdown
Collaborator

⌛ Testing commit a06e308 with merge 761def0...

@bors

bors commented Jul 17, 2016

Copy link
Copy Markdown
Collaborator

💔 Test failed - auto-win-gnu-32-opt-rustbuild

@rillian

Copy link
Copy Markdown
Contributor

This does fix the linker assertion we get on MacOS X debug gecko builds. Thanks for working on this!

@alexcrichton

Copy link
Copy Markdown
Member

Ah and just to clarify the associated bug is this. @vadimcn looks like the failure here may be legitimate?

@vadimcn

Copy link
Copy Markdown
ContributorAuthor

looks like the failure here may be legitimate?

Yes, looks like it. For whatever reason, LLVM decides that __rust_start_unwind is a "nounwind" function. But only on Windows and only with -O -Clto.
...Which causes it to not emit unwind info in its caller, which the new personality routine detects and aborts. I think the was happening for some time now, but the old code was more lenient in this case.

@alexcrichton

Copy link
Copy Markdown
Member

Oh I think that extern functions are by default marked nounwind and it looks like I forgot to tag__rust_start_panic as such. I think that #[unwind] is all we need on that function?

@bors

bors commented Jul 22, 2016

Copy link
Copy Markdown
Collaborator

💔 Test failed - auto-win-gnu-64-opt

@arielb1

Copy link
Copy Markdown
Contributor

@bors retry

@arielb1

Copy link
Copy Markdown
Contributor

Eh no

@bors r-

rustc: x86_64-pc-windows-gnu/stage0/lib/rustlib/x86_64-pc-windows-gnu/lib/libpanic_unwind
../src/libpanic_unwind\seh64_gnu.rs:141:9: 141:19 error: structure `dwarf::eh::EHContext<'_>` has no field named `text_start`
../src/libpanic_unwind\seh64_gnu.rs:141 text_start: dc.ImageBase as usize,
^~~~~~~~~~
../src/libpanic_unwind\seh64_gnu.rs:141:9: 141:19 help: did you mean `get_text_start`?
../src/libpanic_unwind\seh64_gnu.rs:141 text_start: dc.ImageBase as usize,
^~~~~~~~~~
../src/libpanic_unwind\seh64_gnu.rs:142:9: 142:19 error: structure `dwarf::eh::EHContext<'_>` has no field named `data_start`
../src/libpanic_unwind\seh64_gnu.rs:142 data_start: 0,
^~~~~~~~~~
../src/libpanic_unwind\seh64_gnu.rs:142:9: 142:19 help: did you mean `get_data_start`?
../src/libpanic_unwind\seh64_gnu.rs:142 data_start: 0,
^~~~~~~~~~

Well, not quite: ARM EHABI platforms still use the old scheme -- for now.
@vadimcn

Copy link
Copy Markdown
ContributorAuthor

Forgot to update Windows code...

@vadimcn

Copy link
Copy Markdown
ContributorAuthor

@bors: r=alexcrichton

@bors

bors commented Jul 22, 2016

Copy link
Copy Markdown
Collaborator

📌 Commit 051c2d1 has been approved by alexcrichton

@bors

bors commented Jul 22, 2016

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 051c2d1 with merge 5bd4e21...

@bors

bors commented Jul 22, 2016

Copy link
Copy Markdown
Collaborator

💔 Test failed - auto-linux-64-opt

@arielb1

Copy link
Copy Markdown
Contributor

@bors retry

@bors

bors commented Jul 22, 2016

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 051c2d1 with merge 0fab347...

@bors

bors commented Jul 22, 2016

Copy link
Copy Markdown
Collaborator

💔 Test failed - auto-win-gnu-32-opt

@alexcrichton

Copy link
Copy Markdown
Member

@bors: retry

On Fri, Jul 22, 2016 at 4:11 PM, bors notifications@github.com wrote:

💔 Test failed - auto-win-gnu-32-opt
https://buildbot.rust-lang.org/builders/auto-win-gnu-32-opt/builds/5039


You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
#34832 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAD95GgLTv2MsJeOpLaGOytM-FC1iEKDks5qYU4kgaJpZM4JNJR3
.

@bors

bors commented Jul 23, 2016

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 051c2d1 with merge 1864601...

@bors

bors commented Jul 23, 2016

Copy link
Copy Markdown
Collaborator

💔 Test failed - auto-linux-64-cross-freebsd

@arielb1

Copy link
Copy Markdown
Contributor

@bors retry

@bors

bors commented Jul 23, 2016

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 051c2d1 with merge 3664f07...

@bors

bors commented Jul 23, 2016

Copy link
Copy Markdown
Collaborator

💔 Test failed - auto-linux-cross-opt

@arielb1

Copy link
Copy Markdown
Contributor

@bors retry force

@bors

bors commented Jul 23, 2016

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 051c2d1 with merge 2c50f4e...

bors added a commit that referenced this pull request Jul 23, 2016
Implement rust_eh_personality in Rust, remove rust_eh_personality_catch.
Well, not quite: ARM EHABI platforms still use the old scheme -- for now.
r? @alexcrichton
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@vadimcn@bors@alexcrichton@rillian@eddyb@arielb1