Fix rtp packet test

[penguinol]

[ibc]

What is this fixing exactly? I've never seen those tests failing due to memory problems.

[penguinol]

What is this fixing exactly? I've never seen those tests failing due to memory problems.

When i ran tests under windows, it says buffer is overflow. Maybe u can see the warning with AddressSanitzer or valgrind. In these tests, packet size will be 52 after setting excetions, so we need to make sure buffer is large enough.

[nazar-pc]

When i ran tests under windows, it says buffer is overflow. Maybe u can see the warning with AddressSanitzer or valgrind. In these tests, packet size will be 52 after setting excetions, so we need to make sure buffer is large enough.

We do run tests in CI on Windows too :thinking:

Either way, that is a useful details and exactly the kind of information that should be included in PR description from the beginning.

[ibc]

I think we have a test-asan task in Makefile/tasks.py. Let me please test it in 10 days when I'm back before we merge this PR just to confirm.

[penguinol]

(mediasoup_buildvenv) [root@784dfa00-aa18-11ee-bad9-a4bf015565d5 worker]# ./out/Debug/mediasoup-worker-test-asan
Randomness seeded to: 446808779
=================================================================
==2099730==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff7ac12c80 at pc 0x7f231efafe7e bp 0x7fff7abf9940 sp 0x7fff7abf90f0
WRITE of size 16 at 0x7fff7ac12c80 thread T0
    #0 0x7f231efafe7d in memmove (/opt/sysroot64/lib/libasan.so.6+0x41e7d)
    #1 0x66d521 in RTC::RtpPacket::SetExtensions(unsigned char, std::vector<RTC::RtpPacket::GenericExtension, std::allocator<RTC::RtpPacket::GenericExtension> > const&) ../../../src/RTC/RtpPacket.cpp:471
    #2 0x9381bd in CATCH2_INTERNAL_TEST_0 ../../../test/src/RTC/TestRtpPacket.cpp:605
    #3 0xf1395d in invoke ../../../subprojects/Catch2-3.4.0/src/catch2/internal/catch_test_registry.cpp:58
    #4 0xf08a94 in Catch::TestCaseHandle::invoke() const ../../../subprojects/Catch2-3.4.0/src/catch2/catch_test_case_info.hpp:115
    #5 0xf078ac in Catch::RunContext::invokeActiveTestCase() ../../../subprojects/Catch2-3.4.0/src/catch2/internal/catch_run_context.cpp:552
    #6 0xf075fb in Catch::RunContext::runCurrentTest(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) ../../../subprojects/Catch2-3.4.0/src/catch2/internal/catch_run_context.cpp:515
    #7 0xf05cc7 in Catch::RunContext::runTest(Catch::TestCaseHandle const&) ../../../subprojects/Catch2-3.4.0/src/catch2/internal/catch_run_context.cpp:239
    #8 0xf1a773 in execute ../../../subprojects/Catch2-3.4.0/src/catch2/catch_session.cpp:111
    #9 0xf1bb3f in Catch::Session::runInternal() ../../../subprojects/Catch2-3.4.0/src/catch2/catch_session.cpp:333
    #10 0xf1b681 in Catch::Session::run() ../../../subprojects/Catch2-3.4.0/src/catch2/catch_session.cpp:264
    #11 0x8f528d in int Catch::Session::run<char>(int, char const* const*) ../../../subprojects/Catch2-3.4.0/src/catch2/catch_session.hpp:41
    #12 0x8f4f9a in main ../../../test/src/tests.cpp:56
    #13 0x7f231ea1e86c in __libc_start_main (/opt/sysroot64/lib/libc.so.6+0x2386c)
    #14 0x413029 in _start (/root/mediasoup-fix_tcc_feedback/worker/out/Debug/mediasoup-worker-test-asan+0x413029)

[penguinol]

By the way, when i ran mediasoup_test_asan release version, it show:

AddressSanitizer:DEADLYSIGNAL
=================================================================
==2092397==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x0000004e4d91 bp 0x00000000001f sp 0x7ffe438c49e0 T0)
==2092397==The signal is caused by a READ memory access.
==2092397==Hint: address points to the zero page.
    #0 0x4e4d91 in void absl::lts_20230802::container_internal::InitializeSlots<std::allocator<char>, 40ul, 8ul>(absl::lts_20230802::container_internal::CommonFields&, std::allocator<char>) [clone .isra.0] (/root/mediasoup-fix_tcc_feedback/worker/out/Release/mediasoup-worker-test-asan+0x4e4d91)
    #1 0x4f4def in absl::lts_20230802::container_internal::raw_hash_set<absl::lts_20230802::container_internal::FlatHashMapPolicy<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, LogLevel>, absl::lts_20230802::container_internal::StringHash, absl::lts_20230802::container_internal::StringEq, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, LogLevel> > >::raw_hash_set(unsigned long, absl::lts_20230802::container_internal::StringHash const&, absl::lts_20230802::container_internal::StringEq const&, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, LogLevel> > const&) (/root/mediasoup-fix_tcc_feedback/worker/out/Release/mediasoup-worker-test-asan+0x4f4def)
    #2 0x4a4411 in __static_initialization_and_destruction_0(int, int) [clone .constprop.0] (/root/mediasoup-fix_tcc_feedback/worker/out/Release/mediasoup-worker-test-asan+0x4a4411)
    #3 0x121fab4 in __libc_csu_init (/root/mediasoup-fix_tcc_feedback/worker/out/Release/mediasoup-worker-test-asan+0x121fab4)
    #4 0x7faa50b2d7fc in __libc_start_main (/opt/sysroot64/lib/libc.so.6+0x237fc)
    #5 0x4d0af9 in _start (/root/mediasoup-fix_tcc_feedback/worker/out/Release/mediasoup-worker-test-asan+0x4d0af9)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/root/mediasoup-fix_tcc_feedback/worker/out/Release/mediasoup-worker-test-asan+0x4e4d91) in void absl::lts_20230802::container_internal::InitializeSlots<std::allocator<char>, 40ul, 8ul>(absl::lts_20230802::container_internal::CommonFields&, std::allocator<char>) [clone .isra.0]
==2092397==ABORTING

But debug version won't.

[penguinol]

This PR fix the same problem as https://github.com/versatica/mediasoup/pull/1120

[ibc]

@penguinol, here a PR adding ASAN to Node tests: https://github.com/versatica/mediasoup/pull/1415

Perhaps we should have the same in worker tests?

[ibc]

@penguinol, here a PR adding ASAN to Node tests: #1415

Perhaps we should have the same in worker tests?

Actually we already have make/invoke test-asan...

[ibc]

@penguinol: https://github.com/versatica/mediasoup/pull/1416

[ibc]

@penguinol I don't see that changes in this PR fix the ASAN issues related to RtpPacket. Probably unrelated but those are the issues I get:

https://github.com/versatica/mediasoup/pull/1416#issuecomment-2199756593

[ibc]

@penguinol we are addressing these and other ASAN issues in this PR: https://github.com/versatica/mediasoup/pull/1416#issuecomment-2199897238

And yes, changes include those in your PR. So thanks.