Haivision / srt

Secure, Reliable, Transport
https://www.srtalliance.org
Mozilla Public License 2.0
3.12k stars 855 forks source link

[BUG] GCC 13 build warning #3064

Closed maxsharabayko closed 2 weeks ago

maxsharabayko commented 3 weeks ago
cmake . -DENABLE_BONDING=ON
make

produces the following warning with GCC 13.2 (Ubuntu 24.04)

[  1%] Building CXX object CMakeFiles/srt_virtual.dir/srtcore/group.cpp.o
In file included from /usr/include/c++/13/string:51,
                 from /usr/include/c++/13/bits/locale_classes.h:40,
                 from /usr/include/c++/13/bits/ios_base.h:41,
                 from /usr/include/c++/13/streambuf:43,
                 from /usr/include/c++/13/bits/streambuf_iterator.h:35,
                 from /usr/include/c++/13/iterator:66,
                 from /srt/srtcore/group.cpp:3:
In static member function ‘static _Up* std::__copy_move<_IsMove, true, std::random_access_iterator_tag>::__copy_m(_Tp*, _Tp*, _Up*) [with _Tp = unsigned char; _Up = unsigned char; bool _IsMove = false]’,
    inlined from ‘_OI std::__copy_move_a2(_II, _II, _OI) [with bool _IsMove = false; _II = unsigned char*; _OI = unsigned char*]’ at /usr/include/c++/13/bits/stl_algobase.h:506:30,
    inlined from ‘_OI std::__copy_move_a1(_II, _II, _OI) [with bool _IsMove = false; _II = unsigned char*; _OI = unsigned char*]’ at /usr/include/c++/13/bits/stl_algobase.h:533:42,
    inlined from ‘_OI std::__copy_move_a(_II, _II, _OI) [with bool _IsMove = false; _II = unsigned char*; _OI = __gnu_cxx::__normal_iterator<unsigned char*, vector<unsigned char> >]’ at /usr/include/c++/13/bits/stl_algobase.h:540:31,
    inlined from ‘_OI std::copy(_II, _II, _OI) [with _II = unsigned char*; _OI = __gnu_cxx::__normal_iterator<unsigned char*, vector<unsigned char> >]’ at /usr/include/c++/13/bits/stl_algobase.h:633:7,
    inlined from ‘srt::CUDTGroup::ConfigItem::ConfigItem(SRT_SOCKOPT, const void*, int)’ at /srt/srtcore/group.h:88:22,
    inlined from ‘void srt::importOption(std::vector<CUDTGroup::ConfigItem>&, SRT_SOCKOPT, const ValueType&) [with ValueType = bool]’ at /srt/srtcore/group.cpp:458:41:
/usr/include/c++/13/bits/stl_algobase.h:437:30: warning: ‘void* __builtin_memcpy(void*, const void*, long unsigned int)’ forming offset 1 is out of the bounds [0, 1] of object ‘opt’ with type ‘bool’ [-Warray-bounds=]
  437 |             __builtin_memmove(__result, __first, sizeof(_Tp) * _Num);
      |             ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/srt/srtcore/group.cpp: In function ‘void srt::importOption(std::vector<CUDTGroup::ConfigItem>&, SRT_SOCKOPT, const ValueType&) [with ValueType = bool]’:
/srt/srtcore/group.cpp:452:15: note: ‘opt’ declared here
  452 |     ValueType opt              = field;
      |               ^~~
[ 70%] Built target srt_virtual
maxsharabayko commented 2 weeks ago

Investigation notes

1. Any Type of 1 Byte

The warning is shown on any type with a size of 1 byte (char, uint8_t, bool, etc.), and not shown on, e.g. uint16_t. To check comment out all importOption calls in CGroup::deriveSettings(..) and experiment with the type of v:

uint8_t v = 32;
importOption(m_config, SRTO_TLPKTDROP, v);

2. Only If std::copy Is Used

The warning is not shown if std::copy is replaced with memcpy or a manual byte copy:

ConfigItem(SRT_SOCKOPT o, const void* val, int size)
    : so(o)
{
    value.resize(size);
    unsigned char* begin = (unsigned char*)val;
    // No warning with memcpy:
    //memcpy(value.data(), begin, size);
    // No warning with this kind of copy:
    //for (int i = 0; i < size; ++i)
    //    value[i] = begin[i];
    // This produces the warning:
    std::copy(begin, begin + size, value.begin());
}

3. Option Length Is Not Set Directly

The warning is not shown if the following if-statement is removed

template <class ValueType>
static void importOption(vector<CUDTGroup::ConfigItem>& storage, SRT_SOCKOPT optname, const ValueType& field)
{
    ValueType default_opt = ValueType();
    int       default_opt_size = sizeof(ValueType);
    ValueType opt              = field;
    // No warning if we remove this `if`, but keep the `push_back` that follows.
    if (!getOptDefault(optname, (&default_opt), (default_opt_size)) || default_opt != opt)
    {
        // Store the option when:
        // - no default for this option is found
        // - the option value retrieved from the field is different than default
        storage.push_back(CUDTGroup::ConfigItem(optname, &opt, default_opt_size));
    }
}

It looks like passing the default_opt_size by reference and potentially changing it makes the compiler produce the warning. The length is determined by the return value of the following template function:

template <class Type>
struct Value
{
    static int fill(void* optval, int, Type value)
    {
        // XXX assert size >= sizeof(Type) ?
        *(Type*)optval = value;
        return sizeof(Type);
    }
};

Although it seems to return 1 for a Boolean, but the compiler is not happy with such a manipulation.