This revision was automatically updated to reflect the committed changes.
Closed by commit rL356612: Introduce DWARFContext. (authored by zturner,
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D59562?vs=1
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.
The suggestion just avoids the Clear() call at the expense of a branch. No big
deal. Fine either way.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59562/new/
https://reviews.llv
zturner marked an inline comment as done.
zturner added inline comments.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:552
+ data.Clear();
+ m_obj_file->ReadSectionData(section_sp.get(), data);
}
clayborg wrote:
> clayborg wrote:
> > ```
clayborg added inline comments.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:552
+ data.Clear();
+ m_obj_file->ReadSectionData(section_sp.get(), data);
}
clayborg wrote:
> ```
> if (m_obj_file->ReadSectionData(section_sp.get(), data) ==
zturner marked 2 inline comments as done.
zturner added inline comments.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:50
+
+const DWARFDataExtractor *DWARFContext::getOrLoadArangesData() {
+ return LoadOrGetSection(m_module, m_dwarf_data,
eSectionTypeDWARFD
clayborg added inline comments.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:24
+ // forward.
+ extractor.emplace();
+
clayborg wrote:
> Don't we want to put an empty DWARFDataExtractor in here? Maybe this code
> should be:
>
> ```
> cons
clayborg added inline comments.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:24
+ // forward.
+ extractor.emplace();
+
Don't we want to put an empty DWARFDataExtractor in here? Maybe this code
should be:
```
const SectionList *section_lis
zturner updated this revision to Diff 191510.
zturner marked an inline comment as done.
zturner added a comment.
Updated based on suggestions, including removal of the `m_dwarf_data` member
variable which holds the contents of the `__DWARF` segment on MachO.
CHANGES SINCE LAST ACTION
https://
clayborg added inline comments.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:17
+static const DWARFDataExtractor *LoadOrGetSection(
+Module &module, const llvm::Optional
&mapped_dwarf_data,
+SectionType section_type, llvm::Optional &extractor) {
clayborg added a comment.
We can get rid of the m_dwarf_data all together. As Pavel suggested, we used to
not mmap the entire file, and now we do most of the time. The
Section::GetSectionData() will ref count the mmap data in the data extractor
and use the mmap already, so we should only suppor
zturner marked 5 inline comments as done.
zturner added inline comments.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:17
+static const DWARFDataExtractor *LoadOrGetSection(
+Module &module, const llvm::Optional
&mapped_dwarf_data,
+SectionType sectio
labath added inline comments.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h:28
+
+ void SetDwarfData(const DWARFDataExtractor &dwarf);
+
clayborg wrote:
> labath wrote:
> > Don't want to block progress here over this, but I just wanted to say t
clayborg requested changes to this revision.
clayborg added inline comments.
This revision now requires changes to proceed.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.cpp:17
+static const DWARFDataExtractor *LoadOrGetSection(
+Module &module, const llvm::Op
labath added inline comments.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h:28
+
+ void SetDwarfData(const DWARFDataExtractor &dwarf);
+
Don't want to block progress here over this, but I just wanted to say that my
impression was that this Dwa
probinson added a comment.
I remember LLVM's DWARFContext being irritating to deal with when I was doing
the DWARF 5 stuff. The exact details have been swapped out of my memory, but I
suspect it had something to do with needing to know whether I was in DWO mode
or normal mode in order to snag
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.
LGTM!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59562/new/
https://reviews.llvm.org/D59562
___
lldb-commits mailing list
zturner updated this revision to Diff 191421.
zturner added a comment.
I went ahead and just removed the const `get` version entirely, while changing
the other function name to `getOrLoad` as you suggested. I have another patch
while I'll upload after this one that converts all of the rest of t
JDevlieghere added inline comments.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h:30
+
+ const DWARFDataExtractor *maybeLoadArangesData();
+
zturner wrote:
> JDevlieghere wrote:
> > Why do we need these two methods? I presume it's because one d
zturner marked an inline comment as done.
zturner added inline comments.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h:30
+
+ const DWARFDataExtractor *maybeLoadArangesData();
+
JDevlieghere wrote:
> Why do we need these two methods? I presume
JDevlieghere added inline comments.
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFContext.h:30
+
+ const DWARFDataExtractor *maybeLoadArangesData();
+
Why do we need these two methods? I presume it's because one does work and the
other doesn't? If I und
zturner created this revision.
zturner added reviewers: JDevlieghere, aprantl, clayborg.
Herald added subscribers: jdoerfert, mgorny.
LLVM's DWARF parsing library has a class called `DWARFContext` which holds all
of the various DWARF data sections and lots of other information. LLDB's on
the ot
21 matches
Mail list logo
