Convert trace-malloc to using XPCOM stack walking API. b=374829 r+a=brendan

tools/trace-malloc/lib/Makefile.in

@@ -55,17 +55,15 @@ CSRCS		= \
 		$(NULL)
 
 CPPSRCS		= \
-		nsDemangle.cpp \
 		nsTypeInfo.cpp \
 		$(NULL)
 
 EXPORTS         = nsTraceMalloc.h
 
-EXTRA_DSO_LDOPTS = $(NSPR_LIBS)
+EXTRA_DSO_LDOPTS = $(NSPR_LIBS) $(XPCOM_LIBS)
 
 ifeq ($(OS_ARCH),WINNT)
-CPPSRCS		+= nsDebugHelpWin32.cpp nsWinTraceMalloc.cpp nsStackFrameWin.cpp
-EXTRA_DSO_LDOPTS += $(XPCOM_LIBS)
+CPPSRCS		+= nsDebugHelpWin32.cpp nsWinTraceMalloc.cpp
 OS_LIBS		+= shell32.lib ole32.lib uuid.lib imagehlp.lib
 endif
 

tools/trace-malloc/lib/nsDebugHelpWin32.cpp

@@ -48,7 +48,6 @@
 #include "prlock.h"
 #include "nscore.h"
 #include "nsDebugHelpWin32.h"
-#include "nsStackFrameWin.h"
 #else
 #error "nsDebugHelpWin32.cpp should only be built in Win32 x86 builds"
 #endif
@@ -164,7 +163,6 @@ DHWImportHooker::DHWImportHooker(const char* aModuleName,
         gLock = PR_NewLock();
     PR_Lock(gLock);
 
-    EnsureImageHlpInitialized();
     dhwEnsureImageHlpInitialized(); // for the extra ones we care about.
 
     if(!gRealGetProcAddress)

tools/trace-malloc/lib/nsWinTraceMalloc.cpp

@@ -7,122 +7,9 @@
 #include "nscore.h"
 #include "nsAutoLock.h"
 
-#include "nsStackFrameWin.h"
 #include "nsDebugHelpWin32.h"
 #include "nsTraceMallocCallbacks.h"
 
-// XXX These are *very* quick hacks and need improvement!
-
-static PRBool GetSymbolFromAddress(uint32 addr, char* outBuf)
-{
-    PRBool ok;
-    ok = EnsureSymInitialized();
-    if(!ok)
-        return ok;
-
-    char buf[sizeof(IMAGEHLP_SYMBOL) + 512];
-    PIMAGEHLP_SYMBOL symbol = (PIMAGEHLP_SYMBOL) buf;
-    symbol->SizeOfStruct = sizeof(buf);
-    symbol->MaxNameLength = 512;
-
-    DWORD displacement;
-
-    ok = SymGetSymFromAddr(::GetCurrentProcess(),
-                           addr,
-                           &displacement,
-                           symbol);
-
-    if(ok)
-    {
-        char buf2[512];
-        sprintf(buf2, "%s+0x%08X", symbol->Name, displacement);
-        strcat(outBuf, buf2);
-    }
-    else
-        strcat(outBuf, "dunno");
-    return ok;
-}
-
-static PRBool GetModuleFromAddress(uint32 addr, char* outBuf)
-{
-    PRBool ok;
-    ok = EnsureSymInitialized();
-    if(!ok)
-        return ok;
-
-
-    IMAGEHLP_MODULE module;
-    module.SizeOfStruct = sizeof(IMAGEHLP_MODULE);
-
-    ok = SymGetModuleInfo(::GetCurrentProcess(),
-                          addr,
-                          &module);
-
-    if(ok)
-        strcat(outBuf, module.ModuleName);
-    else
-        strcat(outBuf, "dunno");
-    return ok;
-}
-
-/***************************************************************************/
-#ifdef VERBOSE
-#define SHOW(x_, buf_) \
-    printf(#x_" = %#x... %s\n", x_,             \
-    (buf_[0] = 0,                               \
-     GetModuleFromAddress((uint32)x_, buf_),    \
-     strcat(buf," :: "),                        \
-     GetSymbolFromAddress((uint32)x_, buf_),    \
-     buf_));
-#else
-#define SHOW(x_, buf_)
-#endif  //VERBOSE
-
-
-/***************************************************************************/
-
-#ifdef VERBOSE
-// XXX This is a quick hack to show that x86 Win32 stack walking can be done
-// with this sort of loop following the bp. 
-
-void dumpStack()
-{
-    uint32* bp_;
-    uint32* bpdown;
-    uint32  pc;
-
-    char buf[512];
-
-    _asm { mov bp_ , ebp }
-
-    /* Stack walking code adapted from Kipp's "leaky". */
-    while (1) {
-        bpdown = (uint32*) *bp_++;
-        pc = *bp_;
-        // These addresses are iffy...
-        if (pc < 0x00400000 || pc > 0x7fffffff || bpdown < bp_)
-            break;
-        SHOW(pc, buf);
-        bp_ = bpdown;
-    }
-    printf("\n");
-}
-#endif
-
-char* _stdcall call2(void* v)
-{
-//    dumpStack();
-//    return 0; 
-
-    return (char *)malloc(123);   
-}
-
-int call1(char c, int i, double d, ... )
-{
-    free(call2(0));
-    return 0;    
-}
-
 /***************************************************************************/
 // shows how to use the dhw stuff to hook imported functions
 
@@ -239,93 +126,10 @@ void __cdecl dhw_delete(void* p)
     FreeCallback(p, start, end);
 }
 
-
-
-
-/***************************************************************************/
-// A demonstration of using the _CrtSetAllocHook based hooking.
-// This system sucks because you don't get to see the allocated pointer.
-#if 0
-class myAllocationSizePrinter : public DHWAllocationSizeDebugHook
-{
-public:
-    PRBool AllocHook(size_t size)
-    {
-        alloc_calls++ ;
-        total_mem += size;
-        if(verbosity)
-        {
-            printf("alloc called to get %d bytes.\n", size);
-            dumpStack();
-        }
-        return PR_TRUE;
-    }
-
-    PRBool ReallocHook(size_t size, size_t sizeOld)
-    {
-        realloc_calls++ ;
-        total_mem += size;
-        total_mem -= sizeOld;
-        if(verbosity)
-        {
-            printf("realloc called to size to %d bytes. Old size: %d.\n",
-                   size, sizeOld);
-            dumpStack();
-        }
-        return PR_TRUE;
-    }
-
-    PRBool FreeHook(size_t size)
-    {
-        free_calls++ ;
-        total_mem -= size;
-        if(verbosity)
-        {
-            printf("free called to release %d bytes.\n", size);
-            dumpStack();
-        }
-        return PR_TRUE;
-    }
-
-    myAllocationSizePrinter(int v)
-        : verbosity(v),
-          alloc_calls(0),
-          realloc_calls(0),
-          free_calls(0),
-          total_mem(0) {}
-    virtual ~myAllocationSizePrinter(){}
-
-    void report()
-    {
-        printf("%d allocs, %d reallocs, %d frees, %d bytes leaked\n",
-               alloc_calls, realloc_calls, free_calls, total_mem);
-    }
-
-private:
-    void dumpStack()
-    {
-        if(verbosity == 2)
-            ::dumpStack();
-    }
- 
-    int verbosity;
-    int alloc_calls;
-    int realloc_calls;
-    int free_calls;
-    size_t total_mem;
-};
-#endif
-
-
-
-
 /*C Callbacks*/
 PR_IMPLEMENT(void)
 StartupHooker()
 {
-  if (!EnsureSymInitialized())
-    return;
-
   //run through get all hookers
   DHWImportHooker &loadlibraryW = DHWImportHooker::getLoadLibraryWHooker();
   DHWImportHooker &loadlibraryExW = DHWImportHooker::getLoadLibraryExWHooker();
@@ -344,50 +148,3 @@ PR_IMPLEMENT(void)
 ShutdownHooker()
 {
 }
-
-
-
-#if 0
-
-int main()
-{
-    // A demonstration of using the (sucky) _CrtSetAllocHook based hooking.
-    myAllocationSizePrinter ap(0);
-    dhwSetAllocationSizeDebugHook(&ap);
-
-    // show that the ImportHooker is hooking calls from loaded dll
-    DHW_DECLARE_FUN_TYPE(void, __stdcall, SOMECALL_, (void));
-    HMODULE module = ::LoadLibrary("Other.dll");
-    if(module) {
-        SOMECALL_ _SomeCall = (SOMECALL_) GetProcAddress(module, "SomeCall");
-        if(_SomeCall)
-            _SomeCall(); 
-    }
-
-    // show that the ImportHooker is hooking sneaky calls made from this dll.
-    HMODULE module2 = ::LoadLibrary(NS_DEBUG_CRT);
-    if(module2) {
-        MALLOC_ _sneakyMalloc = (MALLOC_) GetProcAddress(module2, "malloc");
-        if(_sneakyMalloc)
-        {
-            void* p = _sneakyMalloc(987); 
-            free(p);
-        }
-    }
-
-    call1('x', 1, 1.0, "hi there", 2);
-
-    char* p = new char[10];
-    delete p;
-
-    void* v = malloc(10);
-    v = realloc(v, 15);
-    v = realloc(v, 5);
-    free(v);`
-
-    ap.report();
-    dhwClearAllocationSizeDebugHook();
-    return 0;
-}
-#endif //0
-

xpcom/base/nsStackFrameWin.cpp

@@ -137,9 +137,8 @@ void PrintError(char *prefix)
       0,
       NULL
     );
-    char buf[512];
-    _snprintf(buf, sizeof(buf), "### ERROR: %s: %s", prefix, lpMsgBuf);
-    fputs(buf, stderr);
+    fprintf(stderr, "### ERROR: %s: %s", prefix, lpMsgBuf);
+    fflush(stderr);
     LocalFree( lpMsgBuf );
 }
 
@@ -448,8 +447,7 @@ EXPORT_XPCOM_API(nsresult)
 NS_StackWalk(NS_WalkStackCallback aCallback, PRUint32 aSkipFrames,
              void *aClosure)
 {
-    HANDLE myProcess = ::GetCurrentProcess();
-    HANDLE myThread, walkerThread;
+    HANDLE myProcess, myThread, walkerThread;
     DWORD walkerReturn;
     struct WalkStackData data;
 
@@ -457,13 +455,23 @@ NS_StackWalk(NS_WalkStackCallback aCallback, PRUint32 aSkipFrames,
         return NS_ERROR_FAILURE;
 
     // Have to duplicate handle to get a real handle.
-    ::DuplicateHandle(
-      ::GetCurrentProcess(),
-      ::GetCurrentThread(),
-      ::GetCurrentProcess(),
-      &myThread,
-      THREAD_ALL_ACCESS, FALSE, 0
-    );
+    if (!::DuplicateHandle(::GetCurrentProcess(),
+                           ::GetCurrentProcess(),
+                           ::GetCurrentProcess(),
+                           &myProcess,
+                           THREAD_ALL_ACCESS, FALSE, 0)) {
+        PrintError("DuplicateHandle (process)");
+        return NS_ERROR_FAILURE;
+    }
+    if (!::DuplicateHandle(::GetCurrentProcess(),
+                           ::GetCurrentThread(),
+                           ::GetCurrentProcess(),
+                           &myThread,
+                           THREAD_ALL_ACCESS, FALSE, 0)) {
+        PrintError("DuplicateHandle (thread)");
+        ::CloseHandle(myProcess);
+        return NS_ERROR_FAILURE;
+    }
 
     data.callback = aCallback;
     data.skipFrames = aSkipFrames;
@@ -476,11 +484,13 @@ NS_StackWalk(NS_WalkStackCallback aCallback, PRUint32 aSkipFrames,
         if (walkerReturn != WAIT_OBJECT_0) {
             PrintError("ThreadWait");
         }
-        CloseHandle(myThread);
+        ::CloseHandle(walkerThread);
     }
     else {
         PrintError("ThreadCreate");
     }
+    ::CloseHandle(myThread);
+    ::CloseHandle(myProcess);
     return NS_OK;
 }
 

xpcom/base/nsStackWalk.cpp

@@ -40,6 +40,7 @@
 #include "nsStackWalk.h"
 
 #if defined(_WIN32) && defined(_M_IX86) && !defined(WINCE) // WIN32 x86 stack walking code
+
 #include "nsStackFrameWin.cpp"
 
 // WIN32 x86 stack walking code