Linux-Block Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH liburing] examples/ucontext-cp.c: cope with variable SIGSTKSZ
@ 2021-04-13 15:03 Stefan Hajnoczi
  2021-04-19 14:34 ` Stefan Hajnoczi
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Hajnoczi @ 2021-04-13 15:03 UTC (permalink / raw)
  To: linux-block; +Cc: libc-alpha, Jens Axboe, Stefan Hajnoczi, H . J . Lu

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(-)

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH liburing] examples/ucontext-cp.c: cope with variable SIGSTKSZ
  2021-04-13 15:03 [PATCH liburing] examples/ucontext-cp.c: cope with variable SIGSTKSZ Stefan Hajnoczi
@ 2021-04-19 14:34 ` Stefan Hajnoczi
  2021-04-19 18:38   ` H.J. Lu
  2021-04-20  0:05   ` Paul Eggert
  0 siblings, 2 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2021-04-19 14:34 UTC (permalink / raw)
  To: libc-alpha, H . J . Lu; +Cc: Jens Axboe, linux-block


[-- Attachment #1: Type: text/plain, Size: 2836 bytes --]

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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH liburing] examples/ucontext-cp.c: cope with variable SIGSTKSZ
  2021-04-19 14:34 ` Stefan Hajnoczi
@ 2021-04-19 18:38   ` H.J. Lu
  2021-04-22  9:59     ` Stefan Hajnoczi
  2021-04-20  0:05   ` Paul Eggert
  1 sibling, 1 reply; 7+ messages in thread
From: H.J. Lu @ 2021-04-19 18:38 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: libc-alpha, Jens Axboe, linux-block

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.

-- 
H.J.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH liburing] examples/ucontext-cp.c: cope with variable SIGSTKSZ
  2021-04-19 14:34 ` Stefan Hajnoczi
  2021-04-19 18:38   ` H.J. Lu
@ 2021-04-20  0:05   ` Paul Eggert
  1 sibling, 0 replies; 7+ messages in thread
From: Paul Eggert @ 2021-04-20  0:05 UTC (permalink / raw)
  To: Stefan Hajnoczi, libc-alpha, H . J . Lu; +Cc: Jens Axboe, linux-block

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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH liburing] examples/ucontext-cp.c: cope with variable SIGSTKSZ
  2021-04-19 18:38   ` H.J. Lu
@ 2021-04-22  9:59     ` Stefan Hajnoczi
  2021-04-22 14:22       ` Stefano Garzarella
  0 siblings, 1 reply; 7+ messages in thread
From: Stefan Hajnoczi @ 2021-04-22  9:59 UTC (permalink / raw)
  To: H.J. Lu; +Cc: libc-alpha, Jens Axboe, linux-block


[-- Attachment #1: Type: text/plain, Size: 1941 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH liburing] examples/ucontext-cp.c: cope with variable SIGSTKSZ
  2021-04-22  9:59     ` Stefan Hajnoczi
@ 2021-04-22 14:22       ` Stefano Garzarella
  2021-04-23 14:13         ` Pavel Begunkov
  0 siblings, 1 reply; 7+ messages in thread
From: Stefano Garzarella @ 2021-04-22 14:22 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: io-uring, Pavel Begunkov, H.J. Lu, libc-alpha, Jens Axboe, linux-block

+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>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH liburing] examples/ucontext-cp.c: cope with variable SIGSTKSZ
  2021-04-22 14:22       ` Stefano Garzarella
@ 2021-04-23 14:13         ` Pavel Begunkov
  0 siblings, 0 replies; 7+ messages in thread
From: Pavel Begunkov @ 2021-04-23 14:13 UTC (permalink / raw)
  To: Stefano Garzarella, Stefan Hajnoczi
  Cc: io-uring, H.J. Lu, libc-alpha, Jens Axboe, linux-block

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


-- 
Pavel Begunkov

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-13 15:03 [PATCH liburing] examples/ucontext-cp.c: cope with variable SIGSTKSZ Stefan Hajnoczi
2021-04-19 14:34 ` Stefan Hajnoczi
2021-04-19 18:38   ` H.J. Lu
2021-04-22  9:59     ` Stefan Hajnoczi
2021-04-22 14:22       ` Stefano Garzarella
2021-04-23 14:13         ` Pavel Begunkov
2021-04-20  0:05   ` Paul Eggert

Linux-Block Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-block/0 linux-block/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-block linux-block/ https://lore.kernel.org/linux-block \
		linux-block@vger.kernel.org
	public-inbox-index linux-block

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-block


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git