Fixed #646 (crash on delete of library) (#685)

* Fixed a compiler warning

* Fixed #646 (crash on PCell Library "delete")

The issue was mainly caused by a "cleanup" call on
the library. Cleanup is supposed to remove excess
top level PCell variants. For libraries this is not
possible, as the library does not know which variants
are used and which are not.

In addition, some hardening against segfaults in
case of defect layouts has been applied.
This commit is contained in:
Matthias Köfferlein 2020-11-27 18:36:56 +01:00 committed by GitHub
parent 22df10f425
commit 511e30ef45
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 51 additions and 5 deletions

View File

@ -272,6 +272,7 @@ Layout::Layout (db::Manager *manager)
m_properties_repository (this), m_properties_repository (this),
m_guiding_shape_layer (-1), m_guiding_shape_layer (-1),
m_waste_layer (-1), m_waste_layer (-1),
m_do_cleanup (false),
m_editable (db::default_editable_mode ()) m_editable (db::default_editable_mode ())
{ {
// .. nothing yet .. // .. nothing yet ..
@ -287,6 +288,7 @@ Layout::Layout (bool editable, db::Manager *manager)
m_properties_repository (this), m_properties_repository (this),
m_guiding_shape_layer (-1), m_guiding_shape_layer (-1),
m_waste_layer (-1), m_waste_layer (-1),
m_do_cleanup (false),
m_editable (editable) m_editable (editable)
{ {
// .. nothing yet .. // .. nothing yet ..
@ -306,6 +308,7 @@ Layout::Layout (const db::Layout &layout)
m_properties_repository (this), m_properties_repository (this),
m_guiding_shape_layer (-1), m_guiding_shape_layer (-1),
m_waste_layer (-1), m_waste_layer (-1),
m_do_cleanup (false),
m_editable (layout.m_editable) m_editable (layout.m_editable)
{ {
*this = layout; *this = layout;
@ -1285,6 +1288,12 @@ Layout::allocate_new_cell ()
void void
Layout::cleanup (const std::set<db::cell_index_type> &keep) Layout::cleanup (const std::set<db::cell_index_type> &keep)
{ {
// only managed layouts will receive cleanup requests. Never library container layouts - these
// cannot know if their proxies are not referenced by other proxies.
if (! m_do_cleanup) {
return;
}
// deleting cells may create new top cells which need to be deleted as well, hence we iterate // deleting cells may create new top cells which need to be deleted as well, hence we iterate
// until there are no more cells to delete // until there are no more cells to delete
while (true) { while (true) {

View File

@ -539,6 +539,21 @@ public:
*/ */
Layout &operator= (const Layout &d); Layout &operator= (const Layout &d);
/**
* @brief Specifies, if the layout participates in cleanup
*
* "cleanup" will be called to get rid of top level proxies.
* This flag controls whether cleanup happens or not. Library
* layouts for example must not loose proxies as they might
* themselves be referenced.
*
* The default is OFF.
*/
void do_cleanup (bool f)
{
m_do_cleanup = f;
}
/** /**
* @brief Clear the layout * @brief Clear the layout
*/ */
@ -1761,6 +1776,7 @@ private:
lib_proxy_map m_lib_proxy_map; lib_proxy_map m_lib_proxy_map;
int m_guiding_shape_layer; int m_guiding_shape_layer;
int m_waste_layer; int m_waste_layer;
bool m_do_cleanup;
bool m_editable; bool m_editable;
meta_info m_meta_info; meta_info m_meta_info;
tl::Mutex m_lock; tl::Mutex m_lock;

View File

@ -103,7 +103,8 @@ Library::unregister_proxy (db::LibraryProxy *lib_proxy, db::Layout *ly)
db::cell_index_type ci = c->first; db::cell_index_type ci = c->first;
m_refcount.erase (c); m_refcount.erase (c);
// remove cells which are itself proxies and are no longer used // remove cells which are itself proxies and are no longer used
if (layout ().cell (ci).is_proxy () && layout ().cell (ci).parent_cells () == 0) { db::Cell *lib_cell = &layout ().cell (ci);
if (lib_cell && lib_cell->is_proxy () && lib_cell->parent_cells () == 0) {
layout ().delete_cell (ci); layout ().delete_cell (ci);
} }
} }
@ -190,6 +191,7 @@ Library::remap_to (db::Library *other)
if (! pn.first) { if (! pn.first) {
// substitute by static layout cell
std::string name = r->first->cell_name (ci); std::string name = r->first->cell_name (ci);
db::Cell *old_cell = r->first->take_cell (ci); db::Cell *old_cell = r->first->take_cell (ci);
r->first->insert_cell (ci, name, new db::Cell (*old_cell)); r->first->insert_cell (ci, name, new db::Cell (*old_cell));
@ -201,6 +203,7 @@ Library::remap_to (db::Library *other)
const db::PCellDeclaration *new_pcell_decl = other->layout ().pcell_declaration (pn.second); const db::PCellDeclaration *new_pcell_decl = other->layout ().pcell_declaration (pn.second);
if (! old_pcell_decl || ! new_pcell_decl) { if (! old_pcell_decl || ! new_pcell_decl) {
// substitute by static layout cell
std::string name = r->first->cell_name (ci); std::string name = r->first->cell_name (ci);
db::Cell *old_cell = r->first->take_cell (ci); db::Cell *old_cell = r->first->take_cell (ci);
r->first->insert_cell (ci, name, new db::Cell (*old_cell)); r->first->insert_cell (ci, name, new db::Cell (*old_cell));
@ -229,7 +232,7 @@ Library::remap_to (db::Library *other)
if (! cn.first) { if (! cn.first) {
// unlink this proxy // unlink this proxy: substitute by static layout cell
std::string name = r->first->cell_name (ci); std::string name = r->first->cell_name (ci);
db::Cell *old_cell = r->first->take_cell (ci); db::Cell *old_cell = r->first->take_cell (ci);
r->first->insert_cell (ci, name, new db::Cell (*old_cell)); r->first->insert_cell (ci, name, new db::Cell (*old_cell));

View File

@ -238,7 +238,12 @@ LibraryProxy::get_basic_name () const
{ {
Library *lib = LibraryManager::instance ().lib (lib_id ()); Library *lib = LibraryManager::instance ().lib (lib_id ());
if (lib) { if (lib) {
return lib->layout ().cell (library_cell_index ()).get_basic_name (); const db::Cell *lib_cell = &lib->layout ().cell (library_cell_index ());
if (! lib_cell) {
return "<defunct>";
} else {
return lib_cell->get_basic_name ();
}
} else { } else {
return Cell::get_basic_name (); return Cell::get_basic_name ();
} }
@ -249,7 +254,12 @@ LibraryProxy::get_display_name () const
{ {
Library *lib = LibraryManager::instance ().lib (lib_id ()); Library *lib = LibraryManager::instance ().lib (lib_id ());
if (lib) { if (lib) {
return lib->get_name () + "." + lib->layout ().cell (library_cell_index ()).get_display_name (); const db::Cell *lib_cell = &lib->layout ().cell (library_cell_index ());
if (! lib_cell) {
return lib->get_name () + "." + "<defunct>";
} else {
return lib->get_name () + "." + lib_cell->get_display_name ();
}
} else { } else {
return Cell::get_display_name (); return Cell::get_display_name ();
} }
@ -260,7 +270,12 @@ LibraryProxy::get_qualified_name () const
{ {
Library *lib = LibraryManager::instance ().lib (lib_id ()); Library *lib = LibraryManager::instance ().lib (lib_id ());
if (lib) { if (lib) {
return lib->get_name () + "." + lib->layout ().cell (library_cell_index ()).get_qualified_name (); const db::Cell *lib_cell = &lib->layout ().cell (library_cell_index ());
if (! lib_cell) {
return lib->get_name () + "." + "<defunct>";
} else {
return lib->get_name () + "." + lib_cell->get_qualified_name ();
}
} else { } else {
return Cell::get_qualified_name (); return Cell::get_qualified_name ();
} }

View File

@ -59,6 +59,9 @@ LayoutHandle::LayoutHandle (db::Layout *layout, const std::string &filename)
m_dirty (false), m_dirty (false),
m_save_options_valid (false) m_save_options_valid (false)
{ {
// layouts in the managed layouts space participate in spare proxy cleanup
layout->do_cleanup (true);
file_watcher ().add_file (m_filename); file_watcher ().add_file (m_filename);
if (! m_filename.empty ()) { if (! m_filename.empty ()) {