riscv
/
yosys
mirror of https://github.com/YosysHQ/yosys.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

write_cxxrtl: avoid undefined behavior on out-of-bounds memory access. 

After this commit, if NDEBUG is not defined, out-of-bounds accesses
cause assertion failures for reads and writes. If NDEBUG is defined,
out-of-bounds reads return zeroes, and out-of-bounds writes are
ignored.

This commit also adds support for memories that start with a non-zero
index (`Memory::start_offset` in RTLIL).
This commit is contained in:
whitequark 2020-04-04 22:53:46 +00:00
parent 5157691f0e
commit 3376dcf37c
2 changed files with 78 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

103
backends/cxxrtl/cxxrtl.cc
View File

@ -798,6 +798,10 @@ struct CxxrtlWorker {
inc_indent(); inc_indent();
} }
RTLIL::Memory *memory = cell->module->memories[cell->getParam(ID(MEMID)).decode_string()]; RTLIL::Memory *memory = cell->module->memories[cell->getParam(ID(MEMID)).decode_string()];
std::string valid_index_temp = fresh_temporary();
f << indent << "std::pair<bool, size_t> " << valid_index_temp << " = memory_index(";
dump_sigspec_rhs(cell->getPort(ID(ADDR)));
f << ", " << memory->start_offset << ", " << memory->size << ");\n";
if (cell->type == ID($memrd)) { if (cell->type == ID($memrd)) {
if (!cell->getPort(ID(EN)).is_fully_ones()) { if (!cell->getPort(ID(EN)).is_fully_ones()) {
f << indent << "if ("; f << indent << "if (";
@ -805,38 +809,54 @@ struct CxxrtlWorker {
f << ") {\n"; f << ") {\n";
inc_indent(); inc_indent();
} }
if (writable_memories[memory]) { // The generated code has two bounds checks; one in an assertion, and another that guards the read.
std::string addr_temp = fresh_temporary(); // This is done so that the code does not invoke undefined behavior under any conditions, but nevertheless
f << indent << "const value<" << cell->getPort(ID(ADDR)).size() << "> &" << addr_temp << " = "; // loudly crashes if an illegal condition is encountered. The assert may be turned off with -NDEBUG not
dump_sigspec_rhs(cell->getPort(ID(ADDR))); // just for release builds, but also to make sure the simulator (which is presumably embedded in some
f << ";\n"; // larger program) will never crash the code that calls into it.
std::string lhs_temp = fresh_temporary(); //
f << indent << "value<" << memory->width << "> " << lhs_temp << " = " // If assertions are disabled, out of bounds reads are defined to return zero.
<< mangle(memory) << "[" << addr_temp << "].curr;\n"; f << indent << "assert(" << valid_index_temp << ".first && \"out of bounds read\");\n";
for (auto memwr_cell : transparent_for[cell]) { f << indent << "if(" << valid_index_temp << ".first) {\n";
f << indent << "if (" << addr_temp << " == "; inc_indent();
dump_sigspec_rhs(memwr_cell->getPort(ID(ADDR))); if (writable_memories[memory]) {
f << ") {\n"; std::string addr_temp = fresh_temporary();
inc_indent(); f << indent << "const value<" << cell->getPort(ID(ADDR)).size() << "> &" << addr_temp << " = ";
f << indent << lhs_temp << " = " << lhs_temp; dump_sigspec_rhs(cell->getPort(ID(ADDR)));
f << ".update("; f << ";\n";
dump_sigspec_rhs(memwr_cell->getPort(ID(EN))); std::string lhs_temp = fresh_temporary();
f << ", "; f << indent << "value<" << memory->width << "> " << lhs_temp << " = "
dump_sigspec_rhs(memwr_cell->getPort(ID(DATA))); << mangle(memory) << "[" << valid_index_temp << ".second].curr;\n";
f << ");\n"; for (auto memwr_cell : transparent_for[cell]) {
dec_indent(); f << indent << "if (" << addr_temp << " == ";
f << indent << "}\n"; dump_sigspec_rhs(memwr_cell->getPort(ID(ADDR)));
f << ") {\n";
inc_indent();
f << indent << lhs_temp << " = " << lhs_temp;
f << ".update(";
dump_sigspec_rhs(memwr_cell->getPort(ID(EN)));
f << ", ";
dump_sigspec_rhs(memwr_cell->getPort(ID(DATA)));
f << ");\n";
dec_indent();
f << indent << "}\n";
}
f << indent;
dump_sigspec_lhs(cell->getPort(ID(DATA)));
f << " = " << lhs_temp << ";\n";
} else {
f << indent;
dump_sigspec_lhs(cell->getPort(ID(DATA)));
f << " = " << mangle(memory) << "[" << valid_index_temp << ".second];\n";
} }
dec_indent();
f << indent << "} else {\n";
inc_indent();
f << indent; f << indent;
dump_sigspec_lhs(cell->getPort(ID(DATA))); dump_sigspec_lhs(cell->getPort(ID(DATA)));
f << " = " << lhs_temp << ";\n"; f << " = value<" << memory->width << "> {};\n";
} else { dec_indent();
f << indent; f << indent << "}\n";
dump_sigspec_lhs(cell->getPort(ID(DATA)));
f << " = " << mangle(memory) << "[";
dump_sigspec_rhs(cell->getPort(ID(ADDR)));
f << "];\n";
}
if (!cell->getPort(ID(EN)).is_fully_ones()) { if (!cell->getPort(ID(EN)).is_fully_ones()) {
dec_indent(); dec_indent();
f << indent << "}\n"; f << indent << "}\n";
@ -844,15 +864,22 @@ struct CxxrtlWorker {
} else /*if (cell->type == ID($memwr))*/ { } else /*if (cell->type == ID($memwr))*/ {
// FIXME: handle write port priority, here and above in transparent $memrd cells // FIXME: handle write port priority, here and above in transparent $memrd cells
log_assert(writable_memories[memory]); log_assert(writable_memories[memory]);
std::string lhs_temp = fresh_temporary(); // See above for rationale of having both the assert and the condition.
f << indent << "wire<" << memory->width << "> &" << lhs_temp << " = " << mangle(memory) << "["; //
dump_sigspec_rhs(cell->getPort(ID(ADDR))); // If assertions are disabled, out of bounds writes are defined to do nothing.
f << "];\n"; f << indent << "assert(" << valid_index_temp << ".first && \"out of bounds write\");\n";
f << indent << lhs_temp << ".next = " << lhs_temp << ".curr.update("; f << indent << "if (" << valid_index_temp << ".first) {\n";
dump_sigspec_rhs(cell->getPort(ID(EN))); inc_indent();
f << ", "; std::string lhs_temp = fresh_temporary();
dump_sigspec_rhs(cell->getPort(ID(DATA))); f << indent << "wire<" << memory->width << "> &" << lhs_temp << " = ";
f << ");\n"; f << mangle(memory) << "[" << valid_index_temp << ".second];\n";
f << indent << lhs_temp << ".next = " << lhs_temp << ".curr.update(";
dump_sigspec_rhs(cell->getPort(ID(EN)));
f << ", ";
dump_sigspec_rhs(cell->getPort(ID(DATA)));
f << ");\n";
dec_indent();
f << indent << "}\n";
} }
if (cell->getParam(ID(CLK_ENABLE)).as_bool()) { if (cell->getParam(ID(CLK_ENABLE)).as_bool()) {
dec_indent(); dec_indent();

21
backends/cxxrtl/cxxrtl.h
View File

@ -23,6 +23,7 @@
#include <cstddef> #include <cstddef>
#include <cstdint> #include <cstdint>
#include <cassert>
#include <limits> #include <limits>
#include <type_traits> #include <type_traits>
#include <tuple> #include <tuple>
@ -614,7 +615,6 @@ struct memory {
template<size_t... InitSize> template<size_t... InitSize>
explicit memory(size_t depth, const init<InitSize> &...init) : data(depth) { explicit memory(size_t depth, const init<InitSize> &...init) : data(depth) {
// FIXME: assert(init.size() <= depth);
data.resize(depth); data.resize(depth);
// This utterly reprehensible construct is the most reasonable way to apply a function to every element // This utterly reprehensible construct is the most reasonable way to apply a function to every element
// of a parameter pack, if the elements all have different types and so cannot be cast to an initializer list. // of a parameter pack, if the elements all have different types and so cannot be cast to an initializer list.
@ -622,15 +622,9 @@ struct memory {
} }
Elem &operator [](size_t index) { Elem &operator [](size_t index) {
// FIXME: assert(index < data.size()); assert(index < data.size());
return data[index]; return data[index];
} }
template<size_t AddrBits>
Elem &operator [](const value<AddrBits> &addr) {
static_assert(value<AddrBits>::chunks <= 1, "memory indexing with unreasonably large address is not supported");
return (*this)[addr.data[0]];
}
}; };
template<size_t Width> template<size_t Width>
@ -1103,6 +1097,17 @@ value<BitsY> mod_ss(const value<BitsA> &a, const value<BitsB> &b) {
return divmod_ss<BitsY>(a, b).second; return divmod_ss<BitsY>(a, b).second;
} }
// Memory helper
template<size_t BitsAddr>
std::pair<bool, size_t> memory_index(const value<BitsAddr> &addr, size_t offset, size_t depth) {
static_assert(value<BitsAddr>::chunks <= 1, "memory address is too wide");
size_t offset_index = addr.data[0];
bool valid = (offset_index >= offset && offset_index < offset + depth);
size_t index = offset_index - offset;
return std::make_pair(valid, index);
}
} // namespace cxxrtl_yosys } // namespace cxxrtl_yosys
#endif #endif