* [PATCH v3] vfprintf-internal: Get rid of alloca.
@ 2023-05-22 17:06 Joe Simmons-Talbott
2023-06-01 13:34 ` Joe Simmons-Talbott
2023-06-12 20:15 ` Adhemerval Zanella Netto
0 siblings, 2 replies; 5+ messages in thread
From: Joe Simmons-Talbott @ 2023-05-22 17:06 UTC (permalink / raw)
To: libc-alpha; +Cc: Joe Simmons-Talbott
Avoid potential stack overflow from unbounded alloca. Use the existing
scratch_buffer instead.
---
Changes to v2:
* Don't assume the first scratch buffer resize adds enough space.
stdio-common/vfprintf-internal.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/stdio-common/vfprintf-internal.c b/stdio-common/vfprintf-internal.c
index c76c06e49b..20fd3c3043 100644
--- a/stdio-common/vfprintf-internal.c
+++ b/stdio-common/vfprintf-internal.c
@@ -1066,6 +1066,8 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format,
union printf_arg *args_value;
int *args_size;
int *args_type;
+ int *args_pa_user;
+ size_t args_pa_user_offset;
{
/* Calculate total size needed to represent a single argument
across all three argument-related arrays. */
@@ -1082,6 +1084,7 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format,
now. */
args_size = &args_value[nargs].pa_int;
args_type = &args_size[nargs];
+ args_pa_user = &args_type[nargs];
memset (args_type, (mode_flags & PRINTF_FORTIFY) != 0 ? '\xff' : '\0',
nargs * sizeof (*args_type));
}
@@ -1171,7 +1174,25 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format,
else if (__glibc_unlikely (__printf_va_arg_table != NULL)
&& __printf_va_arg_table[args_type[cnt] - PA_LAST] != NULL)
{
- args_value[cnt].pa_user = alloca (args_size[cnt]);
+ while (args_pa_user + args_size[cnt] >
+ (int *) &argsbuf + argsbuf.length)
+ {
+ args_pa_user_offset = args_pa_user - &args_type[nargs];
+ if (!scratch_buffer_grow_preserve (&argsbuf))
+ {
+ Xprintf_buffer_mark_failed (buf);
+ goto all_done;
+ }
+ args_value = argsbuf.data;
+ /* Set up the remaining two arrays to each point past the end of
+ the prior array, since space for all three has been allocated
+ now. */
+ args_size = &args_value[nargs].pa_int;
+ args_type = &args_size[nargs];
+ args_pa_user = &args_type[nargs] + args_pa_user_offset;
+ }
+ args_value[cnt].pa_user = args_pa_user;
+ args_pa_user += args_size[cnt];
(*__printf_va_arg_table[args_type[cnt] - PA_LAST])
(args_value[cnt].pa_user, ap_savep);
}
--
2.39.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] vfprintf-internal: Get rid of alloca.
2023-05-22 17:06 [PATCH v3] vfprintf-internal: Get rid of alloca Joe Simmons-Talbott
@ 2023-06-01 13:34 ` Joe Simmons-Talbott
2023-06-09 14:59 ` Joe Simmons-Talbott
2023-06-12 20:15 ` Adhemerval Zanella Netto
1 sibling, 1 reply; 5+ messages in thread
From: Joe Simmons-Talbott @ 2023-06-01 13:34 UTC (permalink / raw)
To: libc-alpha
On Mon, May 22, 2023 at 01:06:27PM -0400, Joe Simmons-Talbott wrote:
> Avoid potential stack overflow from unbounded alloca. Use the existing
> scratch_buffer instead.
Ping.
Thanks,
Joe
> ---
> Changes to v2:
> * Don't assume the first scratch buffer resize adds enough space.
>
> stdio-common/vfprintf-internal.c | 23 ++++++++++++++++++++++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/stdio-common/vfprintf-internal.c b/stdio-common/vfprintf-internal.c
> index c76c06e49b..20fd3c3043 100644
> --- a/stdio-common/vfprintf-internal.c
> +++ b/stdio-common/vfprintf-internal.c
> @@ -1066,6 +1066,8 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format,
> union printf_arg *args_value;
> int *args_size;
> int *args_type;
> + int *args_pa_user;
> + size_t args_pa_user_offset;
> {
> /* Calculate total size needed to represent a single argument
> across all three argument-related arrays. */
> @@ -1082,6 +1084,7 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format,
> now. */
> args_size = &args_value[nargs].pa_int;
> args_type = &args_size[nargs];
> + args_pa_user = &args_type[nargs];
> memset (args_type, (mode_flags & PRINTF_FORTIFY) != 0 ? '\xff' : '\0',
> nargs * sizeof (*args_type));
> }
> @@ -1171,7 +1174,25 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format,
> else if (__glibc_unlikely (__printf_va_arg_table != NULL)
> && __printf_va_arg_table[args_type[cnt] - PA_LAST] != NULL)
> {
> - args_value[cnt].pa_user = alloca (args_size[cnt]);
> + while (args_pa_user + args_size[cnt] >
> + (int *) &argsbuf + argsbuf.length)
> + {
> + args_pa_user_offset = args_pa_user - &args_type[nargs];
> + if (!scratch_buffer_grow_preserve (&argsbuf))
> + {
> + Xprintf_buffer_mark_failed (buf);
> + goto all_done;
> + }
> + args_value = argsbuf.data;
> + /* Set up the remaining two arrays to each point past the end of
> + the prior array, since space for all three has been allocated
> + now. */
> + args_size = &args_value[nargs].pa_int;
> + args_type = &args_size[nargs];
> + args_pa_user = &args_type[nargs] + args_pa_user_offset;
> + }
> + args_value[cnt].pa_user = args_pa_user;
> + args_pa_user += args_size[cnt];
> (*__printf_va_arg_table[args_type[cnt] - PA_LAST])
> (args_value[cnt].pa_user, ap_savep);
> }
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] vfprintf-internal: Get rid of alloca.
2023-06-01 13:34 ` Joe Simmons-Talbott
@ 2023-06-09 14:59 ` Joe Simmons-Talbott
0 siblings, 0 replies; 5+ messages in thread
From: Joe Simmons-Talbott @ 2023-06-09 14:59 UTC (permalink / raw)
To: libc-alpha
On Thu, Jun 01, 2023 at 09:34:28AM -0400, Joe Simmons-Talbott via Libc-alpha wrote:
> On Mon, May 22, 2023 at 01:06:27PM -0400, Joe Simmons-Talbott wrote:
> > Avoid potential stack overflow from unbounded alloca. Use the existing
> > scratch_buffer instead.
>
> Ping.
x2
Thanks,
Joe
>
> Thanks,
> Joe
> > ---
> > Changes to v2:
> > * Don't assume the first scratch buffer resize adds enough space.
> >
> > stdio-common/vfprintf-internal.c | 23 ++++++++++++++++++++++-
> > 1 file changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/stdio-common/vfprintf-internal.c b/stdio-common/vfprintf-internal.c
> > index c76c06e49b..20fd3c3043 100644
> > --- a/stdio-common/vfprintf-internal.c
> > +++ b/stdio-common/vfprintf-internal.c
> > @@ -1066,6 +1066,8 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format,
> > union printf_arg *args_value;
> > int *args_size;
> > int *args_type;
> > + int *args_pa_user;
> > + size_t args_pa_user_offset;
> > {
> > /* Calculate total size needed to represent a single argument
> > across all three argument-related arrays. */
> > @@ -1082,6 +1084,7 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format,
> > now. */
> > args_size = &args_value[nargs].pa_int;
> > args_type = &args_size[nargs];
> > + args_pa_user = &args_type[nargs];
> > memset (args_type, (mode_flags & PRINTF_FORTIFY) != 0 ? '\xff' : '\0',
> > nargs * sizeof (*args_type));
> > }
> > @@ -1171,7 +1174,25 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format,
> > else if (__glibc_unlikely (__printf_va_arg_table != NULL)
> > && __printf_va_arg_table[args_type[cnt] - PA_LAST] != NULL)
> > {
> > - args_value[cnt].pa_user = alloca (args_size[cnt]);
> > + while (args_pa_user + args_size[cnt] >
> > + (int *) &argsbuf + argsbuf.length)
> > + {
> > + args_pa_user_offset = args_pa_user - &args_type[nargs];
> > + if (!scratch_buffer_grow_preserve (&argsbuf))
> > + {
> > + Xprintf_buffer_mark_failed (buf);
> > + goto all_done;
> > + }
> > + args_value = argsbuf.data;
> > + /* Set up the remaining two arrays to each point past the end of
> > + the prior array, since space for all three has been allocated
> > + now. */
> > + args_size = &args_value[nargs].pa_int;
> > + args_type = &args_size[nargs];
> > + args_pa_user = &args_type[nargs] + args_pa_user_offset;
> > + }
> > + args_value[cnt].pa_user = args_pa_user;
> > + args_pa_user += args_size[cnt];
> > (*__printf_va_arg_table[args_type[cnt] - PA_LAST])
> > (args_value[cnt].pa_user, ap_savep);
> > }
> > --
> > 2.39.2
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] vfprintf-internal: Get rid of alloca.
2023-05-22 17:06 [PATCH v3] vfprintf-internal: Get rid of alloca Joe Simmons-Talbott
2023-06-01 13:34 ` Joe Simmons-Talbott
@ 2023-06-12 20:15 ` Adhemerval Zanella Netto
2023-06-14 18:45 ` Joe Simmons-Talbott
1 sibling, 1 reply; 5+ messages in thread
From: Adhemerval Zanella Netto @ 2023-06-12 20:15 UTC (permalink / raw)
To: libc-alpha, Joe Simmons-Talbott
On 22/05/23 14:06, Joe Simmons-Talbott via Libc-alpha wrote:
> Avoid potential stack overflow from unbounded alloca. Use the existing
> scratch_buffer instead.
> ---
> Changes to v2:
> * Don't assume the first scratch buffer resize adds enough space.
>
> stdio-common/vfprintf-internal.c | 23 ++++++++++++++++++++++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/stdio-common/vfprintf-internal.c b/stdio-common/vfprintf-internal.c
> index c76c06e49b..20fd3c3043 100644
> --- a/stdio-common/vfprintf-internal.c
> +++ b/stdio-common/vfprintf-internal.c
> @@ -1066,6 +1066,8 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format,
> union printf_arg *args_value;
> int *args_size;
> int *args_type;
> + int *args_pa_user;
> + size_t args_pa_user_offset;
> {
> /* Calculate total size needed to represent a single argument
> across all three argument-related arrays. */
> @@ -1082,6 +1084,7 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format,
> now. */
> args_size = &args_value[nargs].pa_int;
> args_type = &args_size[nargs];
> + args_pa_user = &args_type[nargs];
> memset (args_type, (mode_flags & PRINTF_FORTIFY) != 0 ? '\xff' : '\0',
> nargs * sizeof (*args_type));
> }
> @@ -1171,7 +1174,25 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format,
> else if (__glibc_unlikely (__printf_va_arg_table != NULL)
> && __printf_va_arg_table[args_type[cnt] - PA_LAST] != NULL)
> {
> - args_value[cnt].pa_user = alloca (args_size[cnt]);
> + while (args_pa_user + args_size[cnt] >
> + (int *) &argsbuf + argsbuf.length)
> + {
> + args_pa_user_offset = args_pa_user - &args_type[nargs];
> + if (!scratch_buffer_grow_preserve (&argsbuf))
> + {
> + Xprintf_buffer_mark_failed (buf);
> + goto all_done;
> + }
> + args_value = argsbuf.data;
> + /* Set up the remaining two arrays to each point past the end of
> + the prior array, since space for all three has been allocated
> + now. */
> + args_size = &args_value[nargs].pa_int;
> + args_type = &args_size[nargs];
> + args_pa_user = &args_type[nargs] + args_pa_user_offset;
> + }
> + args_value[cnt].pa_user = args_pa_user;
> + args_pa_user += args_size[cnt];
> (*__printf_va_arg_table[args_type[cnt] - PA_LAST])
> (args_value[cnt].pa_user, ap_savep);
> }
This does not seem to be fully correct, trying to come up with a testcase
to exercise this code path show a failure:
diff --git a/stdio-common/tst-vfprintf-user-type.c b/stdio-common/tst-vfprintf-user-type.c
index 7cc69dc716..9058bc1978 100644
--- a/stdio-common/tst-vfprintf-user-type.c
+++ b/stdio-common/tst-vfprintf-user-type.c
@@ -189,6 +189,52 @@ do_test (void)
TEST_COMPARE_STRING (str, "[[(123, 456.000000)]]");
free (str);
+ str = NULL;
+ TEST_VERIFY (asprintf_alias (&str, "[[%1$P %1$P]]", 123L, 457.0) >= 0);
+ TEST_COMPARE_STRING (str, "[[(123, 457.000000) (123, 457.000000)]]");
+ free (str);
+
+ str = NULL;
+ TEST_VERIFY (asprintf_alias (&str, "%1$P %2$P %3$P %4$P %5$P %6$P"
+ "%7$P %8$P %9$P %10$P %11$P %12$P"
+ "%13$P %14$P %15$P %16$P %17$P %18$P"
+ "%19$P %20$P %21$P %22$P %23$P %24$P"
+ "%25$P %26$P %27$P %28$P", /*%*29$P %30$P",*/
+ 1L, 1.0,
+ 2L, 2.0,
+ 3L, 3.0,
+ 4L, 4.0,
+ 5L, 6.0,
+ 6L, 6.0,
+ 7L, 7.0,
+ 8L, 8.0,
+ 9L, 9.0,
+ 10L, 10.0,
+ 11L, 11.0,
+ 12L, 12.0,
+ 13L, 13.0,
+ 14L, 14.0,
+ 15L, 15.0,
+ 16L, 16.0,
+ 17L, 17.0,
+ 18L, 18.0,
+ 19L, 19.0,
+ 20L, 20.0,
+ 21L, 21.0,
+ 22L, 22.0,
+ 23L, 23.0,
+ 24L, 34.0,
+ 25L, 25.0,
+ 26L, 26.0,
+ 27L, 27.0,
+ 28L, 28.0,
+ 29L, 29.0,
+ 30L, 30.0)
+ >= 0);
+ printf ("str=%s\n", str);
+ free (str);
+
+
str = NULL;
TEST_VERIFY (asprintf_alias (&str, "[[%1$P %1$P]]", 123L, 457.0) >= 0);
TEST_COMPARE_STRING (str, "[[(123, 457.000000) (123, 457.000000)]]");
$ ./testrun.sh stdio-common/tst-vfprintf-user-type
Didn't expect signal from child: got `Segmentation fault'
While I would expect to:
$ ./testrun.sh stdio-common/tst-vfprintf-user-type
str=(1, 1.000000) (2, 2.000000) (3, 3.000000) (4, 4.000000) (5, 6.000000) (6, 6.000000)(7, 7.000000) (8, 8.000000) (9, 9.000000) (10, 10.000000) (11, 11.000000) (12, 12.000000)(13, 13.000000) (14, 14.000000) (15, 15.000000) (16, 16.000000) (17, 17.000000) (18, 18.000000)(19, 19.000000) (20, 20.000000) (21, 21.000000) (22, 22.000000) (23, 23.000000) (24, 34.000000)(25, 25.000000) (26, 26.000000) (27, 27.000000) (28, 28.000000)
So I think it would be better to extend stdio-common/tst-vfprintf-user-type
to use a large number of input arguments tests, and check why this code
snippet is doing wrong.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] vfprintf-internal: Get rid of alloca.
2023-06-12 20:15 ` Adhemerval Zanella Netto
@ 2023-06-14 18:45 ` Joe Simmons-Talbott
0 siblings, 0 replies; 5+ messages in thread
From: Joe Simmons-Talbott @ 2023-06-14 18:45 UTC (permalink / raw)
To: Adhemerval Zanella Netto; +Cc: libc-alpha
On Mon, Jun 12, 2023 at 05:15:46PM -0300, Adhemerval Zanella Netto wrote:
>
>
> On 22/05/23 14:06, Joe Simmons-Talbott via Libc-alpha wrote:
> > Avoid potential stack overflow from unbounded alloca. Use the existing
> > scratch_buffer instead.
> > ---
> > Changes to v2:
> > * Don't assume the first scratch buffer resize adds enough space.
> >
> > stdio-common/vfprintf-internal.c | 23 ++++++++++++++++++++++-
> > 1 file changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/stdio-common/vfprintf-internal.c b/stdio-common/vfprintf-internal.c
> > index c76c06e49b..20fd3c3043 100644
> > --- a/stdio-common/vfprintf-internal.c
> > +++ b/stdio-common/vfprintf-internal.c
> > @@ -1066,6 +1066,8 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format,
> > union printf_arg *args_value;
> > int *args_size;
> > int *args_type;
> > + int *args_pa_user;
> > + size_t args_pa_user_offset;
> > {
> > /* Calculate total size needed to represent a single argument
> > across all three argument-related arrays. */
> > @@ -1082,6 +1084,7 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format,
> > now. */
> > args_size = &args_value[nargs].pa_int;
> > args_type = &args_size[nargs];
> > + args_pa_user = &args_type[nargs];
> > memset (args_type, (mode_flags & PRINTF_FORTIFY) != 0 ? '\xff' : '\0',
> > nargs * sizeof (*args_type));
> > }
> > @@ -1171,7 +1174,25 @@ printf_positional (struct Xprintf_buffer * buf, const CHAR_T *format,
> > else if (__glibc_unlikely (__printf_va_arg_table != NULL)
> > && __printf_va_arg_table[args_type[cnt] - PA_LAST] != NULL)
> > {
> > - args_value[cnt].pa_user = alloca (args_size[cnt]);
> > + while (args_pa_user + args_size[cnt] >
> > + (int *) &argsbuf + argsbuf.length)
> > + {
> > + args_pa_user_offset = args_pa_user - &args_type[nargs];
> > + if (!scratch_buffer_grow_preserve (&argsbuf))
> > + {
> > + Xprintf_buffer_mark_failed (buf);
> > + goto all_done;
> > + }
> > + args_value = argsbuf.data;
> > + /* Set up the remaining two arrays to each point past the end of
> > + the prior array, since space for all three has been allocated
> > + now. */
> > + args_size = &args_value[nargs].pa_int;
> > + args_type = &args_size[nargs];
> > + args_pa_user = &args_type[nargs] + args_pa_user_offset;
> > + }
> > + args_value[cnt].pa_user = args_pa_user;
> > + args_pa_user += args_size[cnt];
> > (*__printf_va_arg_table[args_type[cnt] - PA_LAST])
> > (args_value[cnt].pa_user, ap_savep);
> > }
>
> This does not seem to be fully correct, trying to come up with a testcase
> to exercise this code path show a failure:
>
> diff --git a/stdio-common/tst-vfprintf-user-type.c b/stdio-common/tst-vfprintf-user-type.c
> index 7cc69dc716..9058bc1978 100644
> --- a/stdio-common/tst-vfprintf-user-type.c
> +++ b/stdio-common/tst-vfprintf-user-type.c
> @@ -189,6 +189,52 @@ do_test (void)
> TEST_COMPARE_STRING (str, "[[(123, 456.000000)]]");
> free (str);
>
> + str = NULL;
> + TEST_VERIFY (asprintf_alias (&str, "[[%1$P %1$P]]", 123L, 457.0) >= 0);
> + TEST_COMPARE_STRING (str, "[[(123, 457.000000) (123, 457.000000)]]");
> + free (str);
> +
> + str = NULL;
> + TEST_VERIFY (asprintf_alias (&str, "%1$P %2$P %3$P %4$P %5$P %6$P"
> + "%7$P %8$P %9$P %10$P %11$P %12$P"
> + "%13$P %14$P %15$P %16$P %17$P %18$P"
> + "%19$P %20$P %21$P %22$P %23$P %24$P"
> + "%25$P %26$P %27$P %28$P", /*%*29$P %30$P",*/
> + 1L, 1.0,
> + 2L, 2.0,
> + 3L, 3.0,
> + 4L, 4.0,
> + 5L, 6.0,
> + 6L, 6.0,
> + 7L, 7.0,
> + 8L, 8.0,
> + 9L, 9.0,
> + 10L, 10.0,
> + 11L, 11.0,
> + 12L, 12.0,
> + 13L, 13.0,
> + 14L, 14.0,
> + 15L, 15.0,
> + 16L, 16.0,
> + 17L, 17.0,
> + 18L, 18.0,
> + 19L, 19.0,
> + 20L, 20.0,
> + 21L, 21.0,
> + 22L, 22.0,
> + 23L, 23.0,
> + 24L, 34.0,
> + 25L, 25.0,
> + 26L, 26.0,
> + 27L, 27.0,
> + 28L, 28.0,
> + 29L, 29.0,
> + 30L, 30.0)
> + >= 0);
> + printf ("str=%s\n", str);
> + free (str);
> +
> +
> str = NULL;
> TEST_VERIFY (asprintf_alias (&str, "[[%1$P %1$P]]", 123L, 457.0) >= 0);
> TEST_COMPARE_STRING (str, "[[(123, 457.000000) (123, 457.000000)]]");
>
>
> $ ./testrun.sh stdio-common/tst-vfprintf-user-type
> Didn't expect signal from child: got `Segmentation fault'
>
> While I would expect to:
>
> $ ./testrun.sh stdio-common/tst-vfprintf-user-type
> str=(1, 1.000000) (2, 2.000000) (3, 3.000000) (4, 4.000000) (5, 6.000000) (6, 6.000000)(7, 7.000000) (8, 8.000000) (9, 9.000000) (10, 10.000000) (11, 11.000000) (12, 12.000000)(13, 13.000000) (14, 14.000000) (15, 15.000000) (16, 16.000000) (17, 17.000000) (18, 18.000000)(19, 19.000000) (20, 20.000000) (21, 21.000000) (22, 22.000000) (23, 23.000000) (24, 34.000000)(25, 25.000000) (26, 26.000000) (27, 27.000000) (28, 28.000000)
>
> So I think it would be better to extend stdio-common/tst-vfprintf-user-type
> to use a large number of input arguments tests, and check why this code
> snippet is doing wrong.
Thanks for the testcase. I added it to v4 of my patch and fixed the code to
properly use (void *) rather than (int *) which was precenting the while
loop from triggering the call to grow the scratch_buffer.
Thanks,
Joe
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-06-14 18:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-22 17:06 [PATCH v3] vfprintf-internal: Get rid of alloca Joe Simmons-Talbott
2023-06-01 13:34 ` Joe Simmons-Talbott
2023-06-09 14:59 ` Joe Simmons-Talbott
2023-06-12 20:15 ` Adhemerval Zanella Netto
2023-06-14 18:45 ` Joe Simmons-Talbott
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).