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

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D82935#2125472 <https://reviews.llvm.org/D82935#2125472>, @jubnzv wrote:

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


Actually, does the `createSymbolTable` function even need to be a member of `ELFBuilder`? My main objection is the use of the builder a long time after initialization of the Object (along with the complexity of calling it due to the template), but I don't think you actually need it.


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

https://reviews.llvm.org/D82935