[PATCH] D90214: [llvm-readobj/elf] - Fix a crash when dumping a dynamic relocation that refer to a symbol past the EOF.

[George Rimar via Phabricator via llvm-commits]

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


================
Comment at: llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test:380
+## This symbol is located right before the EOF, so we can dump it.
+      - Symbol: 0x14
+        Type:   R_X86_64_NONE
----------------
jhenderson wrote:
> I'm a little confused - why isn't this symbol something like 0x1 or 0x2? If the .dynsym is the very last thing in the output, then the at-EOF index should be the number of dynamic symbols.
I've investigated it and found multiple reasons:

1) There is a `.dynstr` section, which we also write after.
2) Also we write the content of `.strtab` and `.shstrtab` implicit sections (both are empty and has size == 1).
    Here I think that the fact we still write the content of `.shstrtab`  with `NoHeaders: true` is a bug.
    We don't add section names to it already, but we should just drop it from the output.
3) We still:
    a) Align the start of the section header table, what affects the output size. (BUG)
    b) We still write it. (BUG)

I am going to fix it. Though it will be a separate independent set of changes.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test:384-385
+    Relocations:
+## A valid symbol right before EOF.
+      - Symbol: 0x1D
+        Type:   R_X86_64_NONE
----------------
jhenderson wrote:
> These values are presumably tied to the size of data in other implicit sections, right? Could we arrange for those sections to appear before .dynsym, as well as the .rela.dyn section? That way, the .dynsym will actually appear at the end of the file, and it would be easier to see that the symbol indexes are what you say they are.
> 
> It's probably also worth expanding on what is meant by "valid" here, since the .dynsym appears to be essentially empty (and therefore any indexes into it could be viewed as invalid).
Done.


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

https://reviews.llvm.org/D90214