Fix PR 4489, a crash in PCH loading that occurs when loading the name

of a top-level declaration loads another top-level declaration of the
same name whose type depends on the first declaration having been
completed. This commit breaks the circular dependency by delaying
loads of top-level declarations triggered by loading a name until we
are no longer recursively loading types or declarations.

llvm-svn: 74847

clang/include/clang/Frontend/PCHReader.h

@@ -30,6 +30,7 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Bitcode/BitstreamReader.h"
 #include "llvm/Support/DataTypes.h"
+#include <deque>
 #include <map>
 #include <string>
 #include <utility>
@@ -361,7 +362,41 @@ private:
 
   /// Number of visible decl contexts read/total.
   unsigned NumVisibleDeclContextsRead, TotalVisibleDeclContexts;
-
+      
+  /// \brief When a type or declaration is being loaded from the PCH file, an 
+  /// instantance of this RAII object will be available on the stack to 
+  /// indicate when we are in a recursive-loading situation.
+  class LoadingTypeOrDecl {
+    PCHReader &Reader;
+    LoadingTypeOrDecl *Parent;
+    
+    LoadingTypeOrDecl(const LoadingTypeOrDecl&); // do not implement
+    LoadingTypeOrDecl &operator=(const LoadingTypeOrDecl&); // do not implement
+    
+  public:
+    explicit LoadingTypeOrDecl(PCHReader &Reader);
+    ~LoadingTypeOrDecl();
+  };
+  friend class LoadingTypeOrDecl;
+  
+  /// \brief If we are currently loading a type or declaration, points to the
+  /// most recent LoadingTypeOrDecl object on the stack.
+  LoadingTypeOrDecl *CurrentlyLoadingTypeOrDecl;
+  
+  /// \brief An IdentifierInfo that has been loaded but whose top-level 
+  /// declarations of the same name have not (yet) been loaded.
+  struct PendingIdentifierInfo {
+    IdentifierInfo *II;
+    llvm::SmallVector<uint32_t, 4> DeclIDs;
+  };
+      
+  /// \brief The set of identifiers that were read while the PCH reader was
+  /// (recursively) loading declarations. 
+  /// 
+  /// The declarations on the identifier chain for these identifiers will be
+  /// loaded once the recursive loading has completed.
+  std::deque<PendingIdentifierInfo> PendingIdentifierInfos;
+      
   /// \brief FIXME: document!
   llvm::SmallVector<uint64_t, 4> SpecialTypes;
 
@@ -555,7 +590,10 @@ public:
     ReadMethodPool(Selector Sel);
 
   void SetIdentifierInfo(unsigned ID, IdentifierInfo *II);
-
+  void SetGloballyVisibleDecls(IdentifierInfo *II, 
+                               const llvm::SmallVectorImpl<uint32_t> &DeclIDs,
+                               bool Nonrecursive = false);
+      
   /// \brief Report a diagnostic.
   DiagnosticBuilder Diag(unsigned DiagID);
 

clang/lib/Frontend/PCHReader.cpp

@@ -361,7 +361,8 @@ PCHReader::PCHReader(SourceManager &SourceMgr, FileManager &FileMgr,
     TotalNumSelectors(0), NumStatHits(0), NumStatMisses(0), 
     NumSLocEntriesRead(0), NumStatementsRead(0), 
     NumMacrosRead(0), NumMethodPoolSelectorsRead(0), NumMethodPoolMisses(0),
-    NumLexicalDeclContextsRead(0), NumVisibleDeclContextsRead(0) { }
+    NumLexicalDeclContextsRead(0), NumVisibleDeclContextsRead(0),
+    CurrentlyLoadingTypeOrDecl(0) { }
 
 PCHReader::~PCHReader() {}
 
@@ -594,26 +595,14 @@ public:
 
     // Read all of the declarations visible at global scope with this
     // name.
-    Sema *SemaObj = Reader.getSema();
     if (Reader.getContext() == 0) return II;
-    
-    while (DataLen > 0) {
-      NamedDecl *D = cast<NamedDecl>(Reader.GetDecl(ReadUnalignedLE32(d)));
-      if (SemaObj) {
-        // Introduce this declaration into the translation-unit scope
-        // and add it to the declaration chain for this identifier, so
-        // that (unqualified) name lookup will find it.
-        SemaObj->TUScope->AddDecl(Action::DeclPtrTy::make(D));
-        SemaObj->IdResolver.AddDeclToIdentifierChain(II, D);
-      } else {
-        // Queue this declaration so that it will be added to the
-        // translation unit scope and identifier's declaration chain
-        // once a Sema object is known.
-        Reader.PreloadedDecls.push_back(D);
-      }
-
-      DataLen -= 4;
+    if (DataLen > 0) {
+      llvm::SmallVector<uint32_t, 4> DeclIDs;
+      for (; DataLen > 0; DataLen -= 4)
+        DeclIDs.push_back(ReadUnalignedLE32(d));
+      Reader.SetGloballyVisibleDecls(II, DeclIDs);
     }
+    
     return II;
   }
 };
@@ -625,7 +614,6 @@ public:
 typedef OnDiskChainedHashTable<PCHIdentifierLookupTrait> 
   PCHIdentifierLookupTable;
 
-// FIXME: use the diagnostics machinery
 bool PCHReader::Error(const char *Msg) {
   unsigned DiagID = Diags.getCustomDiagID(Diagnostic::Fatal, Msg);
   Diag(DiagID);
@@ -1686,6 +1674,9 @@ QualType PCHReader::ReadTypeRecord(uint64_t Offset) {
   // after reading this type.
   SavedStreamPosition SavedPosition(Stream);
 
+  // Note that we are loading a type record.
+  LoadingTypeOrDecl Loading(*this);
+  
   Stream.JumpToBit(Offset);
   RecordData Record;
   unsigned Code = Stream.ReadCode();
@@ -2220,6 +2211,52 @@ void PCHReader::SetIdentifierInfo(unsigned ID, IdentifierInfo *II) {
   IdentifiersLoaded[ID - 1] = II;
 }
 
+/// \brief Set the globally-visible declarations associated with the given
+/// identifier.
+///
+/// If the PCH reader is currently in a state where the given declaration IDs
+/// cannot safely be resolved, they are queued until it is safe to resolve 
+/// them.
+///
+/// \param II an IdentifierInfo that refers to one or more globally-visible
+/// declarations.
+///
+/// \param DeclIDs the set of declaration IDs with the name @p II that are
+/// visible at global scope.
+///
+/// \param Nonrecursive should be true to indicate that the caller knows that
+/// this call is non-recursive, and therefore the globally-visible declarations
+/// will not be placed onto the pending queue.
+void 
+PCHReader::SetGloballyVisibleDecls(IdentifierInfo *II, 
+                              const llvm::SmallVectorImpl<uint32_t> &DeclIDs,
+                                   bool Nonrecursive) {
+  if (CurrentlyLoadingTypeOrDecl && !Nonrecursive) {
+    PendingIdentifierInfos.push_back(PendingIdentifierInfo());
+    PendingIdentifierInfo &PII = PendingIdentifierInfos.back();
+    PII.II = II;
+    for (unsigned I = 0, N = DeclIDs.size(); I != N; ++I)
+      PII.DeclIDs.push_back(DeclIDs[I]);
+    return;
+  }
+      
+  for (unsigned I = 0, N = DeclIDs.size(); I != N; ++I) {
+    NamedDecl *D = cast<NamedDecl>(GetDecl(DeclIDs[I]));
+    if (SemaObj) {
+      // Introduce this declaration into the translation-unit scope
+      // and add it to the declaration chain for this identifier, so
+      // that (unqualified) name lookup will find it.
+      SemaObj->TUScope->AddDecl(Action::DeclPtrTy::make(D));
+      SemaObj->IdResolver.AddDeclToIdentifierChain(II, D);
+    } else {
+      // Queue this declaration so that it will be added to the
+      // translation unit scope and identifier's declaration chain
+      // once a Sema object is known.
+      PreloadedDecls.push_back(D);
+    }
+  }
+}
+
 IdentifierInfo *PCHReader::DecodeIdentifierInfo(unsigned ID) {
   if (ID == 0)
     return 0;
@@ -2432,3 +2469,24 @@ void PCHReader::SetLabelOf(AddrLabelExpr *S, unsigned ID) {
     UnresolvedAddrLabelExprs.insert(std::make_pair(ID, S));
   }
 }
+
+
+PCHReader::LoadingTypeOrDecl::LoadingTypeOrDecl(PCHReader &Reader) 
+  : Reader(Reader), Parent(Reader.CurrentlyLoadingTypeOrDecl) {
+  Reader.CurrentlyLoadingTypeOrDecl = this;
+}
+
+PCHReader::LoadingTypeOrDecl::~LoadingTypeOrDecl() {
+  if (!Parent) {
+    // If any identifiers with corresponding top-level declarations have
+    // been loaded, load those declarations now.
+    while (!Reader.PendingIdentifierInfos.empty()) {
+      Reader.SetGloballyVisibleDecls(Reader.PendingIdentifierInfos.front().II,
+                                 Reader.PendingIdentifierInfos.front().DeclIDs,
+                                     true);
+      Reader.PendingIdentifierInfos.pop_front();
+    }
+  }
+
+  Reader.CurrentlyLoadingTypeOrDecl = Parent;  
+}

clang/lib/Frontend/PCHReaderDecl.cpp

@@ -581,6 +581,9 @@ Decl *PCHReader::ReadDeclRecord(uint64_t Offset, unsigned Index) {
   // after reading this declaration.
   SavedStreamPosition SavedPosition(DeclsCursor);
 
+  // Note that we are loading a declaration record.
+  LoadingTypeOrDecl Loading(*this);
+  
   DeclsCursor.JumpToBit(Offset);
   RecordData Record;
   unsigned Code = DeclsCursor.ReadCode();

clang/test/PCH/pr4489.c

@@ -17,4 +17,12 @@ int x(void)
 void y(void) {
   extern char z;
   fprintf (0, "a");
+}
+
+struct y0 { int i; } y0[1] = {};
+
+void x0(void)
+{
+  extern char z0;
+  fprintf (0, "a");
 }