From e62cfd7c17e3985f7e273f02de56ee7bcc21578f Mon Sep 17 00:00:00 2001 From: Bruno Cardoso Lopes Date: Wed, 30 Mar 2016 23:54:25 +0000 Subject: [PATCH] [CrashReproducer] Add a module map callback for added headers The current ModuleDependencyCollector has a AST listener to collect header files present in loaded modules, but this isn't enough to collect all headers needed in the crash reproducer. One of the reasons is that the AST writer doesn't write symbolic link header paths in the pcm modules, this makes the listeners on the reader only able to collect the real files. Since the module maps could contain submodules that use headers which are symbolic links, not collecting those forbid the reproducer scripts to regen the modules. For instance: usr/include/module.map: ... module pthread { header "pthread.h" export * module impl { header "pthread_impl.h" export * } } ... usr/include/pthread/pthread_impl.h usr/include/pthread_impl.h -> pthread/pthread_impl.h The AST dump for the module above: blob data = 'pthread_impl.h' blob data = '//usr/include/pthread/pthread_impl.h' Note that we don't have "usr/include/pthread_impl.h" which is requested by the module.map in case we want to reconstruct the module in the reproducer. The reason the original symbolic link path isn't used is because the headers are kept by name and requested through the FileManager, which unique files and returns the real path only. To fix that, add a callback to be invoked everytime a header is added while parsing module maps and hook that up to the module dependecy collector. This callback is only registered when generating the reproducer. Differential Revision: http://reviews.llvm.org/D18585 rdar://problem/24499339 llvm-svn: 264971 --- clang/include/clang/Frontend/Utils.h | 3 +- clang/include/clang/Lex/ModuleMap.h | 5 +++ clang/lib/Frontend/ModuleDependencyCollector.cpp | 21 ++++++++- clang/lib/Lex/ModuleMap.cpp | 4 ++ .../Modules/crash-vfs-path-symlink-topheader.m | 50 ++++++++++++++++++++++ 5 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 clang/test/Modules/crash-vfs-path-symlink-topheader.m diff --git a/clang/include/clang/Frontend/Utils.h b/clang/include/clang/Frontend/Utils.h index 6ee2a15..903d70c 100644 --- a/clang/include/clang/Frontend/Utils.h +++ b/clang/include/clang/Frontend/Utils.h @@ -77,7 +77,7 @@ void DoPrintPreprocessedInput(Preprocessor &PP, raw_ostream* OS, /// interface. class DependencyCollector { public: - void attachToPreprocessor(Preprocessor &PP); + virtual void attachToPreprocessor(Preprocessor &PP); virtual void attachToASTReader(ASTReader &R); llvm::ArrayRef getDependencies() const { return Dependencies; } @@ -136,6 +136,7 @@ public: VFSWriter.addFileMapping(VPath, RPath); } + void attachToPreprocessor(Preprocessor &PP) override; void attachToASTReader(ASTReader &R) override; void writeFileMap(); diff --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h index 522b3f2..d8d374b 100644 --- a/clang/include/clang/Lex/ModuleMap.h +++ b/clang/include/clang/Lex/ModuleMap.h @@ -50,6 +50,11 @@ public: /// \param IsSystem Whether this is a module map from a system include path. virtual void moduleMapFileRead(SourceLocation FileStart, const FileEntry &File, bool IsSystem) {} + + /// \brief Called when a header is added during module map parsing. + /// + /// \param File The header file itself. + virtual void moduleMapAddHeader(const FileEntry &File) {} }; class ModuleMap { diff --git a/clang/lib/Frontend/ModuleDependencyCollector.cpp b/clang/lib/Frontend/ModuleDependencyCollector.cpp index fc3958f..e0ec674 100644 --- a/clang/lib/Frontend/ModuleDependencyCollector.cpp +++ b/clang/lib/Frontend/ModuleDependencyCollector.cpp @@ -12,6 +12,7 @@ //===----------------------------------------------------------------------===// #include "clang/Frontend/Utils.h" +#include "clang/Lex/Preprocessor.h" #include "clang/Serialization/ASTReader.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/iterator_range.h" @@ -22,7 +23,7 @@ using namespace clang; namespace { -/// Private implementation for ModuleDependencyCollector +/// Private implementations for ModuleDependencyCollector class ModuleDependencyListener : public ASTReaderListener { ModuleDependencyCollector &Collector; public: @@ -36,12 +37,30 @@ public: return true; } }; + +struct ModuleDependencyMMCallbacks : public ModuleMapCallbacks { + ModuleDependencyCollector &Collector; + ModuleDependencyMMCallbacks(ModuleDependencyCollector &Collector) + : Collector(Collector) {} + + void moduleMapAddHeader(const FileEntry &File) override { + StringRef HeaderPath = File.getName(); + if (llvm::sys::path::is_absolute(HeaderPath)) + Collector.addFile(HeaderPath); + } +}; + } void ModuleDependencyCollector::attachToASTReader(ASTReader &R) { R.addListener(llvm::make_unique(*this)); } +void ModuleDependencyCollector::attachToPreprocessor(Preprocessor &PP) { + PP.getHeaderSearchInfo().getModuleMap().addModuleMapCallbacks( + llvm::make_unique(*this)); +} + void ModuleDependencyCollector::writeFileMap() { if (Seen.empty()) return; diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp index 69cb574..4b782a3 100644 --- a/clang/lib/Lex/ModuleMap.cpp +++ b/clang/lib/Lex/ModuleMap.cpp @@ -811,6 +811,10 @@ void ModuleMap::addHeader(Module *Mod, Module::Header Header, HeaderInfo.MarkFileModuleHeader(Header.Entry, Role, isCompilingModuleHeader); } + + // Notify callbacks that we just added a new header. + for (const auto &Cb : Callbacks) + Cb->moduleMapAddHeader(*Header.Entry); } void ModuleMap::excludeHeader(Module *Mod, Module::Header Header) { diff --git a/clang/test/Modules/crash-vfs-path-symlink-topheader.m b/clang/test/Modules/crash-vfs-path-symlink-topheader.m new file mode 100644 index 0000000..29f6b93 --- /dev/null +++ b/clang/test/Modules/crash-vfs-path-symlink-topheader.m @@ -0,0 +1,50 @@ +// REQUIRES: crash-recovery, shell + +// FIXME: This XFAIL is cargo-culted from crash-report.c. Do we need it? +// XFAIL: mingw32 + +// Test clang can collect symbolic link headers used in modules. +// crash reproducer if there's a symbolic link header file used in a module. + +// RUN: rm -rf %t +// RUN: mkdir -p %t/i %t/m %t %t/sysroot +// RUN: cp -a %S/Inputs/crash-recovery/usr %t/i/ +// RUN: rm -f %t/i/usr/include/pthread_impl.h +// RUN: ln -s pthread/pthread_impl.h %t/i/usr/include/pthread_impl.h + +// RUN: not env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \ +// RUN: %clang -fsyntax-only %s -I %/t/i -isysroot %/t/sysroot/ \ +// RUN: -fmodules -fmodules-cache-path=%t/m/ 2>&1 | FileCheck %s + +// RUN: FileCheck --check-prefix=CHECKSRC %s -input-file %t/crash-vfs-*.m +// RUN: FileCheck --check-prefix=CHECKSH %s -input-file %t/crash-vfs-*.sh +// 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 "usr/include/pthread_impl.h" | count 1 + +#include "usr/include/stdio.h" + +// CHECK: Preprocessed source(s) and associated run script(s) are located at: +// CHECK-NEXT: note: diagnostic msg: {{.*}}.m +// CHECK-NEXT: note: diagnostic msg: {{.*}}.cache + +// CHECKSRC: @import cstd.stdio; + +// CHECKSH: # Crash reproducer +// CHECKSH-NEXT: # Driver args: "-fsyntax-only" +// CHECKSH-NEXT: # Original command: {{.*$}} +// CHECKSH-NEXT: "-cc1" +// CHECKSH: "-isysroot" "{{[^"]*}}/sysroot/" +// CHECKSH-NOT: "-fmodules-cache-path=" +// CHECKSH: "crash-vfs-{{[^ ]*}}.m" +// CHECKSH: "-ivfsoverlay" "crash-vfs-{{[^ ]*}}.cache/vfs/vfs.yaml" + +// CHECKYAML: 'type': 'directory', +// CHECKYAML: 'name': "", +// CHECKYAML-NEXT: 'contents': [ +// CHECKYAML-NEXT: { +// CHECKYAML-NEXT: 'type': 'file', +// CHECKYAML-NEXT: 'name': "pthread_impl.h", +// CHECKYAML-NEXT: 'external-contents': "/{{.*}}/i/usr/include/pthread_impl.h" +// CHECKYAML-NEXT: }, -- 2.7.4