gnustep / libobjc2

Objective-C runtime library intended for use with Clang.
http://www.gnustep.org/
MIT License
426 stars 116 forks source link

Add domestic exception handling support for MinGW #260

Closed qmfrederik closed 6 months ago

qmfrederik commented 6 months ago

This is the exception-handling related part of #190.

It:

which brings basic try/catch support to objc2 on MinGW. C++ exception interop is still not implemented.

When running the tests, 89% tests passed, with all failures being related to exception interop:

Notable passing tests are ExceptionTest, NestedExceptions, NilException, objc_msgSend and RuntimeTest

qmfrederik commented 6 months ago

@davidchisnall @triplef What do you think? It wires up domestic Objective C exception handling on MinGW, which I think is a big win. __gnustep_objc_personality_v0 behaving as __gnustep_objc_personality_seh could be controversial (and I can undo the change), but it could be a short-term fix which can be removed when clang knows about MinGW support in libobjc2.

davidchisnall commented 6 months ago

I think this looks mostly fine, thanks for working on it. Using this model precludes catching MSVC C++ exceptions in MinGW programs, but I presume anyone who opts into using MinGW already chose suffering so that shouldn’t be an issue.

For C++ exception interop to work, we need MinGW’s libstdc++ to expose their C++ personality function as a public symbol. Was there any progress getting them to do this?

I don’t want to make this an officially supported configuration without that because clang throws Objective-C objects as C++ objects in Objective-C++ mode.

The alternative that I proposed is simply to have clang use the C++ personality function and throw functions in MinGW mode. Our type_info subclasses should make that just work with MinGW already.

qmfrederik commented 6 months ago

Thanks @davidchisnall

I presume anyone who opts into using MinGW already chose suffering so that shouldn’t be an issue.

At least in my case, the choice for MinGW wasn't really voluntary, but rather because MinGW is the only environment in which you can compile GNUstep (including the GUI) for Windows.

We need MinGW’s libstdc++ to expose their C++ personality function as a public symbol

There's are multiple MinGW environments. The clang environments use libc++. Would that simplify things a bit?

Both the libstdc++ in the mingw64 environment and libc++ in the clang64 environment export the __gxx_personality_seh0 symbol. Does that help?

vagrant@DESKTOP-RNTVKUC UCRT64 ~
# nm -g /mingw64/lib/libstdc++.a | grep "T __gxx_personality_seh0" 
0000000000000000 T __gxx_personality_seh0

vagrant@DESKTOP-RNTVKUC UCRT64 ~
# nm -g /clang64/lib/libc++.a | grep "T __gxx_personality_seh0"
0000000000000000 T __gxx_personality_seh0

to have clang use the C++ personality function and throw functions in MinGW mode

Could you elaborate on that a bit more? This is not familiar territory for me, but perhaps there's a reference implementation you can point me to?

davidchisnall commented 6 months ago

At least in my case, the choice for MinGW wasn't really voluntary, but rather because MinGW is the only environment in which you can compile GNUstep (including the GUI) for Windows.

It’s a bigger project, but it would be really nice to remove that dependency at some point. Most of GNUstep uses Foundation as an abstraction layer and there aren’t many POSIX dependencies outside of a few classes (e.g. NSFileHandle, NSLock).

Both the libstdc++ in the mingw64 environment and libc++ in the clang64 environment export the __gxx_personality_seh0 symbol. Does that help?

Unfortunately not. This is the adaptor that looks like a Windows EH personality. This then calls a GCC runtime helper function that takes an Itanium personality function as an argument and morphs the Windows unwind info to look like Itanium EH info and calls it. By the time our code is reached, we’ve already had that transformation happen. We need to have access to the function that __gxx_personality_seh0 passes to _GCC_specific_handler. We forward to the C++ personality function here: https://github.com/gnustep/libobjc2/blob/28bb93cfdc25a1f20da9495d9a0efb0060ece451/eh_personality.c#L613

In libcxxabi, this should be __gxx_personality_imp, which is declared static. I’m not sure if any of the Windows introspection functions would let us find it, but the thing that we really want is to drop the static from the declaration here:

https://github.com/llvm/llvm-project/blob/0871c4beb826feba2d2aaf2c3efbe1fdeba7624a/libcxxabi/src/cxa_personality.cpp#L911C1-L911C29

qmfrederik commented 6 months ago

It would be really nice to remove that dependency at some point

Yes, and I think we're pretty close to being able to make that happen. You can use MSYS2 as a Bash shell on Windows to invoke the GNUstep build scripts, and have the build scripts invoke the Windows-native Clang compiler (i.e. no MinGW involved). At the moment, I got stuck because the lld linker does not export symbols for instance variable offsets, but you've provided pointers for that -- thanks.

In libcxxabi, this should be __gxx_personality_imp, which is declared static.

This is probably a long shot, but it looks like at least libc++ exports the __libunwind_seh_personality function. It looks like this is the opposite of _GCC_specific_handler? If so, could we do the same as this function -- convert the Itanium unwind info back to Windows unwind info, and then call __gxx_personality_seh0, have it undo that conversion, and finally invoke __gxx_personality_imp?

The thing that we really want is to drop the static from the declaration here

Do you know whether there's a chance of a patch doing that being accepted by the LLVM project?

davidchisnall commented 6 months ago

This is probably a long shot, but it looks like at least libc++ exports the __libunwind_seh_personality function. It looks like this is the opposite of _GCC_specific_handler?

If I'm reading the code right, I think it handles some of the translation, so it probably breaks if you interleave it but it might be worth a try.

Do you know whether there's a chance of a patch doing that being accepted by the LLVM project?

They'd probably want to rename the function, but I don't see a problem with it landing, if we can justify it by wanting other personality functions to be able to delegate to the C++ one.

qmfrederik commented 6 months ago

@davidchisnall I've rewritten this PR a bit, and removed the #ifdef that disabled the call to test_cxx_eh_implementation on MinGW. Instead, I've disabled support for Objective C++ on MinGW, which results in NO_OBJCXX being set and that entire code path being ignored.

I think that's a more correct way of doing this. It enables domestic exception support in Objective C but doesn't make a guarantee of C++ interop.

I'll explore the other options we've discussed, starting with finding ways to invoke __gxx_personality_imp. Would you be willing to accept this PR as is, for now?

qmfrederik commented 6 months ago

@davidchisnall I spent some time looking into this. It looks like the purpose of test_eh_personality is to extract the values for type_info_offset, exception_struct_size and cxx_exception_class. It needs the exceptionObject and exceptionClass values to do so. Once it has done that, it delegates all work to __gxx_personality_v0.

But looking at _GCC_specific_handler, it appears that you can get this information in the _seh0 variant via (struct _Unwind_Exception *) ms_exc->ExceptionInformation[0].

In that case, could we just extract these values in test_eh_personality on the first call, and have test_eh_personality call __gxx_personality_seh0, without having to worry about _GCC_specific_handler?

davidchisnall commented 6 months ago

The C++ runtime library’s internal structure layout is not public (or stable, it’s changed in a couple of implementations). It isn’t getting these values that’s the problem, it’s setting them.

It may be possible, for MinGW, to move the wrapping one layer up and do it in the function that calls the GCC specific handler adaptor. If the incoming exception is an Objective-C exception and we’re in a C++ frame, we forward directly to the C++ personality function.

That said, the simpler solution is probably to get rid of all of the ifdefs and have a different objc_throw for MinGW that always throws as a C++ exception, and just use the ObjC++ code paths unconditionally for the catch blocks in Clang.

davidchisnall commented 6 months ago

Can you try modifying this line:

https://github.com/llvm/llvm-project/blob/5ed11e767c0c39a3bc8e035588e7a383849d46a8/clang/lib/CodeGen/CGObjCGNU.cpp#L826C3-L826C3

so that we use the C++ being / catch handlers for MinGW, this line:

https://github.com/llvm/llvm-project/blob/5ed11e767c0c39a3bc8e035588e7a383849d46a8/clang/lib/CodeGen/CGException.cpp#L213

and this line:

https://github.com/llvm/llvm-project/blob/5ed11e767c0c39a3bc8e035588e7a383849d46a8/clang/lib/CodeGen/CGException.cpp#L159

So that we use GNU_CPlusPlus_SEH for MingGW, this line:

https://github.com/llvm/llvm-project/blob/5ed11e767c0c39a3bc8e035588e7a383849d46a8/clang/lib/CodeGen/CGObjCGNU.cpp#L2390

So that we use C++ mode unconditionally on MinGW,

and then replace the exception code in libobjc2 with a single definition of objc_throw that looks like this:

static void eh_cleanup(void *exception)
{
   objc_release(*(id*)exception);
}

void objc_throw(id object)
{
    id *exc = (id *)__cxa_allocate_exception(sizeof(id));
    __cxa_throw(exc, & __objc_id_type_info, eh_cleanup);
}

I believe that should be sufficient to make MinGW use C++ exceptions unconditionally for Objective-C[++], which should then avoid needing to change anything. We can just put those two functions in a separate file and compile that instead of eh_personality for MinGW targets.

qmfrederik commented 6 months ago

@davidchisnall I'll try to test this later today, but is this what you hand in mind:

davidchisnall commented 6 months ago

I think that looks about right, yup.

qmfrederik commented 6 months ago

@davidchisnall Looks like I got some first results with https://github.com/gnustep/libobjc2/commit/9e84ba714945ef22e38b59225472644eb0bcead9 - using objc_exception_throw instead of objc_throw, declaring __cxa_throw and marking some additional symbols as public.

The try/catch clause in objc_msgSend.m seems to be working, although the assert assert((TestCls == e) && "Exceptions propagate out of +initialize"); is failing; if I disable that assert the test passes.

davidchisnall commented 6 months ago

Thanks! Looks like we're getting close. Propagating out of +initialize requires that they're thrown through C frames with __attribute__((cleanup)). Can you check whether that works with MinGW with just C and C++ (call C++ -> C -> C++, use __attribute__((cleanup)) in C, check you can throw an exception out)? It's possible that we need to add an extra compiler flag to the compilation unit that handles initialize.

It's a bit surprising to me that the exception seems to be thrown and code continues executing.

qmfrederik commented 6 months ago

Closing in favor of #267