This is an archive of the discontinued LLVM Phabricator instance.

[lldb/DWARF] Change how we construct a llvm::DWARFContext
ClosedPublic

Authored by labath on Jan 17 2020, 5:29 AM.

Details

Summary

The goal of this patch is two-fold. First, it fixes a use-after-free in
the construction of the llvm DWARFContext. This happened because the
construction code was throwing away the lldb DataExtractors it got while
reading the sections (unlike their llvm counterparts, these are also
responsible for memory ownership). In most cases this did not matter,
because the sections are just slices of the mmapped file data. But this
isn't the case for compressed elf sections, in which case the section is
decompressed into a heap buffer. A similar thing also happen with object
files which are loaded from process memory.

The second goal is to make it explicit which sections go into the llvm
DWARFContext -- any access to the sections through both DWARF parsers
carries a risk of parsing things twice, so it's better if this is a
conscious decision. Also, this avoids loading completely irrelevant
sections (e.g. .text). At present, the only section that needs to be
present in the llvm DWARFContext is the debug_line_str. Using it through
both APIs is not a problem, as there is no parsing involved.

The first goal is achieved by loading the sections through the existing
lldb DWARFContext APIs, which already do the caching. The second by
explicitly enumerating the sections we wish to load.

Diff Detail

Event Timeline

labath created this revision.Jan 17 2020, 5:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 17 2020, 5:29 AM
aprantl added inline comments.Jan 17 2020, 11:21 AM
lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
130

This will also work on Darwin where the section names are slightly different (shorter)?

JDevlieghere accepted this revision.Jan 17 2020, 2:58 PM
This revision is now accepted and ready to land.Jan 17 2020, 2:58 PM
labath marked an inline comment as done.Jan 20 2020, 2:45 AM
labath added inline comments.
lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp
130

This is correct, because this constructor of llvm::DWARFContext uses the generic DWARF section names (cf. DWARFObjInMemory::mapSectionToMember). In fact, this was not correct before because that function would not recognize macho-style __debug_line_str (it did work for for elf because we manually stripped the leading .).

However, this still won't be enough to make macho work because ObjectFileMachO is missing the code to recognise debug_line_str in ObjectFileMachO.cpp:GetSectionType (at least, that's the first problem I ran into).

This revision was automatically updated to reflect the committed changes.