From 1c6d8b3f5ef1010a57d5d505e962d308bbc4c930 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Mon, 1 Sep 2025 12:36:44 +0100 Subject: [PATCH] Don't reference stack variables from a long-life lambda. In particular, a lambda for conditional menus should never capture more than `[this]`. Even the tool's frame pointer could change. Fixes https://gitlab.com/kicad/code/kicad/-/issues/21615 --- eeschema/tools/sch_drawing_tools.cpp | 4 +- eeschema/tools/sch_edit_tool.cpp | 4 +- eeschema/tools/sch_line_wire_bus_tool.cpp | 13 +++-- eeschema/tools/sch_selection_tool.cpp | 10 ++-- eeschema/tools/symbol_editor_control.cpp | 71 +++++++++++++++-------- eeschema/tools/symbol_editor_pin_tool.cpp | 5 +- include/tool/edit_table_tool_base.h | 22 +++---- pcbnew/router/router_tool.cpp | 4 +- pcbnew/tools/drawing_tool.cpp | 2 +- pcbnew/tools/edit_tool.cpp | 16 ++--- pcbnew/tools/footprint_editor_control.cpp | 10 ++-- pcbnew/tools/pad_tool.cpp | 4 +- pcbnew/tools/pcb_point_editor.cpp | 31 +++++----- pcbnew/tools/pcb_selection_tool.cpp | 7 ++- pcbnew/tools/pcb_viewer_tools.cpp | 2 +- 15 files changed, 114 insertions(+), 91 deletions(-) diff --git a/eeschema/tools/sch_drawing_tools.cpp b/eeschema/tools/sch_drawing_tools.cpp index 607c59d434..3c9d4bddd7 100644 --- a/eeschema/tools/sch_drawing_tools.cpp +++ b/eeschema/tools/sch_drawing_tools.cpp @@ -109,13 +109,13 @@ bool SCH_DRAWING_TOOLS::Init() SCH_TOOL_BASE::Init(); auto belowRootSheetCondition = - [&]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { return m_frame->GetCurrentSheet().Last() != &m_frame->Schematic().Root(); }; auto inDrawingRuleArea = - [&]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { return m_drawingRuleArea; }; diff --git a/eeschema/tools/sch_edit_tool.cpp b/eeschema/tools/sch_edit_tool.cpp index 61ca601e85..9ac99d9762 100644 --- a/eeschema/tools/sch_edit_tool.cpp +++ b/eeschema/tools/sch_edit_tool.cpp @@ -343,7 +343,7 @@ bool SCH_EDIT_TOOL::Init() auto sheetSelection = S_C::Count( 1 ) && S_C::OnlyTypes( sheetTypes ); auto haveHighlight = - [&]( const SELECTION& sel ) + [this]( const SELECTION& sel ) { SCH_EDIT_FRAME* editFrame = dynamic_cast( m_frame ); @@ -379,7 +379,7 @@ bool SCH_EDIT_TOOL::Init() }; auto propertiesCondition = - [&]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { if( aSel.GetSize() == 0 ) { diff --git a/eeschema/tools/sch_line_wire_bus_tool.cpp b/eeschema/tools/sch_line_wire_bus_tool.cpp index 8ba586950e..1d1afdbdda 100644 --- a/eeschema/tools/sch_line_wire_bus_tool.cpp +++ b/eeschema/tools/sch_line_wire_bus_tool.cpp @@ -186,10 +186,11 @@ bool SCH_LINE_WIRE_BUS_TOOL::Init() { SCH_TOOL_BASE::Init(); - const auto busGetter = [this]() - { - return getBusForUnfolding(); - }; + const auto busGetter = + [this]() + { + return getBusForUnfolding(); + }; std::shared_ptr busUnfoldMenu = std::make_shared( busGetter ); @@ -214,7 +215,7 @@ bool SCH_LINE_WIRE_BUS_TOOL::Init() }; auto belowRootSheetCondition = - [&]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { return m_frame->GetCurrentSheet().Last() != &m_frame->Schematic().Root(); }; @@ -223,7 +224,7 @@ bool SCH_LINE_WIRE_BUS_TOOL::Init() && SCH_CONDITIONS::OnlyTypes( { SCH_ITEM_LOCATE_BUS_T } ); auto haveHighlight = - [&]( const SELECTION& sel ) + [this]( const SELECTION& sel ) { SCH_EDIT_FRAME* editFrame = dynamic_cast( m_frame ); diff --git a/eeschema/tools/sch_selection_tool.cpp b/eeschema/tools/sch_selection_tool.cpp index 2a9584bb0f..119914ac31 100644 --- a/eeschema/tools/sch_selection_tool.cpp +++ b/eeschema/tools/sch_selection_tool.cpp @@ -232,7 +232,7 @@ bool SCH_SELECTION_TOOL::Init() // clang-format on auto schEditSheetPageNumberCondition = - [&] ( const SELECTION& aSel ) + [this] ( const SELECTION& aSel ) { if( m_isSymbolEditor || m_isSymbolViewer ) return false; @@ -248,7 +248,7 @@ bool SCH_SELECTION_TOOL::Init() }; auto belowRootSheetCondition = - [&]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { SCH_EDIT_FRAME* editFrame = dynamic_cast( m_frame ); @@ -257,7 +257,7 @@ bool SCH_SELECTION_TOOL::Init() }; auto haveHighlight = - [&]( const SELECTION& sel ) + [this]( const SELECTION& sel ) { SCH_EDIT_FRAME* editFrame = dynamic_cast( m_frame ); @@ -265,14 +265,14 @@ bool SCH_SELECTION_TOOL::Init() }; auto haveSymbol = - [&]( const SELECTION& sel ) + [this]( const SELECTION& sel ) { return m_isSymbolEditor && static_cast( m_frame )->GetCurSymbol(); }; auto symbolDisplayNameIsEditable = - [&]( const SELECTION& sel ) + [this]( const SELECTION& sel ) { if ( !m_isSymbolEditor ) return false; diff --git a/eeschema/tools/symbol_editor_control.cpp b/eeschema/tools/symbol_editor_control.cpp index f936c5f518..ca2a977c58 100644 --- a/eeschema/tools/symbol_editor_control.cpp +++ b/eeschema/tools/symbol_editor_control.cpp @@ -55,79 +55,104 @@ bool SYMBOL_EDITOR_CONTROL::Init() { LIBRARY_EDITOR_CONTROL* libraryTreeTool = m_toolMgr->GetTool(); CONDITIONAL_MENU& ctxMenu = m_menu->GetMenu(); - SYMBOL_EDIT_FRAME* editFrame = getEditFrame(); - - wxCHECK( editFrame, false ); auto libSelectedCondition = - [ editFrame ]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { - LIB_ID sel = editFrame->GetTreeLIBID(); - return !sel.GetLibNickname().empty() && sel.GetLibItemName().empty(); + if( SYMBOL_EDIT_FRAME* editFrame = getEditFrame() ) + { + LIB_ID sel = editFrame->GetTreeLIBID(); + return !sel.GetLibNickname().empty() && sel.GetLibItemName().empty(); + } + + return false; }; // The libInferredCondition allows you to do things like New Symbol and Paste with a // symbol selected (in other words, when we know the library context even if the library // itself isn't selected. auto libInferredCondition = - [ editFrame ]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { - LIB_ID sel = editFrame->GetTreeLIBID(); - return !sel.GetLibNickname().empty(); + if( SYMBOL_EDIT_FRAME* editFrame = getEditFrame() ) + { + LIB_ID sel = editFrame->GetTreeLIBID(); + return !sel.GetLibNickname().empty(); + } + + return false; }; auto symbolSelectedCondition = - [ editFrame ]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { - LIB_ID sel = editFrame->GetTargetLibId(); - return !sel.GetLibNickname().empty() && !sel.GetLibItemName().empty(); + if( SYMBOL_EDIT_FRAME* editFrame = getEditFrame() ) + { + LIB_ID sel = editFrame->GetTargetLibId(); + return !sel.GetLibNickname().empty() && !sel.GetLibItemName().empty(); + } + + return false; }; /* not used, but used to be used auto multiSelectedCondition = - [ editFrame ]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { - return editFrame->GetTreeSelectionCount() > 1; + SYMBOL_EDIT_FRAME* editFrame = getEditFrame(); + return editFrame && editFrame->GetTreeSelectionCount() > 1; }; */ auto multiSymbolSelectedCondition = - [ editFrame ]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { - if( editFrame->GetTreeSelectionCount() > 1 ) + SYMBOL_EDIT_FRAME* editFrame = getEditFrame(); + + if( editFrame && editFrame->GetTreeSelectionCount() > 1 ) { for( LIB_ID& sel : editFrame->GetSelectedLibIds() ) { if( !sel.IsValid() ) return false; } + return true; } + return false; }; /* not used, yet auto multiLibrarySelectedCondition = - [ editFrame ]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { - if( editFrame->GetTreeSelectionCount() > 1 ) + SYMBOL_EDIT_FRAME* editFrame = getEditFrame(); + + if( editFrame && editFrame->GetTreeSelectionCount() > 1 ) { for( LIB_ID& sel : editFrame->GetSelectedLibIds() ) { if( sel.IsValid() ) return false; } + return true; } + return false; }; */ auto canOpenExternally = - [ editFrame ]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { // The option is shown if the lib has no current edits - LIB_SYMBOL_LIBRARY_MANAGER& libMgr = editFrame->GetLibManager(); - wxString libName = editFrame->GetTargetLibId().GetLibNickname(); - bool ret = !libMgr.IsLibraryModified( libName ); - return ret; + if( SYMBOL_EDIT_FRAME* editFrame = getEditFrame() ) + { + LIB_SYMBOL_LIBRARY_MANAGER& libMgr = editFrame->GetLibManager(); + wxString libName = editFrame->GetTargetLibId().GetLibNickname(); + return !libMgr.IsLibraryModified( libName ); + } + + return false; }; // clang-format off diff --git a/eeschema/tools/symbol_editor_pin_tool.cpp b/eeschema/tools/symbol_editor_pin_tool.cpp index 5eab4858d4..bbfdbb0266 100644 --- a/eeschema/tools/symbol_editor_pin_tool.cpp +++ b/eeschema/tools/symbol_editor_pin_tool.cpp @@ -100,12 +100,11 @@ bool SYMBOL_EDITOR_PIN_TOOL::Init() SCH_TOOL_BASE::Init(); auto canEdit = - [&]( const SELECTION& sel ) + [this]( const SELECTION& sel ) { SYMBOL_EDIT_FRAME* editor = static_cast( m_frame ); - wxCHECK( editor, false ); - return editor->IsSymbolEditable() && !editor->IsSymbolAlias(); + return editor && editor->IsSymbolEditable() && !editor->IsSymbolAlias(); }; static const std::vector pinTypes = { SCH_PIN_T }; diff --git a/include/tool/edit_table_tool_base.h b/include/tool/edit_table_tool_base.h index d020df6dcf..c0794be40d 100644 --- a/include/tool/edit_table_tool_base.h +++ b/include/tool/edit_table_tool_base.h @@ -49,7 +49,7 @@ protected: PCB_TABLECELL_T } ); auto cellBlockSelection = - [&]( const SELECTION& sel ) + [this]( const SELECTION& sel ) { if( sel.Size() < 2 ) return false; @@ -77,7 +77,7 @@ protected: }; auto mergedCellsSelection = - [&]( const SELECTION& sel ) + [this]( const SELECTION& sel ) { for( EDA_ITEM* item : sel ) { @@ -95,18 +95,13 @@ protected: // Add editing actions to the selection tool menu // selToolMenu.AddSeparator( 100 ); - selToolMenu.AddItem( ACTIONS::addRowAbove, - cellSelection && SELECTION_CONDITIONS::Idle, 100 ); - selToolMenu.AddItem( ACTIONS::addRowBelow, - cellSelection && SELECTION_CONDITIONS::Idle, 100 ); - selToolMenu.AddItem( ACTIONS::addColBefore, - cellSelection && SELECTION_CONDITIONS::Idle, 100 ); - selToolMenu.AddItem( ACTIONS::addColAfter, - cellSelection && SELECTION_CONDITIONS::Idle, 100 ); + selToolMenu.AddItem( ACTIONS::addRowAbove, cellSelection && SELECTION_CONDITIONS::Idle, 100 ); + selToolMenu.AddItem( ACTIONS::addRowBelow, cellSelection && SELECTION_CONDITIONS::Idle, 100 ); + selToolMenu.AddItem( ACTIONS::addColBefore, cellSelection && SELECTION_CONDITIONS::Idle, 100 ); + selToolMenu.AddItem( ACTIONS::addColAfter, cellSelection && SELECTION_CONDITIONS::Idle, 100 ); selToolMenu.AddSeparator( 100 ); - selToolMenu.AddItem( ACTIONS::deleteRows, - cellSelection && SELECTION_CONDITIONS::Idle, 100 ); + selToolMenu.AddItem( ACTIONS::deleteRows, cellSelection && SELECTION_CONDITIONS::Idle, 100 ); selToolMenu.AddItem( ACTIONS::deleteColumns, cellSelection && SELECTION_CONDITIONS::Idle, 100 ); @@ -115,8 +110,7 @@ protected: selToolMenu.AddItem( ACTIONS::unmergeCells, cellSelection && mergedCellsSelection, 100 ); selToolMenu.AddSeparator( 100 ); - selToolMenu.AddItem( ACTIONS::editTable, - cellSelection && SELECTION_CONDITIONS::Idle, 100 ); + selToolMenu.AddItem( ACTIONS::editTable, cellSelection && SELECTION_CONDITIONS::Idle, 100 ); selToolMenu.AddSeparator( 100 ); } diff --git a/pcbnew/router/router_tool.cpp b/pcbnew/router/router_tool.cpp index 013206bc38..0cf14a99f1 100644 --- a/pcbnew/router/router_tool.cpp +++ b/pcbnew/router/router_tool.cpp @@ -485,7 +485,7 @@ bool ROUTER_TOOL::Init() m_menu->RegisterSubMenu( m_diffPairMenu ); auto haveHighlight = - [&]( const SELECTION& sel ) + [this]( const SELECTION& sel ) { KIGFX::RENDER_SETTINGS* cfg = m_toolMgr->GetView()->GetPainter()->GetSettings(); @@ -499,7 +499,7 @@ bool ROUTER_TOOL::Init() }; auto hasOtherEnd = - [&]( const SELECTION& ) + [this]( const SELECTION& ) { std::vector currentNets = m_router->GetCurrentNets(); diff --git a/pcbnew/tools/drawing_tool.cpp b/pcbnew/tools/drawing_tool.cpp index 93afd068ce..6f7bc0f150 100644 --- a/pcbnew/tools/drawing_tool.cpp +++ b/pcbnew/tools/drawing_tool.cpp @@ -194,7 +194,7 @@ DRAWING_TOOL::~DRAWING_TOOL() bool DRAWING_TOOL::Init() { auto haveHighlight = - [&]( const SELECTION& sel ) + [this]( const SELECTION& sel ) { KIGFX::RENDER_SETTINGS* cfg = m_toolMgr->GetView()->GetPainter()->GetSettings(); diff --git a/pcbnew/tools/edit_tool.cpp b/pcbnew/tools/edit_tool.cpp index 0bc9ac5eea..a879841301 100644 --- a/pcbnew/tools/edit_tool.cpp +++ b/pcbnew/tools/edit_tool.cpp @@ -219,7 +219,7 @@ bool EDIT_TOOL::Init() m_selectionTool->GetToolMenu().RegisterSubMenu( shapeModificationSubMenu ); auto positioningToolsCondition = - [&]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { std::shared_ptr subMenu = makePositioningToolsMenu( this ); subMenu->Evaluate( aSel ); @@ -227,7 +227,7 @@ bool EDIT_TOOL::Init() }; auto shapeModificationCondition = - [&]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { std::shared_ptr subMenu = makeShapeModificationMenu( this ); subMenu->Evaluate( aSel ); @@ -235,7 +235,7 @@ bool EDIT_TOOL::Init() }; auto propertiesCondition = - [&]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { if( aSel.GetSize() == 0 ) { @@ -264,13 +264,13 @@ bool EDIT_TOOL::Init() }; auto inFootprintEditor = - [ this ]( const SELECTION& aSelection ) + [this]( const SELECTION& aSelection ) { return m_isFootprintEditor; }; auto canMirror = - [ this ]( const SELECTION& aSelection ) + [this]( const SELECTION& aSelection ) { if( !m_isFootprintEditor && SELECTION_CONDITIONS::OnlyTypes( padTypes )( aSelection ) ) @@ -307,7 +307,7 @@ bool EDIT_TOOL::Init() }; auto noActiveToolCondition = - [ this ]( const SELECTION& aSelection ) + [this]( const SELECTION& aSelection ) { return frame()->ToolStackIsEmpty(); }; @@ -319,13 +319,13 @@ bool EDIT_TOOL::Init() }; auto noItemsCondition = - [ this ]( const SELECTION& aSelections ) -> bool + [this]( const SELECTION& aSelections ) -> bool { return frame()->GetBoard() && !frame()->GetBoard()->IsEmpty(); }; auto isSkippable = - [ this ]( const SELECTION& aSelection ) + [this]( const SELECTION& aSelection ) { return frame()->IsCurrentTool( PCB_ACTIONS::moveIndividually ); }; diff --git a/pcbnew/tools/footprint_editor_control.cpp b/pcbnew/tools/footprint_editor_control.cpp index 57982afbff..1c74c5730f 100644 --- a/pcbnew/tools/footprint_editor_control.cpp +++ b/pcbnew/tools/footprint_editor_control.cpp @@ -83,7 +83,7 @@ bool FOOTPRINT_EDITOR_CONTROL::Init() CONDITIONAL_MENU& ctxMenu = m_menu->GetMenu(); auto libSelectedCondition = - [ this ]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { LIB_ID sel = m_frame->GetLibTree()->GetSelectedLibId(); return !sel.GetLibNickname().empty() && sel.GetLibItemName().empty(); @@ -93,28 +93,28 @@ bool FOOTPRINT_EDITOR_CONTROL::Init() // symbol selected (in other words, when we know the library context even if the library // itself isn't selected. auto libInferredCondition = - [ this ]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { LIB_ID sel = m_frame->GetLibTree()->GetSelectedLibId(); return !sel.GetLibNickname().empty(); }; auto fpSelectedCondition = - [ this ]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { LIB_ID sel = m_frame->GetLibTree()->GetSelectedLibId(); return !sel.GetLibNickname().empty() && !sel.GetLibItemName().empty(); }; auto fpExportCondition = - [ this ]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { FOOTPRINT* fp = m_frame->GetBoard()->GetFirstFootprint(); return fp != nullptr; }; auto canOpenExternally = - [ this ]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { // The option is shown if the editor has no current edits, // dumb/simple guard against opening a new file that does not exist on disk diff --git a/pcbnew/tools/pad_tool.cpp b/pcbnew/tools/pad_tool.cpp index 3e886d442d..4422c8b830 100644 --- a/pcbnew/tools/pad_tool.cpp +++ b/pcbnew/tools/pad_tool.cpp @@ -98,13 +98,13 @@ bool PAD_TOOL::Init() SELECTION_CONDITIONS::OnlyTypes( padTypes ); auto explodeCondition = - [&]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { return m_editPad == niluuid && aSel.Size() == 1 && aSel[0]->Type() == PCB_PAD_T; }; auto recombineCondition = - [&]( const SELECTION& aSel ) + [this]( const SELECTION& aSel ) { return m_editPad != niluuid; }; diff --git a/pcbnew/tools/pcb_point_editor.cpp b/pcbnew/tools/pcb_point_editor.cpp index 14bece5bac..54720437c6 100644 --- a/pcbnew/tools/pcb_point_editor.cpp +++ b/pcbnew/tools/pcb_point_editor.cpp @@ -1947,22 +1947,25 @@ bool PCB_POINT_EDITOR::Init() wxASSERT_MSG( m_selectionTool, wxT( "pcbnew.InteractiveSelection tool is not available" ) ); - const auto addCornerCondition = [&]( const SELECTION& aSelection ) -> bool - { - const EDA_ITEM* item = aSelection.Front(); - return ( item != nullptr ) && canAddCorner( *item ); - }; + const auto addCornerCondition = + [this]( const SELECTION& aSelection ) -> bool + { + const EDA_ITEM* item = aSelection.Front(); + return ( item != nullptr ) && canAddCorner( *item ); + }; - const auto addChamferCondition = [&]( const SELECTION& aSelection ) -> bool - { - const EDA_ITEM* item = aSelection.Front(); - return ( item != nullptr ) && canChamferCorner( *item ); - }; + const auto addChamferCondition = + [this]( const SELECTION& aSelection ) -> bool + { + const EDA_ITEM* item = aSelection.Front(); + return ( item != nullptr ) && canChamferCorner( *item ); + }; - const auto removeCornerCondition = [&]( const SELECTION& aSelection ) -> bool - { - return PCB_POINT_EDITOR::removeCornerCondition( aSelection ); - }; + const auto removeCornerCondition = + [this]( const SELECTION& aSelection ) -> bool + { + return PCB_POINT_EDITOR::removeCornerCondition( aSelection ); + }; using S_C = SELECTION_CONDITIONS; diff --git a/pcbnew/tools/pcb_selection_tool.cpp b/pcbnew/tools/pcb_selection_tool.cpp index f744420fb0..0b439607c1 100644 --- a/pcbnew/tools/pcb_selection_tool.cpp +++ b/pcbnew/tools/pcb_selection_tool.cpp @@ -176,13 +176,14 @@ bool PCB_SELECTION_TOOL::Init() auto& menu = m_menu->GetMenu(); auto activeToolCondition = - [ frame ] ( const SELECTION& aSel ) + [this] ( const SELECTION& aSel ) { - return !frame->ToolStackIsEmpty(); + PCB_BASE_FRAME* frame = getEditFrame(); + return frame && !frame->ToolStackIsEmpty(); }; auto haveHighlight = - [&]( const SELECTION& sel ) + [this]( const SELECTION& sel ) { KIGFX::RENDER_SETTINGS* cfg = m_toolMgr->GetView()->GetPainter()->GetSettings(); diff --git a/pcbnew/tools/pcb_viewer_tools.cpp b/pcbnew/tools/pcb_viewer_tools.cpp index ecc2be4915..01ae0419e8 100644 --- a/pcbnew/tools/pcb_viewer_tools.cpp +++ b/pcbnew/tools/pcb_viewer_tools.cpp @@ -42,7 +42,7 @@ bool PCB_VIEWER_TOOLS::Init() { // Populate the context menu displayed during the tool (primarily the measure tool) auto activeToolCondition = - [ this ] ( const SELECTION& aSel ) + [this] ( const SELECTION& aSel ) { return !frame()->ToolStackIsEmpty(); };