Page MenuHomePhabricator

Added -fcrash-diagnostics-dir flag
ClosedPublic

Authored by j10kay on Jun 26 2018, 11:49 AM.

Details

Summary

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

Diff Detail

Repository
rL LLVM

Event Timeline

j10kay created this revision.Jun 26 2018, 11:49 AM
zturner added inline comments.
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 added inline comments.Jun 27 2018, 2:16 AM
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 updated this revision to Diff 153773.Jul 2 2018, 1:00 PM
j10kay marked 3 inline comments as done.

Implemented the code review changes

hans accepted this revision.Jul 3 2018, 12:37 AM

lgtm

This revision is now accepted and ready to land.Jul 3 2018, 12:37 AM
This revision was automatically updated to reflect the committed changes.