Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

[x86/Linux] Revises exception handling macros for x86/Linux #8381

Closed
wants to merge 1 commit into from

Conversation

parjong
Copy link

@parjong parjong commented Nov 30, 2016

This commit revises exception handling macros similarly as amd64/Linux.

@parjong
Copy link
Author

parjong commented Nov 30, 2016

\CC @seanshpark

@janvorli
Copy link
Member

@parjong seeing the several recent changes in the exception handling related stuff, I am worried that after deciding to not to try to define the WIN64EXCEPTIONS, we are heading into the same mess we were trying to avoid, this time with trying to ifdef out chunks of x86 style exception handling code.
My worry is that some functions that we ifdef out are needed by some other code and we either didn't get to the point when a caller gets compiled or that there are prototypes of the excluded functions that keep the compiler happy, but we will hit the problem later when we'll be trying to make everything link together without missing symbols.
We are also polluting the code base with a lot of ifdefs that we will want to remove after we switch to WIN64EXCEPTIONS later. And it would be a pain to keep track of where have we put them.
So I would like us to pause and see if we can rather make most of the x86 exception handling code build on Unix and add dummy structs / stubbed out implementation of Windows APIs that the code uses(if any) to unixstubs.cpp so that we can remove them later easily.
@jkotas does it make sense to you too?

@jkotas
Copy link
Member

jkotas commented Nov 30, 2016

if we can rather make most of the x86 exception handling code build on Unix and add dummy structs / stubbed out implementation of Windows APIs that the code uses(if any) to unixstubs.cpp so that we can remove them later easily.

I agree. It is hard to tell whether the changes like this one are going in the right direction without seeing the more complete picture.

@parjong
Copy link
Author

parjong commented Dec 1, 2016

@janvorli @jkotas While working on x86/Linux port, I also feel that there is some unclear ifdef. IMHO, #8372 may be such an example.

I currently try both approaches (with/without WIN64EXCEPTIONS), and feel that both directions require some code cleanup.

@jkotas
Copy link
Member

jkotas commented Dec 1, 2016

I would recommend to focus on without WIN64EXCEPTIONS for the time being. It will get you to a working hello world much faster.

Note that we had a working MacOS x86 port before for Silverlight browser plugin. It was on without WIN64EXCEPTIONS plan. If you stay on the without WIN64EXCEPTIONS you are basically just fixing bit rot and catching up with changes that happened since then. This territory was fully explored before.

The with WIN64EXCEPTIONS plan is unexplored territory for x86.

@parjong
Copy link
Author

parjong commented Dec 1, 2016

@jkotas I'll focus on that approach. Thank you.

@parjong
Copy link
Author

parjong commented Dec 5, 2016

The recent changes require the revision of this PR. I'll re-open this PR or submit a new PR when it is ready.

@parjong parjong closed this Dec 5, 2016
@parjong parjong deleted the fix/x86_eh_macros branch December 15, 2016 00:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants