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

[Michael Spencer via llvm-commits]

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.

>
>>    Elf_Dyn_Range dynamic_table() const {
>> -    ErrorOr<Elf_Dyn_Range> Ret = Obj->dynamic_table(DynamicProgHeader);
>> -    error(Ret.getError());
>> -    return *Ret;
>> +    return DynamicTable.getAsRange<Elf_Dyn>();
>> +  }
>
> This and other uses of DynamicTable are a refactoring. They should
> have been committed independently of other changes.

I was already at 5 patches and we've had specific requests not to
break commits up into a bunch of tiny pieces, but to split them where
it makes sense. This is a single small patch that unifies the way we
handle data that is represented by both the section table and program
headers and deals with the fallout from that.

>
> 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.

>
> Cheers,
> Rafael

- Michael Spencer