[PATCH] D70959: [ELF] Support for PT_GNU_PROPERTY in program headers and tools

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

The context is missing in the diff uploaded. A few comments from me are inlined.



================
Comment at: llvm/test/tools/llvm-objdump/elf-pt-gnu-property.test:1
+## Test that -p can print PT_GNU_PROPERTY
+
----------------
Seems most of the comments from elf-pt-gnu-property.test below can also be applied for this test case.


================
Comment at: llvm/test/tools/llvm-readobj/elf-pt-gnu-property.test:1
+## Test that we can print PT_GNU_PROPERTY
+
----------------
Missing a full stop.


================
Comment at: llvm/test/tools/llvm-readobj/elf-pt-gnu-property.test:4
+# RUN: yaml2obj %s > %t
+# RUN: llvm-readelf --program-headers %t | FileCheck %s --check-prefix=ELF
+# RUN: llvm-readobj --program-headers %t | FileCheck %s --check-prefix=OBJ
----------------
It is probably a bit more common to use "GNU" and "LLVM" prefixes in llvm-readobj/llvm-readelf tests.


================
Comment at: llvm/test/tools/llvm-readobj/elf-pt-gnu-property.test:18
+# OBJ-NEXT:     ]
+# OBJ-NEXT:     Alignment: 8
+
----------------
Please align properly (see test\tools\llvm-readobj\program-headers.test for example).


================
Comment at: llvm/test/tools/llvm-readobj/elf-pt-gnu-property.test:25
+  Type:            ET_EXEC
+  Machine:         EM_AARCH64
+Sections:
----------------
Please remove the excessive identations here and in other tests.
e.g:

```
  Class:   ELFCLASS64
  Data:    ELFDATA2LSB
  Type:    ET_EXEC
  Machine: EM_AARCH64
```


================
Comment at: llvm/test/tools/llvm-readobj/elf-pt-gnu-property.test:34
+      - Name:            GNU
+        Desc:            000000C0040000000300000000000000
+        Type:            0x00000005
----------------
This needs a comment about what is exactly encoded here.


================
Comment at: llvm/test/tools/llvm-readobj/elf-pt-gnu-property.test:41
+      - Section: .note.gnu.property
+...
----------------
You do not need the last line with dots.


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

https://reviews.llvm.org/D70959