kicad-developers team mailing list archive

[Thomas Pointhuber <thomas.pointhuber@xxxxxx>]

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@xxxxxxxxxxxxxxxxxxx
>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>> More help   : https://help.launchpad.net/ListHelp
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> 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.pointhuber@xxxxxx>
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

Attachment: signature.asc
Description: OpenPGP digital signature