[PATCH] D73484: [llvm-readobj] - Improve error message reported by DynRegionInfo.

[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/dyn-symbols.test:360
+##    a location and an entity size of the dynamic symbol table from the section header.
+##    The code uses sizeof(Elf_Sym) for an entity size, so it can't be incorrect and
+##    the message printed is a bit shorter.
----------------
jhenderson wrote:
> grimar wrote:
> > jhenderson wrote:
> > > Aside: why is it not using the DT_SYMENT dynamic tag?
> > I do not know. Perhaps we want to fix this. Also, I think this area is not well tested.
> > I can investigate it in a follow-up.
> Sounds good.
The behavior of GNU readelf looks broken to me.
See: when we have `DT_SYMENT == 0x123` and a `.dynsym` of size of `0x1`:


```
--- !ELF
FileHeader:
  Class:   ELFCLASS64
  Data:    ELFDATA2LSB
  Type:    ET_DYN
  Machine: EM_X86_64
Sections:
  - Name:    .dynamic
    Type:    SHT_DYNAMIC
    Entries:
      - Tag:   DT_SYMTAB
        Value: 0x100
      - Tag:   DT_SYMENT
        Value: 0x123
      - Tag:   DT_NULL
        Value: 0
  - Name:    .dynsym
    Type:    SHT_DYNSYM
    Address: 0x100
    Size:    0x1
ProgramHeaders:
  - Type: PT_LOAD
    VAddr: 0x100
    Sections:
      - Section: .dynsym
```

what produce the object with following section headers:


```
Section Headers:
  [Nr] Name              Type             Address           Offset
       Size              EntSize          Flags  Link  Info  Align
  [ 0]                   NULL             0000000000000000  00000000
       0000000000000000  0000000000000000           0     0     0
readelf: Warning: [ 1]: Link field (0) should index a string section.
  [ 1] .dynamic          DYNAMIC          0000000000000000  00000078
       0000000000000030  0000000000000010           0     0     0
readelf: Warning: [ 2]: Link field (0) should index a string section.
  [ 2] .dynsym           DYNSYM           0000000000000100  000000a8
       0000000000000001  0000000000000018   A       0     1     0
  [ 3] .strtab           STRTAB           0000000000000000  000000a9
       0000000000000001  0000000000000000           0     0     1
  [ 4] .shstrtab         STRTAB           0000000000000000  000000aa
       0000000000000024  0000000000000000           0     0     1
```

(Note: .dynsym has sh_entsize == 0x18. It is a correct value set by yaml2obj
implicitly. The size is broken as expected and == 0x1).

I have following outputs:

**Case 1:**

```
umb at ubuntu:~/tests/121$ readelf --symbols test.o
readelf: Error: Section .dynsym has an invalid sh_entsize of 0x18

Symbol table '.dynsym' contains 0 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
readelf: Error: Section .dynsym has an invalid sh_entsize of 0x18
```

**Case 2:**
```
umb at ubuntu:~/tests/121$ readelf --symbols test.o -D
readelf: Error: Section .dynsym has an invalid sh_entsize of 0x18

Dynamic symbol information is not available for displaying symbols.
```

Well... there are many broken things here I see:
1) ".dynsym has an invalid sh_entsize" it is not true. The `sh_size` is broken, but `0x18` is a valid value for `sh_entsize`.
2) Duplicate error messages reported in the case #1. It reminds me about plans to switch to `reportUniqueWarning`.
   Perhaps it is a time to do this.
3) The different output. I.e. in the first case we see the dynamic symbols section header,
   in the second we don't have it. Looks a bit inconsistent to me. In both cases we dump symbols.
4) Error messages started from the upper-case. I see we have the same thing in "llvm-readobj\ELF\broken-group.test",
   but our other tests use the lower case form (as expected, I believe).
5) Error? Why not just a warning...



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73484