Crash on adding text to footprint

Bug #1802907 reported by John Beard on 2018-11-12
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
KiCad
High
Jeff Young

Bug Description

I see a crash when adding text to a footprint in Pcbnew

* To recreate: open a PCB with a part that has enough text items that the grid in FP properties needs to scroll if you add one more.
* Add a new row (example, added some more text to "F.Fab")
* Accept dialog (OK)
* Segfault

Crash in void DIALOG_FOOTPRINT_BOARD_EDITOR::OnUpdateUI( wxUpdateUIEvent& ), line 841, grid is null but not checked.

Attached:

* Example PCB (to provoke, add a new row to the properties)
* Screenshot of the dialog before adding the text row

-----

Core was generated by `pcbnew/pcbnew'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x00007f5e25901a8f in DIALOG_FOOTPRINT_BOARD_EDITOR::OnUpdateUI (this=0x55d6c3deb440)
    at /home/john/src/kicad/pcbnew/dialogs/dialog_edit_footprint_for_BoardEditor.cpp:841
841 grid->SetFocus();
[Current thread is 1 (Thread 0x7f5e2dffd940 (LWP 29246))]
(gdb) bt
#0 0x00007f5e25901a8f in DIALOG_FOOTPRINT_BOARD_EDITOR::OnUpdateUI(wxUpdateUIEvent&) (this=0x55d6c3deb440)
    at /home/john/src/kicad/pcbnew/dialogs/dialog_edit_footprint_for_BoardEditor.cpp:841
#1 0x00007f5e2f51089e in wxEvtHandler::ProcessEventIfMatchesId(wxEventTableEntryBase const&, wxEvtHandler*, wxEvent&) () at /usr/lib/libwx_baseu-3.0.so.0
#2 0x00007f5e2f510c1b in wxEvtHandler::SearchDynamicEventTable(wxEvent&) () at /usr/lib/libwx_baseu-3.0.so.0
#3 0x00007f5e2f510cb1 in wxEvtHandler::TryHereOnly(wxEvent&) () at /usr/lib/libwx_baseu-3.0.so.0
#4 0x00007f5e2f510d64 in wxEvtHandler::ProcessEventLocally(wxEvent&) () at /usr/lib/libwx_baseu-3.0.so.0
#5 0x00007f5e2f510e02 in wxEvtHandler::ProcessEvent(wxEvent&) () at /usr/lib/libwx_baseu-3.0.so.0
#6 0x00007f5e2fa66a31 in wxWindowBase::UpdateWindowUI(long) () at /usr/lib/libwx_gtk2u_core-3.0.so.0
#7 0x00007f5e2fa68403 in wxWindowBase::SendIdleEvents(wxIdleEvent&) () at /usr/lib/libwx_gtk2u_core-3.0.so.0
#8 0x00007f5e2fa68438 in wxWindowBase::SendIdleEvents(wxIdleEvent&) () at /usr/lib/libwx_gtk2u_core-3.0.so.0
#9 0x00007f5e2f90aad0 in wxFrame::SendIdleEvents(wxIdleEvent&) () at /usr/lib/libwx_gtk2u_core-3.0.so.0
#10 0x00007f5e2f94c19e in wxAppBase::ProcessIdle() () at /usr/lib/libwx_gtk2u_core-3.0.so.0
#11 0x00007f5e2f8787a6 in wxApp::DoIdle() () at /usr/lib/libwx_gtk2u_core-3.0.so.0
#12 0x00007f5e2f8788b4 in () at /usr/lib/libwx_gtk2u_core-3.0.so.0
#13 0x00007f5e2d24b271 in g_main_context_dispatch () at /usr/lib/libglib-2.0.so.0
#14 0x00007f5e2d24cf89 in () at /usr/lib/libglib-2.0.so.0
#15 0x00007f5e2d24df62 in g_main_loop_run () at /usr/lib/libglib-2.0.so.0
#16 0x00007f5e2d8fbdf3 in gtk_main () at /usr/lib/libgtk-x11-2.0.so.0
#17 0x00007f5e2f8961b6 in wxGUIEventLoop::DoRun() () at /usr/lib/libwx_gtk2u_core-3.0.so.0
#18 0x00007f5e2f3ddbae in wxEventLoopBase::Run() () at /usr/lib/libwx_baseu-3.0.so.0
#19 0x00007f5e2f8ff8f1 in wxDialog::ShowModal() () at /usr/lib/libwx_gtk2u_core-3.0.so.0
#20 0x00007f5e25ba98e9 in PCB_EDIT_FRAME::InstallFootprintPropertiesDialog(MODULE*, wxDC*) (this=
    0x55d6c23cb7b0, Module=0x55d6c2483c70, DC=0x0) at /home/john/src/kicad/pcbnew/pcb_edit_frame.cpp:1264
#21 0x00007f5e25b8dfa7 in PCB_EDIT_FRAME::OnEditItemRequest(wxDC*, BOARD_ITEM*)
    (this=0x55d6c23cb7b0, aDC=0x0, aItem=0x55d6c2483c70) at /home/john/src/kicad/pcbnew/onleftclick.cpp:595
#22 0x00007f5e25c49372 in EDIT_TOOL::Properties(TOOL_EVENT const&) (this=0x55d6c4358480, aEvent=...)
    at /home/john/src/kicad/pcbnew/tools/edit_tool.cpp:640
#23 0x00007f5e25c56d3e in std::__invoke_impl<int, int (EDIT_TOOL::*&)(TOOL_EVENT const&), EDIT_TOOL*&, TOOL_EVENT const&>(std::__invoke_memfun_deref, int (EDIT_TOOL::*&)(TOOL_EVENT const&), EDIT_TOOL*&, TOOL_EVENT const&) (__f=
    @0x55d6c39007e0: (int (EDIT_TOOL::*)(EDIT_TOOL * const, const TOOL_EVENT &)) 0x7f5e25c491ea <EDIT_TOOL::Properties(TOOL_EVENT const&)>, __t=@0x55d6c39007f0: 0x55d6c4358480, __args#0=...) at /usr/include/c++/8.2.1/bits/invoke.h:73
#24 0x00007f5e25c56af2 in std::__invoke<int (EDIT_TOOL::*&)(TOOL_EVENT const&), EDIT_TOOL*&, TOOL_EVENT const&>(int (EDIT_TOOL::*&)(TOOL_EVENT const&), EDIT_TOOL*&, TOOL_EVENT const&) (__fn=
    @0x55d6c39007e0: (int (EDIT_TOOL::*)(EDIT_TOOL * const, const TOOL_EVENT &)) 0x7f5e25c491ea <EDIT_TOOL::Properties(TOOL_EVENT const&)>, __args#0=@0x55d6c39007f0: 0x55d6c4358480, __args#1=...)
    at /usr/include/c++/8.2.1/bits/invoke.h:95
#25 0x00007f5e25c566e6 in std::_Bind<int (EDIT_TOOL::*(EDIT_TOOL*, std::_Placeholder<1>))(TOOL_EVENT const&)>::__call<int, TOOL_EVENT const&, 0ul, 1ul>(std::tuple<TOOL_EVENT const&>&&, std::_Index_tuple<0ul, 1ul>)
    (this=0x55d6c39007e0, __args=...) at /usr/include/c++/8.2.1/functional:400
#26 0x00007f5e25c5610a in std::_Bind<int (EDIT_TOOL::*(EDIT_TOOL*, std::_Placeholder<1>))(TOOL_EVENT const&)>::operator()<TOOL_EVENT const&, int>(TOOL_EVENT const&) (this=0x55d6c39007e0, __args#0=...)
    at /usr/include/c++/8.2.1/functional:484
#27 0x00007f5e25c550a8 in std::_Function_handler<int (TOOL_EVENT const&), std::_Bind<int (EDIT_TOOL::*(EDIT_TOOL*, std::_Placeholder<1>))(TOOL_EVENT const&)> >::_M_invoke(std::_Any_data const&, TOOL_EVENT const&)
    (__functor=..., __args#0=...) at /usr/include/c++/8.2.1/bits/std_function.h:282
#28 0x00007f5e26370b3f in std::function<int (TOOL_EVENT const&)>::operator()(TOOL_EVENT const&) const
    (this=0x55d6c33fe968, __args#0=...) at /usr/include/c++/8.2.1/bits/std_function.h:687
#29 0x00007f5e2636d992 in COROUTINE<int, TOOL_EVENT const&>::callerStub(long) (aData=140732720787504)
    at /home/john/src/kicad/include/tool/coroutine.h:329
#30 0x00007f5e263c8531 in make_fcontext () at /home/john/src/kicad-build/pcbnew/_pcbnew.kiface
#31 0x0000000000000000 in ()

----

Application: pcbnew
Version: (6.0.0-rc1-dev-1178-g62e2fe8bb), debug build
Libraries:
    wxWidgets 3.0.4
    libcurl/7.62.0 OpenSSL/1.1.1 zlib/1.2.11 libidn2/2.0.5 libpsl/0.20.2 (+libidn2/2.0.4) libssh2/1.8.0 nghttp2/1.34.0
Platform: Linux 4.18.16-arch1-1-ARCH x86_64, 64 bit, Little endian, wxGTK
Build Info:
    wxWidgets: 3.0.4 (wchar_t,wx containers,compatible with 2.8) GTK+ 2.24
    Boost: 1.68.0
    OpenCASCADE Community Edition: 6.9.1
    Curl: 7.62.0
    Compiler: GCC 8.2.1 with C++ ABI 1013

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

John Beard (john-j-beard) wrote :
John Beard (john-j-beard) wrote :
description: updated
description: updated
Wayne Stambaugh (stambaughw) wrote :

I cannot reproduce this on windows no matter how many new text items I add so this may be a linux only issue.

John Beard (john-j-beard) wrote :

Hmm actually, this is not the bug. The bug is *any* empty text field will segfault on dialog accept.

However, there seems to be a sub-bug that if you add a row that make the grid scroll (when the grid was not scrollable on dialog open), the text field will evaluate as empty when you click OK, and trigger the primary bug.

Attache video of the sub bug manifesting and triggering the main bug.

John Beard (john-j-beard) wrote :

This also leads to two workaround: if this happens and you need to work around:

* Add another row, enter your text again and delete the empty one that is what remains of the originally-added one, or,
* If you can, resize the window before adding a row to not trigger scrolling

Wayne Stambaugh (stambaughw) wrote :

That did the trick. I can reproduce the crash on windows as well.

Changed in kicad:
status: New → Triaged
importance: Undecided → High
milestone: none → 5.1.0
John Beard (john-j-beard) wrote :

Looks like void DIALOG_FOOTPRINT_BOARD_EDITOR::OnUpdateUI() is called twice.

Because this function sets m_delayedFocusGrid = nullptr, the second time it is called, the local `grid` becomes null, and then gets dereferenced.

No idea why it calls twice in this case (or if this code is supposed to be able to handle that).

John Beard (john-j-beard) wrote :

Also, this bit of OnUpdateUI does not look right:

        m_delayedErrorMessage = wxEmptyString;

        if( !m_delayedErrorMessage.IsEmpty() )
        {
            // Do not use DisplayErrorMessage(); it screws up window order on Mac
            DisplayError( nullptr, msg );
        }

I think the if-conditional should be looking at msg? Fix this doesn't fix the crash though.

Jeff Young (jeyjey) on 2018-11-12
Changed in kicad:
assignee: nobody → Jeff Young (jeyjey)
KiCad Janitor (kicad-janitor) wrote :

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

Changed in kicad:
status: Triaged → Fix Committed
Jeff Young (jeyjey) wrote :

The re-entrancy is something we have to deal with (because of the dialog). That's why we clear the message (and the second error with checking the wrong copy of the message for empty).

However, the main bug was just not setting the delayedFocusGrid to start with.

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

Other bug subscribers