[WTF] Move JSC tools/StackTrace to WTF and unify stack trace dump code
Created attachment 307938 [details] Patch
Comment on attachment 307938 [details] Patch We would like to unify things further.
Created attachment 307951 [details] Patch
Created attachment 307952 [details] Patch
The Windows build failure seems to be relevant. Can you fix that first? Thanks.
Comment on attachment 307952 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307952&action=review LGTM but need to fix broken Windows build. > Source/JavaScriptCore/ChangeLog:8 > + This patch adds an utility method to produce demangled names with dladdr. typo: /an utility/a utility/. > Source/WTF/wtf/DefaultFree.h:33 > +struct DefaultFree { I suggest renaming this to SystemFree to match our parlance elsewhere e.g. see DebugHeap::DebugHeap()'s referral to "System Malloc" in bmalloc/heap/DebugHeap.cpp. > Source/WTF/wtf/Platform.h:673 > +#endif I suggest adding // PLATFORM(GTK) after the #endif. > Source/WTF/wtf/Platform.h:675 > +#endif I suggest adding // OS(DARWIN) || OS(LINUX) after the #endif. > Source/WTF/wtf/StackTrace.h:61 > + DemangleEntry(const char* mangledName, const char* demangledName) > + : m_mangledName(mangledName) > + , m_demangledName(demangledName) > + { } I suggest making this constructor private and adding StackTrace as a friend of DemangleEntry. This way, you can control the life-cycle of DemangleEntry tightly and ensure that demangledName is indeed allocated with the system malloc to match how we free it.
Comment on attachment 307952 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=307952&action=review Thanks! >> Source/JavaScriptCore/ChangeLog:8 >> + This patch adds an utility method to produce demangled names with dladdr. > > typo: /an utility/a utility/. Fixed. >> Source/WTF/wtf/DefaultFree.h:33 >> +struct DefaultFree { > > I suggest renaming this to SystemFree to match our parlance elsewhere e.g. see DebugHeap::DebugHeap()'s referral to "System Malloc" in bmalloc/heap/DebugHeap.cpp. This is originally aligned to std::default_delete. But, since we have bmalloc's fastMalloc along with the system malloc, SystemFree seems more descriptive. Renamed. >> Source/WTF/wtf/Platform.h:673 >> +#endif > > I suggest adding // PLATFORM(GTK) after the #endif. Fixed. >> Source/WTF/wtf/Platform.h:675 >> +#endif > > I suggest adding // OS(DARWIN) || OS(LINUX) after the #endif. Fixed. >> Source/WTF/wtf/StackTrace.h:61 >> + { } > > I suggest making this constructor private and adding StackTrace as a friend of DemangleEntry. This way, you can control the life-cycle of DemangleEntry tightly and ensure that demangledName is indeed allocated with the system malloc to match how we free it. Sounds very nice! Fixed.
Committed r215715: <http://trac.webkit.org/changeset/215715>
Committed r215717: <http://trac.webkit.org/changeset/215717>