haskal
/
kicad
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix a pair of dereference-freed-pointers in DRC. 

Fixes https://gitlab.com/kicad/code/kicad/issues/10335
This commit is contained in:
Jeff Young 2022-01-10 21:05:14 +00:00
parent 4cdfd14f25
commit 21790fcab7
8 changed files with 58 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
common/rc_item.cpp
View File

@ -496,7 +496,7 @@ void RC_TREE_MODEL::DeleteItems( bool aCurrentOnly, bool aIncludeExclusions, boo
m_view->Freeze(); m_view->Freeze();
} }
for( int i = m_rcItemsProvider->GetCount() - 1; i >= 0; --i ) for( int i = m_rcItemsProvider->GetCount( m_severities ) - 1; i >= 0; --i )
{ {
std::shared_ptr<RC_ITEM> rcItem = m_rcItemsProvider->GetItem( i ); std::shared_ptr<RC_ITEM> rcItem = m_rcItemsProvider->GetItem( i );
MARKER_BASE* marker = rcItem->GetParent(); MARKER_BASE* marker = rcItem->GetParent();

3
pcbnew/dialogs/dialog_drc.cpp
View File

@ -657,6 +657,9 @@ void DIALOG_DRC::OnDRCItemRClick( wxDataViewEvent& aEvent )
{ {
bds().m_DRCSeverities[ rcItem->GetErrorCode() ] = RPT_SEVERITY_IGNORE; bds().m_DRCSeverities[ rcItem->GetErrorCode() ] = RPT_SEVERITY_IGNORE;
m_ignoredList->InsertItem( m_ignoredList->GetItemCount(),
wxT( " • " ) + rcItem->GetErrorText() );
std::vector<PCB_MARKER*>& markers = m_frame->GetBoard()->Markers(); std::vector<PCB_MARKER*>& markers = m_frame->GetBoard()->Markers();
for( unsigned i = 0; i < markers.size(); ) for( unsigned i = 0; i < markers.size(); )

2
pcbnew/dialogs/panel_setup_rules.cpp
View File

@ -441,7 +441,7 @@ void PANEL_SETUP_RULES::OnCompile( wxCommandEvent& event )
try try
{ {
std::vector<DRC_RULE*> dummyRules; std::vector<std::shared_ptr<DRC_RULE>> dummyRules;
DRC_RULES_PARSER parser( m_textEditor->GetText(), _( "DRC rules" ) ); DRC_RULES_PARSER parser( m_textEditor->GetText(), _( "DRC rules" ) );

48
pcbnew/drc/drc_engine.cpp
View File

@ -85,8 +85,7 @@ DRC_ENGINE::DRC_ENGINE( BOARD* aBoard, BOARD_DESIGN_SETTINGS *aSettings ) :
DRC_ENGINE::~DRC_ENGINE() DRC_ENGINE::~DRC_ENGINE()
{ {
for( DRC_RULE* rule : m_rules ) m_rules.clear();
delete rule;
for( std::pair<DRC_CONSTRAINT_T, std::vector<DRC_ENGINE_CONSTRAINT*>*> pair : m_constraintMap ) for( std::pair<DRC_CONSTRAINT_T, std::vector<DRC_ENGINE_CONSTRAINT*>*> pair : m_constraintMap )
{ {
@ -127,9 +126,9 @@ static bool isKeepoutZone( const BOARD_ITEM* aItem, bool aCheckFlags )
} }
DRC_RULE* DRC_ENGINE::createImplicitRule( const wxString& name ) std::shared_ptr<DRC_RULE> DRC_ENGINE::createImplicitRule( const wxString& name )
{ {
DRC_RULE *rule = new DRC_RULE; std::shared_ptr<DRC_RULE> rule = std::make_shared<DRC_RULE>();
rule->m_Name = name; rule->m_Name = name;
rule->m_Implicit = true; rule->m_Implicit = true;
@ -148,7 +147,7 @@ void DRC_ENGINE::loadImplicitRules()
// 1) global defaults // 1) global defaults
DRC_RULE* rule = createImplicitRule( _( "board setup constraints" ) ); std::shared_ptr<DRC_RULE> rule = createImplicitRule( _( "board setup constraints" ) );
DRC_CONSTRAINT clearanceConstraint( CLEARANCE_CONSTRAINT ); DRC_CONSTRAINT clearanceConstraint( CLEARANCE_CONSTRAINT );
clearanceConstraint.Value().SetMin( bds.m_MinClearance ); clearanceConstraint.Value().SetMin( bds.m_MinClearance );
@ -216,7 +215,7 @@ void DRC_ENGINE::loadImplicitRules()
// 2) micro-via specific defaults (new DRC doesn't treat microvias in any special way) // 2) micro-via specific defaults (new DRC doesn't treat microvias in any special way)
DRC_RULE* uViaRule = createImplicitRule( _( "board setup micro-via constraints" ) ); std::shared_ptr<DRC_RULE> uViaRule = createImplicitRule( _( "board setup micro-via constraints" ) );
uViaRule->m_Condition = new DRC_RULE_CONDITION( "A.Via_Type == 'Micro'" ); uViaRule->m_Condition = new DRC_RULE_CONDITION( "A.Via_Type == 'Micro'" );
@ -237,7 +236,7 @@ void DRC_ENGINE::loadImplicitRules()
if( !bds.m_BlindBuriedViaAllowed ) if( !bds.m_BlindBuriedViaAllowed )
{ {
DRC_RULE* bbViaRule = createImplicitRule( _( "board setup constraints" ) ); std::shared_ptr<DRC_RULE> bbViaRule = createImplicitRule( _( "board setup constraints" ) );
bbViaRule->m_Condition = new DRC_RULE_CONDITION( "A.Via_Type == 'Blind/buried'" ); bbViaRule->m_Condition = new DRC_RULE_CONDITION( "A.Via_Type == 'Blind/buried'" );
@ -248,20 +247,18 @@ void DRC_ENGINE::loadImplicitRules()
// 3) per-netclass rules // 3) per-netclass rules
std::vector<DRC_RULE*> netclassClearanceRules; std::vector<std::shared_ptr<DRC_RULE>> netclassClearanceRules;
std::vector<DRC_RULE*> netclassItemSpecificRules; std::vector<std::shared_ptr<DRC_RULE>> netclassItemSpecificRules;
auto makeNetclassRules = auto makeNetclassRules =
[&]( const NETCLASSPTR& nc, bool isDefault ) [&]( const NETCLASSPTR& nc, bool isDefault )
{ {
wxString ncName = nc->GetName(); wxString ncName = nc->GetName();
wxString expr;
DRC_RULE* netclassRule;
wxString expr;
if( nc->GetClearance() || nc->GetTrackWidth() ) if( nc->GetClearance() || nc->GetTrackWidth() )
{ {
netclassRule = new DRC_RULE; std::shared_ptr<DRC_RULE> netclassRule = std::make_shared<DRC_RULE>();
netclassRule->m_Name = wxString::Format( _( "netclass '%s'" ), ncName ); netclassRule->m_Name = wxString::Format( _( "netclass '%s'" ), ncName );
netclassRule->m_Implicit = true; netclassRule->m_Implicit = true;
@ -288,7 +285,7 @@ void DRC_ENGINE::loadImplicitRules()
if( nc->GetDiffPairWidth() ) if( nc->GetDiffPairWidth() )
{ {
netclassRule = new DRC_RULE; std::shared_ptr<DRC_RULE> netclassRule = std::make_shared<DRC_RULE>();
netclassRule->m_Name = wxString::Format( _( "netclass '%s' (diff pair)" ), netclassRule->m_Name = wxString::Format( _( "netclass '%s' (diff pair)" ),
ncName ); ncName );
netclassRule->m_Implicit = true; netclassRule->m_Implicit = true;
@ -305,7 +302,7 @@ void DRC_ENGINE::loadImplicitRules()
if( nc->GetDiffPairGap() ) if( nc->GetDiffPairGap() )
{ {
netclassRule = new DRC_RULE; std::shared_ptr<DRC_RULE> netclassRule = std::make_shared<DRC_RULE>();
netclassRule->m_Name = wxString::Format( _( "netclass '%s' (diff pair)" ), netclassRule->m_Name = wxString::Format( _( "netclass '%s' (diff pair)" ),
ncName ); ncName );
netclassRule->m_Implicit = true; netclassRule->m_Implicit = true;
@ -323,7 +320,7 @@ void DRC_ENGINE::loadImplicitRules()
// trimmed to the board min clearance, which is absolute). // trimmed to the board min clearance, which is absolute).
if( nc->GetDiffPairGap() < nc->GetClearance() ) if( nc->GetDiffPairGap() < nc->GetClearance() )
{ {
netclassRule = new DRC_RULE; netclassRule = std::make_shared<DRC_RULE>();
netclassRule->m_Name = wxString::Format( _( "netclass '%s' (diff pair)" ), netclassRule->m_Name = wxString::Format( _( "netclass '%s' (diff pair)" ),
ncName ); ncName );
netclassRule->m_Implicit = true; netclassRule->m_Implicit = true;
@ -342,7 +339,7 @@ void DRC_ENGINE::loadImplicitRules()
if( nc->GetViaDiameter() || nc->GetViaDrill() ) if( nc->GetViaDiameter() || nc->GetViaDrill() )
{ {
netclassRule = new DRC_RULE; std::shared_ptr<DRC_RULE> netclassRule = std::make_shared<DRC_RULE>();
netclassRule->m_Name = wxString::Format( _( "netclass '%s'" ), ncName ); netclassRule->m_Name = wxString::Format( _( "netclass '%s'" ), ncName );
netclassRule->m_Implicit = true; netclassRule->m_Implicit = true;
@ -370,7 +367,7 @@ void DRC_ENGINE::loadImplicitRules()
if( nc->GetuViaDiameter() || nc->GetuViaDrill() ) if( nc->GetuViaDiameter() || nc->GetuViaDrill() )
{ {
netclassRule = new DRC_RULE; std::shared_ptr<DRC_RULE> netclassRule = std::make_shared<DRC_RULE>();
netclassRule->m_Name = wxString::Format( _( "netclass '%s'" ), ncName ); netclassRule->m_Name = wxString::Format( _( "netclass '%s'" ), ncName );
netclassRule->m_Implicit = true; netclassRule->m_Implicit = true;
@ -409,16 +406,16 @@ void DRC_ENGINE::loadImplicitRules()
// The item-specific netclass rules are all unary, so there's no 'A' vs 'B' issue. // The item-specific netclass rules are all unary, so there's no 'A' vs 'B' issue.
std::sort( netclassClearanceRules.begin(), netclassClearanceRules.end(), std::sort( netclassClearanceRules.begin(), netclassClearanceRules.end(),
[]( DRC_RULE* lhs, DRC_RULE* rhs ) []( const std::shared_ptr<DRC_RULE>& lhs, const std::shared_ptr<DRC_RULE>& rhs )
{ {
return lhs->m_Constraints[0].m_Value.Min() return lhs->m_Constraints[0].m_Value.Min()
< rhs->m_Constraints[0].m_Value.Min(); < rhs->m_Constraints[0].m_Value.Min();
} ); } );
for( DRC_RULE* ncRule : netclassClearanceRules ) for( std::shared_ptr<DRC_RULE>& ncRule : netclassClearanceRules )
addRule( ncRule ); addRule( ncRule );
for( DRC_RULE* ncRule : netclassItemSpecificRules ) for( std::shared_ptr<DRC_RULE>& ncRule : netclassItemSpecificRules )
addRule( ncRule ); addRule( ncRule );
// 3) keepout area rules // 3) keepout area rules
@ -485,7 +482,7 @@ void DRC_ENGINE::loadRules( const wxFileName& aPath )
{ {
if( aPath.FileExists() ) if( aPath.FileExists() )
{ {
std::vector<DRC_RULE*> rules; std::vector<std::shared_ptr<DRC_RULE>> rules;
FILE* fp = wxFopen( aPath.GetFullPath(), wxT( "rt" ) ); FILE* fp = wxFopen( aPath.GetFullPath(), wxT( "rt" ) );
@ -498,7 +495,7 @@ void DRC_ENGINE::loadRules( const wxFileName& aPath )
// Copy the rules into the member variable afterwards so that if Parse() throws then // Copy the rules into the member variable afterwards so that if Parse() throws then
// the possibly malformed rules won't contaminate the current ruleset. // the possibly malformed rules won't contaminate the current ruleset.
for( DRC_RULE* rule : rules ) for( std::shared_ptr<DRC_RULE>& rule : rules )
m_rules.push_back( rule ); m_rules.push_back( rule );
} }
} }
@ -508,7 +505,7 @@ void DRC_ENGINE::compileRules()
{ {
ReportAux( wxString::Format( "Compiling Rules (%d rules): ", (int) m_rules.size() ) ); ReportAux( wxString::Format( "Compiling Rules (%d rules): ", (int) m_rules.size() ) );
for( DRC_RULE* rule : m_rules ) for( std::shared_ptr<DRC_RULE>& rule : m_rules )
{ {
DRC_RULE_CONDITION* condition = nullptr; DRC_RULE_CONDITION* condition = nullptr;
@ -545,9 +542,6 @@ void DRC_ENGINE::InitEngine( const wxFileName& aRulePath )
provider->SetDRCEngine( this ); provider->SetDRCEngine( this );
} }
for( DRC_RULE* rule : m_rules )
delete rule;
m_rules.clear(); m_rules.clear();
m_rulesValid = false; m_rulesValid = false;

40
pcbnew/drc/drc_engine.h
View File

@ -194,7 +194,7 @@ public:
static std::shared_ptr<SHAPE> GetShape( BOARD_ITEM* aItem, PCB_LAYER_ID aLayer ); static std::shared_ptr<SHAPE> GetShape( BOARD_ITEM* aItem, PCB_LAYER_ID aLayer );
private: private:
void addRule( DRC_RULE* rule ) void addRule( std::shared_ptr<DRC_RULE>& rule )
{ {
m_rules.push_back(rule); m_rules.push_back(rule);
} }
@ -210,36 +210,36 @@ private:
struct DRC_ENGINE_CONSTRAINT struct DRC_ENGINE_CONSTRAINT
{ {
LSET layerTest; LSET layerTest;
DRC_RULE_CONDITION* condition; DRC_RULE_CONDITION* condition;
DRC_RULE* parentRule; std::shared_ptr<DRC_RULE> parentRule;
DRC_CONSTRAINT constraint; DRC_CONSTRAINT constraint;
}; };
void loadImplicitRules(); void loadImplicitRules();
DRC_RULE* createImplicitRule( const wxString& name ); std::shared_ptr<DRC_RULE> createImplicitRule( const wxString& name );
protected: protected:
BOARD_DESIGN_SETTINGS* m_designSettings; BOARD_DESIGN_SETTINGS* m_designSettings;
BOARD* m_board; BOARD* m_board;
DS_PROXY_VIEW_ITEM* m_drawingSheet; DS_PROXY_VIEW_ITEM* m_drawingSheet;
NETLIST* m_schematicNetlist; NETLIST* m_schematicNetlist;
std::vector<DRC_RULE*> m_rules; std::vector<std::shared_ptr<DRC_RULE>> m_rules;
bool m_rulesValid; bool m_rulesValid;
std::vector<DRC_TEST_PROVIDER*> m_testProviders; std::vector<DRC_TEST_PROVIDER*> m_testProviders;
EDA_UNITS m_userUnits; EDA_UNITS m_userUnits;
std::vector<int> m_errorLimits; std::vector<int> m_errorLimits;
bool m_reportAllTrackErrors; bool m_reportAllTrackErrors;
bool m_testFootprints; bool m_testFootprints;
// constraint -> rule -> provider // constraint -> rule -> provider
std::unordered_map<DRC_CONSTRAINT_T, std::vector<DRC_ENGINE_CONSTRAINT*>*> m_constraintMap; std::unordered_map<DRC_CONSTRAINT_T, std::vector<DRC_ENGINE_CONSTRAINT*>*> m_constraintMap;
DRC_VIOLATION_HANDLER m_violationHandler; DRC_VIOLATION_HANDLER m_violationHandler;
REPORTER* m_reporter; REPORTER* m_reporter;
PROGRESS_REPORTER* m_progressReporter; PROGRESS_REPORTER* m_progressReporter;
wxString m_msg; // Allocating strings gets expensive enough to want to avoid it wxString m_msg; // Allocating strings gets expensive enough to want to avoid it
std::shared_ptr<KIGFX::VIEW_OVERLAY> m_debugOverlay; std::shared_ptr<KIGFX::VIEW_OVERLAY> m_debugOverlay;

15
pcbnew/drc/drc_rule_parser.cpp
View File

@ -90,7 +90,7 @@ void DRC_RULES_PARSER::parseUnknown()
} }
void DRC_RULES_PARSER::Parse( std::vector<DRC_RULE*>& aRules, REPORTER* aReporter ) void DRC_RULES_PARSER::Parse( std::vector<std::shared_ptr<DRC_RULE>>& aRules, REPORTER* aReporter )
{ {
bool haveVersion = false; bool haveVersion = false;
wxString msg; wxString msg;
@ -146,7 +146,7 @@ void DRC_RULES_PARSER::Parse( std::vector<DRC_RULE*>& aRules, REPORTER* aReporte
break; break;
case T_rule: case T_rule:
aRules.push_back( parseDRC_RULE() ); aRules.emplace_back( parseDRC_RULE() );
break; break;
case T_EOF: case T_EOF:
@ -168,11 +168,12 @@ void DRC_RULES_PARSER::Parse( std::vector<DRC_RULE*>& aRules, REPORTER* aReporte
} }
DRC_RULE* DRC_RULES_PARSER::parseDRC_RULE() std::shared_ptr<DRC_RULE> DRC_RULES_PARSER::parseDRC_RULE()
{ {
DRC_RULE* rule = new DRC_RULE(); std::shared_ptr<DRC_RULE> rule = std::make_shared<DRC_RULE>();
T token = NextTok();
wxString msg; T token = NextTok();
wxString msg;
if( !IsSymbol( token ) ) if( !IsSymbol( token ) )
reportError( _( "Missing rule name." ) ); reportError( _( "Missing rule name." ) );
@ -189,7 +190,7 @@ DRC_RULE* DRC_RULES_PARSER::parseDRC_RULE()
switch( token ) switch( token )
{ {
case T_constraint: case T_constraint:
parseConstraint( rule ); parseConstraint( rule.get() );
break; break;
case T_condition: case T_condition:

4
pcbnew/drc/drc_rule_parser.h
View File

@ -43,10 +43,10 @@ public:
DRC_RULES_PARSER( const wxString& aSource, const wxString& aSourceDescr ); DRC_RULES_PARSER( const wxString& aSource, const wxString& aSourceDescr );
DRC_RULES_PARSER( FILE* aFile, const wxString& aFilename ); DRC_RULES_PARSER( FILE* aFile, const wxString& aFilename );
void Parse( std::vector<DRC_RULE*>& aRules, REPORTER* aReporter ); void Parse( std::vector<std::shared_ptr<DRC_RULE>>& aRules, REPORTER* aReporter );
private: private:
DRC_RULE* parseDRC_RULE(); std::shared_ptr<DRC_RULE> parseDRC_RULE();
void parseConstraint( DRC_RULE* aRule ); void parseConstraint( DRC_RULE* aRule );
void parseValueWithUnits( const wxString& aExpr, int& aResult ); void parseValueWithUnits( const wxString& aExpr, int& aResult );

2
pcbnew/pcb_marker.cpp
View File

@ -78,6 +78,8 @@ PCB_MARKER::PCB_MARKER( std::shared_ptr<RC_ITEM> aItem, const VECTOR2I& aPositio
/* destructor */ /* destructor */
PCB_MARKER::~PCB_MARKER() PCB_MARKER::~PCB_MARKER()
{ {
if( m_rcItem )
m_rcItem->SetParent( nullptr );
} }