[LLD] [COFF] Cope with GCC produced weak aliases referring to comdat functions

[mstorsjo]

Summary

For certain cases of inline functions written to comdat sections, GCC 5.x produces a weak symbol in addition, which would end up undefined in some cases.

This no longer seems to happen with GCC 6.x or newer, so this might not be strictly necessary. Also, when linking clang.exe, almost all symbols come from static libraries, so they just end up left as Lazy instead of Undefined, which makes the link succeed.

[ruiu]

Do you know why gcc creates this .weak.* symbol in the first place, and is there any real use of .weak.* symbol in the wild?

[mstorsjo]

In D52602#1248674, @ruiu wrote:

Do you know why gcc creates this .weak.* symbol in the first place, and is there any real use of .weak.* symbol in the wild?

For the normal case of weak symbols (if the user explicitly uses __attribute__((weak))), GCC creates such a symbol because there is no other symbol available for the concrete implementation of the function. For this particular case in combination with inline functions, I think GCC didn't need to produce such a symbol (and didn't need to produce any weak alias at all), as GCC 6.x and newer no longer does it.

If you mean if any actual code in the wild intentionally uses __attribute__((weak)) for COFF targets? Very little I think, but I have seen it in use within libgcc use it in a few places at least.

[mstorsjo]

Also, just to be clear - this patch doesn't skip all of these symbols. Only in the case if such a symbol points to a discarded comdat section, we skip the symbol so that we don't fail the link just for an unused symbol.

[ruiu]

The "proper" way of implementing a weak alias is to embed a "/alternatename" command line option to a .drectve section, so it is unfortunate that gcc took a different approach.

Is there any way to distinguish a symbol explicitly annotated with __attribute__((weak)) and an inline function?

[mstorsjo]

In D52602#1249419, @ruiu wrote:

The "proper" way of implementing a weak alias is to embed a "/alternatename" command line option to a .drectve section, so it is unfortunate that gcc took a different approach.

Is there any way to distinguish a symbol explicitly annotated with __attribute__((weak)) and an inline function?

Yes, they are completely different things. A normal inline function with GCC works just as it does with MSVC, you get a COMDAT symbol. A __attribute__((weak)) func() symbol gets you a COFF weak external named func, which points at .weak.func.<somename>, which LLD can handle just fine (MSVC import libraries use COFF weak externals occasionally as well).

There's just two minor extra details:

• As an "extra feature" maybe, GCC/binutils allows you to have multiple distinct weak implementations of a function. If there's no strong definition, the linker picks the first weak one it saw. This is handled by the other patch, D52601, and IMO is rather straightforward.
• As a bug(?) in GCC 5.x (and maybe older), certain special cases of inline templated functions in C++ can produce a weak symbol pointing to the COMDAT symbol. This is handled by this patch.

[mstorsjo]

Ping

[rnk]

What the heck, one more weird thing.

lld/trunk/COFF/InputFiles.cpp

 
 Symbol *ObjFile::createRegular(COFFSymbolRef Sym) {
   SectionChunk *SC = SparseChunks[Sym.getSectionNumber()];
   if (Sym.isExternal()) {
     StringRef Name;
     COFFObj->getSymbolName(Sym, Name);
     if (SC)
       return Symtab->addRegular(this, Name, Sym.getGeneric(), SC);
+    // For MinGW symbols named .weak.* that point to a discarded section,
+    // don't create an Undefined symbol. If nothing ever refers to the symbol,
+    // everything should be fine. If something actually refers to the symbol
+    // (e.g. the undefined weak alias), linking will fail due to undefined
+    // references at the end.
+    if (Config->MinGW && Name.startswith(".weak."))
+      return nullptr;
     return Symtab->addUndefined(Name, this, false);
   }
   if (SC)
     return make<DefinedRegular>(this, /*Name*/ "", false,
                                 /*IsExternal*/ false, Sym.getGeneric(), SC);
   return nullptr;
 }
 

lld/trunk/test/COFF/comdat-weak.test

+RUN: lld-link -lldmingw %S/Inputs/inline-weak.o %S/Inputs/inline-weak2.o -out:%t.exe
+
+When compiling certain forms of templated inline functions, some
+versions of GCC (tested with 5.4) produces a weak symbol for the function.
+Newer versions of GCC don't do this though.
+
+The bundled object files are an example of that, they can be produced
+with test code like this:
+
+$ cat inline-weak.h
+class MyClass {
+public:
+    template<typename... _Args> int get(_Args&&... args) {
+        return a;
+    }
+private:
+    int a;
+};
+
+$ cat inline-weak.cpp
+#include "inline-weak.h"
+
+int get(MyClass& a);
+
+int main(int argc, char* argv[]) {
+    MyClass a;
+    int ret = a.get();
+    ret += get(a);
+    return ret;
+}
+extern "C" void mainCRTStartup(void) {
+    main(0, (char**)0);
+}
+extern "C" void __main(void) {
+}
+
+$ cat inline-weak2.cpp
+#include "inline-weak.h"
+
+int get(MyClass& a) {
+    return a.get();
+}
+
+$ x86_64-w64-mingw32-g++ -std=c++11 -c inline-weak.cpp
+$ x86_64-w64-mingw32-g++ -std=c++11 -c inline-weak2.cpp
+
+$ x86_64-w64-mingw32-nm inline-weak.o | grep MyClass3get
+0000000000000000 p .pdata$_ZN7MyClass3getIJEEEiDpOT_
+0000000000000000 t .text$_ZN7MyClass3getIJEEEiDpOT_
+0000000000000000 T .weak._ZN7MyClass3getIIEEEiDpOT_.main
+0000000000000000 r .xdata$_ZN7MyClass3getIJEEEiDpOT_
+                 w _ZN7MyClass3getIIEEEiDpOT_
+0000000000000000 T _ZN7MyClass3getIJEEEiDpOT_
+
+$ x86_64-w64-mingw32-nm inline-weak2.o | grep MyClass3get
+0000000000000000 p .pdata$_ZN7MyClass3getIJEEEiDpOT_
+0000000000000000 t .text$_ZN7MyClass3getIJEEEiDpOT_
+0000000000000000 T .weak._ZN7MyClass3getIIEEEiDpOT_._Z3getR7MyClass
+0000000000000000 r .xdata$_ZN7MyClass3getIJEEEiDpOT_
+                 w _ZN7MyClass3getIIEEEiDpOT_
+0000000000000000 T _ZN7MyClass3getIJEEEiDpOT_
+
+This can't be reproduced by assembling .s files with llvm-mc, since that
+always produces a symbol named .weak.<weaksymbol>.default, therefore
+the test uses prebuilt object files instead.
+
+In these cases, the undefined weak symbol points to the regular symbol
+.weak._ZN7MyClass3getIIEEEiDpOT_.<othersymbol>, where <othersymbol>
+varies among the object files that emit the same function. This regular
+symbol points to the same location as the comdat function
+_ZN7MyClass3getIJEEEiDpOT_.
+
+When linking, the comdat section from the second object file gets
+discarded, as it matches the one that already exists. This means that
+the uniquely named symbol .weak.<weakname>.<othername> points to a
+discarded section chunk.
+
+Previously, this would have triggered adding an Undefined symbol for
+this case, which would later break linking. However, also previously,
+if the second object file is linked in via a static library, this
+leftover symbol is retained as a Lazy symbol, which would make the link
+succeed.