[llvm] b87a0f7 - [llvm-readobj] - Report a warning when an unexpected DT_SYMENT tag value is met.

[Georgii Rymar via llvm-commits]

Author: Georgii Rymar
Date: 2020-02-18T14:36:17+03:00
New Revision: b87a0f7416f1a657d808690295b44669849e7bfc

URL: https://github.com/llvm/llvm-project/commit/b87a0f7416f1a657d808690295b44669849e7bfc
DIFF: https://github.com/llvm/llvm-project/commit/b87a0f7416f1a657d808690295b44669849e7bfc.diff

LOG: [llvm-readobj] - Report a warning when an unexpected DT_SYMENT tag value is met.

There was a short discussion about this:
https://reviews.llvm.org/D73484#inline-676942

To summarize:
It is a bit unclear to me why the `DT_SYMENT` tag exist.
LLD has the code that does:
"addInt(DT_SYMENT, sizeof(Elf_Sym));" and I guess other linkers has the same logic.
It is unclear why it can be possible to have other values rather than values of
a size of platform symbol. Seems it is not possible, and atm for me it looks that
this tag should not be used. This patch starts reporting the warning when the
value it contains differs from a symbol size for a 32/64 bit platform for safety.
It keeps the rest of the logic we have unchanged. Before this patch we did not handle
the tag at all.

Differential review: https://reviews.llvm.org/D74479

Added: 
    

Modified: 
    llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test
    llvm/tools/llvm-readobj/ELFDumper.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test b/llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test
index 32cccdd764b0..d72c43aa50ac 100644
--- a/llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test
+++ b/llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test
@@ -355,6 +355,9 @@ DynamicSymbols: []
 # 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
+
+# DYNSYM-SIZE-INVALID1: warning: '[[FILE]]': section with index 1 has invalid size (0x1) or entry size (0x10)
+
 ## 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
@@ -363,10 +366,20 @@ DynamicSymbols: []
 # 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){{$}}
 
+## c) In the case when the DT_SYMENT tag is present, we report when it's value does not match the
+#     value of the symbol size for the platform.
+# RUN: yaml2obj %s -D BITS=32 --docnum=12 -o %t12
+# RUN: llvm-readobj --dyn-symbols %t12 2>&1 | FileCheck %s -DFILE=%t12 --check-prefix=DYNSYM-SIZE-INVALID3
+# RUN: llvm-readelf --dyn-symbols %t12 2>&1 | FileCheck %s -DFILE=%t12 --check-prefix=DYNSYM-SIZE-INVALID3
+# RUN: yaml2obj %s -D BITS=64 --docnum=12 -o %t13
+# RUN: llvm-readobj --dyn-symbols %t13 2>&1 | FileCheck %s -DFILE=%t13 --check-prefix=DYNSYM-SIZE-INVALID4
+# RUN: llvm-readelf --dyn-symbols %t13 2>&1 | FileCheck %s -DFILE=%t13 --check-prefix=DYNSYM-SIZE-INVALID4
+
+# DYNSYM-SIZE-INVALID3: warning: '[[FILE]]': DT_SYMENT value of 0x123 is not the size of a symbol (0x10){{$}}
+# DYNSYM-SIZE-INVALID4: warning: '[[FILE]]': DT_SYMENT value of 0x123 is not the size of a symbol (0x18){{$}}
+
 --- !ELF
 FileHeader:
   Class:   ELFCLASS32
@@ -401,3 +414,18 @@ ProgramHeaders:
     VAddr: 0x100
     Sections:
       - Section: .dynsym
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS[[BITS]]
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:    .dynamic
+    Type:    SHT_DYNAMIC
+    Entries:
+      - Tag:   DT_SYMENT
+        Value: 0x123
+      - Tag:   DT_NULL
+        Value: 0

diff  --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index ec24c3ef706b..794c4630699a 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -2098,6 +2098,16 @@ void ELFDumper<ELFT>::parseDynamicTable(const ELFFile<ELFT> *Obj) {
       }
       break;
     }
+    case ELF::DT_SYMENT: {
+      uint64_t Val = Dyn.getVal();
+      if (Val != sizeof(Elf_Sym))
+        reportWarning(createError("DT_SYMENT value of 0x" +
+                                  Twine::utohexstr(Val) +
+                                  " is not the size of a symbol (0x" +
+                                  Twine::utohexstr(sizeof(Elf_Sym)) + ")"),
+                      ObjF->getFileName());
+      break;
+    }
     case ELF::DT_RELA:
       DynRelaRegion.Addr = toMappedAddr(Dyn.getTag(), Dyn.getPtr());
       break;