[PATCH] D69304: [llvm-objcopy] - Do not crash on object that has relocations but no symbol table.

[George Rimar via Phabricator via llvm-commits]

grimar requested review of this revision.
grimar added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/ELF/Object.cpp:908
   for (const Relocation &Reloc : Relocations)
     Reloc.RelocSymbol->Referenced = true;
 }
----------------
grimar wrote:
> jhenderson wrote:
> > This looks dodgy if no symbol exists.
> "relocation references symbol with index..." error is triggered earlier than execution flow might reach here.
> 
> It is reported from `initRelocations` which is called inside `Reader.create()` here:
> https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-objcopy/ELF/ELFObjcopy.cpp#L773
> 
> `markSymbols()` is called from `updateAndRemoveSymbols` which is itself caled in `handleArgs`:
> https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-objcopy/ELF/ELFObjcopy.cpp#L799
> https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-objcopy/ELF/ELFObjcopy.cpp#L613
> https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-objcopy/ELF/ELFObjcopy.cpp#L402
> 
> `handleArgs` is called later than `Reader.create()`. 
> 
> Also, it checks for an empty symbol table at its begining:
> https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-objcopy/ELF/ELFObjcopy.cpp#L350
I've found this concern was valid. To trigger an issue we need an object with a symbol table and a relocation with a symbol index of 0. Then it is possible to reach `markSymbol` for example when `--strip-unneeded` is given.

I've added a test to `no-symbol-relocation.test`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69304/new/

https://reviews.llvm.org/D69304