[Lldb-commits] [PATCH] D87172: Check if debug line sequences are starting after the first code segment

[António Afonso via Phabricator via lldb-commits]

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 &section_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