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

[core] Resolved GCC13 build warning regarding std::copy of a bool. #3066

Merged
merged 3 commits into from
Nov 7, 2024

Conversation

maxsharabayko
Copy link
Collaborator

The GCC 13 compiler produces a warning complaining that the length used for std::copy is out of bounds of an integer (or any other 1-byte type).
The source of this complaint was that the length was set in the getOptDefault, instead of a direct sizeof value. Even though the length returned for a Boolean is anyway determined via sizeof(Type) called from a template function, it is probably not that safe anyway.
Therefore just assert (in a debug build configuration) that the size returned is similar to the sizeof call used in the importOption function.

Fixes #3064.

@maxsharabayko maxsharabayko added Type: Maintenance Work required to maintain or clean up the code [core] Area: Changes in SRT library core labels Nov 6, 2024
@maxsharabayko maxsharabayko added this to the v1.5.4 milestone Nov 6, 2024
srtcore/group.cpp Outdated Show resolved Hide resolved
srtcore/group.cpp Outdated Show resolved Hide resolved
@maxsharabayko
Copy link
Collaborator Author

maxsharabayko commented Nov 6, 2024

The assertion is not always true. In particular, it is not holding for the Duration type on Travis Linux jobs (see this one).

Default length is 8 (sizeof std::chrono::duration), the length from getOptDefault is 4 (RD(-1) -> w_optlen = fillValue((pw_optval), w_optlen, -1);).

[Current thread is 1 (Thread 0x7f7212ae2700 (LWP 7413))]
#0  0x00007f7215733438 in raise () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x00007f721573503a in abort () from /lib/x86_64-linux-gnu/libc.so.6
#2  0x00007f721572bbe7 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#3  0x00007f721572bc92 in __assert_fail () from /lib/x86_64-linux-gnu/libc.so.6
#4  0x00000000009641b0 in srt::importOption<srt::sync::Duration<srt::sync::steady_clock> > (storage=std::vector of length 2, capacity 2 = {...}, optname=SRTO_CONNTIMEO, field=...) at /home/travis/build/Haivision/srt/srtcore/group.cpp:456
#5  0x000000000094daa6 in srt::CUDTGroup::deriveSettings (this=0x7f71ec006460, u=0x7f71ec000938) at /home/travis/build/Haivision/srt/srtcore/group.cpp:520
#6  0x000000000089bbe2 in srt::CUDT::makeMePeerOf (this=0x7f71ec000938, peergroup=1195754130, gtp=SRT_GTYPE_BROADCAST, link_flags=0) at /home/travis/build/Haivision/srt/srtcore/core.cpp:3327
#7  0x000000000089b10f in srt::CUDT::interpretGroup (this=0x7f71ec000938, groupdata=0x7f7212ae06c0, data_size=2, hsreq_type_cmd=1) at /home/travis/build/Haivision/srt/srtcore/core.cpp:3238
#8  0x0000000000897d9e in srt::CUDT::interpretSrtHandshake (this=0x7f71ec000938, hs=..., hspkt=..., out_data=0x7f7212ae0df0, pw_len=0x7f7212ae0b68) at /home/travis/build/Haivision/srt/srtcore/core.cpp:2923
#9  0x00000000008ae2b2 in srt::CUDT::acceptAndRespond (this=0x7f71ec000938, agent=..., peer=..., hspkt=..., w_hs=...) at /home/travis/build/Haivision/srt/srtcore/core.cpp:5832
#10 0x0000000000844d45 in srt::CUDTUnited::newConnection (this=0xd14fc0 <getInstance()::instance>, listen=122012304, peer=..., hspkt=..., w_hs=..., w_error=@0x7f7212ae14b0: 3, w_acpu=@0x7f7212ae14d0: 0x0) at /home/travis/build/Haivision/srt/srtcore/api.cpp:628
#11 0x00000000008d2d3f in srt::CUDT::processConnectRequest (this=0x7f71e4000f08, addr=..., packet=...) at /home/travis/build/Haivision/srt/srtcore/core.cpp:11249
#12 0x0000000000931ae8 in srt::CRcvQueue::worker_ProcessConnectionRequest (this=0x7f71e4007680, unit=0x7f71e4007888, addr=...) at /home/travis/build/Haivision/srt/srtcore/queue.cpp:1437
#13 0x000000000093035a in srt::CRcvQueue::worker (param=0x7f71e4007680) at /home/travis/build/Haivision/srt/srtcore/queue.cpp:1271
#14 0x00007f72167b56ba in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0
#15 0x00007f721580551d in clone () from /lib/x86_64-linux-gnu/libc.so.6

@maxsharabayko
Copy link
Collaborator Author

A similar bug with:

  • SRTO_PAYLOADSIZE: getOptDefault returns 4, the value in config is size_t (8 bytes)
  • SRTO_PBKEYLEN: getOptDefault returns 4, the value in config is size_t (8 bytes)

SRTO_CONNTIMEO, SRTO_PAYLOADSIZE, SRTO_PBKEYLEN had size autodetection bugs.
@ethouris
Copy link
Collaborator

ethouris commented Nov 7, 2024

We need to have a defined data types assigned for every such option, and all required fields should accord.

For these two options, the data type holding it should be exactly the same as the one expected by the user to supply when calling srt_set/getsockflag.

@maxsharabayko maxsharabayko merged commit a8c6b65 into Haivision:master Nov 7, 2024
12 checks passed
@maxsharabayko maxsharabayko deleted the hotfix/gcc13-warning branch November 7, 2024 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[core] Area: Changes in SRT library core Type: Maintenance Work required to maintain or clean up the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] GCC 13 build warning
2 participants