[PATCH] D29892: ar: add llvm-dlltool support

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: lib/Object/ArchiveWriter.cpp:318
+      if (Symflags & object::SymbolRef::SF_Undefined &&
+          !(Symflags & object::SymbolRef::SF_Weak))
         continue;
----------------
martell wrote:
> rafael wrote:
> > I don't think this is correct in general. For a regular .a file I don't think the .a should include undefined weak symbols in the list.
> For archives containing COFF objects we need to include the undefined weak symbols in the list to support `PE COFF spec (Aux Format 3: Weak Externals)`
> LLD and MSVC Link won't resolve any symbol created with the `createWeakExternal` function without this. 
> I will add a testcase to highlight this..
> 
> Are you saying this specific implementation will affect ELF archives in some way?
Yes, ELF undefined weak symbols would have the flags `SF_Undefined`and `SF_Weak` set, so this would change the behaviour for ELF.

What I would suggest is to change the code in `COFFObjectFile::getSymbolFlags()` to also set the flag `SF_Indirect` if it sees a weak external. Then you can test for presence of the flag `SF_Indirect` here instead of `SF_Weak`.


Repository:
  rL LLVM

https://reviews.llvm.org/D29892