[llvm] r239075 - [Object, MachO] Cache parsed MachO header in MachOObjectFile. NFC.

Alexey Samsonov vonosmas at gmail.com
Thu Jun 4 15:54:16 PDT 2015


On Thu, Jun 4, 2015 at 3:11 PM, Alexey Samsonov <vonosmas at gmail.com> wrote:

>
>
> On Thu, Jun 4, 2015 at 2:58 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>>
>>
>> On Thu, Jun 4, 2015 at 2:37 PM, Rafael EspĂ­ndola <
>> rafael.espindola at gmail.com> wrote:
>>
>>> Is asan OK with this? I would assume that a union of
>>> MachO::mach_header_64 and MachO::mach_header is necessary.
>>>
>>
> Sanitizers are fine, as we don't do out-of-bounds access or uninitialized
> value read. Although I agree that using a union would probably be better...
>

Changed the code to use union in r239110.


>
>
>>
>> Perhaps it'd be better to just make mach_header_64 defined:
>>
>> struct mach_header_64 {
>>   mach_header m;
>>   uint32_t reserved;
>> };
>>
>
> Then we won't be able to access some mach_header_64 fields directly.
>
>
>> or somesuch - then there's no weird type punning, perhaps? (though maybe
>> still invalid if we're trying to point to a 32 bit header from a
>> mach_header_64* ?)
>>
>>
>>>
>>> Cheers,
>>> Rafael
>>>
>>>
>>> On 4 June 2015 at 15:22, Alexey Samsonov <vonosmas at gmail.com> wrote:
>>> > Author: samsonov
>>> > Date: Thu Jun  4 14:22:03 2015
>>> > New Revision: 239075
>>> >
>>> > URL: http://llvm.org/viewvc/llvm-project?rev=239075&view=rev
>>> > Log:
>>> > [Object, MachO] Cache parsed MachO header in MachOObjectFile. NFC.
>>> >
>>> > Summary:
>>> > Avoid parsing object file each time MachOObjectFile::getHeader() is
>>> > called. Instead, cache the header in MachOObjectFile constructor, where
>>> > it's parsed anyway. In future, we must avoid constructing the object
>>> > at all if the header can't be parsed.
>>> >
>>> > Test Plan: regression test suite.
>>> >
>>> > Reviewers: rafael
>>> >
>>> > Subscribers: llvm-commits
>>> >
>>> > Modified:
>>> >     llvm/trunk/include/llvm/Object/MachO.h
>>> >     llvm/trunk/lib/Object/MachOObjectFile.cpp
>>> >
>>> > Modified: llvm/trunk/include/llvm/Object/MachO.h
>>> > URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/MachO.h?rev=239075&r1=239074&r2=239075&view=diff
>>> >
>>> ==============================================================================
>>> > --- llvm/trunk/include/llvm/Object/MachO.h (original)
>>> > +++ llvm/trunk/include/llvm/Object/MachO.h Thu Jun  4 14:22:03 2015
>>> > @@ -385,8 +385,8 @@ public:
>>> >
>>> >    MachO::any_relocation_info getRelocation(DataRefImpl Rel) const;
>>> >    MachO::data_in_code_entry getDice(DataRefImpl Rel) const;
>>> > -  MachO::mach_header getHeader() const;
>>> > -  MachO::mach_header_64 getHeader64() const;
>>> > +  const MachO::mach_header &getHeader() const;
>>> > +  const MachO::mach_header_64 &getHeader64() const;
>>> >    uint32_t
>>> >    getIndirectSymbolTableEntry(const MachO::dysymtab_command &DLC,
>>> >                                unsigned Index) const;
>>> > @@ -433,6 +433,7 @@ private:
>>> >    LoadCommandInfo getFirstLoadCommandInfo() const;
>>> >    LoadCommandInfo getNextLoadCommandInfo(const LoadCommandInfo &L)
>>> const;
>>> >
>>> > +  MachO::mach_header_64 Header64;
>>> >    typedef SmallVector<const char*, 1> SectionList;
>>> >    SectionList Sections;
>>> >    typedef SmallVector<const char*, 1> LibraryList;
>>> >
>>> > Modified: llvm/trunk/lib/Object/MachOObjectFile.cpp
>>> > URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/MachOObjectFile.cpp?rev=239075&r1=239074&r2=239075&view=diff
>>> >
>>> ==============================================================================
>>> > --- llvm/trunk/lib/Object/MachOObjectFile.cpp (original)
>>> > +++ llvm/trunk/lib/Object/MachOObjectFile.cpp Thu Jun  4 14:22:03 2015
>>> > @@ -187,7 +187,16 @@ MachOObjectFile::MachOObjectFile(MemoryB
>>> >        DataInCodeLoadCmd(nullptr), LinkOptHintsLoadCmd(nullptr),
>>> >        DyldInfoLoadCmd(nullptr), UuidLoadCmd(nullptr),
>>> >        HasPageZeroSegment(false) {
>>> > -  uint32_t LoadCommandCount = this->getHeader().ncmds;
>>> > +  // Parse header.
>>> > +  if (is64Bit())
>>> > +    Header64 = getStruct<MachO::mach_header_64>(this, getPtr(this,
>>> 0));
>>> > +  else
>>> > +    // First fields of MachO::mach_header_64 are the same as
>>> > +    // in MachO::mach_header.
>>> > +    *reinterpret_cast<MachO::mach_header *>(&this->Header64) =
>>> > +        getStruct<MachO::mach_header>(this, getPtr(this, 0));
>>> > +
>>> > +  uint32_t LoadCommandCount = getHeader().ncmds;
>>> >    if (LoadCommandCount == 0)
>>> >      return;
>>> >
>>> > @@ -1195,21 +1204,9 @@ unsigned MachOObjectFile::getArch() cons
>>> >
>>> >  Triple MachOObjectFile::getArch(const char **McpuDefault,
>>> >                                  Triple *ThumbTriple) const {
>>> > -  Triple T;
>>> > -  if (is64Bit()) {
>>> > -    MachO::mach_header_64 H_64;
>>> > -    H_64 = getHeader64();
>>> > -    T = MachOObjectFile::getArch(H_64.cputype, H_64.cpusubtype,
>>> McpuDefault);
>>> > -    *ThumbTriple = MachOObjectFile::getThumbArch(H_64.cputype,
>>> H_64.cpusubtype,
>>> > -                                                 McpuDefault);
>>> > -  } else {
>>> > -    MachO::mach_header H;
>>> > -    H = getHeader();
>>> > -    T = MachOObjectFile::getArch(H.cputype, H.cpusubtype,
>>> McpuDefault);
>>> > -    *ThumbTriple = MachOObjectFile::getThumbArch(H.cputype,
>>> H.cpusubtype,
>>> > -                                                 McpuDefault);
>>> > -  }
>>> > -  return T;
>>> > +  const auto &Header = getHeader();
>>> > +  *ThumbTriple = getThumbArch(Header.cputype, Header.cpusubtype,
>>> McpuDefault);
>>> > +  return getArch(Header.cputype, Header.cpusubtype, McpuDefault);
>>> >  }
>>> >
>>> >  relocation_iterator MachOObjectFile::section_rel_begin(unsigned
>>> Index) const {
>>> > @@ -2164,12 +2161,15 @@ MachOObjectFile::getDice(DataRefImpl Rel
>>> >    return getStruct<MachO::data_in_code_entry>(this, P);
>>> >  }
>>> >
>>> > -MachO::mach_header MachOObjectFile::getHeader() const {
>>> > -  return getStruct<MachO::mach_header>(this, getPtr(this, 0));
>>> > +const MachO::mach_header &MachOObjectFile::getHeader() const {
>>> > +  // First fields of MachO::mach_header_64 are the same as
>>> > +  // in MachO::mach_header.
>>> > +  return *reinterpret_cast<const MachO::mach_header
>>> *>(&this->Header64);
>>> >  }
>>> >
>>> > -MachO::mach_header_64 MachOObjectFile::getHeader64() const {
>>> > -  return getStruct<MachO::mach_header_64>(this, getPtr(this, 0));
>>> > +const MachO::mach_header_64 &MachOObjectFile::getHeader64() const {
>>> > +  assert(is64Bit());
>>> > +  return Header64;
>>> >  }
>>> >
>>> >  uint32_t MachOObjectFile::getIndirectSymbolTableEntry(
>>> >
>>> >
>>> > _______________________________________________
>>> > llvm-commits mailing list
>>> > llvm-commits at cs.uiuc.edu
>>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>
>>
>
>
> --
> Alexey Samsonov
> vonosmas at gmail.com
>



-- 
Alexey Samsonov
vonosmas at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150604/3e694335/attachment.html>


More information about the llvm-commits mailing list