Page MenuHomePhabricator

DWZ 07/11: Protect DWARFDebugInfo::m_compile_units by a new mutex
AbandonedPublic

Authored by jankratochvil on Nov 26 2017, 5:14 AM.

Details

Reviewers
clayborg
labath
Summary

DW_TAG_partial_unit is remapped on each of its use, those remapped virtual units are added into DWARFDebugInfo::m_compile_units during multithreaded indexing.

I am not sure what do you prefer about m_compile_units_size_atomic. Overhead of the new mutex will not be measurable in practice I think:
(1) Drop any such atomic_t for its size and always use the mutex.
(2) Use atomic_t to prevent mutex locking in most common cases. But maintain it separately which is error prone during newly coded m_compile_units updates. --- attached patch implements this
(3) Provide m_compile_units class wrapper which transparently updates also its atomic_t.

All DWZ patches are also applied in: git clone -b dwz git://git.jankratochvil.net/lldb

Diff Detail

Event Timeline

jankratochvil created this revision.Nov 26 2017, 5:14 AM
clayborg requested changes to this revision.Nov 27 2017, 10:52 AM

DWARFDebugInfo::ParseCompileUnitHeadersIfNeeded() seems broken, see inlined comments.

source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
114–119

So if some code calls DWARFDebugInfo::ParseCompileUnitHeadersIfNeeded() after this loop has added 1 compile unit they will return and be able to proceed? This doesn't make sense.

source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
80

Why is this named m_dwz_uniq_mutex? Seems to be protected m_compile_units. Should this be named "m_compile_units_mutex"?

This revision now requires changes to proceed.Nov 27 2017, 10:52 AM
jankratochvil retitled this revision from DWZ 09/12: Protect DWARFDebugInfo::m_compile_units by a new mutex to DWZ 07/11: Protect DWARFDebugInfo::m_compile_units by a new mutex.
jankratochvil edited the summary of this revision. (Show Details)
jankratochvil planned changes to this revision.Apr 14 2018, 4:22 AM
jankratochvil abandoned this revision.Apr 22 2018, 11:08 AM

Without FileOffset and the remapping to unique DIE offsets for each DW_TAG_partial_unit inclusion by DW_TAG_imported_unit this patch is no longer needed, as discussed in: https://lists.llvm.org/pipermail/lldb-commits/Week-of-Mon-20180409/040324.html