Added -fcrash-diagnostics-dir flag

[j10kay]

Summary

New flag causes crash reports to be written in the specified directory
rather than the temp directory.

[zturner]

clang/lib/Driver/Driver.cpp

4043–4044 ↗

(On Diff #152933)

Maybe you can combine these into one line with:

SmallString<128> CrashDirectory{ A->getValue() };

clang/test/Driver/crash-diagnostics-dir.c

5 ↗

(On Diff #152933)

This will fail on Windows I think where path separators are backslashes. I think you need to do something like:

{{.*}}Output{{/|\\}}crash-diagnostics-dir.c.tmp{{(/|\\).*}}.c

llvm/include/llvm/Support/FileSystem.h

758–759 ↗

(On Diff #152933)

Is there any reason we can't just use the existing overload? In other words, instead of creating this overload and having the user write createUniqueFile("foo", "bar"), how about just createUniqueFile("foo-%%%%%%.bar") which seems equivalent.

[hans]

clang/lib/Driver/Driver.cpp

4043–4044 ↗

(On Diff #152933)

Or even simpler,

SmallString<128> CrashDirectory = A->getValue();

:-)

clang/test/Driver/crash-diagnostics-dir.c

1 ↗

(On Diff #152933)

The other test/Driver/crash-report-* tests do "rm -rf %t" before creating the directory. Since you're using "-p" with mkdir below, it won't fail if the dir exists, but removing the dir before is still nice since it will clean up temporary files from previous test runs. Without it, a new file will be added to the dir each time the test is run, so it will grow in size over time.

[j10kay]

Implemented the code review changes

[hans]

lgtm

cfe/trunk/include/clang/Driver/Options.td

 def fconstant_cfstrings : Flag<["-"], "fconstant-cfstrings">, Group<f_Group>;
 def fconstant_string_class_EQ : Joined<["-"], "fconstant-string-class=">, Group<f_Group>;
 def fconstexpr_depth_EQ : Joined<["-"], "fconstexpr-depth=">, Group<f_Group>;
 def fconstexpr_steps_EQ : Joined<["-"], "fconstexpr-steps=">, Group<f_Group>;
 def fconstexpr_backtrace_limit_EQ : Joined<["-"], "fconstexpr-backtrace-limit=">,
                                     Group<f_Group>;
 def fno_crash_diagnostics : Flag<["-"], "fno-crash-diagnostics">, Group<f_clang_Group>, Flags<[NoArgumentUnused]>,
   HelpText<"Disable auto-generation of preprocessed source files and a script for reproduction during a clang crash">;
+def fcrash_diagnostics_dir : Joined<["-"], "fcrash-diagnostics-dir=">, Group<f_clang_Group>, Flags<[NoArgumentUnused]>;
 def fcreate_profile : Flag<["-"], "fcreate-profile">, Group<f_Group>;
 def fcxx_exceptions: Flag<["-"], "fcxx-exceptions">, Group<f_Group>,
   HelpText<"Enable C++ exceptions">, Flags<[CC1Option]>;
 def fcxx_modules : Flag <["-"], "fcxx-modules">, Group<f_Group>,
   Flags<[DriverOption]>;
 def fdebug_pass_arguments : Flag<["-"], "fdebug-pass-arguments">, Group<f_Group>;
 def fdebug_pass_structure : Flag<["-"], "fdebug-pass-structure">, Group<f_Group>;
 def fdepfile_entry : Joined<["-"], "fdepfile-entry=">,

cfe/trunk/lib/Driver/Driver.cpp

if (StringRef(TempFile).endswith(".cache")) {
       VFS = llvm::sys::path::filename(TempFile);
       llvm::sys::path::append(VFS, "vfs", "vfs.yaml");
     }
   }
 
   // Assume associated files are based off of the first temporary file.
   CrashReportInfo CrashInfo(TempFiles[0], VFS);
 
-  std::string Script = CrashInfo.Filename.rsplit('.').first.str() + ".sh";
+  llvm::SmallString<128> Script(CrashInfo.Filename);
+  llvm::sys::path::replace_extension(Script, "sh");
   std::error_code EC;
   llvm::raw_fd_ostream ScriptOS(Script, EC, llvm::sys::fs::CD_CreateNew);
   if (EC) {
     Diag(clang::diag::note_drv_command_failed_diag_msg)
-        << "Error generating run script: " + Script + " " + EC.message();
+        << "Error generating run script: " << Script << " " << EC.message();
   } else {
     ScriptOS << "# Crash reproducer for " << getClangFullVersion() << "\n"
              << "# Driver args: ";
     printArgList(ScriptOS, C.getInputArgs());
     ScriptOS << "# Original command: ";
     Cmd.Print(ScriptOS, "\n", /*Quote=*/true);
     Cmd.Print(ScriptOS, "\n", /*Quote=*/true, &CrashInfo);
     if (!AdditionalInformation.empty())
       ScriptOS << "\n# Additional information: " << AdditionalInformation
                << "\n";
     if (Report)
-      Report->TemporaryFiles.push_back(Script);
+      Report->TemporaryFiles.push_back(Script.str());
     Diag(clang::diag::note_drv_command_failed_diag_msg) << Script;
   }
 
   // On darwin, provide information about the .crash diagnostic report.
   if (llvm::Triple(llvm::sys::getProcessTriple()).isOSDarwin()) {
     SmallString<128> CrashDiagDir;
     if (getCrashDiagnosticFile(ReproCrashFilename, CrashDiagDir)) {
       Diag(clang::diag::note_drv_command_failed_diag_msg)
const char *Driver::GetNamedOutputPath(Compilation &C, const JobAction &JA,
   }
 
   // Output to a temporary file?
   if ((!AtTopLevel && !isSaveTempsEnabled() &&
        !C.getArgs().hasArg(options::OPT__SLASH_Fo)) ||
       CCGenDiagnostics) {
     StringRef Name = llvm::sys::path::filename(BaseInput);
     std::pair<StringRef, StringRef> Split = Name.split('.');
-    std::string TmpName = GetTemporaryPath(
-        Split.first, types::getTypeTempSuffix(JA.getType(), IsCLMode()));
+    SmallString<128> TmpName;
+    const char *Suffix = types::getTypeTempSuffix(JA.getType(), IsCLMode());
+    Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir);
+    if (CCGenDiagnostics && A) {
+      SmallString<128> CrashDirectory(A->getValue());
+      llvm::sys::path::append(CrashDirectory, Split.first);
+      const char *Middle = Suffix ? "-%%%%%%." : "-%%%%%%";
+      std::error_code EC =
+          llvm::sys::fs::createUniqueFile(CrashDirectory + Middle + Suffix, TmpName);
+      if (EC) {
+        Diag(clang::diag::err_unable_to_make_temp) << EC.message();
+        return "";
+      }
+    } else {
+      TmpName = GetTemporaryPath(Split.first, Suffix);
+    }
     return C.addTempFile(C.getArgs().MakeArgString(TmpName));
   }
 
   SmallString<128> BasePath(BaseInput);
   StringRef BaseName;
 
   // Dsymutil actions should use the full path.
   if (isa<DsymutilJobAction>(JA) || isa<VerifyJobAction>(JA))

cfe/trunk/test/Driver/crash-diagnostics-dir.c

+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: not %clang -fcrash-diagnostics-dir=%t -c %s 2>&1 | FileCheck %s
+#pragma clang __debug parser_crash
+// CHECK: Preprocessed source(s) and associated run script(s) are located at:
+// CHECK: diagnostic msg: {{.*}}Output{{/|\\}}crash-diagnostics-dir.c.tmp{{(/|\\).*}}.c