bruno created this revision. bruno added a reviewer: benlangmuir. bruno added a subscriber: cfe-commits.
(1) Collect headers under inner frameworks (frameworks inside other other frameworks). (2) Make sure we also collect the right header files inside them. More info on (2): Consider a dummy framework module B, with header Frameworks/B/B.h. Now consider that another framework A, with header Frameworks/A/A.h, has a layout with a inner framework Frameworks/A/Frameworks/B/B.h, where the "B/B.h" part is a symlink for Frameworks/B/B.h. Also assume that Frameworks/A/A.h includes <B/B.h>. When parsing header Frameworks/A/A.h, framework module lookup is performed in search for B, and it happens that "Frameworks/A/Frameworks/B/B.h" path is registered in the module instead of real "Frameworks/B/B.h". This occurs because "Frameworks/A/Frameworks/B/B.h" is scanned first by the FileManager, when looking for inner framework modules under Frameworks/A/Frameworks. This makes Frameworks/A/Frameworks/B/B.h the default cached named inside the FileManager for the B.h file UID. This leads to modules being built without consistent paths to underlying header files. This is usually not a problem in regular compilation flow, but it's an issue when running the crash reproducer. The issue is that clangs collect "Frameworks/A/Frameworks/B/B.h" but not "Frameworks/B/B.h" into the VFS, leading to err_mmap_umbrella_clash. So make sure we also collect the original header. http://reviews.llvm.org/D20194 Files: lib/Lex/ModuleMap.cpp test/Modules/Inputs/crash-recovery/Frameworks/A.framework/Headers/A.h test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Headers/B.h test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Modules/module.modulemap test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Headers/I.h test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Modules/module.modulemap test/Modules/Inputs/crash-recovery/Frameworks/module.modulemap test/Modules/crash-vfs-umbrella-frameworks.m
Index: test/Modules/crash-vfs-umbrella-frameworks.m =================================================================== --- /dev/null +++ test/Modules/crash-vfs-umbrella-frameworks.m @@ -0,0 +1,42 @@ +// REQUIRES: crash-recovery, shell + +// FIXME: This XFAIL is cargo-culted from crash-report.c. Do we need it? +// XFAIL: mingw32 + +// RUN: rm -rf %t +// RUN: mkdir -p %t/i %t/m %t +// RUN: cp -a %S/Inputs/crash-recovery/Frameworks %t/i/ +// RUN: mkdir -p %t/i/Frameworks/A.framework/Frameworks +// RUN: ln -s ../../B.framework %t/i/Frameworks/A.framework/Frameworks/B.framework + +// RUN: not env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \ +// RUN: %clang -nostdinc -fsyntax-only %s \ +// RUN: -F %/t/i/Frameworks -fmodules \ +// RUN: -fmodules-cache-path=%t/m/ 2>&1 | FileCheck %s + +// RUN: FileCheck --check-prefix=CHECKYAML %s -input-file \ +// RUN: %t/crash-vfs-*.cache/vfs/vfs.yaml +// RUN: find %t/crash-vfs-*.cache/vfs | \ +// RUN: grep "B.framework/Headers/B.h" | count 1 + +// CHECK: Preprocessed source(s) and associated run script(s) are located at: +// CHECK-NEXT: note: diagnostic msg: {{.*}}.m +// CHECK-NEXT: note: diagnostic msg: {{.*}}.cache + +// CHECKYAML: 'type': 'directory', +// CHECKYAML: 'name': "/[[PATH:.*]]/i/Frameworks/A.framework/Frameworks/B.framework/Headers", +// CHECKYAML-NEXT: 'contents': [ +// CHECKYAML-NEXT: { +// CHECKYAML-NEXT: 'type': 'file', +// CHECKYAML-NEXT: 'name': "B.h", +// CHECKYAML-NEXT: 'external-contents': "/[[PATH]]/i/Frameworks/B.framework/Headers/B.h" + +// CHECKYAML: 'type': 'directory', +// CHECKYAML: 'name': "/[[PATH]]/i/Frameworks/B.framework/Headers", +// CHECKYAML-NEXT: 'contents': [ +// CHECKYAML-NEXT: { +// CHECKYAML-NEXT: 'type': 'file', +// CHECKYAML-NEXT: 'name': "B.h", +// CHECKYAML-NEXT: 'external-contents': "/[[PATH]]/i/Frameworks/B.framework/Headers/B.h" + +@import I; Index: test/Modules/Inputs/crash-recovery/Frameworks/module.modulemap =================================================================== --- /dev/null +++ test/Modules/Inputs/crash-recovery/Frameworks/module.modulemap @@ -0,0 +1,2 @@ +framework module * [extern_c] { +} Index: test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Modules/module.modulemap =================================================================== --- /dev/null +++ test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Modules/module.modulemap @@ -0,0 +1,5 @@ +framework module I [extern_c] { + umbrella header "I.h" + export * + module * { export * } +} Index: test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Headers/I.h =================================================================== --- /dev/null +++ test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Headers/I.h @@ -0,0 +1,2 @@ + +#import <A/A.h> Index: test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Modules/module.modulemap =================================================================== --- /dev/null +++ test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Modules/module.modulemap @@ -0,0 +1,5 @@ +framework module B [extern_c] { + umbrella header "B.h" + export * + module * { export * } +} Index: test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Headers/B.h =================================================================== --- /dev/null +++ test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Headers/B.h @@ -0,0 +1 @@ +// B.h Index: test/Modules/Inputs/crash-recovery/Frameworks/A.framework/Headers/A.h =================================================================== --- /dev/null +++ test/Modules/Inputs/crash-recovery/Frameworks/A.framework/Headers/A.h @@ -0,0 +1 @@ +#include <B/B.h> Index: lib/Lex/ModuleMap.cpp =================================================================== --- lib/Lex/ModuleMap.cpp +++ lib/Lex/ModuleMap.cpp @@ -760,6 +760,35 @@ Mod->Umbrella = UmbrellaHeader; Mod->UmbrellaAsWritten = NameAsWritten.str(); UmbrellaDirs[UmbrellaHeader->getDir()] = Mod; + + // Notify callbacks that we just added a new header. + for (const auto &Cb : Callbacks) { + StringRef HeaderFilename = UmbrellaHeader->getName(); + Cb->moduleMapAddHeader(HeaderFilename); + // The FileManager can find and cache the symbolic link for a framework + // header before its real path, this means a module can have some of its + // headers to use other paths. Although this is usually not a problem, it's + // inconsistent, and not collecting the original path header leads to + // umbrella clashes while rebuilding modules in the crash reproducer. For + // example: + // ApplicationServices.framework/Frameworks/ImageIO.framework/ImageIO.h + // instead of: + // ImageIO.framework/ImageIO.h + // + // FIXME: this shouldn't be necessary once we have FileName instances + // around instead of FileEntry ones. For now, make sure we collect all + // that we need for the reproducer to work correctly. + StringRef UmbreallDirFromHeader = + llvm::sys::path::parent_path(HeaderFilename); + StringRef UmbrellaDir = UmbrellaHeader->getDir()->getName(); + if (!UmbrellaDir.equals(UmbreallDirFromHeader)) { + SmallString<128> AltHeaderFilename; + llvm::sys::path::append(AltHeaderFilename, UmbrellaDir, + llvm::sys::path::filename(HeaderFilename)); + if (SourceMgr.getFileManager().getFile(AltHeaderFilename)) + Cb->moduleMapAddHeader(AltHeaderFilename); + } + } } void ModuleMap::setUmbrellaDir(Module *Mod, const DirectoryEntry *UmbrellaDir,
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits