From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-4.1 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI, SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 2BC721F4B4 for ; Wed, 7 Oct 2020 17:43:41 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1C995386EC62; Wed, 7 Oct 2020 17:43:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1C995386EC62 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1602092620; bh=rr40AY17+JN/ccHdnYDYpYoE+EL3oWtfHYYLIZ6ISKI=; h=References:In-Reply-To:Date:Subject:To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=xJLCPaJSIWXoKlIK0etjWHIV17F4hl0hiasaYgfHdMfseQRQFuOzaw+YY9FqRJd+9 dtfW8CvQ9xC0eUzpZqfmBbUl5k4R925SMiNO003heQsq2SLtiaKDd/TOYQFptqwvts UWFNOdE5SrFjYRQovX5de1i5f+IKPt+g5wHLLPNc= Received: from mail-oo1-xc43.google.com (mail-oo1-xc43.google.com [IPv6:2607:f8b0:4864:20::c43]) by sourceware.org (Postfix) with ESMTPS id 666893858D35 for ; Wed, 7 Oct 2020 17:43:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 666893858D35 Received: by mail-oo1-xc43.google.com with SMTP id f19so562363oot.4 for ; Wed, 07 Oct 2020 10:43:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=rr40AY17+JN/ccHdnYDYpYoE+EL3oWtfHYYLIZ6ISKI=; b=JBQa5bJ/W1elGgjOdjMHTOKimtzjcO0Q9h0g/sDu7xAX78YeovSdzdJuTq9yUeW2kl T6IohF5ezA3Waqobz2PNr8zlW8bpi90maOww+Yh/f1jrWnTc7MbP38s4xN6ss4HxA9XE j4lh6KVkpHiWenbq//WoDRhGobzKmjm0lFAKkwGSAqlYOny0AGMB8pKlg3ObufKGGZzK yjN0BfgmaPVJ30mLjqONnWB5GLxw9vhrmbOjD7TuhclyekGkRHBvl5Lv7G3YLHeQS0c2 8in+kyl5p4MYeJLrAcObGDVGZRXzY40oLJsBD25vTsuWTCKSf6Bncl22kc/aOLV84+yO SsuQ== X-Gm-Message-State: AOAM532Wh8MbBU2nyCWxSgAGQdPJMljIVffSW5cfJLlMT9fxC8wPIDjp 1QAgLtk6d1Vnv/aDuEI8CYwxK55fpZRiWPCCAB4= X-Google-Smtp-Source: ABdhPJzWMVzGqR5SxyUc86nVDzuxbj/uZppqxB/TRkQD4b0cE2CbMgcvq5u2gDDqnO15qQxtBZjZImNvNE0ffWyvl2Q= X-Received: by 2002:a4a:d40c:: with SMTP id n12mr2801524oos.35.1602092616732; Wed, 07 Oct 2020 10:43:36 -0700 (PDT) MIME-Version: 1.0 References: <20201005134534.GT6642@arm.com> <20201006092532.GU6642@arm.com> <20201006152553.GY6642@arm.com> <20201006165520.GA6642@arm.com> <20201007104720.GH6642@arm.com> <20201007154559.GI6642@arm.com> In-Reply-To: <20201007154559.GI6642@arm.com> Date: Wed, 7 Oct 2020 10:43:00 -0700 Message-ID: Subject: Re: [RFC PATCH 0/4] x86: Improve Minimum Alternate Stack Size To: Dave Martin Content-Type: text/plain; charset="UTF-8" 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: , From: "H.J. Lu via Libc-alpha" Reply-To: "H.J. Lu" Cc: linux-arch , Len Brown , Tony Luck , GNU C Library , "Ravi V. Shankar" , "Chang S. Bae" , the arch/x86 maintainers , LKML , Dave Hansen , Andy Lutomirski , Linux API , Thomas Gleixner , Borislav Petkov , Ingo Molnar Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" On Wed, Oct 7, 2020 at 8:46 AM Dave Martin wrote: > > On Wed, Oct 07, 2020 at 06:30:03AM -0700, H.J. Lu wrote: > > On Wed, Oct 7, 2020 at 3:47 AM Dave Martin wrote: > > > > > > On Tue, Oct 06, 2020 at 10:44:14AM -0700, H.J. Lu wrote: > > [...] > > > > > I updated my glibc patch to add both _SC_MINSIGSTKSZ and _SC_SIGSTKSZ. > > > > _SC_MINSIGSTKSZ is the minimum signal stack size from AT_MINSIGSTKSZ, > > > > which is the signal frame size used by kernel, and _SC_SIGSTKSZ is the value > > > > of sysconf (_SC_MINSIGSTKSZ) + 6KB for user application. > > > > > > Can I suggest sysconf (_SC_MINSIGSTKSZ) * 4 instead? > > > > Done. > > OK. I was prepared to have to argue my case a bit more, but if you > think this is OK then I will stop arguing ;) > > > > > If the arch has more or bigger registers to save in the signal frame, > > > the chances are that they're going to get saved in some userspace stack > > > frames too. So I suspect that the user signal handler stack usage may > > > scale up to some extent rather than being a constant. > > > > > > > > > To help people migrate without unpleasant surprises, I also figured it > > > would be a good idea to make sure that sysconf (_SC_MINSIGSTKSZ) >= > > > legacy MINSIGSTKSZ, and sysconf (_SC_SIGSTKSZ) >= legacy SIGSTKSZ. > > > This should makes it safer to use sysconf (_SC_MINSIGSTKSZ) as a > > > drop-in replacement for MINSIGSTKSZ, etc. > > > > > > (To explain: AT_MINSIGSTKSZ may actually be < MINSIGSTKSZ on AArch64. > > > My idea was that sysconf () should hide this surprise, but people who > > > really want to know the kernel value would tolerate some > > > nonportability and read AT_MINSIGSTKSZ directly.) > > > > > > > > > So then: > > > > > > kernel_minsigstksz = getauxval(AT_MINSIGSTKSZ); > > > minsigstksz = LEGACY_MINSIGSTKSZ; > > > if (kernel_minsigstksz > minsigstksz) > > > minsistksz = kernel_minsigstksz; > > > > > > sigstksz = LEGACY_SIGSTKSZ; > > > if (minsigstksz * 4 > sigstksz) > > > sigstksz = minsigstksz * 4; > > > > I updated users/hjl/AT_MINSIGSTKSZ branch with > > > > +@item _SC_MINSIGSTKSZ > > +@standards{GNU, unistd.h} > > Can we specify these more agressively now? > > > +Inquire about the signal stack size used by the kernel. > > I think we've already concluded that this should included all mandatory > overheads, including those imposed by the compiler and glibc? > > e.g.: > > --8<-- > > The returned value is the minimum number of bytes of free stack space > required in order to gurantee successful, non-nested handling of a > single signal whose handler is an empty function. > > -->8-- > > > + > > +@item _SC_SIGSTKSZ > > +@standards{GNU, unistd.h} > > +Inquire about the default signal stack size for a signal handler. > > Similarly: > > --8<-- > > The returned value is the suggested minimum number of bytes of stack > space required for a signal stack. > > This is not guaranteed to be enough for any specific purpose other than > the invocation of a single, non-nested, empty handler, but nonetheless > should be enough for basic scenarios involving simple signal handlers > and very low levels of signal nesting (say, 2 or 3 levels at the very > most). > > This value is provided for developer convenience and to ease migration > from the legacy SIGSTKSZ constant. Programs requiring stronger > guarantees should avoid using it if at all possible. > > -->8-- Done. > > If these descriptions are too wordy, we might want to move some of it > out to signal.texi, though. > > > > > case _SC_MINSIGSTKSZ: > > assert (GLRO(dl_minsigstacksize) != 0); > > return GLRO(dl_minsigstacksize); > > > > case _SC_SIGSTKSZ: > > { > > /* Return MAX (MINSIGSTKSZ, sysconf (_SC_MINSIGSTKSZ)) * 4. */ > > long int minsigstacksize = GLRO(dl_minsigstacksize); > > _Static_assert (__builtin_constant_p (MINSIGSTKSZ), > > "MINSIGSTKSZ is constant"); > > if (minsigstacksize < MINSIGSTKSZ) > > minsigstacksize = MINSIGSTKSZ; > > return minsigstacksize * 4; > > } > > > > > > > > (Or something like that, unless the architecture provides its own > > > definitions. ia64's MINSIGSTKSZ is enormous, so it probably doesn't > > > want this.) > > > > > > > > > Also: should all these values be rounded up to a multiple of the > > > architecture's preferred stack alignment? > > > > Kernel should provide a properly aligned AT_MINSIGSTKSZ. > > OK. Can you comment on Chang S. Bae's series? I wasn't sure that the > proposal produces an aligned value for AT_MINSIGSTKSZ on x86, but maybe > I just worked it out wrong. In Linux kernel, the signal frame data is composed of the following areas and laid out as: ------------------------------ | alignment padding | ------------------------------ | xsave buffer | <<<<<<< This must be 64 byte aligned ------------------------------ | fsave header (32-bit only) | ------------------------------ | siginfo + ucontext | ------------------------------ In order to get the proper alignment for xsave buffer, the signal stake size should be rounded up to 64 byte aligned. In glibc, I have /* Size of xsave buffer. */ unsigned int sigframe_size = ebx; if (64 bit) /* NB: sizeof(struct rt_sigframe) in Linux kernel. */ sigframe_size += 440; else /* NB: sizeof(struct sigframe_ia32) + sizeof(struct fregs_state)) in Linux kernel. */ sigframe_size += 736 + 112; endif /* Round up to 64-byte aligned. */ sigframe_size = ALIGN_UP (sigframe_size, 64); GLRO(dl_minsigstacksize) = sigframe_size; Kernel should do something similar. > > > > Should the preferred stack alignment also be exposed through sysconf? > > > Portable code otherwise has no way to know this, though if the > > > preferred alignment is <= the minimum malloc()/alloca() alignment then > > > this is generally not an issue.) > > > > Good question. But it is orthogonal to the signal stack size issue. > > Ack. > > There are various other brokennesses around this area, such as the fact > that the register data may now run off the end of the mcontext_t object > that is supposed to contain it. Ideally we should fork ucontext_t or > mcontext_t into two types, one for the ucontext API and one for the > signal API (which are anyway highly incompatible with each other). > > If this stuff is addressed, I guess we could bundle it under a more > general feature test macro. But it's probably best to nail down this > series in something like its current form first. Agreed. > > I'll follow up on libc-alpha with a wishlist, but that will be a > separate conversation... > > [...] > > Cheers > ---Dave -- H.J.