benlangmuir created this revision.
benlangmuir added reviewers: jansvoboda11, Bigcheese.
Herald added a project: All.
benlangmuir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Use a FileEntryRef when retrieving modulemap paths in the scanner so that we
use a path compatible with the original module import, rather than a FileEntry
which can allow unrelated modules to leak paths into how we build a module due
to FileManager mutating the path.
Note: the current change prevents an "unrelated" path, but does not change how
VFS mapped paths are handled (which would be calling getNameAsRequested) nor
canonicalize the path. In that sense, this is a narrower version of
https://reviews.llvm.org/D135636 that should not be blocked by the VFS concerns.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D137989
Files:
clang/include/clang/Serialization/ASTReader.h
clang/lib/Frontend/FrontendAction.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/modules-symlink-dir-from-module.c
Index: clang/test/ClangScanDeps/modules-symlink-dir-from-module.c
===================================================================
--- /dev/null
+++ clang/test/ClangScanDeps/modules-symlink-dir-from-module.c
@@ -0,0 +1,94 @@
+// Check that the path of an imported modulemap file is not influenced by
+// modules outside that module's dependency graph. Specifically, the "Foo"
+// module below does not transitively import Mod via a symlink, so it should not
+// see the symlinked path.
+
+// REQUIRES: shell
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.in > %t/cdb.json
+// RUN: ln -s module %t/include/symlink-to-module
+
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -j 1 \
+// RUN: -format experimental-full -mode=preprocess-dependency-directives \
+// RUN: -optimize-args -module-files-dir %t/build > %t/deps.json
+
+// RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
+
+// CHECK: "modules": [
+// CHECK: {
+// CHECK: "command-line": [
+// CHECK: "-fmodule-map-file=[[PREFIX]]/include/module/module.modulemap"
+// CHECK: ]
+// CHECK: "name": "Foo"
+// CHECK: }
+// CHECK: {
+// CHECK: "command-line": [
+// FIXME: canonicalize this path
+// CHECK: "-fmodule-map-file=[[PREFIX]]/include/symlink-to-module/module.modulemap"
+// CHECK: ]
+// CHECK: "name": "Foo_Private"
+// CHECK: }
+// CHECK: {
+// CHECK: "command-line": [
+// CHECK: "[[PREFIX]]/include/module/module.modulemap"
+// CHECK: ]
+// CHECK: "name": "Mod"
+// CHECK: }
+// CHECK: {
+// CHECK: "command-line": [
+// FIXME: canonicalize this path
+// CHECK: "-fmodule-map-file=[[PREFIX]]/include/symlink-to-module/module.modulemap"
+// CHECK: ]
+// CHECK: "name": "Other"
+// CHECK: }
+// CHECK: {
+// CHECK: "command-line": [
+// FIXME: canonicalize this path
+// CHECK: "-fmodule-map-file=[[PREFIX]]/include/symlink-to-module/module.modulemap"
+// CHECK: ]
+// CHECK: "name": "Test"
+// CHECK: }
+// CHECK: ]
+
+//--- cdb.json.in
+[{
+ "directory": "DIR",
+ "command": "clang -fsyntax-only DIR/test.c -F DIR/Frameworks -I DIR/include -fmodules -fimplicit-module-maps -fmodules-cache-path=DIR/module-cache",
+ "file": "DIR/test.c"
+}]
+
+//--- include/module/module.modulemap
+module Mod { header "mod.h" export * }
+
+//--- include/module/mod.h
+
+//--- include/module.modulemap
+module Other { header "other.h" export * }
+
+//--- include/other.h
+#include "symlink-to-module/mod.h"
+#include "module/mod.h"
+
+//--- Frameworks/Foo.framework/Modules/module.modulemap
+framework module Foo { header "Foo.h" export * }
+//--- Frameworks/Foo.framework/Modules/module.private.modulemap
+framework module Foo_Private { header "Priv.h" export * }
+
+//--- Frameworks/Foo.framework/Headers/Foo.h
+#include "module/mod.h"
+
+//--- Frameworks/Foo.framework/PrivateHeaders/Priv.h
+#include <Foo/Foo.h>
+#include "other.h"
+
+//--- module.modulemap
+module Test { header "test.h" export * }
+
+//--- test.h
+#include <Foo/Priv.h>
+#include <Foo/Foo.h>
+
+//--- test.c
+#include "test.h"
Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -447,10 +447,10 @@
addAllAffectingClangModules(M, MD, SeenDeps);
MDC.ScanInstance.getASTReader()->visitTopLevelModuleMaps(
- *MF, [&](const FileEntry *FE) {
- if (FE->getName().endswith("__inferred_module.map"))
+ *MF, [&](FileEntryRef FE) {
+ if (FE.getName().endswith("__inferred_module.map"))
return;
- MD.ModuleMapFileDeps.emplace_back(FE->getName());
+ MD.ModuleMapFileDeps.emplace_back(FE.getName());
});
CompilerInvocation CI = MDC.makeInvocationForModuleBuildWithoutOutputs(
Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1486,12 +1486,14 @@
/// An input file.
struct InputFileEntry {
- const FileEntry *File;
+ FileEntryRef File;
bool IsSystemFile;
bool IsTransient;
bool BufferOverridden;
bool IsTopLevelModuleMap;
uint32_t ContentHash[2];
+
+ InputFileEntry(FileEntryRef File) : File(File) {}
};
} // namespace
@@ -1541,8 +1543,7 @@
if (!IsSLocAffecting[I])
continue;
- InputFileEntry Entry;
- Entry.File = Cache->OrigEntry;
+ InputFileEntry Entry(*Cache->OrigEntry);
Entry.IsSystemFile = isSystem(File.getFileCharacteristic());
Entry.IsTransient = Cache->IsTransient;
Entry.BufferOverridden = Cache->BufferOverridden;
@@ -1559,7 +1560,7 @@
else
// FIXME: The path should be taken from the FileEntryRef.
PP->Diag(SourceLocation(), diag::err_module_unable_to_hash_content)
- << Entry.File->getName();
+ << Entry.File.getName();
}
auto CH = llvm::APInt(64, ContentHash);
Entry.ContentHash[0] =
@@ -1599,14 +1600,13 @@
RecordData::value_type Record[] = {
INPUT_FILE,
InputFileOffsets.size(),
- (uint64_t)Entry.File->getSize(),
+ (uint64_t)Entry.File.getSize(),
(uint64_t)getTimestampForOutput(Entry.File),
Entry.BufferOverridden,
Entry.IsTransient,
Entry.IsTopLevelModuleMap};
- // FIXME: The path should be taken from the FileEntryRef.
- EmitRecordWithPath(IFAbbrevCode, Record, Entry.File->getName());
+ EmitRecordWithPath(IFAbbrevCode, Record, Entry.File.getName());
}
// Emit content hash for this file.
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -9164,14 +9164,14 @@
void ASTReader::visitTopLevelModuleMaps(
serialization::ModuleFile &MF,
- llvm::function_ref<void(const FileEntry *FE)> Visitor) {
+ llvm::function_ref<void(FileEntryRef FE)> Visitor) {
unsigned NumInputs = MF.InputFilesLoaded.size();
for (unsigned I = 0; I < NumInputs; ++I) {
InputFileInfo IFI = readInputFileInfo(MF, I + 1);
if (IFI.TopLevelModuleMap)
// FIXME: This unnecessarily re-reads the InputFileInfo.
if (auto FE = getInputFile(MF, I + 1).getFile())
- Visitor(FE);
+ Visitor(*FE);
}
}
Index: clang/lib/Frontend/FrontendAction.cpp
===================================================================
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -639,9 +639,9 @@
CI.getFrontendOpts().ModuleFiles.push_back(MF.FileName);
ASTReader->visitTopLevelModuleMaps(
- PrimaryModule, [&](const FileEntry *FE) {
+ PrimaryModule, [&](FileEntryRef FE) {
CI.getFrontendOpts().ModuleMapFiles.push_back(
- std::string(FE->getName()));
+ std::string(FE.getName()));
});
}
Index: clang/include/clang/Serialization/ASTReader.h
===================================================================
--- clang/include/clang/Serialization/ASTReader.h
+++ clang/include/clang/Serialization/ASTReader.h
@@ -2338,8 +2338,7 @@
/// Visit all the top-level module maps loaded when building the given module
/// file.
void visitTopLevelModuleMaps(serialization::ModuleFile &MF,
- llvm::function_ref<
- void(const FileEntry *)> Visitor);
+ llvm::function_ref<void(FileEntryRef)> Visitor);
bool isProcessingUpdateRecords() { return ProcessingUpdateRecords; }
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
