collect minidump if it chrashes

tools/win_dbghelp.cpp

Index: tools/win_dbghelp.cpp
===================================================================
--- tools/win_dbghelp.cpp	(revision 0)
+++ tools/win_dbghelp.cpp	(revision 0)
@@ -0,0 +1,245 @@
+/*
+ * Copyright 2013 Google Inc.
+ *
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+#ifdef SK_BUILD_FOR_WIN32
+
+#include "windows.h"
+#include "win_dbghelp.h"
+#include <process.h>
+#include <string.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+// CDB.EXE prints a lot of garbage and there is no argument to pass which
+// would remove all that noise.
+// Using eval feature that evaluates a number in hexa and prints it to stdout

[epoger, 2013/03/07 21:09:09]

hexa -> hex

[edisonn, 2013/03/08 15:50:57]

Done.

+// to mark where the callstack is printed.
+
+#define MARKER_THREAD_CALLSTACK_START_NUMBER "12340000"

[epoger, 2013/03/07 21:09:09]

So I guess you just made up these numbers (12340000, 12340001, etc.), right? 
And we would search through the output for them in order to see where callstacks
start and end?

If so, please augment the documentation with something like this...

// For example, each thread's callstack will be marked with "12340001" at
// the beginning and "12340002" at the end.
// We just made up these numbers; they could be anything, as long as they
// match up with their decimal equivalents.

[edisonn, 2013/03/08 15:50:57]

Done.

+#define MARKER_THREAD_CALLSTACK_START "Evaluate expression: 305397760 = 12340000"
+
+#define MARKER_THREAD_CALLSTACK_STOP_NUMBER "12340001"
+#define MARKER_THREAD_CALLSTACK_STOP "Evaluate expression: 305397761 = 12340001"
+
+#define MARKER_EXCEPTION_CALLSTACK_START_NUMBER "12340002"
+#define MARKER_EXCEPTION_CALLSTACK_START "Evaluate expression: 305397762 = 12340002"
+
+#define MARKER_EXCEPTION_CALLSTACK_STOP_NUMBER "12340003"
+#define MARKER_EXCEPTION_CALLSTACK_STOP "Evaluate expression: 305397763 = 12340003"
+
+// k - print stack
+// ? val - evaluarte expression. Used to mark the log file.

[epoger, 2013/03/07 21:09:09]

evaluarte -> evaluate

[edisonn, 2013/03/08 15:50:57]

Done.

+// .ecxr - load exception, if exception was thrown.
+// k - print the resolved stack by .ecxr
+// q - quit cdb.exe
+#define CDB_PRINT_CALLSTACK_CURRENT_THREAD "? " MARKER_THREAD_CALLSTACK_START_NUMBER "; k; ? " MARKER_THREAD_CALLSTACK_STOP_NUMBER "; .ecxr; ? " MARKER_EXCEPTION_CALLSTACK_START_NUMBER "; k; ? " MARKER_EXCEPTION_CALLSTACK_STOP_NUMBER "; q"
+
+char debug_app_name[MAX_PATH] = "";
+void setAppName(const char* app_name) { 
+    if (app_name == NULL) {
+        app_name = "";
+    }
+    strcpy(debug_app_name, app_name);

[epoger, 2013/03/07 21:09:09]

throughout this file: please use stncpy to avoid overflow

Seems like you could create a helper function to do it:

void strncpyOrSetBlank(char* dest, const char* src) {
  const char* srcOrEmptyString = (NULL == src) ? "" : src;
  strncpy(dest, srcOrEmptyString, MAX_PATH);
}

char debug_app_name[MAX_PATH] = "";
void setAppName(const char* app_name) { 
  strncpyOrSetBlank(debug_app_name, app_name);
}

[edisonn, 2013/03/08 15:50:57]

Done.

+}
+
+const char* getAppName() {
+    return debug_app_name;
+}
+
+char debug_binaries_path[MAX_PATH] = "";
+void setBinariesPath(const char* binaries_path) {
+    if (binaries_path == NULL) {
+        binaries_path = "";
+    }
+    strcpy(debug_binaries_path, binaries_path);
+}
+
+const char* getBinariesPath() {
+    return debug_binaries_path;
+}
+
+char debug_app_version[100] = "";
+void setAppVersion(const char* version) {
+    if (version == NULL) {
+        version = "";
+    }
+    strcpy(debug_app_version, version);
+}
+
+const char* getAppVersion() {
+    return debug_app_version;
+}
+
+char debug_cdb_path[MAX_PATH] = "";
+void setCdbPath(const char* path) {
+  if (path == NULL) {
+      path = "";
+  }
+  strcpy(debug_cdb_path, path);
+}
+
+const char* getCdbPath() {
+    return debug_cdb_path;
+}
+
+// Not the most performant code, but this will be used only to collect
+// the callstack from a log files, only when the application had failed.

[epoger, 2013/03/07 21:09:09]

Please give more information about WHAT this function actually does.  What you
have noted here is more of an implementation detail, which is good to document
but should probably be done INSIDE the function.

[edisonn, 2013/03/08 15:50:57]

Done.

+static void printCallstack(const char* filename,
+                           const char* start,
+                           const char* stop) {
+    FILE* file = fopen(filename, "rt");
+    char line[1000];
+    bool started = false;
+    while (fgets(line, sizeof(line), file)) {

[epoger, 2013/03/07 21:09:09]

Do we need to worry about lines longer than 1000 characters?  What happens if
this code encounters one?

[edisonn, 2013/03/08 15:50:57]

We will print the frame truncated, or not print it at all. I am not worried bu
1000 character function/dll/exe names.

On 2013/03/07 21:09:09, epoger wrote:

+        if (!started && strncmp(start, line, strlen(start)) == 0) {
+            started = true;
+        } else if (started && strncmp(stop, line, strlen(stop)) == 0) {
+            break;
+        } else if (started) {
+            // Filter messages. Calstack lines contain "exe/dll!function"

[epoger, 2013/03/07 21:09:09]

Calstack -> Callstack

More importantly, I'm confused about what form these lines take.  Do they show
"exe/dll!function" or "ffffeeee ffffdddd!function" ?

Please replace magic number 18 with some sample string(s) constant and strlen
math.

[edisonn, 2013/03/08 15:50:57]

lines are like "ffffeeee ffffdddd render_pdf!processInput"

+done
On 2013/03/07 21:09:09, epoger wrote:

+            if (strchr(line, '!') != NULL && strlen(line) > 18) {
+                // Remove addresses. 18 = 2 * (8 digit hexa + 1 space).
+                printf("%s", line + 18);  // fgets includes \n already.
+            }
+        }
+    }
+    fclose(file);
+}
+
+int GenerateDumpAndPrintCallstack(EXCEPTION_POINTERS* pExceptionPointers) {

[epoger, 2013/03/07 21:09:09]

please document meaning of return value

[edisonn, 2013/03/08 15:50:57]

Done.

+    BOOL bMiniDumpSuccessful;
+    char szPath[MAX_PATH]; 
+    char szFileName[MAX_PATH]; 
+    char szFileNameOutput[MAX_PATH]; 
+    const char* szAppName = getAppName();
+    const char* szVersion = getAppVersion();
+    DWORD dwBufferSize = MAX_PATH;
+    HANDLE hDumpFile;
+    SYSTEMTIME stLocalTime;
+    MINIDUMP_EXCEPTION_INFORMATION ExpParam;
+
+    GetLocalTime( &stLocalTime );
+    GetTempPath( dwBufferSize, szPath );
+
+    sprintf( szFileName, "%s%s", szPath, szAppName );
+    CreateDirectory( szFileName, NULL );
+
+    sprintf( szFileName, "%s%s\\%s-%04d%02d%02d-%02d%02d%02d-%ld-%ld.dmp", 
+               szPath, szAppName, szVersion, 
+               stLocalTime.wYear, stLocalTime.wMonth, stLocalTime.wDay, 
+               stLocalTime.wHour, stLocalTime.wMinute, stLocalTime.wSecond, 
+               GetCurrentProcessId(), GetCurrentThreadId());
+
+    sprintf( szFileNameOutput, "%s%s\\%s-%04d%02d%02d-%02d%02d%02d-%ld-%ld.out", 

[epoger, 2013/03/07 21:09:09]

It looks like this formatting string has a lot in common with the one above. 
Maybe build them as preprocessor macros?

[edisonn, 2013/03/08 15:50:57]

Done.

+               szPath, szAppName, szVersion, 
+               stLocalTime.wYear, stLocalTime.wMonth, stLocalTime.wDay, 
+               stLocalTime.wHour, stLocalTime.wMinute, stLocalTime.wSecond, 
+               GetCurrentProcessId(), GetCurrentThreadId());
+
+    hDumpFile = CreateFile(szFileName,
+                           GENERIC_READ|GENERIC_WRITE, 
+                           FILE_SHARE_WRITE|FILE_SHARE_READ,
+                           0,
+                           CREATE_ALWAYS,
+                           0,
+                           0);
+
+    ExpParam.ThreadId = GetCurrentThreadId();
+    ExpParam.ExceptionPointers = pExceptionPointers;
+    ExpParam.ClientPointers = TRUE;
+
+    bMiniDumpSuccessful = MiniDumpWriteDump(GetCurrentProcess(),
+                                            GetCurrentProcessId(), 
+                                            hDumpFile,
+                                            MiniDumpWithDataSegs,
+                                            &ExpParam,
+                                            NULL,
+                                            NULL);
+
+    printf("MiniDump file:    %s\n", szFileName);
+    printf("App exe and pdb:  %s\n", getBinariesPath());
+
+    const char* cdbExePath = getCdbPath();
+    if (cdbExePath && *cdbExePath != '\0') {
+        printf("Cdb exe:          %s\n", cdbExePath);
+      
+        char command[MAX_PATH * 4];
+        sprintf(command, "%s -y \"%s\" -i \"%s\" -z \"%s\" -c \"%s\" -kqm >\"%s\"",
+                cdbExePath,
+                getBinariesPath(),
+                getBinariesPath(),
+                szFileName,
+                CDB_PRINT_CALLSTACK_CURRENT_THREAD,
+                szFileNameOutput);
+        system(command);
+
+        printf("\nThread Callstack:\n");
+        printCallstack(szFileNameOutput, 
+                       MARKER_THREAD_CALLSTACK_START,
+                       MARKER_THREAD_CALLSTACK_STOP);
+
+        printf("\nException Callstack:\n");        
+        printCallstack(szFileNameOutput,
+                       MARKER_EXCEPTION_CALLSTACK_START,
+                       MARKER_EXCEPTION_CALLSTACK_STOP);
+    } else {
+        printf("Warning: CDB path not set up.\n");        
+    }
+
+    return EXCEPTION_EXECUTE_HANDLER;
+}
+
+/** Sets the debugging variables. Input parameter is app location.
+ *  e.g out\Debug\render_pdfs.exe
+ *  This function expects the .pdb file to be in the same directory.
+ */
+void setUpDebuggingFromArgs(const char* vargs0) {
+    int i = strlen(vargs0);
+
+    if (i >= 4 && 
+            (vargs0[i - 1] == 'E' || vargs0[i - 1] == 'e') &&

[epoger, 2013/03/07 21:09:09]

on windows, I think you can use stricmp()

http://msdn.microsoft.com/en-us/library/k59z8dwe(v=vs.80).aspx

[edisonn, 2013/03/08 15:50:57]

Done.

+            (vargs0[i - 2] == 'X' || vargs0[i - 2] == 'x') &&
+            (vargs0[i - 3] == 'E' || vargs0[i - 3] == 'e') &&
+            vargs0[i - 4] == '.') {
+        // Ignore .exe
+        i -= 4;
+    }
+
+    int pos_period = i;
+
+    // Find last \ in path - this is Windows!
+    while (i >= 0 && vargs0[i] != '\\') {
+        i--;
+    }
+
+    int pos_last_slash = i;
+
+    char app_name[MAX_PATH];
+    strncpy(app_name, vargs0 + pos_last_slash + 1, pos_period - pos_last_slash - 1);
+    app_name[pos_period - pos_last_slash] = '\0';
+    setAppName(app_name);
+
+    char binaries_path[MAX_PATH];
+    strncpy(binaries_path, vargs0, pos_last_slash);
+    binaries_path[pos_last_slash] = '\0';
+    setBinariesPath(binaries_path);
+
+    setAppVersion("1.0");  // Dummy for now, but use revision instead if we use
+                           // the minidump for anything else other than
+                           // collecting the callstack.
+
+    // cdb.exe is the app used to load the minidump which prints the callstack.
+    char cdbExePath[MAX_PATH];
+#ifdef _WIN64
+    sprintf(cdbExePath, "%s\\x64\\cdb.exe", SK_CDB_PATH);
+#else
+    sprintf(cdbExePath, "%s\\cdb.exe", SK_CDB_PATH);
+#endif
+    setCdbPath(cdbExePath);
+}
+
+#endif  // SK_BUILD_FOR_WIN32