From 596e474101ea9a8ecac9d8ee090d31469dbcc61d Mon Sep 17 00:00:00 2001 From: Jim Grosbach Date: Thu, 29 Nov 2012 19:14:11 +0000 Subject: [PATCH] Fix a memory leak in MachOObjectFile. MachOObjectFile owns a MachOObj, but never frees it. Both MachOObjectFile and MachOObj want to own the MemoryBuffer, though, so we have to be careful and give them each one of their own. Thanks to Greg Clayton, Eric Christopher and Michael Spencer for helping figure out what's going wrong here. rdar://12561773 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@168923 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Object/MachO.h | 4 ++-- lib/Object/MachOObjectFile.cpp | 23 +++++++++++++++-------- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/include/llvm/Object/MachO.h b/include/llvm/Object/MachO.h index 4e03daab16a..50c577dc262 100644 --- a/include/llvm/Object/MachO.h +++ b/include/llvm/Object/MachO.h @@ -44,7 +44,7 @@ public: virtual unsigned getArch() const; virtual StringRef getLoadName() const; - MachOObject *getObject() { return MachOObj; } + MachOObject *getObject() { return MachOObj.get(); } static inline bool classof(const Binary *v) { return v->isMachO(); @@ -104,7 +104,7 @@ protected: virtual error_code getLibraryPath(DataRefImpl LibData, StringRef &Res) const; private: - MachOObject *MachOObj; + OwningPtr MachOObj; mutable uint32_t RegisteredStringTable; typedef SmallVector SectionList; SectionList Sections; diff --git a/lib/Object/MachOObjectFile.cpp b/lib/Object/MachOObjectFile.cpp index 45aeaac6b83..bcea3bb5e91 100644 --- a/lib/Object/MachOObjectFile.cpp +++ b/lib/Object/MachOObjectFile.cpp @@ -50,7 +50,14 @@ ObjectFile *ObjectFile::createMachOObjectFile(MemoryBuffer *Buffer) { MachOObject *MachOObj = MachOObject::LoadFromBuffer(Buffer, &Err); if (!MachOObj) return NULL; - return new MachOObjectFile(Buffer, MachOObj, ec); + // MachOObject takes ownership of the Buffer we passed to it, and + // MachOObjectFile does, too, so we need to make sure they don't get the + // same object. A MemoryBuffer is cheap (it's just a reference to memory, + // not a copy of the memory itself), so just make a new copy here for + // the MachOObjectFile. + MemoryBuffer *NewBuffer = + MemoryBuffer::getMemBuffer(Buffer->getBuffer(), "", false); + return new MachOObjectFile(NewBuffer, MachOObj, ec); } /*===-- Symbols -----------------------------------------------------------===*/ @@ -474,7 +481,7 @@ error_code MachOObjectFile::getSectionName(DataRefImpl DRI, StringRef &Result) const { // FIXME: thread safety. static char result[34]; - if (is64BitLoadCommand(MachOObj, DRI)) { + if (is64BitLoadCommand(MachOObj.get(), DRI)) { InMemoryStruct SLC; LoadCommandInfo LCI = MachOObj->getLoadCommandInfo(DRI.d.a); MachOObj->ReadSegment64LoadCommand(LCI, SLC); @@ -501,7 +508,7 @@ error_code MachOObjectFile::getSectionName(DataRefImpl DRI, error_code MachOObjectFile::getSectionAddress(DataRefImpl DRI, uint64_t &Result) const { - if (is64BitLoadCommand(MachOObj, DRI)) { + if (is64BitLoadCommand(MachOObj.get(), DRI)) { InMemoryStruct Sect; getSection64(DRI, Sect); Result = Sect->Address; @@ -515,7 +522,7 @@ error_code MachOObjectFile::getSectionAddress(DataRefImpl DRI, error_code MachOObjectFile::getSectionSize(DataRefImpl DRI, uint64_t &Result) const { - if (is64BitLoadCommand(MachOObj, DRI)) { + if (is64BitLoadCommand(MachOObj.get(), DRI)) { InMemoryStruct Sect; getSection64(DRI, Sect); Result = Sect->Size; @@ -529,7 +536,7 @@ error_code MachOObjectFile::getSectionSize(DataRefImpl DRI, error_code MachOObjectFile::getSectionContents(DataRefImpl DRI, StringRef &Result) const { - if (is64BitLoadCommand(MachOObj, DRI)) { + if (is64BitLoadCommand(MachOObj.get(), DRI)) { InMemoryStruct Sect; getSection64(DRI, Sect); Result = MachOObj->getData(Sect->Offset, Sect->Size); @@ -543,7 +550,7 @@ error_code MachOObjectFile::getSectionContents(DataRefImpl DRI, error_code MachOObjectFile::getSectionAlignment(DataRefImpl DRI, uint64_t &Result) const { - if (is64BitLoadCommand(MachOObj, DRI)) { + if (is64BitLoadCommand(MachOObj.get(), DRI)) { InMemoryStruct Sect; getSection64(DRI, Sect); Result = uint64_t(1) << Sect->Align; @@ -557,7 +564,7 @@ error_code MachOObjectFile::getSectionAlignment(DataRefImpl DRI, error_code MachOObjectFile::isSectionText(DataRefImpl DRI, bool &Result) const { - if (is64BitLoadCommand(MachOObj, DRI)) { + if (is64BitLoadCommand(MachOObj.get(), DRI)) { InMemoryStruct Sect; getSection64(DRI, Sect); Result = !strcmp(Sect->Name, "__text"); @@ -664,7 +671,7 @@ relocation_iterator MachOObjectFile::getSectionRelBegin(DataRefImpl Sec) const { } relocation_iterator MachOObjectFile::getSectionRelEnd(DataRefImpl Sec) const { uint32_t last_reloc; - if (is64BitLoadCommand(MachOObj, Sec)) { + if (is64BitLoadCommand(MachOObj.get(), Sec)) { InMemoryStruct Sect; getSection64(Sec, Sect); last_reloc = Sect->NumRelocationTableEntries;