[PATCH] D89518: [windows-itanium] Add Windows Itanium How-To Guide

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: llvm/docs/HowToBuildWindowsItaniumPrograms.rst:39
+
+link.exe (the MS linker) is unsuitable as it rejects the COMDATs produced by
+LLVM. It also doesn't support auto-importing which is currently required.
----------------
bd1976llvm wrote:
> mstorsjo wrote:
> > Did you look into why it produces such comdats? Because for msvc triple targets, it should work with link.exe.
> I think I conflated several issues here, as I was trying to get the build set up, and got myself confused. Originally, I think I was somehow generating the "leaderless comdats" that you mentioned in another thread. I had thought that the leaderless comdats were related to multiply defined symbols being present. Happily, I no longer see the leaderless comdats and the remaining multiply defined symbol problems have now been solved and were unrelated. I can now link with MS's link.exe and I have adjusted the doc accordingly.
> 
> There is a remaining comdat issue that I see when building libc++ when not using the internal assembler which occurs if I try to use the ' -save-temps=obj' option for debugging. Errors are emitted such as:
> 
>     #                It seems that source -> assembly -> obj may report comdat
>     #                errors (when assembling) that are not reported doing source-> obj.
>     #                Example:
>     #                  src/CMakeFiles/cxx_shared.dir\locale.s:107958:2: error: section '.xdata$' is already linkonce
>     #                  .linkonce       discard
> 
> I doubt this is related to Windows Itanium and I haven't looked into it more as it isn't currently blocking me.
Ok, great to hear that those bits are sorted out.

Regarding roundtripping through assembly, it's possible that it ends up misgenerated for these cases, but it doesn't seem like I can reproduce it for a trivial case at least. For the gnu assembly syntax, GCC (which doesn't properly use all the aspects of COFF comdats) uses one syntax, while llvm has extended the syntax to support indicating the leader symbol.

A COFF comdat generated by GCC looks like this:
```
        .section        .text$_Z6myfuncv,"x"
        .linkonce discard
```
while generated by clang it looks like this:
```
        .section        .text$_Z6myfuncv,"xr",discard,_Z6myfuncv
```

(For comdats without a leader symbol, LLVM also generates the GCC-like syntax.)

It sounds to me like there's some mixup between these two happening?

With a test file like this:
```
inline void myfunc(void) {}
void (*ptr)(void) = myfunc;
```
I get the expected syntax for a `x86_64-windows-itanium` target, but maybe it happens with cases that are a bit harder to trigger, like jump tables or something like that...


================
Comment at: llvm/docs/HowToBuildWindowsItaniumPrograms.rst:53
+addresses are patched into the IAT). Therefore, the compiler must emit some code,
+that runs after IAT patching but before anything that might use the vtable pointers,
+and sets the vtable pointer to the address from the IAT. For the special case of
----------------
bd1976llvm wrote:
> mstorsjo wrote:
> > Actually - the code for patching it isn't emitted by either compiler or linker, it's done by the `_pei386_runtime_relocator` function in the mingw runtime. The linker just generates a list of fixups that the runtime later will need to process. If the runtime that processes this list isn't linked in, it won't work. Unfortunately, this can happen silently...
> > 
> > Have you checked this aspect, that it actually works as intended?
> > 
> > See https://github.com/mstorsjo/llvm-project/commit/lld-pseudo-relocs for a patch (not recently rebased, unfortunately) to lld that makes it force a reference to this function, if runtime fixups actually are needed. That would make it clear if it's needed, but missing.
> > 
> > Finally, the `_pei386_runtime_relocator` function also needs to be called. Currently in the mingw-w64 runtime, it's always called unconditionally by other startup code, but with a change like https://github.com/mstorsjo/mingw-w64/commit/pseudo-reloc-ctor, that object file would have a ctor that forces it to be called autonomously, if the object file is linked in. (Unfortunately, that change doesn't work with ld.bfd as it is right now, so it isn't upstreamed.)
> It does not work as intended. Thanks for providing details here. I have adjusted the doc to add a more detailed explanation. As I understand it one of the goal of Windows Itanium is to use the Itanium C++ ABI on windows without relying on extra dependencies such as  a custom runtime. Therefore, this must remain as a limitation for now.  I spoke to @compnerd briefly about this problem and he though that the solution would be in the compiler emitting code that is called by the MS runtime at start-up and initialises these fields correct. I haven't yet investigated whether this is feasible or not.
Oh, ok, that explains it.

Yeah it might be possible to make clang generate something like that, but IIRC I tried to do that at some point, and it ended up rather messy - see https://reviews.llvm.org/D43184 for an old attempt.

You can check how it works if you build this test file:
```
__declspec(dllimport) extern int dllVar;
int *myptr = &dllVar;
```

If you build this as C, it errors out (initializer element is not a compile-time constant), but if you build it as C++, it generates a constructor that initializes myptr at runtime. (And if you make it `int *const myptr`, the whole symbol vanishes!)


================
Comment at: llvm/docs/HowToBuildWindowsItaniumPrograms.rst:57
+there is no declaration available to the compiler so this can't be done. To allow
+programs to link we currently rely on the -lldmingw switch in LLD to auto-import
+references to __cxxabiv1::__class_type_info pointers (see: https://reviews.llvm.org/D43184
----------------
bd1976llvm wrote:
> mstorsjo wrote:
> > Fwiw, since release 11, you can enable autoimporting alone without enabling the rest of the mingw specific quirks, with the `-autoimport` flag. Also see D89006 for a recent related fixup.
> Thanks. This is a nice improvement!
It seems nicer, yes, but it also gives the false impression that you can enable and use it without the corresponding mingw runtime, so that's not ideal either. I'm pondering if I should spend more effort on making it harder to fall into that trap.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89518/new/

https://reviews.llvm.org/D89518