Out of bound access when compiling tests

[sciprosk]

I was trying to build Ruckig w/ tests and got out-of-bound access errors in some of the tests that was captured by asserts in STL. I've made a quick look into the code, and it looks like that the profile iterator is out bound for some velocity and potion interfaces when the number of valid profiles equals to three.

Please find some details in the pull request: https://github.com/pantor/ruckig/pull/189

[pantor]

Thanks for the heads-up and for the PR!

Which tests were you running exactly so that I have a chance to reproduce this? Just our tests (e.g. test-target) or any custom tests?

[sciprosk]

Thanks for getting back to me. I was running tests-target.

I did more testing this morning. It turns out that this problem is compiler dependent, which is probably an indication of numeric stability issues. When I compile it with GCC 11.4, all tests from test-target pass (from Windows Subsystem for Linux). The issues appear with MSVC 17.9.6 amd64 from Visual Studio Community 2022 with Windows SDK version 10.0.22621.

What exactly happens when I am building it with MSVC is as follows (Debug build).

1. When I try to run test-target I get C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.39.33519\include\array(138) : Assertion failed: cannot dereference out of range array iterator.

2. Stepping in with a debugger show that an issue is VelocityThirdOrderStep1::get_profile in time_acc0

I think this particular else-branch is unsafe because it has a potential of turning "innocent" numeric issues into more serious memory safety issues if profile goes out of bound

if (std::abs(af) < DBL_EPSILON) {
         // snippet
} else {
        time_none(profile, _aMax, _aMin, _jMax, false);
        time_none(profile, _aMin, _aMax, -_jMax, false);
        time_acc0(profile, _aMax, _aMin, _jMax, false);  // <-- trouble happens here
        time_acc0(profile, _aMin, _aMax, -_jMax, false);
    }

(I understand that the number of valid profiles is 3, but what if someone uses invalid input and sets acceleration and jerk to "infinite" values, for example).

3. Next step is to add assert(std::next(profile) != valid_profiles.end()); to get_profile.

inline void add_profile(ProfileIter& profile) const {
        assert(std::next(profile) != valid_profiles.end());  // <-- input validation
        const auto prev_profile = profile;
        ++profile;
        profile->set_boundary(*prev_profile);
 }

Now, I see that what fails is

Assertion failed: std::next(profile) != valid_profiles.end(), file C:\Users\Igor\source\repos\lib\ruckig\include\ruckig/velocity.hpp, line 32
===============================================================================
C:\Users\Igor\source\repos\lib\ruckig\test\test_target.cpp(999):
TEST CASE:  velocity-random-3

4. Finally, when I change the number of valid profiles from 3 to 4, this test passes, and it starts to fail from position.hpp. Then basically repeat steps 1, 2, 3, 4.

Please, let me know if you need some more details, or if I can help somehow.