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

[Greg Clayton via Phabricator via lldb-commits]

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 &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);
```



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87172/new/

https://reviews.llvm.org/D87172