[PATCH] D60555: [llvm-objcopy] Fill .symtab_shndx section correctly

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Test case?



================
Comment at: tools/llvm-objcopy/ELF/Object.cpp:1669
 
+  // layoutSections could have modified section indexes, that why we need
+  // to fill index table after assignOffsets.
----------------
that why -> so


================
Comment at: tools/llvm-objcopy/ELF/Object.cpp:1670
+  // layoutSections could have modified section indexes, that why we need
+  // to fill index table after assignOffsets.
+  if (Obj.SymbolTable != nullptr)
----------------
fill index table -> fill the index table


================
Comment at: tools/llvm-objcopy/ELF/Object.h:484
   void addIndex(uint32_t Index) {
-    Indexes.push_back(Index);
-    Size += 4;
+    assert(Size);
+    Indexes.push_back(Index);    
----------------
Is this assert correct? What happens if you have a very large object with many sections, but no symbols. Will that result in an empty SectionIndexSection?

Assuming it's correct, I would find it more readable for this to be `assert(Size > 0);`


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

https://reviews.llvm.org/D60555