Message ID | 20210413150319.764600-1-stefanha@redhat.com |
---|---|
State | Not applicable |
Headers |
Return-Path: <libc-alpha-bounces@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 4BBA73945C04; Tue, 13 Apr 2021 15:03:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4BBA73945C04 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1618326214; bh=Bt8EIkbFjk1rFc17+/fNBv57ahjZWmhXLRdzCsXZB1c=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=iIpdKFsStY1hO7EAFRrlTr3CoUuVmn5BaVeSo7s3olWO1lZ6LZ+d60LXOJV4hbkk+ zWUmA4LV9YkoUHgOLUzWaoXdISAy2PepV4lsJVytSupN41S/eIMd6WqR/Teh3mh7Xt FupMfMi7nF5rDeCsrRl7CSTvXSnbfoEG4zYTKlxA= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 2740D385702C for <libc-alpha@sourceware.org>; Tue, 13 Apr 2021 15:03:32 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 2740D385702C Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-385-9p7Ue2JWNDe_RjaGIagM_w-1; Tue, 13 Apr 2021 11:03:28 -0400 X-MC-Unique: 9p7Ue2JWNDe_RjaGIagM_w-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 9E5D41B2C984; Tue, 13 Apr 2021 15:03:27 +0000 (UTC) Received: from localhost (ovpn-115-75.ams2.redhat.com [10.36.115.75]) by smtp.corp.redhat.com (Postfix) with ESMTP id E02335D9DE; Tue, 13 Apr 2021 15:03:20 +0000 (UTC) To: linux-block@vger.kernel.org Subject: [PATCH liburing] examples/ucontext-cp.c: cope with variable SIGSTKSZ Date: Tue, 13 Apr 2021 16:03:19 +0100 Message-Id: <20210413150319.764600-1-stefanha@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: base64 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Spam-Status: No, score=-14.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list <libc-alpha.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=subscribe> From: Stefan Hajnoczi via Libc-alpha <libc-alpha@sourceware.org> Reply-To: Stefan Hajnoczi <stefanha@redhat.com> Cc: Jens Axboe <axboe@kernel.dk>, libc-alpha@sourceware.org, Stefan Hajnoczi <stefanha@redhat.com> Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" <libc-alpha-bounces@sourceware.org> |
Series |
[liburing] examples/ucontext-cp.c: cope with variable SIGSTKSZ
|
|
Commit Message
Stefan Hajnoczi
April 13, 2021, 3:03 p.m. UTC
The size of C arrays at file scope must be constant. The following
compiler error occurs with recent upstream glibc (2.33.9000):
CC ucontext-cp
ucontext-cp.c:31:23: error: variably modified ‘stack_buf’ at file scope
31 | unsigned char stack_buf[SIGSTKSZ];
| ^~~~~~~~~
make[1]: *** [Makefile:26: ucontext-cp] Error 1
The following glibc commit changed SIGSTKSZ from a constant value to a
variable:
commit 6c57d320484988e87e446e2e60ce42816bf51d53
Author: H.J. Lu <hjl.tools@gmail.com>
Date: Mon Feb 1 11:00:38 2021 -0800
sysconf: Add _SC_MINSIGSTKSZ/_SC_SIGSTKSZ [BZ #20305]
...
+# define SIGSTKSZ sysconf (_SC_SIGSTKSZ)
Allocate the stack buffer explicitly to avoid declaring an array at file
scope.
Cc: H.J. Lu <hjl.tools@gmail.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
Perhaps the glibc change needs to be revised before releasing glibc 2.34
since it might break applications. That's up to the glibc folks. It
doesn't hurt for liburing to take a safer approach that copes with the
SIGSTKSZ change in any case.
---
examples/ucontext-cp.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
Comments
On Tue, Apr 13, 2021 at 04:03:19PM +0100, Stefan Hajnoczi wrote: > The size of C arrays at file scope must be constant. The following > compiler error occurs with recent upstream glibc (2.33.9000): > > CC ucontext-cp > ucontext-cp.c:31:23: error: variably modified ‘stack_buf’ at file scope > 31 | unsigned char stack_buf[SIGSTKSZ]; > | ^~~~~~~~~ > make[1]: *** [Makefile:26: ucontext-cp] Error 1 > > The following glibc commit changed SIGSTKSZ from a constant value to a > variable: > > commit 6c57d320484988e87e446e2e60ce42816bf51d53 > Author: H.J. Lu <hjl.tools@gmail.com> > Date: Mon Feb 1 11:00:38 2021 -0800 > > sysconf: Add _SC_MINSIGSTKSZ/_SC_SIGSTKSZ [BZ #20305] > ... > +# define SIGSTKSZ sysconf (_SC_SIGSTKSZ) > > Allocate the stack buffer explicitly to avoid declaring an array at file > scope. > > Cc: H.J. Lu <hjl.tools@gmail.com> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > Perhaps the glibc change needs to be revised before releasing glibc 2.34 > since it might break applications. That's up to the glibc folks. It > doesn't hurt for liburing to take a safer approach that copes with the > SIGSTKSZ change in any case. glibc folks, please take a look. The commit referenced above broke compilation of liburing's tests. It's possible that applications will hit similar issues. Can you check whether the SIGSTKSZ change needs to be reverted/fixed before releasing glibc 2.34? Thanks, Stefan > --- > examples/ucontext-cp.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/examples/ucontext-cp.c b/examples/ucontext-cp.c > index 0b2a6b5..ea0c934 100644 > --- a/examples/ucontext-cp.c > +++ b/examples/ucontext-cp.c > @@ -28,7 +28,7 @@ > > typedef struct { > struct io_uring *ring; > - unsigned char stack_buf[SIGSTKSZ]; > + unsigned char *stack_buf; > ucontext_t ctx_main, ctx_fnew; > } async_context; > > @@ -115,8 +115,13 @@ static int setup_context(async_context *pctx, struct io_uring *ring) > perror("getcontext"); > return -1; > } > - pctx->ctx_fnew.uc_stack.ss_sp = &pctx->stack_buf; > - pctx->ctx_fnew.uc_stack.ss_size = sizeof(pctx->stack_buf); > + pctx->stack_buf = malloc(SIGSTKSZ); > + if (!pctx->stack_buf) { > + perror("malloc"); > + return -1; > + } > + pctx->ctx_fnew.uc_stack.ss_sp = pctx->stack_buf; > + pctx->ctx_fnew.uc_stack.ss_size = SIGSTKSZ; > pctx->ctx_fnew.uc_link = &pctx->ctx_main; > > return 0; > @@ -174,6 +179,7 @@ static void copy_file_wrapper(arguments_bundle *pbundle) > free(iov.iov_base); > close(pbundle->infd); > close(pbundle->outfd); > + free(pbundle->pctx->stack_buf); > free(pbundle->pctx); > free(pbundle); > > -- > 2.30.2 >
On Mon, Apr 19, 2021 at 7:35 AM Stefan Hajnoczi <stefanha@redhat.com> wrote: > > On Tue, Apr 13, 2021 at 04:03:19PM +0100, Stefan Hajnoczi wrote: > > The size of C arrays at file scope must be constant. The following > > compiler error occurs with recent upstream glibc (2.33.9000): > > > > CC ucontext-cp > > ucontext-cp.c:31:23: error: variably modified ‘stack_buf’ at file scope > > 31 | unsigned char stack_buf[SIGSTKSZ]; > > | ^~~~~~~~~ > > make[1]: *** [Makefile:26: ucontext-cp] Error 1 > > > > The following glibc commit changed SIGSTKSZ from a constant value to a > > variable: > > > > commit 6c57d320484988e87e446e2e60ce42816bf51d53 > > Author: H.J. Lu <hjl.tools@gmail.com> > > Date: Mon Feb 1 11:00:38 2021 -0800 > > > > sysconf: Add _SC_MINSIGSTKSZ/_SC_SIGSTKSZ [BZ #20305] > > ... > > +# define SIGSTKSZ sysconf (_SC_SIGSTKSZ) > > > > Allocate the stack buffer explicitly to avoid declaring an array at file > > scope. > > > > Cc: H.J. Lu <hjl.tools@gmail.com> > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > > --- > > Perhaps the glibc change needs to be revised before releasing glibc 2.34 > > since it might break applications. That's up to the glibc folks. It > > doesn't hurt for liburing to take a safer approach that copes with the > > SIGSTKSZ change in any case. > > glibc folks, please take a look. The commit referenced above broke > compilation of liburing's tests. It's possible that applications will > hit similar issues. Can you check whether the SIGSTKSZ change needs to > be reverted/fixed before releasing glibc 2.34? > It won't be changed for glibc 2.34.
On 4/19/21 7:34 AM, Stefan Hajnoczi via Libc-alpha wrote: > The commit referenced above broke > compilation of liburing's tests. It's possible that applications will > hit similar issues. Yes, other applications have already had similar issues, and we've fixed that by recoding them to not assume that SIGSTKSZ is an integer constant expression. See, for example, this patch to Gnulib last September: https://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=f9e2b20a12a230efa30f1d479563ae07d276a94b This patch appears in the latest GNU grep, for example.
On Mon, Apr 19, 2021 at 11:38:07AM -0700, H.J. Lu wrote: > On Mon, Apr 19, 2021 at 7:35 AM Stefan Hajnoczi <stefanha@redhat.com> wrote: > > > > On Tue, Apr 13, 2021 at 04:03:19PM +0100, Stefan Hajnoczi wrote: > > > The size of C arrays at file scope must be constant. The following > > > compiler error occurs with recent upstream glibc (2.33.9000): > > > > > > CC ucontext-cp > > > ucontext-cp.c:31:23: error: variably modified ‘stack_buf’ at file scope > > > 31 | unsigned char stack_buf[SIGSTKSZ]; > > > | ^~~~~~~~~ > > > make[1]: *** [Makefile:26: ucontext-cp] Error 1 > > > > > > The following glibc commit changed SIGSTKSZ from a constant value to a > > > variable: > > > > > > commit 6c57d320484988e87e446e2e60ce42816bf51d53 > > > Author: H.J. Lu <hjl.tools@gmail.com> > > > Date: Mon Feb 1 11:00:38 2021 -0800 > > > > > > sysconf: Add _SC_MINSIGSTKSZ/_SC_SIGSTKSZ [BZ #20305] > > > ... > > > +# define SIGSTKSZ sysconf (_SC_SIGSTKSZ) > > > > > > Allocate the stack buffer explicitly to avoid declaring an array at file > > > scope. > > > > > > Cc: H.J. Lu <hjl.tools@gmail.com> > > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > > > --- > > > Perhaps the glibc change needs to be revised before releasing glibc 2.34 > > > since it might break applications. That's up to the glibc folks. It > > > doesn't hurt for liburing to take a safer approach that copes with the > > > SIGSTKSZ change in any case. > > > > glibc folks, please take a look. The commit referenced above broke > > compilation of liburing's tests. It's possible that applications will > > hit similar issues. Can you check whether the SIGSTKSZ change needs to > > be reverted/fixed before releasing glibc 2.34? > > > > It won't be changed for glibc 2.34. Thanks for the response, H.J. and Paul. In that case liburing needs this patch. Stefan
+Cc: io-uring@vger.kernel.org +Cc: Pavel Begunkov <asml.silence@gmail.com> Original message: https://www.spinics.net/lists/linux-block/msg67077.html On Thu, Apr 22, 2021 at 10:59:42AM +0100, Stefan Hajnoczi wrote: >On Mon, Apr 19, 2021 at 11:38:07AM -0700, H.J. Lu wrote: >> On Mon, Apr 19, 2021 at 7:35 AM Stefan Hajnoczi <stefanha@redhat.com> wrote: >> > >> > On Tue, Apr 13, 2021 at 04:03:19PM +0100, Stefan Hajnoczi wrote: >> > > The size of C arrays at file scope must be constant. The following >> > > compiler error occurs with recent upstream glibc (2.33.9000): >> > > >> > > CC ucontext-cp >> > > ucontext-cp.c:31:23: error: variably modified ‘stack_buf’ at file scope >> > > 31 | unsigned char stack_buf[SIGSTKSZ]; >> > > | ^~~~~~~~~ >> > > make[1]: *** [Makefile:26: ucontext-cp] Error 1 >> > > >> > > The following glibc commit changed SIGSTKSZ from a constant value to a >> > > variable: >> > > >> > > commit 6c57d320484988e87e446e2e60ce42816bf51d53 >> > > Author: H.J. Lu <hjl.tools@gmail.com> >> > > Date: Mon Feb 1 11:00:38 2021 -0800 >> > > >> > > sysconf: Add _SC_MINSIGSTKSZ/_SC_SIGSTKSZ [BZ #20305] >> > > ... >> > > +# define SIGSTKSZ sysconf (_SC_SIGSTKSZ) >> > > >> > > Allocate the stack buffer explicitly to avoid declaring an array at file >> > > scope. >> > > >> > > Cc: H.J. Lu <hjl.tools@gmail.com> >> > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> >> > > --- >> > > Perhaps the glibc change needs to be revised before releasing glibc 2.34 >> > > since it might break applications. That's up to the glibc folks. It >> > > doesn't hurt for liburing to take a safer approach that copes with the >> > > SIGSTKSZ change in any case. >> > >> > glibc folks, please take a look. The commit referenced above broke >> > compilation of liburing's tests. It's possible that applications will >> > hit similar issues. Can you check whether the SIGSTKSZ change needs to >> > be reverted/fixed before releasing glibc 2.34? >> > >> >> It won't be changed for glibc 2.34. > >Thanks for the response, H.J. and Paul. > >In that case liburing needs this patch. > I think so: Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
On 4/22/21 3:22 PM, Stefano Garzarella wrote: > +Cc: io-uring@vger.kernel.org > +Cc: Pavel Begunkov <asml.silence@gmail.com> > > Original message: https://www.spinics.net/lists/linux-block/msg67077.html > > On Thu, Apr 22, 2021 at 10:59:42AM +0100, Stefan Hajnoczi wrote: >> On Mon, Apr 19, 2021 at 11:38:07AM -0700, H.J. Lu wrote: >>> On Mon, Apr 19, 2021 at 7:35 AM Stefan Hajnoczi <stefanha@redhat.com> wrote: >>> > >>> > On Tue, Apr 13, 2021 at 04:03:19PM +0100, Stefan Hajnoczi wrote: >>> > > The size of C arrays at file scope must be constant. The following >>> > > compiler error occurs with recent upstream glibc (2.33.9000): >>> > > >>> > > CC ucontext-cp >>> > > ucontext-cp.c:31:23: error: variably modified ‘stack_buf’ at file scope >>> > > 31 | unsigned char stack_buf[SIGSTKSZ]; >>> > > | ^~~~~~~~~ >>> > > make[1]: *** [Makefile:26: ucontext-cp] Error 1 >>> > > >>> > > The following glibc commit changed SIGSTKSZ from a constant value to a >>> > > variable: >>> > > >>> > > commit 6c57d320484988e87e446e2e60ce42816bf51d53 >>> > > Author: H.J. Lu <hjl.tools@gmail.com> >>> > > Date: Mon Feb 1 11:00:38 2021 -0800 >>> > > >>> > > sysconf: Add _SC_MINSIGSTKSZ/_SC_SIGSTKSZ [BZ #20305] >>> > > ... >>> > > +# define SIGSTKSZ sysconf (_SC_SIGSTKSZ) >>> > > >>> > > Allocate the stack buffer explicitly to avoid declaring an array at file >>> > > scope. >>> > > >>> > > Cc: H.J. Lu <hjl.tools@gmail.com> >>> > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> >>> > > --- >>> > > Perhaps the glibc change needs to be revised before releasing glibc 2.34 >>> > > since it might break applications. That's up to the glibc folks. It >>> > > doesn't hurt for liburing to take a safer approach that copes with the >>> > > SIGSTKSZ change in any case. >>> > >>> > glibc folks, please take a look. The commit referenced above broke >>> > compilation of liburing's tests. It's possible that applications will >>> > hit similar issues. Can you check whether the SIGSTKSZ change needs to >>> > be reverted/fixed before releasing glibc 2.34? >>> > >>> >>> It won't be changed for glibc 2.34. >> >> Thanks for the response, H.J. and Paul. >> >> In that case liburing needs this patch. >> > > I think so: > > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Right, and there are already people complaining https://github.com/axboe/liburing/issues/320
diff --git a/examples/ucontext-cp.c b/examples/ucontext-cp.c index 0b2a6b5..ea0c934 100644 --- a/examples/ucontext-cp.c +++ b/examples/ucontext-cp.c @@ -28,7 +28,7 @@ typedef struct { struct io_uring *ring; - unsigned char stack_buf[SIGSTKSZ]; + unsigned char *stack_buf; ucontext_t ctx_main, ctx_fnew; } async_context; @@ -115,8 +115,13 @@ static int setup_context(async_context *pctx, struct io_uring *ring) perror("getcontext"); return -1; } - pctx->ctx_fnew.uc_stack.ss_sp = &pctx->stack_buf; - pctx->ctx_fnew.uc_stack.ss_size = sizeof(pctx->stack_buf); + pctx->stack_buf = malloc(SIGSTKSZ); + if (!pctx->stack_buf) { + perror("malloc"); + return -1; + } + pctx->ctx_fnew.uc_stack.ss_sp = pctx->stack_buf; + pctx->ctx_fnew.uc_stack.ss_size = SIGSTKSZ; pctx->ctx_fnew.uc_link = &pctx->ctx_main; return 0; @@ -174,6 +179,7 @@ static void copy_file_wrapper(arguments_bundle *pbundle) free(iov.iov_base); close(pbundle->infd); close(pbundle->outfd); + free(pbundle->pctx->stack_buf); free(pbundle->pctx); free(pbundle);