From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1034.google.com (mail-pj1-x1034.google.com [IPv6:2607:f8b0:4864:20::1034]) by sourceware.org (Postfix) with ESMTPS id 83C143858C60 for ; Mon, 20 Dec 2021 15:51:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 83C143858C60 Received: by mail-pj1-x1034.google.com with SMTP id a11-20020a17090a854b00b001b11aae38d6so267593pjw.2 for ; Mon, 20 Dec 2021 07:51:08 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=qFNSwCUUuLv6nx6AFQeVLFU0789TDZCtquMRmrEwDxI=; b=JROdX2rAxnHd4+oqm+6EaDJ3/cToll++nD8G42zT7qgJ6+ffSNavrPdMiQt8HoIAuR Hj+K1KNjLSh1/1hzJZD0uL+EzgAsbRVhMClEl1zf7LI4/kHt+8rL+IYEl1ejAYwOc4x3 z8A4YPCMxiZTckKv7aruRbMv3LRWq3JNzHTBXyK2ozjTygOVfwhz50en6AJfPgJoqmXs KMloCLko4JHFAwzCc2pOEo84zLr9hi9UuFhc4+qZdy5kpWqcLqVdFurNek8DvBJvqVDs xBwHyTWpsP+zZDkr2Rf8iIQz8Z/7t6qqFndqbCGfxRdAnyEuCYuOPn1QWF5HVPKLyvpk U34Q== X-Gm-Message-State: AOAM533VsKwCEB4GWqikVZ3BQgZdXm+u7JQaepKv84Lv9wzz2lEOnATj +R/ku0w9+fYEuyJ5K6OLnf45l8d1l9rpiTG7qYZPLhdq X-Google-Smtp-Source: ABdhPJxnUJqFtDtcGFcbm0MmBu6z+tsO3erek0laQlkXKID4Sm2XtJ04ygcuoyp1t18tu96Az+eJX42rGWX0kEwgZyo= X-Received: by 2002:a17:902:7406:b0:148:a2af:4abd with SMTP id g6-20020a170902740600b00148a2af4abdmr17778449pll.4.1640015467206; Mon, 20 Dec 2021 07:51:07 -0800 (PST) MIME-Version: 1.0 References: <20211215163232.1787673-1-hjl.tools@gmail.com> In-Reply-To: <20211215163232.1787673-1-hjl.tools@gmail.com> From: "H.J. Lu" Date: Mon, 20 Dec 2021 07:50:30 -0800 Message-ID: Subject: Re: [PATCH] elf: Set p_align to the common page size if possible To: Binutils , Alan Modra , Nick Clifton Cc: Florian Weimer Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3028.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 20 Dec 2021 15:51:11 -0000 On Wed, Dec 15, 2021 at 8:32 AM H.J. Lu wrote: > > Currently, on 32-bit and 64-bit ARM, it seems that ld generates p_align > values of 0x10000 even if no section alignment is greater than 0x1000. > The issue is more general and probably affects other targets with > multiple common page sizes. > > While file layout absolutely must take 64K page size into account, that > does not have to be reflected in the p_align value. If running on a 64K > kernel, the file will be loaded at a 64K page boundary by necessity. On > a 4K kernel, 64K alignment is not needed. > > The glibc loader has been fixed to honor p_align: > > https://sourceware.org/bugzilla/show_bug.cgi?id=28676 > > similar to kernel: > > commit ce81bb256a224259ab686742a6284930cbe4f1fa > Author: Chris Kennelly > Date: Thu Oct 15 20:12:32 2020 -0700 > > fs/binfmt_elf: use PT_LOAD p_align values for suitable start address > > This means that on 4K kernels, we will start to do extra work for 64K > p_align, but this pointless for pretty much all binaries (whose section > alignment rarely exceeds 16). > > 1. Set p_align to common page size while laying out segments aligning to > maximum page size or section alignment. The run-time loader can align > segments to common page size or maximum page size, depending on system > page size. > 2. If -z max-page-size=NNN is used, p_align will be set to maximum page > size or the largest section alignment. > 3. If a section requires alignment higher than the common page size, > don't set p_align to the common page size. > 4. If a section requires alignment higher than the maximum page size, > set p_align to the section alignment. > 5. For objcopy, when the commone page size != the maximum page size, > p_align may be set to the commone page size while segments are aligned > to the maximum page size. In this case, the input p_align will be > ignored and the maximum page size will be used to align the ouput > segments. > 6. Update linker to disallow the commone page size > the maximum page > size. > > bfd/ > > PR ld/28689 > PR ld/28695 > * elf.c (assign_file_positions_for_load_sections): Set p_align > to common page size while laying out segments aligning to > maximum page size or section alignment. > (elf_is_p_align_valid): New function. > (copy_elf_program_header): Call elf_is_p_align_valid to determine > if p_align is valid. > > include/ > > PR ld/28689 > PR ld/28695 > * bfdlink.h (bfd_link_info): Add maxpagesize_is_set. > > ld/ > > PR ld/28689 > PR ld/28695 > * emultempl/elf.em (gld${EMULATION_NAME}_handle_option): Set > link_info.maxpagesize_is_set for -z max-page-size=NNN. > * ldelf.c (ldelf_after_parse): Disallow link_info.commonpagesize > > link_info.maxpagesize. > * testsuite/ld-elf/elf.exp: Pass -z max-page-size=0x4000 to > linker to build mbind2a and mbind2b. > * testsuite/ld-elf/header.d: Add -z common-page-size=0x100. > * testsuite/ld-elf/linux-x86.exp: Add PR ld/28689 tests. > * testsuite/ld-elf/p_align-1.c: New file. > * testsuite/ld-elf/page-size-1.d: New test. > --- > bfd/elf.c | 78 +++++++++++++++++++++++++++++-- > include/bfdlink.h | 3 ++ > ld/emultempl/elf.em | 1 + > ld/ldelf.c | 3 ++ > ld/testsuite/ld-elf/elf.exp | 4 +- > ld/testsuite/ld-elf/header.d | 2 +- > ld/testsuite/ld-elf/linux-x86.exp | 36 ++++++++++++++ > ld/testsuite/ld-elf/p_align-1.c | 25 ++++++++++ > ld/testsuite/ld-elf/page-size-1.d | 4 ++ > 9 files changed, 149 insertions(+), 7 deletions(-) > create mode 100644 ld/testsuite/ld-elf/p_align-1.c > create mode 100644 ld/testsuite/ld-elf/page-size-1.d > > diff --git a/bfd/elf.c b/bfd/elf.c > index e6c6a8a6c05..431084760ab 100644 > --- a/bfd/elf.c > +++ b/bfd/elf.c > @@ -5406,6 +5406,8 @@ assign_file_positions_for_load_sections (bfd *abfd, > Elf_Internal_Phdr *p; > file_ptr off; /* Octets. */ > bfd_size_type maxpagesize; > + bfd_size_type commonpagesize; > + bool p_align_commonpagesize_p = false; > unsigned int alloc, actual; > unsigned int i, j; > struct elf_segment_map **sorted_seg_map; > @@ -5491,12 +5493,19 @@ assign_file_positions_for_load_sections (bfd *abfd, > elf_sort_segments); > > maxpagesize = 1; > + commonpagesize = 1; > if ((abfd->flags & D_PAGED) != 0) > { > if (link_info != NULL) > - maxpagesize = link_info->maxpagesize; > + { > + maxpagesize = link_info->maxpagesize; > + commonpagesize = link_info->commonpagesize; > + } > else > - maxpagesize = bed->maxpagesize; > + { > + maxpagesize = bed->maxpagesize; > + commonpagesize = bed->commonpagesize; > + } > } > > /* Sections must map to file offsets past the ELF file header. */ > @@ -5560,6 +5569,13 @@ assign_file_positions_for_load_sections (bfd *abfd, > segment. */ > if (m->p_align_valid) > maxpagesize = m->p_align; > + else if (link_info == NULL || !link_info->maxpagesize_is_set) > + /* Set p_align to common page size while laying out segments > + aligning to the maximum page size or the largest section > + alignment. The run-time loader can align segments to the > + common page size or the maximum page size, depending on > + system page size. */ > + p_align_commonpagesize_p = true; > > p->p_align = maxpagesize; > } > @@ -5597,7 +5613,22 @@ assign_file_positions_for_load_sections (bfd *abfd, > } > align = (bfd_size_type) 1 << align_power; > if (align < maxpagesize) > - align = maxpagesize; > + { > + /* If a section requires alignment higher than the > + common page size, don't set p_align to the common > + page size. */ > + if (align > commonpagesize) > + p_align_commonpagesize_p = false; > + align = maxpagesize; > + } > + else > + { > + /* If a section requires alignment higher than the > + maximum page size, set p_align to the section > + alignment. */ > + p_align_commonpagesize_p = true; > + commonpagesize = align; > + } > } > > for (i = 0; i < m->count; i++) > @@ -5976,6 +6007,9 @@ assign_file_positions_for_load_sections (bfd *abfd, > print_segment_map (m); > } > } > + > + if (p_align_commonpagesize_p) > + p->p_align = commonpagesize; > } > } > > @@ -7485,6 +7519,39 @@ rewrite_elf_program_header (bfd *ibfd, bfd *obfd, bfd_vma maxpagesize) > return true; > } > > +/* Return true if p_align in the ELF program header in ABFD is valid. */ > + > +static bool > +elf_is_p_align_valid (bfd *abfd) > +{ > + unsigned int i; > + Elf_Internal_Phdr *segment; > + unsigned int num_segments; > + const struct elf_backend_data *bed = get_elf_backend_data (abfd); > + bfd_size_type maxpagesize = bed->maxpagesize; > + bfd_size_type commonpagesize = bed->commonpagesize; > + > + if (commonpagesize == maxpagesize) > + return true; > + > + /* When the commone page size != the maximum page size, p_align may > + be set to the commone page size while segments are aligned to > + the maximum page size. In this case, the input p_align will be > + ignored and the maximum page size will be used to align the ouput > + segments. */ > + segment = elf_tdata (abfd)->phdr; > + num_segments = elf_elfheader (abfd)->e_phnum; > + for (i = 0; i < num_segments; i++, segment++) > + if (segment->p_type == PT_LOAD > + && (segment->p_align != commonpagesize > + || vma_page_aligned_bias (segment->p_vaddr, > + segment->p_offset, > + maxpagesize) != 0)) > + return true; > + > + return false; > +} > + > /* Copy ELF program header information. */ > > static bool > @@ -7499,6 +7566,7 @@ copy_elf_program_header (bfd *ibfd, bfd *obfd) > unsigned int num_segments; > bool phdr_included = false; > bool p_paddr_valid; > + bool p_palign_valid; > unsigned int opb = bfd_octets_per_byte (ibfd, NULL); > > iehdr = elf_elfheader (ibfd); > @@ -7519,6 +7587,8 @@ copy_elf_program_header (bfd *ibfd, bfd *obfd) > break; > } > > + p_palign_valid = elf_is_p_align_valid (ibfd); > + > for (i = 0, segment = elf_tdata (ibfd)->phdr; > i < num_segments; > i++, segment++) > @@ -7561,7 +7631,7 @@ copy_elf_program_header (bfd *ibfd, bfd *obfd) > map->p_paddr = segment->p_paddr; > map->p_paddr_valid = p_paddr_valid; > map->p_align = segment->p_align; > - map->p_align_valid = 1; > + map->p_align_valid = p_palign_valid; > map->p_vaddr_offset = 0; > > if (map->p_type == PT_GNU_RELRO > diff --git a/include/bfdlink.h b/include/bfdlink.h > index 566529ee644..8cf34b05c1a 100644 > --- a/include/bfdlink.h > +++ b/include/bfdlink.h > @@ -525,6 +525,9 @@ struct bfd_link_info > /* TRUE if all symbol names should be unique. */ > unsigned int unique_symbol : 1; > > + /* TRUE if maxpagesize is set on command-line. */ > + unsigned int maxpagesize_is_set : 1; > + > /* Char that may appear as the first char of a symbol, but should be > skipped (like symbol_leading_char) when looking up symbols in > wrap_hash. Used by PowerPC Linux for 'dot' symbols. */ > diff --git a/ld/emultempl/elf.em b/ld/emultempl/elf.em > index bfaf8130a3e..5eadab9f4b9 100644 > --- a/ld/emultempl/elf.em > +++ b/ld/emultempl/elf.em > @@ -721,6 +721,7 @@ fragment < || (link_info.maxpagesize & (link_info.maxpagesize - 1)) != 0) > einfo (_("%F%P: invalid maximum page size \`%s'\n"), > optarg + 14); > + link_info.maxpagesize_is_set = true; > } > else if (startswith (optarg, "common-page-size=")) > { > diff --git a/ld/ldelf.c b/ld/ldelf.c > index 529992b02ae..ee09df5f35c 100644 > --- a/ld/ldelf.c > +++ b/ld/ldelf.c > @@ -72,6 +72,9 @@ ldelf_after_parse (void) > link_info.dynamic_undefined_weak = 0; > } > after_parse_default (); > + if (link_info.commonpagesize > link_info.maxpagesize) > + einfo (_("%F%P: common page size (0x%v) > maximum page size (0x%v)\n"), > + link_info.commonpagesize, link_info.maxpagesize); > } > > /* Handle the generation of DT_NEEDED tags. */ > diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp > index 01d22faad9a..ae8f76db1c7 100644 > --- a/ld/testsuite/ld-elf/elf.exp > +++ b/ld/testsuite/ld-elf/elf.exp > @@ -365,7 +365,7 @@ if { [istarget *-*-linux*] > run_ld_link_exec_tests [list \ > [list \ > "Run mbind2a" \ > - "$NOPIE_LDFLAGS -Wl,-z,common-page-size=0x4000" \ > + "$NOPIE_LDFLAGS -Wl,-z,common-page-size=0x4000,-z,max-page-size=0x4000" \ > "" \ > { mbind2a.s mbind2b.c } \ > "mbind2a" \ > @@ -374,7 +374,7 @@ if { [istarget *-*-linux*] > ] \ > [list \ > "Run mbind2b" \ > - "-static -Wl,-z,common-page-size=0x4000" \ > + "-static -Wl,-z,common-page-size=0x4000,-z,max-page-size=0x4000" \ > "" \ > { mbind2a.s mbind2b.c } \ > "mbind2b" \ > diff --git a/ld/testsuite/ld-elf/header.d b/ld/testsuite/ld-elf/header.d > index c4d174a98da..67f0c981920 100644 > --- a/ld/testsuite/ld-elf/header.d > +++ b/ld/testsuite/ld-elf/header.d > @@ -1,5 +1,5 @@ > # target: *-*-linux* *-*-gnu* *-*-vxworks arm*-*-uclinuxfdpiceabi > -# ld: -T header.t -z max-page-size=0x100 > +# ld: -T header.t -z max-page-size=0x100 -z common-page-size=0x100 > # objdump: -hpw > > #... > diff --git a/ld/testsuite/ld-elf/linux-x86.exp b/ld/testsuite/ld-elf/linux-x86.exp > index ee03b565faf..25a8d0411d9 100644 > --- a/ld/testsuite/ld-elf/linux-x86.exp > +++ b/ld/testsuite/ld-elf/linux-x86.exp > @@ -185,6 +185,42 @@ run_ld_link_exec_tests [list \ > "" \ > "tmpdir/indirect-extern-access-2.so" \ > ] \ > + [list \ > + "Run p_align-1a without PIE" \ > + "$NOPIE_LDFLAGS" \ > + "" \ > + { p_align-1.c } \ > + "p_align-1a" \ > + "pass.out" \ > + "$NOPIE_CFLAGS" \ > + ] \ > + [list \ > + "Run p_align-1b with PIE" \ > + "-pie" \ > + "" \ > + { p_align-1.c } \ > + "p_align-1b" \ > + "pass.out" \ > + "-fpie" \ > + ] \ > + [list \ > + "Run p_align-1c with -Wl,-z,max-page-size=0x1000 without PIE" \ > + "$NOPIE_LDFLAGS -Wl,-z,max-page-size=0x1000" \ > + "" \ > + { p_align-1.c } \ > + "p_align-1c" \ > + "pass.out" \ > + "$NOPIE_CFLAGS" \ > + ] \ > + [list \ > + "Run p_align-1d with -Wl,-z,max-page-size=0x1000 with PIE" \ > + "-pie -Wl,-z,max-page-size=0x1000" \ > + "" \ > + { p_align-1.c } \ > + "p_align-1d" \ > + "pass.out" \ > + "-fpie" \ > + ] \ > ] > > proc elfedit_test { options test output } { > diff --git a/ld/testsuite/ld-elf/p_align-1.c b/ld/testsuite/ld-elf/p_align-1.c > new file mode 100644 > index 00000000000..6579cd74e52 > --- /dev/null > +++ b/ld/testsuite/ld-elf/p_align-1.c > @@ -0,0 +1,25 @@ > +#include > +#include > +#include > + > +#ifndef ALIGN > +# define ALIGN 0x800000 > +#endif > + > +int > +__attribute__ ((weak)) > +is_aligned (void *p, int align) > +{ > + return (((uintptr_t) p) & (align - 1)) == 0; > +} > + > +int foo __attribute__ ((aligned (ALIGN))) = 1; > + > +int > +main (void) > +{ > + if (!is_aligned (&foo, ALIGN)) > + abort (); > + printf ("PASS\n"); > + return 0; > +} > diff --git a/ld/testsuite/ld-elf/page-size-1.d b/ld/testsuite/ld-elf/page-size-1.d > new file mode 100644 > index 00000000000..04d2153b2f9 > --- /dev/null > +++ b/ld/testsuite/ld-elf/page-size-1.d > @@ -0,0 +1,4 @@ > +#source: dummy.s > +#ld: -z common-page-size=0x4000 -z max-page-size=0x1000 > +#error: common page size \(0x4000\) > maximum page size \(0x1000\) > +#target: *-*-linux-gnu *-*-gnu* arm*-*-uclinuxfdpiceabi > -- > 2.33.1 > Hi Nick, Alan, Can you review this patch? Thanks. -- H.J.