Segfault add exist sheetlist

[Andrey Fedorushkov]

Linux x86_64, distr mageia5.

KiCad bzr6516 segfault:
add sheet -> type exist file -> load? yes -> segfault in SCH_SHEET::SearchHierarchy(wxString const&, SCH_SCREEN**) () at /usr/bin/_eeschema.kiface

gdb out:
Program received signal SIGSEGV, Segmentation fault.
0x00007fffd3a01df7 in SCH_SHEET::SearchHierarchy(wxString const&, SCH_SCREEN**) () from /usr/bin/_eeschema.kiface
(gdb) bt
#0 0x00007fffd3a01df7 in SCH_SHEET::SearchHierarchy(wxString const&, SCH_SCREEN**) () at /usr/bin/_eeschema.kiface
#1 0x00007fffd3a024fa in SCH_SHEET::Load(SCH_EDIT_FRAME*) () at /usr/bin/_eeschema.kiface
#2 0x00007fffd3a43620 in SCH_EDIT_FRAME::EditSheet(SCH_SHEET*, SCH_SHEET_PATH*) () at /usr/bin/_eeschema.kiface
#3 0x00007fffd3a372e7 in SCH_EDIT_FRAME::addCurrentItemToList(bool) () at /usr/bin/_eeschema.kiface
#4 0x00007fffd39829bd in SCH_EDIT_FRAME::OnLeftClick(wxDC*, wxPoint const&) () at /usr/bin/_eeschema.kiface
#5 0x00007fffd3b7be80 in EDA_DRAW_PANEL::OnMouseEvent(wxMouseEvent&) () at /usr/bin/_eeschema.kiface
#6 0x00007ffff654f97e in wxAppConsoleBase::CallEventHandler(wxEvtHandler*, wxEventFunctor&, wxEvent&) const () at /lib64/libwx_baseu-3.0.so.0
#7 0x00007ffff676a408 in wxEvtHandler::ProcessEventIfMatchesId(wxEventTableEntryBase const&, wxEvtHandler*, wxEvent&) () at /lib64/libwx_baseu-3.0.so.0
#8 0x00007ffff676a50b in wxEventHashTable::HandleEvent(wxEvent&, wxEvtHandler*) () at /lib64/libwx_baseu-3.0.so.0
#9 0x00007ffff676a8b8 in wxEvtHandler::TryHereOnly(wxEvent&) () at /lib64/libwx_baseu-3.0.so.0
#10 0x00007ffff676a943 in wxEvtHandler::ProcessEventLocally(wxEvent&) () at /lib64/libwx_baseu-3.0.so.0
#11 0x00007ffff676a9a5 in wxEvtHandler::ProcessEvent(wxEvent&) () at /lib64/libwx_baseu-3.0.so.0
#12 0x00007ffff7003ceb in wxScrollHelperEvtHandler::ProcessEvent(wxEvent&) () at /lib64/libwx_gtk2u_core-3.0.so.0
#13 0x00007ffff676a717 in wxEvtHandler::SafelyProcessEvent(wxEvent&) () at /lib64/libwx_baseu-3.0.so.0
#14 0x00007ffff6d1496b in gtk_window_button_release_callback () at /lib64/libwx_gtk2u_core-3.0.so.0
#15 0x00007ffff51d6fbf in _gtk_marshal_BOOLEAN__BOXED () at /lib64/libgtk-x11-2.0.so.0
#16 0x00007ffff4bb60e5 in g_closure_invoke () at /lib64/libgobject-2.0.so.0
#17 0x00007ffff4bc775c in signal_emit_unlocked_R () at /lib64/libgobject-2.0.so.0
#18 0x00007ffff4bcf980 in g_signal_emit_valist () at /lib64/libgobject-2.0.so.0
#19 0x00007ffff4bd0082 in g_signal_emit () at /lib64/libgobject-2.0.so.0
#20 0x00007ffff52e74fc in gtk_widget_event_internal () at /lib64/libgtk-x11-2.0.so.0
#21 0x00007ffff51d5714 in gtk_propagate_event () at /lib64/libgtk-x11-2.0.so.0
#22 0x00007ffff51d5b8b in gtk_main_do_event () at /lib64/libgtk-x11-2.0.so.0
#23 0x00007ffff4e508bc in gdk_event_dispatch () at /lib64/libgdk-x11-2.0.so.0
#24 0x00007ffff4473c6d in g_main_context_dispatch () at /lib64/libglib-2.0.so.0
#25 0x00007ffff4473f18 in g_main_context_iterate.isra () at /lib64/libglib-2.0.so.0
#26 0x00007ffff4474232 in g_main_loop_run () at /lib64/libglib-2.0.so.0
#27 0x00007ffff51d4b67 in gtk_main () at /lib64/libgtk-x11-2.0.so.0
#28 0x00007ffff6ce1d65 in wxGUIEventLoop::DoRun() () at /lib64/libwx_gtk2u_core-3.0.so.0
#29 0x00007ffff65b3450 in wxEventLoopBase::Run() () at /lib64/libwx_baseu-3.0.so.0
#30 0x00007ffff65539b6 in wxAppConsoleBase::MainLoop() () at /lib64/libwx_baseu-3.0.so.0
#31 0x0000000000460536 in ()
#32 0x00007ffff6618710 in wxEntry(int&, wchar_t**) () at /lib64/libwx_baseu-3.0.so.0
#33 0x00000000004212be in ()
#34 0x00007ffff5704fd0 in __libc_start_main () at /lib64/libc.so.6
#35 0x0000000000425f17 in _start ()

[Wayne Stambaugh (stambaughw)]

I cannot duplicate this on win32, win64, or Debian Testing 64-bit builds. Have you tested any other project to see if this is just your schematic or if this happens every time you create a sheet using an existing schematic file. If it only happens with a certain schematic file or project, please post the project or a minimal example that demonstrates the behavior.

[Andrey Fedorushkov (faa)]

Project of the three sheets in the attachment.
Root sheet and two sheets.
Remove the sheet 1 or sheet 2. Add a new sheet from an existing file (Sheet1.sch or Sheet2.sch) -> "Load?" Ok -> Segfault.

Application: kicad
Version: (2016-jan-29 BZR 6527)-product release build
wxWidgets: Version 3.0.2 (debug,wchar_t,compiler with C++ ABI 1002,GCC 4.9.2,wx containers,compatible with 2.8)
Platform: Linux 4.1.15-desktop-2.mga5 x86_64, 64 bit, Little endian, wxGTK
Boost version: 1.55.0
         USE_WX_GRAPHICS_CONTEXT=OFF
         USE_WX_OVERLAY=OFF
         KICAD_SCRIPTING=ON
         KICAD_SCRIPTING_MODULES=ON
         KICAD_SCRIPTING_WXPYTHON=ON
         USE_FP_LIB_TABLE=HARD_CODED_ON
         BUILD_GITHUB_PLUGIN=ON

[Andrey Fedorushkov (faa)]

bzr6326 does not contain this bug,
regression included somewhere after bzr6326.

[Andrey Fedorushkov (faa)]

Compile bzr6530 with -DCMAKE_BUILD_TYPE=Debug.
Segfault when append sheet from exist file.

gdb trace:

/mnt/sda3/faa/Projects/kicad-dev/eeschema/sch_sheet.cpp(1198): assert "parent->Type() == SCH_SHEET_T" failed in GetRootSheet(): SCH_SHEET parent is not a SCH_SHEET
[Thread 0x7fffdfd45700 (LWP 11547) exited]

Program received signal SIGTRAP, Trace/breakpoint trap.
0x00007ffff3ff6039 in raise () from /lib64/libpthread.so.0
(gdb) bt
#0 0x00007ffff3ff6039 in raise () at /lib64/libpthread.so.0
#1 0x00007ffff6be94c8 in wxGUIAppTraits::ShowAssertDialog(wxString const&) () at /lib64/libwx_gtk2u_core-3.0.so.0
#2 0x00007ffff629d48a in ShowAssertDialog(wxString const&, int, wxString const&, wxString const&, wxString const&, wxAppTraits*) () at /lib64/libwx_baseu-3.0.so.0
#3 0x00007ffff629d82d in wxAppConsoleBase::OnAssertFailure(wchar_t const*, int, wchar_t const*, wchar_t const*, wchar_t const*) () at /lib64/libwx_baseu-3.0.so.0
#4 0x00007ffff6ba9540 in wxApp::OnAssertFailure(wchar_t const*, int, wchar_t const*, wchar_t const*, wchar_t const*) () at /lib64/libwx_gtk2u_core-3.0.so.0
#5 0x00007ffff629dbd8 in wxDefaultAssertHandler(wxString const&, int, wxString const&, wxString const&, wxString const&) () at /lib64/libwx_baseu-3.0.so.0
#6 0x00007ffff629ab1d in wxOnAssert(char const*, int, char const*, char const*, char const*) () at /lib64/libwx_baseu-3.0.so.0
#7 0x00007fffdc8e9ceb in SCH_SHEET::GetRootSheet() (this=0x24db550) at /mnt/sda3/faa/Projects/kicad-dev/eeschema/sch_sheet.cpp:1198
#8 0x00007fffdc8e71b6 in SCH_SHEET::Load(SCH_EDIT_FRAME*) (this=0x24db550, aFrame=0x1aa6e30) at /mnt/sda3/faa/Projects/kicad-dev/eeschema/sch_sheet.cpp:737
#9 0x00007fffdc90f635 in SCH_EDIT_FRAME::EditSheet(SCH_SHEET*, SCH_SHEET_PATH*) (this=0x1aa6e30, aSheet=0x24db550, aHierarchy=0xc79180)
    at /mnt/sda3/faa/Projects/kicad-dev/eeschema/sheet.cpp:227
#10 0x00007fffdc909570 in SCH_EDIT_FRAME::addCurrentItemToList(bool) (this=0x1aa6e30, aRedraw=true) at /mnt/sda3/faa/Projects/kicad-dev/eeschema/schframe.cpp:1277
#11 0x00007fffdc897420 in SCH_EDIT_FRAME::OnLeftClick(wxDC*, wxPoint const&) (this=0x1aa6e30, aDC=0x7fffffffcbc0, aPosition=...)
    at /mnt/sda3/faa/Projects/kicad-dev/eeschema/onleftclick.cpp:85
#12 0x00007fffdc99cdc2 in EDA_DRAW_PANEL::OnMouseEvent(wxMouseEvent&) (this=0xc78400, event=...) at /mnt/sda3/faa/Projects/kicad-dev/common/draw_panel.cpp:1107
#13 0x00007ffff6297a9e in wxAppConsoleBase::CallEventHandler(wxEvtHandler*, wxEventFunctor&, wxEvent&) const () at /lib64/libwx_baseu-3.0.so.0
#14 0x00007ffff6428c08 in wxEvtHandler::ProcessEventIfMatchesId(wxEventTableEntryBase const&, wxEvtHandler*, wxEvent&) () at /lib64/libwx_baseu-3.0.so.0
#15 0x00007ffff6428d0b in wxEventHashTable::HandleEvent(wxEvent&, wxEvtHandler*) () at /lib64/libwx_baseu-3.0.so.0
#16 0x00007ffff64290b8 in wxEvtHandler::TryHereOnly(wxEvent&) () at /lib64/libwx_baseu-3.0.so.0
#17 0x00007ffff6429143 in wxEvtHandler::ProcessEventLocally(wxEvent&) () at /lib64/libwx_baseu-3.0.so.0
#18 0x00007ffff64291a5 in wxEvtHandler::ProcessEvent(wxEvent&) () at /lib64/libwx_baseu-3.0.so.0
#19 0x00007ffff6e67d6b in wxScrollHelperEvtHandler::ProcessEvent(wxEvent&) () at /lib64/libwx_gtk2u_co...

[Andrey Fedorushkov (faa)]

This patch fixes the problem when add a sheet from an existing file.
It is not entirely true, since the undefined g_RootSheet not work.

=== modified file 'eeschema/sch_sheet.cpp'
--- eeschema/sch_sheet.cpp 2016-01-24 20:46:14 +0000
+++ eeschema/sch_sheet.cpp 2016-02-02 07:12:48 +0000
@@ -1192,12 +1192,19 @@
     EDA_ITEM* parent = GetParent();
     SCH_SHEET* rootSheet = this;

- while( parent )
- {
- // The parent of a SCH_SHEET object can only be another SCH_SHEET object or NULL.
- wxASSERT_MSG( parent->Type() == SCH_SHEET_T, "SCH_SHEET parent is not a SCH_SHEET" );
- rootSheet = static_cast<SCH_SHEET*>( parent );
- parent = parent->GetParent();
+ if( g_RootSheet != NULL )
+ {
+ rootSheet = g_RootSheet;
+ }
+ else
+ {
+ while( parent )
+ {
+ // The parent of a SCH_SHEET object can only be another SCH_SHEET object or NULL.
+ wxASSERT_MSG( parent->Type() == SCH_SHEET_T, "SCH_SHEET parent is not a SCH_SHEET" );
+ rootSheet = static_cast<SCH_SHEET*>( parent );
+ parent = parent->GetParent();
+ }
     }

     return rootSheet;

[Wayne Stambaugh (stambaughw)]

@Andrey, thanks for the patch but this is not going work with the
refactoring changes I am working on. The g_RootSheet global variable is
going away in the future. I made a mistake with the parent/child
relationship between the sheet objects when deleting an existing sheet
which I need to fix. I am not ignoring the problem because I'm the one
who created it. I will fix it as soon as I get a chance. I was at
FOSDEM over the weekend and I just did not get a chance to look at it.

Thanks,

Wayne

On 2/2/2016 2:38 AM, Andrey Fedorushkov wrote:
> This patch fixes the problem when add a sheet from an existing file.
> It is not entirely true, since the undefined g_RootSheet not work.
>
> === modified file 'eeschema/sch_sheet.cpp'
> --- eeschema/sch_sheet.cpp 2016-01-24 20:46:14 +0000
> +++ eeschema/sch_sheet.cpp 2016-02-02 07:12:48 +0000
> @@ -1192,12 +1192,19 @@
> EDA_ITEM* parent = GetParent();
> SCH_SHEET* rootSheet = this;
>
> - while( parent )
> - {
> - // The parent of a SCH_SHEET object can only be another SCH_SHEET object or NULL.
> - wxASSERT_MSG( parent->Type() == SCH_SHEET_T, "SCH_SHEET parent is not a SCH_SHEET" );
> - rootSheet = static_cast<SCH_SHEET*>( parent );
> - parent = parent->GetParent();
> + if( g_RootSheet != NULL )
> + {
> + rootSheet = g_RootSheet;
> + }
> + else
> + {
> + while( parent )
> + {
> + // The parent of a SCH_SHEET object can only be another SCH_SHEET object or NULL.
> + wxASSERT_MSG( parent->Type() == SCH_SHEET_T, "SCH_SHEET parent is not a SCH_SHEET" );
> + rootSheet = static_cast<SCH_SHEET*>( parent );
> + parent = parent->GetParent();
> + }
> }
>
> return rootSheet;
>

[Wayne Stambaugh (stambaughw)]

I committed the fix for this bug in the product branch r6533. Please let me know if you find and issues.

[Andrey Fedorushkov (faa)]

Wayne, thanks. Bug fixed.