| Message ID | 20210413150319.764600-1-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [liburing] examples/ucontext-cp.c: cope with variable SIGSTKSZ | expand |
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);
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(-)