<div dir="ltr">So, we don't want weak undefined symbols to fetch archive members, which sounds fine, but I still doesn't get how is this related to --start-group and --end-group in regular ELF linkers. I'm just wondering... Could you describe a bit?</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 6, 2015 at 7:33 AM, Rafael Espindola via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rafael<br>
Date: Tue Oct  6 09:33:58 2015<br>
New Revision: 249406<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=249406&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=249406&view=rev</a><br>
Log:<br>
Handle strong undefined symbols fetching members after a weak undefined.<br>
<br>
This is a case that requires --start-group --end-group with regular ELF<br>
linkers. Fortunately it is still possible to handle it with lazy symbols without<br>
taking a second look at archives.<br>
<br>
Thanks to Michael Spencer for the bug report.<br>
<br>
Added:<br>
    lld/trunk/test/elf2/Inputs/archive3.s<br>
    lld/trunk/test/elf2/Inputs/archive4.s<br>
Modified:<br>
    lld/trunk/ELF/InputSection.cpp<br>
    lld/trunk/ELF/OutputSections.cpp<br>
    lld/trunk/ELF/OutputSections.h<br>
    lld/trunk/ELF/SymbolTable.cpp<br>
    lld/trunk/ELF/Symbols.h<br>
    lld/trunk/test/elf2/archive.s<br>
<br>
Modified: lld/trunk/ELF/InputSection.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputSection.cpp?rev=249406&r1=249405&r2=249406&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputSection.cpp?rev=249406&r1=249405&r2=249406&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/ELF/InputSection.cpp (original)<br>
+++ lld/trunk/ELF/InputSection.cpp Tue Oct  6 09:33:58 2015<br>
@@ -49,8 +49,7 @@ void InputSection<ELFT>::relocate(<br>
         continue;<br>
       SymVA = getLocalSymVA(Sym, File);<br>
     } else {<br>
-      const auto &Body =<br>
-          *cast<ELFSymbolBody<ELFT>>(File.getSymbolBody(SymIndex));<br>
+      SymbolBody &Body = *File.getSymbolBody(SymIndex);<br>
       SymVA = getSymVA<ELFT>(Body, BssSec);<br>
       if (Target->relocNeedsPlt(Type, Body)) {<br>
         SymVA = PltSec.getEntryAddr(Body);<br>
<br>
Modified: lld/trunk/ELF/OutputSections.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/OutputSections.cpp?rev=249406&r1=249405&r2=249406&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/OutputSections.cpp?rev=249406&r1=249405&r2=249406&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/ELF/OutputSections.cpp (original)<br>
+++ lld/trunk/ELF/OutputSections.cpp Tue Oct  6 09:33:58 2015<br>
@@ -349,13 +349,14 @@ void OutputSection<ELFT>::addSection(Inp<br>
<br>
 template <class ELFT><br>
 typename ELFFile<ELFT>::uintX_t<br>
-lld::elf2::getSymVA(const ELFSymbolBody<ELFT> &S,<br>
-                    const OutputSection<ELFT> &BssSec) {<br>
+lld::elf2::getSymVA(const SymbolBody &S, const OutputSection<ELFT> &BssSec) {<br>
   switch (S.kind()) {<br>
-  case SymbolBody::DefinedSyntheticKind:<br>
-    return cast<DefinedSynthetic<ELFT>>(S).Section.getVA() + S.Sym.st_value;<br>
+  case SymbolBody::DefinedSyntheticKind: {<br>
+    auto &D = cast<DefinedSynthetic<ELFT>>(S);<br>
+    return D.Section.getVA() + D.Sym.st_value;<br>
+  }<br>
   case SymbolBody::DefinedAbsoluteKind:<br>
-    return S.Sym.st_value;<br>
+    return cast<DefinedAbsolute<ELFT>>(S).Sym.st_value;<br>
   case SymbolBody::DefinedRegularKind: {<br>
     const auto &DR = cast<DefinedRegular<ELFT>>(S);<br>
     const InputSection<ELFT> *SC = &DR.Section;<br>
@@ -368,9 +369,9 @@ lld::elf2::getSymVA(const ELFSymbolBody<<br>
   case SymbolBody::UndefinedKind:<br>
     return 0;<br>
   case SymbolBody::LazyKind:<br>
-    break;<br>
+    assert(S.isUsedInRegularObj() && "Lazy symbol reached writer");<br>
+    return 0;<br>
   }<br>
-  llvm_unreachable("Lazy symbol reached writer");<br>
 }<br>
<br>
 template <class ELFT><br>
@@ -409,8 +410,6 @@ void StringTableSection<Is64Bits>::write<br>
 }<br>
<br>
 template <class ELFT> bool lld::elf2::includeInSymtab(const SymbolBody &B) {<br>
-  if (B.isLazy())<br>
-    return false;<br>
   if (!B.isUsedInRegularObj())<br>
     return false;<br>
<br>
@@ -521,21 +520,20 @@ void SymbolTableSection<ELFT>::writeGlob<br>
     if (StrTabSec.isDynamic() && !includeInDynamicSymtab(*Body))<br>
       continue;<br>
<br>
-    const auto &EBody = *cast<ELFSymbolBody<ELFT>>(Body);<br>
-    const Elf_Sym &InputSym = EBody.Sym;<br>
     auto *ESym = reinterpret_cast<Elf_Sym *>(Buf);<br>
     Buf += sizeof(*ESym);<br>
+<br>
     ESym->st_name = StrTabSec.getFileOff(Name);<br>
<br>
     const OutputSection<ELFT> *Out = nullptr;<br>
     const InputSection<ELFT> *Section = nullptr;<br>
<br>
-    switch (EBody.kind()) {<br>
+    switch (Body->kind()) {<br>
     case SymbolBody::DefinedSyntheticKind:<br>
       Out = &cast<DefinedSynthetic<ELFT>>(Body)->Section;<br>
       break;<br>
     case SymbolBody::DefinedRegularKind:<br>
-      Section = &cast<DefinedRegular<ELFT>>(EBody).Section;<br>
+      Section = &cast<DefinedRegular<ELFT>>(Body)->Section;<br>
       break;<br>
     case SymbolBody::DefinedCommonKind:<br>
       Out = &BssSec;<br>
@@ -543,25 +541,33 @@ void SymbolTableSection<ELFT>::writeGlob<br>
     case SymbolBody::UndefinedKind:<br>
     case SymbolBody::DefinedAbsoluteKind:<br>
     case SymbolBody::SharedKind:<br>
-      break;<br>
     case SymbolBody::LazyKind:<br>
-      llvm_unreachable("Lazy symbol got to output symbol table!");<br>
+      break;<br>
     }<br>
<br>
-    unsigned char Binding = InputSym.getBinding();<br>
-    unsigned char Visibility = EBody.getMostConstrainingVisibility();<br>
+    unsigned char Binding = Body->isWeak() ? STB_WEAK : STB_GLOBAL;<br>
+    unsigned char Type = STT_NOTYPE;<br>
+    uintX_t Size = 0;<br>
+    if (const auto *EBody = dyn_cast<ELFSymbolBody<ELFT>>(Body)) {<br>
+      const Elf_Sym &InputSym = EBody->Sym;<br>
+      Binding = InputSym.getBinding();<br>
+      Type = InputSym.getType();<br>
+      Size = InputSym.st_size;<br>
+    }<br>
+<br>
+    unsigned char Visibility = Body->getMostConstrainingVisibility();<br>
     if (Visibility != STV_DEFAULT && Visibility != STV_PROTECTED)<br>
       Binding = STB_LOCAL;<br>
<br>
-    ESym->setBindingAndType(Binding, InputSym.getType());<br>
-    ESym->st_size = InputSym.st_size;<br>
+    ESym->setBindingAndType(Binding, Type);<br>
+    ESym->st_size = Size;<br>
     ESym->setVisibility(Visibility);<br>
-    ESym->st_value = getSymVA(EBody, BssSec);<br>
+    ESym->st_value = getSymVA(*Body, BssSec);<br>
<br>
     if (Section)<br>
       Out = Section->getOutputSection();<br>
<br>
-    if (InputSym.isAbsolute())<br>
+    if (isa<DefinedAbsolute<ELFT>>(Body))<br>
       ESym->st_shndx = SHN_ABS;<br>
     else if (Out)<br>
       ESym->st_shndx = Out->getSectionIndex();<br>
@@ -629,17 +635,14 @@ template class SymbolTableSection<ELF32B<br>
 template class SymbolTableSection<ELF64LE>;<br>
 template class SymbolTableSection<ELF64BE>;<br>
<br>
-template ELFFile<ELF32LE>::uintX_t<br>
-getSymVA(const ELFSymbolBody<ELF32LE> &, const OutputSection<ELF32LE> &);<br>
-<br>
-template ELFFile<ELF32BE>::uintX_t<br>
-getSymVA(const ELFSymbolBody<ELF32BE> &, const OutputSection<ELF32BE> &);<br>
-<br>
-template ELFFile<ELF64LE>::uintX_t<br>
-getSymVA(const ELFSymbolBody<ELF64LE> &, const OutputSection<ELF64LE> &);<br>
-<br>
-template ELFFile<ELF64BE>::uintX_t<br>
-getSymVA(const ELFSymbolBody<ELF64BE> &, const OutputSection<ELF64BE> &);<br>
+template ELFFile<ELF32LE>::uintX_t getSymVA(const SymbolBody &,<br>
+                                            const OutputSection<ELF32LE> &);<br>
+template ELFFile<ELF32BE>::uintX_t getSymVA(const SymbolBody &,<br>
+                                            const OutputSection<ELF32BE> &);<br>
+template ELFFile<ELF64LE>::uintX_t getSymVA(const SymbolBody &,<br>
+                                            const OutputSection<ELF64LE> &);<br>
+template ELFFile<ELF64BE>::uintX_t getSymVA(const SymbolBody &,<br>
+                                            const OutputSection<ELF64BE> &);<br>
<br>
 template ELFFile<ELF32LE>::uintX_t<br>
 getLocalSymVA(const ELFFile<ELF32LE>::Elf_Sym *, const ObjectFile<ELF32LE> &);<br>
<br>
Modified: lld/trunk/ELF/OutputSections.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/OutputSections.h?rev=249406&r1=249405&r2=249406&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/OutputSections.h?rev=249406&r1=249405&r2=249406&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/ELF/OutputSections.h (original)<br>
+++ lld/trunk/ELF/OutputSections.h Tue Oct  6 09:33:58 2015<br>
@@ -34,7 +34,7 @@ template <class ELFT> class ELFSymbolBod<br>
<br>
 template <class ELFT><br>
 typename llvm::object::ELFFile<ELFT>::uintX_t<br>
-getSymVA(const ELFSymbolBody<ELFT> &S, const OutputSection<ELFT> &BssSec);<br>
+getSymVA(const SymbolBody &S, const OutputSection<ELFT> &BssSec);<br>
<br>
 template <class ELFT><br>
 typename llvm::object::ELFFile<ELFT>::uintX_t<br>
<br>
Modified: lld/trunk/ELF/SymbolTable.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SymbolTable.cpp?rev=249406&r1=249405&r2=249406&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SymbolTable.cpp?rev=249406&r1=249405&r2=249406&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/ELF/SymbolTable.cpp (original)<br>
+++ lld/trunk/ELF/SymbolTable.cpp Tue Oct  6 09:33:58 2015<br>
@@ -256,10 +256,23 @@ void SymbolTable::addLazy(Lazy *New) {<br>
   if (Sym->Body == New)<br>
     return;<br>
   SymbolBody *Existing = Sym->Body;<br>
-  if (Existing->isDefined() || Existing->isLazy() || Existing->isWeak())<br>
+  if (Existing->isDefined() || Existing->isLazy())<br>
     return;<br>
   Sym->Body = New;<br>
   assert(Existing->isUndefined() && "Unexpected symbol kind.");<br>
+<br>
+  // Weak undefined symbols should not fetch members from archives.<br>
+  // If we were to keep old symbol we would not know that an archive member was<br>
+  // available if a strong undefined symbol shows up afterwards in the link.<br>
+  // If a strong undefined symbol never shows up, this lazy symbol will<br>
+  // get to the end of the link and must be treated as the weak undefined one.<br>
+  // We set UsedInRegularObj in a similar way to what is done with shared<br>
+  // symbols and mark it as weak to reduce how many special cases are needed.<br>
+  if (Existing->isWeak()) {<br>
+    New->setUsedInRegularObj();<br>
+    New->setWeak();<br>
+    return;<br>
+  }<br>
   addMemberFile(New);<br>
 }<br>
<br>
<br>
Modified: lld/trunk/ELF/Symbols.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Symbols.h?rev=249406&r1=249405&r2=249406&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Symbols.h?rev=249406&r1=249405&r2=249406&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/ELF/Symbols.h (original)<br>
+++ lld/trunk/ELF/Symbols.h Tue Oct  6 09:33:58 2015<br>
@@ -103,7 +103,7 @@ protected:<br>
   }<br>
<br>
   const unsigned SymbolKind : 8;<br>
-  const unsigned IsWeak : 1;<br>
+  unsigned IsWeak : 1;<br>
   unsigned MostConstrainingVisibility : 2;<br>
   unsigned IsUsedInRegularObj : 1;<br>
   unsigned IsUsedInDynamicReloc : 1;<br>
@@ -282,6 +282,9 @@ public:<br>
   // was already returned.<br>
   std::unique_ptr<InputFile> getMember();<br>
<br>
+  void setWeak() { IsWeak = true; }<br>
+  void setUsedInRegularObj() { IsUsedInRegularObj = true; }<br>
+<br>
 private:<br>
   ArchiveFile *File;<br>
   const llvm::object::Archive::Symbol Sym;<br>
<br>
Added: lld/trunk/test/elf2/Inputs/archive3.s<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/test/elf2/Inputs/archive3.s?rev=249406&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/test/elf2/Inputs/archive3.s?rev=249406&view=auto</a><br>
==============================================================================<br>
--- lld/trunk/test/elf2/Inputs/archive3.s (added)<br>
+++ lld/trunk/test/elf2/Inputs/archive3.s Tue Oct  6 09:33:58 2015<br>
@@ -0,0 +1,2 @@<br>
+.global bar<br>
+bar:<br>
<br>
Added: lld/trunk/test/elf2/Inputs/archive4.s<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/test/elf2/Inputs/archive4.s?rev=249406&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/test/elf2/Inputs/archive4.s?rev=249406&view=auto</a><br>
==============================================================================<br>
--- lld/trunk/test/elf2/Inputs/archive4.s (added)<br>
+++ lld/trunk/test/elf2/Inputs/archive4.s Tue Oct  6 09:33:58 2015<br>
@@ -0,0 +1 @@<br>
+.quad bar<br>
<br>
Modified: lld/trunk/test/elf2/archive.s<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/test/elf2/archive.s?rev=249406&r1=249405&r2=249406&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/test/elf2/archive.s?rev=249406&r1=249405&r2=249406&view=diff</a><br>
==============================================================================<br>
--- lld/trunk/test/elf2/archive.s (original)<br>
+++ lld/trunk/test/elf2/archive.s Tue Oct  6 09:33:58 2015<br>
@@ -1,8 +1,10 @@<br>
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t<br>
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %S/Inputs/archive.s -o %t2<br>
 # RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %S/Inputs/archive2.s -o %t3<br>
-# RUN: llvm-ar rcs %tar %t2 %t3<br>
-# RUN: lld -flavor gnu2 %t %tar -o %tout<br>
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %S/Inputs/archive3.s -o %t4<br>
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %S/Inputs/archive4.s -o %t5<br>
+# RUN: llvm-ar rcs %tar %t2 %t3 %t4<br>
+# RUN: lld -flavor gnu2 %t %tar %t5 -o %tout<br>
 # RUN: llvm-nm %tout | FileCheck %s<br>
 # REQUIRES: x86<br>
<br>
@@ -13,6 +15,11 @@<br>
 .weak foo<br>
 .quad foo<br>
<br>
-# CHECK: T _start<br>
-# CHECK: T end<br>
-# CHECK: w foo<br>
+.weak bar<br>
+.quad bar<br>
+<br>
+<br>
+# CHECK:      T _start<br>
+# CHECK-NEXT: T bar<br>
+# CHECK-NEXT: T end<br>
+# CHECK-NEXT: w foo<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>