aadsm added inline comments.
================
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();
----------------
clayborg wrote:
> 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);
> ```
>
Sorry about this. I completely changed the original logic with my last update.
I was checking the section type before and also going down the section
hierarchy but not anymore.
We should go through all sub sections just like I did in my original code, I
was also checking if it was a code section, rather than permissions, it's a bit
more abstract that way.
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
