[lldb-dev] [llvm-dev] 13.0.0-rc1 has been tagged
Jim Ingham via lldb-dev
lldb-dev at lists.llvm.org
Thu Aug 5 09:23:54 PDT 2021
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 <vadimcn at 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 <jingham at 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 at 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 at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
More information about the lldb-dev
mailing list