[asan] Optionally print reproducer cmdline in ASan reports.

[m.ostapenko]

Summary

Hi,

for big systems, sometimes it would be nice to extract reproducer cmdline from ASan report to perform further debugging. Actually, sanitizer library already contains necessary functionality and this patch just uses existing GetArgsAndEnv routine to print this information in ASan report destructor.

[ygribov]

lib/asan/asan_report.cc

243 ↗

(On Diff #44515)

This will leak memory (not sure if it's important).

246 ↗

(On Diff #44515)

size_t?

lib/sanitizer_common/sanitizer_common.h

285 ↗

(On Diff #44515)

Stars bind to right.

[m.ostapenko]

lib/asan/asan_report.cc

246 ↗

(On Diff #44515)

size_t is not accessible from ASan code... will use uptr like in many other places.

lib/sanitizer_common/sanitizer_common.h

285 ↗

(On Diff #44515)

This was just copied from sanitizer_linux.cc. Perhaps I should fix stars binding there as well.

[m.ostapenko]

Updating according to Yura's nits.

[kcc]

Will this link on non-linux?
You've inserted a call to GetArgsAndEnv into the main path, while GetArgsAndEnv remains in linux-specific code.

test/asan/TestCases/Posix/print_cmdline.cc

5 ↗

(On Diff #44523)

Why not use the same CHECK prefix for the first two lines?

[m.ostapenko]

Updating according to Kostya's nits. Encapsulate target specific code in GetArgv function, implemented for Linux and Mac. Tested on Linux box. For Mac, GetArgv seems to be trivial, but I'd still prefer OS X guys to take a look. For Windows I left it unimplemented, thought we can use something like GetCommandLine API function to extract Argv.

[filcab]

Why call it a "reproducer", though?
The command line is just one of the various inputs to a program.

Since it isn't guaranteed to "reproduce" the problem, I'd prefer to just remove that word. Something like:

Command line: ...

works perfectly (or even just Command: ...). It also gets rid of the abbreviation in the UI :-)

Thanks!

[m.ostapenko]

Why call it a "reproducer", though?
The command line is just one of the various inputs to a program.

Sounds reasonable, thanks. Updating the patch.

[kcc]

Sounds good, but really this functionality belongs to sanitizer_common.
May I ask you to move it there? (the flags, the print function, etc).
The actual call to PrintCmdline() you may leave in asan only for now.

[m.ostapenko]

Sounds good, but really this functionality belongs to sanitizer_common.
May I ask you to move it there? (the flags, the print function, etc).
The actual call to PrintCmdline() you may leave in asan only for now.

Sure, updating the diff.

[ygribov]

lib/sanitizer_common/sanitizer_flags.inc

204 ↗

(On Diff #44850)

I'm afraid he'd want implementation for other sanitizers as well...

[kcc]

LGTM with a nit

lib/sanitizer_common/sanitizer_common.cc

495 ↗

(On Diff #44850)

either use "argv[i] != nullptr" or just "argv[i]"

lib/sanitizer_common/sanitizer_flags.inc

204 ↗

(On Diff #44850)

Nah, that's fine. As soon as some one needs this functionality in other *san they can add it.
But if the code were in asan adding it to other *san would be a bit more complex.

[m.ostapenko]

Thanks, will commit this patch next week. There is one thing I'm concern about: we call PrintCmdline in ~ScopedInErrorReport and this means that we would miss command line in case of we have nested bugs (here desctructor is never called, right?). Is there any reasonable way to handle this situation (perhaps move PrintCmdline to ScopedInErrorReport constructor)?

compiler-rt/trunk/lib/asan/asan_report.cc

~ScopedInErrorReport() {
     // Make sure the current thread is announced.
     DescribeThread(GetCurrentThread());
     // We may want to grab this lock again when printing stats.
     asanThreadRegistry().Unlock();
     // Print memory stats.
     if (flags()->print_stats)
       __asan_print_accumulated_stats();
 
+    if (common_flags()->print_cmdline)
+      PrintCmdline();
+
     // Copy the message buffer so that we could start logging without holding a
     // lock that gets aquired during printing.
     InternalScopedBuffer<char> buffer_copy(kErrorMessageBufferSize);
     {
       BlockingMutexLock l(&error_message_buf_mutex);
       internal_memcpy(buffer_copy.data(),
                       error_message_buffer, kErrorMessageBufferSize);
     }

compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h

 bool SetEnv(const char *name, const char *value);
 const char *GetPwd();
 char *FindPathToBinary(const char *name);
 bool IsPathSeparator(const char c);
 bool IsAbsolutePath(const char *path);
 
 u32 GetUid();
 void ReExec();
+char **GetArgv();
+void PrintCmdline();
 bool StackSizeIsUnlimited();
 void SetStackSizeLimitInBytes(uptr limit);
 bool AddressSpaceIsUnlimited();
 void SetAddressSpaceUnlimited();
 void AdjustStackSize(void *attr);
 void PrepareForSandboxing(__sanitizer_sandbox_arguments *args);
 void CovPrepareForSandboxing(__sanitizer_sandbox_arguments *args);
 void SetSandboxingCallback(void (*f)());

compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.cc

uptr ReadBinaryNameCached(/*out*/char *buf, uptr buf_len) {
   name_len = (name_len < buf_len - 1) ? name_len : buf_len - 1;
   if (buf_len == 0)
     return 0;
   internal_memcpy(buf, binary_name_cache_str, name_len);
   buf[name_len] = '\0';
   return name_len;
 }
 
+void PrintCmdline() {
+  char **argv = GetArgv();
+  if (!argv) return;
+  Printf("\nCommand: ");
+  for (uptr i = 0; argv[i]; ++i)
+    Printf("%s ", argv[i]);
+  Printf("\n\n");
+}
+
 } // namespace __sanitizer
 
 using namespace __sanitizer;  // NOLINT
 
 extern "C" {
 void __sanitizer_set_report_path(const char *path) {
   report_file.SetReportPath(path);
 }

compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.inc

COMMON_FLAG(int, exitcode, 1, "Override the program exit status if the tool "
                               "found an error")
 COMMON_FLAG(
     bool, abort_on_error, SANITIZER_MAC,
     "If set, the tool calls abort() instead of _exit() after printing the "
     "error report.")
 COMMON_FLAG(bool, suppress_equal_pcs, true,
             "Deduplicate multiple reports for single source location in "
             "halt_on_error=false mode (asan only).")
+COMMON_FLAG(bool, print_cmdline, false, "Print command line on crash "
+            "(asan only).")

compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc

if (buff[i] == 0) {
       CHECK_LE(count, arr_size - 1);  // FIXME: make this more flexible.
       count++;
     }
   }
   (*arr)[count] = nullptr;
 }
 #endif
 
-static void GetArgsAndEnv(char*** argv, char*** envp) {
+static void GetArgsAndEnv(char ***argv, char ***envp) {
 #if !SANITIZER_GO
   if (&__libc_stack_end) {
 #endif
     uptr* stack_end = (uptr*)__libc_stack_end;
     int argc = *stack_end;
     *argv = (char**)(stack_end + 1);
     *envp = (char**)(stack_end + argc + 2);
 #if !SANITIZER_GO
   } else {
     static const int kMaxArgv = 2000, kMaxEnvp = 2000;
     ReadNullSepFileToArray("/proc/self/cmdline", argv, kMaxArgv);
     ReadNullSepFileToArray("/proc/self/environ", envp, kMaxEnvp);
   }
 #endif
 }
 
+char **GetArgv() {
+  char **argv, **envp;
+  GetArgsAndEnv(&argv, &envp);
+  return argv;
+}
+
 void ReExec() {
   char **argv, **envp;
   GetArgsAndEnv(&argv, &envp);
   uptr rv = internal_execve("/proc/self/exe", argv, envp);
   int rverrno;
   CHECK_EQ(internal_iserror(rv, &rverrno), true);
   Printf("execve failed, errno %d\n", rverrno);
   Die();

compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc

void MaybeReexec() {
   // Can't use setenv() here, because it requires the allocator to be
   // initialized.
   // FIXME: instead of filtering DYLD_INSERT_LIBRARIES here, do it in
   // a separate function called after InitializeAllocator().
   if (new_env_pos == new_env + env_name_len + 1) new_env = NULL;
   LeakyResetEnv(kDyldInsertLibraries, new_env);
 }
 
+char **GetArgv() {
+  return *_NSGetArgv();
+}
+
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_MAC

compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc

 void DisableReexec() {
   // No need to re-exec on Windows.
 }
 
 void MaybeReexec() {
   // No need to re-exec on Windows.
 }
 
+char **GetArgv() {
+  // FIXME: Actually implement this function.
+  return 0;
+}
+
 }  // namespace __sanitizer
 
 #endif  // _WIN32

compiler-rt/trunk/test/asan/TestCases/Posix/print_cmdline.cc

+// Check that ASan can print reproducer cmdline for failed binary if desired.
+//
+// RUN: %clang_asan %s -o %t-exe
+//
+// RUN: env not %run %t-exe 2>&1 | FileCheck %s
+// RUN: %env_asan_opts=print_cmdline=false not %run %t-exe 2>&1 | FileCheck %s
+// RUN: %env_asan_opts=print_cmdline=true not %run %t-exe first second/third [fourth] 2>&1 | FileCheck %s --check-prefix CHECK-PRINT
+
+volatile int ten = 10;
+
+int main() {
+  char x[10];
+  // CHECK-NOT: Command:
+  // CHECK-PRINT: {{Command: .*-exe first second/third \[fourth\]}}
+  x[ten] = 1; // BOOM
+  return  0;
+}
+