[ELF] Fix the ELF type for the linker-generated _DYNAMIC symbol

[davide]

Summary

This was wrong before, because the type set was None. Instead it's supposed to be Object (checked both what ld.bfd and gold produce).

[ruiu]

Did it cause any issues that _DYNAMIC symbol has STT_NOTYPE instead of STT_OBJECT?

[davide]

No, not really. I just wanted to be compliant with what gold produces. I can't find easily documentation on _DYNAMIC, so, I can't verify if it's a strict requirement. OTOH, I also don't see why we should diverge when doing what others do comes at no-cost. Your mileage may vary, of course =)

[grimar]

I also checked the gold/bfd output when implemented the _DYNAMIC and yes, it is STT_OBJECT for both.
Please refer to my comment about that (at the end):
http://reviews.llvm.org/D17607#362668

But seems that nobody remembers why it was done in that way:
https://sourceware.org/ml/binutils/2002-03/msg00360.html

So I decided not to do that without need. I think it does not affect anything.

[davide]

On a second though, let's abandon this review.
I think maintaining this code is little effort but it's also little effort introducing this if really needed.
If Ian Taylor doesn't remember the need for it, that says a lot.

[grimar]

In D17876#367841, @davide wrote:

If Ian Taylor doesn't remember the need for it, that says a lot.

Looking on a talk date (2002-03), that was 14 years ago. So if he did not remember it that time, I think nowadays that's even more true :)

[ruiu]

Sure. But that fact that even Ian were not sure why _DYNAMIC symbol has STT_OBJECT type.
It's worth noting as a comment. I'll add that.

ELF/OutputSections.cpp

     uint8_t Type = STT_NOTYPE;
     uintX_t Size = 0;
     if (const Elf_Sym *InputSym = getElfSym<ELFT>(*Body)) {
       Type = InputSym->getType();
       Size = InputSym->st_size;
     } else if (auto *C = dyn_cast<DefinedCommon>(Body)) {
       Type = STT_OBJECT;
       Size = C->Size;
+    } else if (isa<DefinedSynthetic<ELFT>>(Body)) {
+      Type = Body->getType();
     }
 
     ESym->setBindingAndType(getSymbolBinding(Body), Type);
     ESym->st_size = Size;
     ESym->st_name = StrOff;
     ESym->setVisibility(Body->getVisibility());
     ESym->st_value = Body->getVA<ELFT>();
 

ELF/SymbolTable.h

public:
   const std::vector<std::unique_ptr<SharedFile<ELFT>>> &getSharedFiles() const {
     return SharedFiles;
   }
 
   SymbolBody *addUndefined(StringRef Name);
   SymbolBody *addUndefinedOpt(StringRef Name);
   SymbolBody *addAbsolute(StringRef Name, Elf_Sym &ESym);
   SymbolBody *addSynthetic(StringRef Name, OutputSectionBase<ELFT> &Section,
-                           uintX_t Value, uint8_t Visibility);
+                           uintX_t Value, uint8_t Visibility, uint8_t Type);
   SymbolBody *addIgnored(StringRef Name);
 
   void scanShlibUndefined();
   SymbolBody *find(StringRef Name);
   void wrap(StringRef Name);
   InputFile *findFile(SymbolBody *B);
 
 private:

ELF/SymbolTable.cpp

SymbolBody *SymbolTable<ELFT>::addAbsolute(StringRef Name, Elf_Sym &ESym) {
   auto *Sym = new (Alloc) DefinedRegular<ELFT>(Name, ESym, nullptr);
   resolve(Sym);
   return Sym;
 }
 
 template <class ELFT>
 SymbolBody *SymbolTable<ELFT>::addSynthetic(StringRef Name,
                                             OutputSectionBase<ELFT> &Sec,
-                                            uintX_t Val, uint8_t Visibility) {
-  auto *Sym = new (Alloc) DefinedSynthetic<ELFT>(Name, Val, Sec, Visibility);
+                                            uintX_t Val, uint8_t Visibility,
+                                            uint8_t Type) {
+  auto *Sym = new (Alloc) DefinedSynthetic<ELFT>(Name, Val, Sec, Visibility,
+                                                 Type);
   resolve(Sym);
   return Sym;
 }
 
 // Add Name as an "ignored" symbol. An ignored symbol is a regular
 // linker-synthesized defined symbol, but it is not recorded to the output
 // file's symbol table. Such symbols are useful for some linker-defined symbols.
 template <class ELFT>

ELF/Symbols.h

public:
   bool isUsedInRegularObj() const { return IsUsedInRegularObj; }
   bool isFunc() const { return Type == llvm::ELF::STT_FUNC; }
   bool isTls() const { return Type == llvm::ELF::STT_TLS; }
 
   // Returns the symbol name.
   StringRef getName() const { return Name; }
 
   uint8_t getVisibility() const { return Visibility; }
+  uint8_t getType() const { return Type; }
 
   unsigned DynsymIndex = 0;
   uint32_t GlobalDynIndex = -1;
   uint32_t GotIndex = -1;
   uint32_t GotPltIndex = -1;
   uint32_t PltIndex = -1;
   bool hasGlobalDynIndex() { return GlobalDynIndex != uint32_t(-1); }
   bool isInGot() const { return GotIndex != -1U; }

 // The difference from the regular symbol is that DefinedSynthetic symbols
 // don't belong to any input files or sections. Thus, its constructor
 // takes an output section to calculate output VA, etc.
 template <class ELFT> class DefinedSynthetic : public Defined {
 public:
   typedef typename llvm::object::ELFFile<ELFT>::Elf_Sym Elf_Sym;
   typedef typename llvm::object::ELFFile<ELFT>::uintX_t uintX_t;
   DefinedSynthetic(StringRef N, uintX_t Value, OutputSectionBase<ELFT> &Section,
-                   uint8_t Visibility);
+                   uint8_t Visibility, uint8_t Type);
 
   static bool classof(const SymbolBody *S) {
     return S->kind() == SymbolBody::DefinedSyntheticKind;
   }
 
   uintX_t Value;
   const OutputSectionBase<ELFT> &Section;
 };

ELF/Symbols.cpp

UndefinedElf<ELFT>::UndefinedElf(StringRef N, const Elf_Sym &Sym)
     : Undefined(SymbolBody::UndefinedElfKind, N,
                 Sym.getBinding() == llvm::ELF::STB_WEAK, Sym.getVisibility(),
                 Sym.getType()),
       Sym(Sym) {}
 
 template <typename ELFT>
 DefinedSynthetic<ELFT>::DefinedSynthetic(StringRef N, uintX_t Value,
                                          OutputSectionBase<ELFT> &Section,
-                                         uint8_t Visibility)
-    : Defined(SymbolBody::DefinedSyntheticKind, N, false, Visibility,
-              0 /* Type */),
+                                         uint8_t Visibility, uint8_t Type)
+    : Defined(SymbolBody::DefinedSyntheticKind, N, false, Visibility, Type),
       Value(Value), Section(Section) {}
 
 DefinedCommon::DefinedCommon(StringRef N, uint64_t Size, uint64_t Alignment,
                              bool IsWeak, uint8_t Visibility)
     : Defined(SymbolBody::DefinedCommonKind, N, IsWeak, Visibility,
               0 /* Type */) {
   MaxAlignment = Alignment;
   this->Size = Size;

ELF/Writer.cpp

   // symbols for sections, so that the runtime can get the start and end
   // addresses of each section by section name. Add such symbols.
   if (!Config->Relocatable) {
     addStartEndSymbols();
     for (OutputSectionBase<ELFT> *Sec : RegularSections)
       addStartStopSymbols(Sec);
   }
   if (isOutputDynamic())
-    Symtab.addSynthetic("_DYNAMIC", *Out<ELFT>::Dynamic, 0, STV_HIDDEN);
+    Symtab.addSynthetic("_DYNAMIC", *Out<ELFT>::Dynamic, 0, STV_HIDDEN,
+                        STT_OBJECT);
 
   // Define __rel[a]_iplt_{start,end} symbols if needed.
   addRelIpltSymbols();
 
   // Scan relocations. This must be done after every symbol is declared so that
   // we can correctly decide if a dynamic relocation is needed.
   for (const std::unique_ptr<ObjectFile<ELFT>> &F : Symtab.getObjectFiles()) {
     for (InputSectionBase<ELFT> *C : F->getSections()) {

 }
 
 // The linker is expected to define SECNAME_start and SECNAME_end
 // symbols for a few sections. This function defines them.
 template <class ELFT> void Writer<ELFT>::addStartEndSymbols() {
   auto Define = [&](StringRef Start, StringRef End,
                     OutputSectionBase<ELFT> *OS) {
     if (OS) {
-      Symtab.addSynthetic(Start, *OS, 0, STV_DEFAULT);
-      Symtab.addSynthetic(End, *OS, OS->getSize(), STV_DEFAULT);
+      Symtab.addSynthetic(Start, *OS, 0, STV_DEFAULT, 0 /* Type */);
+      Symtab.addSynthetic(End, *OS, OS->getSize(), STV_DEFAULT,
+                          0 /* Type */);
     } else {
       Symtab.addIgnored(Start);
       Symtab.addIgnored(End);
     }
   };
 
   Define("__preinit_array_start", "__preinit_array_end",
          Out<ELFT>::Dynamic->PreInitArraySec);

   StringRef S = Sec->getName();
   if (!isValidCIdentifier(S))
     return;
   StringSaver Saver(Alloc);
   StringRef Start = Saver.save("__start_" + S);
   StringRef Stop = Saver.save("__stop_" + S);
   if (SymbolBody *B = Symtab.find(Start))
     if (B->isUndefined())
-      Symtab.addSynthetic(Start, *Sec, 0, STV_DEFAULT);
+      Symtab.addSynthetic(Start, *Sec, 0, STV_DEFAULT, 0 /* Type */);
   if (SymbolBody *B = Symtab.find(Stop))
     if (B->isUndefined())
-      Symtab.addSynthetic(Stop, *Sec, Sec->getSize(), STV_DEFAULT);
+      Symtab.addSynthetic(Stop, *Sec, Sec->getSize(), STV_DEFAULT,
+                          0 /* Type */);
 }
 
 template <class ELFT> static bool needsPtLoad(OutputSectionBase<ELFT> *Sec) {
   if (!(Sec->getFlags() & SHF_ALLOC))
     return false;
 
   // Don't allocate VA space for TLS NOBITS sections. The PT_TLS PHDR is
   // responsible for allocating space for them, not the PT_LOAD that

test/ELF/shared.s

 // CHECK-NEXT:     Other: 0
 // CHECK-NEXT:     Section: Undefined
 // CHECK-NEXT:   }
 // CHECK-NEXT:   Symbol {
 // CHECK-NEXT:     Name: _DYNAMIC
 // CHECK-NEXT:     Value: 0x12000
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Local
-// CHECK-NEXT:     Type: None
+// CHECK-NEXT:     Type: Object
 // CHECK-NEXT:     Other: 2
 // CHECK-NEXT:     Section: .dynamic
 // CHECK-NEXT:   }
 // CHECK-NEXT:   Symbol {
 // CHECK-NEXT:     Name: _start
 // CHECK-NEXT:     Value: 0x11000
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Global

test/ELF/visibility.s

 // CHECK-NEXT:     Other: 2
 // CHECK-NEXT:     Section: .text
 // CHECK-NEXT:   }
 // CHECK-NEXT:   Symbol {
 // CHECK-NEXT:     Name: _DYNAMIC
 // CHECK-NEXT:     Value: 0x2000
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Local
-// CHECK-NEXT:     Type: None
+// CHECK-NEXT:     Type: Object
 // CHECK-NEXT:     Other: 2
 // CHECK-NEXT:     Section: .dynamic
 // CHECK-NEXT:   }
 // CHECK-NEXT:   Symbol {
 // CHECK-NEXT:     Name: default
 // CHECK-NEXT:     Value:
 // CHECK-NEXT:     Size: 0
 // CHECK-NEXT:     Binding: Global