Skip to content

Commit

Permalink
Avoid premature segfaults. (#1529)
Browse files Browse the repository at this point in the history
In NOCMODL functions that only use global data can be
called without an instance.
  • Loading branch information
1uc authored Oct 22, 2024
1 parent 3cb122d commit 0c789ec
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 11 deletions.
36 changes: 25 additions & 11 deletions src/codegen/codegen_neuron_cpp_visitor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ void CodegenNeuronCppVisitor::print_check_table_entrypoint() {
printer->fmt_line("static void {}({})", table_thread_function_name(), get_parameter_str(args));
printer->push_block();
printer->add_line("_nrn_mechanism_cache_range _lmc{_sorted_token, *nt, *_ml, _type};");
printer->fmt_line("auto inst = make_instance_{}(_lmc);", info.mod_suffix);
printer->fmt_line("auto inst = make_instance_{}(&_lmc);", info.mod_suffix);
if (!info.artificial_cell) {
printer->fmt_line("auto node_data = make_node_data_{}(*nt, *_ml);", info.mod_suffix);
}
Expand Down Expand Up @@ -259,7 +259,9 @@ void CodegenNeuronCppVisitor::print_function_or_procedure(
}

if (!info.artificial_cell) {
printer->add_line("auto v = node_data.node_voltages[node_data.nodeindices[id]];");
printer->add_line(
"double v = node_data.node_voltages ? "
"node_data.node_voltages[node_data.nodeindices[id]] : 0.0;");
}

print_statement_block(*node.get_statement_block(), false, false);
Expand Down Expand Up @@ -351,7 +353,9 @@ void CodegenNeuronCppVisitor::print_hoc_py_wrapper_function_body(
prop_name = "_prop";
}

printer->fmt_line("auto inst = make_instance_{}(_lmc);", info.mod_suffix);
printer->fmt_line("auto inst = make_instance_{}({} ? &_lmc : nullptr);",
info.mod_suffix,
prop_name);
if (!info.artificial_cell) {
printer->fmt_line("auto node_data = make_node_data_{}({});", info.mod_suffix, prop_name);
}
Expand Down Expand Up @@ -868,7 +872,7 @@ void CodegenNeuronCppVisitor::print_neuron_includes() {
}


void CodegenNeuronCppVisitor::print_sdlists_init([[maybe_unused]] bool print_initializers) {
void CodegenNeuronCppVisitor::print_sdlists_init(bool /* print_initializers */) {
/// _initlists() should only be called once by the mechanism registration function
/// (_<mod_file>_reg())
printer->add_newline(2);
Expand Down Expand Up @@ -1526,9 +1530,14 @@ void CodegenNeuronCppVisitor::print_mechanism_range_var_structure(bool print_ini

void CodegenNeuronCppVisitor::print_make_instance() const {
printer->add_newline(2);
printer->fmt_push_block("static {} make_instance_{}(_nrn_mechanism_cache_range& _lmc)",
printer->fmt_push_block("static {} make_instance_{}(_nrn_mechanism_cache_range* _lmc)",
instance_struct(),
info.mod_suffix);

printer->push_block("if(_lmc == nullptr)");
printer->fmt_line("return {}();", instance_struct());
printer->pop_block_nl(2);

printer->fmt_push_block("return {}", instance_struct());

std::vector<std::string> make_instance_args;
Expand All @@ -1545,9 +1554,9 @@ void CodegenNeuronCppVisitor::print_make_instance() const {
const auto& float_var = codegen_float_variables[i];
if (float_var->is_array()) {
make_instance_args.push_back(
fmt::format("_lmc.template data_array_ptr<{}, {}>()", i, float_var->get_length()));
fmt::format("_lmc->template data_array_ptr<{}, {}>()", i, float_var->get_length()));
} else {
make_instance_args.push_back(fmt::format("_lmc.template fpfield_ptr<{}>()", i));
make_instance_args.push_back(fmt::format("_lmc->template fpfield_ptr<{}>()", i));
}
}

Expand All @@ -1561,7 +1570,7 @@ void CodegenNeuronCppVisitor::print_make_instance() const {
} else if (var.is_vdata) {
return "";
} else {
return fmt::format("_lmc.template dptr_field_ptr<{}>()", i);
return fmt::format("_lmc->template dptr_field_ptr<{}>()", i);
}
}();
if (variable != "") {
Expand Down Expand Up @@ -1611,6 +1620,11 @@ void CodegenNeuronCppVisitor::print_make_node_data() const {
printer->fmt_push_block("static {} make_node_data_{}(Prop * _prop)",
node_data_struct(),
info.mod_suffix);

printer->push_block("if(!_prop)");
printer->fmt_line("return {}();", node_data_struct());
printer->pop_block_nl(2);

printer->add_line("static std::vector<int> node_index{0};");
printer->add_line("Node* _node = _nrn_mechanism_access_node(_prop);");

Expand Down Expand Up @@ -1684,7 +1698,7 @@ void CodegenNeuronCppVisitor::print_initial_block(const InitialBlock* node) {
void CodegenNeuronCppVisitor::print_entrypoint_setup_code_from_memb_list() {
printer->add_line(
"_nrn_mechanism_cache_range _lmc{_sorted_token, *nt, *_ml_arg, _ml_arg->type()};");
printer->fmt_line("auto inst = make_instance_{}(_lmc);", info.mod_suffix);
printer->fmt_line("auto inst = make_instance_{}(&_lmc);", info.mod_suffix);
if (!info.artificial_cell) {
printer->fmt_line("auto node_data = make_node_data_{}(*nt, *_ml_arg);", info.mod_suffix);
}
Expand All @@ -1702,7 +1716,7 @@ void CodegenNeuronCppVisitor::print_entrypoint_setup_code_from_prop() {
printer->add_line("_nrn_mechanism_cache_instance _lmc{prop};");
printer->add_line("const size_t id = 0;");

printer->fmt_line("auto inst = make_instance_{}(_lmc);", info.mod_suffix);
printer->fmt_line("auto inst = make_instance_{}(prop ? &_lmc : nullptr);", info.mod_suffix);
if (!info.artificial_cell) {
printer->fmt_line("auto node_data = make_node_data_{}(prop);", info.mod_suffix);
}
Expand Down Expand Up @@ -2488,7 +2502,7 @@ void CodegenNeuronCppVisitor::print_net_receive_common_code() {
printer->add_line("auto * nt = static_cast<NrnThread*>(_pnt->_vnt);");
printer->add_line("auto * _ppvar = _nrn_mechanism_access_dparam(_pnt->prop);");

printer->fmt_line("auto inst = make_instance_{}(_lmc);", info.mod_suffix);
printer->fmt_line("auto inst = make_instance_{}(&_lmc);", info.mod_suffix);
if (!info.artificial_cell) {
printer->fmt_line("auto node_data = make_node_data_{}(_pnt->prop);", info.mod_suffix);
}
Expand Down
16 changes: 16 additions & 0 deletions test/usecases/global/global.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
NEURON {
SUFFIX global
GLOBAL gbl
}

ASSIGNED {
gbl
}

FUNCTION get_gbl() {
get_gbl = gbl
}

PROCEDURE set_gbl(value) {
gbl = value
}
11 changes: 11 additions & 0 deletions test/usecases/global/parameter.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
NEURON {
SUFFIX parameter
}

PARAMETER {
gbl = 42.0
}

FUNCTION get_gbl() {
get_gbl = gbl
}
44 changes: 44 additions & 0 deletions test/usecases/global/test_without_instance.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
from neuron import h, gui


def make_accessors(mech_name, read_only):
get = getattr(h, f"get_gbl_{mech_name}")

if read_only:
return get, None

set = getattr(h, f"set_gbl_{mech_name}")
return get, set


def check_write_read_cycle(mech_name, read_only=False):
get, set = make_accessors(mech_name, read_only)

if read_only:
expected = 42.0
else:
expected = 278.045
set(expected)

actual = get()
assert (
actual == expected
), f"{actual = }, {expected = }, delta = {actual - expected}"


def test_top_local():
check_write_read_cycle("top_local")


def test_global():
check_write_read_cycle("global")


def test_parameter():
check_write_read_cycle("parameter", True)


if __name__ == "__main__":
test_top_local()
test_global()
test_parameter()
8 changes: 8 additions & 0 deletions test/usecases/global/top_local.mod
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,11 @@ BREAKPOINT {
y = gbl
il = 0.0000001 * (v - 10.0)
}

FUNCTION get_gbl() {
get_gbl = gbl
}

PROCEDURE set_gbl(value) {
gbl = value
}

0 comments on commit 0c789ec

Please sign in to comment.