[PATCH] D26043: Replace GAlloc with a template function.

Rafael Espíndola via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 1 13:42:50 PDT 2016


Fixed the problem and a rebased patch is attached.

Cheers,
Rafael


On 1 November 2016 at 16:01, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
> I see. I will fix that.
>
> Cheers,
> Rafael
>
>
> On 1 November 2016 at 13:12, Rui Ueyama <ruiu at google.com> wrote:
>> One test is failing with this patch because of the reason I described in the
>> review thread for https://reviews.llvm.org/D25516.
>>
>> On Tue, Nov 1, 2016 at 5:09 AM, Rafael Espíndola
>> <rafael.espindola at gmail.com> wrote:
>>>
>>> Ping on the commit :-)
>>>
>>> Cheers,
>>> Rafael
>>>
>>>
>>> On 28 October 2016 at 18:22, Rui Ueyama <ruiu at google.com> wrote:
>>> > ruiu added inline comments.
>>> >
>>> >
>>> > ================
>>> > Comment at: ELF/Memory.cpp:23
>>> > +
>>> > +void elf::freeArena() {
>>> > +  for (SpecificAllocBase *Alloc : SpecificAllocBase::Instances)
>>> > ----------------
>>> > rafael wrote:
>>> >> Do we still need to free this at a specific point in time? If not just
>>> >> letting the destructors take care of it should simplify the code.
>>> > This is for the use case in which you directly call lld::elf::link from
>>> > your program. In that use case, we want to free all memories before
>>> > returning from link().
>>> >
>>> >
>>> > https://reviews.llvm.org/D26043
>>> >
>>> >
>>> >
>>
>>
-------------- next part --------------
diff --git a/ELF/Driver.cpp b/ELF/Driver.cpp
index 489ab94..1171087 100644
--- a/ELF/Driver.cpp
+++ b/ELF/Driver.cpp
@@ -54,7 +54,6 @@ bool elf::link(ArrayRef<const char *> Args, bool CanExitEarly,
   ScriptConfig = &SC;
 
   Driver->main(Args, CanExitEarly);
-  InputFile::freePool();
   freeArena();
   return !HasError;
 }
@@ -132,7 +131,7 @@ void LinkerDriver::addFile(StringRef Path) {
   MemoryBufferRef MBRef = *Buffer;
 
   if (InBinary) {
-    Files.push_back(new BinaryFile(MBRef));
+    Files.push_back(new (alloc<BinaryFile>()) BinaryFile(MBRef));
     return;
   }
 
@@ -146,7 +145,7 @@ void LinkerDriver::addFile(StringRef Path) {
         Files.push_back(createObjectFile(MB, Path));
       return;
     }
-    Files.push_back(new ArchiveFile(MBRef));
+    Files.push_back(new (alloc<ArchiveFile>()) ArchiveFile(MBRef));
     return;
   case file_magic::elf_shared_object:
     if (Config->Relocatable) {
@@ -157,7 +156,7 @@ void LinkerDriver::addFile(StringRef Path) {
     return;
   default:
     if (InLib)
-      Files.push_back(new LazyObjectFile(MBRef));
+      Files.push_back(new (alloc<LazyObjectFile>()) LazyObjectFile(MBRef));
     else
       Files.push_back(createObjectFile(MBRef));
   }
diff --git a/ELF/InputFiles.cpp b/ELF/InputFiles.cpp
index dadf3bb..041b41f 100644
--- a/ELF/InputFiles.cpp
+++ b/ELF/InputFiles.cpp
@@ -35,7 +35,6 @@ using namespace llvm::sys::fs;
 using namespace lld;
 using namespace lld::elf;
 
-std::vector<InputFile *> InputFile::Pool;
 
 namespace {
 // In ELF object file all section addresses are zero. If we have multiple
@@ -97,15 +96,6 @@ std::string DIHelper<ELFT>::getLineInfo(InputSectionBase<ELFT> *S,
              : "";
 }
 
-// Deletes all InputFile instances created so far.
-void InputFile::freePool() {
-  // Files are freed in reverse order so that files created
-  // from other files (e.g. object files extracted from archives)
-  // are freed in the proper order.
-  for (int I = Pool.size() - 1; I >= 0; --I)
-    delete Pool[I];
-}
-
 // Returns "(internal)", "foo.a(bar.o)" or "baz.o".
 std::string elf::getFilename(const InputFile *F) {
   if (!F)
@@ -415,7 +405,7 @@ elf::ObjectFile<ELFT>::createInputSection(const Elf_Shdr &Sec) {
     // If -r is given, we do not interpret or apply relocation
     // but just copy relocation sections to output.
     if (Config->Relocatable)
-      return new (GAlloc<ELFT>::IAlloc.Allocate())
+      return new (alloc<InputSection<ELFT>>())
           InputSection<ELFT>(this, &Sec, Name);
 
     // Find the relocation target section and associate this
@@ -458,14 +448,13 @@ elf::ObjectFile<ELFT>::createInputSection(const Elf_Shdr &Sec) {
   // .eh_frame_hdr section for runtime. So we handle them with a special
   // class. For relocatable outputs, they are just passed through.
   if (Name == ".eh_frame" && !Config->Relocatable)
-    return new (GAlloc<ELFT>::EHAlloc.Allocate())
+    return new (alloc<EhInputSection<ELFT>>())
         EhInputSection<ELFT>(this, &Sec, Name);
 
   if (shouldMerge(Sec))
-    return new (GAlloc<ELFT>::MAlloc.Allocate())
+    return new (alloc<MergeInputSection<ELFT>>())
         MergeInputSection<ELFT>(this, &Sec, Name);
-  return new (GAlloc<ELFT>::IAlloc.Allocate())
-      InputSection<ELFT>(this, &Sec, Name);
+  return new (alloc<InputSection<ELFT>>()) InputSection<ELFT>(this, &Sec, Name);
 }
 
 template <class ELFT> void elf::ObjectFile<ELFT>::initializeSymbols() {
@@ -834,13 +823,13 @@ static InputFile *createELFFile(MemoryBufferRef MB) {
 
   InputFile *Obj;
   if (Size == ELFCLASS32 && Endian == ELFDATA2LSB)
-    Obj = new T<ELF32LE>(MB);
+    Obj = new (alloc<T<ELF32LE>>()) T<ELF32LE>(MB);
   else if (Size == ELFCLASS32 && Endian == ELFDATA2MSB)
-    Obj = new T<ELF32BE>(MB);
+    Obj = new (alloc<T<ELF32BE>>()) T<ELF32BE>(MB);
   else if (Size == ELFCLASS64 && Endian == ELFDATA2LSB)
-    Obj = new T<ELF64LE>(MB);
+    Obj = new (alloc<T<ELF64LE>>()) T<ELF64LE>(MB);
   else if (Size == ELFCLASS64 && Endian == ELFDATA2MSB)
-    Obj = new T<ELF64BE>(MB);
+    Obj = new (alloc<T<ELF64BE>>()) T<ELF64BE>(MB);
   else
     fatal("invalid file class: " + MB.getBufferIdentifier());
 
diff --git a/ELF/InputFiles.h b/ELF/InputFiles.h
index 0963634..f5da90d 100644
--- a/ELF/InputFiles.h
+++ b/ELF/InputFiles.h
@@ -37,21 +37,6 @@ class InputFile;
 namespace lld {
 namespace elf {
 
-template <class ELFT> struct GAlloc {
-  static llvm::SpecificBumpPtrAllocator<InputSection<ELFT>> IAlloc;
-  static llvm::SpecificBumpPtrAllocator<MergeInputSection<ELFT>> MAlloc;
-  static llvm::SpecificBumpPtrAllocator<EhInputSection<ELFT>> EHAlloc;
-};
-
-template <class ELFT>
-llvm::SpecificBumpPtrAllocator<InputSection<ELFT>> GAlloc<ELFT>::IAlloc;
-
-template <class ELFT>
-llvm::SpecificBumpPtrAllocator<MergeInputSection<ELFT>> GAlloc<ELFT>::MAlloc;
-
-template <class ELFT>
-llvm::SpecificBumpPtrAllocator<EhInputSection<ELFT>> GAlloc<ELFT>::EHAlloc;
-
 using llvm::object::Archive;
 
 class InputFile;
@@ -78,8 +63,6 @@ private:
 // The root class of input files.
 class InputFile {
 public:
-  virtual ~InputFile() = default;
-
   enum Kind {
     ObjectKind,
     SharedKind,
@@ -111,19 +94,11 @@ public:
   uint16_t EMachine = llvm::ELF::EM_NONE;
   uint8_t OSABI = 0;
 
-  static void freePool();
-
 protected:
-  InputFile(Kind K, MemoryBufferRef M) : MB(M), FileKind(K) {
-    Pool.push_back(this);
-  }
+  InputFile(Kind K, MemoryBufferRef M) : MB(M), FileKind(K) {}
 
 private:
   const Kind FileKind;
-
-  // All InputFile instances are added to the pool
-  // and freed all at once on exit by freePool().
-  static std::vector<InputFile *> Pool;
 };
 
 // Returns "(internal)", "foo.a(bar.o)" or "baz.o".
diff --git a/ELF/Memory.cpp b/ELF/Memory.cpp
index cc21aea..7771317 100644
--- a/ELF/Memory.cpp
+++ b/ELF/Memory.cpp
@@ -10,10 +10,20 @@
 #include "Memory.h"
 
 using namespace llvm;
+using namespace lld;
+using namespace lld::elf;
 
 namespace lld {
 BumpPtrAllocator elf::BAlloc;
 StringSaver elf::Saver{elf::BAlloc};
 
-void elf::freeArena() { elf::BAlloc.Reset(); }
+SpecificAllocBase::SpecificAllocBase() { Instances.push_back(this); }
+
+std::vector<SpecificAllocBase *> SpecificAllocBase::Instances;
+
+void elf::freeArena() {
+  for (SpecificAllocBase *Alloc : SpecificAllocBase::Instances)
+    Alloc->reset();
+  BAlloc.Reset();
+}
 }
diff --git a/ELF/Memory.h b/ELF/Memory.h
index 6ac312b..b7cf18b 100644
--- a/ELF/Memory.h
+++ b/ELF/Memory.h
@@ -24,12 +24,36 @@
 
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/StringSaver.h"
+#include <vector>
 
 namespace lld {
 namespace elf {
+
+// Use this arena if your object doesn't have a destructor.
 extern llvm::BumpPtrAllocator BAlloc;
 extern llvm::StringSaver Saver;
 
+// These two classes are hack to keep track of all
+// SpecificBumpPtrAllocator instances.
+struct SpecificAllocBase {
+  SpecificAllocBase();
+  virtual ~SpecificAllocBase() = default;
+  virtual void reset() = 0;
+  static std::vector<SpecificAllocBase *> Instances;
+};
+
+template <class T> struct SpecificAlloc : public SpecificAllocBase {
+  void reset() override { Alloc.DestroyAll(); }
+  llvm::SpecificBumpPtrAllocator<T> Alloc;
+};
+
+// Use this arean if your object have a destructor.
+// Your destructor will be invoked from freeArena().
+template <class T> static T *alloc() {
+  static SpecificAlloc<T> Alloc;
+  return Alloc.Alloc.Allocate();
+}
+
 void freeArena();
 }
 }


More information about the llvm-commits mailing list