Hi Wayne, thanks for proposing wxDebugReport. I didn't knew about that. I fixed the format issues using clang-format, and fixed a little windows bug as well (which was found by an wx developer). Furthermore I added some preprocessor macro to not overwrite AddContext for wx versions > 3.1.1 because It's already fixed in upstream since a day and will be included in future releases :)
https://github.com/wxWidgets/wxWidgets/pull/940 Personally, I only tested this on Linux x64 with wx 3.1 Regards, Thomas Am 21.09.18 um 16:01 schrieb Wayne Stambaugh: > Hi Thomas, > > I like this much better than your previous patch. What platforms have > you tested this on? Also, please fix the coding policy[1] issues in > your patch. You can use uncrustify or clang-format if you don't want to > do it by hand. > > Cheers, > > Wayne > > [1]: > http://docs.kicad-pcb.org/doxygen/md_Documentation_development_coding-style-policy.html > > On 9/20/2018 5:49 AM, Thomas Pointhuber wrote: >> Hi Tom, >> >> based on your proposal I fixed the Stack Walker (I think it should also >> be fixed upstream). I was required to duplicate quite some wx code and >> we probably need to add an additional copyright notice. >> >> The stack now looks like this, and I verified that I can find the >> location of a crash using addr2line: >> ``` >> <stack> >> <frame level="0" offset="0004752b" address="55aeb750c52b" >> module="pcbnew/pcbnew"/> >> <frame level="1" offset="00045e37" address="55aeb750ae37" >> module="pcbnew/pcbnew"/> >> <frame level="2" offset="000443dc" address="55aeb75093dc" >> module="pcbnew/pcbnew"/> >> <frame level="3" offset="001c830c" address="7f60b12af30c" >> module="/usr/lib/libwx_baseu-3.1.so.1"/> >> <frame level="4" offset="000123c0" address="7f60af51a3c0" >> module="/usr/lib/libpthread.so.0"/> >> <frame level="5" function="__poll" offset="00000051" >> address="7f60af434bb1" module="/usr/lib/libc.so.6"/> >> <frame level="6" offset="0006cee0" address="7f60ae795ee0" >> module="/usr/lib/libglib-2.0.so.0"/> >> <frame level="7" function="g_main_loop_run" offset="000000d2" >> address="7f60ae796f62" module="/usr/lib/libglib-2.0.so.0"/> >> <frame level="8" function="gtk_dialog_run" offset="0000015e" >> address="7f60aedb914e" module="/usr/lib/libgtk-x11-2.0.so.0"/> >> <frame level="9" function="wxMessageDialog::ShowModal()" >> offset="00000083" address="7f60b1acc463" >> module="/usr/lib/libwx_gtk2u_core-3.1.so.1"/> >> ... >> ``` >> >> the current code is more or less a minimal implementation. It would be a >> good idea to add the build parameters like displayed in about_config, >> configuration files, environment variables,... in the future as well. >> >> Regards, >> Thomas >> >> Am 19.09.18 um 20:36 schrieb Tomasz Wlostowski: >>> On 19/09/18 20:13, Thomas Pointhuber wrote: >>>> >>>> I rewrote it to use wxDebugReport and wxHandleFatalExceptions. What I >>>> noticed (at least for my build setup), the stack-trace is unusable in >>>> this version for me because it lacks all kicad internals: >>> >>> Hi Thomas, >>> >>> I like a lot your idea of having a builtin crash reporter (especially >>> under Windows/OSX where people struggle to get a stack trace, Linux >>> users sooner or later are forced to figure out how to use a >>> debugger...). I also second Wayne's comment that it must work on all >>> platforms, so please no glibc-specific code. >>> >>> wxDebugReport is IMHO a good starting point, but for some reasons that >>> are beyond my understanding, wxWidgets doesn't output raw addresses of >>> the functions in the call stack and if there's no symbols in the file >>> (and its dependent DLLs), the trace file is indeed useless. >>> >>> Luckily wxStackFrame contains all the necessary information. The >>> function to blame is XmlStackWalker::OnStackFrame(...). A quick hack >>> like the one below can add raw addresses to the XML dump: >>> >>> -- snip -- >>> >>> if(func.empty()) >>> func = wxT("<unknown>"); >>> >>> nodeFrame->AddAttribute(wxT("function"), func); >>> HexProperty(nodeFrame, wxT("offset"), frame.GetOffset()); >>> HexProperty(nodeFrame, wxT("address"), reinterpret_cast<long >>> unsigned int>(frame.GetAddress())); >>> >>> -- snip -- >>> >>> Cheers, >>> Tom >>> >>> >>> >>> _______________________________________________ >>> Mailing list: https://launchpad.net/~kicad-developers >>> Post to : kicad-developers@lists.launchpad.net >>> Unsubscribe : https://launchpad.net/~kicad-developers >>> More help : https://help.launchpad.net/ListHelp > > _______________________________________________ > Mailing list: https://launchpad.net/~kicad-developers > Post to : kicad-developers@lists.launchpad.net > Unsubscribe : https://launchpad.net/~kicad-developers > More help : https://help.launchpad.net/ListHelp >
From f7a8e59616bad457cf11f745bfce76ae3c6180fd Mon Sep 17 00:00:00 2001 From: Thomas Pointhuber <thomas.pointhu...@gmx.at> Date: Wed, 19 Sep 2018 17:45:27 +0200 Subject: [PATCH] Add initial debug reporter to handle crashes Based on wxDebugReportCompress, but with some adjustments to have an actual useful stacktrace in case of wx <= 3.1.1 Fix was merged upstream for upcomming wx release: * https://github.com/wxWidgets/wxWidgets/pull/940 To convert the offset to actual lines "addr2line" can be used --- CMakeLists.txt | 2 +- common/CMakeLists.txt | 1 + common/debug_report.cpp | 257 ++++++++++++++++++++++++++++++++++++++++ common/single_top.cpp | 12 +- include/debug_report.h | 60 ++++++++++ kicad/kicad.cpp | 12 +- 6 files changed, 339 insertions(+), 5 deletions(-) create mode 100644 common/debug_report.cpp create mode 100644 include/debug_report.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 903835f24..6cfc08f44 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -575,7 +575,7 @@ add_definitions( -DWX_COMPATIBILITY ) # See line 49 of CMakeModules/FindwxWidgets.cmake set( wxWidgets_CONFIG_OPTIONS ${wxWidgets_CONFIG_OPTIONS} --static=no ) -find_package( wxWidgets 3.0.0 COMPONENTS gl aui adv html core net base xml stc REQUIRED ) +find_package( wxWidgets 3.0.0 COMPONENTS gl aui adv html core net base xml stc qa REQUIRED ) # Include wxWidgets macros. include( ${wxWidgets_USE_FILE} ) diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt index dadfad1ae..6f345f9c1 100644 --- a/common/CMakeLists.txt +++ b/common/CMakeLists.txt @@ -256,6 +256,7 @@ set( COMMON_SRCS confirm.cpp convert_basic_shapes_to_polygon.cpp copy_to_clipboard.cpp + debug_report.cpp dialog_shim.cpp displlst.cpp draw_frame.cpp diff --git a/common/debug_report.cpp b/common/debug_report.cpp new file mode 100644 index 000000000..b398529e6 --- /dev/null +++ b/common/debug_report.cpp @@ -0,0 +1,257 @@ +/* + * This program source code file is part of KiCad, a free EDA CAD application. + * + * Copyright (C) 2017 KiCad Developers, see CHANGELOG.txt for contributors. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, you may find one here: + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html + * or you may search the http://www.gnu.org website for the version 2 license, + * or you may write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include <debug_report.h> + +#include "wx/xml/xml.h" +#include <wx/debugrpt.h> + +#if wxUSE_STACKWALKER +#include "wx/stackwalk.h" +#endif + +#include <wx/datetime.h> +#include <wx/ffile.h> +#include <wx/filename.h> + + +void DEBUG_REPORT::GenerateReport( Context ctx ) +{ + DEBUG_REPORT report; + + // Add all wx default reports + report.AddAll( ctx ); + + // Add our own reports + report.AddTimestamp(); + + // Show confirmation dialog and save report + if( wxDebugReportPreviewStd().Show( report ) ) + { + report.Process(); + } +} + +bool DEBUG_REPORT::AddTimestamp() +{ + wxFileName fn( GetDirectory(), wxT( "timestamp.my" ) ); + wxFFile file( fn.GetFullPath(), wxT( "w" ) ); + if( !file.IsOpened() ) + return false; + + wxDateTime dt = wxDateTime::Now(); + bool ret = file.Write( dt.FormatISODate() + wxT( ' ' ) + dt.FormatISOTime() ); + ret &= file.Close(); + + AddFile( fn.GetFullName(), wxT( "timestamp of this report" ) ); + + return ret; +} + +#if !wxCHECK_VERSION( 3, 1, 2 ) && wxUSE_STACKWALKER +class XmlStackWalker : public wxStackWalker +{ +public: + XmlStackWalker( wxXmlNode* nodeStack ) + { + m_isOk = false; + m_nodeStack = nodeStack; + } + + bool IsOk() const + { + return m_isOk; + } + +protected: + virtual void OnStackFrame( const wxStackFrame& frame ) override; + + wxXmlNode* m_nodeStack; + bool m_isOk; +}; + +// ---------------------------------------------------------------------------- +// local functions +// ---------------------------------------------------------------------------- + +static inline void HexProperty( wxXmlNode* node, const wxChar* name, wxUIntPtr value ) +{ + node->AddAttribute( name, wxString::Format( wxT( "%#zx" ), value ) ); +} + +static inline void NumProperty( wxXmlNode* node, const wxChar* name, unsigned long value ) +{ + node->AddAttribute( name, wxString::Format( wxT( "%lu" ), value ) ); +} + +static inline void TextElement( wxXmlNode* node, const wxChar* name, const wxString& value ) +{ + wxXmlNode* nodeChild = new wxXmlNode( wxXML_ELEMENT_NODE, name ); + node->AddChild( nodeChild ); + nodeChild->AddChild( new wxXmlNode( wxXML_TEXT_NODE, wxEmptyString, value ) ); +} + +#if wxUSE_CRASHREPORT && defined( __INTEL__ ) + +static inline void HexElement( wxXmlNode* node, const wxChar* name, unsigned long value ) +{ + TextElement( node, name, wxString::Format( wxT( "%08lx" ), value ) ); +} + +#endif // wxUSE_CRASHREPORT + +// ============================================================================ +// XmlStackWalker implementation based on wx, but with additional information +// like offset, address and module of a stack item +// ============================================================================ + +void XmlStackWalker::OnStackFrame( const wxStackFrame& frame ) +{ + m_isOk = true; + + wxXmlNode* nodeFrame = new wxXmlNode( wxXML_ELEMENT_NODE, wxT( "frame" ) ); + m_nodeStack->AddChild( nodeFrame ); + + NumProperty( nodeFrame, wxT( "level" ), frame.GetLevel() ); + wxString func = frame.GetName(); + if( !func.empty() ) + { + nodeFrame->AddAttribute( wxT( "function" ), func ); + } + + HexProperty( nodeFrame, wxT( "offset" ), frame.GetOffset() ); + HexProperty( nodeFrame, wxT( "address" ), wxPtrToUInt( frame.GetAddress() ) ); + + wxString module = frame.GetModule(); + if( !module.empty() ) + nodeFrame->AddAttribute( wxT( "module" ), module ); + + if( frame.HasSourceLocation() ) + { + nodeFrame->AddAttribute( wxT( "file" ), frame.GetFileName() ); + NumProperty( nodeFrame, wxT( "line" ), frame.GetLine() ); + } + + const size_t nParams = frame.GetParamCount(); + if( nParams ) + { + wxXmlNode* nodeParams = new wxXmlNode( wxXML_ELEMENT_NODE, wxT( "parameters" ) ); + nodeFrame->AddChild( nodeParams ); + + for( size_t n = 0; n < nParams; n++ ) + { + wxXmlNode* nodeParam = new wxXmlNode( wxXML_ELEMENT_NODE, wxT( "parameter" ) ); + nodeParams->AddChild( nodeParam ); + + NumProperty( nodeParam, wxT( "number" ), n ); + + wxString type, name, value; + if( !frame.GetParam( n, &type, &name, &value ) ) + continue; + + if( !type.empty() ) + TextElement( nodeParam, wxT( "type" ), type ); + + if( !name.empty() ) + TextElement( nodeParam, wxT( "name" ), name ); + + if( !value.empty() ) + TextElement( nodeParam, wxT( "value" ), value ); + } + } +} + + +bool DEBUG_REPORT::AddContext( Context ctx ) +{ + wxCHECK_MSG( IsOk(), false, wxT( "use IsOk() first" ) ); + + // create XML dump of current context + wxXmlDocument xmldoc; + wxXmlNode* nodeRoot = new wxXmlNode( wxXML_ELEMENT_NODE, wxT( "report" ) ); + xmldoc.SetRoot( nodeRoot ); + nodeRoot->AddAttribute( wxT( "version" ), wxT( "1.0" ) ); + nodeRoot->AddAttribute( + wxT( "kind" ), ctx == Context_Current ? wxT( "user" ) : wxT( "exception" ) ); + + // add system information + wxXmlNode* nodeSystemInfo = new wxXmlNode( wxXML_ELEMENT_NODE, wxT( "system" ) ); + if( DoAddSystemInfo( nodeSystemInfo ) ) + nodeRoot->AddChild( nodeSystemInfo ); + else + delete nodeSystemInfo; + + // add information about the loaded modules + wxXmlNode* nodeModules = new wxXmlNode( wxXML_ELEMENT_NODE, wxT( "modules" ) ); + if( DoAddLoadedModules( nodeModules ) ) + nodeRoot->AddChild( nodeModules ); + else + delete nodeModules; + + // add CPU context information: this only makes sense for exceptions as our + // current context is not very interesting otherwise + if( ctx == Context_Exception ) + { + wxXmlNode* nodeContext = new wxXmlNode( wxXML_ELEMENT_NODE, wxT( "context" ) ); + if( DoAddExceptionInfo( nodeContext ) ) + nodeRoot->AddChild( nodeContext ); + else + delete nodeContext; + } + + // add stack traceback +#if wxUSE_STACKWALKER + wxXmlNode* nodeStack = new wxXmlNode( wxXML_ELEMENT_NODE, wxT( "stack" ) ); + XmlStackWalker sw( nodeStack ); +#if wxUSE_ON_FATAL_EXCEPTION + if( ctx == Context_Exception ) + { + sw.WalkFromException(); + } + else // Context_Current +#endif // wxUSE_ON_FATAL_EXCEPTION + { + sw.Walk(); + } + + if( sw.IsOk() ) + nodeRoot->AddChild( nodeStack ); + else + delete nodeStack; +#endif // wxUSE_STACKWALKER + + // finally let the user add any extra information he needs + DoAddCustomContext( nodeRoot ); + + + // save the entire context dump in a file + wxFileName fn( GetDirectory(), GetReportName(), wxT( "xml" ) ); + + if( !xmldoc.Save( fn.GetFullPath() ) ) + return false; + + AddFile( fn.GetFullName(), _( "process context description" ) ); + + return true; +} +#endif // !wxCHECK_VERSION(3, 1, 2) && wxUSE_STACKWALKER \ No newline at end of file diff --git a/common/single_top.cpp b/common/single_top.cpp index 30e583892..fce8a0b35 100644 --- a/common/single_top.cpp +++ b/common/single_top.cpp @@ -46,6 +46,7 @@ #include <pgm_base.h> #include <kiway_player.h> #include <confirm.h> +#include <debug_report.h> // Only a single KIWAY is supported in this single_top top level component, @@ -123,9 +124,11 @@ wxIMPLEMENT_DYNAMIC_CLASS(HtmlModule, wxModule); */ struct APP_SINGLE_TOP : public wxApp { -#if defined (__LINUX__) APP_SINGLE_TOP(): wxApp() { + wxHandleFatalExceptions(); + +#if defined (__LINUX__) // Disable proxy menu in Unity window manager. Only usual menubar works with wxWidgets (at least <= 3.1) // When the proxy menu menubar is enable, some important things for us do not work: menuitems UI events and shortcuts. wxString wm; @@ -134,8 +137,8 @@ struct APP_SINGLE_TOP : public wxApp { wxSetEnv ( wxT("UBUNTU_MENUPROXY" ), wxT( "0" ) ); } - } #endif + } bool OnInit() override { @@ -241,6 +244,11 @@ struct APP_SINGLE_TOP : public wxApp } #endif + void OnFatalException() override + { + DEBUG_REPORT::GenerateReport(wxDebugReport::Context_Exception); + } + #ifdef __WXMAC__ /** diff --git a/include/debug_report.h b/include/debug_report.h new file mode 100644 index 000000000..0d0e29fb8 --- /dev/null +++ b/include/debug_report.h @@ -0,0 +1,60 @@ +/* + * This program source code file is part of KiCad, a free EDA CAD application. + * + * Copyright (C) 2017 KiCad Developers, see CHANGELOG.txt for contributors. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, you may find one here: + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html + * or you may search the http://www.gnu.org website for the version 2 license, + * or you may write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#ifndef DEBUG_REPORT_H_ +#define DEBUG_REPORT_H_ + +#include <wx/debugrpt.h> + +/** + * Class DEBUG_REPORT + * + * Based on wxDebugReportCompress but with a improved context + * saver which gives us useful stack-traces. Furthermore it include + * additional information which are helpful for debugging a crash. + */ +class DEBUG_REPORT : public wxDebugReportCompress +{ +public: + DEBUG_REPORT() + { + } + + /** + * Function GenerateReport + * + * generate a KiCad debug report and displays it to the user + * + * @param ctx Context for which the report should be generated + */ + static void GenerateReport( Context ctx ); + + bool AddTimestamp(); + +#if !wxCHECK_VERSION( 3, 1, 2 ) && wxUSE_STACKWALKER + // in case of wx <= 3.1.1 important stack information were not saved + virtual bool AddContext( Context ctx ) override; +#endif +}; + +#endif \ No newline at end of file diff --git a/kicad/kicad.cpp b/kicad/kicad.cpp index 6892de514..c80f66665 100644 --- a/kicad/kicad.cpp +++ b/kicad/kicad.cpp @@ -39,6 +39,7 @@ #include <richio.h> #include <wildcards_and_files_ext.h> #include <systemdirsappend.h> +#include <debug_report.h> #include <stdexcept> @@ -217,9 +218,11 @@ KIWAY Kiway( &Pgm(), KFCTL_CPP_PROJECT_SUITE ); */ struct APP_KICAD : public wxApp { -#if defined (__LINUX__) APP_KICAD(): wxApp() { + wxHandleFatalExceptions(); + +#if defined (__LINUX__) // Disable proxy menu in Unity window manager. Only usual menubar works with // wxWidgets (at least <= 3.1). When the proxy menu menubar is enable, some // important things for us do not work: menuitems UI events and shortcuts. @@ -229,8 +232,8 @@ struct APP_KICAD : public wxApp { wxSetEnv ( wxT("UBUNTU_MENUPROXY" ), wxT( "0" ) ); } - } #endif + } bool OnInit() override { @@ -273,6 +276,11 @@ struct APP_KICAD : public wxApp return -1; } + void OnFatalException() override + { + DEBUG_REPORT::GenerateReport(wxDebugReport::Context_Exception); + } + /** * Set MacOS file associations. * -- 2.19.0
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp