Minres / SystemC-Components

A SystemC productivity library: https://minres.github.io/SystemC-Components/
https://www.minres.com/#opensource
Apache License 2.0
81 stars 21 forks source link

range_lut: range check is incomplete #25

Closed tk-ka closed 2 years ago

tk-ka commented 2 years ago

The following code inside _src/common/util/rangelut.h [126:129] is not fully correct.

template <typename T> inline void range_lut<T>::addEntry(T i, uint64_t base_addr, uint64_t size) {
    auto iter = m_lut.find(base_addr);
    if(iter != m_lut.end() && iter->second.index != null_entry)
        throw std::runtime_error("range already mapped");

The m_lut container stores entries for the start and end address of every registered object (e.g. sc_register). When adding a new entry, it only checks if the demanded base address is available in m_lut (as start or end address of a previously added entry).

Problem: If a new entry starts in-between the borders of existing entries, the overlap is not detected and no error is thrown.

This case can be reproduced with the code from ticket #24 by removing register four.

eyck commented 2 years ago

The correctness of the address map is checked in the validate() call which should be called when all ranges are defined. One point would be the end_of_elaboration() or start_of_simulation() callbacks of the sc_module using the range_lut.

tk-ka commented 2 years ago

Ok thats good: calling range_lut::validate() does the job. I did not recognize that function before. It works if the custom module is a child of scc::tlm_target because there socket_map is a protected member which otherwise cannot be accessed to call validate(). Just something to keep in mind...

But with that the next questions comes up: Why do you check the ranges twice? First in an incomplete way inside range_lut::addEntry()and another time optionally but correct in range_lut::validate().

eyck commented 2 years ago

Well, it follows the fail-fast approach. If things can be checked easily then it should be done where the root cause may occur. Since the complete check is more time consuming it is done once it is ensured the information is complete. That the simple checks are executed as well is more a side-effect.