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

[grimar]

Summary

DynRegionInfo is a helper class used to create memory ranges.
It is used for many things and can report errors.
Errors reported currently do not provide a good diagnostic.
This patch fixes it and adds a test for each possible case.

[MaskRay]

llvm/tools/llvm-readobj/ELFDumper.cpp

153

This order may be more natural: invalid size (32) of section with index 1

[jhenderson]

Should section sizes be in hex in the warnings? Not sure, but I feel like typical sections with entsize will have a more readable size (and entsize) when it is in hex.

llvm/test/Object/invalid.test

140

I'm not sure how to work this into the way you've written the code, but this doesn't read well. It should really be "invalid size (48) or entity size (32) of section with index 1". Perhaps this wording might work too: "section with index 1 has invalid size (48) or entity size (32)".

llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test

54

This doesn't read well (the relocation table isn't stored in the DT_RELASZ entry...). I think it should be something like "Show we print a warning for an invalid relocation table size stored in a DT_RELASZ entry".

91

Similar to above, how about: "Show we print a warning for an invalid relocation table entry size stored in a DT_RELAENT entry"?

128

Same comment as above.

165

Same as above.

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

358

it has a priority -> the dynamic tag has priority

360

Aside: why is it not using the DT_SYMENT dynamic tag?

368

This sort of message should be "invalid size (1) of section..." The current format isn't clear that the (1) is the size.

404

Nit: no new line at EOF.

llvm/tools/llvm-readobj/ELFDumper.cpp

140–141

ELF gabi refers to items in sections with non-zero sh_entsize as "entries", so this should probably be "Entry size name".

Change the last sentence to "If this field is empty, errors will not mention the entry size."

1946–1948

clang-format

[grimar]

• Addressed review comments.

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

360

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.

llvm/tools/llvm-readobj/ELFDumper.cpp

140–141

I've updated "entity"->"entry" in the error text too.

[grimar]

I've forgot to address the comment about changing section size printed to hex. Going to do it.

[jhenderson]

All other changes look good to me.

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

360

Sounds good.

llvm/tools/llvm-readobj/ELFDumper.cpp

139

Let's make this a little more generic. How about something like "InfoContext"?

1883

I'd move the " has " to the usage site, since it's always present, and the error message wouldn't make sense without it.

[grimar]

llvm/tools/llvm-readobj/ELFDumper.cpp

139

Ok. Seems leaving the "Error prefix" sentence for the comment is fine?

1883

ErrPrefix is often empty. E.g. messages like:
invalid DT_RELASZ value (24) or DT_RELAENT value (255) does not suppose to have this prefix about section index.

Do you mean to add "has" to the usage site when ErrPrefix is not empty?

[jhenderson]

llvm/tools/llvm-readobj/ELFDumper.cpp

139

Yes, I think so. The comment might rot if it starts getting used for other things, but I don't think it matters too much in this case.

1883

Oh, I missed that. I'd probably do as you suggest - add "has" when the prefix is non-empty.

[grimar]

• Addressed review comments.

[jhenderson]

LGTM, with the suggested change.

llvm/tools/llvm-readobj/ELFDumper.cpp

139

Having seen the change, I don't think I like InfoContext, sorry! How about just Context?

[grimar]

llvm/tools/llvm-readobj/ELFDumper.cpp

139

Sounds good to me. Will apply, thanks!

[grimar]

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

360

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@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@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...

[jhenderson]

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

360

That does seem very broken in various ways. Perhaps worth reporting to GNU? Either way, I don't think we need to follow any of that behaviour.

[grimar]

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

360

Yes, but in the context of the DT_SYMENT:
I think I do not understand why this value is usefull/was introduced.
E.g. LLD has the following line addInt(DT_SYMENT, sizeof(Elf_Sym));.
I'd expect other linkers to have something the same.
So seems we have 2 possible cases: 32/64 versions of a symbol. LLD works really well as we know...
Is it possible that something else can be valid as a value for the DT_SYMENT?
I just expect that such things are baked too hard and perhaps we do not want to
care about DT_SYMENT? Or may be should just check and report if it is != sizeof(Elf_Sym)?

[jhenderson]

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

360

I guess a warning if the DT_SYMENT does not match the symbol size of the appropriate ELF variety is fine.

[grimar]

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

360

OK. I'll prepare a patch for this.

llvm/test/Object/invalid.test

 Symbols:
   - Name: foo
 
 ## Check that llvm-readobj reports a warning if .dynsym has an invalid sh_entsize.
 
 # RUN: yaml2obj %s --docnum=7 -o %t7
 # RUN: llvm-readobj --dyn-symbols %t7 2>&1 | FileCheck -DFILE=%t7 --check-prefix=INVALID-DYNSYM-SIZE %s
 
-# INVALID-DYNSYM-SIZE: warning: '[[FILE]]': invalid section size (48) or entity size (32)
+# INVALID-DYNSYM-SIZE: warning: '[[FILE]]': section with index 1 has invalid size (0x30) or entry size (0x20)
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_DYN
   Machine: EM_X86_64
 Sections:

 
 ## Check that llvm-readobj reports a warning when a dynamic relocation section
 ## has sh_entsize field with size != sizeof(Elf_Rela).
 
 # RUN: llvm-readobj --dyn-relocations \
 # RUN:   %p/Inputs/corrupt-invalid-relocation-size.elf.x86-64 2>&1 \
 # RUN:    | FileCheck -DFILE=%p/Inputs/corrupt-invalid-relocation-size.elf.x86-64 --check-prefix=RELOC-BROKEN-ENTSIZE %s
 
-# RELOC-BROKEN-ENTSIZE: warning: '[[FILE]]': invalid section size (24) or entity size (25)
+# RELOC-BROKEN-ENTSIZE: warning: '[[FILE]]': invalid DT_RELASZ value (0x18) or DT_RELAENT value (0x19)
 
 ## Check that llvm-readobj reports a warning when .dynamic section has an invalid
 ## size, which isn't a multiple of the dynamic entry size. 
 
 # RUN: yaml2obj %s --docnum=22 -o %t22
 # RUN: llvm-readobj --dyn-relocations %t22 2>&1 | FileCheck -DFILE=%t22 --check-prefix=DYN-TABLE-SIZE %s
 
-# DYN-TABLE-SIZE: warning: '[[FILE]]': invalid section size (1) or entity size (16)
+# DYN-TABLE-SIZE: warning: '[[FILE]]': section with index 1 has invalid size (0x1){{$}}
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_EXEC
   Machine: EM_X86_64
 Sections:

llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc-name.test

-## Check that llvm-readobj/llvm-readelf reports an error when dumping relocations if a dynamic
-## symbol name offset is broken (goes past the end of the dynamic symbol string table).
-
-# RUN: yaml2obj %s -o %t
-# RUN: llvm-readobj --dyn-relocations %t 2>&1 | FileCheck %s -DFILE=%t --check-prefix=LLVM
-# RUN: llvm-readelf --dyn-relocations %t 2>&1 | FileCheck %s -DFILE=%t --check-prefix=GNU
-
-# LLVM:      Dynamic Relocations {
-# LLVM-NEXT: warning: '[[FILE]]': unable to get name of the dynamic symbol with index 1: st_name (0x1234) is past the end of the string table of size 0x1
-# LLVM-NEXT:   0x0 R_X86_64_NONE <corrupt> 0x0
-# LLVM-NEXT: }
-
-# GNU:      'RELA' relocation section at offset {{.+}} contains 24 bytes:
-# GNU-NEXT: Offset           Info             Type          Symbol's Value   Symbol's Name + Addend
-# GNU-NEXT: warning: '[[FILE]]': unable to get name of the dynamic symbol with index 1: st_name (0x1234) is past the end of the string table of size 0x1
-# GNU-NEXT: 0000000000000000 0000000100000000 R_X86_64_NONE 0000000000000000 <corrupt> + 0
-
---- !ELF
-FileHeader:
-  Class:   ELFCLASS64
-  Data:    ELFDATA2LSB
-  Type:    ET_DYN
-  Machine: EM_X86_64
-Sections:
-  - Name:    .rela.dyn
-    Type:    SHT_RELA
-    Link:    .dynsym
-    Relocations:
-      - Offset: 0x0
-        Symbol: 1 ## Index of a dynamic symbol with a broken st_name.
-        Type:   R_X86_64_NONE
-  - Name: .dynamic
-    Type: SHT_DYNAMIC
-    Entries:
-      - Tag:   DT_RELA
-        Value: 0x0000000000000000
-      - Tag:   DT_RELASZ
-        Value: 0x0000000000000018
-      - Tag:   DT_RELAENT
-        Value: 0x0000000000000018
-      - Tag:   DT_NULL
-        Value: 0x0000000000000000
-DynamicSymbols:
-  - NameIndex: 0x1234
-ProgramHeaders:
-  - Type:  PT_LOAD
-    Sections:
-      - Section: .rela.dyn
-      - Section: .dynamic

llvm/test/tools/llvm-readobj/ELF/broken-dynamic-reloc.test

+## Test how we handle cases when dynamic relocations or corresponding dynamic tags are broken.
+
+## Check that llvm-readobj/llvm-readelf reports an error when dumping relocations if a dynamic
+## symbol name offset is broken (goes past the end of the dynamic symbol string table).
+
+# RUN: yaml2obj --docnum=1 %s -o %t1
+# RUN: llvm-readobj --dyn-relocations %t1 2>&1 | FileCheck %s -DFILE=%t1 --check-prefix=LLVM
+# RUN: llvm-readelf --dyn-relocations %t1 2>&1 | FileCheck %s -DFILE=%t1 --check-prefix=GNU
+
+# LLVM:      Dynamic Relocations {
+# LLVM-NEXT: warning: '[[FILE]]': unable to get name of the dynamic symbol with index 1: st_name (0x1234) is past the end of the string table of size 0x1
+# LLVM-NEXT:   0x0 R_X86_64_NONE <corrupt> 0x0
+# LLVM-NEXT: }
+
+# GNU:      'RELA' relocation section at offset {{.+}} contains 24 bytes:
+# GNU-NEXT: Offset           Info             Type          Symbol's Value   Symbol's Name + Addend
+# GNU-NEXT: warning: '[[FILE]]': unable to get name of the dynamic symbol with index 1: st_name (0x1234) is past the end of the string table of size 0x1
+# GNU-NEXT: 0000000000000000 0000000100000000 R_X86_64_NONE 0000000000000000 <corrupt> + 0
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:    .rela.dyn
+    Type:    SHT_RELA
+    Link:    .dynsym
+    Relocations:
+      - Offset: 0x0
+        Symbol: 1 ## Index of a dynamic symbol with a broken st_name.
+        Type:   R_X86_64_NONE
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+    Entries:
+      - Tag:   DT_RELA
+        Value: 0x0000000000000000
+      - Tag:   DT_RELASZ
+        Value: 0x0000000000000018
+      - Tag:   DT_RELAENT
+        Value: 0x0000000000000018
+      - Tag:   DT_NULL
+        Value: 0x0000000000000000
+DynamicSymbols:
+  - NameIndex: 0x1234
+ProgramHeaders:
+  - Type:  PT_LOAD
+    Sections:
+      - Section: .rela.dyn
+      - Section: .dynamic
+
+## Show we print a warning for an invalid relocation table size stored in a DT_RELASZ entry.
+# RUN: yaml2obj --docnum=2 %s -o %t2
+# RUN: llvm-readobj --dyn-relocations %t2 2>&1 | FileCheck %s -DFILE=%t2 --check-prefix=INVALID-DT-RELASZ
+# RUN: llvm-readelf --dyn-relocations %t2 2>&1 | FileCheck %s -DFILE=%t2 --check-prefix=INVALID-DT-RELASZ
+
+# INVALID-DT-RELASZ: warning: '[[FILE]]': invalid DT_RELASZ value (0xff) or DT_RELAENT value (0x18)
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:    .rela.dyn
+    Type:    SHT_RELA
+    Relocations:
+      - Offset: 0x0
+        Type:   R_X86_64_NONE
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+    Entries:
+      - Tag:   DT_RELA
+        Value: 0x0
+      - Tag:   DT_RELASZ
+        Value: 0xFF
+      - Tag:   DT_RELAENT
+        Value: 0x18
+      - Tag:   DT_NULL
+        Value: 0x0
+DynamicSymbols: []
+ProgramHeaders:
+  - Type:  PT_LOAD
+    Sections:
+      - Section: .rela.dyn
+      - Section: .dynamic
+
+## Show we print a warning for an invalid relocation table entry size stored in a DT_RELAENT entry.
+# RUN: yaml2obj --docnum=3 %s -o %t3
+# RUN: llvm-readobj --dyn-relocations %t3 2>&1 | FileCheck %s -DFILE=%t3 --check-prefix=INVALID-DT-RELAENT
+# RUN: llvm-readelf --dyn-relocations %t3 2>&1 | FileCheck %s -DFILE=%t3 --check-prefix=INVALID-DT-RELAENT
+
+## INVALID-DT-RELAENT: warning: '[[FILE]]': invalid DT_RELASZ value (0x18) or DT_RELAENT value (0xff)
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:    .rela.dyn
+    Type:    SHT_RELA
+    Relocations:
+      - Offset: 0x0
+        Type:   R_X86_64_NONE
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+    Entries:
+      - Tag:   DT_RELA
+        Value: 0x0
+      - Tag:   DT_RELASZ
+        Value: 0x18
+      - Tag:   DT_RELAENT
+        Value: 0xFF
+      - Tag:   DT_NULL
+        Value: 0x0
+DynamicSymbols: []
+ProgramHeaders:
+  - Type:  PT_LOAD
+    Sections:
+      - Section: .rela.dyn
+      - Section: .dynamic
+
+## Show we print a warning for an invalid relocation table size stored in a DT_RELSZ entry.
+# RUN: yaml2obj --docnum=4 %s -o %t4
+# RUN: llvm-readobj --dyn-relocations %t4 2>&1 | FileCheck %s -DFILE=%t4 --check-prefix=INVALID-DT-RELSZ
+# RUN: llvm-readelf --dyn-relocations %t4 2>&1 | FileCheck %s -DFILE=%t4 --check-prefix=INVALID-DT-RELSZ
+
+## INVALID-DT-RELSZ: warning: '[[FILE]]': invalid DT_RELSZ value (0xff) or DT_RELENT value (0x18)
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_386
+Sections:
+  - Name:    .rela.dyn
+    Type:    SHT_REL
+    Relocations:
+      - Offset: 0x0
+        Type:   R_386_NONE
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+    Entries:
+      - Tag:   DT_REL
+        Value: 0x0
+      - Tag:   DT_RELSZ
+        Value: 0xFF
+      - Tag:   DT_RELENT
+        Value: 0x18
+      - Tag:   DT_NULL
+        Value: 0x0
+DynamicSymbols: []
+ProgramHeaders:
+  - Type:  PT_LOAD
+    Sections:
+      - Section: .rela.dyn
+      - Section: .dynamic
+
+## Show we print a warning for an invalid relocation table entry size stored in a DT_RELENT entry.
+# RUN: yaml2obj --docnum=5 %s -o %t5
+# RUN: llvm-readobj --dyn-relocations %t5 2>&1 | FileCheck %s -DFILE=%t5 --check-prefix=INVALID-DT-RELENT
+# RUN: llvm-readelf --dyn-relocations %t5 2>&1 | FileCheck %s -DFILE=%t5 --check-prefix=INVALID-DT-RELENT
+
+## INVALID-DT-RELENT: warning: '[[FILE]]': invalid DT_RELSZ value (0x18) or DT_RELENT value (0xff)
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_386
+Sections:
+  - Name:    .rela.dyn
+    Type:    SHT_REL
+    Relocations:
+      - Offset: 0x0
+        Type:   R_386_NONE
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+    Entries:
+      - Tag:   DT_REL
+        Value: 0x0
+      - Tag:   DT_RELSZ
+        Value: 0x18
+      - Tag:   DT_RELENT
+        Value: 0xFF
+      - Tag:   DT_NULL
+        Value: 0x0
+DynamicSymbols: []
+ProgramHeaders:
+  - Type:  PT_LOAD
+    Sections:
+      - Section: .rela.dyn
+      - Section: .dynamic
+
+## Show we print a warning for an invalid relocation table size stored in a DT_RELRSZ/DT_ANDROID_RELRSZ entry.
+# RUN: yaml2obj --docnum=6 %s -o %t6
+# RUN: llvm-readobj --dyn-relocations %t6 2>&1 | FileCheck %s -DFILE=%t6 --check-prefix=INVALID-DT-RELRSZ
+# RUN: llvm-readelf --dyn-relocations %t6 2>&1 | FileCheck %s -DFILE=%t6 --check-prefix=INVALID-DT-RELRSZ
+# RUN: yaml2obj --docnum=7 %s -o %t7
+# RUN: llvm-readobj --dyn-relocations %t7 2>&1 | FileCheck %s -DFILE=%t7 --check-prefix=INVALID-DT-ANDROID-RELRSZ
+# RUN: llvm-readelf --dyn-relocations %t7 2>&1 | FileCheck %s -DFILE=%t7 --check-prefix=INVALID-DT-ANDROID-RELRSZ
+
+## INVALID-DT-RELRSZ:         warning: '[[FILE]]': invalid DT_RELRSZ value (0xff) or DT_RELRENT value (0x18)
+## INVALID-DT-ANDROID-RELRSZ: warning: '[[FILE]]': invalid DT_ANDROID_RELRSZ value (0xff) or DT_ANDROID_RELRENT value (0x18)
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:    .relr.dyn
+    Type:    SHT_RELR
+    Flags:   [ SHF_ALLOC ]
+    Content: ""
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+    Entries:
+      - Tag:   DT_RELR
+        Value: 0x0
+      - Tag:   DT_RELRSZ
+        Value: 0xFF
+      - Tag:   DT_RELRENT
+        Value: 0x18
+      - Tag:   DT_NULL
+        Value: 0x0
+DynamicSymbols: []
+ProgramHeaders:
+  - Type:  PT_LOAD
+    Sections:
+      - Section: .relr.dyn
+      - Section: .dynamic
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:    .relr.dyn
+    Type:    SHT_RELR
+    Flags:   [ SHF_ALLOC ]
+    Content: ""
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+    Entries:
+      - Tag:   DT_RELR
+        Value: 0x0
+      - Tag:   DT_ANDROID_RELRSZ
+        Value: 0xFF
+      - Tag:   DT_ANDROID_RELRENT
+        Value: 0x18
+      - Tag:   DT_NULL
+        Value: 0x0
+DynamicSymbols: []
+ProgramHeaders:
+  - Type:  PT_LOAD
+    Sections:
+      - Section: .relr.dyn
+      - Section: .dynamic
+
+## Show we print a warning for an invalid relocation table entry size stored in a DT_RELRENT/DT_ANDROID_RELRENT entry.
+# RUN: yaml2obj --docnum=8 %s -o %t8
+# RUN: llvm-readobj --dyn-relocations %t8 2>&1 | FileCheck %s -DFILE=%t8 --check-prefix=INVALID-DT-RELRENT
+# RUN: llvm-readelf --dyn-relocations %t8 2>&1 | FileCheck %s -DFILE=%t8 --check-prefix=INVALID-DT-RELRENT
+# RUN: yaml2obj --docnum=9 %s -o %t9
+# RUN: llvm-readobj --dyn-relocations %t9 2>&1 | FileCheck %s -DFILE=%t9 --check-prefix=INVALID-DT-ANDROID-RELRENT
+# RUN: llvm-readelf --dyn-relocations %t9 2>&1 | FileCheck %s -DFILE=%t9 --check-prefix=INVALID-DT-ANDROID-RELRENT
+
+## INVALID-DT-RELRENT:         invalid DT_RELRSZ value (0x18) or DT_RELRENT value (0xff)
+## INVALID-DT-ANDROID-RELRENT: invalid DT_ANDROID_RELRSZ value (0x18) or DT_ANDROID_RELRENT value (0xff)
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:    .relr.dyn
+    Type:    SHT_RELR
+    Flags:   [ SHF_ALLOC ]
+    Content: ""
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+    Entries:
+      - Tag:   DT_RELR
+        Value: 0x0
+      - Tag:   DT_RELRSZ
+        Value: 0x18
+      - Tag:   DT_RELRENT
+        Value: 0xFF
+      - Tag:   DT_NULL
+        Value: 0x0
+DynamicSymbols: []
+ProgramHeaders:
+  - Type:  PT_LOAD
+    Sections:
+      - Section: .relr.dyn
+      - Section: .dynamic
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:    .relr.dyn
+    Type:    SHT_RELR
+    Flags:   [ SHF_ALLOC ]
+    Content: ""
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+    Entries:
+      - Tag:   DT_RELR
+        Value: 0x0
+      - Tag:   DT_ANDROID_RELRSZ
+        Value: 0x18
+      - Tag:   DT_ANDROID_RELRENT
+        Value: 0xFF
+      - Tag:   DT_NULL
+        Value: 0x0
+DynamicSymbols: []
+ProgramHeaders:
+  - Type:  PT_LOAD
+    Sections:
+      - Section: .relr.dyn
+      - Section: .dynamic
+
+## Show we print a warning for an invalid value of DT_PLTRELSZ, which describes the total size
+## of the relocation entries associated with the procedure linkage table.
+# RUN: yaml2obj --docnum=10 %s -o %t10
+# RUN: llvm-readobj --dyn-relocations %t10 2>&1 | FileCheck %s -DFILE=%t10 --check-prefix=INVALID-DT-PLTRELSZ
+# RUN: llvm-readelf --dyn-relocations %t10 2>&1 | FileCheck %s -DFILE=%t10 --check-prefix=INVALID-DT-PLTRELSZ
+
+# INVALID-DT-PLTRELSZ: warning: '[[FILE]]': invalid DT_PLTRELSZ value (0xff){{$}}
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name:  .rela.plt
+    Type:  SHT_RELA
+    Relocations:
+      - Offset: 0x0
+        Type:   R_X86_64_NONE
+  - Name:  .dynamic
+    Type:  SHT_DYNAMIC
+    Entries:
+      - Tag:   DT_JMPREL
+        Value: 0x0
+      - Tag:   DT_PLTRELSZ
+        Value: 0xFF ## The valid value would be 0x18.
+      - Tag:   DT_PLTREL
+        Value: 0x7 ## DT_RELA
+      - Tag:   DT_NULL
+        Value: 0x0
+DynamicSymbols: []
+ProgramHeaders:
+  - Type:  PT_LOAD
+    Sections:
+      - Section: .rela.plt
+      - Section: .dynamic

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS32
   Data:    ELFDATA2LSB
   Type:    ET_DYN
   Machine: EM_386
 DynamicSymbols: []
+
+## Case 9: Check what we print when:
+## a) The size of the dynamic symbol table is not a multiple of its entry size.
+# RUN: yaml2obj %s --docnum=10 -o %t10
+# RUN: llvm-readobj --dyn-symbols %t10 2>&1 | FileCheck %s -DFILE=%t10 --check-prefix=DYNSYM-SIZE-INVALID1
+# RUN: llvm-readelf --dyn-symbols %t10 2>&1 | FileCheck %s -DFILE=%t10 --check-prefix=DYNSYM-SIZE-INVALID1
+## b) The same, but the DT_SYMTAB tag is present. In this case the dynamic tag has priority over the
+##    information about 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.
+# RUN: yaml2obj %s --docnum=11 -o %t11
+# RUN: llvm-readobj --dyn-symbols %t11 2>&1 | FileCheck %s -DFILE=%t11 --check-prefix=DYNSYM-SIZE-INVALID2
+# RUN: llvm-readelf --dyn-symbols %t11 2>&1 | FileCheck %s -DFILE=%t11 --check-prefix=DYNSYM-SIZE-INVALID2
+
+# DYNSYM-SIZE-INVALID1: warning: '[[FILE]]': section with index 1 has invalid size (0x1) or entry size (0x10)
+
+# DYNSYM-SIZE-INVALID2: warning: '[[FILE]]': section with index 2 has invalid size (0x1){{$}}
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_386
+Sections:
+  - Name: .dynsym
+    Type: SHT_DYNSYM
+    Size: 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_NULL
+        Value: 0
+  - Name:    .dynsym
+    Type:    SHT_DYNSYM
+    Address: 0x100
+    Size:    0x1
+ProgramHeaders:
+  - Type: PT_LOAD
+    VAddr: 0x100
+    Sections:
+      - Section: .dynsym

llvm/test/tools/llvm-readobj/ELF/dynamic-malformed.test

 ## Test handling of a dynamic section size which is not a multiple of its entry size.
 ## Test the full output to demonstrate how we print the warnings.
 
 # RUN: yaml2obj %s --docnum=1 -o %t.bad-size
 # RUN: llvm-readobj --all %t.bad-size 2>&1 \
 # RUN:   | FileCheck %s -DFILE=%t.bad-size --implicit-check-not=warning --check-prefix WARN
 # RUN: llvm-readelf --all %t.bad-size 2>&1 \
 # RUN:   | FileCheck %s -DFILE=%t.bad-size --implicit-check-not=warning --check-prefix WARN-GNU
 
-# WARN: warning: '[[FILE]]': invalid section size (4) or entity size (16)
-# WARN: warning: '[[FILE]]': invalid section size (4) or entity size (16)
+# WARN: warning: '[[FILE]]': invalid PT_DYNAMIC size (0x4){{$}}
+# WARN: warning: '[[FILE]]': section with index 1 has invalid size (0x4){{$}}
 # WARN: warning: '[[FILE]]': no valid dynamic table was found
 # WARN-EMPTY:
 # WARN: File:
 # WARN: Symbols [
 # WARN: ]
 # WARN: ProgramHeaders [
 
-# WARN-GNU: warning: '[[FILE]]': invalid section size (4) or entity size (16)
-# WARN-GNU: warning: '[[FILE]]': invalid section size (4) or entity size (16)
+# WARN-GNU: warning: '[[FILE]]': invalid PT_DYNAMIC size (0x4){{$}}
+# WARN-GNU: warning: '[[FILE]]': section with index 1 has invalid size (0x4){{$}}
 # WARN-GNU: warning: '[[FILE]]': no valid dynamic table was found
 # WARN-GNU-NEXT: ELF Header:
 # WARN-GNU:      Symbol table '.symtab' contains 1 entries:
 # WARN-GNU:        0:
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64

llvm/test/tools/llvm-readobj/ELF/dynamic-not-in-pt-dynamic.test

 ## Show that llvm-readobj/llvm-readelf tools sometimes can dump the
 ## dynamic table when it is not in a PT_DYNAMIC segment.
 
 ## Case 1: The dynamic table found using the dynamic program header is corrupted
 ##         (<size of data> % <size of dynamic entry> != 0). So the table is taken
 ##         from the section header.
 
 # RUN: yaml2obj --docnum=1 %s -o %t1.o
 # RUN: llvm-readobj --dynamic-table %t1.o 2>&1 \
 # RUN:   | FileCheck -DFILE=%t1.o --check-prefixes=WARNING1,LLVM1 %s
 # RUN: llvm-readelf --dynamic-table %t1.o 2>&1 \
 # RUN:   | FileCheck -DFILE=%t1.o --check-prefixes=WARNING1,GNU1 %s
 
 # WARNING1: warning: '[[FILE]]': The SHT_DYNAMIC section '.dynamic' is not contained within the PT_DYNAMIC segment
-# WARNING1: warning: '[[FILE]]': invalid section size (1) or entity size (16)
+# WARNING1: warning: '[[FILE]]': invalid PT_DYNAMIC size (0x1){{$}}
 # WARNING1: warning: '[[FILE]]': SHT_DYNAMIC section header and PT_DYNAMIC program header disagree about the location of the dynamic table
 # WARNING1: warning: '[[FILE]]': PT_DYNAMIC dynamic table is invalid: SHT_DYNAMIC will be used
 
 # LLVM1:      DynamicSection [ (2 entries)
 # LLVM1-NEXT:   Tag                Type     Name/Value
 # LLVM1-NEXT:   0x0000000000000018 BIND_NOW 0x1
 # LLVM1-NEXT:   0x0000000000000000 NULL     0x0
 # LLVM1-NEXT: ]

 ## Case 3: Both dynamic tables found using SHT_DYNAMIC/PT_DYNAMIC are corrupted.
 
 # RUN: yaml2obj --docnum=3 %s -o %t3.o
 # RUN: llvm-readobj --dynamic-table %t3.o 2>&1 \
 # RUN:   | FileCheck -DFILE=%t3.o --check-prefix=WARNING3 --implicit-check-not="Dynamic" %s
 # RUN: llvm-readelf --dynamic-table %t3.o 2>&1 \
 # RUN:   | FileCheck -DFILE=%t3.o --check-prefix=WARNING3 --implicit-check-not="Dynamic" %s
 
-# WARNING3: warning: '[[FILE]]': invalid section size (1) or entity size (16)
+# WARNING3: warning: '[[FILE]]': invalid PT_DYNAMIC size (0x1){{$}}
 # WARNING3: warning: '[[FILE]]': SHT_DYNAMIC section header and PT_DYNAMIC program header disagree about the location of the dynamic table
 # WARNING3: warning: '[[FILE]]': no valid dynamic table was found
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_EXEC

llvm/test/tools/llvm-readobj/ELF/non-dynamic-in-pt-dynamic.test

 ## Show that llvm-readobj/llvm-readelf tools can dump the .dynamic section which
 ## is not alone in PT_DYNAMIC segment.
 
 ## In the first case .text is placed before .dynamic.
 ## We check that we warn about this case.
 
 # RUN: yaml2obj --docnum=1 %s -o %t1.o
 # RUN: llvm-readobj --dynamic-table %t1.o 2>&1 \
 # RUN:   | FileCheck %s --DFILE=%t1.o --check-prefixes=WARNING,LLVM
 # RUN: llvm-readelf --dynamic-table %t1.o 2>&1 \
 # RUN:   | FileCheck %s --DFILE=%t1.o --check-prefixes=WARNING,GNU
 
 # WARNING: warning: '[[FILE]]': The SHT_DYNAMIC section '.dynamic' is not at the start of PT_DYNAMIC segment
-# WARNING: warning: '[[FILE]]': invalid section size (33) or entity size (16)
+# WARNING: warning: '[[FILE]]': invalid PT_DYNAMIC size (0x21){{$}}
 # WARNING: warning: '[[FILE]]': SHT_DYNAMIC section header and PT_DYNAMIC program header disagree about the location of the dynamic table
 # WARNING: warning: '[[FILE]]': PT_DYNAMIC dynamic table is invalid: SHT_DYNAMIC will be used
 
 # LLVM:      DynamicSection [ (2 entries)
 # LLVM-NEXT:   Tag                Type     Name/Value
 # LLVM-NEXT:   0x0000000000000018 BIND_NOW 0x1
 # LLVM-NEXT:   0x0000000000000000 NULL     0x0
 # LLVM-NEXT: ]

 
 ## In this case .text goes after .dynamic, but (PT_DYNAMIC segment size % dynamic entry size != 0)
 ## and we have to use the information from the section header instead.
 
 # RUN: yaml2obj --docnum=3 %s -o %t3.o
 # RUN: llvm-readobj --dynamic-table %t3.o 2>&1 | FileCheck %s --DFILE=%t3.o --check-prefixes=WARNING2,LLVM3
 # RUN: llvm-readelf --dynamic-table %t3.o 2>&1 | FileCheck %s --DFILE=%t3.o --check-prefixes=WARNING2,GNU3
 
-# WARNING2: warning: '[[FILE]]': invalid section size (257) or entity size (16)
+# WARNING2: warning: '[[FILE]]': invalid PT_DYNAMIC size (0x101){{$}}
 # WARNING2: warning: '[[FILE]]': PT_DYNAMIC dynamic table is invalid: SHT_DYNAMIC will be used
 
 # LLVM3:      DynamicSection [ (2 entries)
 # LLVM3-NEXT:   Tag                Type     Name/Value
 # LLVM3-NEXT:   0x0000000000000018 BIND_NOW 0x1
 # LLVM3-NEXT:   0x0000000000000000 NULL     0x0
 # LLVM3-NEXT: ]
 

llvm/tools/llvm-readobj/ELFDumper.cpp

struct DynRegionInfo {
   const void *Addr = nullptr;
   /// Size in bytes of the region.
   uint64_t Size = 0;
   /// Size of each entity in the region.
   uint64_t EntSize = 0;
 
   /// Name of the file. Used for error reporting.
   StringRef FileName;
+  /// Error prefix. Used for error reporting to provide more information.
+  std::string Context;
+  /// Region size name. Used for error reporting.
+  StringRef SizePrintName = "size";
+  /// Entry size name. Used for error reporting. If this field is empty, errors
+  /// will not mention the entry size.
+  StringRef EntSizePrintName = "entry size";
 
   template <typename Type> ArrayRef<Type> getAsArrayRef() const {
     const Type *Start = reinterpret_cast<const Type *>(Addr);
     if (!Start)
       return {Start, Start};
-    if (EntSize != sizeof(Type) || Size % EntSize) {
-      // TODO: Add a section index to this warning.
-      reportWarning(createError("invalid section size (" + Twine(Size) +
-                                ") or entity size (" + Twine(EntSize) + ")"),
-                    FileName);
-      return {Start, Start};
-    }
+    if (EntSize == sizeof(Type) && (Size % EntSize == 0))
       return {Start, Start + (Size / EntSize)};
+
+    std::string Msg;
+    if (!Context.empty())
+      Msg += Context + " has ";
+
+    Msg += ("invalid " + SizePrintName + " (0x" + Twine::utohexstr(Size) + ")")
+               .str();
+    if (!EntSizePrintName.empty())
+      Msg +=
+          (" or " + EntSizePrintName + " (0x" + Twine::utohexstr(EntSize) + ")")
+              .str();
+
+    reportWarning(createError(Msg.c_str()), FileName);
+    return {Start, Start};
   }
 };
 
 namespace {
 struct VerdAux {
   unsigned Offset;
   std::string Name;
 };
void ELFDumper<ELFT>::loadDynamicTable(const ELFFile<ELFT> *Obj) {
   std::tie(DynamicPhdr, DynamicSec) = findDynamic(Obj);
   if (!DynamicPhdr && !DynamicSec)
     return;
 
   DynRegionInfo FromPhdr(ObjF->getFileName());
   bool IsPhdrTableValid = false;
   if (DynamicPhdr) {
     FromPhdr = createDRIFrom(DynamicPhdr, sizeof(Elf_Dyn));
+    FromPhdr.SizePrintName = "PT_DYNAMIC size";
+    FromPhdr.EntSizePrintName = "";
+
     IsPhdrTableValid = !FromPhdr.getAsArrayRef<Elf_Dyn>().empty();
   }
 
   // Locate the dynamic table described in a section header.
   // Ignore sh_entsize and use the expected value for entry size explicitly.
   // This allows us to dump dynamic sections with a broken sh_entsize
   // field.
   DynRegionInfo FromSec(ObjF->getFileName());
   bool IsSecTableValid = false;
   if (DynamicSec) {
     FromSec =
         checkDRI({ObjF->getELFFile()->base() + DynamicSec->sh_offset,
                   DynamicSec->sh_size, sizeof(Elf_Dyn), ObjF->getFileName()});
+    FromSec.Context = ("section with index " +
+                       Twine(DynamicSec - &cantFail(Obj->sections()).front()))
+                          .str();
+    FromSec.EntSizePrintName = "";
+
     IsSecTableValid = !FromSec.getAsArrayRef<Elf_Dyn>().empty();
   }
 
   // When we only have information from one of the SHT_DYNAMIC section header or
   // PT_DYNAMIC program header, just use that.
   if (!DynamicPhdr || !DynamicSec) {
     if ((DynamicPhdr && IsPhdrTableValid) || (DynamicSec && IsSecTableValid)) {
       DynamicTable = DynamicPhdr ? FromPhdr : FromSec;

 template <typename ELFT>
 ELFDumper<ELFT>::ELFDumper(const object::ELFObjectFile<ELFT> *ObjF,
                            ScopedPrinter &Writer)
     : ObjDumper(Writer), ObjF(ObjF), DynRelRegion(ObjF->getFileName()),
       DynRelaRegion(ObjF->getFileName()), DynRelrRegion(ObjF->getFileName()),
       DynPLTRelRegion(ObjF->getFileName()), DynSymRegion(ObjF->getFileName()),
       DynamicTable(ObjF->getFileName()) {
   const ELFFile<ELFT> *Obj = ObjF->getELFFile();
-  for (const Elf_Shdr &Sec :
-       unwrapOrError(ObjF->getFileName(), Obj->sections())) {
+  typename ELFT::ShdrRange Sections =
+      unwrapOrError(ObjF->getFileName(), Obj->sections());
+  for (const Elf_Shdr &Sec : Sections) {
     switch (Sec.sh_type) {
     case ELF::SHT_SYMTAB:
       if (!DotSymtabSec)
         DotSymtabSec = &Sec;
       break;
     case ELF::SHT_DYNSYM:
       if (!DynSymRegion.Size) {
         DynSymRegion = createDRIFrom(&Sec);
+        DynSymRegion.Context =
+            ("section with index " + Twine(&Sec - &Sections.front())).str();
         // This is only used (if Elf_Shdr present)for naming section in GNU
         // style
         DynSymtabName =
             unwrapOrError(ObjF->getFileName(), Obj->getSectionName(&Sec));
 
         if (Expected<StringRef> E = Obj->getStringTableForSymtab(Sec))
           DynamicStringTable = *E;
         else
case ELF::DT_SYMTAB: {
           reportWarning(
               createError(
                   "SHT_DYNSYM section header and DT_SYMTAB disagree about "
                   "the location of the dynamic symbol table"),
               ObjF->getFileName());
 
         DynSymRegion.Addr = VA;
         DynSymRegion.EntSize = sizeof(Elf_Sym);
+        DynSymRegion.EntSizePrintName = "";
       }
       break;
     }
     case ELF::DT_RELA:
       DynRelaRegion.Addr = toMappedAddr(Dyn.getTag(), Dyn.getPtr());
       break;
     case ELF::DT_RELASZ:
       DynRelaRegion.Size = Dyn.getVal();
+      DynRelaRegion.SizePrintName = "DT_RELASZ value";
       break;
     case ELF::DT_RELAENT:
       DynRelaRegion.EntSize = Dyn.getVal();
+      DynRelaRegion.EntSizePrintName = "DT_RELAENT value";
       break;
     case ELF::DT_SONAME:
       SONameOffset = Dyn.getVal();
       break;
     case ELF::DT_REL:
       DynRelRegion.Addr = toMappedAddr(Dyn.getTag(), Dyn.getPtr());
       break;
     case ELF::DT_RELSZ:
       DynRelRegion.Size = Dyn.getVal();
+      DynRelRegion.SizePrintName = "DT_RELSZ value";
       break;
     case ELF::DT_RELENT:
       DynRelRegion.EntSize = Dyn.getVal();
+      DynRelRegion.EntSizePrintName = "DT_RELENT value";
       break;
     case ELF::DT_RELR:
     case ELF::DT_ANDROID_RELR:
       DynRelrRegion.Addr = toMappedAddr(Dyn.getTag(), Dyn.getPtr());
       break;
     case ELF::DT_RELRSZ:
     case ELF::DT_ANDROID_RELRSZ:
       DynRelrRegion.Size = Dyn.getVal();
+      DynRelrRegion.SizePrintName = Dyn.d_tag == ELF::DT_RELRSZ
+                                        ? "DT_RELRSZ value"
+                                        : "DT_ANDROID_RELRSZ value";
       break;
     case ELF::DT_RELRENT:
     case ELF::DT_ANDROID_RELRENT:
       DynRelrRegion.EntSize = Dyn.getVal();
+      DynRelrRegion.EntSizePrintName = Dyn.d_tag == ELF::DT_RELRENT
+                                           ? "DT_RELRENT value"
+                                           : "DT_ANDROID_RELRENT value";
       break;
     case ELF::DT_PLTREL:
       if (Dyn.getVal() == DT_REL)
         DynPLTRelRegion.EntSize = sizeof(Elf_Rel);
       else if (Dyn.getVal() == DT_RELA)
         DynPLTRelRegion.EntSize = sizeof(Elf_Rela);
       else
         reportError(createError(Twine("unknown DT_PLTREL value of ") +
                                 Twine((uint64_t)Dyn.getVal())),
                     ObjF->getFileName());
+      DynPLTRelRegion.EntSizePrintName = "";
       break;
     case ELF::DT_JMPREL:
       DynPLTRelRegion.Addr = toMappedAddr(Dyn.getTag(), Dyn.getPtr());
       break;
     case ELF::DT_PLTRELSZ:
       DynPLTRelRegion.Size = Dyn.getVal();
+      DynPLTRelRegion.SizePrintName = "DT_PLTRELSZ value";
       break;
     }
   }
   if (StringTableBegin)
     DynamicStringTable = StringRef(StringTableBegin, StringTableSize);
   SOName = getDynamicString(SONameOffset);
 }