[llvm] r260488 - [readobj] Handle ELF files with no section table or with no program headers.

[Rafael Espíndola via llvm-commits]

On 12 February 2016 at 19:29, Michael Spencer <bigcheesegs at gmail.com> wrote:
> On Fri, Feb 12, 2016 at 2:15 PM, Rafael Espíndola
> <rafael.espindola at gmail.com> wrote:
>> There are a few problems with this patch
>>
>>>    uint32_t Index = Sym->st_shndx;
>>>    if (Index == ELF::SHN_XINDEX)
>>> -    return getSection(getExtendedSymbolTableIndex(Sym, SymTab, ShndxTable));
>>> +    return getSection(
>>> +        getExtendedSymbolTableIndex(Sym, symbol_begin(SymTab), ShndxTable));
>>
>>
>> Why call the more complicated version in here?
>
> I was going to remove the const Elf_Shdr * version as this was the
> only remaining use, but lld uses it so I kept the forwarder. I forgot
> to switch this back to using it.
>
>>
>>>
>>> -RUN: not llvm-readobj -dynamic-table \
>>> -RUN:   %p/Inputs/corrupt-invalid-virtual-addr.elf.x86-64 2>&1 | \
>>> -RUN:   FileCheck --check-prefix=VIRTADDR %s
>>> -
>>> -VIRTADDR: Virtual address is not in any segment
>>
>> Why is this not an error anymore?
>
> Because it was checking the validity of addresses that were never
> accessed. There's no reason to bail out when the user hasn't requested
> us to print invalid data. Specifically the code that emitted this
> error was blocking dynamic-table-exec-no-phdrs.x86 from dumping data
> even though we had full section table information.

The data is still invalid and it should still be reported when found
to be broken. If it was not needed on this test, you should should
have added a new test.


>>
>> Could you revert this and send it back for review? Please split it up too.
>
> Unless there is a glaring design issue with the approach taken in this
> patch I don't believe that it is reasonable to revert. I'm glad to fix
> any issues you've found with the code.


There is. You deleted error checking code without review.

Cheers,
Rafael