From 0c789ec9b6e92cd999abf182d196651c941c4101 Mon Sep 17 00:00:00 2001 From: Luc Grosheintz Date: Tue, 22 Oct 2024 09:59:39 +0200 Subject: [PATCH] Avoid premature segfaults. (#1529) In NOCMODL functions that only use global data can be called without an instance. --- src/codegen/codegen_neuron_cpp_visitor.cpp | 36 ++++++++++----- test/usecases/global/global.mod | 16 +++++++ test/usecases/global/parameter.mod | 11 +++++ test/usecases/global/test_without_instance.py | 44 +++++++++++++++++++ test/usecases/global/top_local.mod | 8 ++++ 5 files changed, 104 insertions(+), 11 deletions(-) create mode 100644 test/usecases/global/global.mod create mode 100644 test/usecases/global/parameter.mod create mode 100644 test/usecases/global/test_without_instance.py diff --git a/src/codegen/codegen_neuron_cpp_visitor.cpp b/src/codegen/codegen_neuron_cpp_visitor.cpp index 2d66b07b1..ebd5b5d79 100644 --- a/src/codegen/codegen_neuron_cpp_visitor.cpp +++ b/src/codegen/codegen_neuron_cpp_visitor.cpp @@ -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); } @@ -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); @@ -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); } @@ -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 /// (__reg()) printer->add_newline(2); @@ -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 make_instance_args; @@ -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)); } } @@ -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 != "") { @@ -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 node_index{0};"); printer->add_line("Node* _node = _nrn_mechanism_access_node(_prop);"); @@ -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); } @@ -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); } @@ -2488,7 +2502,7 @@ void CodegenNeuronCppVisitor::print_net_receive_common_code() { printer->add_line("auto * nt = static_cast(_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); } diff --git a/test/usecases/global/global.mod b/test/usecases/global/global.mod new file mode 100644 index 000000000..3f2e17a14 --- /dev/null +++ b/test/usecases/global/global.mod @@ -0,0 +1,16 @@ +NEURON { + SUFFIX global + GLOBAL gbl +} + +ASSIGNED { + gbl +} + +FUNCTION get_gbl() { + get_gbl = gbl +} + +PROCEDURE set_gbl(value) { + gbl = value +} diff --git a/test/usecases/global/parameter.mod b/test/usecases/global/parameter.mod new file mode 100644 index 000000000..4bbab617c --- /dev/null +++ b/test/usecases/global/parameter.mod @@ -0,0 +1,11 @@ +NEURON { + SUFFIX parameter +} + +PARAMETER { + gbl = 42.0 +} + +FUNCTION get_gbl() { + get_gbl = gbl +} diff --git a/test/usecases/global/test_without_instance.py b/test/usecases/global/test_without_instance.py new file mode 100644 index 000000000..3f32af085 --- /dev/null +++ b/test/usecases/global/test_without_instance.py @@ -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() diff --git a/test/usecases/global/top_local.mod b/test/usecases/global/top_local.mod index ad4d36e7e..f40789960 100644 --- a/test/usecases/global/top_local.mod +++ b/test/usecases/global/top_local.mod @@ -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 +}