luke
/
klayout
mirror of https://github.com/KLayout/klayout.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fixed a problem triggered by the new foreign layout handling 

Problem was that with forward-referenced text strings
in OASIS, StringRef objects were created (string placeholders).
Those where not properly migrated when transferring the foreign
layouts into DEF space, causing a memory corruption issue.

This solution is to provide a central, singleton string
repository. This needs to be MT safe (reference counting)
which should also fix potential problems when accessing
StringRef-based Text objects from multiple threads.
This commit is contained in:
Matthias Koefferlein 2024-05-26 01:03:24 +02:00
parent cf8ff2f750
commit 81872d41f0
7 changed files with 145 additions and 163 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
src/db/db/dbLayout.cc
View File

@ -816,7 +816,6 @@ Layout::mem_stat (MemStatistics *stat, MemStatistics::purpose_t purpose, int cat
db::mem_stat (stat, purpose, cat, m_pcell_ids, true, (void *) this);
db::mem_stat (stat, purpose, cat, m_lib_proxy_map, true, (void *) this);
db::mem_stat (stat, purpose, cat, m_meta_info, true, (void *) this);
db::mem_stat (stat, purpose, cat, m_string_repository, true, (void *) this);
db::mem_stat (stat, purpose, cat, m_shape_repository, true, (void *) this);
db::mem_stat (stat, purpose, cat, m_properties_repository, true, (void *) this);
db::mem_stat (stat, purpose, cat, m_array_repository, true, (void *) this);

17
src/db/db/dbLayout.h
View File

@ -588,22 +588,6 @@ public:
return m_array_repository;
}
/**
* @brief Accessor to the string repository
*/
StringRepository &string_repository ()
{
return m_string_repository;
}
/**
* @brief Accessor to the string repository (const version)
*/
const StringRepository &string_repository () const
{
return m_string_repository;
}
/**
* @brief Accessor to the shape repository
*/
@ -2170,7 +2154,6 @@ private:
cell_map_type m_cell_map;
double m_dbu;
db::properties_id_type m_prop_id;
StringRepository m_string_repository;
GenericRepository m_shape_repository;
PropertiesRepository m_properties_repository;
ArrayRepository m_array_repository;

72
src/db/db/dbText.cc
View File

@ -22,10 +22,12 @@
#include "dbText.h"
#include "tlThreads.h"
namespace db
{
static char halign2code (db::HAlign ha)
{
if (ha == db::HAlignCenter) {
@ -78,13 +80,79 @@ static db::VAlign extract_valign (tl::Extractor &ex)
}
}
// ----------------------------------------------------------------------------
// StringRepository implementation
static StringRepository s_repository;
static StringRepository *sp_repository = 0;
static tl::Mutex s_repository_lock;
StringRepository *
StringRepository::instance ()
{
return sp_repository;
}
StringRepository::StringRepository ()
{
sp_repository = this;
}
StringRepository::~StringRepository ()
{
if (sp_repository == this) {
sp_repository = 0;
}
for (std::set<StringRef *>::const_iterator s = m_string_refs.begin (); s != m_string_refs.end (); ++s) {
delete *s;
}
}
const StringRef *
StringRepository::create_string_ref ()
{
tl::MutexLocker locker (&s_repository_lock);
StringRef *ref = new StringRef ();
m_string_refs.insert (ref);
return ref;
}
void
StringRepository::unregister_ref (StringRef *ref)
{
tl::MutexLocker locker (&s_repository_lock);
if (! m_string_refs.empty ()) {
m_string_refs.erase (ref);
}
}
// ----------------------------------------------------------------------------
// StringRef implementation
static tl::Mutex s_ref_lock;
StringRef::~StringRef ()
{
if (mp_rep) {
mp_rep->unregister_ref (this);
if (StringRepository::instance ()) {
StringRepository::instance ()->unregister_ref (this);
}
}
void
StringRef::add_ref ()
{
tl::MutexLocker locker (&s_ref_lock);
++m_ref_count;
}
void
StringRef::remove_ref ()
{
tl::MutexLocker locker (&s_ref_lock);
--m_ref_count;
if (m_ref_count == 0) {
delete this;
}
}

155
src/db/db/dbText.h
View File

@ -61,21 +61,12 @@ public:
/**
* @brief Increment the reference counter
*/
void add_ref ()
{
++m_ref_count;
}
void add_ref ();
/**
* @brief Decrement the reference counter and remove the object when it reaches 0
*/
void remove_ref ()
{
--m_ref_count;
if (m_ref_count == 0) {
delete this;
}
}
void remove_ref ();
/**
* @brief Assignment of a std::string object
@ -102,14 +93,6 @@ public:
return m_value;
}
/**
* @brief Access to the repository the strings are in
*/
const StringRepository *rep () const
{
return mp_rep;
}
void mem_stat (MemStatistics *stat, MemStatistics::purpose_t purpose, int cat, bool no_self = false, void *parent = 0) const
{
if (! no_self) {
@ -120,15 +103,15 @@ public:
private:
friend class StringRepository;
StringRepository *mp_rep;
std::string m_value;
size_t m_ref_count;
/**
* @brief Hidden constructor attaching the reference to a repository
*/
StringRef (StringRepository *rep)
: mp_rep (rep), m_ref_count (0)
StringRef ()
: m_ref_count (0)
{
// .. nothing yet ..
}
@ -165,22 +148,12 @@ public:
/**
* @brief Constructor
*/
StringRepository ()
{
// .. nothing yet ..
}
StringRepository ();
/**
* @brief Destructor
*/
~StringRepository ()
{
std::set<StringRef *> st;
m_string_refs.swap (st);
for (std::set<StringRef *>::const_iterator s = st.begin (); s != st.end (); ++s) {
delete *s;
}
}
~StringRepository ();
/**
* @brief Create a string reference object.
@ -196,77 +169,37 @@ public:
* A string reference's text can be set by using the change_string_ref
* method.
*/
const StringRef *create_string_ref ()
{
StringRef *ref = new StringRef (this);
m_string_refs.insert (ref);
return ref;
}
const StringRef *create_string_ref ();
/**
* @brief Change the string associated with a StringRef
*/
void change_string_ref (const StringRef *ref, const std::string &s)
static void change_string_ref (const StringRef *ref, const std::string &s)
{
*(const_cast<StringRef *> (ref)) = s;
}
/**
* @brief For debugging purposes: get the number of entries
* @brief The singleton instance of the string repository
*/
size_t size () const
static StringRepository *instance ();
/**
* @brief For testing purposes: size of the repository
*/
size_t size ()
{
return m_string_refs.size ();
}
/**
* @brief Iterates over the string refs (begin)
*/
iterator begin () const
{
return m_string_refs.begin ();
}
/**
* @brief Iterates over the string refs (end)
*/
iterator end () const
{
return m_string_refs.end ();
}
void mem_stat (MemStatistics *stat, MemStatistics::purpose_t purpose, int cat, bool no_self = false, void *parent = 0) const
{
if (! no_self) {
stat->add (typeid (*this), (void *) this, sizeof (*this), sizeof (*this), parent, purpose, cat);
}
db::mem_stat (stat, purpose, cat, &m_string_refs, true, (void *) this);
for (std::set<StringRef *>::const_iterator r = m_string_refs.begin (); r != m_string_refs.end (); ++r) {
db::mem_stat (stat, purpose, cat, **r, true, parent);
}
}
private:
friend class StringRef;
std::set<StringRef *> m_string_refs;
void unregister_ref (StringRef *ref)
{
if (! m_string_refs.empty ()) {
m_string_refs.erase (ref);
}
}
void unregister_ref (StringRef *ref);
};
/**
* @brief Collect memory statistics
*/
inline void mem_stat (MemStatistics *stat, MemStatistics::purpose_t purpose, int cat, const StringRepository &x, bool no_self = false, void *parent = 0)
{
x.mem_stat (stat, purpose, cat, no_self, parent);
}
/**
* @brief A text object
*
@ -457,14 +390,7 @@ public:
*/
void translate (const text<C> &d, db::generic_repository<C> &, db::ArrayRepository &)
{
// don't use StringRef's on translate - since those live in the source layout, we must not copy them
m_trans = d.m_trans;
m_size = d.m_size;
m_font = d.m_font;
m_halign = d.m_halign;
m_valign = d.m_valign;
string (d.string ());
*this = d;
}
/**
@ -473,7 +399,7 @@ public:
template <class T>
void translate (const text<C> &d, const T &t, db::generic_repository<C> &r, db::ArrayRepository &a)
{
translate (d, r, a);
*this = d;
transform (t);
}
@ -854,24 +780,11 @@ private:
if (c != 0) {
return c < 0;
}
} else {
if (mp_ptr != b.mp_ptr) {
// if references are present, use their pointers rather than the strings
// if they belong to the same collection
const StringRef *r1 = reinterpret_cast<const StringRef *> (mp_ptr - 1);
const StringRef *r2 = reinterpret_cast<const StringRef *> (b.mp_ptr - 1);
if (r1->rep () != r2->rep ()) {
int c = strcmp (r1->value ().c_str (), r2->value ().c_str ());
if (c != 0) {
return c < 0;
}
} else {
return mp_ptr < b.mp_ptr;
}
}
} else if (mp_ptr != b.mp_ptr) {
// if references are present, use their pointers rather than the strings
return mp_ptr < b.mp_ptr;
}
#if 1
// Compare size and presentation flags - without that, the text repository does not work properly.
if (m_size != b.m_size) {
return m_size < b.m_size;
@ -885,7 +798,6 @@ private:
if (m_valign != b.m_valign) {
return m_valign < b.m_valign;
}
#endif
return false;
}
@ -900,33 +812,16 @@ private:
if (c != 0) {
return false;
}
} else {
if (mp_ptr != b.mp_ptr) {
// if references are present, use their pointers rather than the strings
// if they belong to the same collection
const StringRef *r1 = reinterpret_cast<const StringRef *> (mp_ptr - 1);
const StringRef *r2 = reinterpret_cast<const StringRef *> (b.mp_ptr - 1);
if (r1->rep () != r2->rep ()) {
int c = strcmp (r1->value ().c_str (), r2->value ().c_str ());
if (c != 0) {
return false;
}
} else {
return false;
}
}
} else if (mp_ptr != b.mp_ptr) {
// if references are present, use their pointers rather than the strings
return false;
}
#if 1
// Compare size and presentation flags - without that, the text repository does not work properly.
if (m_size != b.m_size) {
return false;
}
return m_font == b.m_font && m_halign == b.m_halign && m_valign == b.m_valign;
#else
// Don't compare size, font and alignment
return true;
#endif
}
};

37
src/db/unit_tests/dbLayoutTests.cc
View File

@ -26,6 +26,7 @@
#include "dbColdProxy.h"
#include "dbLibraryProxy.h"
#include "dbTextWriter.h"
#include "dbCellMapping.h"
#include "tlString.h"
#include "tlUnitTest.h"
@ -808,3 +809,39 @@ TEST(9_ErrorLayer)
EXPECT_EQ (l.is_special_layer (1), true);
EXPECT_EQ (int (l.layers ()), 2);
}
TEST(10_TranslateStringRefs)
{
db::Manager m;
db::Layout l (&m);
db::Cell &top = l.cell (l.add_cell ("TOP"));
l.insert_layer (db::LayerProperties (1, 0));
{
std::unique_ptr<db::Layout> t (new db::Layout ());
db::Cell &ttop = t->cell (t->add_cell ("TOP"));
unsigned int tl1 = t->insert_layer (db::LayerProperties (1, 0));
const db::StringRef *string_ref = db::StringRepository::instance ()->create_string_ref ();
db::StringRepository::change_string_ref (string_ref, "TEXT");
db::Text txt (string_ref, db::Trans ());
ttop.shapes (tl1).insert (db::TextRef (txt, t->shape_repository ()));
ttop.shapes (tl1).insert (txt);
EXPECT_EQ (l2s (*t), "begin_lib 0.001\nbegin_cell {TOP}\ntext 1 0 0 0 {0 0} {TEXT}\ntext 1 0 0 0 {0 0} {TEXT}\nend_cell\nend_lib\n");
db::CellMapping cm;
cm.create_single_mapping (l, top.cell_index (), *t, ttop.cell_index ());
l.copy_tree_shapes (*t, cm);
EXPECT_EQ (l2s (l), "begin_lib 0.001\nbegin_cell {TOP}\ntext 1 0 0 0 {0 0} {TEXT}\ntext 1 0 0 0 {0 0} {TEXT}\nend_cell\nend_lib\n");
db::StringRepository::change_string_ref (string_ref, "TEXT_NEW");
EXPECT_EQ (l2s (*t), "begin_lib 0.001\nbegin_cell {TOP}\ntext 1 0 0 0 {0 0} {TEXT_NEW}\ntext 1 0 0 0 {0 0} {TEXT_NEW}\nend_cell\nend_lib\n");
// also the copy changes:
EXPECT_EQ (l2s (l), "begin_lib 0.001\nbegin_cell {TOP}\ntext 1 0 0 0 {0 0} {TEXT_NEW}\ntext 1 0 0 0 {0 0} {TEXT_NEW}\nend_cell\nend_lib\n");
}
// after deleting the tmp layout, l is still valid
EXPECT_EQ (l2s (l), "begin_lib 0.001\nbegin_cell {TOP}\ntext 1 0 0 0 {0 0} {TEXT_NEW}\ntext 1 0 0 0 {0 0} {TEXT_NEW}\nend_cell\nend_lib\n");
}

22
src/db/unit_tests/dbTextTests.cc
View File

@ -55,10 +55,10 @@ TEST(1)
TEST(2)
{
db::StringRepository rep;
size_t n = db::StringRepository::instance ()->size ();
const db::StringRef *ref = rep.create_string_ref ();
rep.change_string_ref (ref, "ABER");
const db::StringRef *ref = db::StringRepository::instance ()->create_string_ref ();
db::StringRepository::change_string_ref (ref, "ABER");
db::Text t (ref, db::Trans ());
db::Text tt (t);
@ -69,9 +69,9 @@ TEST(2)
EXPECT_EQ (t < tt, false);
EXPECT_EQ (tt < t, false);
EXPECT_EQ (rep.size (), size_t (1));
EXPECT_EQ (db::StringRepository::instance ()->size (), n + size_t (1));
rep.change_string_ref (ref, "NOCHWAS");
db::StringRepository::change_string_ref (ref, "NOCHWAS");
EXPECT_EQ (std::string (t.string ()), "NOCHWAS");
EXPECT_EQ (std::string (tt.string ()), "NOCHWAS");
@ -80,12 +80,12 @@ TEST(2)
EXPECT_EQ (t < tt, false);
EXPECT_EQ (tt < t, false);
EXPECT_EQ (rep.size (), size_t (1));
EXPECT_EQ (db::StringRepository::instance ()->size (), n + size_t (1));
t = db::Text ();
tt = db::Text ();
EXPECT_EQ (rep.size (), size_t (0));
EXPECT_EQ (db::StringRepository::instance ()->size (), n);
}
TEST(3)
@ -98,8 +98,8 @@ TEST(3)
unsigned int l2 = ly2.insert_layer ();
db::Cell *c2 = &ly2.cell (ly2.add_cell ("TOP"));
const db::StringRef *ref1 = ly1.string_repository ().create_string_ref ();
ly1.string_repository ().change_string_ref (ref1, "X");
const db::StringRef *ref1 = db::StringRepository::instance ()->create_string_ref ();
db::StringRepository::change_string_ref (ref1, "X");
db::Text t (ref1, db::Trans ());
db::Shape s1 = c1->shapes (l1).insert (t);
@ -111,7 +111,7 @@ TEST(3)
db::Shape s1dup = *c1dup->shapes (l1dup).begin (db::ShapeIterator::All);
EXPECT_EQ (std::string (s1dup.text_string ()), "X");
ly1.string_repository ().change_string_ref (ref1, "U");
db::StringRepository::change_string_ref (ref1, "U");
EXPECT_EQ (std::string (s1.text_string ()), "U");
EXPECT_EQ (std::string (s1dup.text_string ()), "X");
@ -125,7 +125,7 @@ TEST(3)
EXPECT_EQ (std::string (s2a.text_string ()), "U");
EXPECT_EQ (std::string (s2b.text_string ()), "U");
ly1.string_repository ().change_string_ref (ref1, "A");
db::StringRepository::change_string_ref (ref1, "A");
EXPECT_EQ (std::string (tt.string ()), "U");
EXPECT_EQ (std::string (s1.text_string ()), "A");

4
src/plugins/streamers/oasis/db_plugin/dbOASISReader.cc
View File

@ -1229,7 +1229,7 @@ OASISReader::do_read (db::Layout &layout)
if (ts == m_textstrings.end ()) {
error (tl::sprintf (tl::to_string (tr ("No text string defined for text string id %ld")), fw->first));
} else {
layout.string_repository ().change_string_ref (fw->second, ts->second);
StringRepository::change_string_ref (fw->second, ts->second);
}
}
@ -2017,7 +2017,7 @@ OASISReader::do_read_text (bool xy_absolute,
mm_text_string.reset ();
mm_text_string_id = id;
const db::StringRef *string_ref = layout.string_repository ().create_string_ref ();
const db::StringRef *string_ref = db::StringRepository::instance ()->create_string_ref ();
m_text_forward_references.insert (std::make_pair (id, string_ref));
} else {