[clang] e12f6c2 - [modules] Fix error "malformed or corrupted AST file: 'SourceLocation remap refers to unknown module...'".

[Volodymyr Sapsai via cfe-commits]

Author: Volodymyr Sapsai
Date: 2022-09-20T17:55:37-07:00
New Revision: e12f6c26c394bd5b49e7c1e1c157bcee3a33d1de

URL: https://github.com/llvm/llvm-project/commit/e12f6c26c394bd5b49e7c1e1c157bcee3a33d1de
DIFF: https://github.com/llvm/llvm-project/commit/e12f6c26c394bd5b49e7c1e1c157bcee3a33d1de.diff

LOG: [modules] Fix error "malformed or corrupted AST file: 'SourceLocation remap refers to unknown module...'".

When a framework can be found at a new location, all references to it in
the module cache become outdated. When we try to load such outdated .pcm
file, we shouldn't change any already loaded and processed modules.

If `Module` has `ASTFile`, it means we've read its AST block already and
it is too late to undo that. If `ASTFile` is `None`, there is no value
in setting it to `None` again. So we don't reset `ASTFile` in
`ModuleManager::removeModules` at all.

rdar://97216258

Differential Revision: https://reviews.llvm.org/D134249

Added: 
    clang/test/Modules/dependent-module-different-location.m

Modified: 
    clang/include/clang/Basic/Module.h
    clang/include/clang/Serialization/ModuleManager.h
    clang/lib/Serialization/ASTReader.cpp
    clang/lib/Serialization/ModuleManager.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h
index a2309f5d7fae..1a1feb9d4d3f 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -608,8 +608,7 @@ class Module {
 
   /// Set the serialized AST file for the top-level module of this module.
   void setASTFile(Optional<FileEntryRef> File) {
-    assert((!File || !getASTFile() || getASTFile() == File) &&
-           "file path changed");
+    assert((!getASTFile() || getASTFile() == File) && "file path changed");
     getTopLevelModule()->ASTFile = File;
   }
 

diff  --git a/clang/include/clang/Serialization/ModuleManager.h b/clang/include/clang/Serialization/ModuleManager.h
index 4305bae5ee95..1623dd446036 100644
--- a/clang/include/clang/Serialization/ModuleManager.h
+++ b/clang/include/clang/Serialization/ModuleManager.h
@@ -250,7 +250,7 @@ class ModuleManager {
                             std::string &ErrorStr);
 
   /// Remove the modules starting from First (to the end).
-  void removeModules(ModuleIterator First, ModuleMap *modMap);
+  void removeModules(ModuleIterator First);
 
   /// Add an in-memory buffer the list of known buffers
   void addInMemoryBuffer(StringRef FileName,

diff  --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 2716a5971c8f..ad6db3ce1993 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -4227,10 +4227,7 @@ ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName,
           ReadASTCore(FileName, Type, ImportLoc,
                       /*ImportedBy=*/nullptr, Loaded, 0, 0, ASTFileSignature(),
                       ClientLoadCapabilities)) {
-    ModuleMgr.removeModules(ModuleMgr.begin() + NumModules,
-                            PP.getLangOpts().Modules
-                                ? &PP.getHeaderSearchInfo().getModuleMap()
-                                : nullptr);
+    ModuleMgr.removeModules(ModuleMgr.begin() + NumModules);
 
     // If we find that any modules are unusable, the global index is going
     // to be out-of-date. Just remove it.

diff  --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp
index 4fd217cf7a6e..544dac1716b0 100644
--- a/clang/lib/Serialization/ModuleManager.cpp
+++ b/clang/lib/Serialization/ModuleManager.cpp
@@ -249,7 +249,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
   return NewlyLoaded;
 }
 
-void ModuleManager::removeModules(ModuleIterator First, ModuleMap *modMap) {
+void ModuleManager::removeModules(ModuleIterator First) {
   auto Last = end();
   if (First == Last)
     return;
@@ -280,19 +280,10 @@ void ModuleManager::removeModules(ModuleIterator First, ModuleMap *modMap) {
     }
   }
 
-  // Delete the modules and erase them from the various structures.
-  for (ModuleIterator victim = First; victim != Last; ++victim) {
+  // Delete the modules.
+  for (ModuleIterator victim = First; victim != Last; ++victim)
     Modules.erase(victim->File);
 
-    if (modMap) {
-      StringRef ModuleName = victim->ModuleName;
-      if (Module *mod = modMap->findModule(ModuleName)) {
-        mod->setASTFile(None);
-      }
-    }
-  }
-
-  // Delete the modules.
   Chain.erase(Chain.begin() + (First - begin()), Chain.end());
 }
 

diff  --git a/clang/test/Modules/dependent-module-
diff erent-location.m b/clang/test/Modules/dependent-module-
diff erent-location.m
new file mode 100644
index 000000000000..f969aa9958bd
--- /dev/null
+++ b/clang/test/Modules/dependent-module-
diff erent-location.m
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+//
+// At first build Stable.pcm that references Movable.framework from StableFrameworks.
+// RUN: %clang_cc1 -fsyntax-only -F %t/JustBuilt -F %t/StableFrameworks %t/prepopulate-module-cache.m \
+// RUN:            -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+//
+// Now add Movable.framework to JustBuilt.
+// RUN: mkdir %t/JustBuilt
+// RUN: cp -r %t/StableFrameworks/Movable.framework %t/JustBuilt/Movable.framework
+//
+// Load Movable.pcm at first for JustBuilt location and then in the same TU try to load transitively for StableFrameworks location.
+// RUN: %clang_cc1 -fsyntax-only -F %t/JustBuilt -F %t/StableFrameworks %t/trigger-error.m \
+// RUN:            -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
+
+// Test the case when a dependent module is found in a 
diff erent location, so
+// module cache has outdated information. <rdar://97216258>
+
+//--- StableFrameworks/Movable.framework/Headers/Movable.h
+// empty
+
+//--- StableFrameworks/Movable.framework/Modules/module.modulemap
+framework module Movable {
+  header "Movable.h"
+  export *
+}
+
+
+//--- StableFrameworks/Stable.framework/Headers/Stable.h
+#import <Movable/Movable.h>
+
+//--- StableFrameworks/Stable.framework/Modules/module.modulemap
+framework module Stable {
+  header "Stable.h"
+  export *
+}
+
+
+//--- prepopulate-module-cache.m
+#import <Stable/Stable.h>
+
+//--- trigger-error.m
+#import <Movable/Movable.h>
+#import <Stable/Stable.h>