Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix UndefinedBehaviourSanitizer errors. #1518

Merged
merged 1 commit into from
Oct 28, 2021
Merged

Conversation

olupton
Copy link
Collaborator

@olupton olupton commented Oct 28, 2021

This PR contains fixes for issues uncovered by the UndefinedBehaviour sanitizer: https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html.

units.cpp: index -1 out of bounds for type 'struct unit [20]'

A unit* pointer uspused the -1th element of an array as an invalid value. Fixed by using nullptr instead.

{path}/nrn/src/nmodl/../modlunit/units.cpp:569:19: runtime error: index -1 out of bounds for type 'struct unit [20]'
    #0 0x10fc3bfcf in unit_stk_clean() units.cpp:569
    #1 0x10fc3c7ed in unit_init() units.cpp:667
    #2 0x10fc3c069 in modl_units() units.cpp:607
    #3 0x10fb8aea8 in yyparse() parse1.ypp:1112
    #4 0x10fbdb9f7 in main modl.cpp:164
    #5 0x7fff20423f3c in start+0x0 (libdyld.dylib:x86_64+0x15f3c)

parallel_partrans test, null pointer arithmetic

{path}/nrn/src/nrniv/nonlinz.cpp:545:24: runtime error: applying non-zero offset 8 to null pointer
    #0 0x10178d03d in NonLinImpRep::current(int, Memb_list*, int) nonlinz.cpp:545
    #1 0x10177f71b in NonLinImpRep::didv() nonlinz.cpp:349
    #2 0x10177bc43 in NonLinImp::compute(double, double, int) nonlinz.cpp:148
    #3 0x101681cd3 in Imp::compute(double, bool, int) impedanc.cpp:265
    #4 0x101686917 in compute(void*) impedanc.cpp
    #5 0x101995f66 in hoc_call_ob_proc hoc_oop.cpp:759
    #6 0x10199913d in hoc_object_component() hoc_oop.cpp:1174
    #7 0x101dd135d in component(PyHocObject*) nrnpy_hoc.cpp:464
    #8 0x101dd03df in fcall(void*, void*) nrnpy_hoc.cpp:660
    #9 0x101837a7a in OcJumpImpl::fpycall(void* (*)(void*, void*), void*, void*) ocjump.cpp:222
    #10 0x101dcfe4d in hocobj_call(PyHocObject*, _object*, _object*) nrnpy_hoc.cpp:764
    #11 0x102a2ab13 in _PyObject_MakeTpCall+0x109 (Python:x86_64+0x32b13)
    #12 0x102ad3df2 in call_function+0x1c6 (Python:x86_64+0xdbdf2)
    #13 0x102ad1354 in _PyEval_EvalFrameDefault+0x6416 (Python:x86_64+0xd9354)
    #14 0x102a2b1b8 in function_code_fastcall+0x60 (Python:x86_64+0x331b8)
    #15 0x102ad3dbe in call_function+0x192 (Python:x86_64+0xdbdbe)
    #16 0x102ad13f2 in _PyEval_EvalFrameDefault+0x64b4 (Python:x86_64+0xd93f2)
    #17 0x102ad4897 in _PyEval_EvalCode+0x75e (Python:x86_64+0xdc897)
    #18 0x102acae85 in PyEval_EvalCode+0x38 (Python:x86_64+0xd2e85)
    #19 0x102b05a14 in run_eval_code_obj+0x6d (Python:x86_64+0x10da14)
    #20 0x102b04be8 in run_mod+0x66 (Python:x86_64+0x10cbe8)
    #21 0x102b04d66 in pyrun_file+0xd7 (Python:x86_64+0x10cd66)
    #22 0x102b033dd in PyRun_SimpleFileExFlags+0x2a3 (Python:x86_64+0x10b3dd)
    #23 0x101dcacc2 in nrnpython_start nrnpython.cpp:184
    #24 0x101451d97 in ivocmain_session(int, char const**, char const**, int) ivocmain.cpp:859
    #25 0x101451755 in ivocmain(int, char const**, char const**) ivocmain.cpp:415
    #26 0x10141da8f in main nrnmain.cpp:53
    #27 0x7fff20423f3c in start+0x0 (libdyld.dylib:x86_64+0x15f3c)

coming from https://github.com/neuronsimulator/nrn/blob/5873be8a580973b790d187a91b36102cb99a431d/src/nrniv/nonlinz.cpp#L545
fixed with mfake.prop = ml->prop ? ml->prop + in : nullptr;

Others

This is not comprehensive. #1479 and #1483 both describe additional issues.

Copy link
Member

@pramodk pramodk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

(cc @nrnhines: just a note, we are splitting #1479 into two smaller PRs so that easy/smaller changes could be merged quicker)

@codecov-commenter
Copy link

Codecov Report

Merging #1518 (b4ceebc) into master (f4089c8) will increase coverage by 0.08%.
The diff coverage is 100.00%.

❗ Current head b4ceebc differs from pull request most recent head bfcc603. Consider uploading reports for the commit bfcc603 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1518      +/-   ##
==========================================
+ Coverage   42.86%   42.95%   +0.08%     
==========================================
  Files         550      550              
  Lines      110342   110346       +4     
==========================================
+ Hits        47301    47400      +99     
+ Misses      63041    62946      -95     
Impacted Files Coverage Δ
src/modlunit/units.cpp 78.44% <100.00%> (+0.14%) ⬆️
src/nrniv/nonlinz.cpp 73.03% <100.00%> (ø)
share/lib/python/neuron/rxd/species.py 77.01% <0.00%> (+0.07%) ⬆️
src/parallel/bbs.cpp 64.55% <0.00%> (+1.68%) ⬆️
src/parallel/bbssrv2mpi.cpp 56.14% <0.00%> (+3.74%) ⬆️
src/nrnmpi/bbsmpipack.cpp 86.15% <0.00%> (+11.28%) ⬆️
src/parallel/bbsclimpi.cpp 50.58% <0.00%> (+22.09%) ⬆️
src/parallel/bbssrvmpi.cpp 44.57% <0.00%> (+27.71%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f4089c8...bfcc603. Read the comment docs.

@pramodk pramodk merged commit 72730bb into master Oct 28, 2021
@pramodk pramodk deleted the olupton/minimal-ubsan-fixes branch October 28, 2021 08:36
@alexsavulescu alexsavulescu mentioned this pull request Mar 22, 2022
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants