[llvm] r303576 - Make TypeSerializer's StringMap use the same allocator.
Richard Trieu via llvm-commits
llvm-commits at lists.llvm.org
Mon May 22 20:53:16 PDT 2017
Zach,
I think this patch is causing a memory leak. Removing the function
TypeSerializer::insertRecordBytesWithCopy and not making the copy will
cause the memory backing the array to be deleted before it is used. This
has been detected by the sanitizer bot.
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/5121
Richard
On Mon, May 22, 2017 at 2:07 PM, Zachary Turner via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: zturner
> Date: Mon May 22 16:07:14 2017
> New Revision: 303576
>
> URL: http://llvm.org/viewvc/llvm-project?rev=303576&view=rev
> Log:
> Make TypeSerializer's StringMap use the same allocator.
>
> Modified:
> llvm/trunk/include/llvm/DebugInfo/CodeView/TypeSerializer.h
> llvm/trunk/include/llvm/DebugInfo/CodeView/TypeTableBuilder.h
> llvm/trunk/include/llvm/DebugInfo/CodeView/TypeTableCollection.h
> llvm/trunk/lib/DebugInfo/CodeView/TypeSerializer.cpp
> llvm/trunk/lib/DebugInfo/CodeView/TypeTableCollection.cpp
> llvm/trunk/tools/llvm-pdbdump/llvm-pdbdump.cpp
>
> Modified: llvm/trunk/include/llvm/DebugInfo/CodeView/TypeSerializer.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/
> llvm/DebugInfo/CodeView/TypeSerializer.h?rev=303576&
> r1=303575&r2=303576&view=diff
> ============================================================
> ==================
> --- llvm/trunk/include/llvm/DebugInfo/CodeView/TypeSerializer.h (original)
> +++ llvm/trunk/include/llvm/DebugInfo/CodeView/TypeSerializer.h Mon May
> 22 16:07:14 2017
> @@ -45,12 +45,13 @@ class TypeSerializer : public TypeVisito
> }
> };
>
> - typedef SmallVector<MutableArrayRef<uint8_t>, 2> RecordList;
> + typedef SmallVector<MutableArrayRef<uint8_t>, 2> MutableRecordList;
> + typedef SmallVector<ArrayRef<uint8_t>, 2> RecordList;
>
> static constexpr uint8_t ContinuationLength = 8;
> BumpPtrAllocator &RecordStorage;
> RecordSegment CurrentSegment;
> - RecordList FieldListSegments;
> + MutableRecordList FieldListSegments;
>
> TypeIndex LastTypeIndex;
> Optional<TypeLeafKind> TypeKind;
> @@ -61,7 +62,7 @@ class TypeSerializer : public TypeVisito
> TypeRecordMapping Mapping;
>
> RecordList SeenRecords;
> - StringMap<TypeIndex> HashedRecords;
> + StringMap<TypeIndex, BumpPtrAllocator&> HashedRecords;
>
> bool isInFieldList() const;
> TypeIndex calcNextTypeIndex() const;
> @@ -69,9 +70,7 @@ class TypeSerializer : public TypeVisito
> MutableArrayRef<uint8_t> getCurrentSubRecordData();
> MutableArrayRef<uint8_t> getCurrentRecordData();
> Error writeRecordPrefix(TypeLeafKind Kind);
> - TypeIndex insertRecordBytesPrivate(MutableArrayRef<uint8_t> Record);
> - TypeIndex insertRecordBytesWithCopy(CVType &Record,
> - MutableArrayRef<uint8_t> Data);
> + TypeIndex insertRecordBytesPrivate(ArrayRef<uint8_t> &Record);
>
> Expected<MutableArrayRef<uint8_t>>
> addPadding(MutableArrayRef<uint8_t> Record);
> @@ -79,9 +78,9 @@ class TypeSerializer : public TypeVisito
> public:
> explicit TypeSerializer(BumpPtrAllocator &Storage);
>
> - ArrayRef<MutableArrayRef<uint8_t>> records() const;
> + ArrayRef<ArrayRef<uint8_t>> records() const;
> TypeIndex getLastTypeIndex() const;
> - TypeIndex insertRecordBytes(MutableArrayRef<uint8_t> Record);
> + TypeIndex insertRecordBytes(ArrayRef<uint8_t> Record);
> Expected<TypeIndex> visitTypeEndGetIndex(CVType &Record);
>
> Error visitTypeBegin(CVType &Record) override;
>
> Modified: llvm/trunk/include/llvm/DebugInfo/CodeView/TypeTableBuilder.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/
> llvm/DebugInfo/CodeView/TypeTableBuilder.h?rev=303576&
> r1=303575&r2=303576&view=diff
> ============================================================
> ==================
> --- llvm/trunk/include/llvm/DebugInfo/CodeView/TypeTableBuilder.h
> (original)
> +++ llvm/trunk/include/llvm/DebugInfo/CodeView/TypeTableBuilder.h Mon May
> 22 16:07:14 2017
> @@ -64,7 +64,7 @@ public:
> return *ExpectedIndex;
> }
>
> - TypeIndex writeSerializedRecord(MutableArrayRef<uint8_t> Record) {
> + TypeIndex writeSerializedRecord(ArrayRef<uint8_t> Record) {
> return Serializer.insertRecordBytes(Record);
> }
>
> @@ -77,7 +77,7 @@ public:
> }
> }
>
> - ArrayRef<MutableArrayRef<uint8_t>> records() const {
> + ArrayRef<ArrayRef<uint8_t>> records() const {
> return Serializer.records();
> }
> };
>
> Modified: llvm/trunk/include/llvm/DebugInfo/CodeView/TypeTableCollection.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/
> llvm/DebugInfo/CodeView/TypeTableCollection.h?rev=
> 303576&r1=303575&r2=303576&view=diff
> ============================================================
> ==================
> --- llvm/trunk/include/llvm/DebugInfo/CodeView/TypeTableCollection.h
> (original)
> +++ llvm/trunk/include/llvm/DebugInfo/CodeView/TypeTableCollection.h Mon
> May 22 16:07:14 2017
> @@ -18,7 +18,7 @@ namespace codeview {
>
> class TypeTableCollection : public TypeCollection {
> public:
> - explicit TypeTableCollection(ArrayRef<MutableArrayRef<uint8_t>>
> Records);
> + explicit TypeTableCollection(ArrayRef<ArrayRef<uint8_t>> Records);
>
> Optional<TypeIndex> getFirst() override;
> Optional<TypeIndex> getNext(TypeIndex Prev) override;
> @@ -33,7 +33,7 @@ private:
> bool hasCapacityFor(TypeIndex Index) const;
> void ensureTypeExists(TypeIndex Index);
>
> - ArrayRef<MutableArrayRef<uint8_t>> Records;
> + ArrayRef<ArrayRef<uint8_t>> Records;
> TypeDatabase Database;
> };
> }
>
> Modified: llvm/trunk/lib/DebugInfo/CodeView/TypeSerializer.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
> DebugInfo/CodeView/TypeSerializer.cpp?rev=303576&
> r1=303575&r2=303576&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/DebugInfo/CodeView/TypeSerializer.cpp (original)
> +++ llvm/trunk/lib/DebugInfo/CodeView/TypeSerializer.cpp Mon May 22
> 16:07:14 2017
> @@ -52,43 +52,24 @@ Error TypeSerializer::writeRecordPrefix(
> }
>
> TypeIndex
> -TypeSerializer::insertRecordBytesPrivate(MutableArrayRef<uint8_t>
> Record) {
> +TypeSerializer::insertRecordBytesPrivate(ArrayRef<uint8_t> &Record) {
> assert(Record.size() % 4 == 0 && "Record is not aligned to 4 bytes!");
>
> StringRef S(reinterpret_cast<const char *>(Record.data()),
> Record.size());
>
> TypeIndex NextTypeIndex = calcNextTypeIndex();
> auto Result = HashedRecords.try_emplace(S, NextTypeIndex);
> +
> + StringRef NewData = Result.first->getKey();
> + Record = ArrayRef<uint8_t>(NewData.bytes_begin(), NewData.bytes_end());
> +
> if (Result.second) {
> + // If this triggered an insert into the map, store the bytes.
> LastTypeIndex = NextTypeIndex;
> SeenRecords.push_back(Record);
> }
> - return Result.first->getValue();
> -}
>
> -TypeIndex
> -TypeSerializer::insertRecordBytesWithCopy(CVType &Record,
> - MutableArrayRef<uint8_t> Data) {
> - assert(Data.size() % 4 == 0 && "Record is not aligned to 4 bytes!");
> -
> - StringRef S(reinterpret_cast<const char *>(Data.data()), Data.size());
> -
> - // Do a two state lookup / insert so that we don't have to allocate
> unless
> - // we're going
> - // to do an insert. This is a big memory savings.
> - auto Iter = HashedRecords.find(S);
> - if (Iter != HashedRecords.end())
> - return Iter->second;
> -
> - LastTypeIndex = calcNextTypeIndex();
> - uint8_t *Copy = RecordStorage.Allocate<uint8_t>(Data.size());
> - ::memcpy(Copy, Data.data(), Data.size());
> - Data = MutableArrayRef<uint8_t>(Copy, Data.size());
> - S = StringRef(reinterpret_cast<const char *>(Data.data()),
> Data.size());
> - HashedRecords.insert(std::make_pair(S, LastTypeIndex));
> - SeenRecords.push_back(Data);
> - Record.RecordData = Data;
> - return LastTypeIndex;
> + return Result.first->getValue();
> }
>
> Expected<MutableArrayRef<uint8_t>>
> @@ -112,19 +93,19 @@ TypeSerializer::TypeSerializer(BumpPtrAl
> : RecordStorage(Storage), LastTypeIndex(),
> RecordBuffer(MaxRecordLength * 2),
> Stream(RecordBuffer, llvm::support::little), Writer(Stream),
> - Mapping(Writer) {
> + Mapping(Writer), HashedRecords(Storage) {
> // RecordBuffer needs to be able to hold enough data so that if we are 1
> // byte short of MaxRecordLen, and then we try to write MaxRecordLen
> bytes,
> // we won't overflow.
> }
>
> -ArrayRef<MutableArrayRef<uint8_t>> TypeSerializer::records() const {
> +ArrayRef<ArrayRef<uint8_t>> TypeSerializer::records() const {
> return SeenRecords;
> }
>
> TypeIndex TypeSerializer::getLastTypeIndex() const { return
> LastTypeIndex; }
>
> -TypeIndex TypeSerializer::insertRecordBytes(MutableArrayRef<uint8_t>
> Record) {
> +TypeIndex TypeSerializer::insertRecordBytes(ArrayRef<uint8_t> Record) {
> assert(!TypeKind.hasValue() && "Already in a type mapping!");
> assert(Writer.getOffset() == 0 && "Stream has data already!");
>
> @@ -163,8 +144,8 @@ Expected<TypeIndex> TypeSerializer::visi
> Prefix->RecordLen = ThisRecordData.size() - sizeof(uint16_t);
>
> Record.Type = *TypeKind;
> - TypeIndex InsertedTypeIndex =
> - insertRecordBytesWithCopy(Record, ThisRecordData);
> + Record.RecordData = ThisRecordData;
> + TypeIndex InsertedTypeIndex = insertRecordBytesPrivate(
> Record.RecordData);
>
> // Write out each additional segment in reverse order, and update each
> // record's continuation index to point to the previous one.
>
> Modified: llvm/trunk/lib/DebugInfo/CodeView/TypeTableCollection.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
> DebugInfo/CodeView/TypeTableCollection.cpp?rev=303576&r1=303575&r2=303576&
> view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/DebugInfo/CodeView/TypeTableCollection.cpp (original)
> +++ llvm/trunk/lib/DebugInfo/CodeView/TypeTableCollection.cpp Mon May 22
> 16:07:14 2017
> @@ -25,7 +25,7 @@ static void error(Error &&EC) {
> }
>
> TypeTableCollection::TypeTableCollection(
> - ArrayRef<MutableArrayRef<uint8_t>> Records)
> + ArrayRef<ArrayRef<uint8_t>> Records)
> : Records(Records), Database(Records.size()) {}
>
> Optional<TypeIndex> TypeTableCollection::getFirst() {
>
> Modified: llvm/trunk/tools/llvm-pdbdump/llvm-pdbdump.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-
> pdbdump/llvm-pdbdump.cpp?rev=303576&r1=303575&r2=303576&view=diff
> ============================================================
> ==================
> --- llvm/trunk/tools/llvm-pdbdump/llvm-pdbdump.cpp (original)
> +++ llvm/trunk/tools/llvm-pdbdump/llvm-pdbdump.cpp Mon May 22 16:07:14
> 2017
> @@ -878,11 +878,11 @@ static void mergePdbs() {
> auto &DestTpi = Builder.getTpiBuilder();
> auto &DestIpi = Builder.getIpiBuilder();
> MergedTpi.ForEachRecord(
> - [&DestTpi](TypeIndex TI, MutableArrayRef<uint8_t> Data) {
> + [&DestTpi](TypeIndex TI, ArrayRef<uint8_t> Data) {
> DestTpi.addTypeRecord(Data, None);
> });
> MergedIpi.ForEachRecord(
> - [&DestIpi](TypeIndex TI, MutableArrayRef<uint8_t> Data) {
> + [&DestIpi](TypeIndex TI, ArrayRef<uint8_t> Data) {
> DestIpi.addTypeRecord(Data, None);
> });
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170522/0bee7ed4/attachment-0001.html>
More information about the llvm-commits
mailing list