[PATCH] D82274: [llvm-objcopy] Emit error if removing symbol table referenced by SHT_GROUP section

[Georgy Komarov via Phabricator via llvm-commits]

jubnzv marked an inline comment as done.
jubnzv added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/ELF/Object.cpp:983
     bool AllowBrokenLinks, function_ref<bool(const SectionBase *)> ToRemove) {
+  if (ToRemove(LinkSection)) {
+    if (!AllowBrokenLinks)
----------------
jhenderson wrote:
> Similar to above, use `SymTab`, not `LinkSection` in this method.
> 
> What happens if `LinkSection` (later `SymTab`) is a nullptr? Please double check to see whether `ToRemove` handles nullptr or not. If it doesn't, you'll need to handle it here and will want a test that shows the behaviour in this situation.
`ToRemove` is represented as `find` method of `std::unordered_set`,  so it will returns past-the-end iterator if it can't find the requested key. So `ToRemove` could be used without additional null check.

I've also added additional test for possible regressions.


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

https://reviews.llvm.org/D82274