Closed
Bug 374689
Opened 17 years ago
Closed 17 years ago
export stack walking API usable from C from XPCOM
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha8
People
(Reporter: dbaron, Assigned: dbaron)
References
Details
(Whiteboard: [patch])
Attachments
(2 files, 3 obsolete files)
28.17 KB,
patch
|
bzbarsky
:
approval1.9+
|
Details | Diff | Splinter Review |
2.63 KB,
patch
|
dbaron
:
review+
dbaron
:
approval1.9+
|
Details | Diff | Splinter Review |
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.)
Assignee | ||
Comment 1•17 years ago
|
||
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.
Assignee | ||
Updated•17 years ago
|
Whiteboard: [patch]
Assignee | ||
Comment 2•17 years ago
|
||
Hrm, it's broken because of nsTraceRefcnt::LoadLibrarySymbols. Boy, do I wish bug 189221 never landed.
Assignee | ||
Comment 3•17 years ago
|
||
Attachment #259161 -
Attachment is obsolete: true
Assignee | ||
Comment 4•17 years ago
|
||
Comment on attachment 259278 [details] [diff] [review] patch 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.
Assignee | ||
Comment 6•17 years ago
|
||
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•17 years ago
|
||
Comment on attachment 259278 [details] [diff] [review] patch Instead of nsStackWalk.h, put the function header her: http://lxr.mozilla.org/mozilla/source/xpcom/build/nsXPCOMPrivate.h#50 And use XPCOM_API(nsresult). This will ensure that the function is exported even when --enable-libxul
Attachment #259278 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 8•17 years ago
|
||
That file doesn't look like it can be included from C -- it's got a bunch of (forward) class declarations in it.
Assignee | ||
Comment 9•17 years ago
|
||
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
Assignee | ||
Comment 10•17 years ago
|
||
Merged to trunk again (which mostly consists of removing some no-longer-needed xpfe/bootstrap changes).
Attachment #260487 -
Attachment is obsolete: true
Assignee | ||
Comment 11•17 years ago
|
||
Comment on attachment 275634 [details] [diff] [review] patch 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?
Assignee | ||
Comment 12•17 years ago
|
||
And bsmedberg said yesterday on IRC that he was ok with keeping nsStackWalk.h.
Comment 13•17 years ago
|
||
Comment on attachment 275634 [details] [diff] [review] patch a=bzbarsky
Attachment #275634 -
Flags: approval1.9? → approval1.9+
Comment 14•17 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•17 years ago
|
||
Attachment #276447 -
Flags: review?
Attachment #276447 -
Flags: review? → review?(dbaron)
Assignee | ||
Comment 16•17 years ago
|
||
Comment on attachment 276447 [details] [diff] [review] patch for build issues (committed) r=dbaron
Attachment #276447 -
Flags: review?(dbaron) → review+
Assignee | ||
Updated•17 years ago
|
Attachment #276447 -
Flags: approval1.9+
Attachment #276447 -
Attachment description: patch for build issue → patch for build issues
(committed)
Assignee | ||
Comment 17•17 years ago
|
||
Fix checked in to trunk, 2007-08-10 14:28 -0700.
Status: NEW → RESOLVED
Closed: 17 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.
Description
•