[PATCH] D70961: [LLD][ELF] add support for PT_GNU_PROPERTY

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 3 10:24:13 PST 2019


MaskRay added a comment.

The code change looks good. I only get some nits about the tests.



================
Comment at: lld/test/ELF/aarch64-feature-bti.s:66
+# BTISO-NEXT:    10360: bti     c
+# BTISO-NEXT:    10364: stp     x16, x30, [sp, #-16]!
+# BTISO-NEXT:    10368: adrp    x16, #131072
----------------
Recently I think we can probably keep the address for the first instruction of a basic block and omit addresses for the rest of instructions.

```
10360: bti c
       stp x16, x30, [sp, #-16]!
       adrp    x16, #131072
```

Addresses of instructions that are neither the first instructions nor jump targets are not useful, and can make address updating annoying.


================
Comment at: lld/test/ELF/aarch64-pt-gnu-property.s:1
+# REQUIRES: aarch64
+# RUN: llvm-mc -filetype=obj -triple=aarch64-linux-gnu %s -o %t.o
----------------
grimar wrote:
> I think you can use x86 for this test.
Eventually we may need both. I think we also need a similar x86-64-pt-gnu-property.s. 


================
Comment at: lld/test/ELF/aarch64-pt-gnu-property.s:14
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Address: 0x200190
+# CHECK-NEXT:     Offset: 0x190
----------------
`Address: [[ADDR:0x.*]]`

Then below, `VirtualAddress: [[ADDR]]`


================
Comment at: lld/test/ELF/aarch64-pt-gnu-property.s:21
+
+# CHECK:          Type: PT_GNU_PROPERTY (0x6474E553)
+# CHECK-NEXT:     Offset: 0x190
----------------
We should probably also test PT_NOTE, which has the same offset/size.


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

https://reviews.llvm.org/D70961





More information about the llvm-commits mailing list