From ceceda37ccb570feae9e00a2871d248de3929c30 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Sun, 31 Oct 2021 19:57:52 +0000 Subject: [PATCH] No more long-lived parsers. We've had too many bugs from improper re-initialization. Fixes https://gitlab.com/kicad/code/kicad/issues/9429 --- common/drawing_sheet/drawing_sheet_parser.cpp | 12 ++++-- pcbnew/kicad_clipboard.cpp | 19 ++++----- .../netlist_reader/kicad_netlist_reader.cpp | 9 ++-- pcbnew/netlist_reader/netlist_reader.h | 32 +++----------- pcbnew/plugins/kicad/pcb_parser.h | 40 +++--------------- pcbnew/plugins/kicad/pcb_plugin.cpp | 42 +++++++------------ pcbnew/plugins/kicad/pcb_plugin.h | 3 +- .../tools/pcb_parser/pcb_parser_tool.cpp | 32 +++++--------- qa/pcbnew_utils/board_file_utils.cpp | 18 +------- 9 files changed, 61 insertions(+), 146 deletions(-) diff --git a/common/drawing_sheet/drawing_sheet_parser.cpp b/common/drawing_sheet/drawing_sheet_parser.cpp index 3329739609..d848b5d5ab 100644 --- a/common/drawing_sheet/drawing_sheet_parser.cpp +++ b/common/drawing_sheet/drawing_sheet_parser.cpp @@ -857,16 +857,20 @@ void DS_DATA_MODEL::SetPageLayout( const char* aPageLayout, bool Append, const w if( ! Append ) ClearList(); - DRAWING_SHEET_PARSER lp_parser( aPageLayout, wxT( "Sexpr_string" ) ); + DRAWING_SHEET_PARSER parser( aPageLayout, wxT( "Sexpr_string" ) ); try { - lp_parser.Parse( this ); + parser.Parse( this ); } catch( const IO_ERROR& ioe ) { wxLogMessage( ioe.What() ); } + catch( const std::bad_alloc& ) + { + wxLogMessage( "Memory exhaustion reading drawing sheet" ); + } } @@ -920,11 +924,11 @@ bool DS_DATA_MODEL::LoadDrawingSheet( const wxString& aFullFileName, bool Append if( ! Append ) ClearList(); - DRAWING_SHEET_PARSER pl_parser( buffer.get(), fullFileName ); + DRAWING_SHEET_PARSER parser( buffer.get(), fullFileName ); try { - pl_parser.Parse( this ); + parser.Parse( this ); } catch( const IO_ERROR& ioe ) { diff --git a/pcbnew/kicad_clipboard.cpp b/pcbnew/kicad_clipboard.cpp index 6e98b69a1b..98efba298b 100644 --- a/pcbnew/kicad_clipboard.cpp +++ b/pcbnew/kicad_clipboard.cpp @@ -2,6 +2,7 @@ * This program source code file is part of KiCad, a free EDA CAD application. * * Copyright (C) 2017 KiCad Developers, see AUTHORS.TXT for contributors. + * Copyright (C) 2017-2021 KiCad Developers, see AUTHORS.txt for contributors. * @author Kristoffer Ödmark * * This program is free software; you can redistribute it and/or @@ -25,7 +26,6 @@ #include #include -#include #include #include #include @@ -401,19 +401,17 @@ BOARD* CLIPBOARD_IO::Load( const wxString& aFileName, BOARD* aAppendToMe, result = data.GetText().mb_str(); } - STRING_LINE_READER reader(result, wxT( "clipboard" ) ); + STRING_LINE_READER reader( result, wxT( "clipboard" ) ); + PCB_PARSER parser( &reader, aAppendToMe ); init( aProperties ); - m_parser->SetLineReader( &reader ); - m_parser->SetBoard( aAppendToMe ); - BOARD_ITEM* item; BOARD* board; try { - item = m_parser->Parse(); + item = parser.Parse(); } catch( const FUTURE_FORMAT_ERROR& ) { @@ -422,8 +420,8 @@ BOARD* CLIPBOARD_IO::Load( const wxString& aFileName, BOARD* aAppendToMe, } catch( const PARSE_ERROR& parse_error ) { - if( m_parser->IsTooRecent() ) - throw FUTURE_FORMAT_ERROR( parse_error, m_parser->GetRequiredVersion() ); + if( parser.IsTooRecent() ) + throw FUTURE_FORMAT_ERROR( parse_error, parser.GetRequiredVersion() ); else throw; } @@ -431,9 +429,8 @@ BOARD* CLIPBOARD_IO::Load( const wxString& aFileName, BOARD* aAppendToMe, if( item->Type() != PCB_T ) { // The parser loaded something that was valid, but wasn't a board. - THROW_PARSE_ERROR( _( "Clipboard content is not KiCad compatible" ), - m_parser->CurSource(), m_parser->CurLine(), - m_parser->CurLineNumber(), m_parser->CurOffset() ); + THROW_PARSE_ERROR( _( "Clipboard content is not KiCad compatible" ), parser.CurSource(), + parser.CurLine(), parser.CurLineNumber(), parser.CurOffset() ); } else { diff --git a/pcbnew/netlist_reader/kicad_netlist_reader.cpp b/pcbnew/netlist_reader/kicad_netlist_reader.cpp index a437e4a88c..6a08596f5b 100644 --- a/pcbnew/netlist_reader/kicad_netlist_reader.cpp +++ b/pcbnew/netlist_reader/kicad_netlist_reader.cpp @@ -1,11 +1,8 @@ -/** - * @file kicad_netlist_reader.cpp - */ /* * This program source code file is part of KiCad, a free EDA CAD application. * * Copyright (C) 1992-2011 Jean-Pierre Charras. - * Copyright (C) 1992-2020 KiCad Developers, see change_log.txt for contributors. + * Copyright (C) 1992-2021 KiCad Developers, see change_log.txt for contributors. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -36,7 +33,9 @@ using namespace NL_T; void KICAD_NETLIST_READER::LoadNetlist() { - m_parser->Parse(); + KICAD_NETLIST_PARSER parser( m_lineReader, m_netlist ); + + parser.Parse(); if( m_footprintReader ) { diff --git a/pcbnew/netlist_reader/netlist_reader.h b/pcbnew/netlist_reader/netlist_reader.h index 7f775a7c55..f91af7ea0c 100644 --- a/pcbnew/netlist_reader/netlist_reader.h +++ b/pcbnew/netlist_reader/netlist_reader.h @@ -1,10 +1,3 @@ -#ifndef NETLIST_READER_H -#define NETLIST_READER_H - -/** - * @file netlist_reader.h - */ - /* * This program source code file is part of KiCad, a free EDA CAD application. * @@ -30,6 +23,9 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA */ +#ifndef NETLIST_READER_H +#define NETLIST_READER_H + #include #include @@ -172,11 +168,6 @@ public: */ virtual void LoadNetlist() = 0; - /** - * @return the #LINE_READER associated with the #NETLIST_READER. - */ - LINE_READER* GetLineReader(); - protected: NETLIST* m_netlist; ///< The net list to read the file(s) into. bool m_loadFootprintFilters; ///< Load the component footprint filters section if true. @@ -280,10 +271,6 @@ class KICAD_NETLIST_PARSER : public NETLIST_LEXER public: KICAD_NETLIST_PARSER( LINE_READER* aReader, NETLIST* aNetlist ); - void SetLineReader( LINE_READER* aLineReader ); - - void SetNetlist( NETLIST* aNetlist ) { m_netlist = aNetlist; } - /** * Function Parse * parse the full netlist @@ -364,20 +351,13 @@ public: KICAD_NETLIST_READER( LINE_READER* aLineReader, NETLIST* aNetlist, CMP_READER* aFootprintLinkReader = nullptr ) : - NETLIST_READER( aLineReader, aNetlist, aFootprintLinkReader ), - m_parser( new KICAD_NETLIST_PARSER( aLineReader, aNetlist ) ) - { - } + NETLIST_READER( aLineReader, aNetlist, aFootprintLinkReader ) + { } virtual ~KICAD_NETLIST_READER() - { - delete m_parser; - } + { } virtual void LoadNetlist() override; - -private: - KICAD_NETLIST_PARSER* m_parser; ///< The s-expression format parser. }; diff --git a/pcbnew/plugins/kicad/pcb_parser.h b/pcbnew/plugins/kicad/pcb_parser.h index 4b13096411..d01af95123 100644 --- a/pcbnew/plugins/kicad/pcb_parser.h +++ b/pcbnew/plugins/kicad/pcb_parser.h @@ -72,48 +72,20 @@ class PROGRESS_REPORTER; class PCB_PARSER : public PCB_LEXER { public: - PCB_PARSER( LINE_READER* aReader = nullptr ) : + PCB_PARSER( LINE_READER* aReader, BOARD* aBoard = nullptr, + PROGRESS_REPORTER* aProgressReporter = nullptr, unsigned aLineCount = 0 ) : PCB_LEXER( aReader ), - m_board( nullptr ), - m_resetKIIDs( false ), - m_progressReporter( nullptr ), + m_board( aBoard ), + m_resetKIIDs( aBoard != nullptr ), + m_progressReporter( aProgressReporter ), m_lastProgressTime( std::chrono::steady_clock::now() ), - m_lineCount( 0 ) + m_lineCount( aLineCount ) { init(); } // ~PCB_PARSER() {} - /** - * Set @a aLineReader into the parser, and returns the previous one, if any. - * - * @param aReader is what to read from for tokens, no ownership is received. - * @return LINE_READER* - previous LINE_READER or NULL if none. - */ - LINE_READER* SetLineReader( LINE_READER* aReader ) - { - LINE_READER* ret = PopReader(); - PushReader( aReader ); - return ret; - } - - void SetBoard( BOARD* aBoard ) - { - init(); - m_board = aBoard; - - if( aBoard != nullptr ) - m_resetKIIDs = true; - } - - void SetProgressReporter( PROGRESS_REPORTER* aProgressReporter, unsigned aLineCount ) - { - m_progressReporter = aProgressReporter; - m_lastProgressTime = std::chrono::steady_clock::now(); - m_lineCount = aLineCount; - } - BOARD_ITEM* Parse(); /** diff --git a/pcbnew/plugins/kicad/pcb_plugin.cpp b/pcbnew/plugins/kicad/pcb_plugin.cpp index 2aac0b9046..8df127ea1d 100644 --- a/pcbnew/plugins/kicad/pcb_plugin.cpp +++ b/pcbnew/plugins/kicad/pcb_plugin.cpp @@ -262,16 +262,10 @@ void FP_CACHE::Load() // Queue I/O errors so only files that fail to parse don't get loaded. try { - FILE_LINE_READER reader( fn.GetFullPath() ); + FILE_LINE_READER reader( fn.GetFullPath() ); + PCB_PARSER parser( &reader ); - m_owner->m_parser->SetLineReader( &reader ); - - // For better or worse (mostly worse), the parser is a long-lived object. - // Make sure we start with a fresh state. - m_owner->m_parser->InitParserState(); - m_owner->m_parser->SetBoard( nullptr ); // calls PCB_PARSER::init() - - FOOTPRINT* footprint = (FOOTPRINT*) m_owner->m_parser->Parse(); + FOOTPRINT* footprint = (FOOTPRINT*) parser.Parse(); wxString fpName = fn.GetName(); footprint->SetFPID( LIB_ID( wxEmptyString, fpName ) ); @@ -379,18 +373,17 @@ BOARD_ITEM* PCB_PLUGIN::Parse( const wxString& aClipboardSourceInput ) { std::string input = TO_UTF8( aClipboardSourceInput ); - STRING_LINE_READER reader( input, wxT( "clipboard" ) ); - - m_parser->SetLineReader( &reader ); + STRING_LINE_READER reader( input, wxT( "clipboard" ) ); + PCB_PARSER parser( &reader ); try { - return m_parser->Parse(); + return parser.Parse(); } catch( const PARSE_ERROR& parse_error ) { - if( m_parser->IsTooRecent() ) - throw FUTURE_FORMAT_ERROR( parse_error, m_parser->GetRequiredVersion() ); + if( parser.IsTooRecent() ) + throw FUTURE_FORMAT_ERROR( parse_error, parser.GetRequiredVersion() ); else throw; } @@ -2285,7 +2278,6 @@ void PCB_PLUGIN::format( const ZONE* aZone, int aNestLevel ) const PCB_PLUGIN::PCB_PLUGIN( int aControlFlags ) : m_cache( nullptr ), m_ctl( aControlFlags ), - m_parser( new PCB_PARSER() ), m_mapping( new NETINFO_MAPPING() ) { init( nullptr ); @@ -2296,7 +2288,6 @@ PCB_PLUGIN::PCB_PLUGIN( int aControlFlags ) : PCB_PLUGIN::~PCB_PLUGIN() { delete m_cache; - delete m_parser; delete m_mapping; } @@ -2336,15 +2327,12 @@ BOARD* PCB_PLUGIN::DoLoad( LINE_READER& aReader, BOARD* aAppendToMe, const PROPE { init( aProperties ); - m_parser->SetLineReader( &aReader ); - m_parser->SetBoard( aAppendToMe ); - m_parser->SetProgressReporter( aProgressReporter, aLineCount ); - - BOARD* board; + PCB_PARSER parser( &aReader, aAppendToMe, aProgressReporter, aLineCount ); + BOARD* board; try { - board = dynamic_cast( m_parser->Parse() ); + board = dynamic_cast( parser.Parse() ); } catch( const FUTURE_FORMAT_ERROR& ) { @@ -2353,8 +2341,8 @@ BOARD* PCB_PLUGIN::DoLoad( LINE_READER& aReader, BOARD* aAppendToMe, const PROPE } catch( const PARSE_ERROR& parse_error ) { - if( m_parser->IsTooRecent() ) - throw FUTURE_FORMAT_ERROR( parse_error, m_parser->GetRequiredVersion() ); + if( parser.IsTooRecent() ) + throw FUTURE_FORMAT_ERROR( parse_error, parser.GetRequiredVersion() ); else throw; } @@ -2362,8 +2350,8 @@ BOARD* PCB_PLUGIN::DoLoad( LINE_READER& aReader, BOARD* aAppendToMe, const PROPE if( !board ) { // The parser loaded something that was valid, but wasn't a board. - THROW_PARSE_ERROR( _( "This file does not contain a PCB." ), m_parser->CurSource(), - m_parser->CurLine(), m_parser->CurLineNumber(), m_parser->CurOffset() ); + THROW_PARSE_ERROR( _( "This file does not contain a PCB." ), parser.CurSource(), + parser.CurLine(), parser.CurLineNumber(), parser.CurOffset() ); } return board; diff --git a/pcbnew/plugins/kicad/pcb_plugin.h b/pcbnew/plugins/kicad/pcb_plugin.h index 5658a45b1f..0ef6b6265f 100644 --- a/pcbnew/plugins/kicad/pcb_plugin.h +++ b/pcbnew/plugins/kicad/pcb_plugin.h @@ -2,7 +2,7 @@ * This program source code file is part of KiCad, a free EDA CAD application. * * Copyright (C) 2012 CERN. - * Copyright (C) 1992-2020 KiCad Developers, see AUTHORS.txt for contributors. + * Copyright (C) 1992-2021 KiCad Developers, see AUTHORS.txt for contributors. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -297,7 +297,6 @@ protected: STRING_FORMATTER m_sf; OUTPUTFORMATTER* m_out; ///< output any Format()s to this, no ownership int m_ctl; - PCB_PARSER* m_parser; NETINFO_MAPPING* m_mapping; ///< mapping for net codes, so only not empty net codes ///< are stored with consecutive integers as net codes }; diff --git a/qa/pcbnew_tools/tools/pcb_parser/pcb_parser_tool.cpp b/qa/pcbnew_tools/tools/pcb_parser/pcb_parser_tool.cpp index f9cf0e8d66..2d632f5dd2 100644 --- a/qa/pcbnew_tools/tools/pcb_parser/pcb_parser_tool.cpp +++ b/qa/pcbnew_tools/tools/pcb_parser/pcb_parser_tool.cpp @@ -1,7 +1,7 @@ /* * This program source code file is part of KiCad, a free EDA CAD application. * - * Copyright (C) 2019 KiCad Developers, see AUTHORS.TXT for contributors. + * Copyright (C) 2019-2021 KiCad Developers, see AUTHORS.TXT for contributors. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -25,7 +25,6 @@ #include #include - #include #include @@ -35,11 +34,7 @@ #include #include #include - -#include - #include -#include using PARSE_DURATION = std::chrono::microseconds; @@ -57,10 +52,7 @@ bool parse( std::istream& aStream, bool aVerbose ) STDISTREAM_LINE_READER reader; reader.SetStream( aStream ); - PCB_PARSER parser; - - parser.SetLineReader( &reader ); - + PCB_PARSER parser( &reader ); BOARD_ITEM* board = nullptr; PARSE_DURATION duration{}; @@ -110,10 +102,9 @@ int pcb_parser_main_func( int argc, char** argv ) wxMessageOutput::Set( new wxMessageOutputStderr ); wxCmdLineParser cl_parser( argc, argv ); cl_parser.SetDesc( g_cmdLineDesc ); - cl_parser.AddUsageText( - _( "This program parses PCB files, either from the " - "stdin stream or from the given filenames. This can be used either for " - "standalone testing of the parser or for fuzz testing." ) ); + cl_parser.AddUsageText( _( "This program parses PCB files, either from the stdin stream or " + "from the given filenames. This can be used either for standalone " + "testing of the parser or for fuzz testing." ) ); int cmd_parsed_ok = cl_parser.Parse(); if( cmd_parsed_ok != 0 ) @@ -122,11 +113,9 @@ int pcb_parser_main_func( int argc, char** argv ) return ( cmd_parsed_ok == -1 ) ? KI_TEST::RET_CODES::OK : KI_TEST::RET_CODES::BAD_CMDLINE; } - const bool verbose = cl_parser.Found( "verbose" ); - - bool ok = true; - - const auto file_count = cl_parser.GetParamCount(); + const bool verbose = cl_parser.Found( "verbose" ); + bool ok = true; + const size_t file_count = cl_parser.GetParamCount(); if( file_count == 0 ) { @@ -163,5 +152,6 @@ int pcb_parser_main_func( int argc, char** argv ) } -static bool registered = UTILITY_REGISTRY::Register( - { "pcb_parser", "Parse a KiCad PCB file", pcb_parser_main_func } ); +static bool registered = UTILITY_REGISTRY::Register( { "pcb_parser", + "Parse a KiCad PCB file", + pcb_parser_main_func } ); diff --git a/qa/pcbnew_utils/board_file_utils.cpp b/qa/pcbnew_utils/board_file_utils.cpp index c34d233b3a..5fba200c18 100644 --- a/qa/pcbnew_utils/board_file_utils.cpp +++ b/qa/pcbnew_utils/board_file_utils.cpp @@ -1,7 +1,7 @@ /* * This program source code file is part of KiCad, a free EDA CAD application. * - * Copyright (C) 2019 KiCad Developers, see AUTHORS.txt for contributors. + * Copyright (C) 2019-2021 KiCad Developers, see AUTHORS.txt for contributors. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -69,27 +69,13 @@ void DumpBoardToFile( BOARD& board, const std::string& aFilename ) } -std::unique_ptr ReadBoardItemFromFile( const std::string& aFilename ) -{ - FILE_LINE_READER reader( aFilename ); - - PCB_PARSER parser; - parser.SetLineReader( &reader ); - - return std::unique_ptr( parser.Parse() ); -} - - std::unique_ptr ReadBoardItemFromStream( std::istream& aStream ) { // Take input from stdin STDISTREAM_LINE_READER reader; reader.SetStream( aStream ); - PCB_PARSER parser; - - parser.SetLineReader( &reader ); - + PCB_PARSER parser( &reader ); std::unique_ptr board; try