Segfault in "cut" command in symbol editor

Bug #1738999 reported by Oliver
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
KiCad
Fix Released
Critical
Maciej Suminski

Bug Description

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

Revision history for this message
Oliver (schrodingersgat) wrote :
Changed in kicad:
assignee: nobody → Maciej Suminski (orsonmmz)
importance: Undecided → Critical
Revision history for this message
Maciej Suminski (orsonmmz) wrote :

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

Revision history for this message
Maciej Suminski (orsonmmz) wrote :

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

Revision history for this message
Oliver (schrodingersgat) wrote :

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

Revision history for this message
KiCad Janitor (kicad-janitor) wrote :

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

Changed in kicad:
status: New → Fix Committed
Revision history for this message
Janis Skujenieks (janis-skujenieks) wrote :

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

Changed in kicad:
status: Fix Committed → New
Revision history for this message
Janis Skujenieks (janis-skujenieks) wrote :

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.

Revision history for this message
Janis Skujenieks (janis-skujenieks) wrote :

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.

Changed in kicad:
milestone: none → 5.0.0-rc1
Revision history for this message
Janis Skujenieks (janis-skujenieks) wrote :

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

Revision history for this message
Janis Skujenieks (janis-skujenieks) wrote :

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.

Revision history for this message
Wayne Stambaugh (stambaughw) wrote :

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

Changed in kicad:
status: New → Invalid
Revision history for this message
Janis Skujenieks (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.

Revision history for this message
Wayne Stambaugh (stambaughw) wrote : Re: [Bug 1738999] Re: Segfault in "cut" command in symbol editor

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.
>

Revision history for this message
Maciej Suminski (orsonmmz) wrote :

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

Changed in kicad:
status: Invalid → New
Revision history for this message
Maciej Suminski (orsonmmz) wrote :

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).

Changed in kicad:
status: New → Fix Committed
Revision history for this message
Janis Skujenieks (janis-skujenieks) wrote :

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

Jeff Young (jeyjey)
Changed in kicad:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.