Fix a memory leak found by check-lld asan tests.

[krasin]

Summary

This bug was introduced by http://reviews.llvm.org/rL265059,
where InputSectionBase got Thunks field, which can do memory allocations.
Since InputSectionBase destructors were never called (I count it as another bug),
that caused a memory leak when 2 or more thunks are added to a section.

The fix to is properly call InputSectionBase destructors from ~ObjectFile.

[• rafael]

+ EHInputSection<ELFT> and MergeInputSection<ELFT> are allocated
+ using SpecificBumpPtrAllocator, which takes ownership.

Can't you do the same for InputSection? This would be the first
virtual function for the class.

Cheers,
Rafael

[ruiu]

ELF/InputFiles.h

114–125

This seems to be a bit too fragile. Generally, I prefer unique_ptr or SpecificBumpPtrAllocator or something like that over explicit destructors. Can you update InputFiles.cpp so that InputSection<ELFT> are allocated in the same way as EHInputSection<ELFT> or MergeInputSection<ELFT>? (i.e. using SpecificBumpPtrAllocator)

[krasin]

Rafael, Rui,

thanks for the unison comments. Will do.

[krasin]

Add SpecificBumpPtrAllocator for InputSection<ELFT>.

[krasin]

Done. Please, take a look.

[• rafael]

I think this is fine.

BTW, was this failing with an existing test with asan? If not, LGTM conditional on a test :-)

[krasin]

In D18809#392907, @rafael wrote:

I think this is fine.

BTW, was this failing with an existing test with asan? If not, LGTM conditional on a test :-)

Oh, yes: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/11679/steps/check-lld%20asan/logs/stdio

I came to the fix, because of my sheriff duty to keep sanitizers bots green. :)

[ruiu]

LGTM

[krasin]

For the record, this CL fixed the build: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/11680

ELF/InputFiles.h

static bool classof(const InputFile *F) {
     return F->kind() == Base::ObjectKind;
   }
 
   ArrayRef<SymbolBody *> getSymbols();
   ArrayRef<SymbolBody *> getLocalSymbols();
   ArrayRef<SymbolBody *> getNonLocalSymbols();
 
   explicit ObjectFile(MemoryBufferRef M);
   void parse(llvm::DenseSet<StringRef> &ComdatGroups);
 
   ArrayRef<InputSectionBase<ELFT> *> getSections() const { return Sections; }
   InputSectionBase<ELFT> *getSection(const Elf_Sym &Sym) const;
 
   SymbolBody &getSymbolBody(uint32_t SymbolIndex) const {
     return *SymbolBodies[SymbolIndex];
   }
 
   const Elf_Shdr *getSymbolTable() const { return this->Symtab; };
 
   // Get MIPS GP0 value defined by this file. This value represents the gp value
   // used to create the relocatable object and required to support
   // R_MIPS_GPREL16 / R_MIPS_GPREL32 relocations.
   uint32_t getMipsGp0() const;
 
   // The number is the offset in the string table. It will be used as the
   // st_name of the symbol.
   std::vector<std::pair<const DefinedRegular<ELFT> *, unsigned>> KeptLocalSyms;
 
private:
 
   // List of all symbols referenced or defined by this file.
   std::vector<SymbolBody *> SymbolBodies;
 
   // MIPS .reginfo section defined by this file.
   MipsReginfoInputSection<ELFT> *MipsReginfo = nullptr;
 
   llvm::BumpPtrAllocator Alloc;
+  llvm::SpecificBumpPtrAllocator<InputSection<ELFT>> IAlloc;
   llvm::SpecificBumpPtrAllocator<MergeInputSection<ELFT>> MAlloc;
   llvm::SpecificBumpPtrAllocator<EHInputSection<ELFT>> EHAlloc;
 };
 
 class ArchiveFile : public InputFile {
 public:
   explicit ArchiveFile(MemoryBufferRef M) : InputFile(ArchiveKind, M) {}
   static bool classof(const InputFile *F) { return F->kind() == ArchiveKind; }

ELF/InputFiles.cpp

for (const Elf_Shdr &Sec : Obj.sections()) {
     case SHT_NULL:
       break;
     case SHT_RELA:
     case SHT_REL: {
       // This section contains relocation information.
       // If -r is given, we do not interpret or apply relocation
       // but just copy relocation sections to output.
       if (Config->Relocatable) {
-        Sections[I] = new (Alloc) InputSection<ELFT>(this, &Sec);
+        Sections[I] = new (IAlloc.Allocate()) InputSection<ELFT>(this, &Sec);
         break;
       }
 
       // Find the relocation target section and associate this
       // section with it.
       InputSectionBase<ELFT> *Target = getRelocTarget(Sec);
       if (!Target)
         break;
elf::ObjectFile<ELFT>::createInputSection(const Elf_Shdr &Sec) {
   }
 
   // We dont need special handling of .eh_frame sections if relocatable
   // output was choosen. Proccess them as usual input sections.
   if (!Config->Relocatable && Name == ".eh_frame")
     return new (EHAlloc.Allocate()) EHInputSection<ELFT>(this, &Sec);
   if (shouldMerge<ELFT>(Sec))
     return new (MAlloc.Allocate()) MergeInputSection<ELFT>(this, &Sec);
-  return new (Alloc) InputSection<ELFT>(this, &Sec);
+  return new (IAlloc.Allocate()) InputSection<ELFT>(this, &Sec);
 }
 
 template <class ELFT> void elf::ObjectFile<ELFT>::initializeSymbols() {
   this->initStringTable();
   Elf_Sym_Range Syms = this->getElfSymbols(false);
   uint32_t NumSymbols = std::distance(Syms.begin(), Syms.end());
   SymbolBodies.reserve(NumSymbols);
   for (const Elf_Sym &Sym : Syms)