r215810 - When loading a module with no local entities, still bump the size of the

[Ben Langmuir]

Author: benlangmuir
Date: Fri Aug 15 23:54:18 2014
New Revision: 215810

URL: http://llvm.org/viewvc/llvm-project?rev=215810&view=rev
Log:
When loading a module with no local entities, still bump the size of the

tables that correspond to ContinuousRangeMaps, since the keys to those
maps need to be unique, or we may map to the wrong offset.

This fixes a crash + malformed AST file seen when loading some modules
that import Cocoa on Darwin, which is a module with no contents except
imports of other modules. Unfortunately I have not been able to find a
reduced test case that reproduces this problem.

Also add an assert that we aren't mapping one key to multiple values
in CRM.  We ought to be able to say there are no duplicate keys at all,
but there are a bunch of 0 -> 0 mappings that are showing up, probably
coming from the source location table.

Modified:
    cfe/trunk/include/clang/Serialization/ContinuousRangeMap.h
    cfe/trunk/lib/Serialization/ASTReader.cpp
    cfe/trunk/lib/Serialization/ASTWriter.cpp

Modified: cfe/trunk/include/clang/Serialization/ContinuousRangeMap.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ContinuousRangeMap.h?rev=215810&r1=215809&r2=215810&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ContinuousRangeMap.h (original)
+++ cfe/trunk/include/clang/Serialization/ContinuousRangeMap.h Fri Aug 15 23:54:18 2014
@@ -117,6 +117,14 @@ public:
     
     ~Builder() {
       std::sort(Self.Rep.begin(), Self.Rep.end(), Compare());
+      std::unique(Self.Rep.begin(), Self.Rep.end(),
+                  [](const_reference A, const_reference B) {
+        // FIXME: we should not allow any duplicate keys, but there are a lot of
+        // duplicate 0 -> 0 mappings to remove first.
+        assert((A == B || A.first != B.first) &&
+               "ContinuousRangeMap::Builder given non-unique keys");
+        return A == B;
+      });
     }
     
     void insert(const value_type &Val) {

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=215810&r1=215809&r2=215810&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Fri Aug 15 23:54:18 2014
@@ -2620,9 +2620,9 @@ ASTReader::ReadASTBlock(ModuleFile &F, u
         F.TypeRemap.insertOrReplace(
           std::make_pair(LocalBaseTypeIndex, 
                          F.BaseTypeIndex - LocalBaseTypeIndex));
-        
-        TypesLoaded.resize(TypesLoaded.size() + F.LocalNumTypes);
       }
+      // Increase size by >= 1 so we get a unique base index in the next module.
+      TypesLoaded.resize(TypesLoaded.size() + std::max(F.LocalNumTypes, 1U));
       break;
     }
         
@@ -2650,9 +2650,10 @@ ASTReader::ReadASTBlock(ModuleFile &F, u
         // Introduce the global -> local mapping for declarations within this
         // module.
         F.GlobalToLocalDeclIDs[&F] = LocalBaseDeclID;
-        
-        DeclsLoaded.resize(DeclsLoaded.size() + F.LocalNumDecls);
       }
+
+      // Increase size by >= 1 so we get a unique base index in the next module.
+      DeclsLoaded.resize(DeclsLoaded.size() + std::max(F.LocalNumDecls, 1U));
       break;
     }
         
@@ -2720,10 +2721,11 @@ ASTReader::ReadASTBlock(ModuleFile &F, u
         F.IdentifierRemap.insertOrReplace(
           std::make_pair(LocalBaseIdentifierID,
                          F.BaseIdentifierID - LocalBaseIdentifierID));
-        
-        IdentifiersLoaded.resize(IdentifiersLoaded.size() 
-                                 + F.LocalNumIdentifiers);
       }
+
+      // Increase size by >= 1 so we get a unique base index in the next module.
+      IdentifiersLoaded.resize(IdentifiersLoaded.size() +
+                               std::max(F.LocalNumIdentifiers, 1U));
       break;
     }
 
@@ -2813,9 +2815,10 @@ ASTReader::ReadASTBlock(ModuleFile &F, u
         F.SelectorRemap.insertOrReplace(
           std::make_pair(LocalBaseSelectorID,
                          F.BaseSelectorID - LocalBaseSelectorID));
-
-        SelectorsLoaded.resize(SelectorsLoaded.size() + F.LocalNumSelectors);        
       }
+      // Increase size by >= 1 so we get a unique base index in the next module.
+      SelectorsLoaded.resize(SelectorsLoaded.size() +
+                             std::max(F.LocalNumSelectors, 1U));
       break;
     }
         
@@ -2855,8 +2858,10 @@ ASTReader::ReadASTBlock(ModuleFile &F, u
       F.SLocEntryOffsets = (const uint32_t *)Blob.data();
       F.LocalNumSLocEntries = Record[0];
       unsigned SLocSpaceSize = Record[1];
+
+      // Increase size by >= 1 so we get a unique base index in the next module.
       std::tie(F.SLocEntryBaseID, F.SLocEntryBaseOffset) =
-          SourceMgr.AllocateLoadedSLocEntries(F.LocalNumSLocEntries,
+          SourceMgr.AllocateLoadedSLocEntries(std::max(F.LocalNumSLocEntries, 1U),
                                               SLocSpaceSize);
       // Make our entry in the range map. BaseID is negative and growing, so
       // we invert it. Because we invert it, though, we need the other end of
@@ -3049,9 +3054,11 @@ ASTReader::ReadASTBlock(ModuleFile &F, u
         PP.createPreprocessingRecord();
       if (!PP.getPreprocessingRecord()->getExternalSource())
         PP.getPreprocessingRecord()->SetExternalSource(*this);
+
+      // Increase size by >= 1 so we get a unique base index in the next module.
       StartingID 
         = PP.getPreprocessingRecord()
-            ->allocateLoadedEntities(F.NumPreprocessedEntities);
+            ->allocateLoadedEntities(std::max(F.NumPreprocessedEntities, 1U));
       F.BasePreprocessedEntityID = StartingID;
 
       if (F.NumPreprocessedEntities > 0) {
@@ -3255,9 +3262,9 @@ ASTReader::ReadASTBlock(ModuleFile &F, u
         F.MacroRemap.insertOrReplace(
           std::make_pair(LocalBaseMacroID,
                          F.BaseMacroID - LocalBaseMacroID));
-
-        MacrosLoaded.resize(MacrosLoaded.size() + F.LocalNumMacros);
       }
+      // Increase size by >= 1 so we get a unique base index in the next module.
+      MacrosLoaded.resize(MacrosLoaded.size() + std::max(F.LocalNumMacros, 1U));
       break;
     }
 
@@ -4509,9 +4516,11 @@ ASTReader::ReadSubmoduleBlock(ModuleFile
         F.SubmoduleRemap.insertOrReplace(
           std::make_pair(LocalBaseSubmoduleID,
                          F.BaseSubmoduleID - LocalBaseSubmoduleID));
-        
-        SubmodulesLoaded.resize(SubmodulesLoaded.size() + F.LocalNumSubmodules);
-      }      
+      }
+
+      // Increase size by >= 1 so we get a unique base index in the next module.
+      SubmodulesLoaded.resize(SubmodulesLoaded.size() +
+                              std::max(F.LocalNumSubmodules, 1U));
       break;
     }
         

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=215810&r1=215809&r2=215810&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Fri Aug 15 23:54:18 2014
@@ -4458,6 +4458,9 @@ void ASTWriter::WriteASTCore(Sema &SemaR
         StringRef FileName = (*M)->FileName;
         LE.write<uint16_t>(FileName.size());
         Out.write(FileName.data(), FileName.size());
+
+        // These values should be unique within a chain, since they will be read
+        // as keys into ContinuousRangeMaps.
         LE.write<uint32_t>((*M)->SLocEntryBaseOffset);
         LE.write<uint32_t>((*M)->BaseIdentifierID);
         LE.write<uint32_t>((*M)->BaseMacroID);