Beckhoff / ADS

Beckhoff protocol to communicate with TwinCAT devices.
MIT License
493 stars 194 forks source link

Fail to compile on ArchLinux: Wrong deleter used in library #180

Closed alexanderwwagner closed 1 year ago

alexanderwwagner commented 1 year ago

Describe the Bug I try to compile ADS on Archlinux and get an error after calling ninja.

Environment OS: Archlinux gcc: 12.2.0

To Reproduce meson build

The Meson build system
Version: 0.64.1
Source dir: /home/alexander/Entwicklung/KMT/ADS_Test_C++/ADS
Build dir: /home/alexander/Entwicklung/KMT/ADS_Test_C++/ADS/build
Build type: native build
Project name: AdsLib
Project version: 0.1
C++ compiler for the host machine: c++ (gcc 12.2.0 "c++ (GCC) 12.2.0")
C++ linker for the host machine: c++ ld.bfd 2.39.0
Host machine cpu family: x86_64
Host machine cpu: x86_64
Library ws2_32 found: NO
Run-time dependency threads found: YES
Build targets in project: 4

Found ninja-1.11.1 at /usr/bin/ninja
WARNING: Running the setup command as `meson [options]` instead of `meson setup [options]` is ambiguous and deprecated.

ninja -C build

ninja: Entering directory `build'
[13/24] Compiling C++ object libAdsLib.a.p/AdsLib_standalone_AdsLib.cpp.o
FAILED: libAdsLib.a.p/AdsLib_standalone_AdsLib.cpp.o 
c++ -IlibAdsLib.a.p -I. -I.. -I../AdsLib -I../tools -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Wpedantic -Werror -O3 -D_FORTIFY_SOURCE=2 -fPIC -MD -MQ libAdsLib.a.p/AdsLib_standalone_AdsLib.cpp.o -MF libAdsLib.a.p/AdsLib_standalone_AdsLib.cpp.o.d -o libAdsLib.a.p/AdsLib_standalone_AdsLib.cpp.o -c ../AdsLib/standalone/AdsLib.cpp
In file included from /usr/include/c++/12.2.0/bits/shared_ptr.h:53,
                 from /usr/include/c++/12.2.0/memory:77,
                 from ../AdsLib/RingBuffer.h:10,
                 from ../AdsLib/AdsNotification.h:9,
                 from ../AdsLib/NotificationDispatcher.h:8,
                 from ../AdsLib/AmsPort.h:8,
                 from ../AdsLib/AmsConnection.h:8,
                 from ../AdsLib/AmsRouter.h:8,
                 from ../AdsLib/standalone/AdsLib.cpp:7:
In constructor ‘std::__shared_count<_Lp>::__shared_count(_Ptr) [with _Ptr = unsigned char*; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’,
    inlined from ‘std::__shared_count<_Lp>::__shared_count(_Ptr, std::false_type) [with _Ptr = unsigned char*; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’ at /usr/include/c++/12.2.0/bits/shared_ptr_base.h:928:22,
    inlined from ‘std::__shared_ptr<_Tp, _Lp>::__shared_ptr(_Yp*) [with _Yp = unsigned char; <template-parameter-2-2> = void; _Tp = unsigned char; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’ at /usr/include/c++/12.2.0/bits/shared_ptr_base.h:1469:17,
    inlined from ‘std::shared_ptr<_Tp>::shared_ptr(_Yp*) [with _Yp = unsigned char; <template-parameter-2-2> = void; _Tp = unsigned char]’ at /usr/include/c++/12.2.0/bits/shared_ptr.h:214:46,
    inlined from ‘Notification::Notification(PAdsNotificationFuncEx, uint32_t, uint32_t, AmsAddr, uint16_t)’ at ../AdsLib/AdsNotification.h:25:9,
    inlined from ‘void std::_Construct(_Tp*, _Args&& ...) [with _Tp = Notification; _Args = {void (*&)(const AmsAddr*, const AdsNotificationHeader*, unsigned int), unsigned int&, const unsigned int&, const AmsAddr&, short unsigned int}]’ at /usr/include/c++/12.2.0/bits/stl_construct.h:119:7,
    inlined from ‘static void std::allocator_traits<std::allocator<void> >::construct(allocator_type&, _Up*, _Args&& ...) [with _Up = Notification; _Args = {void (*&)(const AmsAddr*, const AdsNotificationHeader*, unsigned int), unsigned int&, const unsigned int&, const AmsAddr&, short unsigned int}]’ at /usr/include/c++/12.2.0/bits/alloc_traits.h:635:19,
    inlined from ‘std::_Sp_counted_ptr_inplace<_Tp, _Alloc, _Lp>::_Sp_counted_ptr_inplace(_Alloc, _Args&& ...) [with _Args = {void (*&)(const AmsAddr*, const AdsNotificationHeader*, unsigned int), unsigned int&, const unsigned int&, const AmsAddr&, short unsigned int}; _Tp = Notification; _Alloc = std::allocator<void>; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’ at /usr/include/c++/12.2.0/bits/shared_ptr_base.h:604:39,
    inlined from ‘std::__shared_count<_Lp>::__shared_count(_Tp*&, std::_Sp_alloc_shared_tag<_Alloc>, _Args&& ...) [with _Tp = Notification; _Alloc = std::allocator<void>; _Args = {void (*&)(const AmsAddr*, const AdsNotificationHeader*, unsigned int), unsigned int&, const unsigned int&, const AmsAddr&, short unsigned int}; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’ at /usr/include/c++/12.2.0/bits/shared_ptr_base.h:971:16,
    inlined from ‘std::__shared_ptr<_Tp, _Lp>::__shared_ptr(std::_Sp_alloc_shared_tag<_Tp>, _Args&& ...) [with _Alloc = std::allocator<void>; _Args = {void (*&)(const AmsAddr*, const AdsNotificationHeader*, unsigned int), unsigned int&, const unsigned int&, const AmsAddr&, short unsigned int}; _Tp = Notification; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’ at /usr/include/c++/12.2.0/bits/shared_ptr_base.h:1712:14,
    inlined from ‘std::shared_ptr<_Tp>::shared_ptr(std::_Sp_alloc_shared_tag<_Tp>, _Args&& ...) [with _Alloc = std::allocator<void>; _Args = {void (*&)(const AmsAddr*, const AdsNotificationHeader*, unsigned int), unsigned int&, const unsigned int&, const AmsAddr&, short unsigned int}; _Tp = Notification]’ at /usr/include/c++/12.2.0/bits/shared_ptr.h:464:59,
    inlined from ‘std::shared_ptr<typename std::enable_if<(! std::is_array< <template-parameter-1-1> >::value), _Tp>::type> std::make_shared(_Args&& ...) [with _Tp = Notification; _Args = {void (*&)(const AmsAddr*, const AdsNotificationHeader*, unsigned int), unsigned int&, const unsigned int&, const AmsAddr&, short unsigned int}]’ at /usr/include/c++/12.2.0/bits/shared_ptr.h:1010:39,
    inlined from ‘long int AdsSyncAddDeviceNotificationReqEx(long int, const AmsAddr*, uint32_t, uint32_t, const AdsNotificationAttrib*, PAdsNotificationFuncEx, uint32_t, uint32_t*)’ at ../AdsLib/standalone/AdsLib.cpp:294:109:
/usr/include/c++/12.2.0/bits/shared_ptr_base.h:921:15: error: ‘void operator delete(void*, std::size_t)’ called on pointer returned from a mismatched allocation function [-Werror=mismatched-new-delete]
  921 |               delete __p;
      |               ^~~~~~~~~~
In constructor ‘Notification::Notification(PAdsNotificationFuncEx, uint32_t, uint32_t, AmsAddr, uint16_t)’,
    inlined from ‘void std::_Construct(_Tp*, _Args&& ...) [with _Tp = Notification; _Args = {void (*&)(const AmsAddr*, const AdsNotificationHeader*, unsigned int), unsigned int&, const unsigned int&, const AmsAddr&, short unsigned int}]’ at /usr/include/c++/12.2.0/bits/stl_construct.h:119:7,
    inlined from ‘static void std::allocator_traits<std::allocator<void> >::construct(allocator_type&, _Up*, _Args&& ...) [with _Up = Notification; _Args = {void (*&)(const AmsAddr*, const AdsNotificationHeader*, unsigned int), unsigned int&, const unsigned int&, const AmsAddr&, short unsigned int}]’ at /usr/include/c++/12.2.0/bits/alloc_traits.h:635:19,
    inlined from ‘std::_Sp_counted_ptr_inplace<_Tp, _Alloc, _Lp>::_Sp_counted_ptr_inplace(_Alloc, _Args&& ...) [with _Args = {void (*&)(const AmsAddr*, const AdsNotificationHeader*, unsigned int), unsigned int&, const unsigned int&, const AmsAddr&, short unsigned int}; _Tp = Notification; _Alloc = std::allocator<void>; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’ at /usr/include/c++/12.2.0/bits/shared_ptr_base.h:604:39,
    inlined from ‘std::__shared_count<_Lp>::__shared_count(_Tp*&, std::_Sp_alloc_shared_tag<_Alloc>, _Args&& ...) [with _Tp = Notification; _Alloc = std::allocator<void>; _Args = {void (*&)(const AmsAddr*, const AdsNotificationHeader*, unsigned int), unsigned int&, const unsigned int&, const AmsAddr&, short unsigned int}; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’ at /usr/include/c++/12.2.0/bits/shared_ptr_base.h:971:16,
    inlined from ‘std::__shared_ptr<_Tp, _Lp>::__shared_ptr(std::_Sp_alloc_shared_tag<_Tp>, _Args&& ...) [with _Alloc = std::allocator<void>; _Args = {void (*&)(const AmsAddr*, const AdsNotificationHeader*, unsigned int), unsigned int&, const unsigned int&, const AmsAddr&, short unsigned int}; _Tp = Notification; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’ at /usr/include/c++/12.2.0/bits/shared_ptr_base.h:1712:14,
    inlined from ‘std::shared_ptr<_Tp>::shared_ptr(std::_Sp_alloc_shared_tag<_Tp>, _Args&& ...) [with _Alloc = std::allocator<void>; _Args = {void (*&)(const AmsAddr*, const AdsNotificationHeader*, unsigned int), unsigned int&, const unsigned int&, const AmsAddr&, short unsigned int}; _Tp = Notification]’ at /usr/include/c++/12.2.0/bits/shared_ptr.h:464:59,
    inlined from ‘std::shared_ptr<typename std::enable_if<(! std::is_array< <template-parameter-1-1> >::value), _Tp>::type> std::make_shared(_Args&& ...) [with _Tp = Notification; _Args = {void (*&)(const AmsAddr*, const AdsNotificationHeader*, unsigned int), unsigned int&, const unsigned int&, const AmsAddr&, short unsigned int}]’ at /usr/include/c++/12.2.0/bits/shared_ptr.h:1010:39,
    inlined from ‘long int AdsSyncAddDeviceNotificationReqEx(long int, const AmsAddr*, uint32_t, uint32_t, const AdsNotificationAttrib*, PAdsNotificationFuncEx, uint32_t, uint32_t*)’ at ../AdsLib/standalone/AdsLib.cpp:294:109:
../AdsLib/AdsNotification.h:25:66: note: returned from ‘void* operator new [](std::size_t)’
   25 |         buffer(new uint8_t[sizeof(AdsNotificationHeader) + length]),
      |                                                                  ^
cc1plus: all warnings being treated as errors
[20/24] Compiling C++ object AdsLibTest.p/AdsLibTest_main.cpp.o
ninja: build stopped: subcommand failed.

Expected Behavior Succeed library build

Reproducibility 100%

pbruenn commented 1 year ago

Ooops, we use the wrong deleter. This fix should work, but I guess it would be better to switch to std::vector

diff --git a/AdsLib/AdsNotification.h b/AdsLib/AdsNotification.h
index 5cd0192..f686c65 100644
--- a/AdsLib/AdsNotification.h
+++ b/AdsLib/AdsNotification.h
@@ -55,6 +55,6 @@ struct Notification {

 private:
     const PAdsNotificationFuncEx callback;
-    const std::shared_ptr<uint8_t> buffer;
+    const std::shared_ptr<uint8_t[]> buffer;
     const uint32_t hUser;
 };
alexanderwwagner commented 1 year ago

Okay thank you for the workaround. It works!

pbruenn commented 1 year ago

This should be fixed by commit b0b1e7846a0693ec0c574c34d743c53c1038057d additionally we now build test internally on archlinux, too (05d084bd328e63e84531400c1363a3a4da091168)