Use owning pointers instead of raw pointers for Atom's to fix leaks.

Currently each File contains an BumpPtrAllocator in which Atom's are
allocated.  Some Atom's contain data structures like std::vector which
leak as we don't run ~Atom when they are BumpPtrAllocate'd.

Now each File actually owns its Atom's using an OwningAtomPtr.  This
is analygous to std::unique_ptr and may be replaced by it if possible.

An Atom can therefore only be owned by a single File, so the Resolver now
moves them from one File to another.  The MachOLinkingContext owns the File's
and so clears all the Atom's in ~MachOLinkingContext, then delete's all the
File's.  This makes sure all Atom's have been destructed before any of the
BumpPtrAllocator's in which they run have gone away.

Should hopefully fix the remaining leaks.  Will keep an eye on the bots to
make sure.

llvm-svn: 264022

1 files changed, 16 insertions, 10 deletions

diff --git a/lld/lib/ReaderWriter/MachO/File.h b/lld/lib/ReaderWriter/MachO/File.h
index a0d20ea540f..f7262bbfaef 100644
--- a/lld/lib/ReaderWriter/MachO/File.h
+++ b/lld/lib/ReaderWriter/MachO/File.h
@@ -275,7 +275,8 @@ public:
 
   MachODylibFile(StringRef path) : SharedLibraryFile(path) {}
 
-  const SharedLibraryAtom *exports(StringRef name, bool isData) const override {
+  OwningAtomPtr<SharedLibraryAtom> exports(StringRef name,
+                                           bool isData) const override {
     // Pass down _installName so that if this requested symbol
     // is re-exported through this dylib, the SharedLibraryAtom's loadName()
     // is this dylib installName and not the implementation dylib's.
@@ -328,25 +329,30 @@ public:
   }
 
 private:
-  const SharedLibraryAtom *exports(StringRef name,
+  OwningAtomPtr<SharedLibraryAtom> exports(StringRef name,
                                    StringRef installName) const {
     // First, check if requested symbol is directly implemented by this dylib.
     auto entry = _nameToAtom.find(name);
     if (entry != _nameToAtom.end()) {
-      if (!entry->second.atom) {
-        // Lazily create SharedLibraryAtom.
-        entry->second.atom =
-          new (allocator()) MachOSharedLibraryAtom(*this, name, installName,
-                                                   entry->second.weakDef);
-      }
-      return entry->second.atom;
+      // FIXME: Make this map a set and only used in assert builds.
+      // Note, its safe to assert here as the resolver is the only client of
+      // this API and it only requests exports for undefined symbols.
+      // If we return from here we are no longer undefined so we should never
+      // get here again.
+      assert(!entry->second.atom && "Duplicate shared library export");
+      bool weakDef = entry->second.weakDef;
+      auto *atom = new (allocator()) MachOSharedLibraryAtom(*this, name,
+                                                            installName,
+                                                            weakDef);
+      entry->second.atom = atom;
+      return atom;
     }
 
     // Next, check if symbol is implemented in some re-exported dylib.
     for (const ReExportedDylib &dylib : _reExportedDylibs) {
       assert(dylib.file);
       auto atom = dylib.file->exports(name, installName);
-      if (atom)
+      if (atom.get())
         return atom;
     }