JDevlieghere added a comment.
In https://reviews.llvm.org/D52543#1246877, @clayborg wrote:
> It is the SymbolVendor's job to do the locking. And in many cases it already
> is. I stopped with inlined comments after a few comments as it would apply to
> this entire patch.
Thanks Greg. Honestly I don't know the code well enough to be sure where we
*need* a lock and where we can get away without. Since it's a recursive mutex I
figured there's no harm in playing it safe. Not everything goes through the
symbol vendor though, for example the backtrace below is the result of a
corruption and it looks like we're calling directly into SymbolFileDWARF.
> 1 com.apple.LLDB.framework 0x02f8dcf5 bool
llvm::DenseMapBase<llvm::DenseMap<void*, DIERef, llvm::DenseMapInfo<void*>,
llvm::detail::DenseMapPair<void*, DIERef> >, void*, DIERef,
llvm::DenseMapInfo<void*>, llvm::detail::DenseMapPair<void*, DIERef>
>::LookupBucketFor<void*>(void* const&, llvm::detail::DenseMapPair<void*,
DIERef> const*&) const + 45
2 com.apple.LLDB.framework 0x02f8e91e
llvm::DenseMapBase<llvm::DenseMap<DWARFDebugInfoEntry const*, clang::Decl*,
llvm::DenseMapInfo<DWARFDebugInfoEntry const*>,
llvm::detail::DenseMapPair<DWARFDebugInfoEntry const*, clang::Decl*> >,
DWARFDebugInfoEntry const*, clang::Decl*,
llvm::DenseMapInfo<DWARFDebugInfoEntry const*>,
llvm::detail::DenseMapPair<DWARFDebugInfoEntry const*, clang::Decl*>
>::FindAndConstruct(DWARFDebugInfoEntry const*&&) + 28
3 com.apple.LLDB.framework 0x02f81f94
DWARFASTParserClang::ParseTypeFromDWARF(lldb_private::SymbolContext const&,
DWARFDIE const&, lldb_private::Log*, bool*) + 6144
4 com.apple.LLDB.framework 0x03150f75
SymbolFileDWARF::ParseType(lldb_private::SymbolContext const&, DWARFDIE const&,
bool*) + 209
5 com.apple.LLDB.framework 0x0314a70e
SymbolFileDWARF::GetTypeForDIE(DWARFDIE const&, bool) + 486
6 com.apple.LLDB.framework 0x03149ee2
SymbolFileDWARF::ResolveType(DWARFDIE const&, bool, bool) + 68
7 com.apple.LLDB.framework 0x02f88728
DWARFASTParserClang::GetClangDeclContextForDIE(DWARFDIE const&) + 176
8 com.apple.LLDB.framework 0x02f8c0ae
DWARFASTParserClang::GetDeclContextForUIDFromDWARF(DWARFDIE const&) + 24
9 com.apple.LLDB.framework 0x031636fe DWARFDIE::GetDeclContext()
const + 60
10 com.apple.LLDB.framework 0x03149dd5
SymbolFileDWARF::GetDeclContextForUID(unsigned long long) + 53
11 com.apple.LLDB.framework 0x0315c6ed
SymbolFileDWARFDebugMap::GetDeclContextForUID(u
> It would be great if we can replace all of the locations where you added lock
> guards with lldbasserts to assert that the lock is already taken. Then we
> might be able to catch places where the locks are not being respected. But in
> general the symbol vendor should take the lock, use N number of SymbolFile
> instances to complete the request, and then release the lock. Makes things
> easier on the lldb_private::SymbolFile subclasses this way.
That sounds like a really good idea, I'll definitely do that.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D52543
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
