Once again we see the elegance of C++ never failing to impress...
I wonder if it wouldn't be simpler to make a copy constructor that just makes a
new mutex. The ThreadPlanStack is a vector of ThreadPlan shared pointers, so
it is cheap to copy, and the stack just uses the mutex internally, so making a
new one would be fine. Slightly less efficient, but then we wouldn't have to
deal with this sort of silliness.
But otherwise, if this is necessary, I have opinions but no serious objections
to this change. Feel free to put up a patch.
Jim
> On Aug 4, 2021, at 11:17 PM, Vadim Chugunov <vadi...@gmail.com> wrote:
>
> Not sure what's up with clang's inclusion chain reporting, but I found a
> couple of SO threads [1] [2] regarding the need of the copy constructor:
> apparently, until GCC 6, std::unordered_map could not emplace non-copyable
> values. Following the advice in [2], I was able to overcome the error by
> rewriting ThreadPlanStack::AddThread like this:
> void AddThread(Thread &thread) {
> lldb::tid_t tid = thread.GetID();
> m_plans_list.emplace(std::piecewise_construct,
> std::forward_as_tuple(tid),
> std::forward_as_tuple(thread));
> }
>
> [1]:
> https://stackoverflow.com/questions/44699545/why-does-stdmap-emplace-need-a-copy-constructor-on-gcc
> [2]:
> https://stackoverflow.com/questions/27960325/stdmap-emplace-without-copying-valuseue
>
>
> On Wed, Aug 4, 2021 at 5:10 PM Jim Ingham <jing...@apple.com> wrote:
> That is certainly an odd chain of includes to end up complaining about
> ThreadPlanStacks...
>
> ABIMacOSX.cpp does see the definition of ThreadPlanStacks but not along that
> path... Something got fairly confused.
>
> There IS an implicitly deleted copy constructor for ThreadPlanStacks, but it
> doesn't get called anywhere. This is code that is built pretty much
> everywhere and this is the first time I've seen this error.
>
> In the ThreadPlanStack header file, there are a couple of routines the
> iterate over the map of ThreadPlanStacks like (m_plans_list is:
> std::unordered_map<lldb:tid_t, ThreadPlanStack>):
>
> void ClearThreadCache() {
> for (auto &plan_list : m_plans_list)
> plan_list.second.ClearThreadCache();
> }
>
> But those get "auto &" so they should just be pulling references to
> ThreadPlanStacks out, it shouldn't need to copy them.
>
> And then we do call find on the ThreadPlanStackMap and do something with the
> iterator returned:
>
> bool RemoveTID(lldb::tid_t tid) {
> auto result = m_plans_list.find(tid);
> if (result == m_plans_list.end())
> return false;
> result->second.ThreadDestroyed(nullptr);
> m_plans_list.erase(result);
> return true;
> }
>
> But the iterator of a map shouldn't require a copy of the value, that doesn't
> make sense.
>
> Other than that, I can't see anything funny here.
>
> Is this something triggered by -pedantic?
>
> Jim
>
>
> > On Aug 4, 2021, at 4:22 PM, Vadim Chugunov via lldb-dev
> > <lldb-dev@lists.llvm.org> wrote:
> >
> > Not sure if this is a supported configuration, but I am hitting this error
> > when compiling on Ubuntu 16.04 with clang 12:
> >
> > FAILED: /usr/local/bin/clang++ -DGTEST_HAS_RTTI=0 -DHAVE_ROUND
> > -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS
> > -D__STDC_LIMIT_MACROS -Itools/lldb/source/Plugins/ABI/ARM
> > -I/__w/1/s/llvm-project/lldb/source/Plugins/ABI/ARM -Itools/lldb/source
> > -I/__w/1/s/llvm-project/lldb/include -Itools/lldb/include -Iinclude
> > -I/__w/1/s/llvm-project/llvm/include
> > -I/__w/1/python/install/include/python3.9
> > -I/__w/1/s/llvm-project/llvm/../clang/include -Itools/lldb/../clang/include
> > -I/__w/1/libxml2/install/include/libxml2
> > -I/__w/1/s/llvm-project/lldb/source/. -target x86_64-linux-gnu -fPIC -fPIC
> > -fvisibility-inlines-hidden -Werror=date-time
> > -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter
> > -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic
> > -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough
> > -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor
> > -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment
> > -Wstring-conversion -Wmisleading-indentation -fdiagnostics-color
> > -ffunction-sections -fdata-sections -Wno-deprecated-declarations
> > -Wno-unknown-pragmas -Wno-strict-aliasing -Wno-deprecated-register
> > -Wno-vla-extension -Os -DNDEBUG -fno-exceptions -fno-rtti -std=c++14 -MD
> > -MT
> > tools/lldb/source/Plugins/ABI/ARM/CMakeFiles/lldbPluginABIARM.dir/ABIMacOSX_arm.cpp.o
> > -MF
> > tools/lldb/source/Plugins/ABI/ARM/CMakeFiles/lldbPluginABIARM.dir/ABIMacOSX_arm.cpp.o.d
> > -o
> > tools/lldb/source/Plugins/ABI/ARM/CMakeFiles/lldbPluginABIARM.dir/ABIMacOSX_arm.cpp.o
> > -c /__w/1/s/llvm-project/lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp
> > In file included from
> > /__w/1/s/llvm-project/lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp:9:
> > In file included from
> > /__w/1/s/llvm-project/lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.h:12:
> > In file included from
> > /__w/1/s/llvm-project/lldb/include/lldb/Target/ABI.h:12:
> > In file included from
> > /__w/1/s/llvm-project/lldb/include/lldb/Core/PluginInterface.h:12:
> > In file included from
> > /__w/1/s/llvm-project/lldb/include/lldb/lldb-private.h:15:
> > In file included from
> > /__w/1/s/llvm-project/lldb/include/lldb/lldb-private-enumerations.h:12:
> > In file included from
> > /__w/1/s/llvm-project/llvm/include/llvm/ADT/StringRef.h:12:
> > In file included from
> > /__w/1/s/llvm-project/llvm/include/llvm/ADT/STLExtras.h:19:
> > In file included from
> > /__w/1/s/llvm-project/llvm/include/llvm/ADT/Optional.h:18:
> > In file included from
> > /__w/1/s/llvm-project/llvm/include/llvm/ADT/Hashing.h:48:
> > In file included from
> > /__w/1/s/llvm-project/llvm/include/llvm/Support/ErrorHandling.h:18:
> > In file included from
> > /usr/lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/string:40:
> > In file included from
> > /usr/lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/char_traits.h:39:
> > In file included from
> > /usr/lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/stl_algobase.h:64:
> > /usr/lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/stl_pair.h:134:35:
> > error: call to implicitly-deleted copy constructor of
> > 'lldb_private::ThreadPlanStack'
> > : first(std::forward<_U1>(__x)), second(__y) { }
> > ^ ~~~
> > /usr/lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/ext/new_allocator.h:120:23:
> > note: in instantiation of function template specialization
> > 'std::pair<const unsigned long,
> > lldb_private::ThreadPlanStack>::pair<unsigned long &, void>' requested here
> > { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); }
> > ^
> > /usr/lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/alloc_traits.h:530:8:
> > note: in instantiation of function template specialization
> > '__gnu_cxx::new_allocator<std::pair<const unsigned long,
> > lldb_private::ThreadPlanStack>>::construct<std::pair<const unsigned long,
> > lldb_private::ThreadPlanStack>, unsigned long &, lldb_private::Thread &>'
> > requested here
> > { __a.construct(__p, std::forward<_Args>(__args)...); }
> > ^
> > /usr/lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/hashtable_policy.h:1955:28:
> > note: in instantiation of function template specialization
> > 'std::allocator_traits<std::allocator<std::pair<const unsigned long,
> > lldb_private::ThreadPlanStack>>>::construct<std::pair<const unsigned long,
> > lldb_private::ThreadPlanStack>, unsigned long &, lldb_private::Thread &>'
> > requested here
> > __value_alloc_traits::construct(__a, __n->_M_valptr(),
> > ^
> > /usr/lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/hashtable.h:1526:30:
> > note: in instantiation of function template specialization
> > 'std::__detail::_Hashtable_alloc<std::allocator<std::__detail::_Hash_node<std::pair<const
> > unsigned long, lldb_private::ThreadPlanStack>,
> > false>>>::_M_allocate_node<unsigned long &, lldb_private::Thread &>'
> > requested here
> > __node_type* __node =
> > this->_M_allocate_node(std::forward<_Args>(__args)...);
> > ^
> > /usr/lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/hashtable.h:726:11:
> > note: in instantiation of function template specialization
> > 'std::_Hashtable<unsigned long, std::pair<const unsigned long,
> > lldb_private::ThreadPlanStack>, std::allocator<std::pair<const unsigned
> > long, lldb_private::ThreadPlanStack>>, std::__detail::_Select1st,
> > std::equal_to<unsigned long>, std::hash<unsigned long>,
> > std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash,
> > std::__detail::_Prime_rehash_policy,
> > std::__detail::_Hashtable_traits<false, false, true>>::_M_emplace<unsigned
> > long &, lldb_private::Thread &>' requested here
> > { return _M_emplace(__unique_keys(),
> > std::forward<_Args>(__args)...); }
> > ^
> > /usr/lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/unordered_map.h:380:16:
> > note: in instantiation of function template specialization
> > 'std::_Hashtable<unsigned long, std::pair<const unsigned long,
> > lldb_private::ThreadPlanStack>, std::allocator<std::pair<const unsigned
> > long, lldb_private::ThreadPlanStack>>, std::__detail::_Select1st,
> > std::equal_to<unsigned long>, std::hash<unsigned long>,
> > std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash,
> > std::__detail::_Prime_rehash_policy,
> > std::__detail::_Hashtable_traits<false, false, true>>::emplace<unsigned
> > long &, lldb_private::Thread &>' requested here
> > { return _M_h.emplace(std::forward<_Args>(__args)...); }
> > ^
> > /__w/1/s/llvm-project/lldb/include/lldb/Target/ThreadPlanStack.h:127:18:
> > note: in instantiation of function template specialization
> > 'std::unordered_map<unsigned long, lldb_private::ThreadPlanStack,
> > std::hash<unsigned long>, std::equal_to<unsigned long>,
> > std::allocator<std::pair<const unsigned long,
> > lldb_private::ThreadPlanStack>>>::emplace<unsigned long &,
> > lldb_private::Thread &>' requested here
> > m_plans_list.emplace(tid, thread);
> > ^
> > /__w/1/s/llvm-project/lldb/include/lldb/Target/ThreadPlanStack.h:113:32:
> > note: copy constructor of 'ThreadPlanStack' is implicitly deleted because
> > field 'm_stack_mutex' has a deleted copy constructor
> > mutable std::recursive_mutex m_stack_mutex;
> > ^
> > /usr/lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/mutex:170:5:
> > note: 'recursive_mutex' has been explicitly marked deleted here
> > recursive_mutex(const recursive_mutex&) = delete;
> > ^
> > 1 error generated.
> >
> > _______________________________________________
> > lldb-dev mailing list
> > lldb-dev@lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
_______________________________________________
lldb-dev mailing list
lldb-dev@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
