[llvm] r250117 - Fixed bugs in llvm-obdump while parsing Mach-O files from malformed archives

[Kevin Enderby via llvm-commits]

Hi Rafael,

That is an excellent suggestion.  But I’m hoping to do a bit better coming up soon.  As this change was a bit of a stop gap solution to prevent a crash. I’ve been pushing a lot of malformed stuff through llvm’s libObject and see this and may other areas for improvement in its error handling.

There are many places that we abort on malformed input or call report_fatal_error().  I would like to see better error checking up front with a bit more detailed messages like which object, archive member, which universal slice, which load command, or symbol table entry is causing the error.  This is similar to what is done with the native darwin tools.

I hope to do this next and work with the team here at Apple to come up with a proposal and some good example code for Mach-O file to present to the LLVM community.

I hope we’ll have something better soon,
Kev

> On Oct 13, 2015, at 10:59 AM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
> 
> Hi Kevin,
> 
> Instead of adding a second function, please change
> ArchiveMemberHeader::getSize() to return ErrorOr<uint32_t>.
> 
> Thanks,
> Rafael
> 
> 
> On 12 October 2015 at 18:04, Kevin Enderby via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
>> Author: enderby
>> Date: Mon Oct 12 17:04:54 2015
>> New Revision: 250117
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=250117&view=rev
>> Log:
>> Fixed bugs in llvm-obdump while parsing Mach-O files from malformed archives
>> that caused aborts.  This was because of the characters of the ‘Size’ field in
>> the archive header did not contain decimal characters.
>> 
>> rdar://22983603
>> 
>> Added:
>>    llvm/trunk/test/tools/llvm-objdump/X86/Inputs/malformed-machos/00000031.a   (with props)
>> Modified:
>>    llvm/trunk/include/llvm/Object/Archive.h
>>    llvm/trunk/lib/Object/Archive.cpp
>>    llvm/trunk/test/tools/llvm-objdump/X86/malformed-machos.test
>> 
>> Modified: llvm/trunk/include/llvm/Object/Archive.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/Archive.h?rev=250117&r1=250116&r2=250117&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Object/Archive.h (original)
>> +++ llvm/trunk/include/llvm/Object/Archive.h Mon Oct 12 17:04:54 2015
>> @@ -38,6 +38,7 @@ struct ArchiveMemberHeader {
>> 
>>   /// Members are not larger than 4GB.
>>   uint32_t getSize() const;
>> +  bool isSizeValid() const;
>> 
>>   sys::fs::perms getAccessMode() const;
>>   sys::TimeValue getLastModified() const;
>> 
>> Modified: llvm/trunk/lib/Object/Archive.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/Archive.cpp?rev=250117&r1=250116&r2=250117&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Object/Archive.cpp (original)
>> +++ llvm/trunk/lib/Object/Archive.cpp Mon Oct 12 17:04:54 2015
>> @@ -50,6 +50,13 @@ uint32_t ArchiveMemberHeader::getSize()
>>   return Ret;
>> }
>> 
>> +bool ArchiveMemberHeader::isSizeValid() const {
>> +  uint32_t Ret;
>> +  if (llvm::StringRef(Size, sizeof(Size)).rtrim(" ").getAsInteger(10, Ret))
>> +    return false;
>> +  return true;
>> +}
>> +
>> sys::fs::perms ArchiveMemberHeader::getAccessMode() const {
>>   unsigned Ret;
>>   if (StringRef(AccessMode, sizeof(AccessMode)).rtrim(" ").getAsInteger(8, Ret))
>> @@ -89,6 +96,11 @@ Archive::Child::Child(const Archive *Par
>> 
>>   uint64_t Size = sizeof(ArchiveMemberHeader);
>>   Data = StringRef(Start, Size);
>> +  // Check to make sure the size is valid.
>> +  const ArchiveMemberHeader *Header =
>> +    reinterpret_cast<const ArchiveMemberHeader *>(Data.data());
>> +  if (!Header->isSizeValid())
>> +    return;
>>   if (!isThinMember()) {
>>     Size += getRawSize();
>>     Data = StringRef(Start, Size);
>> 
>> Added: llvm/trunk/test/tools/llvm-objdump/X86/Inputs/malformed-machos/00000031.a
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/X86/Inputs/malformed-machos/00000031.a?rev=250117&view=auto
>> ==============================================================================
>> Binary file - no diff available.
>> 
>> Propchange: llvm/trunk/test/tools/llvm-objdump/X86/Inputs/malformed-machos/00000031.a
>> ------------------------------------------------------------------------------
>>    svn:mime-type = application/octet-stream
>> 
>> Modified: llvm/trunk/test/tools/llvm-objdump/X86/malformed-machos.test
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/llvm-objdump/X86/malformed-machos.test?rev=250117&r1=250116&r2=250117&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/tools/llvm-objdump/X86/malformed-machos.test (original)
>> +++ llvm/trunk/test/tools/llvm-objdump/X86/malformed-machos.test Mon Oct 12 17:04:54 2015
>> @@ -39,3 +39,9 @@
>> # RUN:   | FileCheck -check-prefix=m0337 %s
>> 
>> # m0337: subq  $16, %rsp
>> +
>> +# RUN: llvm-objdump -macho -disassemble \
>> +# RUN:   %p/Inputs/malformed-machos/00000031.a \
>> +# RUN:   | FileCheck -check-prefix=0031a %s
>> +
>> +# 0031a: Archive
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits