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

Eugene Leviant via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 11 02:38:46 PDT 2019


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


================
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);    
----------------
jhenderson wrote:
> 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);`
> Is this assert correct? What happens if you have a very large object with many sections, but no symbols

This function will never be called then, because `Index` is actually index of a section where symbol is defined.

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

Makes sense


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

https://reviews.llvm.org/D60555





More information about the llvm-commits mailing list