[Lldb-commits] [PATCH] D65647: Fix line table resolution near the end of a section

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

I vote to not add any API calls with allow_section_end and just take care of this in source/Symbol/LineTable.cpp. All of my inline comments will reflect this notion.



================
Comment at: include/lldb/Core/Module.h:683-684
 
-  bool ResolveFileAddress(lldb::addr_t vm_addr, Address &so_addr);
+  bool ResolveFileAddress(lldb::addr_t vm_addr, Address &so_addr,
+                          bool allow_section_end = false);
 
----------------
revert


================
Comment at: include/lldb/Core/Section.h:74-80
+  /// Look up the given file address in the contained sections. Return a
+  /// section-relative Address object if a section is found. if
+  /// allow_section_end is true, an address pointing past the end of a section
+  /// is permitted and resolved relative to the section which ends at the given
+  /// address.
+  llvm::Optional<Address> ResolveFileAddress(lldb::addr_t file_addr,
+                                             bool allow_section_end) const;
----------------
revert


================
Comment at: include/lldb/Core/Section.h:133-137
+  /// Return true iff this section contains the given file address. If
+  /// allow_section_end is true, the function returns true also for address
+  /// pointing past the end of the section.
+  bool ContainsFileAddress(lldb::addr_t vm_addr,
+                           bool allow_section_end = false) const;
----------------
revert


================
Comment at: source/Core/Module.cpp:426-440
+bool Module::ResolveFileAddress(lldb::addr_t vm_addr, Address &so_addr,
+                                bool allow_section_end) {
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
   Timer scoped_timer(func_cat,
                      "Module::ResolveFileAddress (vm_addr = 0x%" PRIx64 ")",
                      vm_addr);
----------------
revert


================
Comment at: source/Core/Section.cpp:270-276
+bool Section::ContainsFileAddress(addr_t vm_addr,
+                                  bool allow_section_end) const {
   const addr_t file_addr = GetFileAddress();
   if (file_addr != LLDB_INVALID_ADDRESS && !IsThreadSpecific()) {
     if (file_addr <= vm_addr) {
       const addr_t offset = (vm_addr - file_addr) * m_target_byte_size;
+      return offset < GetByteSize() + (allow_section_end ? 1 : 0);
----------------
revert


================
Comment at: source/Core/Section.cpp:590-603
+llvm::Optional<Address>
+SectionList::ResolveFileAddress(addr_t file_addr,
+                                bool allow_section_end) const {
+  for (const SectionSP &section_sp : m_sections) {
+    if (section_sp->ContainsFileAddress(file_addr, allow_section_end)) {
+      Address addr;
+      section_sp->ResolveContainedAddress(
----------------
revert


================
Comment at: source/Symbol/LineTable.cpp:247-249
+    if (module_sp && module_sp->ResolveFileAddress(
+                         entry.file_addr, line_entry.range.GetBaseAddress(),
+                         /*allow_section_end = */ entry.is_terminal_entry)) {
----------------
Take care of backing up the entry address if this is the last entry so we don't need any of the other API changes:

```
lldb::addr_t file_addr = entry.file_addr - (entry.is_terminal_entry ? 1 : 0);
if (module_sp && module_sp->ResolveFileAddress(file_addr, line_entry.range.GetBaseAddress())) {
  if (entry.is_terminal_entry) 
    line_entry.range.GetBaseAddress().Slide(1);
``` 


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

https://reviews.llvm.org/D65647