From 4158722a6cff5d19e228356c525946b6c4b83396 Mon Sep 17 00:00:00 2001 From: Sylvain Henry Date: Tue, 7 Mar 2023 11:42:20 +0100 Subject: linker: fix linking with aligned sections (#23066) Take section alignment into account instead of assuming 16 bytes (which is wrong when the section requires 32 bytes, cf #23066). --- rts/linker/Elf.c | 14 ++++++----- testsuite/tests/rts/linker/Makefile | 5 ++++ testsuite/tests/rts/linker/T23066.stdout | 2 ++ testsuite/tests/rts/linker/T23066_c.c | 42 ++++++++++++++++++++++++++++++++ testsuite/tests/rts/linker/all.T | 8 ++++++ 5 files changed, 65 insertions(+), 6 deletions(-) create mode 100644 testsuite/tests/rts/linker/T23066.stdout create mode 100644 testsuite/tests/rts/linker/T23066_c.c diff --git a/rts/linker/Elf.c b/rts/linker/Elf.c index 3595a4c3d4..040107c7f1 100644 --- a/rts/linker/Elf.c +++ b/rts/linker/Elf.c @@ -872,12 +872,14 @@ ocGetNames_ELF ( ObjectCode* oc ) else if (!oc->imageMapped || size < getPageSize() / 3) { bool executable = kind == SECTIONKIND_CODE_OR_RODATA; m32_allocator *allocator = executable ? oc->rx_m32 : oc->rw_m32; - // align on 16 bytes. The reason being that llvm will emit see - // paddq statements for x86_64 under optimisation and load from - // RODATA sections. Specifically .rodata.cst16. However we don't - // handle the cst part in any way what so ever, so 16 seems - // better than 8. - start = m32_alloc(allocator, size, 16); + // Correctly align the section. This is particularly important for + // the alignment of .rodata.cstNN sections. + // + // llvm will emit see paddq statements for x86_64 under + // optimisation and load from RODATA sections, specifically + // .rodata.cst16. Also we may encounter .rodata.cst32 sections + // in objects using AVX instructions (see #23066). + start = m32_alloc(allocator, size, align); if (start == NULL) goto fail; memcpy(start, oc->image + offset, size); alloc = SECTION_M32; diff --git a/testsuite/tests/rts/linker/Makefile b/testsuite/tests/rts/linker/Makefile index cc3e118243..c8e113c88c 100644 --- a/testsuite/tests/rts/linker/Makefile +++ b/testsuite/tests/rts/linker/Makefile @@ -12,6 +12,11 @@ section_alignment: '$(TEST_HC)' $(TEST_HC_OPTS_NO_RTSOPTS) -v0 --make -no-rtsopts-suggestions -no-hs-main -o runner runner.c ./runner section_alignment.o isAligned +T23066: + '$(TEST_CC)' $(TEST_CC_OPTS) -c -o T23066_c.o T23066_c.c + '$(TEST_HC)' $(TEST_HC_OPTS_NO_RTSOPTS) -v0 --make -no-rtsopts-suggestions -no-hs-main -o runner runner.c -static + ./runner T23066_c.o isAligned + T2615-prep: $(RM) libfoo_T2615.so '$(TEST_HC)' $(TEST_HC_OPTS) -fPIC -c libfoo_T2615.c -o libfoo_T2615.o diff --git a/testsuite/tests/rts/linker/T23066.stdout b/testsuite/tests/rts/linker/T23066.stdout new file mode 100644 index 0000000000..e33c5c1e49 --- /dev/null +++ b/testsuite/tests/rts/linker/T23066.stdout @@ -0,0 +1,2 @@ +Linking: path = T23066_c.o, symname = isAligned +1 diff --git a/testsuite/tests/rts/linker/T23066_c.c b/testsuite/tests/rts/linker/T23066_c.c new file mode 100644 index 0000000000..cd754597c5 --- /dev/null +++ b/testsuite/tests/rts/linker/T23066_c.c @@ -0,0 +1,42 @@ +#include +#include + +extern int foo32_1, foo32_2; + +// The bug in #23066 was that we wouldn't correctly align 32-bytes aligned +// sections, except by chance (we were always aligning on 16 bytes). +// +// Hence we intersperse two 16-bytes aligned sections with two 32-bytes aligned +// sections to ensure that at least one of the 32-bytes aligned section +// triggers the bug (the order of the sections seems to be preserved). + +__asm__( +" .section pad16_1,\"aM\",@progbits,16\n\t" +" .align 16\n\t" +" .byte 0\n\t" +"\n\t" +" .global foo32_1\n\t" +" .section sfoo32_1,\"aM\",@progbits,32\n\t" +" .align 32\n\t" +"foo32_1:\n\t" +" .byte 0\n\t" +"\n\t" +" .section pad16_2,\"aM\",@progbits,16\n\t" +" .align 16\n\t" +" .byte 0\n\t" +"\n\t" +" .global foo32_2\n\t" +" .section sfoo32_2,\"aM\",@progbits,32\n\t" +" .align 32\n\t" +"foo32_2:\n\t" +" .byte 0\n\t" +); + + +#define ALIGN32(x) (((intptr_t)(&x) & 0x1F) == 0) + +int isAligned() { + //printf("%p\n", &foo32_1); + //printf("%p\n", &foo32_2); + return (ALIGN32(foo32_1) && ALIGN32(foo32_2)); +} diff --git a/testsuite/tests/rts/linker/all.T b/testsuite/tests/rts/linker/all.T index bd51b3ed95..ba17069fd5 100644 --- a/testsuite/tests/rts/linker/all.T +++ b/testsuite/tests/rts/linker/all.T @@ -16,6 +16,14 @@ test('section_alignment', ], makefile_test, []) +###################################### +test('T23066', + [ unless(arch('x86_64'), skip) + , unless(opsys('linux'), skip) + , extra_files(['runner.c', 'T23066_c.c']) + ], + makefile_test, []) + ###################################### # Test to see if linker scripts link properly to real ELF files test('T2615', -- cgit v1.2.1