[PATCH] D25025: [ELF] - Do not crash on invalid symbol index.

[Rafael Espíndola via llvm-commits]

since both tests hit the same check, why do you need 2?

Cheers,
Rafael


On 30 September 2016 at 08:51, George Rimar <grimar at accesssoftek.com> wrote:
> grimar added inline comments.
>
>
>> ruiu wrote in InputFiles.h:154-155
>> In the previous patch, you checked a value in a constructor of a symbol class. Here, you are checking it in InputFile. That seems inconsistent. Probably we should do all error checking in InputFile.cpp?
>
> In a construnctor of DefinedCommon, right. Because st_value contains alignment if symbol is SHT_COMMON and I tried to isolate check.
>
> We know that symbol is common somewhere in ObjectFile<ELFT>::createSymbolBody() first time I think. Do you think it is better to have check there:
>
>   case SHN_COMMON:
>     // Here ?
>     return elf::Symtab<ELFT>::X
>         ->addCommon(Name, Sym->st_size, Sym->st_value, Binding, Sym->st_other,
>                     Sym->getType(), this)
>         ->body();
>
> I think constructor or SymbolTable::addCommon are more isolated and convenient places for checks.
> Do you think we should change https://reviews.llvm.org/D25085 ?
>
> https://reviews.llvm.org/D25025
>
>
>