From 167bcbcc5f64f90897d2feed8a6ee4a410232ca3 Mon Sep 17 00:00:00 2001 From: klayoutmatthias Date: Mon, 25 Jul 2022 21:06:56 +0200 Subject: [PATCH 1/7] Enabling MSVC debug builds with iterator debugging on --- src/db/db/dbCircuit.cc | 11 ++--- src/db/db/dbDevice.cc | 7 ++-- src/db/db/dbGenericShapeIterator.h | 4 ++ src/db/db/dbHierarchyBuilder.cc | 6 +-- src/db/db/dbSubCircuit.cc | 9 +++-- src/klayout.pri | 23 +++++++++-- src/lay/lay/lay.pro | 11 ----- src/tl/tl/tlIteratorUtils.h | 64 ++++++++++++++++++++++++++++++ 8 files changed, 106 insertions(+), 29 deletions(-) create mode 100644 src/tl/tl/tlIteratorUtils.h diff --git a/src/db/db/dbCircuit.cc b/src/db/db/dbCircuit.cc index bf11738b2..bb2d4281e 100644 --- a/src/db/db/dbCircuit.cc +++ b/src/db/db/dbCircuit.cc @@ -23,6 +23,7 @@ #include "dbCircuit.h" #include "dbNetlist.h" #include "dbLayout.h" +#include "tlIteratorUtils.h" #include @@ -169,7 +170,7 @@ const Pin *Circuit::pin_by_id (size_t id) const return 0; } else { pin_list::iterator pi = m_pin_by_id [id]; - if (pi == pin_list::iterator ()) { + if (tl::is_null_iterator (pi)) { return 0; } else { return pi.operator-> (); @@ -179,7 +180,7 @@ const Pin *Circuit::pin_by_id (size_t id) const void Circuit::rename_pin (size_t id, const std::string &name) { - if (id < m_pin_by_id.size () && m_pin_by_id [id] != pin_list::iterator ()) { + if (id < m_pin_by_id.size () && ! tl::is_null_iterator (m_pin_by_id [id])) { m_pin_by_id [id]->set_name (name); } } @@ -327,7 +328,7 @@ Pin &Circuit::add_pin (const std::string &name) void Circuit::remove_pin (size_t id) { - if (id < m_pin_by_id.size () && m_pin_by_id [id] != pin_list::iterator ()) { + if (id < m_pin_by_id.size () && ! tl::is_null_iterator (m_pin_by_id [id])) { m_pins.erase (m_pin_by_id [id]); m_pin_by_id [id] = pin_list::iterator (); } @@ -653,7 +654,7 @@ const Net *Circuit::net_for_pin (size_t pin_id) const { if (pin_id < m_pin_refs.size ()) { Net::pin_iterator p = m_pin_refs [pin_id]; - if (p != Net::pin_iterator ()) { + if (! tl::is_null_iterator (p)) { return p->net (); } } @@ -668,7 +669,7 @@ void Circuit::connect_pin (size_t pin_id, Net *net) if (pin_id < m_pin_refs.size ()) { Net::pin_iterator p = m_pin_refs [pin_id]; - if (p != Net::pin_iterator () && p->net ()) { + if (! tl::is_null_iterator (p) && p->net ()) { p->net ()->erase_pin (p); } m_pin_refs [pin_id] = Net::pin_iterator (); diff --git a/src/db/db/dbDevice.cc b/src/db/db/dbDevice.cc index 3440c143f..eadffc772 100644 --- a/src/db/db/dbDevice.cc +++ b/src/db/db/dbDevice.cc @@ -23,6 +23,7 @@ #include "dbDevice.h" #include "dbCircuit.h" #include "dbDeviceClass.h" +#include "tlIteratorUtils.h" namespace db { @@ -39,7 +40,7 @@ Device::Device () Device::~Device () { for (std::vector::const_iterator t = m_terminal_refs.begin (); t != m_terminal_refs.end (); ++t) { - if (*t != Net::terminal_iterator () && (*t)->net ()) { + if (! tl::is_null_iterator (*t) && (*t)->net ()) { (*t)->net ()->erase_terminal (*t); } } @@ -125,7 +126,7 @@ const Net *Device::net_for_terminal (size_t terminal_id) const { if (terminal_id < m_terminal_refs.size ()) { Net::terminal_iterator p = m_terminal_refs [terminal_id]; - if (p != Net::terminal_iterator ()) { + if (! tl::is_null_iterator (p)) { return p->net (); } } @@ -140,7 +141,7 @@ void Device::connect_terminal (size_t terminal_id, Net *net) if (terminal_id < m_terminal_refs.size ()) { Net::terminal_iterator p = m_terminal_refs [terminal_id]; - if (p != Net::terminal_iterator () && p->net ()) { + if (! tl::is_null_iterator (p) && p->net ()) { p->net ()->erase_terminal (p); } m_terminal_refs [terminal_id] = Net::terminal_iterator (); diff --git a/src/db/db/dbGenericShapeIterator.h b/src/db/db/dbGenericShapeIterator.h index 6b55a2ea3..3de6953b9 100644 --- a/src/db/db/dbGenericShapeIterator.h +++ b/src/db/db/dbGenericShapeIterator.h @@ -94,7 +94,11 @@ public: virtual bool equals (const generic_shape_iterator_delegate_base *other) const { const generic_shape_iterator_delegate2 *o = dynamic_cast *> (other); +#if defined(_MSC_VER) && defined(_ITERATOR_DEBUG_LEVEL) && _ITERATOR_DEBUG_LEVEL >= 2 + return o && o->m_iter._Unwrapped () == m_iter._Unwrapped (); +#else return o && o->m_iter == m_iter; +#endif } private: diff --git a/src/db/db/dbHierarchyBuilder.cc b/src/db/db/dbHierarchyBuilder.cc index 0093bd9bb..77e5ea235 100644 --- a/src/db/db/dbHierarchyBuilder.cc +++ b/src/db/db/dbHierarchyBuilder.cc @@ -25,14 +25,14 @@ #include "dbClip.h" #include "dbRegion.h" #include "dbPolygonTools.h" +#include "tlIteratorUtils.h" namespace db { static HierarchyBuilderShapeInserter def_inserter; -static HierarchyBuilder::cell_map_type null_map; -static HierarchyBuilder::cell_map_type::const_iterator null_iterator = null_map.end (); +static HierarchyBuilder::cell_map_type::const_iterator null_iterator = HierarchyBuilder::cell_map_type::const_iterator (); // ------------------------------------------------------------------------------------------- @@ -286,7 +286,7 @@ HierarchyBuilder::end (const RecursiveShapeIterator *iter) void HierarchyBuilder::enter_cell (const RecursiveShapeIterator * /*iter*/, const db::Cell * /*cell*/, const db::Box & /*region*/, const box_tree_type * /*complex_region*/) { - tl_assert (m_cm_entry != m_cell_map.end () && m_cm_entry != null_iterator); + tl_assert (! tl::is_equal_iterator_unchecked (m_cm_entry, null_iterator) && m_cm_entry != m_cell_map.end ()); m_cells_seen.insert (m_cm_entry->first); diff --git a/src/db/db/dbSubCircuit.cc b/src/db/db/dbSubCircuit.cc index 72a37221b..851a27647 100644 --- a/src/db/db/dbSubCircuit.cc +++ b/src/db/db/dbSubCircuit.cc @@ -22,6 +22,7 @@ #include "dbSubCircuit.h" #include "dbCircuit.h" +#include "tlIteratorUtils.h" namespace db { @@ -38,7 +39,7 @@ SubCircuit::SubCircuit () SubCircuit::~SubCircuit() { for (std::vector::const_iterator p = m_pin_refs.begin (); p != m_pin_refs.end (); ++p) { - if (*p != Net::subcircuit_pin_iterator () && (*p)->net ()) { + if (! tl::is_null_iterator (*p) && (*p)->net ()) { (*p)->net ()->erase_subcircuit_pin (*p); } } @@ -112,7 +113,7 @@ const Net *SubCircuit::net_for_pin (size_t pin_id) const { if (pin_id < m_pin_refs.size ()) { Net::subcircuit_pin_iterator p = m_pin_refs [pin_id]; - if (p != Net::subcircuit_pin_iterator ()) { + if (! tl::is_null_iterator (p)) { return p->net (); } } @@ -123,7 +124,7 @@ const NetSubcircuitPinRef *SubCircuit::netref_for_pin (size_t pin_id) const { if (pin_id < m_pin_refs.size ()) { Net::subcircuit_pin_iterator p = m_pin_refs [pin_id]; - if (p != Net::subcircuit_pin_iterator ()) { + if (! tl::is_null_iterator (p)) { return p.operator-> (); } } @@ -138,7 +139,7 @@ void SubCircuit::connect_pin (size_t pin_id, Net *net) if (pin_id < m_pin_refs.size ()) { Net::subcircuit_pin_iterator p = m_pin_refs [pin_id]; - if (p != Net::subcircuit_pin_iterator () && p->net ()) { + if (! tl::is_null_iterator (p) && p->net ()) { p->net ()->erase_subcircuit_pin (p); } m_pin_refs [pin_id] = Net::subcircuit_pin_iterator (); diff --git a/src/klayout.pri b/src/klayout.pri index ad0504dd2..48ddb352c 100644 --- a/src/klayout.pri +++ b/src/klayout.pri @@ -145,9 +145,10 @@ msvc { QMAKE_CXXFLAGS_WARN_ON += \ - # as we're using default-constructed iterators as "null" we can't have - # checked iterators with MSVC - DEFINES += _ITERATOR_DEBUG_LEVEL=0 + # for stack trace support: + # lpsapi for GetModuleFileName and others + # dbghelp for SymFromAddr and other + LIBS += -lpsapi -ldbghelp } else { @@ -171,9 +172,25 @@ msvc { QMAKE_CXXFLAGS += -std=c++11 win32 { + QMAKE_LFLAGS += -Wl,--exclude-all-symbols + + # for stack trace support: + # lpsapi for GetModuleFileName and others + # dbghelp for SymFromAddr and other + LIBS += -lpsapi -ldbghelp + + QMAKE_CXXFLAGS += -Wa,-mbig-obj + } else { + QMAKE_CXXFLAGS += -fvisibility=hidden + + } + + *bsd* { + # stack trace support + LIBS += -lexecinfo } } diff --git a/src/lay/lay/lay.pro b/src/lay/lay/lay.pro index c93131cc1..3a6001cc0 100644 --- a/src/lay/lay/lay.pro +++ b/src/lay/lay/lay.pro @@ -181,17 +181,6 @@ INCLUDEPATH += $$TL_INC $$GSI_INC $$DB_INC $$RDB_INC $$LAYBASIC_INC $$LAYUI_INC DEPENDPATH += $$TL_INC $$GSI_INC $$DB_INC $$RDB_INC $$LAYBASIC_INC $$LAYUI_INC $$LAYVIEW_INC $$ANT_INC $$IMG_INC $$EDT_INC $$LYM_INC LIBS += -L$$DESTDIR -lklayout_tl -lklayout_gsi -lklayout_db -lklayout_rdb -lklayout_lym -lklayout_laybasic -lklayout_layview -lklayout_layui -lklayout_ant -lklayout_img -lklayout_edt -win32 { - # for stack trace support: - # lpsapi for GetModuleFileName and others - # dbghelp for SymFromAddr and other - LIBS += -lpsapi -ldbghelp -} - -*bsd* { - LIBS += -lexecinfo -} - INCLUDEPATH += $$QTBASIC_INC DEPENDPATH += $$QTBASIC_INC diff --git a/src/tl/tl/tlIteratorUtils.h b/src/tl/tl/tlIteratorUtils.h new file mode 100644 index 000000000..89aa7874a --- /dev/null +++ b/src/tl/tl/tlIteratorUtils.h @@ -0,0 +1,64 @@ + +/* + + KLayout Layout Viewer + Copyright (C) 2006-2022 Matthias Koefferlein + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + +*/ + + +#ifndef HDR_tlIteratorUtils +#define HDR_tlIteratorUtils + +namespace tl +{ + +/** + * @brief Checks an iterator against the default-constructed value + * + * This function takes care of using the right way of comparing iterators + * suitable for MSVC with iterator debugging on. + */ +template +static inline bool is_null_iterator (const Iter &iter) +{ +#if defined(_MSC_VER) && defined(_ITERATOR_DEBUG_LEVEL) && _ITERATOR_DEBUG_LEVEL >= 2 + return iter._Unwrapped () == Iter ()._Unwrapped (); +#else + return iter == Iter (); +#endif +} + +/** + * @brief Checks an iterator against another one without iterator checking + * + * This function takes care of using the right way of comparing iterators + * suitable for MSVC with iterator debugging on. + */ +template +static inline bool is_equal_iterator_unchecked (const Iter &a, const Iter &b) +{ +#if defined(_MSC_VER) && defined(_ITERATOR_DEBUG_LEVEL) && _ITERATOR_DEBUG_LEVEL >= 2 + return a._Unwrapped () == b._Unwrapped (); +#else + return a == b; +#endif +} + +} + +#endif From da398e77fddf3f935ed88cf16394b05040163cfe Mon Sep 17 00:00:00 2001 From: klayoutmatthias Date: Mon, 25 Jul 2022 22:03:01 +0200 Subject: [PATCH 2/7] [Consider merging] avoid a memory corruption issue in Netlist::flatten --- src/db/db/dbNetlist.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/db/db/dbNetlist.cc b/src/db/db/dbNetlist.cc index 7f0a04077..3cc519076 100644 --- a/src/db/db/dbNetlist.cc +++ b/src/db/db/dbNetlist.cc @@ -505,6 +505,8 @@ void Netlist::flatten_circuit (Circuit *circuit) void Netlist::flatten () { + db::NetlistLocker locker (this); + std::set top_circuits; size_t ntop = top_circuit_count (); for (db::Netlist::top_down_circuit_iterator tc = begin_top_down (); tc != end_top_down () && ntop > 0; ++tc) { From 50a863bb2685f01c656f97df3254de681808b172 Mon Sep 17 00:00:00 2001 From: klayoutmatthias Date: Tue, 26 Jul 2022 00:13:26 +0200 Subject: [PATCH 3/7] [Consider merging] Fixed a potential memory corruption problem --- src/db/db/dbStreamLayers.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/db/db/dbStreamLayers.cc b/src/db/db/dbStreamLayers.cc index c29945d1f..7c8625ce4 100644 --- a/src/db/db/dbStreamLayers.cc +++ b/src/db/db/dbStreamLayers.cc @@ -787,7 +787,7 @@ LayerMap::unmap (const LDPair &p1, const LDPair &p2) if (db::is_static_ld (p1.layer) && db::is_static_ld (p2.layer)) { m_ld_map.add (p1.layer, p2.layer + 1, LayerMap::datatype_map (), op); } else { - m_ld_map.add (m_ld_map.begin ()->first.first, m_ld_map.end ()->first.second, LayerMap::datatype_map (), op); + m_ld_map.add (m_ld_map.begin ()->first.first, (--m_ld_map.end ())->first.second, LayerMap::datatype_map (), op); } } From 0401f3b89cb3564db1fc8f600c2e245b5b633b28 Mon Sep 17 00:00:00 2001 From: klayoutmatthias Date: Tue, 26 Jul 2022 08:28:22 +0200 Subject: [PATCH 4/7] Fixed a non-problem that pops up with iterator assertions --- src/layui/layui/layNetlistBrowserModel.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/layui/layui/layNetlistBrowserModel.cc b/src/layui/layui/layNetlistBrowserModel.cc index 39c3031d1..c70e6455d 100644 --- a/src/layui/layui/layNetlistBrowserModel.cc +++ b/src/layui/layui/layNetlistBrowserModel.cc @@ -457,6 +457,7 @@ static std::vectorfirst, j->first)); nn2.erase (j); found = true; + break; } } From 5a9e3f91882a948685dd05f3fd07ad1eaa6c2647 Mon Sep 17 00:00:00 2001 From: klayoutmatthias Date: Tue, 26 Jul 2022 21:02:17 +0200 Subject: [PATCH 5/7] [Consider merging] One more iterator issue fixed --- src/db/db/gsiDeclDbNetlist.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/db/db/gsiDeclDbNetlist.cc b/src/db/db/gsiDeclDbNetlist.cc index fdaab035a..83f4a429f 100644 --- a/src/db/db/gsiDeclDbNetlist.cc +++ b/src/db/db/gsiDeclDbNetlist.cc @@ -197,10 +197,10 @@ static bool is_combined_device (const db::Device *device) return ! device->reconnected_terminals ().empty (); } +static std::vector empty; + static std::vector::const_iterator begin_reconnected_terminals_for (const db::Device *device, size_t terminal_id) { - static std::vector empty; - const std::vector *ti = device->reconnected_terminals_for ((unsigned int) terminal_id); if (! ti) { return empty.begin (); @@ -211,8 +211,6 @@ static std::vector::const_iterator begin_reconnec static std::vector::const_iterator end_reconnected_terminals_for (const db::Device *device, size_t terminal_id) { - static std::vector empty; - const std::vector *ti = device->reconnected_terminals_for ((unsigned int) terminal_id); if (! ti) { return empty.end (); From bca7486082446a2d910754494d8d16ee67dff9fd Mon Sep 17 00:00:00 2001 From: klayoutmatthias Date: Tue, 26 Jul 2022 21:02:48 +0200 Subject: [PATCH 6/7] [Consider merging] object lifetime problem with QLatin1String and GSI fixed --- src/gsi/gsi/gsiSerialisation.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/gsi/gsi/gsiSerialisation.h b/src/gsi/gsi/gsiSerialisation.h index 6feb7d8cf..4b89dfc62 100644 --- a/src/gsi/gsi/gsiSerialisation.h +++ b/src/gsi/gsi/gsiSerialisation.h @@ -1036,11 +1036,12 @@ public: return m_s_utf8.constData (); } - virtual void set (const char *c_str, size_t s, tl::Heap &) + virtual void set (const char *c_str, size_t s, tl::Heap &heap) { if (! m_is_const) { - m_latin1_holder = QString::fromUtf8 (c_str, int (s)).toLatin1 (); - *mp_s = QLatin1String (m_latin1_holder.constData (), m_latin1_holder.size ()); + QByteArray *latin1_holder = new QByteArray (QString::fromUtf8 (c_str, int (s)).toLatin1 ()); + heap.push (latin1_holder); + *mp_s = QLatin1String (latin1_holder->constData (), latin1_holder->size ()); } } @@ -1048,7 +1049,6 @@ private: QLatin1String *mp_s; bool m_is_const; QLatin1String m_s; - QByteArray m_latin1_holder; mutable QByteArray m_s_utf8; }; From 1b728f1067320f4e97921728d832a18483a12a87 Mon Sep 17 00:00:00 2001 From: klayoutmatthias Date: Thu, 28 Jul 2022 20:52:19 +0200 Subject: [PATCH 7/7] Removed duplicated method declarations in LayoutViewBase --- .../laybasic/gsiDeclLayLayoutViewBase.cc | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/src/laybasic/laybasic/gsiDeclLayLayoutViewBase.cc b/src/laybasic/laybasic/gsiDeclLayLayoutViewBase.cc index fbbbb4ed9..1a4e28170 100644 --- a/src/laybasic/laybasic/gsiDeclLayLayoutViewBase.cc +++ b/src/laybasic/laybasic/gsiDeclLayLayoutViewBase.cc @@ -553,18 +553,6 @@ LAYBASIC_PUBLIC Class decl_LayoutViewBase (QT_EXTERNAL_BASE "\n" "This constant has been introduced in version 0.27.\n" ) + - gsi::method ("call_menu", static_cast (&lay::LayoutViewBase::menu_activated), - "@brief Calls the menu item with the provided symbol.\n" - "To obtain all symbols, use get_menu_symbols.\n" - "\n" - "This method has been introduced in version 0.27." - ) + - gsi::method ("menu_symbols", &lay::LayoutViewBase::menu_symbols, - "@brief Gets all available menu symbols (see \\call_menu).\n" - "NOTE: currently this method delivers a superset of all available symbols. Depending on the context, no all symbols may trigger actual functionality.\n" - "\n" - "This method has been introduced in version 0.27." - ) + gsi::method ("stop_redraw", static_cast (&lay::LayoutViewBase::stop_redraw), "@brief Stops the redraw thread\n" "\n" @@ -993,13 +981,13 @@ LAYBASIC_PUBLIC Class decl_LayoutViewBase (QT_EXTERNAL_BASE "@brief Calls the menu item with the provided symbol.\n" "To obtain all symbols, use \\menu_symbols.\n" "\n" - "This method has been introduced in version 0.28." + "This method has been introduced in version 0.27." ) + gsi::method ("menu_symbols", &lay::LayoutViewBase::menu_symbols, - "@brief Gets all available menu symbols.\n" + "@brief Gets all available menu symbols (see \\call_menu).\n" "NOTE: currently this method delivers a superset of all available symbols. Depending on the context, no all symbols may trigger actual functionality.\n" "\n" - "This method has been introduced in version 0.28." + "This method has been introduced in version 0.27." ) + gsi::method ("cancel", &lay::LayoutViewBase::cancel, "@brief Cancels all edit operations\n"