From 935a07ed21434825a96eb6d3acd2163edd0abe88 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 1 Dec 2022 19:09:09 -0800 Subject: [PATCH] [clang][deps][lex] Avoid canonicalization of remapped framework directories In D134923, the scanner introduced canonicalization of framework directories when reporting module map paths in order to increase module sharing. However, if we canonicalize framework directory that plays a role in a VFS remapping, and later try to use that module map to build the module, header lookup can fail. This happens when the module headers are remapped using the original framework path. This patch fixes that. The implementation relies on the fact that the chain of directories in VFS remapping are assigned `DirectoryEntry` objects distinct from their on-disk counterparts. If we detect that case, we avoid the canonicalization. Reviewed By: benlangmuir Differential Revision: https://reviews.llvm.org/D135841 --- clang/lib/Lex/ModuleMap.cpp | 13 +++- clang/test/ClangScanDeps/modules-symlink-dir-vfs.c | 90 ++++++++++++++++++++++ 2 files changed, 100 insertions(+), 3 deletions(-) create mode 100644 clang/test/ClangScanDeps/modules-symlink-dir-vfs.c diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp index 9ff0204..4e86c5c 100644 --- a/clang/lib/Lex/ModuleMap.cpp +++ b/clang/lib/Lex/ModuleMap.cpp @@ -1303,9 +1303,16 @@ ModuleMap::canonicalizeModuleMapPath(SmallVectorImpl &Path) { // Canonicalize the directory. StringRef CanonicalDir = FM.getCanonicalName(*DirEntry); if (CanonicalDir != Dir) { - bool Done = llvm::sys::path::replace_path_prefix(Path, Dir, CanonicalDir); - (void)Done; - assert(Done && "Path should always start with Dir"); + auto CanonicalDirEntry = FM.getDirectory(CanonicalDir); + // Only use the canonicalized path if it resolves to the same entry as the + // original. This is not true if there's a VFS overlay on top of a FS where + // the directory is a symlink. The overlay would not remap the target path + // of the symlink to the same directory entry in that case. + if (CanonicalDirEntry && *CanonicalDirEntry == *DirEntry) { + bool Done = llvm::sys::path::replace_path_prefix(Path, Dir, CanonicalDir); + (void)Done; + assert(Done && "Path should always start with Dir"); + } } // In theory, the filename component should also be canonicalized if it diff --git a/clang/test/ClangScanDeps/modules-symlink-dir-vfs.c b/clang/test/ClangScanDeps/modules-symlink-dir-vfs.c new file mode 100644 index 0000000..bc384b0 --- /dev/null +++ b/clang/test/ClangScanDeps/modules-symlink-dir-vfs.c @@ -0,0 +1,90 @@ +// This test checks that we're not canonicalizing framework directories that +// play a role in VFS remapping. This could lead header search to fail when +// building that module. + +// RUN: rm -rf %t +// RUN: split-file %s %t + +// REQUIRES: shell + +// RUN: mkdir -p %t/frameworks-symlink +// RUN: ln -s %t/frameworks/FW.framework %t/frameworks-symlink/FW.framework + +// RUN: mkdir -p %t/copy +// RUN: cp %t/frameworks/FW.framework/Headers/FW.h %t/copy +// RUN: cp %t/frameworks/FW.framework/Headers/Header.h %t/copy + +//--- frameworks/FW.framework/Modules/module.modulemap +framework module FW { umbrella header "FW.h" } +//--- frameworks/FW.framework/Headers/FW.h +#import +//--- frameworks/FW.framework/Headers/Header.h +// empty + +//--- tu.m +@import FW; + +//--- overlay.json.template +{ + "version": 0, + "case-sensitive": "false", + "roots": [ + { + "contents": [ + { + "external-contents": "DIR/copy/Header.h", + "name": "Header.h", + "type": "file" + }, + { + "external-contents": "DIR/copy/FW.h", + "name": "FW.h", + "type": "file" + } + ], + "name": "DIR/frameworks-symlink/FW.framework/Headers", + "type": "directory" + } + ] +} + +//--- cdb.json.template +[{ + "directory": "DIR", + "file": "DIR/tu.m", + "command": "clang -fmodules -fmodules-cache-path=DIR/cache -ivfsoverlay DIR/overlay.json -F DIR/frameworks-symlink -c DIR/tu.m -o DIR/tu.o -Werror=non-modular-include-in-framework-module" +}] + +// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json +// RUN: sed -e "s|DIR|%/t|g" %t/overlay.json.template > %t/overlay.json + +// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/result.json +// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t + +// CHECK: { +// CHECK-NEXT: "modules": [ +// CHECK-NEXT: { +// CHECK-NEXT: "clang-module-deps": [], +// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/frameworks-symlink/FW.framework/Modules/module.modulemap", +// CHECK-NEXT: "command-line": [ +// CHECK-NEXT: "-cc1", +// CHECK: "-emit-module", +// CHECK-NEXT: "-x", +// CHECK-NEXT: "objective-c", +// CHECK-NEXT: "[[PREFIX]]/frameworks-symlink/FW.framework/Modules/module.modulemap", +// CHECK: ], +// CHECK-NEXT: "context-hash": "{{.*}}", +// CHECK-NEXT: "file-deps": [ +// CHECK-NEXT: "[[PREFIX]]/copy/FW.h", +// CHECK-NEXT: "[[PREFIX]]/copy/Header.h", +// CHECK-NEXT: "[[PREFIX]]/frameworks-symlink/FW.framework/Modules/module.modulemap" +// CHECK-NEXT: ], +// CHECK-NEXT: "name": "FW" +// CHECK-NEXT: } +// CHECK-NEXT: ], +// CHECK-NEXT: "translation-units": [ +// CHECK: ] +// CHECK: } + +// RUN: %deps-to-rsp %t/result.json --module-name=FW > %t/FW.cc1.rsp +// RUN: %clang @%t/FW.cc1.rsp -- 2.7.4