✔️ f0524bb -> Azure artifacts URL

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.55%. Comparing base (499cea9) to head (13ac83b).
⚠️ Report is 29 commits behind head on master.

@@           Coverage Diff           @@
##           master    #3640   +/-   ##
=======================================
  Coverage   68.55%   68.55%           
=======================================
  Files         686      686           
  Lines      116791   116791           
=======================================
+ Hits        80065    80069    +4     
+ Misses      36726    36722    -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

✔️ f0524bb -> artifacts URL

Would be useful to add a test, like the most minimal mod file imaginable (does this work, or do we need something else?):

NEURON {
    RANGE foo
}
NET_RECEIVE b {
    INITIAL {
      foo = 1
    }
}

and then a corresponding minimal Python script that runs it (I see most mod file tests are in https://github.com/neuronsimulator/nrn/tree/master/test/hoctests, and the Python/HOC scripts in https://github.com/neuronsimulator/nrn/tree/master/test/hoctests/tests, and since we glob files, no CMake changes are necessary), but that would still trigger the segfault, just to make sure it works as intended.

add a test

I agree and will do so.

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

✔️ 13ac83b -> Azure artifacts URL

✔️ 13ac83b -> artifacts URL

The test fails on master with:

87: Traceback (most recent call last):
87:   File "/test/hoctests/test_netrec_init_py/tests/test_netrec_init.py", line 25, in <module>
87:     test_netrec_init()
87:   File "/test/hoctests/test_netrec_init_py/tests/test_netrec_init.py", line 20, in test_netrec_init
87:     assert nc.weight[1] == nri.foo == 2.0
87:            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
87: AssertionError
1/1 Test #87: hoctests::test_netrec_init_py ....***Failed    0.33 sec

Shouldn't it trigger a segfault?

Shouldn't it trigger a segfault?

That is a puzzle. It does on my machine with master

test/hoctests$ nrnivmodl
test/hoctests$ python tests/test_netrec_init.py
Segmentation fault (core dumped)

Are you sure that netrec_init.mod was translated by the master version of nocmodl? But if translated by this pr version it should have succeeded instead of failing at assert nc.weight[1] == nri.foo == 2.0. I noticed you tested with ctest. How did you do that?

Are you sure that netrec_init.mod was translated by the master version of nocmodl?

Pretty sure, yes. I started from a fresh project, did git checkout --patch hines/not-vectorized, selected the added test and mod file, built the project, and I always hit that assert instead of segfaulting.

I noticed you tested with ctest. How did you do that?

ctest -R hoctests::test_netrec_init_py

In general, you can pass a regex pattern to -R and it will run all of the tests matching that regex. You can use the -VV flag to make it more verbose, and ctest will then output the actual command it runs, such as:

test 87
    Start 87: hoctests::test_netrec_init_py

87: Test command: /usr/bin/cmake "-E" "env" "NEURONHOME=/build/share/nrn" "NRNHOME=/build" "NMODLHOME=/build/" "NMODL_PYLIB=/usr/lib/x86_64-linux-gnu/libpython3.12.so" "PATH=/build/test/nrnivmodl/a70e16caf786bea51b0cd709cbe9b320db9831ec5af7d1c4131b6fb3dbf48207/x86_64:/build/bin:/venv_3.12/bin" "LD_LIBRARY_PATH=/build/lib" "CORENRNHOME=/build" "PYTHONPATH=/docs/nmodl/python_scripts:/build/lib/python:/test/rxd" "CC=/usr/bin/cc" "/venv_3.12/bin/python3" "tests/test_netrec_init.py"
87: Working Directory: /build/test/hoctests/test_netrec_init_py

I believe your test workflow is valid. In my case from a fresh clone

git checkout --patch origin/hines/not-vectorized # select only the mod and py file
mkdir build
cd build
cmake .. -G Ninja -DCMAKE_INSTALL_PREFIX=install -DNRN_ENABLE_TESTS=ON
ninja install
ctest -R hoctests::test_netrec_init_py -VV
...
79: Segmentation fault
1/1 Test #79: hoctests::test_netrec_init_py ....***Failed    1.72 sec

The puzzle is that you do not get a segfault, however the assertion fails. Ie. assert nc.weight[1] == nri.foo == 2.0.
Now I'm sorry I did not separate that into two tests, assert nc.weight[1] == 2.0 and nri.foo == 2.0. Is it possible that the mod file translation build$ find . -name netrec_init.cpp ./test/nrnivmodl/a70e16caf786bea51b0cd709cbe9b320db9831ec5af7d1c4131b6fb3dbf48207/x86_64/netrec_init.cpp

static void _net_init(Point_process* _pnt, double* _args, double _lflag) {
       _ppvar = _nrn_mechanism_access_dparam(_pnt->_prop);
 _args[1] = foo ;
   _args[2] = ptr ;
   }

is not a segfault at _args[1] = foo ; where foo is

#define foo _ml->template fpfield<0>(_iml)

because _iml happens to be a valid number and _ml happens to also be a valid address your machine when _net_init is called (albeit on your machine the evaluation of nri.foo is wrong and hence the assertion failure). I wonder if you would get a segfault if there were several instances of NetRecInit

Okay this is once again related to build flags; by default NEURON uses CMAKE_BUILD_TYPE=RelWithDebInfo, which basically activates -O2, and I was building locally with CMAKE_BUILD_TYPE=Debug, which has -O0. This is also reflected in the CI failures (#3642):

• the coverage CI has Debug build type, and hence fails on the assert
• most of the other GitHub-hosted CIs have the default, i.e. RelWithDebInfo, so they segfault

I don't really understand why the behavior is different; is it possible to come up with a test that will segfault on both?

is it possible to come up with a test that will segfault on both
With -DCMAKE_BUILD_TYPE=Debug I did see the assert errror instead of the segfault.

Probably, but it might be tricky since the segfault depends on having a pointer that points to invalid memory and std::vector that underlies the range variable foo may be valid for the value of _iml index that is most likely 1 greater than the last value in the vector (due to looping over all instances in the nrn_init function). In our case we are accessing the std::vector at whatever index gets computed for _iml (possibly not the value of _iml itself since there are thread and cache range offset effects.) In our case I see that it was left over at _iml = 1 where its only valid value for our singleton instance is _iml = 0. I guess instead of a segfault it would be possible to add a VERBATIM block in the NET_RECEIVE INITIAL statement that asserts that _iml is less than the number of instances (it is not immediately clear how to determine the number of instances within the net_init function. I can turn all the segfaults into assert with a weird VERBATIM such as

NET_RECEIVE(w, a, b) {
    INITIAL {
VERBATIM
size_t _tst_iml{99999999999};
neuron::legacy::set_globals_from_prop(_pnt->_prop, _ml_real, _ml, _tst_iml);
assert(_iml == _tst_iml);
ENDVERBATIM
        a = foo
        b = ptr
    }
}

and I suppose the VERBATIM should have some version conditionals.
The alternative is just to rely on what seems to be the case that there is always a segfault or an assert error in the python test.

To answer my own question, I can actually trigger the segfault also with Debug; the trick is to do the following in gdb:

break netrec_init.cpp:252
p _ml->fpfield<1>(_iml)

which will trigger the segfault:

(gdb) p _ml->fpfield<1>(_iml)

Thread 1 "python3" received signal SIGSEGV, Segmentation fault.
neuron::cache::MechanismRange<2ul, 3ul>::data_array<1, 1> (this=0x7fffffffc910, instance=1)
    at build/include/neuron/cache/mechanism_range.hpp:101
101             return std::next(m_data_ptrs[variable], array_size * (m_data_offset + instance));
The program being debugged was signaled while in a function called from GDB.
GDB remains in the frame where the signal was received.
To change this behavior use "set unwind-on-signal on".
Evaluation of the expression containing the function
(neuron::cache::MechanismRange<2ul, 3ul>::fpfield<1>(unsigned long)) will be abandoned.
When the function is done executing, GDB will silently stop.

The use of macros makes the debugging a bit annoying, but in any case, this PR fixes the UB, so we can proceed.