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

[George Rimar via Phabricator via llvm-commits]

grimar marked an inline comment as done.
grimar 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
----------------
jhenderson wrote:
> 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.
It is a bit more complex actually.

Both are used to read the hash sections data:
https://github.com/llvm-mirror/llvm/blob/master/lib/Object/ELF.cpp#L581
```
  uint64_t Delta = VAddr - Phdr.p_vaddr;
  if (Delta >= Phdr.p_filesz)
    return createError("virtual address is not in any segment: 0x" +
                       Twine::utohexstr(VAddr));
  return base() + Phdr.p_offset + Delta;
```

But program header's p_vaddr is null here, so I mentioned only `p_offset` field.


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

https://reviews.llvm.org/D64788