sch connection: prevent double-free on bus members

StoreImplicitConnection accepted a raw SCH_CONNECTION* and
~CONNECTION_SUBGRAPH always deleted everything in
m_bus_element_connections. A double‑free could happen if that same
pointer was also deleted somewhere else. Make the ownership semantics
more clear. Hopefully this fixes / prevents future issues.

Fixes: https://gitlab.com/kicad/code/kicad/-/issues/17413
This commit is contained in:
Mike Williams
2026-02-04 08:36:55 -05:00
parent 19d463d144
commit 913d38b74b
2 changed files with 23 additions and 12 deletions
+5 -4
View File
@@ -1746,17 +1746,18 @@ void CONNECTION_GRAPH::generateBusAliasMembers()
// This connection cannot form a part of the item because the item is not, itself
// connected to this subgraph. It exists as part of a virtual item that may be
// connected to other items but is not in the schematic.
SCH_CONNECTION* new_conn = new SCH_CONNECTION( item, subgraph->m_sheet );
auto new_conn = std::make_unique<SCH_CONNECTION>( item, subgraph->m_sheet );
new_conn->SetGraph( this );
new_conn->SetName( name );
new_conn->SetType( CONNECTION_TYPE::NET );
subgraph->StoreImplicitConnection( new_conn );
int code = assignNewNetCode( *new_conn );
SCH_CONNECTION* new_conn_ptr = subgraph->StoreImplicitConnection( std::move( new_conn ) );
int code = assignNewNetCode( *new_conn_ptr );
wxLogTrace( ConnTrace, wxS( "SG(%ld), Adding full local name (%s) with sg (%d) on subsheet %s" ),
subgraph->m_code, name, code, subgraph->m_sheet.PathHumanReadable() );
new_sg->m_driver_connection = new_conn;
new_sg->m_driver_connection = new_conn_ptr;
new_sg->m_code = m_last_subgraph_code++;
new_sg->m_sheet = subgraph->GetSheet();
new_sg->m_is_bus_member = true;
+18 -8
View File
@@ -22,6 +22,7 @@
#ifndef _CONNECTION_GRAPH_H
#define _CONNECTION_GRAPH_H
#include <memory>
#include <mutex>
#include <utility>
#include <vector>
@@ -93,11 +94,6 @@ public:
m_driver_connection( nullptr )
{}
~CONNECTION_SUBGRAPH()
{
for( SCH_CONNECTION* connection : m_bus_element_connections )
delete connection;
}
friend class CONNECTION_GRAPH;
@@ -219,9 +215,13 @@ public:
// Use this to keep a connection pointer that is not owned by any item
// This will be destroyed with the subgraph
void StoreImplicitConnection( SCH_CONNECTION* aConnection )
SCH_CONNECTION* StoreImplicitConnection( std::unique_ptr<SCH_CONNECTION> aConnection )
{
m_bus_element_connections.insert( aConnection );
SCH_CONNECTION* raw = aConnection.get();
m_bus_element_connections.insert( std::move( aConnection ) );
return raw;
}
private:
@@ -317,9 +317,19 @@ private:
/// Cache for driver connection.
SCH_CONNECTION* m_driver_connection;
// A comparator for unique_ptr<SCH_CONNECTION> to allow storage in a set
struct CompareConnectionPtr
{
bool operator()( const std::unique_ptr<SCH_CONNECTION>& aLeft,
const std::unique_ptr<SCH_CONNECTION>& aRight ) const
{
return aLeft.get() < aRight.get();
}
};
/// A cache of connections that are part of this subgraph but that don't have
/// an owning element (i.e. bus members)
std::set<SCH_CONNECTION*> m_bus_element_connections;
std::set<std::unique_ptr<SCH_CONNECTION>, CompareConnectionPtr> m_bus_element_connections;
std::mutex m_driver_mutex;
};