[llvm] r303576 - Make TypeSerializer's StringMap use the same allocator.
Zachary Turner via llvm-commits
llvm-commits at lists.llvm.org
Mon May 22 20:57:18 PDT 2017
Hmm, what's supposed to happen is that it creates the StringMap using an
allocator that should outlive any of this code. So we can then re-use the
key value that the StringMap allocated since the storage should stay
around. It's storing the allocator by reference too so the StringMap
should be getting the same one.
In any case, apparently something isn't working. I'm not in a position to
revert right now so unless you can do it for me, I can get to it tomorrow
:-/ Sorry
On Mon, May 22, 2017 at 8:53 PM Richard Trieu <rtrieu at google.com> wrote:
> 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/20170523/93726a53/attachment.html>
More information about the llvm-commits
mailing list