Fix bug in bidirectional devices handling

In some cases (wrongly defined netlists), segmentation faults and wrong
net handling could occur.
It may need further checks in the near future.

3 files changed, 70 insertions, 17 deletions

diff --git a/src/parser/parse_element.h b/src/parser/parse_element.h
index 58ad694..ebd08ab 100644
--- a/src/parser/parse_element.h
+++ b/src/parser/parse_element.h
@@ -40,11 +40,12 @@ using std::unique_ptr;
         virtual ParseElement *clone() const                                            \
         { return new ELEM_NAME(*this); }                                               \
         /* Implement virtual function create(...) to construct element */              \
-        virtual sc_module *create(ParseTreeCreationHelper &pt_helper) const;               \
+        virtual sc_module *create(ParseTreeCreationHelper &pt_helper) const;           \
         virtual element_type_base *                                                    \
-        instantiate_and_connect_uni(ParseTreeCreationHelper &pt_helper) const;             \
+        instantiate_and_connect_uni(ParseTreeCreationHelper &pt_helper) const;         \
                                                                                        \
         virtual string kind() const { return ELEM_NAME_LONG; }                         \
+        virtual bool bidir_capable() const { return false; }                           \
     };
 
 /*************************************************/
@@ -64,13 +65,14 @@ using std::unique_ptr;
         virtual ParseElement *clone() const                                            \
         { return new ELEM_NAME(*this); }                                               \
         /* Implement virtual function create(...) to construct element */              \
-        virtual sc_module *create(ParseTreeCreationHelper &pt_helper) const;               \
+        virtual sc_module *create(ParseTreeCreationHelper &pt_helper) const;           \
         virtual element_type_base *                                                    \
-        instantiate_and_connect_uni(ParseTreeCreationHelper &pt_helper) const;             \
+        instantiate_and_connect_uni(ParseTreeCreationHelper &pt_helper) const;         \
         virtual element_type_base *                                                    \
-        instantiate_and_connect_bi(ParseTreeCreationHelper &pt_helper) const;              \
+        instantiate_and_connect_bi(ParseTreeCreationHelper &pt_helper) const;          \
                                                                                        \
         virtual string kind() const { return string(ELEM_NAME_LONG) + " (bidir)"; }    \
+        virtual bool bidir_capable() const { return true; }                            \
     };
 
 /** ******************************************* **/
diff --git a/src/parser/parse_tree.cpp b/src/parser/parse_tree.cpp
index bbafeb8..99dbcd3 100644
--- a/src/parser/parse_tree.cpp
+++ b/src/parser/parse_tree.cpp
@@ -29,7 +29,7 @@ vector<shared_ptr<sc_object>> ParseNet::create(const string &name, bool force_bi
     else
     {
         assert(type() == OANALOG && "Only optical nets can be bidirectional for now");
-        return { create_uni((name + "_0").c_str()), create_uni((name + "_1").c_str()) };
+        return { create_uni((name + ":0").c_str()), create_uni((name + ":1").c_str()) };
     }
 }
 
@@ -181,10 +181,18 @@ void ParseTreeCreationHelper::upgrade_signal(const string &net_name)
     // The signal should exist because it has been created by pt_helper.create_signals()
     // but it doesn't hurt to check once more time
     assert(circuit_signals.find(net_name) != circuit_signals.end());
+    assert(circuit_signals.at(net_name).size() >= 1);
+    assert(circuit_signals.at(net_name).size() <= 2);
 
     // Set the bidirectional flag to true
     pt->nets[net_name].m_bidirectional = true;
 
+    if (circuit_signals.at(net_name).size() == 2)
+    {
+        // No need to upgrade the signal (it's already bidirectional)
+        return;
+    }
+
     // Check if the net was created as unidirectional (only one signal)
     if (circuit_signals.at(net_name).size() == 1)
     {
@@ -196,10 +204,10 @@ void ParseTreeCreationHelper::upgrade_signal(const string &net_name)
             // if yes, that means a port is already connected to the signal
             // we cannot easily re-instantiate it as it may be bound to a port;
             // instead, we add a second signal to the vector to make it bidirectional
-            circuit_signals[net_name].push_back(pt->nets[net_name].create_uni(net_name + "_1"));
+            circuit_signals[net_name].push_back(pt->nets[net_name].create_uni(net_name + ":1"));
 
-            // For bidirectional signals, first port to connect binds to signal_0
-            // for writing and signal_1 for reading
+            // For bidirectional signals, first port to connect binds to signal:0
+            // for writing and signal:1 for reading
             // Therefore we have to "re-create" this scenario, as if the first device
             // connected to a bidirectional net. But for this we need to know which
             // if the first device was a writer or a reader.
@@ -215,14 +223,14 @@ void ParseTreeCreationHelper::upgrade_signal(const string &net_name)
             if (has_writer)
             {
                 // if a writer, then it's as expected:
-                // the next port will connect to signal_1 for writing
+                // the next port will connect to signal:1 for writing
                 // we just need to update the number of "readers" connected to the net
                 pt->nets[net_name].m_readers_count++;
             }
             else
             {
                 // if a writer, then the next port needs to reverse its order.
-                // for this we swap signal_0 and signal_1
+                // for this we swap signal:0 and signal:1
                 swap(circuit_signals[net_name][0], circuit_signals[net_name][1]);
                 // then we need to update the number of "writers" connected to the net
                 pt->nets[net_name].m_writers_count++;
@@ -230,10 +238,11 @@ void ParseTreeCreationHelper::upgrade_signal(const string &net_name)
         }
         else
         {
-            // otherwise, we can recreate the first signal as well
-            circuit_signals[net_name].clear(); // will delete the net through the constructor
+            // otherwise, we can create the first signal as well
+            circuit_signals[net_name].clear(); // will delete the net through the destructor
             circuit_signals[net_name] = pt->nets.at(net_name).create(net_name);
         }
+        cout << "\t" << circuit_signals[net_name][0]->name() << "," << circuit_signals[net_name][1]->name() << endl;
     }
 }
 
@@ -716,7 +725,7 @@ void ParseTree::build_circuit()
             {
                 auto it = elements_backlog.begin();
                 auto &backlog_elem = *it;
-                cout << "Creating " << backlog_elem->name << " (reason: backlog)" << endl;
+                cout << "Creating - " << backlog_elem->name << " (reason: bidir backlog)" << endl;
                 auto mod = backlog_elem->create(pt_helper);
                 cout << "Done creating " << backlog_elem->name << endl;
 
@@ -769,7 +778,7 @@ void ParseTree::build_circuit()
             {
                 auto it = elements_backlog.begin();
                 auto &backlog_elem = *it;
-                cout << "Creating " << backlog_elem->name << " (reason: backlog)" << endl;
+                cout << "Creating " << backlog_elem->name << " (reason: unidir backlog)" << endl;
                 auto mod = backlog_elem->create(pt_helper);
                 cout << "Done creating " << backlog_elem->name << endl;
 
diff --git a/src/parser/parse_tree.h b/src/parser/parse_tree.h
index fbcefbc..54786de 100644
--- a/src/parser/parse_tree.h
+++ b/src/parser/parse_tree.h
@@ -560,6 +560,8 @@ struct ParseElement {
     }
 
     virtual string kind() const { return "UNDEFINED"; }
+    virtual bool bidir_capable() const { return false; }                            \
+
     virtual ~ParseElement() {}
 
     virtual string to_json() const;
@@ -806,6 +808,7 @@ void ParseTreeCreationHelper::connect_uni(port_type &port, const string& net_nam
     const auto &net = pt->nets.at(net_name);
 
     // Get the signal to be written or read
+    sc_object *sig_raw;
     signal_type *sig = nullptr;
     if ( !net.bidirectional())
     {
@@ -814,7 +817,16 @@ void ParseTreeCreationHelper::connect_uni(port_type &port, const string& net_nam
     }
     else
     {
-        int i_write = pt->nets.at(net_name).m_connect_count;
+        // verify no more than 2 writer in total
+        if(pt->nets.at(net_name).m_connect_writer_count + as_writer > 2)
+        {
+            cerr << "Attempted to add a third writer to a bidirectional net ("
+                 << net_name << ")." << endl;
+            exit(2);
+        }
+
+        int i_write = pt->nets.at(net_name).m_connect_writer_count;
+        i_write = max(i_write, 1);// If there are already 2 writers, act as a reader for the second one (imitate the second writer but only as a reader)
         int i_read = (i_write + 1) % 2;
         int i = as_writer ? i_write : i_read;
 
@@ -822,6 +834,14 @@ void ParseTreeCreationHelper::connect_uni(port_type &port, const string& net_nam
         sig = dynamic_cast<signal_type *>(sig_raw);
     }
 
+    // Check corresponding signal could be found
+    if (!sig_raw)
+    {
+        cerr << "Missing object for signal connected to "
+                << net_name << endl;
+        exit(1);
+    }
+
     // Check it could correctly be cast to requested type
     if (!sig)
     {
@@ -849,13 +869,35 @@ void ParseTreeCreationHelper::connect_bi(port_in_type &p_in, port_out_type &p_ou
 
     if (net.bidirectional())
     {
+        // verify no more than 2 writer in total
+        if(pt->nets.at(net_name).m_connect_writer_count >= 2)
+        {
+            cerr << "Attempted to add a third writer to a bidirectional net ("
+                 << net_name << ")." << endl;
+            exit(1);
+        }
 
         // First get the signal to be written or read (in the right order)
-        int i_write = pt->nets.at(net_name).m_connect_count;
+        int i_write = pt->nets.at(net_name).m_connect_writer_count;
         int i_read = (i_write + 1) % 2;
 
         auto sig_writeable_raw = circuit_signals.at(net_name)[i_write].get();
         auto sig_readable_raw = circuit_signals.at(net_name)[i_read].get();
+
+        // Check corresponding signals could be found
+        if (!sig_writeable_raw)
+        {
+            cerr << "Missing object for output signal connected to "
+                 << net_name << endl;
+            exit(1);
+        }
+        if (!sig_readable_raw)
+        {
+            cerr << "Missing object for input signal connected to "
+                 << net_name << endl;
+            exit(1);
+        }
+
         auto sig_writeable = dynamic_cast<signal_type *>(sig_writeable_raw);
         auto sig_readable = dynamic_cast<signal_type *>(sig_readable_raw);