clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
See inlined comments for full details.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:506
+ for (uint32_t i = 0; i < num_sections; ++i) {
+ SectionSP section_sp(section_list->GetSectionAtIndex(i));
+ const addr_t section_file_address = section_sp->GetFileAddress();
----------------
labath wrote:
> Shouldn't this be also checking the permissions?
Yes we need to check permissions. But we also have to watch out for sections
that contain sections. For ELF, we have the PT_LOAD[N] sections that contain
sections. On Mac, we have __TEXT that contains sections, but the first section
can be __PAGEZERO (which has no read, no write, no execute).
So the main question becomes: if a section contains subsections, do we ignore
the top level section? I would prefer to see us do this.
So this code should be factored into a lambda or function:
```
bool SymbolFileDWARF::SetFirstCodeAddress(const SectionList §ion_list) {
const uint32_t num_sections = section_list.GetSize();
for (uint32_t i = 0; i < num_sections; ++i) {
SectionSP section_sp(section_list.GetSectionAtIndex(i));
if (sections_sp->GetChildren().GetSize() > 0) {
SetFirstCodeAddress(sections_sp->GetChildren());
continue;
}
if (HasExecutePermissions(section_sp)) {
const addr_t section_file_address = section_sp->GetFileAddress();
if (m_first_code_address > section_file_address)
m_first_code_address = section_file_address;
}
}
}
```
And then this code will just look like:
```
const SectionList *section_list = m_objfile_sp->GetModule()->GetSectionList();
if (section_list)
SetFirstCodeAddress(*section_list);
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87172/new/
https://reviews.llvm.org/D87172
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
