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