andrewjcg updated this revision to Diff 166709.
andrewjcg added a comment.
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.
Repository:
rC Clang
https://reviews.llvm.org/D51568
Files:
include/clang/Driver/CC1Options.td
include/clang/Lex/HeaderSearchOptions.h
include/clang/Serialization/ASTReader.h
include/clang/Serialization/ASTWriter.h
lib/Frontend/CompilerInvocation.cpp
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTReaderInternals.h
lib/Serialization/ASTWriter.cpp
lib/Serialization/GlobalModuleIndex.cpp
test/Modules/relocatable-modules.modulemap
Index: test/Modules/relocatable-modules.modulemap
===================================================================
--- /dev/null
+++ test/Modules/relocatable-modules.modulemap
@@ -0,0 +1,29 @@
+// 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>
Index: lib/Serialization/GlobalModuleIndex.cpp
===================================================================
--- lib/Serialization/GlobalModuleIndex.cpp
+++ lib/Serialization/GlobalModuleIndex.cpp
@@ -628,6 +628,7 @@
SmallString<128> ImportedFile(Record.begin() + Idx,
Record.begin() + Idx + Length);
Idx += Length;
+ Idx++; // Relative
// Find the imported module file.
const FileEntry *DependsOnFile
Index: lib/Serialization/ASTWriter.cpp
===================================================================
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -1327,9 +1327,14 @@
///
/// \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
@@ -1493,7 +1498,10 @@
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
@@ -1708,6 +1716,7 @@
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));
@@ -1772,6 +1781,7 @@
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));
@@ -1822,7 +1832,7 @@
// 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(),
@@ -1937,6 +1947,7 @@
StringRef Filename;
off_t Size;
time_t ModTime;
+ bool IsRelativeModuleDirectory;
};
using key_type_ref = const key_type &;
@@ -1965,7 +1976,7 @@
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)
@@ -1985,6 +1996,8 @@
KeyLen -= 8;
LE.write<uint64_t>(key.ModTime);
KeyLen -= 8;
+ LE.write<uint8_t>(key.IsRelativeModuleDirectory);
+ KeyLen -= 1;
Out.write(key.Filename.data(), KeyLen);
}
@@ -2091,13 +2104,15 @@
// 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)}
@@ -2137,15 +2152,17 @@
// 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), {}
@@ -4504,34 +4521,43 @@
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);
}
Index: lib/Serialization/ASTReaderInternals.h
===================================================================
--- lib/Serialization/ASTReaderInternals.h
+++ lib/Serialization/ASTReaderInternals.h
@@ -254,6 +254,7 @@
struct internal_key_type {
off_t Size;
time_t ModTime;
+ bool IsRelativeModuleDirectory;
StringRef Filename;
bool Imported;
};
Index: lib/Serialization/ASTReader.cpp
===================================================================
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -1702,6 +1702,7 @@
HeaderFileInfoTrait::GetInternalKey(const FileEntry *FE) {
internal_key_type ikey = {FE->getSize(),
M.HasTimestamps ? FE->getModificationTime() : 0,
+ /*IsRelativeModuleDirectory*/ false,
FE->getName(), /*Imported*/ false};
return ikey;
}
@@ -1720,7 +1721,8 @@
return FileMgr.getFile(Key.Filename);
std::string Resolved = Key.Filename;
- Reader.ResolveImportedPath(M, Resolved);
+ if (Key.IsRelativeModuleDirectory)
+ Reader.ResolveImportedPath(M, Resolved);
return FileMgr.getFile(Resolved);
};
@@ -1745,6 +1747,7 @@
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;
@@ -1793,7 +1796,7 @@
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
@@ -2088,8 +2091,11 @@
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;
- ResolveImportedPath(F, R.Filename);
+ if (IsRelativeModuleDirectory) {
+ ResolveImportedPath(F, R.Filename);
+ }
return R;
}
@@ -2594,12 +2600,15 @@
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;
- ResolveImportedPath(F, F.OriginalSourceFileName);
+ if (IsRelativeModuleDirectory)
+ ResolveImportedPath(F, F.OriginalSourceFileName);
break;
+ }
case ORIGINAL_FILE_ID:
F.OriginalSourceFileID = FileID::get(Record[0]);
@@ -4842,8 +4851,10 @@
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;
- ResolveImportedPath(Filename, ModuleDir);
+ if (IsRelativeModuleDirectory)
+ ResolveImportedPath(Filename, ModuleDir);
shouldContinue = Listener.visitInputFile(
Filename, isSystemFile, Overridden, /*IsExplicitModule*/false);
break;
@@ -9086,7 +9097,9 @@
std::string ASTReader::ReadPath(ModuleFile &F, const RecordData &Record,
unsigned &Idx) {
std::string Filename = ReadString(Record, Idx);
- ResolveImportedPath(F, Filename);
+ bool IsRelativeModuleDirectory = Record[Idx++];
+ if (IsRelativeModuleDirectory)
+ ResolveImportedPath(F, Filename);
return Filename;
}
Index: lib/Frontend/CompilerInvocation.cpp
===================================================================
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1770,6 +1770,7 @@
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);
Index: include/clang/Serialization/ASTWriter.h
===================================================================
--- include/clang/Serialization/ASTWriter.h
+++ include/clang/Serialization/ASTWriter.h
@@ -640,13 +640,14 @@
/// 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
Index: include/clang/Serialization/ASTReader.h
===================================================================
--- include/clang/Serialization/ASTReader.h
+++ include/clang/Serialization/ASTReader.h
@@ -2238,6 +2238,7 @@
// Skip a path
static void SkipPath(const RecordData &Record, unsigned &Idx) {
SkipString(Record, Idx);
+ Idx++; // Relative
}
/// Read a version tuple.
Index: include/clang/Lex/HeaderSearchOptions.h
===================================================================
--- include/clang/Lex/HeaderSearchOptions.h
+++ include/clang/Lex/HeaderSearchOptions.h
@@ -203,14 +203,18 @@
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,
Index: include/clang/Driver/CC1Options.td
===================================================================
--- include/clang/Driver/CC1Options.td
+++ include/clang/Driver/CC1Options.td
@@ -751,6 +751,8 @@
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">,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
