Index: include/clang/Lex/ModuleMap.h =================================================================== --- include/clang/Lex/ModuleMap.h +++ include/clang/Lex/ModuleMap.h @@ -55,6 +55,13 @@ /// /// \param Filename The header file itself. virtual void moduleMapAddHeader(StringRef Filename) {} + + /// \brief Called when an umbrella header is added during module map parsing. + /// + /// \param FileMgr FileManager instance + /// \param Filename The umbreall header to collect. + virtual void moduleMapAddUmbrellaHeader(FileManager *FileMgr, + const FileEntry *Header) {} }; class ModuleMap { Index: lib/Frontend/ModuleDependencyCollector.cpp =================================================================== --- lib/Frontend/ModuleDependencyCollector.cpp +++ lib/Frontend/ModuleDependencyCollector.cpp @@ -48,6 +48,34 @@ if (llvm::sys::path::is_absolute(HeaderPath)) Collector.addFile(HeaderPath); } + void moduleMapAddUmbrellaHeader(FileManager *FileMgr, + const FileEntry *Header) override { + StringRef HeaderFilename = Header->getName(); + 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 = Header->getDir()->getName(); + if (!UmbrellaDir.equals(UmbreallDirFromHeader)) { + SmallString<128> AltHeaderFilename; + llvm::sys::path::append(AltHeaderFilename, UmbrellaDir, + llvm::sys::path::filename(HeaderFilename)); + if (FileMgr->getFile(AltHeaderFilename)) + moduleMapAddHeader(AltHeaderFilename); + } + } }; } Index: lib/Lex/ModuleMap.cpp =================================================================== --- lib/Lex/ModuleMap.cpp +++ lib/Lex/ModuleMap.cpp @@ -760,6 +760,10 @@ 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) + Cb->moduleMapAddUmbrellaHeader(&SourceMgr.getFileManager(), UmbrellaHeader); } void ModuleMap::setUmbrellaDir(Module *Mod, const DirectoryEntry *UmbrellaDir, 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 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/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/I.framework/Headers/I.h =================================================================== --- /dev/null +++ test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Headers/I.h @@ -0,0 +1,2 @@ + +#import 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/module.modulemap =================================================================== --- /dev/null +++ test/Modules/Inputs/crash-recovery/Frameworks/module.modulemap @@ -0,0 +1,2 @@ +framework module * [extern_c] { +} 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;