kicad-developers team mailing list archive

[Thomas Pointhuber <thomas.pointhuber@xxxxxx>]

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
> 

From dcb15b221033c69b2afcf186c8f9f9cdb7137ce2 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. To do this some code parts of wx had
to be duplicated to be able to overwrite them.

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  |  57 +++++++++
 kicad/kicad.cpp         |  12 +-
 6 files changed, 336 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..995305803
--- /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/debugrpt.h>
+#include "wx/xml/xml.h"
+
+#if wxUSE_STACKWALKER
+#include "wx/stackwalk.h"
+#endif
+
+#include <wx/datetime.h>
+#include <wx/filename.h>
+#include <wx/ffile.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 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, unsigned long value)
+{
+    node->AddAttribute(name, wxString::Format(wxT("%08lx"), 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"), reinterpret_cast<long unsigned int>(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 // 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..fc08fd5f2
--- /dev/null
+++ b/include/debug_report.h
@@ -0,0 +1,57 @@
+/*
+ * 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 wxUSE_STACKWALKER
+    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