[lld] [ELF] Fix linking when a regular object defines a symbol that is used in a DSO

[rafaelauler]

Summary

When a shared lib has an undefined symbol that is defined in a regular object (the program), the final executable must export this symbol in the dynamic symbol table. However, in the current logic, lld only puts the symbol in the dynamic symbol table if the symbol is weak. This patch fixes lld to put the symbol in the dynamic symbol table regardless if it is weak or not.

This caused a problem in FreeBSD10, whose programs link against a crt1.o that defines the symbol __progname, which is, in turn, undefined in libc.so.7 and will only be resolved in runtime.

[shankarke]

LGTM.

[rafaelauler]

Hi Shankar,

I don't have commit access, could you please commit it?

Thanks,
Rafael Auler

[rafaelauler]

Hi Rui,

I updated the patch to use obj2yaml and yaml2obj to avoid binary files. While I was able to do this to the simple defobj.o file, I was not able to convert the YAML form of libundef2.so to a .so file. The error I got was:

error: Unknown section referenced: '.dynsym' at YAML section '.hash'.

Then I investigated my libundef2.so yaml representation and noticed that it was really missing .dymsym. I looked obj2yaml's source code and found this:

// Dump sections
for (const Elf_Shdr &Sec : Obj.sections()) {
  switch (Sec.sh_type) {
  case ELF::SHT_NULL:
  case ELF::SHT_SYMTAB:
  case ELF::SHT_DYNSYM:
  case ELF::SHT_STRTAB:
    // Do not dump these sections.

...which suggested that the tool currently does not support dumping the dynamic symbol table.

Therefore, my new patch still depends on uploading the libundef2.so binary file. Is there any problem?

Best regards,
Rafael Auler

[ruiu]

If the tool does not support that and if it would take time to add the
feature, it's OK to check in binary files for now.

But your patch does not contain the .so file because diff does not compare
binary files. Please send me the file so that I can submit.

[rafaelauler]

I saw some examples from ELF/MIPS testing and updated my patch to generate the shared library on the fly, in this way we avoid the binary file.

include/lld/ReaderWriter/ELFLinkingContext.h

public:
 
   // By default, the linker would merge sections that are read only with
   // segments that have read and execute permissions. When the user specifies a
   // flag --rosegment, a separate segment needs to be created.
   bool mergeRODataToTextSegment() const { return _mergeRODataToTextSegment; }
 
   void setCreateSeparateROSegment() { _mergeRODataToTextSegment = false; }
 
-  bool hasCoalescedWeakPair(StringRef name) const {
-    return _weakCoalescedSymbols.count(name) != 0;
+  bool hasCoalescedSharedLibPair(StringRef name) const {
+    return _sharedLibCoalescedSymbols.count(name) != 0;
   }
 
 private:
   ELFLinkingContext() LLVM_DELETED_FUNCTION;
 
 protected:
   ELFLinkingContext(llvm::Triple, std::unique_ptr<TargetHandlerBase>);
 
protected:
   StringRef _dynamicLinkerPath;
   StringRefVector _initFunctions;
   StringRefVector _finiFunctions;
   StringRef _sysrootPath;
   StringRef _soname;
   StringRefVector _rpathList;
   StringRefVector _rpathLinkList;
   std::map<std::string, uint64_t> _absoluteSymbols;
-  llvm::StringSet<> _weakCoalescedSymbols;
+  llvm::StringSet<> _sharedLibCoalescedSymbols;
 };
 } // end namespace lld
 
 #endif

lib/ReaderWriter/ELF/ELFLinkingContext.cpp

std::unique_ptr<File> ELFLinkingContext::createUndefinedSymbolFile() const {
   std::unique_ptr<SimpleFile> undefinedSymFile(
       new SimpleFile("command line option -u"));
   for (auto undefSymStr : _initialUndefinedSymbols)
     undefinedSymFile->addAtom(*(new (_allocator) CommandLineUndefinedAtom(
         *undefinedSymFile, undefSymStr)));
   return std::move(undefinedSymFile);
 }
 
-static bool isSharedWeakAtom(const UndefinedAtom *ua) {
-  return ua->canBeNull() != UndefinedAtom::canBeNullNever &&
-         isa<SharedLibraryFile>(ua->file());
-}
-
 void ELFLinkingContext::notifySymbolTableCoalesce(const Atom *existingAtom,
                                                   const Atom *newAtom,
                                                   bool &useNew) {
   // First suppose that the `existingAtom` is defined
   // and the `newAtom` is undefined.
   auto *da = dyn_cast<DefinedAtom>(existingAtom);
   auto *ua = dyn_cast<UndefinedAtom>(newAtom);
   if (!da && !ua) {
     // Then try to reverse the assumption.
     da = dyn_cast<DefinedAtom>(newAtom);
     ua = dyn_cast<UndefinedAtom>(existingAtom);
   }
 
-  if (da && ua && da->scope() == Atom::scopeGlobal && isSharedWeakAtom(ua))
-    // If strong defined atom coalesces away weak atom declared
-    // in the shared object the strong atom needs to be dynamicaly exported.
+  if (da && ua && da->scope() == Atom::scopeGlobal &&
+      isa<SharedLibraryFile>(ua->file()))
+    // If strong defined atom coalesces away an atom declared
+    // in the shared object the strong atom needs to be dynamically exported.
     // Save its name.
-    _weakCoalescedSymbols.insert(ua->name());
+    _sharedLibCoalescedSymbols.insert(ua->name());
 }
 
 } // end namespace lld

lib/ReaderWriter/ELF/OutputELFWriter.h

 template <class ELFT>
 void OutputELFWriter<ELFT>::buildDynamicSymbolTable(const File &file) {
   ScopedTask task(getDefaultDomain(), "buildDynamicSymbolTable");
   for (auto sec : this->_layout.sections())
     if (auto section = dyn_cast<AtomSection<ELFT>>(sec))
       for (const auto &atom : section->atoms()) {
         const DefinedAtom *da = dyn_cast<const DefinedAtom>(atom->_atom);
         if (da && (da->dynamicExport() == DefinedAtom::dynamicExportAlways ||
-                   _context.hasCoalescedWeakPair(da->name())))
+                   _context.hasCoalescedSharedLibPair(da->name())))
           _dynamicSymbolTable->addSymbol(atom->_atom, section->ordinal(),
                                          atom->_virtualAddr, atom);
       }
   for (const auto &sla : file.sharedLibrary()) {
     if (isDynSymEntryRequired(sla))
       _dynamicSymbolTable->addSymbol(sla, ELF::SHN_UNDEF);
     if (isNeededTagRequired(sla))
       _soNeeded.insert(sla->loadName());

test/elf/Inputs/undef2-so.o.yaml

+---
+FileHeader:      
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
+Sections:        
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x0000000000000001
+    Content:         554889E5488B05000000008B005DC3
+  - Name:            .rela.text
+    Type:            SHT_RELA
+    Link:            .symtab
+    AddressAlign:    0x0000000000000008
+    Info:            .text
+    Relocations:     
+      - Offset:          0x0000000000000007
+        Symbol:          myexportedsymbol
+        Type:            R_X86_64_GOTPCREL
+        Addend:          -4
+  - Name:            .data
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC ]
+    AddressAlign:    0x0000000000000001
+    Content:         ''
+  - Name:            .bss
+    Type:            SHT_NOBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC ]
+    AddressAlign:    0x0000000000000001
+    Content:         ''
+Symbols:         
+  Local:           
+    - Name:            .text
+      Type:            STT_SECTION
+      Section:         .text
+    - Name:            .data
+      Type:            STT_SECTION
+      Section:         .data
+    - Name:            .bss
+      Type:            STT_SECTION
+      Section:         .bss
+  Global:          
+    - Name:            func
+      Type:            STT_FUNC
+      Section:         .text
+      Size:            0x000000000000000F
+    - Name:            _GLOBAL_OFFSET_TABLE_
+    - Name:            myexportedsymbol

test/elf/undef-from-dso-to-main.test

+# Tests that a reference from a DSO to a regular object
+# forces the final executable to export the symbol.
+
+#RUN: yaml2obj -format=elf %p/Inputs/undef2-so.o.yaml -o=%t.o.so
+#RUN: lld -flavor gnu  -target x86_64 -shared %t.o.so -o %T/libundef2.so
+#RUN: yaml2obj -format=elf %s -o=%t.o
+#RUN: lld -flavor gnu  -target x86_64 %t.o -L%T -lundef2 -o %t1
+#RUN: llvm-readobj -dyn-symbols %t1 | FileCheck -check-prefix CHECKSYMS %s
+
+---
+FileHeader:      
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
+Sections:        
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x0000000000000001
+    Content:         C3
+  - Name:            .data
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC ]
+    AddressAlign:    0x0000000000000001
+    Content:         ''
+  - Name:            .bss
+    Type:            SHT_NOBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC ]
+    AddressAlign:    0x0000000000000004
+    Content:         '002E7379'
+Symbols:         
+  Local:           
+    - Name:            .text
+      Type:            STT_SECTION
+      Section:         .text
+    - Name:            .data
+      Type:            STT_SECTION
+      Section:         .data
+    - Name:            .bss
+      Type:            STT_SECTION
+      Section:         .bss
+  Global:          
+    - Name:            myexportedsymbol
+      Type:            STT_OBJECT
+      Section:         .bss
+      Size:            0x0000000000000004
+    - Name:            _start
+      Section:         .text
+      Size:            0x0000000000000001
+
+#CHECKSYMS: myexportedsymbol