Skip to content
This repository has been archived by the owner on Nov 1, 2020. It is now read-only.

Don't fall through endfinally and others #2560

Merged
merged 1 commit into from
Jan 21, 2017
Merged

Conversation

mikedn
Copy link
Contributor

@mikedn mikedn commented Jan 21, 2017

endfinally, endfilter and rethrow were treated as fallthrough capable when they are not. endfinally was also failing to clear the stack.

Fixes #2518

@@ -66,9 +66,6 @@ private void InitLibraryInitializers()

foreach (var entry in s_assembliesWithLibraryInitializers)
{
if (_isCppCodeGen && !entry.UseWithCppCodeGen)
Copy link
Member

@jkotas jkotas Jan 21, 2017

Choose a reason for hiding this comment

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

Could please clear the flag isCppCodeGen flag here, and delete link to the fixed issues here instead?

I think the flag to filter based on isCppCodeGen can stay around; even though it is unused right now. We will likely want different implementations of some parts for it eventually; or it may be useful to workaround CppCodeGen bugs.

@jkotas
Copy link
Member

jkotas commented Jan 21, 2017

Thank you for the fix!

endfinally, endfilter and rethrow were treated as fallthrough capable when they are not. endfinally was also failing to clear the stack.
@mikedn
Copy link
Contributor Author

mikedn commented Jan 21, 2017

Updated, hope I got it right.

Out of curiosity: what's the plan for exception handling in CPP codegen? EH isn't exactly trivial even in native codegen, in CPP codegen it's probably even more problematic.

@jkotas
Copy link
Member

jkotas commented Jan 21, 2017

what's the plan for exception handling in CPP codegen

Piggy back on C++ exception handling, use explicit checks instead of hardware exceptions (e.g. explicit checks for null reference exceptions).

Unity IP2CPP internals blog shows example of possible way to do this. Mono LLVM pure bitcode backend has same problem and it uses similar scheme to solve it.

@jkotas
Copy link
Member

jkotas commented Jan 21, 2017

Mentioned the same in #910

@jkotas jkotas merged commit 55aec26 into dotnet:master Jan 21, 2017
@mikedn mikedn deleted the catch-block branch April 10, 2017 05:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants