mdavidsaver / pvxs

PVA protocol client/server library and utilities.
https://mdavidsaver.github.io/pvxs/
Other
19 stars 25 forks source link

(apparently) spurious compile error with GCC 12 #51

Closed mdavidsaver closed 11 months ago

mdavidsaver commented 11 months ago

G++ 12 (default w/ debian 12) gives the following error with -O2 -Wall. This appears to be spurious. G++ 11.x and 13.x do not complain.

In file included from ../testshared.cpp:12:
In constructor ‘pvxs::shared_array<E, Enable>::shared_array(size_t, V) [with V = std::nullptr_t; E = std::unique_ptr<unsigned int>; Enable = void]’,
    inlined from ‘void {anonymous}::testComplex()’ at ../testshared.cpp:225:57:
../../src/pvxs/sharedArray.h:288:17: error: pointer used after ‘void operator delete [](void*, std::size_t)’ [-Werror=use-after-free]
  288 |         :base_t(new _E_non_const[c], c)
      |                 ^~~~~~~~~~~~~~~~~~~
In member function ‘void pvxs::detail::sa_default_delete<E>::operator()(E*) const [with E = std::unique_ptr<unsigned int>]’,
    inlined from ‘std::__shared_count<_Lp>::__shared_count(_Ptr, _Deleter, _Alloc) [with _Ptr = std::unique_ptr<unsigned int>*; _Deleter = pvxs::detail::sa_default_delete<std::unique_ptr<unsigned int> >; _Alloc = std::allocator<void>; <template-parameter-2-4> = void; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’ at /usr/include/c++/12/bits/shared_ptr_base.h:958:11,
    inlined from ‘std::__shared_count<_Lp>::__shared_count(_Ptr, _Deleter) [with _Ptr = std::unique_ptr<unsigned int>*; _Deleter = pvxs::detail::sa_default_delete<std::unique_ptr<unsigned int> >; <template-parameter-2-3> = void; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’ at /usr/include/c++/12/bits/shared_ptr_base.h:939:57,
    inlined from ‘std::__shared_ptr<_Tp, _Lp>::__shared_ptr(_Yp*, _Deleter) [with _Yp = std::unique_ptr<unsigned int>; _Deleter = pvxs::detail::sa_default_delete<std::unique_ptr<unsigned int> >; <template-parameter-2-3> = void; _Tp = std::unique_ptr<unsigned int>; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’ at /usr/include/c++/12/bits/shared_ptr_base.h:1478:17,
    inlined from ‘std::shared_ptr<_Tp>::shared_ptr(_Yp*, _Deleter) [with _Yp = std::unique_ptr<unsigned int>; _Deleter = pvxs::detail::sa_default_delete<std::unique_ptr<unsigned int> >; <template-parameter-2-3> = void; _Tp = std::unique_ptr<unsigned int>]’ at /usr/include/c++/12/bits/shared_ptr.h:232:48,
    inlined from ‘pvxs::detail::sa_base<E>::sa_base(A*, size_t) [with A = std::unique_ptr<unsigned int>; E = std::unique_ptr<unsigned int>]’ at ../../src/pvxs/sharedArray.h:136:10,
    inlined from ‘pvxs::shared_array<E, Enable>::shared_array(size_t, V) [with V = std::nullptr_t; E = std::unique_ptr<unsigned int>; Enable = void]’ at ../../src/pvxs/sharedArray.h:288:39,
    inlined from ‘void {anonymous}::testComplex()’ at ../testshared.cpp:225:57:
../../src/pvxs/sharedArray.h:92:35: note: call to ‘void operator delete [](void*, std::size_t)’ here
   92 |     void operator()(E* e) const { delete[] e; }
      |                                   ^~~~~~~~~~
mdavidsaver commented 11 months ago

A reduced [test case](https://godbolt.org/#g:!((g:!((g:!((h:codeEditor,i:(filename:'1',fontScale:14,fontUsePx:'0',j:1,lang:c%2B%2B,selection:(endColumn:1,endLineNumber:14,positionColumn:1,positionLineNumber:14,selectionStartColumn:1,selectionStartLineNumber:14,startColumn:1,startLineNumber:14),source:'%23include+%3Cmemory%3E%0A%0Atemplate%3Ctypename+E%3E%0Astruct+sa_default_delete+%7B%0A++++void+operator()(E*+e)+const+%7B+delete%5B%5D+e%3B+%7D%0A%7D%3B%0A%0Aint+x(int+z)+%7B%0A++++typedef+std::unique_ptr%3Cint%3E+T%3B%0A++++std::shared_ptr%3CT%3E+y(new+T%5B2%5D,+sa_default_delete%3CT%3E())%3B%0A++++y.get()%5B0%5D+%3D+T(new+int(z))%3B%0A++++return+*y.get()%5B0%5D%3B%0A%7D%0A'),l:'5',n:'0',o:'C%2B%2B+source+%231',t:'0')),k:33.333333333333336,l:'4',n:'0',o:'',s:0,t:'0'),(g:!((h:compiler,i:(compiler:g123,deviceViewOpen:'1',filters:(b:'0',binary:'1',binaryObject:'1',commentOnly:'0',debugCalls:'1',demangle:'0',directives:'0',execute:'1',intel:'0',libraryCode:'0',trim:'1'),flagsViewOpen:'1',fontScale:14,fontUsePx:'0',j:1,lang:c%2B%2B,libs:!(),options:'-O2+-Wall+-Werror++-g',overrides:!(),selection:(endColumn:1,endLineNumber:1,positionColumn:1,positionLineNumber:1,selectionStartColumn:1,selectionStartLineNumber:1,startColumn:1,startLineNumber:1),source:1),l:'5',n:'0',o:'+x86-64+gcc+12.3+(Editor+%231)',t:'0')),k:33.333333333333336,l:'4',n:'0',o:'',s:0,t:'0'),(g:!((h:output,i:(compilerName:'x86-64+gcc+12.2',editorid:1,fontScale:14,fontUsePx:'0',j:1,wrap:'1'),l:'5',n:'0',o:'Output+of+x86-64+gcc+12.3+(Compiler+%231)',t:'0')),k:33.33333333333333,l:'4',n:'0',o:'',s:0,t:'0')),l:'2',n:'0',o:'',t:'0')),version:4). Triggers with g++ 12 and -O2 or -O3, but not -Os.

As far as I can tell, the compiler appears to be triggering on a code path in __shared_count(_Ptr __p, _Deleter __d, _Alloc __a) where std::bad_alloc is thrown while allocating the ref. counter. And then only when the element type has a non-trivial destructor. This code does not appear to change between G++ 12 and 13.

#include <memory>

template<typename E>
struct sa_default_delete {
    void operator()(E* e) const { delete[] e; }
};

int x(int z) {
    typedef std::unique_ptr<int> T;
    std::shared_ptr<T> y(new T[2], sa_default_delete<T>());
    y.get()[0] = T(new int(z));
    return *y.get()[0];
}

The trigger seems to be a non-trivial destructor looking at *this.

mdavidsaver commented 11 months ago

Omitting -Wall suppresses the error. Then g++ 12 and 13 generate identical code. So, this I am going to conclude that this is a bug in the warning logic, not code generation. Now to find a workaround...

mdavidsaver commented 11 months ago

I find some references to GCC 12 specific errors involving -Wuse-after-free. Although none exactly match this situation.

mdavidsaver commented 11 months ago

Apparently, only the case in testshared w/ std::unique_ptr triggers this issue. The usage with Value (built around std::shared_ptr) does not. 88f09659e46497c353832aaa77f87dfb491702c2 changes that case to a simpler non-copyable element type, which was my original intent.

mdavidsaver commented 11 months ago

Another (maybe) similar situation

        size_t len = regerror(err, &ex, nullptr, 0u);
        std::vector<char> msg(len+1);
        (void)regerror(err, &ex, msg.data(), len);

Here g++ 12 complains that msg.data() may be null while len is not. However, I don't believe this is possible as msg.size() will always be at least 1. In this case, I could workaround by inserting if(!msg.data()) { __builtin_unreachable(); }. However, this code is normally only used with G++ 4.9 and 4.10 when std::regexp was defined, but non-functional.