297723 - stackwalk code not working for WinXP

Status: RESOLVED INCOMPLETE

(Core :: XPCOM, defect)

Description

[Colin Blake]

The windows stack walking code isn't working on WinXP. I get two stack frames 
and then stackwalk returns zero. So, three changes made to the stack walk code 
(windows only):

1. Use the xxx64 routines if available (eg stackwalk64 instead of stackwalk)
2. Heed the warning in the doc and suspend the thread whose stack is to
   be walked (and walk it from another thread)
3. Heed the other warning in the doc about these debug routines not being
   threadsafe.

I'll post the patches next.

Comment 1

[Colin Blake]

Attachment: patch

Comment 2

[Doug Turner (:dougt)]

this is very very cool.  Reviewing now.

Comment 3

[Doug Turner (:dougt)]

Comment on attachment 186272 [details] [diff] [review]
patch

is IMAGEHLP.H only for _M_IX86?

spacing is a bit off:

+    hStackWalkMutex = CreateMutex(
+      NULL,			   // default security attributes
+      FALSE,			   // initially not owned
+      NULL);			   // unnamed mutex
+

+    // Have to duplicate handle to get a real handle.
+    ::DuplicateHandle(
+      ::GetCurrentProcess(),
+      ::GetCurrentThread(),
+      ::GetCurrentProcess(),
+      &myThread,
+      THREAD_ALL_ACCESS, FALSE, 0
+    );


Does this still produce output acceptable for the memory ref counting tools? 
Have you tried looking for a cycle?

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

how does this bug relate to bug 198938?

Comment 5

[Colin Blake]

I'd say this would fix bug 198938.

Comment 6

[David Bradley]

*** Bug 198938 has been marked as a duplicate of this bug. ***

Comment 7

[Doug Turner (:dougt)]

Comment on attachment 186272 [details] [diff] [review]
patch

fix the style nits as I mentioned.  Colin sent me output from the xpcom
refcount balancer -- it looked correct.

Comment 8

[Colin Blake]

> is IMAGEHLP.H only for _M_IX86?

IMAGEHLP is only for the old stuff, so we don't need it for AMD64 or IA64 which 
need the newer 64-bit stuff.

> spacing is a bit off: 

Just following what's already in the module (FormatMessage) at 
http://lxr.mozilla.org/mozilla/source/xpcom/base/nsStackFrameWin.cpp#298

Comment 9

[timeless]

Comment on attachment 186272 [details] [diff] [review]
patch

changes required to make vc6's platform sdk happy:
http://tinderbox.mozilla.org/bonsai/cvsview2.cgi?command=DIFF&subdir=mozilla%2F
xpcom%2Fbase&file=nsStackFrameWin.cpp&rev1=1.4&rev2=1.6&whitespace_mode=show&di
ff_mode=context
http://tinderbox.mozilla.org/bonsai/cvsview2.cgi?command=DIFF&subdir=mozilla%2F
xpcom%2Fbase&file=nsStackFrameWin.h&rev1=1.8&rev2=1.10&whitespace_mode=show&dif
f_mode=context

Comment 10

[Colin Blake]

The changes to make it build on VC6 unfortunately meant that it stopped working 
on WinXP. We can't use #ifdef _IMAGEHLP64 as it's not defined on WXP. I'll post 
another patch to fix this.

Comment 11

[Colin Blake]

Attachment: Make it use the new APIs on WXP

Comment 12

[timeless]

Comment on attachment 186272 [details] [diff] [review]
patch

mozilla/xpcom/base/nsStackFrameWin.cpp	1.6
mozilla/xpcom/base/nsStackFrameWin.h	1.10

Comment 13

[timeless]

Comment on attachment 187433 [details] [diff] [review]
Make it use the new APIs on WXP

mozilla/xpcom/base/nsStackFrameWin.h	1.11
mozilla/xpcom/base/nsStackFrameWin.cpp	1.7

Comment 14

[timeless]

Comment on attachment 187433 [details] [diff] [review]
Make it use the new APIs on WXP

mozilla/xpcom/base/nsStackFrameWin.h	1.11
mozilla/xpcom/base/nsStackFrameWin.cpp	1.7

Comment 15

[Michael Moy]

This apparently broke the AMD 64 build:

Building deps for nsStackFrameWin.cpp
/cygdrive/f/Mozilla/mozilla/build/cygwin-wrapper cl -FonsStackFrameWin.obj -c 
-DMOZILLA_INTERNAL_API -D_IMPL_
NS_GFX -D_IMPL_NS_MSG_BASE -D_IMPL_NS_WIDGET  -DOSTYPE=\"WINNT5.2\"
-DOSARCH=\"WINNT\" -D_WIN64 -D_AMD64_ -DBU
ILD_ID=0000000000 -D_IMPL_NS_COM  -I../../dist/include/string
-I../../dist/include/xpcom -I../../dist/include
-I../../dist/include/nspr          -TP -nologo -GS- -W3 -Gy
-FdnsStackFrameWin.pdb  -DNDEBUG -DTRIMMED -O2 -GL
 -fp:fast -GS- -MD            -DX_DISPLAY_MISSING=1 -DHAVE_64BIT_OS=1
-DMOZILLA_VERSION=\"1.8b3\" -DMOZILLA_VE
RSION_MAJOR=1 -DMOZILLA_VERSION_MINOR=8 -DHAVE_SNPRINTF=1 -D_WINDOWS=1
-D_WIN32=1 -DWIN32=1 -DXP_WIN=1 -DXP_WI
N32=1 -DHW_THREADS=1 -DSTDC_HEADERS=1 -DWIN32_LEAN_AND_MEAN=1 -DNO_X11=1
-D_AMD64_=1 -D_M_AMD64=1 -D_WIN64=1 -
DWINVER=0x501 -DD_INO=d_ino -DMOZ_DEFAULT_TOOLKIT=\"windows\" -DMOZ_PHOENIX=1
-DMOZ_BUILD_APP=browser -DMOZ_XU
L_APP=1 -DMOZ_DISTRIBUTION_ID=\"org.mozilla\" -DOJI=1 -DIBMBIDI=1
-DMOZ_VIEW_SOURCE=1 -DACCESSIBILITY=1 -DMOZ_
XPINSTALL=1 -DMOZ_JSLOADER=1 -DMOZ_XTF=1 -DMOZ_MATHML=1 -DMOZ_SVG=1
-DMOZ_SVG_RENDERER_GDIPLUS=1 -DMOZ_LOGGING
=1 -DMOZ_USER_DIR=\"Mozilla\" -DMOZ_XUL=1 -DMOZ_PROFILELOCKING=1
-DMOZ_DLL_SUFFIX=\".dll\" -DJS_THREADSAFE=1 -
DNS_PRINT_PREVIEW=1 -DNS_PRINTING=1 -DMOZILLA_LOCALE_VERSION=\"1.8b2\"
-DMOZILLA_REGION_VERSION=\"1.8b2\" -DMO
ZILLA_SKIN_VERSION=\"1.5\"  -D_MOZILLA_CONFIG_H_ -DMOZILLA_CLIENT
/cygdrive/f/Mozilla/mozilla/xpcom/base/nsSta
ckFrameWin.cpp
nsStackFrameWin.cpp
f:\Mozilla\mozilla\xpcom\base\nsstackframewin.h(78) : error C2061: syntax error
: identifier 'LPSTACKFRAME'
f:\Mozilla\mozilla\xpcom\base\nsstackframewin.h(115) : error C2061: syntax error
: identifier 'PIMAGEHLP_SYMBO
L'
f:\Mozilla\mozilla\xpcom\base\nsstackframewin.h(131) : error C2065:
'PIMAGEHLP_SYMBOL' : undeclared identifier

f:\Mozilla\mozilla\xpcom\base\nsstackframewin.h(131) : error C2275: 'PSTR' :
illegal use of this type as an ex
pression
        d:\Program Files\Microsoft Platform SDK\Include\WinNT.h(288) : see
declaration of 'PSTR'
f:\Mozilla\mozilla\xpcom\base\nsstackframewin.h(131) : error C2275: 'DWORD' :
illegal use of this type as an e
xpression
        d:\Program Files\Microsoft Platform SDK\Include\WinDef.h(151) : see
declaration of 'DWORD'
f:\Mozilla\mozilla\xpcom\base\nsstackframewin.h(131) : error C2165: 'left-side
modifier' : cannot modify point
ers to data
f:\Mozilla\mozilla\xpcom\base\nsstackframewin.h(131) : error C2513: 'DWORD *' :
no variable declared before '=
'
f:\Mozilla\mozilla\xpcom\base\nsstackframewin.h(134) : error C2061: syntax error
: identifier 'PIMAGEHLP_MODUL
E'
f:\Mozilla\mozilla\xpcom\base\nsstackframewin.h(142) : error C2061: syntax error
: identifier 'PENUMLOADED_MOD
ULES_CALLBACK'
f:\Mozilla\mozilla\xpcom\base\nsstackframewin.h(150) : error C2061: syntax error
: identifier 'PIMAGEHLP_LINE'

f:\Mozilla\mozilla\xpcom\base\nsstackframewin.h(176) : error C2061: syntax error
: identifier 'PIMAGEHLP_MODUL
E'
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(305) : error C2061: syntax
error : identifier 'PIMAGEHLP_MOD
ULE'
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(312) : error C2065:
'aModuleInfo' : undeclared identifier
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(312) : error C2227: left of
'->SizeOfStruct' must point to c
lass/struct/union
        type is ''unknown-type''
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(312) : error C2065:
'IMAGEHLP_MODULE' : undeclared identifie
r
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(312) : error C2070:
''unknown-type'': illegal sizeof operand

f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(313) : error C2065:
'aLineInfo' : undeclared identifier
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(314) : error C2227: left of
'->SizeOfStruct' must point to c
lass/struct/union
        type is ''unknown-type''
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(314) : error C2065:
'IMAGEHLP_LINE' : undeclared identifier
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(314) : error C2070:
''unknown-type'': illegal sizeof operand

f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(330) : error C2197:
'ENUMLOADEDMODULES' : too many arguments
 for call through pointer-to-function
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(422) : error C2065:
'SYMOPT_LOAD_LINES' : undeclared identif
ier
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(422) : error C2065:
'SYMOPT_UNDNAME' : undeclared identifier

f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(644) : error C2065:
'STACKFRAME' : undeclared identifier
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(644) : error C2146: syntax
error : missing ';' before identi
fier 'frame'
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(644) : error C2065: 'frame' :
undeclared identifier
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(693) : error C2228: left of
'.AddrPC' must have class/struct
/union type
        type is ''unknown-type''
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(693) : error C2228: left of
'.Offset' must have class/struct
/union type
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(712) : error C2146: syntax
error : missing ';' before identi
fier 'modInfo'
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(712) : error C2065: 'modInfo'
: undeclared identifier
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(713) : error C2228: left of
'.SizeOfStruct' must have class/
struct/union type
        type is ''unknown-type''
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(713) : error C2070:
''unknown-type'': illegal sizeof operand

f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(728) : error C2065:
'IMAGEHLP_SYMBOL' : undeclared identifie
r
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(728) : error C2070:
''unknown-type'': illegal sizeof operand

f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(729) : error C2146: syntax
error : missing ';' before identi
fier 'pSymbol'
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(729) : error C2065: 'pSymbol'
: undeclared identifier
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(729) : error C2146: syntax
error : missing ';' before identi
fier 'buf'
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(730) : error C2227: left of
'->SizeOfStruct' must point to c
lass/struct/union
        type is ''unknown-type''
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(731) : error C2227: left of
'->MaxNameLength' must point to
class/struct/union
        type is ''unknown-type''
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(736) : error C2228: left of
'.AddrPC' must have class/struct
/union type
        type is ''unknown-type''
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(736) : error C2228: left of
'.Offset' must have class/struct
/union type
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(745) : error C2228: left of
'.ImageName' must have class/str
uct/union type
        type is ''unknown-type''
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(745) : error C2227: left of
'->Name' must point to class/str
uct/union
        type is ''unknown-type''
f:/Mozilla\mozilla\xpcom\base\nsStackFrameWin.cpp(750) : error C2228: left of
'.ImageName' must have class/str
uct/union type
        type is ''unknown-type''
make[4]: *** [nsStackFrameWin.obj] Error 2
make[4]: Leaving directory `/cygdrive/f/Mozilla/mozilla/xpcom/base'
make[3]: *** [libs] Error 2
make[3]: Leaving directory `/cygdrive/f/Mozilla/mozilla/xpcom'
make[2]: *** [tier_2] Error 2
make[2]: Leaving directory `/cygdrive/f/Mozilla/mozilla'
make[1]: *** [default] Error 2
make[1]: Leaving directory `/cygdrive/f/Mozilla/mozilla'
make: *** [build] Error 2

F:\Mozilla\mozilla>cd \mozilla

F:\Mozilla>

Comment 16

[Michael Moy]

Normally, I apply Makoto Kato's patches to nsStackFrameWin.cpp, .h but they don't
apply due to these changes. Running without Makoto's changes produces the
compiler errors above.

I just got the 2005-06-16 Win 64 build working and am trying to get DPA2 built.

Comment 17

[Ben Turner (not reading bugmail, use the needinfo flag!)]

So whatever happened to this bug? Is it still valid?

Comment 18

[Matthew Gertner]

I think the stack-walking stuff needs a fairly substantial overhaul in order to consolidate all the code in one place and enable hooks so it can be used by various consumers in the code. I'm planning to do this in the context of bug 322959 but I have no timeline for this right now.

Comment 19

[Ben Turner (not reading bugmail, use the needinfo flag!)]

And, have you seen bug 331357?

Comment 20

[Matthew Gertner]

I hadn't and I'm not sure it is related. We're talking about generating a stack from an active thread. I would expect the minidump stack extraction stuff to be difference, though I'm not sure about this.

Comment 21

[Matthew Gertner]

For the record, I think the patch I've submitted to bug 322959 is the right solution for this. I've implemented both ref count logging and crash reporting on top of it with no problem.

Comment 22

[Benjamin Smedberg]

I believe this was fixed as part of the profiler work, but in any case tracking it here isn't going to be helpful.