[sanitizer] Add suffix to report file name

[WallStProg]

Summary

For those using a GUI, it can be very helpful to have a
particular suffix appended to the report file name, so
it can be opened with a double-click.

(see also: https://github.com/google/sanitizers/issues/951)

[kcc]

I am a bit reluctant to add more flags, but this change seems to be pretty small, so probably ok.
Could you please add a test?
(Something similar to test/asan/TestCases/verbose-log-path_test.cc, but in test/sanitizer_common)

[WallStProg]

Could you please add a test?
(Something similar to test/asan/TestCases/verbose-log-path_test.cc, but in test/sanitizer_common)

OK -- give me a couple days to figure out the test infrastructure. AFAICT verbose-log-path_test.cc isn't even being built in my env.

[WallStProg]

Well, I've added a test pgm but the only way I've been able to is within asan (using monkey-see, monkey-do approach ;-) The test is modeled on existing verbose-log-path_test, and appears to run successfully with "make check-all".

Getting this into sanitizer-common is way beyond what I can hope to do at this point. If you can provide some pointers on writing tests I can take another shot at it (although frankly I'd much rather not ;-)

Hope this is sufficient -- pls let me know.

[filcab]

Check out test/sanitizer_common/TestCases/options-include.cc as an example you can use to adapt yours.
You might only need to change env_asan_opts to env_tool_opts to get it to work.

Instead of having a positive FIXME, please change it to a negative: windows and android don't support X (which is what the line below is doing).

[WallStProg]

In D46546#1150695, @filcab wrote:

Check out test/sanitizer_common/TestCases/options-include.cc as an example you can use to adapt yours.
You might only need to change env_asan_opts to env_tool_opts to get it to work.

Instead of having a positive FIXME, please change it to a negative: windows and android don't support X (which is what the line below is doing).

Sorry, but I have no idea what all those decorations are, nor do I have any desire to learn how to shave that particular yak. The test program was simply copied and pasted from verbose-log-path-test.cc, so any comment should apply to it as well.

Hopefully this will make into trunk at some point -- it's quite useful, and I'd love to be able to stop patching my local builds.

[bill-torpey]

It's getting to be a real PITA to build new versions of clang just to get this very useful but trivial change -- not to mention I would *much* rather use the native clang on a particular platform like Ubuntu.

Please consider making this part of an upcoming release. Thanks!

[vitalybuka]

In D46546#2543151, @bill-torpey wrote:

It's getting to be a real PITA to build new versions of clang just to get this very useful but trivial change -- not to mention I would *much* rather use the native clang on a particular platform like Ubuntu.

Please consider making this part of an upcoming release. Thanks!

Sorry, I didn't notice that you have no committer access at that time.

compiler-rt/lib/sanitizer_common/sanitizer_file.cpp

void ReportFile::ReopenIfNecessary() {
 
   const char *exe_name = GetProcessName();
   if (common_flags()->log_exe_name && exe_name) {
     internal_snprintf(full_path, kMaxPathLength, "%s.%s.%zu", path_prefix,
                       exe_name, pid);
   } else {
     internal_snprintf(full_path, kMaxPathLength, "%s.%zu", path_prefix, pid);
   }
+  if (common_flags()->log_suffix) {
+    internal_strlcat(full_path, common_flags()->log_suffix, kMaxPathLength);
+  }
   error_t err;
   fd = OpenFile(full_path, WrOnly, &err);
   if (fd == kInvalidFd) {
     const char *ErrorMsgPrefix = "ERROR: Can't open file: ";
     WriteToFile(kStderrFd, ErrorMsgPrefix, internal_strlen(ErrorMsgPrefix));
     WriteToFile(kStderrFd, full_path, internal_strlen(full_path));
     char errmsg[100];
     internal_snprintf(errmsg, sizeof(errmsg), " (reason: %d)", err);

compiler-rt/lib/sanitizer_common/sanitizer_flags.inc

 COMMON_FLAG(
     const char *, log_path, nullptr,
     "Write logs to \"log_path.pid\". The special values are \"stdout\" and "
     "\"stderr\". If unspecified, defaults to \"stderr\".")
 COMMON_FLAG(
     bool, log_exe_name, false,
     "Mention name of executable when reporting error and "
     "append executable name to logs (as in \"log_path.exe_name.pid\").")
+COMMON_FLAG(const char *, log_suffix, nullptr,
+            "String to append to log file name, e.g. \".txt\".")
 COMMON_FLAG(
     bool, log_to_syslog, (bool)SANITIZER_ANDROID || (bool)SANITIZER_MAC,
     "Write all sanitizer output to syslog in addition to other means of "
     "logging.")
 COMMON_FLAG(
     int, verbosity, 0,
     "Verbosity level (0 - silent, 1 - a bit of output, 2+ - more output).")
 COMMON_FLAG(bool, strip_env, 1,

compiler-rt/test/sanitizer_common/TestCases/suffix-log-path_test.c

+// RUN: %clang %s -o %T/suffix-log-path_test-binary
+
+// The glob below requires bash.
+// REQUIRES: shell
+
+// Good log_path with suffix.
+// RUN: rm -f %T/sanitizer.log.*.txt
+// RUN: %env_tool_opts=log_path=%T/sanitizer.log:log_exe_name=1:log_suffix=.txt %run %T/suffix-log-path_test-binary 2> %t.out
+// RUN: FileCheck %s < %T/sanitizer.log.suffix-log-path_test-binary.*.txt
+
+// UNSUPPORTED: ios, android
+
+#include <stdlib.h>
+#include <string.h>
+
+#include <sanitizer/common_interface_defs.h>
+
+int main(int argc, char **argv) {
+  __sanitizer_print_stack_trace();
+  return 0;
+}
+// CHECK: #{{.*}} main