paullouisageneau / libdatachannel

C/C++ WebRTC network library featuring Data Channels, Media Transport, and WebSockets
https://libdatachannel.org/
Mozilla Public License 2.0
1.8k stars 361 forks source link

clang analyzer issue in rtc.cpp #91

Closed arvidn closed 4 years ago

arvidn commented 4 years ago

Sorry for just dumping this here, I haven't had time to look into this myself yet. Feel free to close if you think this is a false positive:

/home/arvid/dev/libtorrent/deps/libdatachannel/src/rtc.cpp:332:3: error: Inner pointer of container used after re/deallocation [clang-analyzer-cplusplus.InnerPointer,-warnings-as-errors]
                std::copy(addr->data(), addr->data() + size, buffer);
                ^
/home/arvid/dev/libtorrent/deps/libdatachannel/src/rtc.cpp:327:2: note: Taking false branch
        if (!peerConnection)
        ^
/home/arvid/dev/libtorrent/deps/libdatachannel/src/rtc.cpp:330:11: note: Assuming the condition is true
        if (auto addr = peerConnection->localAddress()) {
                 ^
/home/arvid/dev/libtorrent/deps/libdatachannel/src/rtc.cpp:330:2: note: Taking true branch
        if (auto addr = peerConnection->localAddress()) {
        ^
/home/arvid/dev/libtorrent/deps/libdatachannel/src/rtc.cpp:332:13: note: Pointer to inner buffer of 'std::__1::__optional_destruct_base<class std::__1::basic_string<char>, false>::value_type' obtained here
                std::copy(addr->data(), addr->data() + size, buffer);
                          ^
/home/arvid/dev/libtorrent/deps/libdatachannel/src/rtc.cpp:332:27: note: Calling 'optional::operator->'
                std::copy(addr->data(), addr->data() + size, buffer);
                                        ^
/usr/lib/llvm-10/bin/../include/c++/v1/optional:894:16: note: Inner buffer of 'std::__1::__optional_destruct_base<class std::__1::basic_string<char>, false>::value_type' reallocated by call to 'addressof'
        return _VSTD::addressof(this->__get());
               ^
/usr/lib/llvm-10/bin/../include/c++/v1/__config:782:15: note: expanded from macro '_VSTD'
#define _VSTD std::_LIBCPP_ABI_NAMESPACE
              ^
/home/arvid/dev/libtorrent/deps/libdatachannel/src/rtc.cpp:332:27: note: Returning; inner buffer was reallocated
                std::copy(addr->data(), addr->data() + size, buffer);
                                        ^
/home/arvid/dev/libtorrent/deps/libdatachannel/src/rtc.cpp:332:3: note: Inner pointer of container used after re/deallocation
                std::copy(addr->data(), addr->data() + size, buffer);
                ^
/home/arvid/dev/libtorrent/deps/libdatachannel/src/rtc.cpp:346:3: error: Inner pointer of container used after re/deallocation [clang-analyzer-cplusplus.InnerPointer,-warnings-as-errors]
                std::copy(addr->data(), addr->data() + size, buffer);
                ^
/home/arvid/dev/libtorrent/deps/libdatachannel/src/rtc.cpp:341:2: note: Taking false branch
        if (!peerConnection)
        ^
/home/arvid/dev/libtorrent/deps/libdatachannel/src/rtc.cpp:344:11: note: Assuming the condition is true
        if (auto addr = peerConnection->remoteAddress()) {
                 ^
/home/arvid/dev/libtorrent/deps/libdatachannel/src/rtc.cpp:344:2: note: Taking true branch
        if (auto addr = peerConnection->remoteAddress()) {
        ^
/home/arvid/dev/libtorrent/deps/libdatachannel/src/rtc.cpp:346:13: note: Pointer to inner buffer of 'std::__1::__optional_destruct_base<class std::__1::basic_string<char>, false>::value_type' obtained here
                std::copy(addr->data(), addr->data() + size, buffer);
                          ^
/home/arvid/dev/libtorrent/deps/libdatachannel/src/rtc.cpp:346:27: note: Calling 'optional::operator->'
                std::copy(addr->data(), addr->data() + size, buffer);
                                        ^
/usr/lib/llvm-10/bin/../include/c++/v1/optional:894:16: note: Inner buffer of 'std::__1::__optional_destruct_base<class std::__1::basic_string<char>, false>::value_type' reallocated by call to 'addressof'
        return _VSTD::addressof(this->__get());
               ^
/usr/lib/llvm-10/bin/../include/c++/v1/__config:782:15: note: expanded from macro '_VSTD'
#define _VSTD std::_LIBCPP_ABI_NAMESPACE
              ^
/home/arvid/dev/libtorrent/deps/libdatachannel/src/rtc.cpp:346:27: note: Returning; inner buffer was reallocated
                std::copy(addr->data(), addr->data() + size, buffer);
                                        ^
/home/arvid/dev/libtorrent/deps/libdatachannel/src/rtc.cpp:346:3: note: Inner pointer of container used after re/deallocation
                std::copy(addr->data(), addr->data() + size, buffer);
                ^
paullouisageneau commented 4 years ago

I wonder why arrowing the optional would reallocate the string like clang suggests. Anyway it'd be easy to go around it. FYI this is the implementation of the C API, so this part is not used from libtorrent.

arvidn commented 4 years ago

yeah, looking a bit closer, it looks suspicious. addressof() shouldn't cause anything to reallocate