From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x32d.google.com (mail-ot1-x32d.google.com [IPv6:2607:f8b0:4864:20::32d]) by sourceware.org (Postfix) with ESMTPS id 656933858D32 for ; Mon, 9 May 2022 16:52:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 656933858D32 Received: by mail-ot1-x32d.google.com with SMTP id m15-20020a9d608f000000b00606a788887aso498748otj.0 for ; Mon, 09 May 2022 09:52:58 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=y0zOE9OkDW2s+jQdwWz/+KGco+Zxq2MurqoIpsbx+Mw=; b=vUpaUtgbKf/YZT0iXJhNdGVSKCoYMriGhFuL1WN/b+bcOJ3AbVAMYgOBLgb/gcXsgj /TSgSS/jvDtw5smowGquhpHJ/06kkghDyraxVCrA2l/T+L8kNH5aAUmdBDF46hQyuKX8 NMJOAJ0fIaThMOUSyaamT9HGWIrEPPL2CIzfAD5UCg9SR3LNPRgkf00rzFct71Pxv9Lr Ij4STQ0vp3xsCNL9zSxkQ8HHErIdgXRk047sq09Z6O6sWaaT43lRorL0AEopPYo8NIgC jiIElqeVAysmdIoTKfG27L7XdlMZqPBEcRAzVO5XZlJXDKVArbGGG17YlyOigFJTYFlD l/Qg== X-Gm-Message-State: AOAM530/7TfzFtGkpdEGarLNSN3Xl/BkTnabSqylhkzpW+lVPl6Ea4Xb OxSk/F3hYionEjWYzonO8Qxj/izhSTn4ag== X-Google-Smtp-Source: ABdhPJwOEKinCThkxz2FgLiHdVUTKuBFvpcpcZt0ny0eO+p7zPFicsjBPtRRY9evXL8LVgF4JF1b2g== X-Received: by 2002:a05:6830:1609:b0:606:685:27e6 with SMTP id g9-20020a056830160900b00606068527e6mr6051849otr.383.1652115177398; Mon, 09 May 2022 09:52:57 -0700 (PDT) Received: from ?IPV6:2804:431:c7ca:5fbd:82eb:6a4d:6bc4:fb11? ([2804:431:c7ca:5fbd:82eb:6a4d:6bc4:fb11]) by smtp.gmail.com with ESMTPSA id 14-20020a9d048e000000b006060322125dsm4926162otm.45.2022.05.09.09.52.55 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 09 May 2022 09:52:56 -0700 (PDT) Message-ID: Date: Mon, 9 May 2022 13:52:54 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH v4 3/3] csu: Implement and use _dl_early_allocate during static startup Content-Language: en-US To: Florian Weimer Cc: libc-alpha@sourceware.org References: <1b8e10100702d11449eb4ec02dbfbaa1db5d4c9f.1651762968.git.fweimer@redhat.com> <871qx2adqe.fsf@oldenburg.str.redhat.com> From: Adhemerval Zanella In-Reply-To: <871qx2adqe.fsf@oldenburg.str.redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE 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: Mon, 09 May 2022 16:53:00 -0000 On 09/05/2022 13:10, Florian Weimer wrote: > * Adhemerval Zanella: > >>> diff --git a/scripts/tst-elf-edit.py b/scripts/tst-elf-edit.py >>> index a514179bbf..0e19ce1e73 100644 >>> --- a/scripts/tst-elf-edit.py >>> +++ b/scripts/tst-elf-edit.py > >>> @@ -172,24 +181,35 @@ def elf_edit(f, align): >>> >>> ehdr = Elf_Ehdr(e_ident) >>> ehdr.read(f) >>> - if ehdr.e_type != ET_DYN: >>> - error('{}: not a shared library'.format(f.name)) >>> + if ehdr.e_type not in (ET_EXEC, ET_DYN): >>> + error('{}: not an executable or shared library'.format(f.name)) >>> >>> phdr = Elf_Phdr(e_ident) >>> + maximize_tls_size_done = False >>> for i in range(0, ehdr.e_phnum): >>> f.seek(ehdr.e_phoff + i * phdr.len) >>> phdr.read(f) >>> - if phdr.p_type == PT_LOAD: >>> - elf_edit_align(phdr, align) >>> + if phdr.p_type == PT_LOAD and opts.align is not None: >> >> I think you can omit the None check. > > I think this will raise an exception in elf_edit_align because the -a > argument is now optional. > >>> +void * >>> +_dl_early_allocate (size_t size) >>> +{ >>> + void *result; >>> + >>> + if (__curbrk != NULL) >>> + /* If the break has been initialized, brk must have run before, >>> + so just call it once more. */ >>> + { >>> + result = __sbrk (size); >>> + if (result == (void *) -1) >>> + result = NULL; >>> + } >>> + else >>> + { >>> + /* If brk has not been invoked, there is no need to update >>> + __curbrk. The first call to brk will take care of that. */ >>> + void *previous = (void *) INTERNAL_SYSCALL_CALL (brk, 0); >>> + result = (void *) INTERNAL_SYSCALL_CALL (brk, previous + size); >>> + if (result == previous) >>> + result = NULL; >>> + else >>> + result = previous; >> >> You will need to factor it be arch-specific since alpha return -ENOMEM >> in case of failure and sparc has different calling convention (similar >> to what it does for clone). Maybe add a >> >> static inline void * >> brk_call (void *addr) >> { >> void *r = syscall; >> r = check_error (r) ? -1 : 0; >> } >> >> And then refactor Linux brk.c versions to use brk_call as well. > > Thanks. I'm testing a new version with this change. > >>> + /* If brk fails, fall back to mmap. This can happen due to >>> + unfortunate ASLR layout decisions and kernel bugs, particularly >>> + for static PIE. */ >>> + if (result == NULL) >>> + { >>> + long int ret; >>> + int prot = PROT_READ | PROT_WRITE; >>> + int flags = MAP_PRIVATE | MAP_ANONYMOUS; >>> +#ifdef __NR_mmap2 >>> + ret = MMAP_CALL_INTERNAL (mmap2, 0, size, prot, flags, -1, 0); >>> +#else >>> + ret = MMAP_CALL_INTERNAL (mmap, 0, size, prot, flags, -1, 0); >>> +#endif >>> + if (INTERNAL_SYSCALL_ERROR_P (ret)) >> >> Maybe move it to mmap_call.h and make it a static inline: >> >> static inline void * >> mmap64_call (void *addr, size_t len, int prot, int flags, int fd, >> off64_t offset) >> { >> long int ret; >> #ifdef __NR_mmap2 >> ret = MMAP_CALL_INTERNAL (mmap2, addr, len, prot, flags, fd, >> (off_t) (offset / MMAP2_PAGE_UNIT)); >> #else >> ret = MMAP_CALL_INTERNAL (mmap, addr, len, prot, flags, fd, >> offset); >> #endif >> return INTERNAL_SYSCALL_ERROR_P (ret) ? NULL : (void *) ret; >> } >> >> static inline void * >> mmap_call_internal (size_t len) >> { >> int prot = PROT_READ | PROT_WRITE; >> int flags = MAP_PRIVATE | MAP_ANONYMOUS; >> return mmap64_call (0, len, PROT_READ | PROT_WRITE, >> MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); >> } > > We have several customization points for mmap and mmap2. > MMAP2_PAGE_UNIT can expand to a static global variable page_unit. This > is difficult to encapsulate properly in an inline function. Adding > __brk_call was simple enough, but this looks way more complex. The idea is to allow __mmap64 use the mmap64_call and have the syscall logic in one place. Something like: void * __mmap64 (void *addr, size_t len, int prot, int flags, int fd, off64_t offset) { MMAP_CHECK_PAGE_UNIT (); if (offset & MMAP_OFF_MASK) return (void *) INLINE_SYSCALL_ERROR_RETURN_VALUE (EINVAL); MMAP_PREPARE (addr, len, prot, flags, fd, offset); return mmap64_call (addr, len, prot, flags, fd, offset); }