From 97475834207bf5abb5b58534f783c9b71d4b9df1 Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Tue, 5 Oct 2010 18:37:06 +0000 Subject: [PATCH] Fix a marvelous chained AST writing bug, where we end up with the following amusing sequence: - AST writing schedules writing a type X* that it had never seen before - AST writing starts writing another declaration, ends up deserializing X* from a prior AST file. Now we have two type IDs for the same type! - AST writer tries to write X*. It only has the lower-numbered ID from the the prior AST file, so references to the higher-numbered ID that was scheduled for writing go off into lalaland. To fix this, keep the higher-numbered ID so we end up writing the type twice. Since this issue occurs so rarely, and type records are generally rather small, I deemed this better than the alternative: to keep a separate mapping from the higher-numbered IDs to the lower-numbered IDs, which we would end up having to check whenever we want to deserialize any type. Fixes , I think. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@115647 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Serialization/ASTReader.cpp | 3 +++ lib/Serialization/ASTWriter.cpp | 16 ++++++++++------ test/PCH/Inputs/chain-remap-types1.h | 10 ++++++++++ test/PCH/Inputs/chain-remap-types2.h | 8 ++++++++ test/PCH/chain-remap-types.m | 12 ++++++++++++ tools/libclang/CIndexUSRs.cpp | 3 ++- 6 files changed, 45 insertions(+), 7 deletions(-) create mode 100644 test/PCH/Inputs/chain-remap-types1.h create mode 100644 test/PCH/Inputs/chain-remap-types2.h create mode 100644 test/PCH/chain-remap-types.m diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp index 8413faa65d..33c032dcaa 100644 --- a/lib/Serialization/ASTReader.cpp +++ b/lib/Serialization/ASTReader.cpp @@ -3115,6 +3115,9 @@ QualType ASTReader::GetType(TypeID ID) { assert(Index < TypesLoaded.size() && "Type index out-of-range"); if (TypesLoaded[Index].isNull()) { TypesLoaded[Index] = ReadTypeRecord(Index); + if (TypesLoaded[Index].isNull()) + return QualType(); + TypesLoaded[Index]->setFromAST(); TypeIdxs[TypesLoaded[Index]] = TypeIdx::fromTypeID(ID); if (DeserializationListener) diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp index 7e0cdf41d7..f26731caea 100644 --- a/lib/Serialization/ASTWriter.cpp +++ b/lib/Serialization/ASTWriter.cpp @@ -1419,10 +1419,7 @@ void ASTWriter::WriteType(QualType T) { if (Idx.getIndex() == 0) // we haven't seen this type before. Idx = TypeIdx(NextTypeID++); - // If this type comes from a previously-loaded PCH/AST file, don't try to - // write the type again. - if (Idx.getIndex() < FirstTypeID) - return; + assert(Idx.getIndex() >= FirstTypeID && "Re-writing a type from a prior AST"); // Record the offset for this type. unsigned Index = Idx.getIndex() - FirstTypeID; @@ -2872,7 +2869,7 @@ DeclID ASTWriter::GetDeclRef(const Decl *D) { if (D == 0) { return 0; } - + assert(!(reinterpret_cast(D) & 0x01) && "Invalid decl pointer"); DeclID &ID = DeclIDs[D]; if (ID == 0) { // We haven't seen this declaration before. Give it a new ID and @@ -3130,7 +3127,14 @@ void ASTWriter::IdentifierRead(IdentID ID, IdentifierInfo *II) { } void ASTWriter::TypeRead(TypeIdx Idx, QualType T) { - TypeIdxs[T] = Idx; + // Always take the highest-numbered type index. This copes with an interesting + // case for chained AST writing where we schedule writing the type and then, + // later, deserialize the type from another AST. In this case, we want to + // keep the higher-numbered entry so that we can properly write it out to + // the AST file. + TypeIdx &StoredIdx = TypeIdxs[T]; + if (Idx.getIndex() >= StoredIdx.getIndex()) + StoredIdx = Idx; } void ASTWriter::DeclRead(DeclID ID, const Decl *D) { diff --git a/test/PCH/Inputs/chain-remap-types1.h b/test/PCH/Inputs/chain-remap-types1.h new file mode 100644 index 0000000000..d105489abd --- /dev/null +++ b/test/PCH/Inputs/chain-remap-types1.h @@ -0,0 +1,10 @@ +@class X; + +struct Y { + X *my_X; +}; + +@interface X { +} +@property X *prop; +@end diff --git a/test/PCH/Inputs/chain-remap-types2.h b/test/PCH/Inputs/chain-remap-types2.h new file mode 100644 index 0000000000..55ca8a9e31 --- /dev/null +++ b/test/PCH/Inputs/chain-remap-types2.h @@ -0,0 +1,8 @@ +void h(X*); + +@interface X (Blah) { +} +@end + +void g(X*); + diff --git a/test/PCH/chain-remap-types.m b/test/PCH/chain-remap-types.m new file mode 100644 index 0000000000..a45a79d75c --- /dev/null +++ b/test/PCH/chain-remap-types.m @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -emit-pch -x objective-c-header -o %t1 %S/Inputs/chain-remap-types1.h +// RUN: %clang_cc1 -emit-pch -x objective-c-header -o %t2 %S/Inputs/chain-remap-types2.h -include-pch %t1 -chained-pch +// RUN: %clang_cc1 -include-pch %t2 -fsyntax-only -verify %s +// RUN: %clang_cc1 -ast-print -include-pch %t2 %s | FileCheck %s + +// CHECK: @class X; +// CHECK: struct Y +// CHECK: @property ( assign,readwrite ) X * prop +// CHECK: void h(X *); +// CHECK: @interface X(Blah) +// CHECK: void g(X *); + diff --git a/tools/libclang/CIndexUSRs.cpp b/tools/libclang/CIndexUSRs.cpp index d7135b94be..e22980df39 100644 --- a/tools/libclang/CIndexUSRs.cpp +++ b/tools/libclang/CIndexUSRs.cpp @@ -699,7 +699,8 @@ void USRGenerator::VisitTemplateArgument(const TemplateArgument &Arg) { break; case TemplateArgument::Declaration: - Visit(Arg.getAsDecl()); + if (Decl *D = Arg.getAsDecl()) + Visit(D); break; case TemplateArgument::Template: