Segfault in "cut" command in symbol editor

[Oliver]

When performing "cut" operation to move symbol between libraries, segfault occurs ~30% of the time.

Segfault occurs as soon as "cut" operation is selected from right-click menu in tree view.

See attached file for gdb backtrace.

Application: kicad
Version: (2017-12-16 revision fb291cb)-sym-loader, debug build
Libraries:
    wxWidgets 3.0.2
    libcurl/7.47.0 OpenSSL/1.0.2g zlib/1.2.8 libidn/1.32 librtmp/2.3
Platform: Linux 4.10.0-42-generic x86_64, 64 bit, Little endian, wxGTK
Build Info:
    wxWidgets: 3.0.2 (wchar_t,wx containers,compatible with 2.8) GTK+ 2.24
    Boost: 1.58.0
    Curl: 7.47.0
    Compiler: GCC 5.4.0 with C++ ABI 1009

Build settings:
    USE_WX_GRAPHICS_CONTEXT=OFF
    USE_WX_OVERLAY=OFF
    KICAD_SCRIPTING=ON
    KICAD_SCRIPTING_MODULES=ON
    KICAD_SCRIPTING_WXPYTHON=ON
    KICAD_SCRIPTING_ACTION_MENU=OFF
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=OFF
    KICAD_SPICE=OFF

[Maciej Suminski (orsonmmz)]

I cannot reproduce it, but it seems to be related to lp:#1738635. Thank you for the backtrace, it gives some clues.

[Maciej Suminski (orsonmmz)]

As I said, I cannot observe the problem, but perhaps the attached patch will fix it. Could you test it?

[Oliver (schrodingersgat)]

That seems to have fixed it, I can no longer get it to fault (running on the same machine). Thanks :)

[KiCad Janitor (kicad-janitor)]

Fixed in revision f3a6bc665147f6a0f1c4498482fe537f05955d9d
https://git.launchpad.net/kicad/patch/?id=f3a6bc665147f6a0f1c4498482fe537f05955d9d

[Janis Skujenieks (janis-skujenieks)]

Hello!

I still have the bug on latest commit. Only its triggered by different event. I say that based on stack trace.

Steps to trigger segfault:
1)select any library
2)select symbol
3)select edit symbol name (doesn't matter if dbl click or E)
4)change text to anything and hit enter or click ok

Majority of times (would say ~90%) get segfault.

I did some debugging and it is generated because in GetSelectedLibId in component_tree.cpp
line 119 "auto sel = m_tree_ctrl->GetSelection();" returns bad pointer. Its not null and doesn't
point to valid memory.

I don't know code well enough so for now I'm stuck. But if you need more info I can do something specific.

I tested on latest commit. Here is my info:
Application: kicad
Version: (2017-12-26 revision 90818af)-sym_lib_edit_segf, debug build
Libraries:
    wxWidgets 3.0.2
    libcurl/7.35.0 OpenSSL/1.0.1f zlib/1.2.8 libidn/1.28 librtmp/2.3
Platform: Linux 4.8.0-040800-generic x86_64, 64 bit, Little endian, wxGTK
Build Info:
    wxWidgets: 3.0.2 (wchar_t,wx containers,compatible with 2.8) GTK+ 2.24
    Boost: 1.54.0
    Curl: 7.35.0
    Compiler: GCC 4.9.4 with C++ ABI 1002

Build settings:
    USE_WX_GRAPHICS_CONTEXT=OFF
    USE_WX_OVERLAY=OFF
    KICAD_SCRIPTING=ON
    KICAD_SCRIPTING_MODULES=ON
    KICAD_SCRIPTING_WXPYTHON=OFF
    KICAD_SCRIPTING_ACTION_MENU=OFF
    BUILD_GITHUB_PLUGIN=ON
    KICAD_USE_OCE=OFF
    KICAD_SPICE=ON

[Janis Skujenieks (janis-skujenieks)]

I've pinpointed source of this bug. I'll try to describe sequence that causes it in SW.

1)LIB_EDIT_FRAME::EditField()
2)LIB_MANAGER::UpdatePart() - update of memory copy is ok, no problems here
3)LIB_EDIT_FRAME::SyncLibraries() - calls Sync without progress dialog (still ok)
3.1)LIB_MANAGER::Sync()
3.2)LIB_MANAGER_ADAPTER::Sync() calls updateLibrary line 80 to update stored lib.
3.3)LIB_MANAGER_ADAPTER::updateLibrary() - here is the root
In the for loop this code:
// node does not exist in the library manager, remove the corresponding node
nodeIt = aLibNode.Children.erase( nodeIt );

Problem is that when erasing this old entry name wxDataViewCtrl is left still selecting that node.
And node items wxDataViewItem are just pointers.

After that step out till LIB_EDIT_FRAME::SyncLibraries()
4)CMP_TREE_PANE::Regenerate() is called from LIB_EDIT_FRAME::SyncLibraries()
5)From here its like in stack backtraces.
6)It ends when COMPONENT_TREE::GetSelectedLibId() calls m_tree_ctrl->GetSelection()
It returns invalid memory because it still points to that deleted node.

I hope I made sense, but I could verify this in point 3.3, by just copying smart pointer to heap so memory stays valid. Then no segfault is generated.

As for actual fix, at the moment I can't come up with anything reasonable.

[Janis Skujenieks (janis-skujenieks)]

I've created patch for this bug. My resolution is to unselect item in the library tree before updating all libraries. Then select new item that replaces old.

[Janis Skujenieks (janis-skujenieks)]

I noticed my patch only fixes renaming crash. It seems "cut" is still unresolved.

[Janis Skujenieks (janis-skujenieks)]

OK, that was a bug in GTK and only related to my system not KiCAD.

If I disable animations in desktop environment everything works ok.

[Wayne Stambaugh (stambaughw)]

I'll set the status to invalid. Thanks for figuring out it wasn't an issue with KiCad.

[Janis Skujenieks (janis-skujenieks)]

Sorry, if I wasn't clear. The bug that I provided a patch for is still there.

1)Rename symbol needs patch and is a bug in KiCAD.
2)"cut" symbol from library is resolved for me.

[Wayne Stambaugh (stambaughw)]

I'm going to let Orson address this since this is his code.

On 12/29/2017 10:59 AM, Janis Skujenieks wrote:
> Sorry, if I wasn't clear. The bug that I provided a patch for is still
> there.
>
> 1)Rename symbol needs patch and is a bug in KiCAD.
> 2)"cut" symbol from library is resolved for me.
>

[Maciej Suminski (orsonmmz)]

I changed the bug status to 'New', as seemingly there are still problems to be solved.

[Maciej Suminski (orsonmmz)]

I misunderstood the report, as there are two separate issues described here. As far as I understand the "cut" crash is fixed and as this is the subject of this report, I change the status to 'Fix Committed'. The renaming issue is described in another one (lp:#1740952).

[Janis Skujenieks (janis-skujenieks)]

Yes, I think I made a bit of confusion. It looked like same bug, but it turns out not related.