Replace PrintStackTrace(FILE*) with PrintStackTrace(raw_ostream&)

[zturner]

Summary

A FILE* method isn't as useful as an overload that takes an llvm::raw_ostream&. This way we can use raw_fd_ostream if we want the old behavior, or more commonly just llvm::errs() if we want to write to stderr. Plus, this adds the flexibility necessary to save the results to a string if we want to tee them to a log file as well as stderr, for example.

Note: This patch is currently untested on Linux and Mac. There should be no functional change, but I need to at least compile on Linux / Mac to make sure it's not broken. I plan to do this tomorrow. I'm mostly looking for comments on the general idea, because I don't expect this to be too controversial.

There is only 1 existing callsite of the FILE* method, which is in clang/CIndex.cpp. It calls PrintStackTrace(stderr). I will fix this to use PrintStackTrace(llvm::errs()) in a separate patch.

[rnk]

There is a functional difference between raw_ostream and FILE*. raw_fd_ostreams have their own buffering that doesn't mesh well with the buffering done with the C stdio implementation. I think CIndex.cpp and all that code makes sure to print with C stdio. We should be able to resolve this pretty easily by creating a raw_file_ostream adapter class, though.

[zturner]

I don't think it matters actually. The place in CIndex that was using this
was in cindex::Logger::~Logger(). It was using the `stderr' FILE*. stderr
is unbuffered for one thing, but anwyay as long as we change the rest of
the function to use llvm::errs() then everything should be fine.

[zturner]

Actually even better, it was *already* using llvm::errs() except for the
call to this function, where it passed in llvm::errs(). So this is
actually an improvement to the function, even if only a theoretical
improvement.

llvm/trunk/include/llvm/Support/Signals.h

 
 #ifndef LLVM_SUPPORT_SIGNALS_H
 #define LLVM_SUPPORT_SIGNALS_H
 
 #include "llvm/Support/Path.h"
 #include <cstdio>
 
 namespace llvm {
+  class raw_ostream;
+
 namespace sys {
 
   /// This function runs all the registered interrupt handlers, including the
   /// removal of files registered by RemoveFileOnSignal.
   void RunInterruptHandlers();
 
   /// This function registers signal handlers to ensure that if a signal gets
   /// delivered that the named file is removed.
   /// @brief Remove a file if a fatal signal occurs.
   bool RemoveFileOnSignal(StringRef Filename, std::string* ErrMsg = nullptr);
 
   /// This function removes a file from the list of files to be removed on
   /// signal delivery.
   void DontRemoveFileOnSignal(StringRef Filename);
 
   /// When an error signal (such as SIBABRT or SIGSEGV) is delivered to the
   /// process, print a stack trace and then exit.
   /// @brief Print a stack trace if a fatal signal occurs.
   void PrintStackTraceOnErrorSignal();
 
   /// Disable all system dialog boxes that appear when the process crashes.
   void DisableSystemDialogsOnCrash();
 
-  /// \brief Print the stack trace using the given \c FILE object.
-  void PrintStackTrace(FILE *);
+  /// \brief Print the stack trace using the given \c raw_ostream object.
+  void PrintStackTrace(raw_ostream &OS);
 
   /// AddSignalHandler - Add a function to be called when an abort/kill signal
   /// is delivered to the process.  The handler can have a cookie passed to it
   /// to identify what instance of the handler it is.
   void AddSignalHandler(void (*FnPtr)(void *), void *Cookie);
 
   /// This function registers a function to be called when the user "interrupts"
   /// the program (typically by pressing ctrl-c).  When the user interrupts the

llvm/trunk/lib/Support/Unix/Signals.inc

 //===- Signals.cpp - Generic Unix Signals Implementation -----*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines some helpful functions for dealing with the possibility of
 // Unix signals occurring while your program is running.
 //
 //===----------------------------------------------------------------------===//
 
 #include "Unix.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Format.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Mutex.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/UniqueLock.h"
 #include "llvm/Support/raw_ostream.h"

 #else
 static bool findModulesAndOffsets(void **StackTrace, int Depth,
                                   const char **Modules, intptr_t *Offsets,
                                   const char *MainExecutableName) {
   return false;
 }
 #endif
 
-static bool printSymbolizedStackTrace(void **StackTrace, int Depth, FILE *FD) {
+static bool printSymbolizedStackTrace(void **StackTrace, int Depth,
+                                      llvm::raw_ostream &OS) {
   // FIXME: Subtract necessary number from StackTrace entries to turn return addresses
   // into actual instruction addresses.
   // Use llvm-symbolizer tool to symbolize the stack traces.
   ErrorOr<std::string> LLVMSymbolizerPathOrErr =
       sys::findProgramByName("llvm-symbolizer");
   if (!LLVMSymbolizerPathOrErr)
     return false;
   const std::string &LLVMSymbolizerPath = *LLVMSymbolizerPathOrErr;
if (!OutputBuf)
     return false;
   StringRef Output = OutputBuf.get()->getBuffer();
   SmallVector<StringRef, 32> Lines;
   Output.split(Lines, "\n");
   auto CurLine = Lines.begin();
   int frame_no = 0;
   for (int i = 0; i < Depth; i++) {
     if (!Modules[i]) {
-      fprintf(FD, "#%d %p\n", frame_no++, StackTrace[i]);
+      OS << format("#%d %p\n", frame_no++, StackTrace[i]);
       continue;
     }
     // Read pairs of lines (function name and file/line info) until we
     // encounter empty line.
     for (;;) {
       if (CurLine == Lines.end())
         return false;
       StringRef FunctionName = *CurLine++;
       if (FunctionName.empty())
         break;
-      fprintf(FD, "#%d %p ", frame_no++, StackTrace[i]);
+      OS << format("#%d %p ", frame_no++, StackTrace[i]);
       if (!FunctionName.startswith("??"))
-        fprintf(FD, "%s ", FunctionName.str().c_str());
+        OS << format("%s ", FunctionName.str().c_str());
       if (CurLine == Lines.end())
         return false;
       StringRef FileLineInfo = *CurLine++;
       if (!FileLineInfo.startswith("??"))
-        fprintf(FD, "%s", FileLineInfo.str().c_str());
+        OS << format("%s", FileLineInfo.str().c_str());
       else
-        fprintf(FD, "(%s+%p)", Modules[i], (void *)Offsets[i]);
-      fprintf(FD, "\n");
+        OS << format("(%s+%p)", Modules[i], (void *)Offsets[i]);
+      OS << "\n";
     }
   }
   return true;
 }
 #endif // defined(HAVE_BACKTRACE) && defined(ENABLE_BACKTRACES)
 
 // PrintStackTrace - In the case of a program crash or fault, print out a stack
 // trace so that the user has an indication of why and where we died.
 //
 // On glibc systems we have the 'backtrace' function, which works nicely, but
 // doesn't demangle symbols.
-void llvm::sys::PrintStackTrace(FILE *FD) {
+void llvm::sys::PrintStackTrace(raw_ostream &OS) {
 #if defined(HAVE_BACKTRACE) && defined(ENABLE_BACKTRACES)
   static void* StackTrace[256];
   // Use backtrace() to output a backtrace on Linux systems with glibc.
   int depth = backtrace(StackTrace,
                         static_cast<int>(array_lengthof(StackTrace)));
-  if (printSymbolizedStackTrace(StackTrace, depth, FD))
+  if (printSymbolizedStackTrace(StackTrace, depth, OS))
     return;
 #if HAVE_DLFCN_H && __GNUG__
   int width = 0;
   for (int i = 0; i < depth; ++i) {
     Dl_info dlinfo;
     dladdr(StackTrace[i], &dlinfo);
     const char* name = strrchr(dlinfo.dli_fname, '/');
 
     int nwidth;
     if (!name) nwidth = strlen(dlinfo.dli_fname);
     else       nwidth = strlen(name) - 1;
 
     if (nwidth > width) width = nwidth;
   }
 
   for (int i = 0; i < depth; ++i) {
     Dl_info dlinfo;
     dladdr(StackTrace[i], &dlinfo);
 
-    fprintf(FD, "%-2d", i);
+    OS << format("%-2d", i);
 
     const char* name = strrchr(dlinfo.dli_fname, '/');
-    if (!name) fprintf(FD, " %-*s", width, dlinfo.dli_fname);
-    else       fprintf(FD, " %-*s", width, name+1);
+    if (!name) OS << format(" %-*s", width, dlinfo.dli_fname);
+    else       OS << format(" %-*s", width, name+1);
 
-    fprintf(FD, " %#0*lx",
-            (int)(sizeof(void*) * 2) + 2, (unsigned long)StackTrace[i]);
+    OS << format(" %#0*lx", (int)(sizeof(void*) * 2) + 2,
+                 (unsigned long)StackTrace[i]);
 
     if (dlinfo.dli_sname != nullptr) {
-      fputc(' ', FD);
+      OS << ' ';
 #  if HAVE_CXXABI_H
       int res;
       char* d = abi::__cxa_demangle(dlinfo.dli_sname, nullptr, nullptr, &res);
 #  else
       char* d = NULL;
 #  endif
-      if (!d) fputs(dlinfo.dli_sname, FD);
-      else    fputs(d, FD);
+      if (!d) OS << dlinfo.dli_sname;
+      else    OS << d;
       free(d);
 
       // FIXME: When we move to C++11, use %t length modifier. It's not in
       // C++03 and causes gcc to issue warnings. Losing the upper 32 bits of
       // the stack offset for a stack dump isn't likely to cause any problems.
-      fprintf(FD, " + %u",(unsigned)((char*)StackTrace[i]-
+      OS << format(" + %u",(unsigned)((char*)StackTrace[i]-
                                       (char*)dlinfo.dli_saddr));
     }
-    fputc('\n', FD);
+    OS << '\n';
   }
 #else
   backtrace_symbols_fd(StackTrace, depth, STDERR_FILENO);
 #endif
 #endif
 }
 
 static void PrintStackTraceSignalHandler(void *) {
-  PrintStackTrace(stderr);
+  PrintStackTrace(llvm::errs());
 }
 
 void llvm::sys::DisableSystemDialogsOnCrash() {}
 
 /// PrintStackTraceOnErrorSignal - When an error signal (such as SIGABRT or
 /// SIGSEGV) is delivered to the process, print a stack trace and then exit.
 void llvm::sys::PrintStackTraceOnErrorSignal() {
   AddSignalHandler(PrintStackTraceSignalHandler, nullptr);

llvm/trunk/lib/Support/Windows/Signals.inc

 //
 //===----------------------------------------------------------------------===//
 #include "llvm/Support/FileSystem.h"
 #include <algorithm>
 #include <signal.h>
 #include <stdio.h>
 #include <vector>
 
+#include "llvm/Support/Format.h"
+#include "llvm/Support/raw_ostream.h"
+
 // The Windows.h header must be after LLVM and standard headers.
 #include "WindowsSupport.h"
 
 #ifdef __MINGW32__
  #include <imagehlp.h>
 #else
  #include <dbghelp.h>
 #endif

 static bool CleanupExecuted = false;
 static PTOP_LEVEL_EXCEPTION_FILTER OldFilter = NULL;
 
 // Windows creates a new thread to execute the console handler when an event
 // (such as CTRL/C) occurs.  This causes concurrency issues with the above
 // globals which this critical section addresses.
 static CRITICAL_SECTION CriticalSection;
 
-static void PrintStackTraceForThread(FILE *File, HANDLE hProcess,
+static void PrintStackTraceForThread(llvm::raw_ostream &OS, HANDLE hProcess,
                                      HANDLE hThread, STACKFRAME64 &StackFrame,
                                      CONTEXT *Context) {
   DWORD machineType;
 #if defined(_M_X64)
   machineType = IMAGE_FILE_MACHINE_AMD64;
 #else
   machineType = IMAGE_FILE_MACHINE_I386;
 #endif
 
   // Initialize the symbol handler.
   SymSetOptions(SYMOPT_DEFERRED_LOADS | SYMOPT_LOAD_LINES);
   SymInitialize(hProcess, NULL, TRUE);
 
   while (true) {
     if (!StackWalk64(machineType, hProcess, hThread, &StackFrame, Context, NULL,
                      SymFunctionTableAccess64, SymGetModuleBase64, NULL)) {
       break;
     }
 
     if (StackFrame.AddrFrame.Offset == 0)
       break;
 
+    using namespace llvm;
     // Print the PC in hexadecimal.
     DWORD64 PC = StackFrame.AddrPC.Offset;
 #if defined(_M_X64)
-    fprintf(File, "0x%016llX", PC);
+    OS << format("0x%016llX", PC);
 #elif defined(_M_IX86)
-    fprintf(File, "0x%08lX", static_cast<DWORD>(PC));
+    OS << format("0x%08lX", static_cast<DWORD>(PC));
 #endif
 
 // Print the parameters.  Assume there are four.
 #if defined(_M_X64)
-    fprintf(File, " (0x%016llX 0x%016llX 0x%016llX 0x%016llX)",
+    OS << format(" (0x%016llX 0x%016llX 0x%016llX 0x%016llX)",
             StackFrame.Params[0], StackFrame.Params[1], StackFrame.Params[2],
             StackFrame.Params[3]);
 #elif defined(_M_IX86)
-    fprintf(File, " (0x%08lX 0x%08lX 0x%08lX 0x%08lX)",
+    OS << format(" (0x%08lX 0x%08lX 0x%08lX 0x%08lX)",
             static_cast<DWORD>(StackFrame.Params[0]),
             static_cast<DWORD>(StackFrame.Params[1]),
             static_cast<DWORD>(StackFrame.Params[2]),
             static_cast<DWORD>(StackFrame.Params[3]));
 #endif
     // Verify the PC belongs to a module in this process.
     if (!SymGetModuleBase64(hProcess, PC)) {
-      fputs(" <unknown module>\n", File);
+      OS << " <unknown module>\n";
       continue;
     }
 
     // Print the symbol name.
     char buffer[512];
     IMAGEHLP_SYMBOL64 *symbol = reinterpret_cast<IMAGEHLP_SYMBOL64 *>(buffer);
     memset(symbol, 0, sizeof(IMAGEHLP_SYMBOL64));
     symbol->SizeOfStruct = sizeof(IMAGEHLP_SYMBOL64);
     symbol->MaxNameLength = 512 - sizeof(IMAGEHLP_SYMBOL64);
 
     DWORD64 dwDisp;
     if (!SymGetSymFromAddr64(hProcess, PC, &dwDisp, symbol)) {
-      fputc('\n', File);
+      OS << '\n';
       continue;
     }
 
     buffer[511] = 0;
     if (dwDisp > 0)
-      fprintf(File, ", %s() + 0x%llX bytes(s)", symbol->Name, dwDisp);
+      OS << format(", %s() + 0x%llX bytes(s)", (const char*)symbol->Name,
+                   dwDisp);
     else
-      fprintf(File, ", %s", symbol->Name);
+      OS << format(", %s", (const char*)symbol->Name);
 
     // Print the source file and line number information.
     IMAGEHLP_LINE64 line;
     DWORD dwLineDisp;
     memset(&line, 0, sizeof(line));
     line.SizeOfStruct = sizeof(line);
     if (SymGetLineFromAddr64(hProcess, PC, &dwLineDisp, &line)) {
-      fprintf(File, ", %s, line %lu", line.FileName, line.LineNumber);
+      OS << format(", %s, line %lu", line.FileName, line.LineNumber);
       if (dwLineDisp > 0)
-        fprintf(File, " + 0x%lX byte(s)", dwLineDisp);
+        OS << format(" + 0x%lX byte(s)", dwLineDisp);
     }
 
-    fputc('\n', File);
+    OS << '\n';
   }
 }
 
 namespace llvm {
 
 //===----------------------------------------------------------------------===//
 //=== WARNING: Implementation here must contain only Win32 specific code
 //===          and must not be UNIX code

 /// PrintStackTraceOnErrorSignal - When an error signal (such as SIBABRT or
 /// SIGSEGV) is delivered to the process, print a stack trace and then exit.
 void sys::PrintStackTraceOnErrorSignal() {
   DisableSystemDialogsOnCrash();
   RegisterHandler();
   LeaveCriticalSection(&CriticalSection);
 }
 
-void llvm::sys::PrintStackTrace(FILE *File) {
+void llvm::sys::PrintStackTrace(raw_ostream &OS) {
 
   STACKFRAME64 StackFrame = {};
   CONTEXT Context = {0};
   ::RtlCaptureContext(&Context);
 #if defined(_M_X64)
   StackFrame.AddrPC.Offset = Context.Rip;
   StackFrame.AddrStack.Offset = Context.Rsp;
   StackFrame.AddrFrame.Offset = Context.Rbp;
 #else
   StackFrame.AddrPC.Offset = Context.Eip;
   StackFrame.AddrStack.Offset = Context.Esp;
   StackFrame.AddrFrame.Offset = Context.Ebp;
 #endif
   StackFrame.AddrPC.Mode = AddrModeFlat;
   StackFrame.AddrStack.Mode = AddrModeFlat;
   StackFrame.AddrFrame.Mode = AddrModeFlat;
-  PrintStackTraceForThread(File, GetCurrentProcess(), GetCurrentThread(),
+  PrintStackTraceForThread(OS, GetCurrentProcess(), GetCurrentThread(),
                            StackFrame, &Context);
 }
 
 
 void sys::SetInterruptFunction(void (*IF)()) {
   RegisterHandler();
   InterruptFunction = IF;
   LeaveCriticalSection(&CriticalSection);
#elif defined(_M_IX86)
   StackFrame.AddrStack.Offset = ep->ContextRecord->Esp;
   StackFrame.AddrStack.Mode = AddrModeFlat;
   StackFrame.AddrFrame.Offset = ep->ContextRecord->Ebp;
   StackFrame.AddrFrame.Mode = AddrModeFlat;
 #endif
 
   HANDLE hProcess = GetCurrentProcess();
   HANDLE hThread = GetCurrentThread();
-  PrintStackTraceForThread(stderr, hProcess, hThread, StackFrame,
+  PrintStackTraceForThread(llvm::errs(), hProcess, hThread, StackFrame,
                            ep->ContextRecord);
 
   _exit(ep->ExceptionRecord->ExceptionCode);
 }
 
 static BOOL WINAPI LLVMConsoleCtrlHandler(DWORD dwCtrlType) {
   // We are running in our very own thread, courtesy of Windows.
   EnterCriticalSection(&CriticalSection);