From e3a0aef2cf93c7dc144c52798aa5546dc5ec304b Mon Sep 17 00:00:00 2001 From: Bruno Cardoso Lopes Date: Fri, 9 Dec 2016 02:22:47 +0000 Subject: [PATCH] [CrashReproducer] Rewrite relative include paths When -fmodules is on, the reproducer invocation currently leave paths for include-like flags as is. If the path is relative, the reproducer doesn't know how to access that file during reproduction time because the VFS cannot reason about relative paths. Expand relative paths to absolute ones when creating the reproducer command line. This allows, for example, the reproducer to work for crashes while building clang with modules; this wasn't possible before because building clang requires using relative inc dir from within the build directory. rdar://problem/28655070 llvm-svn: 289174 --- clang/lib/Driver/Job.cpp | 118 ++++++++++++++++++++----- clang/test/Modules/crash-vfs-relative-incdir.m | 58 ++++++++++++ 2 files changed, 152 insertions(+), 24 deletions(-) create mode 100644 clang/test/Modules/crash-vfs-relative-incdir.m diff --git a/clang/lib/Driver/Job.cpp b/clang/lib/Driver/Job.cpp index a8f585e..94ea606 100644 --- a/clang/lib/Driver/Job.cpp +++ b/clang/lib/Driver/Job.cpp @@ -19,6 +19,7 @@ #include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringSet.h" #include "llvm/ADT/StringSwitch.h" +#include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" #include "llvm/Support/Program.h" #include "llvm/Support/raw_ostream.h" @@ -38,46 +39,59 @@ Command::Command(const Action &Source, const Tool &Creator, InputFilenames.push_back(II.getFilename()); } -static int skipArgs(const char *Flag, bool HaveCrashVFS) { +/// @brief Check if the compiler flag in question should be skipped when +/// emitting a reproducer. Also track how many arguments it has and if the +/// option is some kind of include path. +static bool skipArgs(const char *Flag, bool HaveCrashVFS, int &SkipNum, + bool &IsInclude) { + SkipNum = 2; // These flags are all of the form -Flag and are treated as two // arguments. Therefore, we need to skip the flag and the next argument. - bool Res = llvm::StringSwitch(Flag) + bool ShouldSkip = llvm::StringSwitch(Flag) .Cases("-MF", "-MT", "-MQ", "-serialize-diagnostic-file", true) .Cases("-o", "-coverage-file", "-dependency-file", true) - .Cases("-fdebug-compilation-dir", "-idirafter", true) - .Cases("-include", "-include-pch", "-internal-isystem", true) - .Cases("-internal-externc-isystem", "-iprefix", "-iwithprefix", true) - .Cases("-iwithprefixbefore", "-isystem", "-iquote", true) + .Cases("-fdebug-compilation-dir", "-include-pch", true) .Cases("-dwarf-debug-flags", "-ivfsoverlay", true) - .Cases("-header-include-file", "-diagnostic-log-file", true) - // Some include flags shouldn't be skipped if we have a crash VFS - .Cases("-isysroot", "-I", "-F", "-resource-dir", !HaveCrashVFS) + .Case("-diagnostic-log-file", true) .Default(false); - - // Match found. - if (Res) - return 2; + if (ShouldSkip) + return true; + + // Some include flags shouldn't be skipped if we have a crash VFS + IsInclude = llvm::StringSwitch(Flag) + .Cases("-include", "-header-include-file", true) + .Cases("-idirafter", "-internal-isystem", "-iwithprefix", true) + .Cases("-internal-externc-isystem", "-iprefix", true) + .Cases("-iwithprefixbefore", "-isystem", "-iquote", true) + .Cases("-isysroot", "-I", "-F", "-resource-dir", true) + .Case("-iframework", true) + .Default(false); + if (IsInclude) + return HaveCrashVFS ? false : true; // The remaining flags are treated as a single argument. // These flags are all of the form -Flag and have no second argument. - Res = llvm::StringSwitch(Flag) + ShouldSkip = llvm::StringSwitch(Flag) .Cases("-M", "-MM", "-MG", "-MP", "-MD", true) .Case("-MMD", true) .Default(false); // Match found. - if (Res) - return 1; + SkipNum = 1; + if (ShouldSkip) + return true; // These flags are treated as a single argument (e.g., -F). StringRef FlagRef(Flag); - if ((!HaveCrashVFS && - (FlagRef.startswith("-F") || FlagRef.startswith("-I"))) || - FlagRef.startswith("-fmodules-cache-path=")) - return 1; - - return 0; + IsInclude = FlagRef.startswith("-F") || FlagRef.startswith("-I"); + if (IsInclude) + return HaveCrashVFS ? false : true; + if (FlagRef.startswith("-fmodules-cache-path=")) + return true; + + SkipNum = 0; + return false; } void Command::printArg(raw_ostream &OS, StringRef Arg, bool Quote) { @@ -153,6 +167,45 @@ void Command::buildArgvForResponseFile( } } +/// @brief Rewrite relative include-like flag paths to absolute ones. +static void +rewriteIncludes(const llvm::ArrayRef &Args, size_t Idx, + size_t NumArgs, + llvm::SmallVectorImpl> &IncFlags) { + using namespace llvm; + using namespace sys; + auto getAbsPath = [](StringRef InInc, SmallVectorImpl &OutInc) -> bool { + if (path::is_absolute(InInc)) // Nothing to do here... + return false; + std::error_code EC = fs::current_path(OutInc); + if (EC) + return false; + path::append(OutInc, InInc); + return true; + }; + + SmallString<128> NewInc; + if (NumArgs == 1) { + StringRef FlagRef(Args[Idx + NumArgs - 1]); + assert((FlagRef.startswith("-F") || FlagRef.startswith("-I")) && + "Expecting -I or -F"); + StringRef Inc = FlagRef.slice(2, StringRef::npos); + if (getAbsPath(Inc, NewInc)) { + SmallString<128> NewArg(FlagRef.slice(0, 2)); + NewArg += NewInc; + IncFlags.push_back(std::move(NewArg)); + } + return; + } + + assert(NumArgs == 2 && "Not expecting more than two arguments"); + StringRef Inc(Args[Idx + NumArgs - 1]); + if (!getAbsPath(Inc, NewInc)) + return; + IncFlags.push_back(SmallString<128>(Args[Idx])); + IncFlags.push_back(std::move(NewInc)); +} + void Command::Print(raw_ostream &OS, const char *Terminator, bool Quote, CrashReportInfo *CrashInfo) const { // Always quote the exe. @@ -171,10 +224,27 @@ void Command::Print(raw_ostream &OS, const char *Terminator, bool Quote, const char *const Arg = Args[i]; if (CrashInfo) { - if (int Skip = skipArgs(Arg, HaveCrashVFS)) { - i += Skip - 1; + int NumArgs = 0; + bool IsInclude = false; + if (skipArgs(Arg, HaveCrashVFS, NumArgs, IsInclude)) { + i += NumArgs - 1; continue; } + + // Relative includes need to be expanded to absolute paths. + if (HaveCrashVFS && IsInclude) { + SmallVector, 2> NewIncFlags; + rewriteIncludes(Args, i, NumArgs, NewIncFlags); + if (!NewIncFlags.empty()) { + for (auto &F : NewIncFlags) { + OS << ' '; + printArg(OS, F.c_str(), Quote); + } + i += NumArgs - 1; + continue; + } + } + auto Found = std::find_if(InputFilenames.begin(), InputFilenames.end(), [&Arg](StringRef IF) { return IF == Arg; }); if (Found != InputFilenames.end() && diff --git a/clang/test/Modules/crash-vfs-relative-incdir.m b/clang/test/Modules/crash-vfs-relative-incdir.m new file mode 100644 index 0000000..733b795 --- /dev/null +++ b/clang/test/Modules/crash-vfs-relative-incdir.m @@ -0,0 +1,58 @@ +// REQUIRES: crash-recovery, shell, system-darwin + +// RUN: rm -rf %t +// RUN: mkdir -p %t/m +// RUN: cd %S/Inputs/crash-recovery/usr + +// RUN: not env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \ +// RUN: %clang -fsyntax-only -nostdinc %s -Iinclude \ +// 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 "Inputs/crash-recovery/usr/include/stdio.h" | count 1 + +#include + +// 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: "-resource-dir" +// CHECKSH: "-I" "/[[INCPATH:.*]]/include" +// CHECKSH: "crash-vfs-{{[^ ]*}}.m" +// CHECKSH: "-ivfsoverlay" "crash-vfs-{{[^ ]*}}.cache/vfs/vfs.yaml" +// CHECKSH: "-fmodules-cache-path=crash-vfs-{{[^ ]*}}.cache/modules" + +// CHECKYAML: 'case-sensitive': +// CHECKYAML-NEXT: 'use-external-names': 'false', +// CHECKYAML-NEXT: 'overlay-relative': 'true', +// CHECKYAML-NEXT: 'ignore-non-existent-contents': 'false' +// CHECKYAML: 'type': 'directory' +// CHECKYAML: 'name': "/[[PATH:.*]]/Inputs/crash-recovery/usr/include", +// CHECKYAML-NEXT: 'contents': [ +// CHECKYAML-NEXT: { +// CHECKYAML-NEXT: 'type': 'file', +// CHECKYAML-NEXT: 'name': "module.map", +// CHECKYAML-NOT: 'external-contents': "{{[^ ]*}}.cache +// CHECKYAML-NEXT: 'external-contents': "/[[PATH]]/Inputs/crash-recovery/usr/include/module.map" +// CHECKYAML-NEXT: }, + +// Run the reproducer script - regular exit code is enough to test it works. +// Note that we don't yet support reusing the modules pcm; what we do +// support is re-building the modules relying solely on the header files dumped +// inside .cache/vfs, mapped by .cache/vfs/vfs.yaml. + +// RUN: cd %t +// RUN: rm -rf crash-vfs-relative-incdir-*.cache/modules +// RUN: chmod 755 crash-vfs-*.sh +// RUN: ./crash-vfs-*.sh -- 2.7.4