[PATCH] D82935: [llvm-objcopy] Fix crash when removing symbol table at same time as adding a symbol

[Georgy Komarov via Phabricator via llvm-commits]

jubnzv updated this revision to Diff 274787.
jubnzv marked 3 inline comments as done.
jubnzv added a comment.

Thanks for the comment @jhenderson. I updated the test.

>   I think you probably just want to completely remove the logic from ELFBuilder for creating a symbol table. With a bit of care, you then should be able to create the symbol table when you need to rather than early, and at the same time remove the whole EnsureSymtab logic from the ELFBuilder/ELFReader stack.

It is possible to move logic for creating symbol table in a free function or make method in the Object class that performs initialization on demand.
But the problem is that ELFBuilder also sets reference symbols for SH_LINK and SH_INFO fields of relocation sections when creating a new Object. To perform this, it needs an initialized symbol table. All cases that requires symtab are better seen at D69093 <https://reviews.llvm.org/D69093> diff: https://reviews.llvm.org/differential/changeset/?ref=1658725.
So I will also need to separate set relocations logic and apply it only when it needed, after creating an Object. I think it makes code more hard to read.
Or are simpler solutions possible?


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

https://reviews.llvm.org/D82935

Files:
  llvm/test/tools/llvm-objcopy/ELF/add-symbol-new-symtab.test
  llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
  llvm/tools/llvm-objcopy/ELF/Object.cpp
  llvm/tools/llvm-objcopy/ELF/Object.h

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D82935.274787.patch
Type: text/x-patch
Size: 6151 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200701/6b14d1db/attachment-0001.bin>