Fix debug assert in Simple8b and Simple16 when input lenght is unknown (0)

[gustingonzalez]

Fixes #69 for Simple8b and Simple16.

The issue was occurring only in debug mode (not release mode) because decoding with an unknown input length (indicated by a value of 0) but passing nvalue (the number of elements to decode) failed on debug assertions not contemplating this case.

This patch fixes the assertion logic, and also adds tests for reproduce the case.

[lemire]

Please sync with our main branch. This should (???) fix the CI errors.

[gustingonzalez]

Thank you, @lemire, the branch is now updated; but (if I’m not wrong), the CI workflow requires approval?

[lemire]

It fails in CI, see:

=================================================================
==3436==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x515000002d00 at pc 0x55c75a5ef00b bp 0x7ffee5a3ed40 sp 0x7ffee5a3ed30
WRITE of size 4 at 0x515000002d00 thread T0
    #0 0x55c75a5ef00a in FastPForLib::Simple16<false>::unpack7_4(unsigned int**, unsigned int const**) /home/runner/work/FastPFOR/FastPFOR/headers/simple16.h:1083
    #1 0x55c75a5e9547 in FastPForLib::Simple16<false>::decodeArray(unsigned int const*, unsigned long, unsigned int*, unsigned long&) /home/runner/work/FastPFOR/FastPFOR/headers/simple16.h:739
    #2 0x55c75a5e4a31 in void FastPForLib::verifyUnknownInputLengthDecode<FastPForLib::Simple16<false> >(FastPForLib::Simple16<false>&, std::vector<unsigned int, std::allocator<unsigned int> > const&) /home/runner/work/FastPFOR/FastPFOR/unittest/util.h:20
    #3 0x55c75a5e3e56 in FastPForLib::Simple16Test_DecodesWithUnknownLength_Test::TestBody() /home/runner/work/FastPFOR/FastPFOR/unittest/test_simple16.cpp:14
    #4 0x55c75a63c8f2 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/runner/work/FastPFOR/FastPFOR/build/_deps/googletest-src/googletest/src/gtest.cc:2664
    #5 0x55c75a6348f4 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/runner/work/FastPFOR/FastPFOR/build/_deps/googletest-src/googletest/src/gtest.cc:2700
    #6 0x55c75a60b337 in testing::Test::Run() /home/runner/work/FastPFOR/FastPFOR/build/_deps/googletest-src/googletest/src/gtest.cc:2739
    #7 0x55c75a60be61 in testing::TestInfo::Run() /home/runner/work/FastPFOR/FastPFOR/build/_deps/googletest-src/googletest/src/gtest.cc:2885
    #8 0x55c75a60c84d in testing::TestSuite::Run() /home/runner/work/FastPFOR/FastPFOR/build/_deps/googletest-src/googletest/src/gtest.cc:3063
    #9 0x55c75a61e283 in testing::internal::UnitTestImpl::RunAllTests() /home/runner/work/FastPFOR/FastPFOR/build/_deps/googletest-src/googletest/src/gtest.cc:6054
    #10 0x55c75a63d86d in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/runner/work/FastPFOR/FastPFOR/build/_deps/googletest-src/googletest/src/gtest.cc:2664
    #11 0x55c75a635bc4 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/runner/work/FastPFOR/FastPFOR/build/_deps/googletest-src/googletest/src/gtest.cc:2700
    #12 0x55c75a61c27f in testing::UnitTest::Run() /home/runner/work/FastPFOR/FastPFOR/build/_deps/googletest-src/googletest/src/gtest.cc:5594
    #13 0x55c75a5a1e5a in RUN_ALL_TESTS() /home/runner/work/FastPFOR/FastPFOR/build/_deps/googletest-src/googletest/include/gtest/gtest.h:2334
    #14 0x55c75a5a1da8 in main /home/runner/work/FastPFOR/FastPFOR/unittest/test_driver.cpp:6
    #15 0x7f6a40c2a1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
    #16 0x7f6a40c2a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
    #17 0x55c75a57bff4 in _start (/home/runner/work/FastPFOR/FastPFOR/build/FastPFOR_unittest+0x40ff4) (BuildId: 371e9476f062d852505e5987dae9a41ee7d6a0ba)

0x515000002d00 is located 0 bytes after 512-byte region [0x515000002b00,0x515000002d00)
allocated by thread T0 here:
    #0 0x7f6a414fe548 in operator new(unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:95
    #1 0x55c75a591523 in std::__new_allocator<unsigned int>::allocate(unsigned long, void const*) /usr/include/c++/13/bits/new_allocator.h:151
    #2 0x55c75a58cd28 in std::allocator_traits<std::allocator<unsigned int> >::allocate(std::allocator<unsigned int>&, unsigned long) /usr/include/c++/13/bits/alloc_traits.h:482
    #3 0x55c75a58cd28 in std::_Vector_base<unsigned int, std::allocator<unsigned int> >::_M_allocate(unsigned long) /usr/include/c++/13/bits/stl_vector.h:381
    #4 0x55c75a58c980 in std::_Vector_base<unsigned int, std::allocator<unsigned int> >::_M_create_storage(unsigned long) /usr/include/c++/13/bits/stl_vector.h:398
    #5 0x55c75a587846 in std::_Vector_base<unsigned int, std::allocator<unsigned int> >::_Vector_base(unsigned long, std::allocator<unsigned int> const&) /usr/include/c++/13/bits/stl_vector.h:335
    #6 0x55c75a5b2880 in std::vector<unsigned int, std::allocator<unsigned int> >::vector(unsigned long, unsigned int const&, std::allocator<unsigned int> const&) /usr/include/c++/13/bits/stl_vector.h:571
    #7 0x55c75a5e4902 in void FastPForLib::verifyUnknownInputLengthDecode<FastPForLib::Simple16<false> >(FastPForLib::Simple16<false>&, std::vector<unsigned int, std::allocator<unsigned int> > const&) /home/runner/work/FastPFOR/FastPFOR/unittest/util.h:17
    #8 0x55c75a5e3e56 in FastPForLib::Simple16Test_DecodesWithUnknownLength_Test::TestBody() /home/runner/work/FastPFOR/FastPFOR/unittest/test_simple16.cpp:14
    #9 0x55c75a63c8f2 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/runner/work/FastPFOR/FastPFOR/build/_deps/googletest-src/googletest/src/gtest.cc:2664
    #10 0x55c75a6348f4 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/runner/work/FastPFOR/FastPFOR/build/_deps/googletest-src/googletest/src/gtest.cc:2700
    #11 0x55c75a60b337 in testing::Test::Run() /home/runner/work/FastPFOR/FastPFOR/build/_deps/googletest-src/googletest/src/gtest.cc:2739
    #12 0x55c75a60be61 in testing::TestInfo::Run() /home/runner/work/FastPFOR/FastPFOR/build/_deps/googletest-src/googletest/src/gtest.cc:2885
    #13 0x55c75a60c84d in testing::TestSuite::Run() /home/runner/work/FastPFOR/FastPFOR/build/_deps/googletest-src/googletest/src/gtest.cc:3063
    #14 0x55c75a61e283 in testing::internal::UnitTestImpl::RunAllTests() /home/runner/work/FastPFOR/FastPFOR/build/_deps/googletest-src/googletest/src/gtest.cc:6054
    #15 0x55c75a63d86d in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/runner/work/FastPFOR/FastPFOR/build/_deps/googletest-src/googletest/src/gtest.cc:2664
    #16 0x55c75a635bc4 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/runner/work/FastPFOR/FastPFOR/build/_deps/googletest-src/googletest/src/gtest.cc:2700
    #17 0x55c75a61c27f in testing::UnitTest::Run() /home/runner/work/FastPFOR/FastPFOR/build/_deps/googletest-src/googletest/src/gtest.cc:5594
    #18 0x55c75a5a1e5a in RUN_ALL_TESTS() /home/runner/work/FastPFOR/FastPFOR/build/_deps/googletest-src/googletest/include/gtest/gtest.h:2334
    #19 0x55c75a5a1da8 in main /home/runner/work/FastPFOR/FastPFOR/unittest/test_driver.cpp:6
    #20 0x7f6a40c2a1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
    #21 0x7f6a40c2a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 274eec488d230825a136fa9c4d85370fed7a0a5e)
    #22 0x55c75a57bff4 in _start (/home/runner/work/FastPFOR/FastPFOR/build/FastPFOR_unittest+0x40ff4) (BuildId: 371e9476f062d852505e5987dae9a41ee7d6a0ba)

[gustingonzalez]

Ok, got it!

I was testing only in debug mode, and the issue was happening only in release mode. Honestly, I don't know why this difference. However, I see the cause of the error is that Simple16 can overrun the output buffer.

Suppose a case where n = 4, and Simple16 stores two packs: the first pack encoding 1 element, and the second pack encoding the remaining ones. Also, suppose the second pack uses the selector that encodes 28 integers. In that case, Simple16 will try to decode 1 + 28 = 29 elements, even though n = 4. As a result, the output buffer gets overrun.

The solution wasn't complex. I simply added a headroom of "28" (the maximum number of elements in a single pack) to the output, and it worked.

However, my question now is why this doesn’t happen with Simple8b. From my understanding, this is because it uses carefulunpack that avoids the described situation. Is that correct?

[lemire]

Merging.