[PCH] Make sure that all newly introduced visible decls in a DeclContext

coming from an AST file are registered for serialization.

A static data member instantiation of in a chained PCH could be missed
when serializing decls; the result was that when emitting the visible decls
map of its DeclContext, we would use a DeclID that was not actually emitted,
leading to crashes or hangs.

Fix this by making sure such decls are always registered for serialization.
Also introduce extra sanity checks to make sure we don't register new
declarations or types after we have serialized the types/decls block.

rdar://11728990

llvm-svn: 159550

clang/include/clang/Serialization/ASTWriter.h

@@ -110,6 +110,10 @@ private:
   /// serialization, rather than just queueing updates.
   bool WritingAST;
 
+  /// \brief Indicates that we are done serializing the collection of decls
+  /// and types to emit.
+  bool DoneWritingDeclsAndTypes;
+
   /// \brief Indicates that the AST contained compiler errors.
   bool ASTHasCompilerErrors;
 
@@ -296,6 +300,10 @@ private:
   /// it.
   llvm::SmallPtrSet<const DeclContext *, 16> UpdatedDeclContexts;
 
+  /// \brief Keeps track of visible decls that were added in DeclContexts
+  /// coming from another AST file.
+  SmallVector<const Decl *, 16> UpdatingVisibleDecls;
+
   typedef llvm::SmallPtrSet<const Decl *, 16> DeclsToRewriteTy;
   /// \brief Decls that will be replaced in the current dependent AST file.
   DeclsToRewriteTy DeclsToRewrite;

clang/lib/Serialization/ASTReader.cpp

@@ -4673,7 +4673,9 @@ Decl *ASTReader::GetDecl(DeclID ID) {
   unsigned Index = ID - NUM_PREDEF_DECL_IDS;
 
   if (Index >= DeclsLoaded.size()) {
+    assert(0 && "declaration ID out-of-range for AST file");
     Error("declaration ID out-of-range for AST file");
+    return 0;
   }
   
   if (!DeclsLoaded[Index]) {

clang/lib/Serialization/ASTWriter.cpp

@@ -3140,7 +3140,8 @@ void ASTWriter::SetSelectorOffset(Selector Sel, uint32_t Offset) {
 
 ASTWriter::ASTWriter(llvm::BitstreamWriter &Stream)
   : Stream(Stream), Context(0), PP(0), Chain(0), WritingModule(0),
-    WritingAST(false), ASTHasCompilerErrors(false),
+    WritingAST(false), DoneWritingDeclsAndTypes(false),
+    ASTHasCompilerErrors(false),
     FirstDeclID(NUM_PREDEF_DECL_IDS), NextDeclID(FirstDeclID),
     FirstTypeID(NUM_PREDEF_TYPE_IDS), NextTypeID(FirstTypeID),
     FirstIdentID(NUM_PREDEF_IDENT_IDS), NextIdentID(FirstIdentID), 
@@ -3400,7 +3401,15 @@ void ASTWriter::WriteASTCore(Sema &SemaRef, MemorizeStatCalls *StatCalls,
       Record.push_back(reinterpret_cast<uint64_t>(NS));
     }
   }
-  
+
+  // Make sure visible decls, added to DeclContexts previously loaded from
+  // an AST file, are registered for serialization.
+  for (SmallVector<const Decl *, 16>::iterator
+         I = UpdatingVisibleDecls.begin(),
+         E = UpdatingVisibleDecls.end(); I != E; ++I) {
+    GetDeclRef(*I);
+  }
+
   // Resolve any declaration pointers within the declaration updates block.
   ResolveDeclUpdatesBlocks();
   
@@ -3433,6 +3442,8 @@ void ASTWriter::WriteASTCore(Sema &SemaRef, MemorizeStatCalls *StatCalls,
   }
   Stream.ExitBlock();
 
+  DoneWritingDeclsAndTypes = true;
+
   WriteFileDeclIDsMap();
   WriteSourceManagerBlock(Context.getSourceManager(), PP, isysroot);
   WriteComments();
@@ -3819,6 +3830,11 @@ TypeIdx ASTWriter::GetOrCreateTypeIdx(QualType T) {
 
   TypeIdx &Idx = TypeIdxs[T];
   if (Idx.getIndex() == 0) {
+    if (DoneWritingDeclsAndTypes) {
+      assert(0 && "New type seen after serializing all the types to emit!");
+      return TypeIdx();
+    }
+
     // We haven't seen this type before. Assign it a new ID and put it
     // into the queue of types to emit.
     Idx = TypeIdx(NextTypeID++);
@@ -3856,6 +3872,11 @@ DeclID ASTWriter::GetDeclRef(const Decl *D) {
   assert(!(reinterpret_cast<uintptr_t>(D) & 0x01) && "Invalid decl pointer");
   DeclID &ID = DeclIDs[D];
   if (ID == 0) {
+    if (DoneWritingDeclsAndTypes) {
+      assert(0 && "New decl seen after serializing all the decls to emit!");
+      return 0;
+    }
+
     // We haven't seen this declaration before. Give it a new ID and
     // enqueue it in the list of declarations to emit.
     ID = NextDeclID++;
@@ -4477,6 +4498,7 @@ void ASTWriter::AddedVisibleDecl(const DeclContext *DC, const Decl *D) {
     return; // Not a source decl added to a DeclContext from PCH.
 
   AddUpdatedDeclContext(DC);
+  UpdatingVisibleDecls.push_back(D);
 }
 
 void ASTWriter::AddedCXXImplicitMember(const CXXRecordDecl *RD, const Decl *D) {

clang/test/PCH/chain-staticvar-instantiation.cpp

@@ -0,0 +1,44 @@
+// Without PCH
+// RUN: %clang_cc1 -fsyntax-only -verify -include %s -include %s %s
+
+// With PCH
+// RUN: %clang_cc1 -fsyntax-only -verify %s -chain-include %s -chain-include %s
+
+#ifndef HEADER1
+#define HEADER1
+//===----------------------------------------------------------------------===//
+
+namespace NS {
+
+template <class _Tp, _Tp __v>
+struct TS
+{
+  static const _Tp value = __v;
+};
+
+template <class _Tp, _Tp __v>
+const _Tp TS<_Tp, __v>::value;
+
+TS<int, 2> g1;
+
+}
+
+//===----------------------------------------------------------------------===//
+#elif not defined(HEADER2)
+#define HEADER2
+#if !defined(HEADER1)
+#error Header inclusion order messed up
+#endif
+
+int g2 = NS::TS<int, 2>::value;
+
+//===----------------------------------------------------------------------===//
+#else
+//===----------------------------------------------------------------------===//
+
+#warning reached main file // expected-warning {{reached main file}}
+
+int g3 = NS::TS<int, 2>::value;
+
+//===----------------------------------------------------------------------===//
+#endif