vtr-verilog-to-routing icon indicating copy to clipboard operation
vtr-verilog-to-routing copied to clipboard

[Netlist] Potential Issue When Re-Creating Elements in Atom and Clustered Netlists

Open AlexandreSinger opened this issue 6 months ago • 2 comments

When implementing another Netlist class based on the Atom and Clustered netlists, I noticed a bug in their create methods.

This bug occurs in a few places, but for example the create block method specifies that it creates a block or returns the ID if it already exists: https://github.com/verilog-to-routing/vtr-verilog-to-routing/blob/21d015036969a955298cbc9b94ca74fd35a016be/vpr/src/base/atom_netlist.h#L167-L175

However, in the implementation, the push_back method is used on the internalized data: https://github.com/verilog-to-routing/vtr-verilog-to-routing/blob/21d015036969a955298cbc9b94ca74fd35a016be/vpr/src/base/atom_netlist.cpp#L140-L155

Netlist::create_block will return the ID of a block if one with that name already exists; but then there will be a mismatch with the internal data. This will cause a crash if a block is re-created into the netlist.

This can be simply fixed by changing push_back into insert. The performance should not be too different since the vtr::vector_map class treats insertions as a resize (similar to a push back).

This should be fixed for many of the create methods in both the Atom Netlist and the Clustered Netlist (or the documentation should be updated to reflect that re-creation of IDs is not supported).

AlexandreSinger avatar Aug 15 '24 14:08 AlexandreSinger