[PATCH] D64788: [llvm-readelf] - A fix for: "--hash-symbols asserts for 64-bit ELFs"

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: test/tools/llvm-readobj/elf-hash-symbols.test:120
+      - Tag:   DT_HASH
+## 0x2b8 + PT_LOAD's p_offset == offset of .hash
+        Value: 0x00000000000002b8
----------------
grimar wrote:
> jhenderson wrote:
> > Why is this 0x2be, rather than 0x0 like in the 32-bit case?
> > 
> > Worth a comment saying what "0x2b8" (and "0x2e4" below) represent.
> In this case I set `Offset` to `0` for `PT_LOAD`. And so `0x2b8` and `0x2e4` are the offsets of the hash sections:
> 
> > Section Headers:
> >   [Nr] Name              Type             Address           Offset
> >        Size              EntSize          Flags  Link  Info  Align
> > ...
> >   [ 1] .hash             HASH             0000000000000000  000002b8
> >        000000000000002c  0000000000000000   A       7     0     0
> >   [ 2] .gnu.hash         GNU_HASH         0000000000000000  000002e4
> >        0000000000000034  0000000000000000   A       7     0     0
> > 
> > Program Headers:
> >   Type           Offset             VirtAddr           PhysAddr
> >                  FileSiz            MemSiz              Flags  Align
> >   LOAD           0x0000000000000000 0x0000000000000000 0x0000000000000000
> >                  0x0000000000000348 0x0000000000000348  R E    0x1
> 
> In 32-bit case I did not set `Offset`, so it was not 0.
> I thought my comments saying something like `0x2b8 + PT_LOAD's p_offset == offset of .hash` are good enough,
> as it does not really important what is `0x2b8`, only the result of the formula is important.
> 
> I do not mind expanding the comment.
> Which approach would you prefer? (used in 32 bit or 64 bit case, i.e. with `Offset` set to `0` or not).
> 
> 
I think I'd prefer the Offset to not be explicitly set, i.e. the 32-bit approach.

By the way, the comment is slightly misleading, because the dynamic tag value is the address, not the offset, so really it should be referencing the program header's p_vaddr field.


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

https://reviews.llvm.org/D64788