Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5afddb06 authored by Christopher Ferris's avatar Christopher Ferris
Browse files

Remove Memory::ReadField. 

In almost all cases, it is faster to read the entire structure rather
than do multiple reads using ReadField. The only case where it would be
slower is if doing a remote unwind and ptrace is the only way to read. In
all other cases, it's a single system call. In the ptrace call, it will be
multiple calls. Given that it is unusual to be forced to use ptrace,
it's better to avoid it.

It also reduces the code complexity to do a single read, and avoids
issues where the code forgets to read the field it needs.

Test: Unit tests pass on host and target.
Change-Id: I7b3875b2c85d0d88115b1776e1be28521dc0b932
parent 32960e54

libunwindstack/ElfInterface.cpp

+7 −64
Original line number Diff line number Diff line
@@ -204,49 +204,19 @@ bool ElfInterface::ReadProgramHeaders(const EhdrType& ehdr, uint64_t* load_bias) 
  uint64_t offset = ehdr.e_phoff;

  for (size_t i = 0; i < ehdr.e_phnum; i++, offset += ehdr.e_phentsize) {

    PhdrType phdr;

    if (!memory_->ReadField(offset, &phdr, &phdr.p_type, sizeof(phdr.p_type))) {

    if (!memory_->ReadFully(offset, &phdr, sizeof(phdr))) {

      last_error_.code = ERROR_MEMORY_INVALID;

      last_error_.address =

          offset + reinterpret_cast<uintptr_t>(&phdr.p_type) - reinterpret_cast<uintptr_t>(&phdr);

      last_error_.address = offset;

      return false;

    }



    if (HandleType(offset, phdr.p_type)) {

      continue;

    }



    switch (phdr.p_type) {

    case PT_LOAD:

    {

      // Get the flags first, if this isn't an executable header, ignore it.

      if (!memory_->ReadField(offset, &phdr, &phdr.p_flags, sizeof(phdr.p_flags))) {

        last_error_.code = ERROR_MEMORY_INVALID;

        last_error_.address = offset + reinterpret_cast<uintptr_t>(&phdr.p_flags) -

                              reinterpret_cast<uintptr_t>(&phdr);

        return false;

      }

      if ((phdr.p_flags & PF_X) == 0) {

        continue;

      }



      if (!memory_->ReadField(offset, &phdr, &phdr.p_vaddr, sizeof(phdr.p_vaddr))) {

        last_error_.code = ERROR_MEMORY_INVALID;

        last_error_.address = offset + reinterpret_cast<uintptr_t>(&phdr.p_vaddr) -

                              reinterpret_cast<uintptr_t>(&phdr);

        return false;

      }

      if (!memory_->ReadField(offset, &phdr, &phdr.p_offset, sizeof(phdr.p_offset))) {

        last_error_.code = ERROR_MEMORY_INVALID;

        last_error_.address = offset + reinterpret_cast<uintptr_t>(&phdr.p_offset) -

                              reinterpret_cast<uintptr_t>(&phdr);

        return false;

      }

      if (!memory_->ReadField(offset, &phdr, &phdr.p_memsz, sizeof(phdr.p_memsz))) {

        last_error_.code = ERROR_MEMORY_INVALID;

        last_error_.address = offset + reinterpret_cast<uintptr_t>(&phdr.p_memsz) -

                              reinterpret_cast<uintptr_t>(&phdr);

        return false;

      }

      pt_loads_[phdr.p_offset] = LoadInfo{phdr.p_offset, phdr.p_vaddr,

                                          static_cast<size_t>(phdr.p_memsz)};

      if (phdr.p_offset == 0) {
@@ -256,46 +226,20 @@ bool ElfInterface::ReadProgramHeaders(const EhdrType& ehdr, uint64_t* load_bias) 
    }



    case PT_GNU_EH_FRAME:

      if (!memory_->ReadField(offset, &phdr, &phdr.p_offset, sizeof(phdr.p_offset))) {

        last_error_.code = ERROR_MEMORY_INVALID;

        last_error_.address = offset + reinterpret_cast<uintptr_t>(&phdr.p_offset) -

                              reinterpret_cast<uintptr_t>(&phdr);

        return false;

      }

      // This is really the pointer to the .eh_frame_hdr section.

      eh_frame_hdr_offset_ = phdr.p_offset;

      if (!memory_->ReadField(offset, &phdr, &phdr.p_memsz, sizeof(phdr.p_memsz))) {

        last_error_.code = ERROR_MEMORY_INVALID;

        last_error_.address = offset + reinterpret_cast<uintptr_t>(&phdr.p_memsz) -

                              reinterpret_cast<uintptr_t>(&phdr);

        return false;

      }

      eh_frame_hdr_size_ = phdr.p_memsz;

      break;



    case PT_DYNAMIC:

      if (!memory_->ReadField(offset, &phdr, &phdr.p_offset, sizeof(phdr.p_offset))) {

        last_error_.code = ERROR_MEMORY_INVALID;

        last_error_.address = offset + reinterpret_cast<uintptr_t>(&phdr.p_offset) -

                              reinterpret_cast<uintptr_t>(&phdr);

        return false;

      }

      dynamic_offset_ = phdr.p_offset;

      if (!memory_->ReadField(offset, &phdr, &phdr.p_vaddr, sizeof(phdr.p_vaddr))) {

        last_error_.code = ERROR_MEMORY_INVALID;

        last_error_.address = offset + reinterpret_cast<uintptr_t>(&phdr.p_vaddr) -

                              reinterpret_cast<uintptr_t>(&phdr);

        return false;

      }

      dynamic_vaddr_ = phdr.p_vaddr;

      if (!memory_->ReadField(offset, &phdr, &phdr.p_memsz, sizeof(phdr.p_memsz))) {

        last_error_.code = ERROR_MEMORY_INVALID;

        last_error_.address = offset + reinterpret_cast<uintptr_t>(&phdr.p_memsz) -

                              reinterpret_cast<uintptr_t>(&phdr);

        return false;

      }

      dynamic_size_ = phdr.p_memsz;

      break;



    default:

      HandleUnknownType(phdr.p_type, phdr.p_offset, phdr.p_filesz);

      break;

    }

  }

  return true;
@@ -313,8 +257,7 @@ bool ElfInterface::ReadSectionHeaders(const EhdrType& ehdr) { 
  ShdrType shdr;

  if (ehdr.e_shstrndx < ehdr.e_shnum) {

    uint64_t sh_offset = offset + ehdr.e_shstrndx * ehdr.e_shentsize;

    if (memory_->ReadField(sh_offset, &shdr, &shdr.sh_offset, sizeof(shdr.sh_offset)) &&

        memory_->ReadField(sh_offset, &shdr, &shdr.sh_size, sizeof(shdr.sh_size))) {

    if (memory_->ReadFully(sh_offset, &shdr, sizeof(shdr))) {

      sec_offset = shdr.sh_offset;

      sec_size = shdr.sh_size;

    }

libunwindstack/ElfInterfaceArm.cpp

+4 −10
Original line number Diff line number Diff line
@@ -87,23 +87,17 @@ bool ElfInterfaceArm::GetPrel31Addr(uint32_t offset, uint32_t* addr) { 
#define PT_ARM_EXIDX 0x70000001

#endif



bool ElfInterfaceArm::HandleType(uint64_t offset, uint32_t type) {

void ElfInterfaceArm::HandleUnknownType(uint32_t type, uint64_t ph_offset, uint64_t ph_filesz) {

  if (type != PT_ARM_EXIDX) {

    return false;

  }



  Elf32_Phdr phdr;

  if (!memory_->ReadFully(offset, &phdr, sizeof(phdr))) {

    return true;

    return;

  }



  // The offset already takes into account the load bias.

  start_offset_ = phdr.p_offset;

  start_offset_ = ph_offset;



  // Always use filesz instead of memsz. In most cases they are the same,

  // but some shared libraries wind up setting one correctly and not the other.

  total_entries_ = phdr.p_filesz / 8;

  return true;

  total_entries_ = ph_filesz / 8;

}



bool ElfInterfaceArm::Step(uint64_t pc, Regs* regs, Memory* process_memory, bool* finished) {

libunwindstack/ElfInterfaceArm.h

+1 −1
Original line number Diff line number Diff line
@@ -70,7 +70,7 @@ class ElfInterfaceArm : public ElfInterface32 { 


  bool FindEntry(uint32_t pc, uint64_t* entry_offset);



  bool HandleType(uint64_t offset, uint32_t type) override;

  void HandleUnknownType(uint32_t type, uint64_t ph_offset, uint64_t ph_filesz) override;



  bool Step(uint64_t pc, Regs* regs, Memory* process_memory, bool* finished) override;

libunwindstack/include/unwindstack/ElfInterface.h

+1 −1
Original line number Diff line number Diff line
@@ -118,7 +118,7 @@ class ElfInterface { 
  template <typename SymType>

  bool GetGlobalVariableWithTemplate(const std::string& name, uint64_t* memory_address);



  virtual bool HandleType(uint64_t, uint32_t) { return false; }

  virtual void HandleUnknownType(uint32_t, uint64_t, uint64_t) {}



  template <typename EhdrType>

  static void GetMaxSizeWithTemplate(Memory* memory, uint64_t* size);

libunwindstack/include/unwindstack/Memory.h

+0 −12
Original line number Diff line number Diff line
@@ -41,18 +41,6 @@ class Memory { 


  bool ReadFully(uint64_t addr, void* dst, size_t size);



  inline bool ReadField(uint64_t addr, void* start, void* field, size_t size) {

    if (reinterpret_cast<uintptr_t>(field) < reinterpret_cast<uintptr_t>(start)) {

      return false;

    }

    uint64_t offset = reinterpret_cast<uintptr_t>(field) - reinterpret_cast<uintptr_t>(start);

    if (__builtin_add_overflow(addr, offset, &offset)) {

      return false;

    }

    // The read will check if offset + size overflows.

    return ReadFully(offset, field, size);

  }



  inline bool Read32(uint64_t addr, uint32_t* dst) {

    return ReadFully(addr, dst, sizeof(uint32_t));

  }