From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x62f.google.com (mail-pl1-x62f.google.com [IPv6:2607:f8b0:4864:20::62f]) by sourceware.org (Postfix) with ESMTPS id 05FC73858402 for ; Thu, 9 Dec 2021 15:14:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 05FC73858402 Received: by mail-pl1-x62f.google.com with SMTP id u17so4091133plg.9 for ; Thu, 09 Dec 2021 07:14:46 -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=2hAirzsv/rbwLFPcg7wnAOgTNEEeV8l0zbp5u+I3akA=; b=nwlD3fInFQdN7SqFd2+QmJ9Ai5PeunJRNbpXc6vF0VUmUvsgrC5+007vDAWS7UkD/F A2k90qlNkHaAALygOCjrw9dYAEO10LBGpoyJASxd/m5jST4B8OJ+KBWa+rrZ3wJuLdpb HB1H4wY8Cysi53ZJfVjNlXSUgc0h/60aweah63L0BK5ya079pniKLnBdXL0wLO3YSpir vOvRFm29pTyLtdI5azntrvBwI9qauYx9f2LUE3Ix2HENJssiA2iQMdO03TP0xDwqyKzn v4vA2+4CyFJiMav4krK5Na1TPxdtfYSIx4M/BhF06ymqq6xw5BVXpZcB/uUMFD10QOuj nrhw== X-Gm-Message-State: AOAM5307WuTn3OfL4okXr7v/nwglU3PVqwQBoMVBb+HdaoYAzH2sOV0r I4bcnFuobMbpXhwLPXLiuqzMm5k8UtmGzwaThiX93zcFnKE= X-Google-Smtp-Source: ABdhPJxpR0ygcDQqAz73AhyaSdrBoB7m32N2SbfIIXoqRpnDfh1gUD7PL7xSj2TCj7gOer956mwVI5NYUPNzrBYU7Fg= X-Received: by 2002:a17:90b:3b82:: with SMTP id pc2mr16459258pjb.120.1639062886024; Thu, 09 Dec 2021 07:14:46 -0800 (PST) MIME-Version: 1.0 References: <20211209055719.56245-1-rongwei.wang@linux.alibaba.com> <20211209055719.56245-2-rongwei.wang@linux.alibaba.com> In-Reply-To: <20211209055719.56245-2-rongwei.wang@linux.alibaba.com> From: "H.J. Lu" Date: Thu, 9 Dec 2021 07:14:10 -0800 Message-ID: Subject: Re: [PATCH v2 1/1] elf: align the mapping address of LOAD segments with p_align To: 20211204045848.71105-1-rongwei.wang@linux.alibaba.com Cc: GNU C Library , Florian Weimer , xuyu@linux.alibaba.com, gavin.dg@linux.alibaba.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3029.2 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: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Dec 2021 15:14:49 -0000 On Wed, Dec 8, 2021 at 9:57 PM Rongwei Wang wrote: > > Now, ld.so always map the LOAD segments and aligned by base > page size (e.g. 4k in x86 or 4k, 16k and 64k in arm64). This > is a bug, and had been reported: > > https://sourceware.org/bugzilla/show_bug.cgi?id=28676 > > This patch mainly to fix it. In this patch, ld.so can align > the mapping address of the first LOAD segment with p_align > when p_align is greater than the current base page size. > > A testcase: > main.c: > > extern void dso_test(void); > int main(void) > { > dso_test(); > getchar(); > > return 0; > } > > load.c, used to generate libload.so: > > int foo __attribute__((aligned(0x200000))) = 1; > void dso_test(void) > { > printf("dso test\n"); > printf("foo: %p\n", &foo); > } > > The steps: > $ gcc -O2 -fPIC -c -o load.o load.c > $ gcc -shared -Wl,-z,max-page-size=0x200000 -o libload.so load.o > $ gcc -no-pie -Wl,-z,max-page-size=0x200000 -O2 -o dso main.c libload.so -Wl,-R,. > > Before fixing: > $ ./dso > dso test > foo: 0xffff88ae2000 > > After fixed: > $ ./dso > dso test > foo: 0xffff9e000000 > > And this fix can help code segments use huge pages become > simple and available. Please include a testcase, like https://gitlab.com/x86-glibc/glibc/-/commits/users/hjl/pr28676/master > Signed-off-by: Xu Yu > Signed-off-by: Rongwei Wang > --- > elf/dl-load.c | 1 + > elf/dl-map-segments.h | 63 +++++++++++++++++++++++++++++++++++++++---- > include/link.h | 3 +++ > 3 files changed, 62 insertions(+), 5 deletions(-) > > diff --git a/elf/dl-load.c b/elf/dl-load.c > index e39980fb19..136cfe2fa8 100644 > --- a/elf/dl-load.c > +++ b/elf/dl-load.c > @@ -1154,6 +1154,7 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd, > c->dataend = ph->p_vaddr + ph->p_filesz; > c->allocend = ph->p_vaddr + ph->p_memsz; > c->mapoff = ALIGN_DOWN (ph->p_offset, GLRO(dl_pagesize)); > + l->l_load_align = ph->p_align; Can you add an alignment field to /* This structure describes one PT_LOAD command. Its details have been expanded out and converted. */ struct loadcmd { ElfW(Addr) mapstart, mapend, dataend, allocend; ElfW(Off) mapoff; int prot; /* PROT_* bits. */ }; instead? > > /* Determine whether there is a gap between the last segment > and this one. */ > diff --git a/elf/dl-map-segments.h b/elf/dl-map-segments.h > index ac9f09ab4c..fad98eb984 100644 > --- a/elf/dl-map-segments.h > +++ b/elf/dl-map-segments.h > @@ -18,6 +18,48 @@ > > #include > > +static __always_inline void * > +_dl_map_segments_align (const struct loadcmd *c, > + ElfW(Addr) mappref, int fd, size_t alignment, > + const size_t maplength) > +{ > + unsigned long map_start, map_start_align, map_end; Use ElfW(Addr) instead of long. > + unsigned long maplen = (maplength >= alignment) ? > + (maplength + alignment) : (2 * alignment); > + > + /* Allocate enough space to ensure that address aligned by > + p_align is included. */ > + map_start = (ElfW(Addr)) __mmap ((void *) mappref, maplen, > + PROT_NONE, > + MAP_ANONYMOUS | MAP_PRIVATE, > + -1, 0); > + if (__glibc_unlikely ((void *) map_start == MAP_FAILED)) > + { > + /* If mapping a aligned address failed, then ... */ an aligned > + map_start = (ElfW(Addr)) __mmap ((void *) mappref, maplength, > + c->prot, > + MAP_COPY|MAP_FILE, > + fd, c->mapoff); > + > + return (void *) map_start; > + } > + map_start_align = ALIGN_UP(map_start, alignment); > + map_end = map_start_align + maplength; > + > + /* Remember which part of the address space this object uses. */ > + map_start_align = (ElfW(Addr)) __mmap ((void *) map_start_align, maplength, > + c->prot, > + MAP_COPY|MAP_FILE|MAP_FIXED, > + fd, c->mapoff); > + if (__glibc_unlikely ((void *) map_start_align == MAP_FAILED)) > + return MAP_FAILED; > + if (map_start_align > map_start) > + __munmap((void *)map_start, map_start_align - map_start); > + __munmap((void *)map_end, map_start + maplen - map_end); > + > + return (void *) map_start_align; > +} > + > /* This implementation assumes (as does the corresponding implementation > of _dl_unmap_segments, in dl-unmap-segments.h) that shared objects > are always laid out with all segments contiguous (or with gaps > @@ -52,11 +94,22 @@ _dl_map_segments (struct link_map *l, int fd, > c->mapstart & GLRO(dl_use_load_bias)) > - MAP_BASE_ADDR (l)); > > - /* Remember which part of the address space this object uses. */ > - l->l_map_start = (ElfW(Addr)) __mmap ((void *) mappref, maplength, > - c->prot, > - MAP_COPY|MAP_FILE, > - fd, c->mapoff); > + /* During mapping, align the mapping address of the LOAD segments > + according to own p_align. This helps OS map its code segment to > + huge pages. */ > + if (l->l_load_align > GLRO(dl_pagesize)) > + { > + l->l_map_start = (ElfW(Addr)) _dl_map_segments_align (c, mappref, > + fd, l->l_load_align, maplength); > + } > + else > + { > + /* Remember which part of the address space this object uses. */ > + l->l_map_start = (ElfW(Addr)) __mmap ((void *) mappref, maplength, > + c->prot, > + MAP_COPY|MAP_FILE, > + fd, c->mapoff); > + } Don't use {} for a single statement. > if (__glibc_unlikely ((void *) l->l_map_start == MAP_FAILED)) > return DL_MAP_SEGMENTS_ERROR_MAP_SEGMENT; > > diff --git a/include/link.h b/include/link.h > index aea268439c..fc6ce29fab 100644 > --- a/include/link.h > +++ b/include/link.h > @@ -298,6 +298,9 @@ struct link_map > > /* Thread-local storage related info. */ > > + /* Alignment requirement of the LOAD block. */ > + size_t l_load_align; > + > /* Start of the initialization image. */ > void *l_tls_initimage; > /* Size of the initialization image. */ > -- > 2.27.0 > -- H.J.