[llvm] r299403 - [codeview] Cope with unsorted streams in type merging
Reid Kleckner via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 3 16:58:15 PDT 2017
Author: rnk
Date: Mon Apr 3 18:58:15 2017
New Revision: 299403
URL: http://llvm.org/viewvc/llvm-project?rev=299403&view=rev
Log:
[codeview] Cope with unsorted streams in type merging
Summary:
MASM can produce type streams that are not topologically sorted. It can
even produce type streams with circular references, but those are not
common in practice.
Reviewers: inglorion, ruiu
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D31629
Added:
llvm/trunk/test/tools/llvm-readobj/Inputs/codeview-cycle.obj (with props)
llvm/trunk/test/tools/llvm-readobj/Inputs/codeview-unsorted.obj (with props)
llvm/trunk/test/tools/llvm-readobj/codeview-merging-cycle.test
llvm/trunk/test/tools/llvm-readobj/codeview-merging-unsorted.test
Modified:
llvm/trunk/include/llvm/DebugInfo/CodeView/TypeTableBuilder.h
llvm/trunk/lib/DebugInfo/CodeView/TypeStreamMerger.cpp
llvm/trunk/tools/llvm-readobj/COFFDumper.cpp
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=299403&r1=299402&r2=299403&view=diff
==============================================================================
--- llvm/trunk/include/llvm/DebugInfo/CodeView/TypeTableBuilder.h (original)
+++ llvm/trunk/include/llvm/DebugInfo/CodeView/TypeTableBuilder.h Mon Apr 3 18:58:15 2017
@@ -121,6 +121,12 @@ public:
}
return Index;
}
+
+ /// Stop building the record.
+ void reset() {
+ if (auto EC = TempSerializer.visitTypeEnd(Type))
+ consumeError(std::move(EC));
+ }
};
} // end namespace codeview
Modified: llvm/trunk/lib/DebugInfo/CodeView/TypeStreamMerger.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/CodeView/TypeStreamMerger.cpp?rev=299403&r1=299402&r2=299403&view=diff
==============================================================================
--- llvm/trunk/lib/DebugInfo/CodeView/TypeStreamMerger.cpp (original)
+++ llvm/trunk/lib/DebugInfo/CodeView/TypeStreamMerger.cpp Mon Apr 3 18:58:15 2017
@@ -64,6 +64,8 @@ public:
: DestIdStream(DestIdStream), DestTypeStream(DestTypeStream),
FieldListBuilder(DestTypeStream), Handler(Handler) {}
+ static const TypeIndex Untranslated;
+
/// TypeVisitorCallbacks overrides.
#define TYPE_RECORD(EnumName, EnumVal, Name) \
Error visitKnownRecord(CVType &CVR, Name##Record &Record) override;
@@ -82,43 +84,54 @@ public:
Error mergeStream(const CVTypeArray &Types);
private:
+ void addMapping(TypeIndex Idx);
+
bool remapIndex(TypeIndex &Idx);
+ size_t slotForIndex(TypeIndex Idx) const {
+ assert(!Idx.isSimple() && "simple type indices have no slots");
+ return Idx.getIndex() - TypeIndex::FirstNonSimpleIndex;
+ }
+
+ Error errorCorruptRecord() const {
+ return llvm::make_error<CodeViewError>(cv_error_code::corrupt_record);
+ }
+
template <typename RecordType>
Error writeRecord(RecordType &R, bool RemapSuccess) {
- if (!RemapSuccess) {
- LastError = joinErrors(
- std::move(*LastError),
- llvm::make_error<CodeViewError>(cv_error_code::corrupt_record));
- }
- IndexMap.push_back(DestTypeStream.writeKnownType(R));
+ TypeIndex DestIdx = Untranslated;
+ if (RemapSuccess)
+ DestIdx = DestTypeStream.writeKnownType(R);
+ addMapping(DestIdx);
return Error::success();
}
template <typename RecordType>
Error writeIdRecord(RecordType &R, bool RemapSuccess) {
- if (!RemapSuccess) {
- LastError = joinErrors(
- std::move(*LastError),
- llvm::make_error<CodeViewError>(cv_error_code::corrupt_record));
- }
- IndexMap.push_back(DestIdStream.writeKnownType(R));
+ TypeIndex DestIdx = Untranslated;
+ if (RemapSuccess)
+ DestIdx = DestIdStream.writeKnownType(R);
+ addMapping(DestIdx);
return Error::success();
}
template <typename RecordType>
Error writeMember(RecordType &R, bool RemapSuccess) {
- if (!RemapSuccess) {
- LastError = joinErrors(
- std::move(*LastError),
- llvm::make_error<CodeViewError>(cv_error_code::corrupt_record));
- }
- FieldListBuilder.writeMemberType(R);
+ if (RemapSuccess)
+ FieldListBuilder.writeMemberType(R);
+ else
+ HadUntranslatedMember = true;
return Error::success();
}
Optional<Error> LastError;
+ bool IsSecondPass = false;
+
+ bool HadUntranslatedMember = false;
+
+ unsigned NumBadIndices = 0;
+
BumpPtrAllocator Allocator;
TypeTableBuilder &DestIdStream;
@@ -126,11 +139,7 @@ private:
FieldListRecordBuilder FieldListBuilder;
TypeServerHandler *Handler;
-#ifndef NDEBUG
- /// Track the size of the index map in visitTypeBegin so we can check it in
- /// visitTypeEnd.
- size_t BeginIndexMapSize = 0;
-#endif
+ TypeIndex CurIndex{TypeIndex::FirstNonSimpleIndex};
/// Map from source type index to destination type index. Indexed by source
/// type index minus 0x1000.
@@ -139,16 +148,17 @@ private:
} // end anonymous namespace
+const TypeIndex TypeStreamMerger::Untranslated(SimpleTypeKind::NotTranslated);
+
Error TypeStreamMerger::visitTypeBegin(CVRecord<TypeLeafKind> &Rec) {
-#ifndef NDEBUG
- BeginIndexMapSize = IndexMap.size();
-#endif
return Error::success();
}
Error TypeStreamMerger::visitTypeEnd(CVRecord<TypeLeafKind> &Rec) {
- assert(IndexMap.size() == BeginIndexMapSize + 1 &&
- "visitKnownRecord should add one index map entry");
+ CurIndex = TypeIndex(CurIndex.getIndex() + 1);
+ if (!IsSecondPass)
+ assert(IndexMap.size() == slotForIndex(CurIndex) &&
+ "visitKnownRecord should add one index map entry");
return Error::success();
}
@@ -156,19 +166,44 @@ Error TypeStreamMerger::visitMemberEnd(C
return Error::success();
}
+void TypeStreamMerger::addMapping(TypeIndex Idx) {
+ if (!IsSecondPass) {
+ assert(IndexMap.size() == slotForIndex(CurIndex) &&
+ "visitKnownRecord should add one index map entry");
+ IndexMap.push_back(Idx);
+ } else {
+ assert(slotForIndex(CurIndex) < IndexMap.size());
+ IndexMap[slotForIndex(CurIndex)] = Idx;
+ }
+}
+
bool TypeStreamMerger::remapIndex(TypeIndex &Idx) {
// Simple types are unchanged.
if (Idx.isSimple())
return true;
- unsigned MapPos = Idx.getIndex() - TypeIndex::FirstNonSimpleIndex;
- if (MapPos < IndexMap.size()) {
+
+ // Check if this type index refers to a record we've already translated
+ // successfully. If it refers to a type later in the stream or a record we
+ // had to defer, defer it until later pass.
+ unsigned MapPos = slotForIndex(Idx);
+ if (MapPos < IndexMap.size() && IndexMap[MapPos] != Untranslated) {
Idx = IndexMap[MapPos];
return true;
}
+ // If this is the second pass and this index isn't in the map, then it points
+ // outside the current type stream, and this is a corrupt record.
+ if (IsSecondPass && MapPos >= IndexMap.size()) {
+ // FIXME: Print a more useful error. We can give the current record and the
+ // index that we think its pointing to.
+ LastError = joinErrors(std::move(*LastError), errorCorruptRecord());
+ }
+
+ ++NumBadIndices;
+
// This type index is invalid. Remap this to "not translated by cvpack",
// and return failure.
- Idx = TypeIndex(SimpleTypeKind::NotTranslated, SimpleTypeMode::Direct);
+ Idx = Untranslated;
return false;
}
@@ -248,11 +283,13 @@ Error TypeStreamMerger::visitKnownRecord
return writeRecord(R, Success);
}
-Error TypeStreamMerger::visitKnownRecord(CVType &, ArgListRecord &R) {
+Error TypeStreamMerger::visitKnownRecord(CVType &Type, ArgListRecord &R) {
bool Success = true;
for (TypeIndex &Arg : R.ArgIndices)
Success &= remapIndex(Arg);
- return writeRecord(R, Success);
+ if (auto EC = writeRecord(R, Success))
+ return EC;
+ return Error::success();
}
Error TypeStreamMerger::visitKnownRecord(CVType &, PointerRecord &R) {
@@ -322,12 +359,20 @@ Error TypeStreamMerger::visitKnownRecord
Error TypeStreamMerger::visitKnownRecord(CVType &, FieldListRecord &R) {
// Visit the members inside the field list.
+ HadUntranslatedMember = false;
FieldListBuilder.begin();
CVTypeVisitor Visitor(*this);
if (auto EC = Visitor.visitFieldListMemberStream(R.Data))
return EC;
- TypeIndex Index = FieldListBuilder.end();
- IndexMap.push_back(Index);
+
+ // Write the record if we translated all field list members.
+ TypeIndex DestIdx = Untranslated;
+ if (!HadUntranslatedMember)
+ DestIdx = FieldListBuilder.end();
+ else
+ FieldListBuilder.reset();
+ addMapping(DestIdx);
+
return Error::success();
}
@@ -389,9 +434,8 @@ Error TypeStreamMerger::visitKnownMember
Error TypeStreamMerger::visitUnknownType(CVType &Rec) {
// We failed to translate a type. Translate this index as "not translated".
- IndexMap.push_back(
- TypeIndex(SimpleTypeKind::NotTranslated, SimpleTypeMode::Direct));
- return llvm::make_error<CodeViewError>(cv_error_code::corrupt_record);
+ addMapping(TypeIndex(SimpleTypeKind::NotTranslated));
+ return errorCorruptRecord();
}
Error TypeStreamMerger::mergeStream(const CVTypeArray &Types) {
@@ -409,6 +453,30 @@ Error TypeStreamMerger::mergeStream(cons
if (auto EC = Visitor.visitTypeStream(Types))
return EC;
+
+ // If we found bad indices but no other errors, try doing another pass and see
+ // if we can resolve the indices that weren't in the map on the first pass.
+ // This may require multiple passes, but we should always make progress. MASM
+ // is the only known CodeView producer that makes type streams that aren't
+ // topologically sorted. The standard library contains MASM-produced objects,
+ // so this is important to handle correctly, but we don't have to be too
+ // efficient. MASM type streams are usually very small.
+ while (!*LastError && NumBadIndices > 0) {
+ unsigned BadIndicesRemaining = NumBadIndices;
+ IsSecondPass = true;
+ NumBadIndices = 0;
+ CurIndex = TypeIndex(TypeIndex::FirstNonSimpleIndex);
+ if (auto EC = Visitor.visitTypeStream(Types))
+ return EC;
+
+ assert(NumBadIndices <= BadIndicesRemaining &&
+ "second pass found more bad indices");
+ if (!*LastError && NumBadIndices == BadIndicesRemaining) {
+ return llvm::make_error<CodeViewError>(
+ cv_error_code::corrupt_record, "input type graph contains cycles");
+ }
+ }
+
IndexMap.clear();
Error Ret = std::move(*LastError);
Added: llvm/trunk/test/tools/llvm-readobj/Inputs/codeview-cycle.obj
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-readobj/Inputs/codeview-cycle.obj?rev=299403&view=auto
==============================================================================
Binary file - no diff available.
Propchange: llvm/trunk/test/tools/llvm-readobj/Inputs/codeview-cycle.obj
------------------------------------------------------------------------------
svn:mime-type = application/octet-stream
Added: llvm/trunk/test/tools/llvm-readobj/Inputs/codeview-unsorted.obj
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-readobj/Inputs/codeview-unsorted.obj?rev=299403&view=auto
==============================================================================
Binary file - no diff available.
Propchange: llvm/trunk/test/tools/llvm-readobj/Inputs/codeview-unsorted.obj
------------------------------------------------------------------------------
svn:mime-type = application/octet-stream
Added: llvm/trunk/test/tools/llvm-readobj/codeview-merging-cycle.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-readobj/codeview-merging-cycle.test?rev=299403&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-readobj/codeview-merging-cycle.test (added)
+++ llvm/trunk/test/tools/llvm-readobj/codeview-merging-cycle.test Mon Apr 3 18:58:15 2017
@@ -0,0 +1,19 @@
+; RUN: not llvm-readobj -codeview-merged-types %S/Inputs/codeview-cycle.obj 2>&1 | FileCheck %s
+
+; CHECK: Error{{.*}} input type graph contains cycles
+
+; To reproduce codeview-cycle.obj:
+; $ cat codeview-cycle.asm
+; .model flat, C
+; .code
+; pfoo_list TYPEDEF PTR foo_list
+; foo_list STRUCT
+; next pfoo_list ?
+; data dd ?
+; foo_list ENDS
+; public foo
+; foo proc dst:ptr foo_list
+; ret
+; foo endp
+; end
+; $ ml -c -Zi codeview-cycle.asm
Added: llvm/trunk/test/tools/llvm-readobj/codeview-merging-unsorted.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-readobj/codeview-merging-unsorted.test?rev=299403&view=auto
==============================================================================
--- llvm/trunk/test/tools/llvm-readobj/codeview-merging-unsorted.test (added)
+++ llvm/trunk/test/tools/llvm-readobj/codeview-merging-unsorted.test Mon Apr 3 18:58:15 2017
@@ -0,0 +1,40 @@
+; RUN: llvm-readobj -codeview %S/Inputs/codeview-unsorted.obj | FileCheck %s
+; RUN: llvm-readobj -codeview-merged-types %S/Inputs/codeview-unsorted.obj | FileCheck %s --check-prefix=MERGED
+
+; The input type stream has records that refer to later type indices in the same
+; stream:
+
+; CHECK: Pointer (0x1000)
+; CHECK: Struct (0x1001)
+; CHECK: FieldList: {{.*}} (0x1002)
+; CHECK: FieldList (0x1002)
+; CHECK: Pointer (0x1003)
+; CHECK: Procedure (0x1004)
+; CHECK: ArgListType: {{.*}} (0x1005)
+; CHECK: ArgList (0x1005)
+
+; MERGED: Pointer (0x1000)
+; MERGED: FieldList (0x1001)
+; MERGED: Struct (0x1002)
+; MERGED: FieldList: {{.*}} (0x1001)
+; MERGED: Pointer (0x1003)
+; MERGED: ArgList (0x1004)
+; MERGED: Procedure (0x1005)
+; MERGED: ArgListType: {{.*}} (0x1004)
+
+
+; To reproduce codeview-unsorted.obj:
+; $ cat codeview-unsorted.asm
+; .model flat, C
+; .code
+; PBYTE TYPEDEF PTR BYTE
+; foo_list STRUCT
+; next PBYTE ?
+; data dd ?
+; foo_list ENDS
+; public foo
+; foo proc dst:ptr foo_list
+; ret
+; foo endp
+; end
+; $ ml -c -Zi codeview-unsorted.asm
Modified: llvm/trunk/tools/llvm-readobj/COFFDumper.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-readobj/COFFDumper.cpp?rev=299403&r1=299402&r2=299403&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-readobj/COFFDumper.cpp (original)
+++ llvm/trunk/tools/llvm-readobj/COFFDumper.cpp Mon Apr 3 18:58:15 2017
@@ -1088,10 +1088,8 @@ void COFFDumper::mergeCodeViewTypes(Type
error(object_error::parse_failed);
}
- if (auto EC = mergeTypeStreams(CVIDs, CVTypes, nullptr, Types)) {
- consumeError(std::move(EC));
- return error(object_error::parse_failed);
- }
+ if (auto EC = mergeTypeStreams(CVIDs, CVTypes, nullptr, Types))
+ return error(std::move(EC));
}
}
}
@@ -1575,7 +1573,7 @@ void llvm::dumpCodeViewMergedTypes(Scope
if (auto EC = CVTD.dump(
{TypeBuf.str().bytes_begin(), TypeBuf.str().bytes_end()}, TDV)) {
Writer.flush();
- error(llvm::errorToErrorCode(std::move(EC)));
+ error(std::move(EC));
}
}
@@ -1595,7 +1593,7 @@ void llvm::dumpCodeViewMergedTypes(Scope
if (auto EC = CVTD.dump(
{IDBuf.str().bytes_begin(), IDBuf.str().bytes_end()}, TDV)) {
Writer.flush();
- error(llvm::errorToErrorCode(std::move(EC)));
+ error(std::move(EC));
}
}
}
More information about the llvm-commits
mailing list