[ELF] Initial support for symbols visibility

[davide]

Summary

This can't be committed until I modify llvm-objdump -t to print visibility properly for ELF symbols, but the lld bits should be all there, so this is ready for review.
I'll add an unit test once I'll figure out the llvm-objdump bits properly.

P.S. This also makes _GLOBAL_OFFSET_TABLE hidden by default. Probably should be committed separately.

[ruiu]

Maybe this patch is too small to review since it doesn't include any use of the newly-added member. If you have another patch to use the member, please send it for review.

[davide]

hmm, maybe I'm misunderstanding what you say but this patch sets also visibility for the global offset table atom. What is unused?

[ruiu]

Apologies, I missed the change you made to ELF/Atom.h. LGTM.

[shankarke]

I dont think we need a separate visibility field in DefinedAtoms. The visibility is already modeled as part of Scope.

<snip>

/// The scope in which this atom is acessible to other atoms.
enum Scope {
¦ scopeTranslationUnit,  ///< Accessible only to atoms in the same translation
¦ ¦ ¦ ¦ ¦ ¦ ¦ ¦ ¦ ¦      ///  unit (e.g. a C static).
¦ scopeLinkageUnit,      ///< Accessible to atoms being linked but not visible
¦ ¦ ¦ ¦ ¦ ¦ ¦ ¦ ¦ ¦      ///  to runtime loader (e.g. visibility=hidden).
¦ scopeGlobal            ///< Accessible to all atoms and visible to runtime
¦ ¦ ¦ ¦ ¦ ¦ ¦ ¦ ¦ ¦      ///  loader (e.g. visibility=default).
};

</end snip>

[• shankar.easwaran]

Yes protected need to be added to this as it's missing.

[davide]

Updated to address Shankar and Joerg comments

[shankarke]

Forgot to mention about your previous change, that you need to add changes to ReaderWriterYAML(YAML Testing) and ReaderNative and WriterNative(Native formats) too.

Adding other reviewers here.

lib/ReaderWriter/ELF/SectionChunks.h

790

This is very confusing, if you see line 771. We set binding for local symbols to be STB_LOCAL if scope is set to scopeTranslationUnit, so visibility would need to be set as STV_DEFAULT ? No ?

804

could this be sym.st_other = sym.st_other & ~0x3 |llvm::ELF::STV_HIDDEN ?

814

assert

include/lld/Core/Atom.h

enum Definition {
     definitionAbsolute,     ///< Asm-only (foo = 10). Not tied to any content.
     definitionUndefined,    ///< Only in .o files to model reference to undef.
     definitionSharedLibrary ///< Only in shared libraries to model export.
   };
 
   /// The scope in which this atom is acessible to other atoms.
   enum Scope {
     scopeTranslationUnit,  ///< Accessible only to atoms in the same translation
-                           ///  unit (e.g. a C static).
+                           ///  unit (e.g. visibility=internal).
     scopeLinkageUnit,      ///< Accessible to atoms being linked but not visible
                            ///  to runtime loader (e.g. visibility=hidden).
-    scopeGlobal            ///< Accessible to all atoms and visible to runtime
+    scopeGlobal,           ///< Accessible to all atoms and visible to runtime
                            ///  loader (e.g. visibility=default).
+    scopeProtected         ///< Accessible to all atoms and visible to runtime
+                           ///  loader, but can't be overriden
+                           ///  (e.g. visibility=protected).
   };
 
 
   /// file - returns the File that produced/owns this Atom
   virtual const File& file() const = 0;
 
   /// name - The name of the atom. For a function atom, it is the (mangled)
   /// name of the function.

lib/ReaderWriter/ELF/SectionChunks.h

SymbolTable<ELFT>::SymbolTable(const ELFLinkingContext &context,
   this->_fsize = sizeof(Elf_Sym);
   this->_alignment = sizeof(Elf_Addr);
   this->_type = SHT_SYMTAB;
 }
 
 template <class ELFT>
 void SymbolTable<ELFT>::addDefinedAtom(Elf_Sym &sym, const DefinedAtom *da,
                                        int64_t addr) {
-  unsigned char binding = 0, type = 0;
+  unsigned char binding = 0, type = 0, visibility = 0;
   sym.st_size = da->size();
   DefinedAtom::ContentType ct;
   switch (ct = da->contentType()) {
   case DefinedAtom::typeCode:
   case DefinedAtom::typeStub:
     sym.st_value = addr;
     type = llvm::ELF::STT_FUNC;
     break;
void SymbolTable<ELFT>::addDefinedAtom(Elf_Sym &sym, const DefinedAtom *da,
   if (da->customSectionName() == da->name())
     type = llvm::ELF::STT_SECTION;
 
   if (da->scope() == DefinedAtom::scopeTranslationUnit)
     binding = llvm::ELF::STB_LOCAL;
   else
     binding = llvm::ELF::STB_GLOBAL;
 
+  switch (DefinedAtom::Scope vt = da->scope()) {
+    case DefinedAtom::scopeLinkageUnit:
+      visibility = llvm::ELF::STV_HIDDEN;
+      break;
+    case DefinedAtom::scopeTranslationUnit:
+      visibility = llvm::ELF::STV_INTERNAL;
+      break;
+    case DefinedAtom::scopeGlobal:
+      visibility = llvm::ELF::STV_DEFAULT;
+      break;
+    case DefinedAtom::scopeProtected:
+      visibility = llvm::ELF::STV_PROTECTED;
+      break;
+  }
+  sym.setVisibility(visibility);
   sym.setBindingAndType(binding, type);
 }
 
 template <class ELFT>
 void SymbolTable<ELFT>::addAbsoluteAtom(Elf_Sym &sym, const AbsoluteAtom *aa,
                                         int64_t addr) {
   unsigned char binding = 0, type = 0;
   type = llvm::ELF::STT_OBJECT;
   sym.st_shndx = llvm::ELF::SHN_ABS;
   switch (aa->scope()) {
   case AbsoluteAtom::scopeLinkageUnit:
+    // XXX this is unsafe because it might override
+    // the other bits of st_other.
     sym.st_other = llvm::ELF::STV_HIDDEN;
     binding = llvm::ELF::STB_LOCAL;
     break;
   case AbsoluteAtom::scopeTranslationUnit:
     binding = llvm::ELF::STB_LOCAL;
     break;
   case AbsoluteAtom::scopeGlobal:
     binding = llvm::ELF::STB_GLOBAL;
     break;
+  case AbsoluteAtom::scopeProtected:
+    break;
   }
   sym.st_value = addr;
   sym.setBindingAndType(binding, type);
 }
 
 template <class ELFT>
 void SymbolTable<ELFT>::addSharedLibAtom(Elf_Sym &sym,
                                          const SharedLibraryAtom *aa) {

lib/ReaderWriter/MachO/MachONormalizedFileFromAtoms.cpp

if (_context.exportRestrictMode()) {
         return std::error_code();
       }
     } else {
       scope = N_EXT;
       inGlobalsRegion = true;
       return std::error_code();
     }
     break;
-  }
   llvm_unreachable("atom->scope() unknown enum value");
   }
+}
 
 std::error_code Util::addSymbols(const lld::File &atomFile,
                                  NormalizedFile &file) {
   bool rMode = (_context.outputMachOType() == llvm::MachO::MH_OBJECT);
   // Mach-O symbol table has three regions: locals, globals, undefs.
 
   // Add all local (non-global) symbols in address order
   std::vector<AtomAndIndex> globals;

test/elf/Mips/mips-options-gp0.test

 # Check reading GP0 value from .MIPS.options section
 #
 # RUN: yaml2obj -format=elf %s > %t.o
 # RUN: lld -flavor gnu -target mipsel -e G1 -shared -o %t.so %t.o
 # RUN: llvm-readobj -symbols %t.so | FileCheck -check-prefix=SYM %s
 # RUN: llvm-objdump -s %t.so | FileCheck -check-prefix=SEC %s
 
 # SYM:      Name: L1 (1)
 # SYM-NEXT: Value: 0xCC
 # SYM-NEXT: Size: 4
 # SYM-NEXT: Binding: Local (0x0)
 # SYM-NEXT: Type: Function (0x2)
-# SYM-NEXT: Other: 0
+# SYM-NEXT: Other: 1
 # SYM-NEXT: Section: .text (0x4)
 
 # SYM:      Name: _gp (34)
 # SYM-NEXT: Value: 0x8FF0
 # SYM-NEXT: Size: 0
 # SYM-NEXT: Binding: Global (0x1)
 # SYM-NEXT: Type: Object (0x1)
 # SYM-NEXT: Other: 0

test/elf/Mips/rel-gprel16.test

 # Check R_MIPS_GPREL16 relocation handling.
 #
 # RUN: yaml2obj -format=elf %s > %t-obj
 # RUN: lld -flavor gnu -target mipsel -e G1 -shared -o %t.so %t-obj
 # RUN: llvm-readobj -symbols %t.so | FileCheck -check-prefix=SYM %s
 # RUN: llvm-objdump -s %t.so | FileCheck -check-prefix=SEC %s
 
 # SYM:      Name: L1 (1)
 # SYM-NEXT: Value: 0xCC
 # SYM-NEXT: Size: 4
 # SYM-NEXT: Binding: Local (0x0)
 # SYM-NEXT: Type: Function (0x2)
-# SYM-NEXT: Other: 0
+# SYM-NEXT: Other: 1
 # SYM-NEXT: Section: .text (0x4)
 
 # SYM:      Name: G1 (4)
 # SYM-NEXT: Value: 0xD0
 # SYM-NEXT: Size: 4
 # SYM-NEXT: Binding: Global (0x1)
 # SYM-NEXT: Type: Function (0x2)
 # SYM-NEXT: Other: 0

test/elf/Mips/rel-gprel32.test

 # Check R_MIPS_GPREL32 relocation handling.
 #
 # RUN: yaml2obj -format=elf %s > %t-obj
 # RUN: lld -flavor gnu -target mipsel -o %t-exe %t-obj
 # RUN: llvm-readobj -symbols %t-exe | FileCheck -check-prefix=SYM %s
 # RUN: llvm-objdump -s %t-exe | FileCheck -check-prefix=SEC %s
 
 # SYM:      Name: $L1 (1)
 # SYM-NEXT: Value: 0x400108
 # SYM-NEXT: Size: 4
 # SYM-NEXT: Binding: Local (0x0)
 # SYM-NEXT: Type: Function (0x2)
-# SYM-NEXT: Other: 0
+# SYM-NEXT: Other: 1
 # SYM-NEXT: Section: .text (0x5)
 #
 # SYM:      Name: _gp (212)
 # SYM-NEXT: Value: 0x408FF0
 # SYM-NEXT: Size: 0
 # SYM-NEXT: Binding: Global (0x1)
 # SYM-NEXT: Type: Object (0x1)
 # SYM-NEXT: Other: 0