[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