r289174 - [CrashReproducer] Rewrite relative include paths

Bruno Cardoso Lopes via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 8 18:22:47 PST 2016


Author: bruno
Date: Thu Dec  8 20:22:47 2016
New Revision: 289174

URL: http://llvm.org/viewvc/llvm-project?rev=289174&view=rev
Log:
[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

Added:
    cfe/trunk/test/Modules/crash-vfs-relative-incdir.m
Modified:
    cfe/trunk/lib/Driver/Job.cpp

Modified: cfe/trunk/lib/Driver/Job.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Job.cpp?rev=289174&r1=289173&r2=289174&view=diff
==============================================================================
--- cfe/trunk/lib/Driver/Job.cpp (original)
+++ cfe/trunk/lib/Driver/Job.cpp Thu Dec  8 20:22:47 2016
@@ -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, c
       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 <Arg> and are treated as two
   // arguments.  Therefore, we need to skip the flag and the next argument.
-  bool Res = llvm::StringSwitch<bool>(Flag)
+  bool ShouldSkip = llvm::StringSwitch<bool>(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);
+  if (ShouldSkip)
+    return true;
 
-  // Match found.
-  if (Res)
-    return 2;
+  // Some include flags shouldn't be skipped if we have a crash VFS
+  IsInclude = llvm::StringSwitch<bool>(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<bool>(Flag)
+  ShouldSkip = llvm::StringSwitch<bool>(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<Dir>).
   StringRef FlagRef(Flag);
-  if ((!HaveCrashVFS &&
-       (FlagRef.startswith("-F") || FlagRef.startswith("-I"))) ||
-      FlagRef.startswith("-fmodules-cache-path="))
-    return 1;
+  IsInclude = FlagRef.startswith("-F") || FlagRef.startswith("-I");
+  if (IsInclude)
+    return HaveCrashVFS ? false : true;
+  if (FlagRef.startswith("-fmodules-cache-path="))
+    return true;
 
-  return 0;
+  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<const char *> &Args, size_t Idx,
+                size_t NumArgs,
+                llvm::SmallVectorImpl<llvm::SmallString<128>> &IncFlags) {
+  using namespace llvm;
+  using namespace sys;
+  auto getAbsPath = [](StringRef InInc, SmallVectorImpl<char> &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, con
     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<SmallString<128>, 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() &&

Added: cfe/trunk/test/Modules/crash-vfs-relative-incdir.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/crash-vfs-relative-incdir.m?rev=289174&view=auto
==============================================================================
--- cfe/trunk/test/Modules/crash-vfs-relative-incdir.m (added)
+++ cfe/trunk/test/Modules/crash-vfs-relative-incdir.m Thu Dec  8 20:22:47 2016
@@ -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 <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: "-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




More information about the cfe-commits mailing list