[modules] Add `-fno-absolute-module-directory` flag for relocatable modules

[andrewjcg]

Summary

Currently, modules built using module map files embed the path to the directory
that houses their inputs (e.g. headers and module map file) into the output
module file. This path is embedded as an absolute path and the various input
files are either relativized to this path (if they are underneath it) or also
embedded as an absolute path.

This adds a flag which disables making the module directory an absolute path.
To support relative module directory paths, this diff also updates the serialized
AST to write out an extra bit with serialized paths to indicate whether they are
relative to the module directory or not (before this diff, these were handled by
absolutify paths that weren't relative the module directory).

[andrewjcg]

I'm not sure this is the best approach, but I wasn't sure of a better one (to support module files w/o absolute paths). Another approach I tried, was relativizing the other input files (from outside the module directory) using chains of ../ (e.g. ../../../../../other/modules/module.modulemap), but this causes issues when symlinks appear in the module directory path. Another potential option could be to add a bit to the serialized module format for each input, to allow some inputs to mark themselves as relative to the CWD, rather than the module directory.

[andrewjcg]

fix umbrella writing

[andrewjcg]

Dropping the module directory entirely and fully resolving paths on serialization
broke some things during deserialization, specifically when the deserializer wanted
to update paths to use an alternate module directory.

This switches to a different strategy of only relativizing the paths that are
actually under the module home dir, and adding a bit to the serialized paths to
indiciate this. This bit is read on deserializiation to determine whether the
path is resolved against the module directory or not.

[modocache]

Friendly ping! Could someone recommend a reviewer for this? Or is there something wrong with the patch?

Looking at the blame I can see @manmanren; I believe you're not actively working in this part of the codebase, but could you review, or recommend someone who could? Or maybe @bruno?

[manmanren]

I am not sure if this is the best approach, but the implementation looks okay to me. @bruno @rsmith What do you think?

Manman

include/clang/Serialization/ASTReader.h

2241

Can you expand on this comment? Skip the relative bit?

include/clang/Serialization/ASTWriter.h

642

Can you add comment here on how we set IsRelativeModuleDirectory?

650

Can you update the comment to say that record is updated with the extra bit?

[rsmith]

I don't think we need to change the serialization format for this: a serialized path beginning with / is already treated as absolute and any other path is already treated as relative, so we don't need a flag to carry that information.

I'm also not convinced we need to put this behind a flag. It would seem reasonable to me to simply always emit the MODULE_DIRECTORY as relative (if we found the module via a relative path), at least for an explicitly-built module. (For an implicitly built module, we probably need to track the absolute path so we can detect whether we found the right PCM file.)

lib/Serialization/ASTWriter.cpp

4524–4546

The intent of this function is to use an absolute form of both the file path and BaseDirectory so that we can strip off BaseDirectory even if the file path ends up absolute somehow. Rather than changing BaseDirectory above and then changing the code here to compensate, could you only change the code that writes out the MODULE_DIRECTORY record to write a relative path?

[bruno]

Thanks for working on this @andrewjcg

In D51568#1313717, @rsmith wrote:

I'm also not convinced we need to put this behind a flag. It would seem reasonable to me to simply always emit the MODULE_DIRECTORY as relative (if we found the module via a relative path), at least for an explicitly-built module. (For an implicitly built module, we probably need to track the absolute path so we can detect whether we found the right PCM file.)

+1

[andrewjcg]

I don't think we need to change the serialization format for this: a serialized path beginning with / is already treated as absolute and any other path is already treated as relative, so we don't need a flag to carry that information.

But I think we need this since we now have two types of relative paths -- a CWD-relative path and a module-home-relative path -- and we use this flag to discern them for the AST reader. Previously, because cleanPathForOutput would always absolutify input paths, we didn't need this flag -- any relative path was relative the module home and all other paths were absolute.

I attempted another take on this diff which just made all paths relative the CWD (and did away with module home relative paths), but this didn't work since the reader substitutes in a new module home.

I'm also not convinced we need to put this behind a flag. It would seem reasonable to me to simply always emit the MODULE_DIRECTORY as relative (if we found the module via a relative path), at least for an explicitly-built module.

Yeah, makes sense. Will fix this.

lib/Serialization/ASTWriter.cpp

4524–4546

So we also several non-module-home relative paths get processed here, so just I think we'd have to throw away the output of cleanPathForOutput if the Changed == false, to preserve the input relative path when non-module-home-relative?

[rsmith]

In D51568#1314004, @andrewjcg wrote:

I don't think we need to change the serialization format for this: a serialized path beginning with / is already treated as absolute and any other path is already treated as relative, so we don't need a flag to carry that information.

But I think we need this since we now have two types of relative paths -- a CWD-relative path and a module-home-relative path -- and we use this flag to discern them for the AST reader. Previously, because cleanPathForOutput would always absolutify input paths, we didn't need this flag -- any relative path was relative the module home and all other paths were absolute.

Ah, of course, that's exactly what I was missing :) I guess I live in a too--fmodule-map-file-home-is-cwd-centric world where the two are always the same (even when modules are relocated).

Following the 0-1-infinity principle, I think it might make sense to have a 'catalog' of prefixes for paths (relative to the module, relative to the compiler's CWD, relative to the compiler's resource directory, relative to the sysroot) that we try stripping off, with different markers to say which one we removed. (Right now, if you relocate the compiler, you invalidate any .pcm file that references files in its resource directory, for instance.) But this seems like a step in a good direction.

Just checking that we're on the same page here... suppose I do this:

• compile module described in foo/module.modulemap (with no -fmodule-map-file-home-is-cwd, so module-relative paths have the foo/ prefix stripped off them)
• use -Ibar/ to find some textual headers

Then, if I relocate the foo/ module to elsewhere/foo and pass the corresponding pcm file to a compilation using that module, I will still expect to find the bar/ files referenced by the pcm file relative to the compiler's working directory, not in elsewhere/bar. Is that what you're expecting, or would you expect the file paths to be emitted as module-relative ../bar/... paths? (Note that the latter can break if foo is a symlink.)

lib/Serialization/ASTReader.cpp

2094–2096

This'd be more readable (throughout the patch) spelled as IsRelativeToModuleDirectory. (I was wondering "What is a relative module directory?")

include/clang/Driver/CC1Options.td

 //===----------------------------------------------------------------------===//
 
 def nostdsysteminc : Flag<["-"], "nostdsysteminc">,
   HelpText<"Disable standard system #include directories">;
 def fdisable_module_hash : Flag<["-"], "fdisable-module-hash">,
   HelpText<"Disable the module hash">;
 def fmodules_hash_content : Flag<["-"], "fmodules-hash-content">,
   HelpText<"Enable hashing the content of a module file">;
+def fno_absolute_module_directory : Flag<["-"], "fno-absolute-module-directory">,
+  HelpText<"Do not force module home directory path to be absolute.">;
 def c_isystem : JoinedOrSeparate<["-"], "c-isystem">, MetaVarName<"<directory>">,
   HelpText<"Add directory to the C SYSTEM include search path">;
 def objc_isystem : JoinedOrSeparate<["-"], "objc-isystem">,
   MetaVarName<"<directory>">,
   HelpText<"Add directory to the ObjC SYSTEM include search path">;
 def objcxx_isystem : JoinedOrSeparate<["-"], "objcxx-isystem">,
   MetaVarName<"<directory>">,
   HelpText<"Add directory to the ObjC++ SYSTEM include search path">;

include/clang/Lex/HeaderSearchOptions.h

public:
 
   /// Whether the module includes debug information (-gmodules).
   unsigned UseDebugInfo : 1;
 
   unsigned ModulesValidateDiagnosticOptions : 1;
 
   unsigned ModulesHashContent : 1;
 
+  /// Do not absolutify the path to the module directory.
+  unsigned NoAbsoluteModuleDirectory : 1;
+
   HeaderSearchOptions(StringRef _Sysroot = "/")
       : Sysroot(_Sysroot), ModuleFormat("raw"), DisableModuleHash(false),
         ImplicitModuleMaps(false), ModuleMapFileHomeIsCwd(false),
         UseBuiltinIncludes(true), UseStandardSystemIncludes(true),
         UseStandardCXXIncludes(true), UseLibcxx(false), Verbose(false),
         ModulesValidateOncePerBuildSession(false),
         ModulesValidateSystemHeaders(false), UseDebugInfo(false),
-        ModulesValidateDiagnosticOptions(true), ModulesHashContent(false) {}
+        ModulesValidateDiagnosticOptions(true), ModulesHashContent(false),
+        NoAbsoluteModuleDirectory(false) {}
 
   /// AddPath - Add the \p Path path to the specified \p Group list.
   void AddPath(StringRef Path, frontend::IncludeDirGroup Group,
                bool IsFramework, bool IgnoreSysRoot) {
     UserEntries.emplace_back(Path, Group, IsFramework, IgnoreSysRoot);
   }
 
   /// AddSystemHeaderPrefix - Override whether \#include directives naming a

include/clang/Serialization/ASTReader.h

public:
   }
 
   // Read a path
   std::string ReadPath(ModuleFile &F, const RecordData &Record, unsigned &Idx);
 
   // Skip a path
   static void SkipPath(const RecordData &Record, unsigned &Idx) {
     SkipString(Record, Idx);
+    Idx++;  // Relative
   }
 
   /// Read a version tuple.
   static VersionTuple ReadVersionTuple(const RecordData &Record, unsigned &Idx);
 
   CXXTemporary *ReadCXXTemporary(ModuleFile &F, const RecordData &Record,
                                  unsigned &Idx);
 

include/clang/Serialization/ASTWriter.h

public:
   serialization::DeclID getDeclID(const Decl *D);
 
   unsigned getAnonymousDeclarationNumber(const NamedDecl *D);
 
   /// Add a string to the given record.
   void AddString(StringRef Str, RecordDataImpl &Record);
 
   /// Convert a path from this build process into one that is appropriate
   /// for emission in the module file.
-  bool PreparePathForOutput(SmallVectorImpl<char> &Path);
+  bool PreparePathForOutput(SmallVectorImpl<char> &Path,
+                            bool &IsRelativeModuleDirectory);
 
   /// Add a path to the given record.
   void AddPath(StringRef Path, RecordDataImpl &Record);
 
   /// Emit the current record with the given path as a blob.
-  void EmitRecordWithPath(unsigned Abbrev, RecordDataRef Record,
+  void EmitRecordWithPath(unsigned Abbrev, RecordDataImpl &Record,
                           StringRef Path);
 
   /// Add a version tuple to the given record
   void AddVersionTuple(const VersionTuple &Version, RecordDataImpl &Record);
 
   /// Retrieve or create a submodule ID for this module, or return 0 if
   /// the submodule is neither local (a submodle of the currently-written module)
   /// nor from an imported module.

lib/Frontend/CompilerInvocation.cpp

for (const auto *A : Args.filtered(OPT_fmodule_file)) {
     StringRef Val = A->getValue();
     if (Val.find('=') != StringRef::npos)
       Opts.PrebuiltModuleFiles.insert(Val.split('='));
   }
   for (const auto *A : Args.filtered(OPT_fprebuilt_module_path))
     Opts.AddPrebuiltModulePath(A->getValue());
   Opts.DisableModuleHash = Args.hasArg(OPT_fdisable_module_hash);
   Opts.ModulesHashContent = Args.hasArg(OPT_fmodules_hash_content);
+  Opts.NoAbsoluteModuleDirectory = Args.hasArg(OPT_fno_absolute_module_directory);
   Opts.ModulesValidateDiagnosticOptions =
       !Args.hasArg(OPT_fmodules_disable_diagnostic_validation);
   Opts.ImplicitModuleMaps = Args.hasArg(OPT_fimplicit_module_maps);
   Opts.ModuleMapFileHomeIsCwd = Args.hasArg(OPT_fmodule_map_file_home_is_cwd);
   Opts.ModuleCachePruneInterval =
       getLastArgIntValue(Args, OPT_fmodules_prune_interval, 7 * 24 * 60 * 60);
   Opts.ModuleCachePruneAfter =
       getLastArgIntValue(Args, OPT_fmodules_prune_after, 31 * 24 * 60 * 60);

lib/Serialization/ASTReader.cpp

 unsigned HeaderFileInfoTrait::ComputeHash(internal_key_ref ikey) {
   return llvm::hash_combine(ikey.Size, ikey.ModTime);
 }
 
 HeaderFileInfoTrait::internal_key_type
 HeaderFileInfoTrait::GetInternalKey(const FileEntry *FE) {
   internal_key_type ikey = {FE->getSize(),
                             M.HasTimestamps ? FE->getModificationTime() : 0,
+                            /*IsRelativeModuleDirectory*/ false,
                             FE->getName(), /*Imported*/ false};
   return ikey;
 }
 
 bool HeaderFileInfoTrait::EqualKey(internal_key_ref a, internal_key_ref b) {
   if (a.Size != b.Size || (a.ModTime && b.ModTime && a.ModTime != b.ModTime))
     return false;
 
   if (llvm::sys::path::is_absolute(a.Filename) && a.Filename == b.Filename)
     return true;
 
   // Determine whether the actual files are equivalent.
   FileManager &FileMgr = Reader.getFileManager();
   auto GetFile = [&](const internal_key_type &Key) -> const FileEntry* {
     if (!Key.Imported)
       return FileMgr.getFile(Key.Filename);
 
     std::string Resolved = Key.Filename;
+    if (Key.IsRelativeModuleDirectory)
       Reader.ResolveImportedPath(M, Resolved);
     return FileMgr.getFile(Resolved);
   };
 
   const FileEntry *FEA = GetFile(a);
   const FileEntry *FEB = GetFile(b);
   return FEA && FEA == FEB;
 }
 
 std::pair<unsigned, unsigned>
 HeaderFileInfoTrait::ReadKeyDataLength(const unsigned char*& d) {
   using namespace llvm::support;
 
   unsigned KeyLen = (unsigned) endian::readNext<uint16_t, little, unaligned>(d);
   unsigned DataLen = (unsigned) *d++;
   return std::make_pair(KeyLen, DataLen);
 }
 
 HeaderFileInfoTrait::internal_key_type
 HeaderFileInfoTrait::ReadKey(const unsigned char *d, unsigned) {
   using namespace llvm::support;
 
   internal_key_type ikey;
   ikey.Size = off_t(endian::readNext<uint64_t, little, unaligned>(d));
   ikey.ModTime = time_t(endian::readNext<uint64_t, little, unaligned>(d));
+  ikey.IsRelativeModuleDirectory = bool(endian::readNext<uint8_t, little, unaligned>(d));
   ikey.Filename = (const char *)d;
   ikey.Imported = true;
   return ikey;
 }
 
 HeaderFileInfoTrait::data_type
 HeaderFileInfoTrait::ReadData(internal_key_ref key, const unsigned char *d,
                               unsigned DataLen) {
while (d != End) {
     // implicit module import.
     SubmoduleID GlobalSMID = Reader.getGlobalSubmoduleID(M, LocalSMID);
     Module *Mod = Reader.getSubmodule(GlobalSMID);
     FileManager &FileMgr = Reader.getFileManager();
     ModuleMap &ModMap =
         Reader.getPreprocessor().getHeaderSearchInfo().getModuleMap();
 
     std::string Filename = key.Filename;
-    if (key.Imported)
+    if (key.Imported && key.IsRelativeModuleDirectory)
       Reader.ResolveImportedPath(M, Filename);
     // FIXME: This is not always the right filename-as-written, but we're not
     // going to use this information to rebuild the module, so it doesn't make
     // a lot of difference.
     Module::Header H = { key.Filename, FileMgr.getFile(Filename) };
     ModMap.addHeader(Mod, H, HeaderRole, /*Imported*/true);
     HFI.isModuleHeader |= !(HeaderRole & ModuleMap::TextualHeader);
   }
ASTReader::readInputFileInfo(ModuleFile &F, unsigned ID) {
 
   assert(Record[0] == ID && "Bogus stored ID or offset");
   InputFileInfo R;
   R.StoredSize = static_cast<off_t>(Record[1]);
   R.StoredTime = static_cast<time_t>(Record[2]);
   R.Overridden = static_cast<bool>(Record[3]);
   R.Transient = static_cast<bool>(Record[4]);
   R.TopLevelModuleMap = static_cast<bool>(Record[5]);
+  bool IsRelativeModuleDirectory = static_cast<bool>(Record[6]);
   R.Filename = Blob;
+  if (IsRelativeModuleDirectory) {
     ResolveImportedPath(F, R.Filename);
+  }
   return R;
 }
 
 static unsigned moduleKindForDiagnostic(ModuleKind Kind);
 InputFile ASTReader::getInputFile(ModuleFile &F, unsigned ID, bool Complain) {
   // If this ID is bogus, just return an empty input file.
   if (ID == 0 || ID > F.InputFilesLoaded.size())
     return InputFile();
case IMPORTS: {
         case ConfigurationMismatch: return ConfigurationMismatch;
         case HadErrors: return HadErrors;
         case Success: break;
         }
       }
       break;
     }
 
-    case ORIGINAL_FILE:
+    case ORIGINAL_FILE: {
       F.OriginalSourceFileID = FileID::get(Record[0]);
+      bool IsRelativeModuleDirectory = static_cast<bool>(Record[1]);
       F.ActualOriginalSourceFileName = Blob;
       F.OriginalSourceFileName = F.ActualOriginalSourceFileName;
+      if (IsRelativeModuleDirectory)
         ResolveImportedPath(F, F.OriginalSourceFileName);
       break;
+    }
 
     case ORIGINAL_FILE_ID:
       F.OriginalSourceFileID = FileID::get(Record[0]);
       break;
 
     case ORIGINAL_PCH_DIR:
       F.OriginalDir = Blob;
       break;
case INPUT_FILE_OFFSETS: {
 
         unsigned Code = Cursor.ReadCode();
         RecordData Record;
         StringRef Blob;
         bool shouldContinue = false;
         switch ((InputFileRecordTypes)Cursor.readRecord(Code, Record, &Blob)) {
         case INPUT_FILE:
           bool Overridden = static_cast<bool>(Record[3]);
+          bool IsRelativeModuleDirectory = static_cast<bool>(Record[4]);
           std::string Filename = Blob;
+          if (IsRelativeModuleDirectory)
             ResolveImportedPath(Filename, ModuleDir);
           shouldContinue = Listener.visitInputFile(
               Filename, isSystemFile, Overridden, /*IsExplicitModule*/false);
           break;
         }
         if (!shouldContinue)
           break;
       }
       break;
std::string ASTReader::ReadString(const RecordData &Record, unsigned &Idx) {
   std::string Result(Record.data() + Idx, Record.data() + Idx + Len);
   Idx += Len;
   return Result;
 }
 
 std::string ASTReader::ReadPath(ModuleFile &F, const RecordData &Record,
                                 unsigned &Idx) {
   std::string Filename = ReadString(Record, Idx);
+  bool IsRelativeModuleDirectory = Record[Idx++];
+  if (IsRelativeModuleDirectory)
     ResolveImportedPath(F, Filename);
   return Filename;
 }
 
 VersionTuple ASTReader::ReadVersionTuple(const RecordData &Record,
                                          unsigned &Idx) {
   unsigned Major = Record[Idx++];
   unsigned Minor = Record[Idx++];
   unsigned Subminor = Record[Idx++];

lib/Serialization/ASTReaderInternals.h

class HeaderFileInfoTrait {
   const char *FrameworkStrings;
 
 public:
   using external_key_type = const FileEntry *;
 
   struct internal_key_type {
     off_t Size;
     time_t ModTime;
+    bool IsRelativeModuleDirectory;
     StringRef Filename;
     bool Imported;
   };
 
   using internal_key_ref = const internal_key_type &;
 
   using data_type = HeaderFileInfo;
   using hash_value_type = unsigned;

lib/Serialization/ASTWriter.cpp

#undef BLOCK
   Stream.ExitBlock();
 }
 
 /// Prepares a path for being written to an AST file by converting it
 /// to an absolute path and removing nested './'s.
 ///
 /// \return \c true if the path was changed.
 static bool cleanPathForOutput(FileManager &FileMgr,
-                               SmallVectorImpl<char> &Path) {
-  bool Changed = FileMgr.makeAbsolutePath(Path);
-  return Changed | llvm::sys::path::remove_dots(Path);
+                               SmallVectorImpl<char> &Path,
+                               bool MakeAbsolute = true) {
+  bool Changed = false;
+  if (MakeAbsolute) {
+    Changed |= FileMgr.makeAbsolutePath(Path);
+  }
+  Changed |= llvm::sys::path::remove_dots(Path);
+  return Changed;
 }
 
 /// Adjusts the given filename to only write out the portion of the
 /// filename that is not part of the system root directory.
 ///
 /// \param Filename the file name to adjust.
 ///
 /// \param BaseDir When non-NULL, the PCH file is a relocatable AST file and
if (WritingModule) {
     Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Name
     unsigned AbbrevCode = Stream.EmitAbbrev(std::move(Abbrev));
     RecordData::value_type Record[] = {MODULE_NAME};
     Stream.EmitRecordWithBlob(AbbrevCode, Record, WritingModule->Name);
   }
 
   if (WritingModule && WritingModule->Directory) {
     SmallString<128> BaseDir(WritingModule->Directory->getName());
-    cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir);
+    cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir,
+                       !PP.getHeaderSearchInfo()
+                            .getHeaderSearchOpts()
+                            .NoAbsoluteModuleDirectory);
 
     // If the home of the module is the current working directory, then we
     // want to pick up the cwd of the build process loading the module, not
     // our cwd, when we load this module.
     if (!PP.getHeaderSearchInfo()
              .getHeaderSearchOpts()
              .ModuleMapFileHomeIsCwd ||
         WritingModule->Directory->getName() != StringRef(".")) {
#include "clang/Basic/Sanitizers.def"
   Stream.ExitBlock();
 
   // Original file name and file ID
   SourceManager &SM = Context.getSourceManager();
   if (const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID())) {
     auto FileAbbrev = std::make_shared<BitCodeAbbrev>();
     FileAbbrev->Add(BitCodeAbbrevOp(ORIGINAL_FILE));
     FileAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // File ID
+    FileAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Relative
     FileAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // File name
     unsigned FileAbbrevCode = Stream.EmitAbbrev(std::move(FileAbbrev));
 
     Record.clear();
     Record.push_back(ORIGINAL_FILE);
     Record.push_back(SM.getMainFileID().getOpaqueValue());
     EmitRecordWithPath(FileAbbrevCode, Record, MainFile->getName());
   }
void ASTWriter::WriteInputFiles(SourceManager &SourceMgr,
   auto IFAbbrev = std::make_shared<BitCodeAbbrev>();
   IFAbbrev->Add(BitCodeAbbrevOp(INPUT_FILE));
   IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // ID
   IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 12)); // Size
   IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 32)); // Modification time
   IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Overridden
   IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Transient
   IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Module map
+  IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Relative
   IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // File name
   unsigned IFAbbrevCode = Stream.EmitAbbrev(std::move(IFAbbrev));
 
   // Get all ContentCache objects for files, sorted by whether the file is a
   // system one or not. System files go at the back, users files at the front.
   std::deque<InputFileEntry> SortedFiles;
   for (unsigned I = 1, N = SourceMgr.local_sloc_entry_size(); I != N; ++I) {
     // Get this source location entry.
for (const auto &Entry : SortedFiles) {
 
     InputFileID = InputFileOffsets.size();
 
     if (!Entry.IsSystemFile)
       ++UserFilesNum;
 
     // Emit size/modification time for this file.
     // And whether this file was overridden.
-    RecordData::value_type Record[] = {
+    RecordData Record = {
         INPUT_FILE,
         InputFileOffsets.size(),
         (uint64_t)Entry.File->getSize(),
         (uint64_t)getTimestampForOutput(Entry.File),
         Entry.BufferOverridden,
         Entry.IsTransient,
         Entry.IsTopLevelModuleMap};
 
namespace {
 
   public:
     HeaderFileInfoTrait(ASTWriter &Writer) : Writer(Writer) {}
 
     struct key_type {
       StringRef Filename;
       off_t Size;
       time_t ModTime;
+      bool IsRelativeModuleDirectory;
     };
     using key_type_ref = const key_type &;
 
     using UnresolvedModule =
         llvm::PointerIntPair<Module *, 2, ModuleMap::ModuleHeaderRole>;
 
     struct data_type {
       const HeaderFileInfo &HFI;
hash_value_type ComputeHash(key_type_ref key) {
       return llvm::hash_combine(key.Size, key.ModTime);
     }
 
     std::pair<unsigned, unsigned>
     EmitKeyDataLength(raw_ostream& Out, key_type_ref key, data_type_ref Data) {
       using namespace llvm::support;
 
       endian::Writer LE(Out, little);
-      unsigned KeyLen = key.Filename.size() + 1 + 8 + 8;
+      unsigned KeyLen = key.Filename.size() + 1 + 8 + 8 + 1;
       LE.write<uint16_t>(KeyLen);
       unsigned DataLen = 1 + 2 + 4 + 4;
       for (auto ModInfo : Data.KnownHeaders)
         if (Writer.getLocalOrImportedSubmoduleID(ModInfo.getModule()))
           DataLen += 4;
       if (Data.Unresolved.getPointer())
         DataLen += 4;
       LE.write<uint8_t>(DataLen);
       return std::make_pair(KeyLen, DataLen);
     }
 
     void EmitKey(raw_ostream& Out, key_type_ref key, unsigned KeyLen) {
       using namespace llvm::support;
 
       endian::Writer LE(Out, little);
       LE.write<uint64_t>(key.Size);
       KeyLen -= 8;
       LE.write<uint64_t>(key.ModTime);
       KeyLen -= 8;
+      LE.write<uint8_t>(key.IsRelativeModuleDirectory);
+      KeyLen -= 1;
       Out.write(key.Filename.data(), KeyLen);
     }
 
     void EmitData(raw_ostream &Out, key_type_ref key,
                   data_type_ref Data, unsigned DataLen) {
       using namespace llvm::support;
 
       endian::Writer LE(Out, little);
while (!Worklist.empty()) {
             << WritingModule->getFullModuleName() << U.Size.hasValue()
             << U.FileName;
           continue;
         }
 
         // Form the effective relative pathname for the file.
         SmallString<128> Filename(M->Directory->getName());
         llvm::sys::path::append(Filename, U.FileName);
-        PreparePathForOutput(Filename);
+        bool IsRelativeModuleDirectory = false;
+        PreparePathForOutput(Filename, IsRelativeModuleDirectory);
 
         StringRef FilenameDup = strdup(Filename.c_str());
         SavedStrings.push_back(FilenameDup.data());
 
         HeaderFileInfoTrait::key_type Key = {
-          FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0
+          FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0,
+          IsRelativeModuleDirectory,
         };
         HeaderFileInfoTrait::data_type Data = {
           Empty, {}, {M, ModuleMap::headerKindToRole(U.Kind)}
         };
         // FIXME: Deal with cases where there are multiple unresolved header
         // directives in different submodules for the same header.
         Generator.insert(Key, Data, GeneratorTrait);
         ++NumHeaderSearchEntries;
for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
     const HeaderFileInfo *HFI =
         HS.getExistingFileInfo(File, /*WantExternal*/!Chain);
     if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
       continue;
 
     // Massage the file path into an appropriate form.
     StringRef Filename = File->getName();
     SmallString<128> FilenameTmp(Filename);
-    if (PreparePathForOutput(FilenameTmp)) {
+    bool IsRelativeModuleDirectory = false;
+    if (PreparePathForOutput(FilenameTmp, IsRelativeModuleDirectory)) {
       // If we performed any translation on the file name at all, we need to
       // save this string, since the generator will refer to it later.
       Filename = StringRef(strdup(FilenameTmp.c_str()));
       SavedStrings.push_back(Filename.data());
     }
 
     HeaderFileInfoTrait::key_type Key = {
-      Filename, File->getSize(), getTimestampForOutput(File)
+      Filename, File->getSize(), getTimestampForOutput(File),
+      IsRelativeModuleDirectory,
     };
     HeaderFileInfoTrait::data_type Data = {
       *HFI, HS.getModuleMap().findAllModulesForHeader(File), {}
     };
     Generator.insert(Key, Data, GeneratorTrait);
     ++NumHeaderSearchEntries;
   }
 
void ASTWriter::AddToken(const Token &Tok, RecordDataImpl &Record) {
   Record.push_back(Tok.getFlags());
 }
 
 void ASTWriter::AddString(StringRef Str, RecordDataImpl &Record) {
   Record.push_back(Str.size());
   Record.insert(Record.end(), Str.begin(), Str.end());
 }
 
-bool ASTWriter::PreparePathForOutput(SmallVectorImpl<char> &Path) {
+bool ASTWriter::PreparePathForOutput(SmallVectorImpl<char> &Path,
+                                     bool &IsRelativeModuleDirectory) {
   assert(Context && "should have context when outputting path");
 
   bool Changed =
-      cleanPathForOutput(Context->getSourceManager().getFileManager(), Path);
+      cleanPathForOutput(Context->getSourceManager().getFileManager(), Path,
+                         llvm::sys::path::is_absolute(BaseDirectory));
 
   // Remove a prefix to make the path relative, if relevant.
   const char *PathBegin = Path.data();
   const char *PathPtr =
       adjustFilenameForRelocatableAST(PathBegin, BaseDirectory);
   if (PathPtr != PathBegin) {
     Path.erase(Path.begin(), Path.begin() + (PathPtr - PathBegin));
     Changed = true;
+    IsRelativeModuleDirectory = true;
+  } else {
+    IsRelativeModuleDirectory = false;
   }
 
   return Changed;
 }
 
 void ASTWriter::AddPath(StringRef Path, RecordDataImpl &Record) {
   SmallString<128> FilePath(Path);
-  PreparePathForOutput(FilePath);
+  bool IsRelativeModuleDirectory = false;
+  PreparePathForOutput(FilePath, IsRelativeModuleDirectory);
   AddString(FilePath, Record);
+  Record.push_back(IsRelativeModuleDirectory);
 }
 
-void ASTWriter::EmitRecordWithPath(unsigned Abbrev, RecordDataRef Record,
+void ASTWriter::EmitRecordWithPath(unsigned Abbrev, RecordDataImpl &Record,
                                    StringRef Path) {
   SmallString<128> FilePath(Path);
-  PreparePathForOutput(FilePath);
+  bool IsRelativeModuleDirectory = false;
+  PreparePathForOutput(FilePath, IsRelativeModuleDirectory);
+  Record.push_back(IsRelativeModuleDirectory);
   Stream.EmitRecordWithBlob(Abbrev, Record, FilePath);
 }
 
 void ASTWriter::AddVersionTuple(const VersionTuple &Version,
                                 RecordDataImpl &Record) {
   Record.push_back(Version.getMajor());
   if (Optional<unsigned> Minor = Version.getMinor())
     Record.push_back(*Minor + 1);

lib/Serialization/GlobalModuleIndex.cpp

if (State == ControlBlock && Code == IMPORTS) {
         // modules while here we are only dealing with cached).
         Idx += Record[Idx] + 1;
 
         // Retrieve the imported file name.
         unsigned Length = Record[Idx++];
         SmallString<128> ImportedFile(Record.begin() + Idx,
                                       Record.begin() + Idx + Length);
         Idx += Length;
+        Idx++;  // Relative
 
         // Find the imported module file.
         const FileEntry *DependsOnFile
           = FileMgr.getFile(ImportedFile, /*openFile=*/false,
                             /*cacheFailure=*/false);
 
         if (!DependsOnFile)
           return true;

test/Modules/relocatable-modules.modulemap

+// Build two otherwise identical modules in two different directories and
+// verify that using `-fno-absolute-module-directory` makes them identical.
+//
+// RUN: rm -rf %t
+//
+// RUN: mkdir -p %t/p1
+// RUN: cd %t/p1
+// RUN: mkdir -p main other
+// RUN: grep "<AM>" %s > main/a.modulemap
+// RUN: grep "<AH>" %s > main/a.h
+// RUN: grep "<BH>" %s > other/b.h
+// RUN: %clang_cc1 -x c++ -fmodules -emit-module -fno-absolute-module-directory \
+// RUN:   -fmodule-name="a" -Imain -I. -o - main/a.modulemap > a.pcm
+//
+// RUN: mkdir -p %t/p2
+// RUN: cd %t/p2
+// RUN: mkdir -p main other
+// RUN: grep "<AM>" %s > main/a.modulemap
+// RUN: grep "<AH>" %s > main/a.h
+// RUN: grep "<BH>" %s > other/b.h
+// RUN: %clang_cc1 -x c++ -fmodules -emit-module -fno-absolute-module-directory \
+// RUN:   -fmodule-name="a" -Imain -I. -o - main/a.modulemap > a.pcm
+//
+// RUN: diff %t/p1/a.pcm %t/p2/a.pcm
+
+module "a" {                // <AM>
+}                           // <AM>
+
+#include "b.h"              // <AH>