From 511e30ef459d994bb27a4ed9693c1551ee56d0be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20K=C3=B6fferlein?= Date: Fri, 27 Nov 2020 18:36:56 +0100 Subject: [PATCH] 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. --- src/db/db/dbLayout.cc | 9 +++++++++ src/db/db/dbLayout.h | 16 ++++++++++++++++ src/db/db/dbLibrary.cc | 7 +++++-- src/db/db/dbLibraryProxy.cc | 21 ++++++++++++++++++--- src/laybasic/laybasic/layCellView.cc | 3 +++ 5 files changed, 51 insertions(+), 5 deletions(-) diff --git a/src/db/db/dbLayout.cc b/src/db/db/dbLayout.cc index 199441231..2e767684e 100644 --- a/src/db/db/dbLayout.cc +++ b/src/db/db/dbLayout.cc @@ -272,6 +272,7 @@ Layout::Layout (db::Manager *manager) m_properties_repository (this), m_guiding_shape_layer (-1), m_waste_layer (-1), + m_do_cleanup (false), m_editable (db::default_editable_mode ()) { // .. nothing yet .. @@ -287,6 +288,7 @@ Layout::Layout (bool editable, db::Manager *manager) m_properties_repository (this), m_guiding_shape_layer (-1), m_waste_layer (-1), + m_do_cleanup (false), m_editable (editable) { // .. nothing yet .. @@ -306,6 +308,7 @@ Layout::Layout (const db::Layout &layout) m_properties_repository (this), m_guiding_shape_layer (-1), m_waste_layer (-1), + m_do_cleanup (false), m_editable (layout.m_editable) { *this = layout; @@ -1285,6 +1288,12 @@ Layout::allocate_new_cell () void Layout::cleanup (const std::set &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 // until there are no more cells to delete while (true) { diff --git a/src/db/db/dbLayout.h b/src/db/db/dbLayout.h index fd6d897aa..f1555cb61 100644 --- a/src/db/db/dbLayout.h +++ b/src/db/db/dbLayout.h @@ -539,6 +539,21 @@ public: */ 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 */ @@ -1761,6 +1776,7 @@ private: lib_proxy_map m_lib_proxy_map; int m_guiding_shape_layer; int m_waste_layer; + bool m_do_cleanup; bool m_editable; meta_info m_meta_info; tl::Mutex m_lock; diff --git a/src/db/db/dbLibrary.cc b/src/db/db/dbLibrary.cc index 6c0ca490c..0e10fe3e9 100644 --- a/src/db/db/dbLibrary.cc +++ b/src/db/db/dbLibrary.cc @@ -103,7 +103,8 @@ Library::unregister_proxy (db::LibraryProxy *lib_proxy, db::Layout *ly) db::cell_index_type ci = c->first; m_refcount.erase (c); // 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); } } @@ -190,6 +191,7 @@ Library::remap_to (db::Library *other) if (! pn.first) { + // substitute by static layout cell std::string name = r->first->cell_name (ci); db::Cell *old_cell = r->first->take_cell (ci); 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); if (! old_pcell_decl || ! new_pcell_decl) { + // substitute by static layout cell std::string name = r->first->cell_name (ci); db::Cell *old_cell = r->first->take_cell (ci); r->first->insert_cell (ci, name, new db::Cell (*old_cell)); @@ -229,7 +232,7 @@ Library::remap_to (db::Library *other) if (! cn.first) { - // unlink this proxy + // unlink this proxy: substitute by static layout cell std::string name = r->first->cell_name (ci); db::Cell *old_cell = r->first->take_cell (ci); r->first->insert_cell (ci, name, new db::Cell (*old_cell)); diff --git a/src/db/db/dbLibraryProxy.cc b/src/db/db/dbLibraryProxy.cc index db254964a..195c82b99 100644 --- a/src/db/db/dbLibraryProxy.cc +++ b/src/db/db/dbLibraryProxy.cc @@ -238,7 +238,12 @@ LibraryProxy::get_basic_name () const { Library *lib = LibraryManager::instance ().lib (lib_id ()); 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 ""; + } else { + return lib_cell->get_basic_name (); + } } else { return Cell::get_basic_name (); } @@ -249,7 +254,12 @@ LibraryProxy::get_display_name () const { Library *lib = LibraryManager::instance ().lib (lib_id ()); 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 () + "." + ""; + } else { + return lib->get_name () + "." + lib_cell->get_display_name (); + } } else { return Cell::get_display_name (); } @@ -260,7 +270,12 @@ LibraryProxy::get_qualified_name () const { Library *lib = LibraryManager::instance ().lib (lib_id ()); 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 () + "." + ""; + } else { + return lib->get_name () + "." + lib_cell->get_qualified_name (); + } } else { return Cell::get_qualified_name (); } diff --git a/src/laybasic/laybasic/layCellView.cc b/src/laybasic/laybasic/layCellView.cc index f76937caa..0b28e12d0 100644 --- a/src/laybasic/laybasic/layCellView.cc +++ b/src/laybasic/laybasic/layCellView.cc @@ -59,6 +59,9 @@ LayoutHandle::LayoutHandle (db::Layout *layout, const std::string &filename) m_dirty (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); if (! m_filename.empty ()) {