[LLD][ELF] .note.gnu.property sections should have wordsize alignment

[peter.smith]

Summary

The .note.gnu.property SHT_NOTE sections on AArch64 (a 64-bit target) should have alignment 8 to more closely match the binutils implementation where alignment is 4-bytes on 32-bit machines and 8-bytes on 64-bit machines.

Previously LLD was using 4 for both 32-bit and 64-bit machines.

There was a long discussion on the right alignment of the .note.gnu.property section on the binutils mailing list. The basic argument was that generic ELF requires 8-byte alignment for SHT_NOTES sections, however this hadn't been respected by other GNU notes sections. The implementation in GNU ld uses 8 and as there is no binary legacy of using 4 in LLD (BTI is only just being picked up and used) I'd like to keep LLD in line with GNU ld.

Although at present LLD only does something useful with AArch64 properties, this also applies to X86.

Depends on D70961 as it updates a test added there.

[peter.smith]

Updated to add context.

[grimar]

lld/test/ELF/aarch64-gnu-property-align.s

1 ↗

(On Diff #231898)

Seems you can use x86 here too?

lld/test/ELF/aarch64-pt-gnu-property.s

19 ↗

(On Diff #231898)

Do we have a test for !config->is64?

[MaskRay]

[LLD][ELF][AArch64] .note.gnu.property sections should have alignment 8

Mention x86-64 or just delete [AArch64]. I think all 64-bit architectures use 8 (though currently only x86-64 and AArch64 define architecture specific GNU_PROPERTY_*). The code in binutils-gdb is architecture-agnostic bfd/elf-properties.c:_bfd_elf_convert_gnu_properties

bed = get_elf_backend_data (obfd);
align_shift = bed->s->elfclass == ELFCLASS64 ? 3 : 2;

/* Get the output .note.gnu.property section size.  */
size = bfd_section_size (isec->output_section);

/* Update the output .note.gnu.property section alignment.  */
bfd_set_section_alignment (isec->output_section, align_shift);

lld/ELF/SyntheticSections.cpp

302

config->wordsize

lld/test/ELF/aarch64-pt-gnu-property.s

19 ↗

(On Diff #231898)

A !config->is64 test will need GNU_PROPERTY_X86_* on EM_386. There is no GNU_PROPERTY_ARM_*.

[peter.smith]

Thanks both for the comments. Updated to use config->wordsize, converted test to x86_64 and added 32-bit x86 test for the alignment 4 case.

[MaskRay]

lld/test/ELF/gnu-property-align.s

34

There is no GNU_PROPERTY_X86_FEATURE_1_AND...

@grimar I don't mind keeping this as REQUIRES: aarch64. I think an aarch64 test for 64-bit and an x86 test for 32-bit should be sufficient.

[MaskRay]

lld/test/ELF/gnu-property-align.s

34

Sorry, GNU_PROPERTY_X86_FEATURE_1_AND exists. This looks fine.

[grimar]

LGTM.

lld/test/ELF/gnu-property-align.s

34

I think an aarch64 test for 64-bit and an x86 test for 32-bit should be sufficient.

One of the reasons to use x86 for testing generic features is build bots. x86 is the most popular target,
which is enabled everywhere (or almost everywhere). x86 BB are also faster than others, in case of the breakage
they usually report the issue first.

It is also hard to imagine that for local development x86 target is disabled. Most of LLD tests rely on x86.
While aarch64 can be off.

lld/ELF/SyntheticSections.cpp

 //
 // lld reads the sections from input files and merges them by computing AND of
 // the flags. The result is written as a new .note.gnu.property section.
 //
 // If the flag is zero (which indicates that the intersection of the feature
 // sets is empty, or some input files didn't have .note.gnu.property sections),
 // we don't create this section.
 GnuPropertySection::GnuPropertySection()
-    : SyntheticSection(llvm::ELF::SHF_ALLOC, llvm::ELF::SHT_NOTE, 4,
-                       ".note.gnu.property") {}
+    : SyntheticSection(llvm::ELF::SHF_ALLOC, llvm::ELF::SHT_NOTE,
+                       config->wordsize, ".note.gnu.property") {}
 
 void GnuPropertySection::writeTo(uint8_t *buf) {
   uint32_t featureAndType = config->emachine == EM_AARCH64
                                 ? GNU_PROPERTY_AARCH64_FEATURE_1_AND
                                 : GNU_PROPERTY_X86_FEATURE_1_AND;
 
   write32(buf, 4);                                   // Name size
   write32(buf + 4, config->is64 ? 16 : 12);          // Content size

lld/test/ELF/gnu-property-align-32.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=i686-linux-gnu %s -o %t.o
+# RUN: ld.lld %t.o -o %t
+# RUN: llvm-readobj --sections -n %t | FileCheck %s
+
+## Check that .note.gnu.property has alignment 4 and is readable by llvm-readobj
+
+# CHECK: Name: .note.gnu.property
+# CHECK-NEXT: Type: SHT_NOTE (0x7)
+# CHECK-NEXT: Flags [ (0x2)
+# CHECK-NEXT:   SHF_ALLOC (0x2)
+# CHECK-NEXT: ]
+# CHECK-NEXT: Address: 0x4000F4
+# CHECK-NEXT: Offset: 0xF4
+# CHECK-NEXT: Size: 28
+# CHECK-NEXT: Link: 0
+# CHECK-NEXT: Info: 0
+# CHECK-NEXT: AddressAlignment: 4
+
+# CHECK:      Note {
+# CHECK-NEXT:   Owner: GNU
+# CHECK-NEXT:   Data size: 0xC
+# CHECK-NEXT:   Type: NT_GNU_PROPERTY_TYPE_0 (property note)
+# CHECK-NEXT:   Property [
+# CHECK-NEXT:     x86 feature: IBT
+
+.section ".note.gnu.property", "a"
+.p2align 2
+.long 4
+.long 0xc
+.long 0x5
+.asciz "GNU"
+.p2align 2
+.long 0xc0000002 # GNU_PROPERTY_X86_FEATURE_1_AND
+.long 4
+.long 1          # GNU_PROPERTY_X86_FEATURE_1_IBT
+
+.text
+.globl _start
+ ret

lld/test/ELF/gnu-property-align.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-linux-gnu %s -o %t.o
 # RUN: ld.lld %t.o -o %t
-# RUN: llvm-readobj --sections --program-headers %t | FileCheck %s
+# RUN: llvm-readobj --sections -n %t | FileCheck %s
 
-## Test that we generate the PT_GNU_PROPERTY segment type that describes the
-## .note.gnu.property if it is present.
+## Check that .note.gnu.property has alignment 8 and is readable by llvm-readobj
 
 # CHECK:      Name: .note.gnu.property
 # CHECK-NEXT: Type: SHT_NOTE (0x7)
 # CHECK-NEXT: Flags [ (0x2)
 # CHECK-NEXT:   SHF_ALLOC (0x2)
 # CHECK-NEXT: ]
 # CHECK-NEXT: Address: 0x200190
 # CHECK-NEXT: Offset: 0x190
 # CHECK-NEXT: Size: 32
 # CHECK-NEXT: Link: 0
 # CHECK-NEXT: Info: 0
-# CHECK-NEXT: AddressAlignment: 4
+# CHECK-NEXT: AddressAlignment: 8
+
+# CHECK:      Note {
+# CHECK-NEXT:   Owner: GNU
+# CHECK-NEXT:   Data size: 0x10
+# CHECK-NEXT:   Type: NT_GNU_PROPERTY_TYPE_0 (property note)
+# CHECK-NEXT:   Property [
+# CHECK-NEXT:     x86 feature: IBT
 
-# CHECK:      Type: PT_GNU_PROPERTY (0x6474E553)
-# CHECK-NEXT: Offset: 0x190
-# CHECK-NEXT: VirtualAddress: 0x200190
-# CHECK-NEXT: PhysicalAddress: 0x200190
-# CHECK-NEXT: FileSize: 32
-# CHECK-NEXT: MemSize: 32
-# CHECK-NEXT: Flags [ (0x4)
-# CHECK-NEXT:   PF_R (0x4)
-# CHECK-NEXT: ]
-# CHECK-NEXT: Alignment: 4
 
 .section ".note.gnu.property", "a"
 .long 4
 .long 0x10
 .long 0x5
 .asciz "GNU"
 
 .long 0xc0000002 # GNU_PROPERTY_X86_FEATURE_1_AND
 .long 4
 .long 1          # GNU_PROPERTY_X86_FEATURE_1_IBT
 .long 0
 
  .text
  .globl _start
- ret
+ .type _start, %function
+_start: ret

lld/test/ELF/pt-gnu-property.s

 # CHECK-NEXT: Flags [ (0x2)
 # CHECK-NEXT:   SHF_ALLOC (0x2)
 # CHECK-NEXT: ]
 # CHECK-NEXT: Address: 0x200190
 # CHECK-NEXT: Offset: 0x190
 # CHECK-NEXT: Size: 32
 # CHECK-NEXT: Link: 0
 # CHECK-NEXT: Info: 0
-# CHECK-NEXT: AddressAlignment: 4
+# CHECK-NEXT: AddressAlignment: 8
 
 # CHECK:      Type: PT_GNU_PROPERTY (0x6474E553)
 # CHECK-NEXT: Offset: 0x190
 # CHECK-NEXT: VirtualAddress: 0x200190
 # CHECK-NEXT: PhysicalAddress: 0x200190
 # CHECK-NEXT: FileSize: 32
 # CHECK-NEXT: MemSize: 32
 # CHECK-NEXT: Flags [ (0x4)
 # CHECK-NEXT:   PF_R (0x4)
 # CHECK-NEXT: ]
-# CHECK-NEXT: Alignment: 4
+# CHECK-NEXT: Alignment: 8
 
 .section ".note.gnu.property", "a"
 .long 4
 .long 0x10
 .long 0x5
 .asciz "GNU"
 
 .long 0xc0000002 # GNU_PROPERTY_X86_FEATURE_1_AND
 .long 4
 .long 1          # GNU_PROPERTY_X86_FEATURE_1_IBT
 .long 0
 
 .text
 .globl _start
  ret