Add test variant testing GCC runtime compatibility #259
Conversation
davidchisnall
force-pushed
the
gcc-runtime-test
branch
from
f7f05be to
3e6a543
Compare
|
[ Updating the branch to pick up fixes to the CI scripts. ] |
|
I don't really like this because ripping out all of the GCC-runtime-compat code is top of my to-do list for after the next release. There's discussion in Clang of removing support for all fragile ObjC ABIs, which would make this go away. |
|
The only way I know of to compile GNUstep code reliably on Windows is to use the fragile ObjC ABI (with e.g. GCC's objc runtime). If you use the 2.0 ABI, you need to use the It'd be helpful to have at least one release which supports compiling under MinGW and with the fragile ABI, and to have Clang support the fragile ObjC ABIs until we find a way for GNUstep to export the ivar symbols (either by workarounds in GNUstep or through a fix in ldd). |
|
Hmm, interesting, I haven't actually tried GNUstep on Windows (and, since I left MS, no longer have a Windows machine to test with). I'd personally regard accessing an ivar from another DLL as a bad idea, but I can imagine that there are cases where it's unavoidable (I believe WinObjC just didn't do this at all and used declared properties for anything where a subclass may need to access it in another DLL). You shouldn't need to use lld. Last time I tested it, LINK.EXE worked as long as you disabled incremental linking (otherwise it added padding in the middle of data structures, which was unhelpful). I think the problem is here: https://github.com/llvm/llvm-project/blob/c7c912cff945033918367c4a37121dfc09b9759e/clang/lib/CodeGen/CGObjCGNU.cpp#L1682 We should be setting the ivar offset variable as dllexport if the ivar is public and the class is dllexport (when targeting Windows). Does doing that fix the issue? If I remember correctly, PE/COFF allows you to refer to exported globals in code from other DLLs, just not in initialisation of globals, and that's all that we need to do here. |
If you link GNUstep on MinGW with the ld linker, it exports those symbols (e.g. Thanks for the tip about link.exe, I'll try that in the coming days. |
I believe this is because MinGW defaults to exporting all symbols.
I believe lld is a drop-in replacement for LINK.EXE on Windows, so you should be able to list them in a .DEF file. The right fix is to make clang properly mark them as exported though. |
|
Another data point which may be relevant: most Linux distributions ship a copy of gnustep which is linked with the GCC libobjc implementation: So if you are developing a GNUstep application on those platforms and use the distribution-provided copy of GNUstep, you’ll end up using the GCC objc runtime. I’d love for those distributions to start packaging libobjc2 and use it as the default objc runtime, but that will take time. |
|
Most distros either require packages to be compiled with the default compiler (which is usually gcc) or simply do not care about GNUstep. We switched the GNUstep packages over to compiling with clang and using libobjc2 for the FreeBSD packages around ten years ago. Objective-C support has been unmaintained in GCC for longer than that. |
|
Just chiming in here about link.exe: unfortunately using link.exe for Objective-C code does not seem to be working any more (even with incremental linking disabled) and causes crashes in objc_msgSend, at least when I tried last summer. This mailing list thread has some more info. I’m pretty sure it worked at one point a couple years ago, not sure what might have changed. |
|
Thanks for that reference, @triplef . I believe on of the most impactful things we can do to move GNUstep forward on the Windows platform, is to make sure lld properly exports symbols for instance variable offsets. Working around that limitation is tedious, at the least, and maintainers (rightfully) push back to those kinds of workarounds: gnustep/libs-gui#177 (comment) |
I believe the clang change outlined earlier should fix that (setting the correct dllexport attribute on the ivar variables). This isn't a problem with lld, it's a problem with clang not marking the symbols as exported. Clang is also the only thing that knows which of them should be exported (ones for private / package ivars should not be). |
Makes sense. @qmfrederik if you need help with making the change outlined above in LLVM I can put you in touch with @weliveindetail (about), who fixed a bunch of other LLVM issues for ObjC on Windows for us. |
|
It should be a simple change. You will need to do I think you will also need to do a similar check for |
|
@davidchisnall Something like this: qmfrederik/llvm-project@00e7567 ? I'll try to get this tested later today. |
|
Yup. I don’t think you need the visibility check on the import, we shouldn’t get to codegen with IVars you can’t see. |
|
I tried that patch, and compiled a simple library: #import "../objc/runtime.h"
#import "../objc/objc-arc.h"
__declspec(dllexport)
@interface Visibility {
@public BOOL*testVar;
}
@end
@implementation Visibility
@endcmake_minimum_required(VERSION 3.9)
project(Visibility)
add_library(visibility SHARED Visibility.m)
target_include_directories(visibility PRIVATE ../build/)
target_link_libraries(visibility objc)I compiled with both vanilla clang 17 and clang 18 with the patch described above. In both cases, the Here's the output for the library built with vanilla clang 17: and here the equivalent output for clang 18 with the patch: |
|
I am not sure what the output of nm means on Windows. What does |
|
Here's the dumpbin output: clang 18: clang 17: |
|
Oh, I see the problem! On line 1689, you're modifying the AST, not the emitted global. This should be setting the DLL storage class on IvarOffsetPointer, not Ivar. |
|
Hmm, looks I already made that change: llvm/llvm-project@ffaa238 and the result appears to be the same. I also uncommented the |
|
It shouldn't make a difference, but can you move the code setting up the global into the if block above where it's created (line 1681)? Can you also dump the IR and make sure that the dllexport attribute is actually added to the ivar global? |
|
@davidchisnall It looks like llvm/llvm-project@9584557 did the trick. Does this look good to you? If so, I can open a PR in the llvm-project repo. |
|
... it also helped to specify clang 18: clang 17: |
|
Clang changes look good, please raise a PR and tag me to review. |
|
With that PR merged, I now got here: https://lists.nongnu.org/archive/html/gnustep-dev/2021-10/msg00030.html |
|
I think the linked post has the solution. Does that work? |
|
Yes, that's on my todo list. I'd like to get llvm/llvm-project#77255 and #267 merged first before starting something new. Also, using the GNU ABI acts as a (short-term) workaround; here's the |
|
@davidchisnall This behavior seems to be specific to lld. Consider this file: #define GS_EXPORT_CLASS __declspec(dllexport)
typedef struct autorelease_array_list
{
struct autorelease_array_list *next;
unsigned size;
unsigned count;
__unsafe_unretained id objects[0];
} array_list_struct;
GS_EXPORT_CLASS
@interface NSAutoreleasePool
{
struct autorelease_array_list *_released;
}
@end
@implementation NSAutoreleasePool
@endYou can compile and link this to a shared with ld: and the ivar symbol is exported correctly: But you'll get an error with lld: |
|
Probably easier to work around than fix, if we just turn |
|
Yes, I'll give that a try: llvm/llvm-project@c045bbf. clang is rebuilding (sigh), will let you know once that's done. (Looks like this is known to work: https://lists.nongnu.org/archive/html/gnustep-dev/2021-11/msg00000.html) @triplef, how did you work around this for Windows MSVC? |
|
If you mean work around the linker errors with libs-gui: I didn’t – I gave up on building libs-gui on Windows back then since the issues were too involved to justify spending more time on it while we don’t need it ourselves (we use Qt/QML for our UI on Windows and Android). |
|
@triplef Well, I'm seeing this linker issue when building and linking libs-base, so I was wondering whether you had to do something special to make this work with clang-cl? Did you apply patches to libs-base to work around this (e.g. don't expose any ivars which are of a struct type), or use a different linker? |
|
@davidchisnall Yes, the patch proposed in that mail thread works. I can now compile and link libs-base on MinGW using the gnustep 2.0 ABI, using this patch: llvm/llvm-project@c65ae76. 🎉 So it looks like clang should be in a good shape if we can get that change upstreamed. I've got a couple of questions:
Once we've got some clarity on how we want to implement this, I can go ahead and create a PR. |
|
Update: libs-gui compiles and links, though libgmodel still fails. That'll be for another day. |
I don’t think so. We’re using the LLD linker and we don’t apply any patches in that regard to libs-base. I guess the codegen must be somehow different with MinGW? |
|
Thanks. We do want to do it in all of those places. It’s worth pulling it out into a separate function called makeValidSybolName or similar. The @ replacement should be necessary only on ELF platforms (it conflicts with symbol versioning). The = replacement should not be needed anywhere other than Windows. I think it’s probably okay as an ABI break on Windows. The ivar variables are the only ones where it matters (the others will simply do less uniquing). If we were not able to link with the old mangling then the impact should be low. |
|
llvm/llvm-project#77797 should implement this |
|
We now have good support for Objective C on MSYS2 using Clang 18.1.0, so I'm closing this for now. |
Using
-fobjc-runtime=gccwith clang will cause clang to use the GNU_ObjC_SEH personality, and link with__gnu_objc_personality_seh0. This allows testing of SEH exception handling in MinGW.