From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x102d.google.com (mail-pj1-x102d.google.com [IPv6:2607:f8b0:4864:20::102d]) by sourceware.org (Postfix) with ESMTPS id AB1F53858C5E for ; Thu, 23 Feb 2023 13:54:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org AB1F53858C5E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pj1-x102d.google.com with SMTP id q31-20020a17090a17a200b0023750b69614so4541894pja.5 for ; Thu, 23 Feb 2023 05:54:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=/o5U50GXE3qJD1vQlsqhjsdQPHgmtrYu79CMZahu10c=; b=bDM5axVrNgwWHVCHD14lE08laFlOV9Y5rSsBOd1j1bIuFv8f9XHIu3DYCg6clG5w3Q 7NRd5BIB3XrHyPTh6uyjSL4qa6Oe4szZnFn6Hh9OykIaAPgeM/5WhFG0Pw1pNzig2gH1 L61gJz1Ui8UMieLTcocPIOQCN8AlFAQuzLMOQSpXa2nBkuvKfxMmD8XkeKSyqXu9W1cV pu/drTfej02p6rxtzyn5QhmsUWa+afa0KG++WcvaCZ6m9YaUkWxTH+Jw1s8zX8AAPYd0 pwoX8yhp9mDUuf01DmE6WNQF2gpQV5zCGZ7g6pa3VdPyew9aDh9y59vPqeFQ3BmWq6kr GLLA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=/o5U50GXE3qJD1vQlsqhjsdQPHgmtrYu79CMZahu10c=; b=BYtc+RhctDaiti2V+N7xVFhwUgxNyLLVzoWgD8Q60yKwT70JI8JlAp+U39Yi411cH+ pCHfuQQJI5PurXM6NThMNbYLDDN+RcukRr4X4/uAG1bKnrqMKGiggXD1fzU8C8WJ0WWZ 5WuTkP77dgcSieFT48bDknYBAtaFYEMVNXjxRRosEdUWvEeoMA1N9AMBmm/jOvZBgvkB IZiMqI7hfl/Q8Geizb6/NvEO9HTwdD+0Umjg+ZHvlIIaIUBYJW8juejC2vpNH+dDNoXL xaxZgHb1/pecdVfaDtsGbiuankPdNiROo57MoqtxjPlY+iuYxne0sSMj7UA17E4Q7ura fmwg== X-Gm-Message-State: AO0yUKU8ETCs7gl6XBwq1nhY0hOT0ssBYbQY+aHwG9aMc7Kb/3LkSNN1 MbgvdbcwcpSbDa38ZT9X/g7hmj2u0sFrMbCHWUVqw1O/y7TRlw== X-Google-Smtp-Source: AK7set8f327t8WRZYajm3E8KsS1gh5kGLYaMv7cA/yrZS4IPRHaYuJbXsaYRgW04nl7woA08HG4L8FCQUCcJ0+SCwWk= X-Received: by 2002:a17:90b:3eca:b0:235:1fb7:3939 with SMTP id rm10-20020a17090b3eca00b002351fb73939mr671336pjb.2.1677160456491; Thu, 23 Feb 2023 05:54:16 -0800 (PST) MIME-Version: 1.0 References: <20230221211932.296459-1-bugaevc@gmail.com> <20230221211932.296459-2-bugaevc@gmail.com> <20230222232646.sc3l7rec2wpalpxy@begin> In-Reply-To: <20230222232646.sc3l7rec2wpalpxy@begin> From: Sergey Bugaev Date: Thu, 23 Feb 2023 16:54:05 +0300 Message-ID: Subject: Re: [PATCH v2 1/4] hurd: Simplify init-first.c further To: Samuel Thibault Cc: bug-hurd@gnu.org, libc-alpha@sourceware.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-8.6 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.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Thu, Feb 23, 2023 at 2:26 AM Samuel Thibault w= rote: > Did you try to run make check? No. I'm cross-compiling, so I don't think make check would be able to run any tests. And from what I remember from building glibc on the Hurd itself back in 2021, make check takes a very long time and either never really completes or brings the system into some weird state. If you're able to run make check on your end, please do so (but wait until I send v3 with the changes you've requested below). Are there specific tests for the various combinations of startup variants? (shared vs static, args already on the stack vs not, exec server present vs not) If so, maybe I could run just them and not the full thing; that would be easier on my system. > Sergey Bugaev via Libc-alpha, le mer. 22 f=C3=A9vr. 2023 00:19:29 +0300, = a ecrit: > > This drops all of the return address rewriting kludges. The only > > remaining hack is the jump out of a call stack while adjusting the > > stack pointer. > > Is this hack really still needed? Since we don't switch stack any more, > we could as well just return normally? No, we can't just return normally, we have to adjust the stack pointer and jump out. We don't _switch_ the stack as in use an entirely different area of memory for the stack, but we do adjust the stack pointer within the existing stack area. Are the comments I have added not clear enough about this? If so, maybe they should be expanded further. The reason we have to return in this weird way is that: 1. The normal startup code (_start1) expects to find args/env on the top of the stack. Like, literally, argc at sp[0], argv[0] at sp[1] and so on. 2. _hurd_startup, which is the code that receives args/env from the exec server, allocates them in its own stack frame, with alloca. So it cannot return, and neither can _hurd_stack_setup. Instead of returning, _hurd_startup invokes a callback (doinit) that (eventually) just sets the stack pointer to point to this data (so it now is on the top of the stack, just as _start1 expects) and jumps to _hurd_stack_setup's caller (i.e. to _start). This would very much break things if _start was itself using the stack for anything, since the stack pointer is now different, but the only thing _start does is it calls _hurd_startup and then jumps to _start1, so that works. And the same is done in the SHARED startup path by _dl_sysdep_start, except that it uses the RETURN_TO macro instead of direct inline assembly. That macro only really differs in that it does *not* zero out ebp/rbp, so I wonder how come that doesn't break backtraces. > > --- a/sysdeps/mach/hurd/dl-sysdep.c > > +++ b/sysdeps/mach/hurd/dl-sysdep.c > > @@ -207,6 +207,9 @@ _dl_sysdep_start (void **start_argptr, > > } > > } > > > > + extern void _dl_init_first (void *data); > > Please put extern function declaration into a header, dl-sysdep.h for > instance. Makes sense, thanks. > > > diff --git a/sysdeps/mach/hurd/i386/init-first.c b/sysdeps/mach/hurd/i3= 86/init-first.c > > index a558da16..34e8dcc0 100644 > > --- a/sysdeps/mach/hurd/i386/init-first.c > > +++ b/sysdeps/mach/hurd/i386/init-first.c > > + { > > + /* Check if the stack we are now on is different from > > + the one described by _hurd_stack_{base,size}. */ > > > > + char dummy; > > + const vm_address_t newsp =3D (vm_address_t) &dummy; > > + > > + if (d->stack_size !=3D 0 && (newsp < d->stack_base > > + || newsp - d->stack_base > d->stack_size)) > > + /* The new stack pointer does not intersect with the > > + stack the exec server set up for us, so free that stack. */ > > + __vm_deallocate (__mach_task_self (), d->stack_base, d->stack_si= ze); > > + } > > Again, I don't think this is needed any more since we don't switch stack > any more? Good point, most likely not. I'll drop it and see if anything breaks. Sergey