MSVC/x86 exception handling: rewrite avoiding VC runtime internals #54

rainers · 2016-01-22T22:53:58Z

setup exception handler for exceptions during cleanup to avoid C++ calling std::terminate
remove vcruntime hacks
when converting exception to error, do not rethrow if the catch handles errors, too
add support for the "safeseh" attribute

kinke · 2016-01-23T10:48:11Z

src/ldc/eh/win32.d

 }

-extern(C) void _d_leave_cleanup(void* ptr)
+// @safeseh to be marked as "safe" for the OS securtity check


securtity => security

JohanEngelen · 2016-01-25T23:19:58Z

Could you also make an IR testcase for this? ;-)
See https://github.com/JohanEngelen/ldc/blob/8d078f3bd8939dc6a6c3d5209bf63e2f08d81e24/tests/ir/attributes.d
or
https://github.com/ldc-developers/ldc/pull/1244/files

rainers · 2016-01-26T08:37:09Z

Could you also make an IR testcase for this? ;-)

I'll try though I've yet to see any of the IR tests to pass on my system.
Do you think adding an attribute for safeseh is the right direction? It could also be buried in the target-specific attributes, as it is very specific to Win32.

rainers · 2016-01-26T08:42:39Z

securtity => security
void * => void* etc. pls, for consistency, also for function below

Done.

JohanEngelen · 2016-01-26T10:21:44Z

I'll try though I've yet to see any of the IR tests to pass on my system.
Really? Not to hijack this thread: can you open a new issue for it with some details of error messages?

rainers · 2016-01-27T21:26:40Z

Really? Not to hijack this thread: can you open a new issue for it with some details of error messages?

Sorry for being slow with this, I've just reported it here: ldc-developers/ldc#1265

rainers · 2016-01-28T22:27:24Z

Finally implemented swapping exception information for fibers and figured why exceptions where never caught in fibers.
With this change, core.thread also passes its unittests in debug and release builds.

rainers · 2016-01-29T10:38:08Z

I've added support for catching D exceptions in C++, see ldc-developers/dmd-testsuite#15 for a demonstrating test case.

dnadlinger · 2016-01-29T19:55:46Z

src/core/thread.d

@@ -4854,16 +4858,21 @@ private:
                finalHandler = reg.handler;
            }

-            pstack -= EXCEPTION_REGISTRATION.sizeof;
+            // With safeseh(?), the chain must not extend to the very top


Thanks for the explanation and the change, it's a lot clearer to me now. (I thought you were referring to the beginning of the record before instead of to the past-the-end address.)

Shouldn't the change also go into the upstream implementation, though? (We can just cherry-pick the fix from master afterwards.)

Shouldn't the change also go into the upstream implementation, though? (We can just cherry-pick the fix from master afterwards.)

I don't think dmd will ever support safeseh. It shouldn't do harm to add it, though.

rainers · 2016-01-30T10:45:58Z

Updated to allow any exception type to be mapped to C++.
I also applied some of the ideas from rainers#4

kinke · 2016-01-30T22:40:07Z

src/ldc/eh/win32.d

+    {
+        auto olength = other._length; other._length = _length; _length = olength;
+        auto op      = other._p;      other._p      = _p;      _p = op;
+        auto ocap    = other._cap;    other._cap    = _cap;    _cap = ocap;


Just nit-picking, but how about:

static void swapField(T)(ref T a, ref T b) { T o = b; b = a; a = o; } swapField(_length, other._length); swapField(_p, other._p); swapField(_cap, other._cap);

Done. I think there should be a swap function somewhere in druntime, it is implemented again and again...

kinke · 2016-01-31T20:03:42Z

I've cherry-picked your upstream commit regarding core.thread, so another rebase is needed. While at it, could you please squash the last 4 commits to a single one and then factor out the C++ stuff (2nd mangling, _d_toString()) in a separate commit again?

…g std::terminate remove vcruntime hacks when converting to exception to error, do not rethrow if the catch handles errors, too

…ion chain

rainers · 2016-01-31T22:23:06Z

Rebased and squashed some commits.

There's still one open issue that doesn't work with this design: cleanup funclets cannot have exception handling themselves, but that's valid in D, e.g. by using try/catch in scope(exit). The LLVM devs don't plan to implement this any time soon.

That's why I want to try outlining cleanup code in the front-end. This might also help the x64 implementation, as it is unlikely that you can setup exception information on x64 as done here (emplace an exception frame in the calling functions stack space).

kinke · 2016-01-31T23:32:42Z

Rebased and squashed some commits.

👍

That's why I want to try outlining cleanup code in the front-end. This might also help the x64 implementation, as it is unlikely that you can setup exception information on x64 as done here (emplace an exception frame in the calling functions stack space).

And I thought it'd be finished. ;) All the best for your next adventure then, you rock! :)

rainers · 2016-02-01T06:59:04Z

Not sure it'll work out this time. I expect some troubles regarding cleanups that are already inside nested functions.
So maybe it's better to get this in as an (almost) working solution for x86...

kinke · 2016-02-02T22:38:45Z

It is unlikely that you can setup exception information on x64 as done here

Do you think the current method (setting up the std::terminate hook) is going to be the (first) x64 solution? That's the reason I haven't merged this yet... ;)

rainers · 2016-02-05T07:49:23Z

Do you think the current method (setting up the std::terminate hook) is going to be the (first) x64 solution? That's the reason I haven't merged this yet... ;)

I hope I can try the frontend-outling this weekend. If that doesn't work out, merging the two solutions might be the best solution ATM for x64 support.

rainers · 2016-02-09T07:34:39Z

I hope I can try the frontend-outling this weekend.

FYI: I actually managed to rewrite cleanup blocks, e.g. finally { code } to finally { _d_cleanup(() { code }); }, with _d_cleanup just calling the delegate, but replacing the function for unwind cleanups with another that catches exceptions.

Unfortunately, it causes some of the expected compile errors when having to access several contexts. In addition, errors show the additional indirection in instantiation traces (_d_cleanup must be a template to inherit function attributes). So it doesn't seem a good solution ATM.

I'm currently torn between this and the previous version using the std::terminate hook. Both are rather hacky. I'll investigate how to make the latter safer to avoid skipping a std::terminate call that's not initiated by an exception during cleanup (it would probably also crash).

rainers · 2016-03-01T14:12:16Z

Closing this in favor of #60.

rainers mentioned this pull request Jan 22, 2016

[WIP] support building for x86 with MSVC runtime ldc-developers/ldc#1168

Closed

kinke reviewed Jan 23, 2016
View reviewed changes

rainers force-pushed the catch_cleanup_exceptions branch from fa62b2e to 9d8f75e Compare January 29, 2016 08:25

rainers mentioned this pull request Jan 29, 2016

add test for catching D exceptions from C++ on Win32 ldc-developers/dmd-testsuite#15

Open

dnadlinger reviewed Jan 29, 2016
View reviewed changes

kinke reviewed Jan 30, 2016
View reviewed changes

rainers added 4 commits January 31, 2016 22:49

setup exception handler for exceptions during cleanup to avoid callin…

c91d50a

…g std::terminate remove vcruntime hacks when converting to exception to error, do not rethrow if the catch handles errors, too

fibers: implement switching exception information, fix invalid except…

d09d8ca

…ion chain

refactorings inspired by #4

ac41bcf

add support for catching D exception in C++

7484da5

rainers force-pushed the catch_cleanup_exceptions branch from f0d795f to 7484da5 Compare January 31, 2016 22:09

rainers mentioned this pull request Feb 13, 2016

MSVC EH updates and preparation for x64 #60

Merged

rainers closed this Mar 1, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MSVC/x86 exception handling: rewrite avoiding VC runtime internals #54

MSVC/x86 exception handling: rewrite avoiding VC runtime internals #54

rainers commented Jan 22, 2016

kinke Jan 23, 2016

JohanEngelen commented Jan 25, 2016

rainers commented Jan 26, 2016

rainers commented Jan 26, 2016

JohanEngelen commented Jan 26, 2016

rainers commented Jan 27, 2016

rainers commented Jan 28, 2016

rainers commented Jan 29, 2016

dnadlinger Jan 29, 2016

rainers Jan 30, 2016

rainers Jan 30, 2016

rainers commented Jan 30, 2016

kinke Jan 30, 2016

rainers Jan 31, 2016

kinke commented Jan 31, 2016

rainers commented Jan 31, 2016

kinke commented Jan 31, 2016

rainers commented Feb 1, 2016

kinke commented Feb 2, 2016

rainers commented Feb 5, 2016

rainers commented Feb 9, 2016

rainers commented Mar 1, 2016

MSVC/x86 exception handling: rewrite avoiding VC runtime internals #54

MSVC/x86 exception handling: rewrite avoiding VC runtime internals #54

Conversation

rainers commented Jan 22, 2016

kinke Jan 23, 2016

Choose a reason for hiding this comment

JohanEngelen commented Jan 25, 2016

rainers commented Jan 26, 2016

rainers commented Jan 26, 2016

JohanEngelen commented Jan 26, 2016

rainers commented Jan 27, 2016

rainers commented Jan 28, 2016

rainers commented Jan 29, 2016

dnadlinger Jan 29, 2016

Choose a reason for hiding this comment

rainers Jan 30, 2016

Choose a reason for hiding this comment

rainers Jan 30, 2016

Choose a reason for hiding this comment

rainers commented Jan 30, 2016

kinke Jan 30, 2016

Choose a reason for hiding this comment

rainers Jan 31, 2016

Choose a reason for hiding this comment

kinke commented Jan 31, 2016

rainers commented Jan 31, 2016

kinke commented Jan 31, 2016

rainers commented Feb 1, 2016

kinke commented Feb 2, 2016

rainers commented Feb 5, 2016

rainers commented Feb 9, 2016

rainers commented Mar 1, 2016