r269502 - [ModuleMap][CrashReproducer] Collect headers from inner frameworks

Bruno Cardoso Lopes via cfe-commits cfe-commits at lists.llvm.org
Fri May 13 15:21:51 PDT 2016


Author: bruno
Date: Fri May 13 17:21:51 2016
New Revision: 269502

URL: http://llvm.org/viewvc/llvm-project?rev=269502&view=rev
Log:
[ModuleMap][CrashReproducer] Collect headers from inner frameworks

(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.

Differential Revision: http://reviews.llvm.org/D20194

rdar://problem/25880368

Added:
    cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/
    cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/A.framework/
    cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/A.framework/Headers/
    cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/A.framework/Headers/A.h
    cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/B.framework/
    cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Headers/
    cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Headers/B.h
    cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Modules/
    cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Modules/module.modulemap
    cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/I.framework/
    cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Headers/
    cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Headers/I.h
    cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Modules/
    cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Modules/module.modulemap
    cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/module.modulemap
    cfe/trunk/test/Modules/crash-vfs-umbrella-frameworks.m
Modified:
    cfe/trunk/include/clang/Lex/ModuleMap.h
    cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp
    cfe/trunk/lib/Lex/ModuleMap.cpp

Modified: cfe/trunk/include/clang/Lex/ModuleMap.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=269502&r1=269501&r2=269502&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/ModuleMap.h (original)
+++ cfe/trunk/include/clang/Lex/ModuleMap.h Fri May 13 17:21:51 2016
@@ -55,6 +55,13 @@ public:
   ///
   /// \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 {

Modified: cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp?rev=269502&r1=269501&r2=269502&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp (original)
+++ cfe/trunk/lib/Frontend/ModuleDependencyCollector.cpp Fri May 13 17:21:51 2016
@@ -48,6 +48,34 @@ struct ModuleDependencyMMCallbacks : pub
     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);
+    }
+  }
 };
 
 }

Modified: cfe/trunk/lib/Lex/ModuleMap.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=269502&r1=269501&r2=269502&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/ModuleMap.cpp (original)
+++ cfe/trunk/lib/Lex/ModuleMap.cpp Fri May 13 17:21:51 2016
@@ -760,6 +760,10 @@ void ModuleMap::setUmbrellaHeader(Module
   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,

Added: cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/A.framework/Headers/A.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/A.framework/Headers/A.h?rev=269502&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/A.framework/Headers/A.h (added)
+++ cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/A.framework/Headers/A.h Fri May 13 17:21:51 2016
@@ -0,0 +1 @@
+#include <B/B.h>

Added: cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Headers/B.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Headers/B.h?rev=269502&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Headers/B.h (added)
+++ cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Headers/B.h Fri May 13 17:21:51 2016
@@ -0,0 +1 @@
+// B.h

Added: cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Modules/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Modules/module.modulemap?rev=269502&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Modules/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/B.framework/Modules/module.modulemap Fri May 13 17:21:51 2016
@@ -0,0 +1,5 @@
+framework module B [extern_c] {
+  umbrella header "B.h"
+  export *
+  module * { export * }
+}

Added: cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Headers/I.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Headers/I.h?rev=269502&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Headers/I.h (added)
+++ cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Headers/I.h Fri May 13 17:21:51 2016
@@ -0,0 +1,2 @@
+
+#import <A/A.h>

Added: cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Modules/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Modules/module.modulemap?rev=269502&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Modules/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/I.framework/Modules/module.modulemap Fri May 13 17:21:51 2016
@@ -0,0 +1,5 @@
+framework module I [extern_c] {
+  umbrella header "I.h"
+  export *
+  module * { export * }
+}

Added: cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/module.modulemap?rev=269502&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/crash-recovery/Frameworks/module.modulemap Fri May 13 17:21:51 2016
@@ -0,0 +1,2 @@
+framework module * [extern_c] {
+}

Added: cfe/trunk/test/Modules/crash-vfs-umbrella-frameworks.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/crash-vfs-umbrella-frameworks.m?rev=269502&view=auto
==============================================================================
--- cfe/trunk/test/Modules/crash-vfs-umbrella-frameworks.m (added)
+++ cfe/trunk/test/Modules/crash-vfs-umbrella-frameworks.m Fri May 13 17:21:51 2016
@@ -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"
+
+ at import I;




More information about the cfe-commits mailing list