[driver] Completely rework how superfluous options are stripped out of the crash

diagnostics script.

This addresses the FIXME pertaining to quoted arguments.  We also delineate
between those flags that have an argument (e.g., -D macro, -MF file) and
those that do not (e.g., -M, -MM, -MG).  Finally, we add the -dwarf-debug-flags
to the list of flags to be removed.
rdar://12329974

llvm-svn: 167152

clang/include/clang/Driver/Compilation.h

@@ -141,6 +141,14 @@ public:
   void PrintJob(raw_ostream &OS, const Job &J,
                 const char *Terminator, bool Quote) const;
 
+  /// PrintDiagnosticJob - Print one job in -### format, but with the 
+  /// superfluous options removed, which are not necessary for 
+  /// reproducing the crash.
+  ///
+  /// \param OS - The stream to print on.
+  /// \param J - The job to print.
+  void PrintDiagnosticJob(raw_ostream &OS, const Job &J) const;
+
   /// ExecuteCommand - Execute an actual command.
   ///
   /// \param FailingCommand - For non-zero results, this will be set to the

clang/lib/Driver/Compilation.cpp

@@ -17,6 +17,7 @@
 #include "clang/Driver/ToolChain.h"
 
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/Program.h"
 #include <sys/stat.h>
@@ -101,6 +102,105 @@ void Compilation::PrintJob(raw_ostream &OS, const Job &J,
   }
 }
 
+static bool skipArg(const char *Flag, bool &SkipNextArg) {
+  StringRef FlagRef(Flag);
+
+  // Assume we're going to see -Flag <Arg>.
+  SkipNextArg = true;
+
+  // 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)
+    .Cases("-I", "-MF", "-MT", "-MQ", true)
+    .Cases("-o", "-coverage-file", "-dependency-file", true)
+    .Cases("-fdebug-compilation-dir", "-fmodule-cache-path", "-idirafter", true)
+    .Cases("-include", "-include-pch", "-internal-isystem", true)
+    .Cases("-internal-externc-isystem ", "-iprefix ", "-iwithprefix", true)
+    .Cases("-iwithprefixbefore", "-isysroot", "-isystem", "-iquote", true)
+    .Cases("-resource-dir", "-serialize-diagnostic-file", true)
+    .Case("-dwarf-debug-flags", true)
+    .Default(false);
+
+  // Match found.
+  if (Res)
+    return Res;
+
+  // The remaining flags are treated as a single argument.
+  SkipNextArg = false;
+
+  // These flags are all of the form -Flag and have no second argument.
+  Res = llvm::StringSwitch<bool>(Flag)
+    .Cases("-M", "-MM", "-MG", "-MP", "-MD", true)
+    .Case("-MMD", true)
+    .Default(false);
+
+  // Match found.
+  if (Res)
+    return Res;
+
+  // These flags are treated as a single argument (e.g., -F<Dir>).
+  if (FlagRef.startswith("-F") || FlagRef.startswith("-I"))
+    return true;
+
+  return false;
+}
+
+static bool quoteNextArg(const char *flag) {
+  return llvm::StringSwitch<bool>(flag)
+    .Case("-D", true)
+    .Default(false);
+}
+
+void Compilation::PrintDiagnosticJob(raw_ostream &OS, const Job &J) const {
+  if (const Command *C = dyn_cast<Command>(&J)) {
+    OS << C->getExecutable();
+    unsigned QuoteNextArg = 0;
+    for (ArgStringList::const_iterator it = C->getArguments().begin(),
+           ie = C->getArguments().end(); it != ie; ++it) {
+
+      bool SkipNext;
+      if (skipArg(*it, SkipNext)) {
+        if (SkipNext) ++it;
+        continue;
+      }
+
+      if (!QuoteNextArg)
+        QuoteNextArg = quoteNextArg(*it) ? 2 : 0;
+
+      OS << ' ';
+
+      if (QuoteNextArg == 1)
+        OS << '"';
+
+      if (!std::strpbrk(*it, " \"\\$")) {
+        OS << *it;
+      } else {
+        // Quote the argument and escape shell special characters; this isn't
+        // really complete but is good enough.
+        OS << '"';
+        for (const char *s = *it; *s; ++s) {
+          if (*s == '"' || *s == '\\' || *s == '$')
+            OS << '\\';
+          OS << *s;
+        }
+        OS << '"';
+      }
+
+      if (QuoteNextArg) {
+        if (QuoteNextArg == 1)
+          OS << '"';
+        --QuoteNextArg;
+      }
+    }
+    OS << '\n';
+  } else {
+    const JobList *Jobs = cast<JobList>(&J);
+    for (JobList::const_iterator
+           it = Jobs->begin(), ie = Jobs->end(); it != ie; ++it)
+      PrintDiagnosticJob(OS, **it);
+  }
+}
+
 bool Compilation::CleanupFileList(const ArgStringList &Files,
                                   bool IssueErrors) const {
   bool Success = true;

clang/lib/Driver/Driver.cpp

@@ -362,11 +362,11 @@ void Driver::generateCompilationDiagnostics(Compilation &C,
   std::string Cmd;
   llvm::raw_string_ostream OS(Cmd);
   if (FailingCommand)
-    C.PrintJob(OS, *FailingCommand, "\n", false);
+    C.PrintDiagnosticJob(OS, *FailingCommand);
   else
     // Crash triggered by FORCE_CLANG_DIAGNOSTICS_CRASH, which doesn't have an
     // associated FailingCommand, so just pass all jobs.
-    C.PrintJob(OS, C.getJobs(), "\n", false);
+    C.PrintDiagnosticJob(OS, C.getJobs());
   OS.flush();
 
   // Clear stale state and suppress tool output.
@@ -464,57 +464,6 @@ void Driver::generateCompilationDiagnostics(Compilation &C,
         Diag(clang::diag::note_drv_command_failed_diag_msg)
           << "Error generating run script: " + Script + " " + Err;
       } else {
-        // Strip away options not necessary to reproduce the crash.
-        // FIXME: This doesn't work with quotes (e.g., -D "foo bar").
-        SmallVector<std::string, 16> Flag;
-        Flag.push_back("-D ");
-        Flag.push_back("-F");
-        Flag.push_back("-I ");
-        Flag.push_back("-M ");
-        Flag.push_back("-MD ");
-        Flag.push_back("-MF ");
-        Flag.push_back("-MG ");
-        Flag.push_back("-MM ");
-        Flag.push_back("-MMD ");
-        Flag.push_back("-MP ");
-        Flag.push_back("-MQ ");
-        Flag.push_back("-MT ");
-        Flag.push_back("-o ");
-        Flag.push_back("-coverage-file ");
-        Flag.push_back("-dependency-file ");
-        Flag.push_back("-fdebug-compilation-dir ");
-        Flag.push_back("-fmodule-cache-path ");
-        Flag.push_back("-idirafter ");
-        Flag.push_back("-include ");
-        Flag.push_back("-include-pch ");
-        Flag.push_back("-internal-isystem ");
-        Flag.push_back("-internal-externc-isystem ");
-        Flag.push_back("-iprefix ");
-        Flag.push_back("-iwithprefix ");
-        Flag.push_back("-iwithprefixbefore ");
-        Flag.push_back("-isysroot ");
-        Flag.push_back("-isystem ");
-        Flag.push_back("-iquote ");
-        Flag.push_back("-resource-dir ");
-        Flag.push_back("-serialize-diagnostic-file ");
-        for (unsigned i = 0, e = Flag.size(); i < e; ++i) {
-          size_t I = 0, E = 0;
-          do {
-            I = Cmd.find(Flag[i], I);
-            if (I == std::string::npos) break;
-
-            E = Cmd.find(" ", I + Flag[i].length());
-            if (E == std::string::npos) break;
-            // The -D option is not removed.  Instead, the argument is quoted.
-            if (Flag[i] != "-D ") {
-              Cmd.erase(I, E - I + 1);
-            } else {
-              Cmd.insert(I+3, "\"");
-              Cmd.insert(++E, "\"");
-              I = E;
-            }
-          } while(1);
-        }
         // Append the new filename with correct preprocessed suffix.
         size_t I, E;
         I = Cmd.find("-main-file-name ");

clang/test/Driver/crash-report.c

@@ -1,6 +1,6 @@
 // RUN: rm -rf %t
 // RUN: mkdir %t
-// RUN: env TMPDIR=%t TEMP=%t TMP=%t %clang -fsyntax-only %s \
+// RUN: env TMPDIR=%t TEMP=%t TMP=%t RC_DEBUG_OPTIONS=1 %clang -fsyntax-only %s \
 // RUN:  -F/tmp/ -I /tmp/ -idirafter /tmp/ -iquote /tmp/ -isystem /tmp/ \
 // RUN:  -iprefix /the/prefix -iwithprefix /tmp -iwithprefixbefore /tmp/ \
 // RUN:  -internal-isystem /tmp/ -internal-externc-isystem /tmp/ \
@@ -25,3 +25,4 @@ FOO
 // CHECKSH-NOT: -iwithprefixbefore /tmp/
 // CHECKSH-NOT: -internal-isystem /tmp/
 // CHECKSH-NOT: -internal-externc-isystem /tmp/
+// CHECKSH-NOT: -dwarf-debug-flags