export stack walking API usable from C from XPCOM

RESOLVED FIXED in mozilla1.9alpha8



12 years ago
12 years ago


(Reporter: dbaron, Assigned: dbaron)



Firefox Tracking Flags

(Not tracked)


(Whiteboard: [patch])


(2 attachments, 3 obsolete attachments)

I'd like to clean up the nsStackFrame* API (I never understood why they were split into separate files per platform) by having a single function that is exported from XPCOM.

I'd also like to clean up the mess of our compiling that code into three different places rather than using a single instance of the code.

(Actually, as a followup, I'd like to split it into a pair of functions so that trace-malloc can use it, and can keep up with platform-parity improvements in it; that will be a separate bug.)
Created attachment 259161 [details] [diff] [review]

So far I've only built and tested this on Linux.  I still need to test on Windows.  The Solaris stack walking code looks pretty broken to begin with (it depends on uninitialized stack memory being initialized to zero); I don't plan to test it.
Hrm, it's broken because of nsTraceRefcnt::LoadLibrarySymbols.  Boy, do I wish bug 189221 never landed.
Comment on attachment 259278 [details] [diff] [review]

I'm not sure whether this is an appropriate way to export something of this type from XPCOM given where I want to use it.  But I do want to start cleaning up this mess.  Please let me know.  Note that this is used from toolkit/xre and xpfe/bootstrap (in nsSigHandlers) and I'd like to use it from within libtracemalloc.so.

Note that the API here isn't ready to be frozen, but after bug 374829 I'd probably be comfortable freezing it if that were helpful.  I'd like to take things one step at a time to keep it simple, though.

I'd also like to combine files back together; that is, replace the #includes that I currently have with actually putting the text back in the files.  (I wish it was never split up this way.)  When I do that I'd also want to move nsTraceRefcnt::LoadLibrarySymbols into that nsStackWalk.cpp.
Attachment #259278 - Flags: review?(benjamin)
dbaron, is this something that could be hooked up to the nsAutoLock/nsAutoMonitor potential deadlock detection code? plasticmillion did some work on trace-malloc and nsAutoLock in bug 322959, but it doesn't look like it ever got any traction.
Did you intend that comment for bug 363334?  If so, the answer is no, since one of the locks isn't ours.  In general, if we switched nsTraceMalloc to being a C++ file we probably could, although the bugs it would show would be bugs only when trace-malloc is enabled.

Comment 7

12 years ago
Comment on attachment 259278 [details] [diff] [review]

Instead of nsStackWalk.h, put the function header her:

And use XPCOM_API(nsresult). This will ensure that the function is exported even  when --enable-libxul
Attachment #259278 - Flags: review?(benjamin) → review+
That file doesn't look like it can be included from C -- it's got a bunch of (forward) class declarations in it.
Created attachment 260487 [details] [diff] [review]

This addresses Benjamin's comment about using XPCOM_API / EXPORT_XPCOM_API, but I'm still using nsStackWalk.h pending response to comment 8.  I also fixed some errors in the aSkipFrames handling based on testing on Linux and Windows (off by one in different directions on each platform)
Attachment #259278 - Attachment is obsolete: true
Created attachment 275634 [details] [diff] [review]

Merged to trunk again (which mostly consists of removing some no-longer-needed xpfe/bootstrap changes).
Attachment #260487 - Attachment is obsolete: true
Comment on attachment 275634 [details] [diff] [review]

This is mostly DEBUG-only, but not completely, so requesting approval.  It's part of the plan to consolidate stack-walking code and make trace-malloc usable on platforms other than Linux.
Attachment #275634 - Flags: approval1.9?
And bsmedberg said yesterday on IRC that he was ok with keeping nsStackWalk.h.
Comment on attachment 275634 [details] [diff] [review]

Attachment #275634 - Flags: approval1.9? → approval1.9+

Comment 14

12 years ago
Please fix the build problem on Solaris

"/export/home/mozilla/uild/firefox-nightly/src/mozilla/xpcom/base/nsStackFrameUnix.cpp", line 281: Error: Variable args is not a structure.
"/export/home/mozilla/uild/firefox-nightly/src/mozilla/xpcom/base/nsStackFrameUnix.cpp", line 328: Error: Too few arguments in call to "int(*)(void*,void*,__FILE*)".
"/export/home/mozilla/uild/firefox-nightly/src/mozilla/xpcom/base/nsStackFrameUnix.cpp", line 359: Error: Could not find a match for cs_operate(int(void*,void*), my_user_args*) needed in NS_StackWalk(extern "C" void(*)(void*,void*), unsigned, void*).
"/export/home/mozilla/uild/firefox-nightly/src/mozilla/xpcom/base/nsStackFrameUnix.cpp", line 387: Error: func is not defined.
4 Error(s) detected.

Comment 15

12 years ago
Created attachment 276447 [details] [diff] [review]
patch for build issues
Attachment #276447 - Flags: review?


12 years ago
Attachment #276447 - Flags: review? → review?(dbaron)


12 years ago
Attachment #276447 - Attachment description: patch for build issue → patch for build issues (committed)
Fix checked in to trunk, 2007-08-10 14:28 -0700.
Last Resolved: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9 M8
You need to log in before you can comment on or make changes to this bug.