public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [committed 1/4] libstdc++ Fix undefined behaviour in testsuite
@ 2021-05-04 22:17 Jonathan Wakely
  2021-05-04 22:18 ` [committed 2/4] libstdc++: Fix null dereference in pb_ds containers Jonathan Wakely
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Jonathan Wakely @ 2021-05-04 22:17 UTC (permalink / raw)
  To: libstdc++, gcc-patches

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

Fix some test bugs found by ubsan.

libstdc++-v3/ChangeLog:

	* testsuite/20_util/from_chars/3.cc: Use unsigned type to avoid
	overflow.
	* testsuite/24_iterators/reverse_iterator/2.cc: Do not add
	non-zero value to null pointer.
	* testsuite/25_algorithms/copy_backward/move_iterators/69478.cc:
	Use past-the-end iterator for result.
	* testsuite/25_algorithms/move_backward/69478.cc: Likewise.
	* testsuite/25_algorithms/move_backward/93872.cc: Likewise.

Tested x86_64-linux. Committed to trunk.


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 3722 bytes --]

commit 6fb8b67089119b737ccb38f75f403b8d279e2229
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue May 4 15:28:39 2021

    libstdc++ Fix undefined behaviour in testsuite
    
    Fix some test bugs found by ubsan.
    
    libstdc++-v3/ChangeLog:
    
            * testsuite/20_util/from_chars/3.cc: Use unsigned type to avoid
            overflow.
            * testsuite/24_iterators/reverse_iterator/2.cc: Do not add
            non-zero value to null pointer.
            * testsuite/25_algorithms/copy_backward/move_iterators/69478.cc:
            Use past-the-end iterator for result.
            * testsuite/25_algorithms/move_backward/69478.cc: Likewise.
            * testsuite/25_algorithms/move_backward/93872.cc: Likewise.

diff --git a/libstdc++-v3/testsuite/20_util/from_chars/3.cc b/libstdc++-v3/testsuite/20_util/from_chars/3.cc
index c99353a7284..d3d70394ed9 100644
--- a/libstdc++-v3/testsuite/20_util/from_chars/3.cc
+++ b/libstdc++-v3/testsuite/20_util/from_chars/3.cc
@@ -29,7 +29,7 @@ long long
 read(const char* first, const char* last, int base)
 {
   long long val = 0;
-  long long place = 1;
+  unsigned long long place = 1;
   while (last > first)
   {
     val += (*--last - '0') * place;
diff --git a/libstdc++-v3/testsuite/24_iterators/reverse_iterator/2.cc b/libstdc++-v3/testsuite/24_iterators/reverse_iterator/2.cc
index a6d7d6b0191..633c1426b96 100644
--- a/libstdc++-v3/testsuite/24_iterators/reverse_iterator/2.cc
+++ b/libstdc++-v3/testsuite/24_iterators/reverse_iterator/2.cc
@@ -27,7 +27,7 @@ void test02()
   iterator_type it01;
   iterator_type it02;
 
-  // Sanity check non-member operators and functions can be instantiated. 
+  // Sanity check non-member operators and functions can be instantiated.
   it01 == it02;
   it01 != it02;
   it01 < it02;
@@ -35,11 +35,11 @@ void test02()
   it01 > it02;
   it01 >= it02;
   it01 - it02;
-  5 + it02;
+  0 + it02;
 }
 
-int main() 
-{ 
+int main()
+{
   test02();
   return 0;
 }
diff --git a/libstdc++-v3/testsuite/25_algorithms/copy_backward/move_iterators/69478.cc b/libstdc++-v3/testsuite/25_algorithms/copy_backward/move_iterators/69478.cc
index 88a01b84a8e..9c84cfb176c 100644
--- a/libstdc++-v3/testsuite/25_algorithms/copy_backward/move_iterators/69478.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/copy_backward/move_iterators/69478.cc
@@ -35,6 +35,6 @@ test01()
   static_assert(std::is_trivial<trivial_rvalstruct>::value, "");
 
   trivial_rvalstruct a[1], b[1];
-  copy_backward(std::make_move_iterator(a), std::make_move_iterator(a+1), b);
+  copy_backward(std::make_move_iterator(a), std::make_move_iterator(a+1), b+1);
 }
 // { dg-prune-output "use of deleted" }
diff --git a/libstdc++-v3/testsuite/25_algorithms/move_backward/69478.cc b/libstdc++-v3/testsuite/25_algorithms/move_backward/69478.cc
index 25bfe29fc14..10e31be1e08 100644
--- a/libstdc++-v3/testsuite/25_algorithms/move_backward/69478.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/move_backward/69478.cc
@@ -35,6 +35,6 @@ test01()
   static_assert(std::is_trivial<trivial_rvalstruct>::value, "");
 
   trivial_rvalstruct a[1], b[1];
-  std::move_backward(a, a + 1, b);
+  std::move_backward(a, a + 1, b + 1);
 }
 // { dg-prune-output "use of deleted" }
diff --git a/libstdc++-v3/testsuite/25_algorithms/move_backward/93872.cc b/libstdc++-v3/testsuite/25_algorithms/move_backward/93872.cc
index df96c94b394..a9fd2697f5b 100644
--- a/libstdc++-v3/testsuite/25_algorithms/move_backward/93872.cc
+++ b/libstdc++-v3/testsuite/25_algorithms/move_backward/93872.cc
@@ -35,5 +35,5 @@ void
 test01()
 {
   X a[2], b[2];
-  std::move_backward(std::begin(a), std::end(a), std::begin(b));
+  std::move_backward(std::begin(a), std::end(a), std::end(b));
 }

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

* [committed 2/4] libstdc++: Fix null dereference in pb_ds containers
  2021-05-04 22:17 [committed 1/4] libstdc++ Fix undefined behaviour in testsuite Jonathan Wakely
@ 2021-05-04 22:18 ` Jonathan Wakely
  2021-05-04 22:19 ` [committed 3/4] libstdc++: Fix undefined behaviour in std::string Jonathan Wakely
  2021-05-04 22:20 ` [committed 4/4] libstdc++: Fix null dereferences in std::promise Jonathan Wakely
  2 siblings, 0 replies; 4+ messages in thread
From: Jonathan Wakely @ 2021-05-04 22:18 UTC (permalink / raw)
  To: libstdc++, gcc-patches

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

This fixes ubsan errors:

ext/pb_ds/detail/cc_hash_table_map_/cc_ht_map_.hpp:533:15: runtime error: member access within null pointer of type 'struct entry'

libstdc++-v3/ChangeLog:

	* include/ext/pb_ds/detail/cc_hash_table_map_/cc_ht_map_.hpp
	(find_key_pointer(key_const_reference, false_type))
	(find_key_pointer(key_const_reference, true_type)): Do not
	dereference null pointer.

Tested x86_64-linux. Committed to trunk.



[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 1837 bytes --]

commit ca871701c2822c3c4537745d4aa44a7b8f408337
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue May 4 15:46:28 2021

    libstdc++: Fix null dereference in pb_ds containers
    
    This fixes ubsan errors:
    
    ext/pb_ds/detail/cc_hash_table_map_/cc_ht_map_.hpp:533:15: runtime error: member access within null pointer of type 'struct entry'
    
    libstdc++-v3/ChangeLog:
    
            * include/ext/pb_ds/detail/cc_hash_table_map_/cc_ht_map_.hpp
            (find_key_pointer(key_const_reference, false_type))
            (find_key_pointer(key_const_reference, true_type)): Do not
            dereference null pointer.

diff --git a/libstdc++-v3/include/ext/pb_ds/detail/cc_hash_table_map_/cc_ht_map_.hpp b/libstdc++-v3/include/ext/pb_ds/detail/cc_hash_table_map_/cc_ht_map_.hpp
index aa5c94b9aed..c81bf3abfef 100644
--- a/libstdc++-v3/include/ext/pb_ds/detail/cc_hash_table_map_/cc_ht_map_.hpp
+++ b/libstdc++-v3/include/ext/pb_ds/detail/cc_hash_table_map_/cc_ht_map_.hpp
@@ -524,13 +524,16 @@ namespace __gnu_pbds
 
 	resize_base::notify_find_search_end();
 
-#ifdef _GLIBCXX_DEBUG
 	if (p_e == 0)
-	  PB_DS_CHECK_KEY_DOES_NOT_EXIST(r_key)
+	  {
+	    PB_DS_CHECK_KEY_DOES_NOT_EXIST(r_key)
+	    return 0;
+	  }
 	else
-	  PB_DS_CHECK_KEY_EXISTS(r_key)
-#endif
-	return &p_e->m_value;
+	  {
+	    PB_DS_CHECK_KEY_EXISTS(r_key)
+	    return &p_e->m_value;
+	  }
       }
 
       inline pointer
@@ -550,13 +553,16 @@ namespace __gnu_pbds
 
 	resize_base::notify_find_search_end();
 
-#ifdef _GLIBCXX_DEBUG
 	if (p_e == 0)
-	  PB_DS_CHECK_KEY_DOES_NOT_EXIST(r_key)
+	  {
+	    PB_DS_CHECK_KEY_DOES_NOT_EXIST(r_key)
+	    return 0;
+	  }
 	else
-	  PB_DS_CHECK_KEY_EXISTS(r_key)
-#endif
-	return &p_e->m_value;
+	  {
+	    PB_DS_CHECK_KEY_EXISTS(r_key)
+	    return &p_e->m_value;
+	  }
       }
 
       inline bool

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

* [committed 3/4] libstdc++: Fix undefined behaviour in std::string
  2021-05-04 22:17 [committed 1/4] libstdc++ Fix undefined behaviour in testsuite Jonathan Wakely
  2021-05-04 22:18 ` [committed 2/4] libstdc++: Fix null dereference in pb_ds containers Jonathan Wakely
@ 2021-05-04 22:19 ` Jonathan Wakely
  2021-05-04 22:20 ` [committed 4/4] libstdc++: Fix null dereferences in std::promise Jonathan Wakely
  2 siblings, 0 replies; 4+ messages in thread
From: Jonathan Wakely @ 2021-05-04 22:19 UTC (permalink / raw)
  To: libstdc++, gcc-patches

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

This fixes a ubsan error when constructing a string with a null pointer:

bits/basic_string.h:534:21: runtime error: applying non-zero offset 18446744073709551615 to null pointer

The _M_construct function only cares whether the second pointer is
non-null, so create a non-null value without undefined arithmetic.

We can also pass the random_access_iterator_tag directly to the
_M_construct function, to avoid going via the tag dispatching
_M_construct_aux, because we know we have pointers not integers here.

libstdc++-v3/ChangeLog:

         * include/bits/basic_string.h (basic_string(const CharT*, const A&)):
         Do not do arithmetic on null pointer.

Tested x86_64-linux. Committed to trunk.




[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 1722 bytes --]

commit 789c57bc5fe023fc6dc72ade4afcb0916ff788d3
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue May 4 15:49:38 2021

    libstdc++: Fix undefined behaviour in std::string
    
    This fixes a ubsan error when constructing a string with a null pointer:
    
    bits/basic_string.h:534:21: runtime error: applying non-zero offset 18446744073709551615 to null pointer
    
    The _M_construct function only cares whether the second pointer is
    non-null, so create a non-null value without undefined arithmetic.
    
    We can also pass the random_access_iterator_tag directly to the
    _M_construct function, to avoid going via the tag dispatching
    _M_construct_aux, because we know we have pointers not integers here.
    
    libstdc++-v3/ChangeLog:
    
            * include/bits/basic_string.h (basic_string(const CharT*, const A&)):
            Do not do arithmetic on null pointer.

diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index fba7c6f3354..84356adc7ae 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -531,7 +531,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
 #endif
       basic_string(const _CharT* __s, const _Alloc& __a = _Alloc())
       : _M_dataplus(_M_local_data(), __a)
-      { _M_construct(__s, __s ? __s + traits_type::length(__s) : __s+npos); }
+      {
+	const _CharT* __end = __s ? __s + traits_type::length(__s)
+	  // We just need a non-null pointer here to get an exception:
+	  : reinterpret_cast<const _CharT*>(__alignof__(_CharT));
+	_M_construct(__s, __end, random_access_iterator_tag());
+      }
 
       /**
        *  @brief  Construct string as multiple characters.

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

* [committed 4/4] libstdc++: Fix null dereferences in std::promise
  2021-05-04 22:17 [committed 1/4] libstdc++ Fix undefined behaviour in testsuite Jonathan Wakely
  2021-05-04 22:18 ` [committed 2/4] libstdc++: Fix null dereference in pb_ds containers Jonathan Wakely
  2021-05-04 22:19 ` [committed 3/4] libstdc++: Fix undefined behaviour in std::string Jonathan Wakely
@ 2021-05-04 22:20 ` Jonathan Wakely
  2 siblings, 0 replies; 4+ messages in thread
From: Jonathan Wakely @ 2021-05-04 22:20 UTC (permalink / raw)
  To: libstdc++, gcc-patches

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

This fixes some ubsan errors in std::promise:

future:1153:34: runtime error: member call on null pointer of type 'struct element_type'
future:1153:34: runtime error: member access within null pointer of type 'struct element_type'

The problem is that the check for a null pointer is done inside the
_State::__Setter function, which is only evaluated after evaluating the
_M_future->_M_set_result postfix-expression.

This change adds a new promise::_M_state() helper for accessing
_M_future, and moves the check for no shared state into there, instead
of inside the __setter functions. The __setter functions are made
always_inline, to avoid the situation where the linker selects the old
version of set_value (without the _S_check call) and the new version of
__setter (without the _S_check call) and so there is no check. With the
always_inline attribute the old version of set_value will either inline
the old __setter or call an extern definition of it, and the new
set_value will do the check itself, so both versions do the check.

libstdc++-v3/ChangeLog:

         * include/std/future (promise::set_value): Check for existence
         of shared state before dereferncing it.
         (promise::set_exception, promise::set_value_at_thread_exit)
         (promise::set_exception_at_thread_exit): Likewise.
         (promise<R&>::set_value, promise<R&>::set_exception)
         (promise<R&>::set_value_at_thread_exit)
         (promise<R&>::set_exception_at_thread_exit): Likewise.
         (promise<void>::set_value, promise<void>::set_exception)
         (promise<void>::set_value_at_thread_exit)
         (promise<void>::set_exception_at_thread_exit): Likewise.
         * testsuite/30_threads/promise/members/at_thread_exit2.cc:
         Remove unused variable.


Tested x86_64-linux. Committed to trunk.



[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 7515 bytes --]

commit 058d6acefe8bac4a66c8e7fb4951276db188e2d8
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue May 4 16:28:57 2021

    libstdc++: Fix null dereferences in std::promise
    
    This fixes some ubsan errors in std::promise:
    
    future:1153:34: runtime error: member call on null pointer of type 'struct element_type'
    future:1153:34: runtime error: member access within null pointer of type 'struct element_type'
    
    The problem is that the check for a null pointer is done inside the
    _State::__Setter function, which is only evaluated after evaluating the
    _M_future->_M_set_result postfix-expression.
    
    This change adds a new promise::_M_state() helper for accessing
    _M_future, and moves the check for no shared state into there, instead
    of inside the __setter functions. The __setter functions are made
    always_inline, to avoid the situation where the linker selects the old
    version of set_value (without the _S_check call) and the new version of
    __setter (without the _S_check call) and so there is no check. With the
    always_inline attribute the old version of set_value will either inline
    the old __setter or call an extern definition of it, and the new
    set_value will do the check itself, so both versions do the check.
    
    libstdc++-v3/ChangeLog:
    
            * include/std/future (promise::set_value): Check for existence
            of shared state before dereferncing it.
            (promise::set_exception, promise::set_value_at_thread_exit)
            (promise::set_exception_at_thread_exit): Likewise.
            (promise<R&>::set_value, promise<R&>::set_exception)
            (promise<R&>::set_value_at_thread_exit)
            (promise<R&>::set_exception_at_thread_exit): Likewise.
            (promise<void>::set_value, promise<void>::set_exception)
            (promise<void>::set_value_at_thread_exit)
            (promise<void>::set_exception_at_thread_exit): Likewise.
            * testsuite/30_threads/promise/members/at_thread_exit2.cc:
            Remove unused variable.

diff --git a/libstdc++-v3/include/std/future b/libstdc++-v3/include/std/future
index ef15fefa53c..09e54c3703b 100644
--- a/libstdc++-v3/include/std/future
+++ b/libstdc++-v3/include/std/future
@@ -532,26 +532,26 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
         };
 
       template<typename _Res, typename _Arg>
+	__attribute__((__always_inline__))
         static _Setter<_Res, _Arg&&>
-        __setter(promise<_Res>* __prom, _Arg&& __arg)
+        __setter(promise<_Res>* __prom, _Arg&& __arg) noexcept
         {
-	  _S_check(__prom->_M_future);
           return _Setter<_Res, _Arg&&>{ __prom, std::__addressof(__arg) };
         }
 
       template<typename _Res>
+	__attribute__((__always_inline__))
         static _Setter<_Res, __exception_ptr_tag>
-        __setter(exception_ptr& __ex, promise<_Res>* __prom)
+        __setter(exception_ptr& __ex, promise<_Res>* __prom) noexcept
         {
-	  _S_check(__prom->_M_future);
           return _Setter<_Res, __exception_ptr_tag>{ __prom, &__ex };
         }
 
       template<typename _Res>
+	__attribute__((__always_inline__))
 	static _Setter<_Res, void>
-	__setter(promise<_Res>* __prom)
+	__setter(promise<_Res>* __prom) noexcept
 	{
-	  _S_check(__prom->_M_future);
 	  return _Setter<_Res, void>{ __prom };
 	}
 
@@ -1130,36 +1130,44 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       // Setting the result
       void
       set_value(const _Res& __r)
-      { _M_future->_M_set_result(_State::__setter(this, __r)); }
+      { _M_state()._M_set_result(_State::__setter(this, __r)); }
 
       void
       set_value(_Res&& __r)
-      { _M_future->_M_set_result(_State::__setter(this, std::move(__r))); }
+      { _M_state()._M_set_result(_State::__setter(this, std::move(__r))); }
 
       void
       set_exception(exception_ptr __p)
-      { _M_future->_M_set_result(_State::__setter(__p, this)); }
+      { _M_state()._M_set_result(_State::__setter(__p, this)); }
 
       void
       set_value_at_thread_exit(const _Res& __r)
       {
-	_M_future->_M_set_delayed_result(_State::__setter(this, __r),
+	_M_state()._M_set_delayed_result(_State::__setter(this, __r),
 					 _M_future);
       }
 
       void
       set_value_at_thread_exit(_Res&& __r)
       {
-	_M_future->_M_set_delayed_result(
+	_M_state()._M_set_delayed_result(
 	    _State::__setter(this, std::move(__r)), _M_future);
       }
 
       void
       set_exception_at_thread_exit(exception_ptr __p)
       {
-	_M_future->_M_set_delayed_result(_State::__setter(__p, this),
+	_M_state()._M_set_delayed_result(_State::__setter(__p, this),
 					 _M_future);
       }
+
+    private:
+      _State&
+      _M_state()
+      {
+	__future_base::_State_base::_S_check(_M_future);
+	return *_M_future;
+      }
     };
 
   template<typename _Res>
@@ -1241,25 +1249,33 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       // Setting the result
       void
       set_value(_Res& __r)
-      { _M_future->_M_set_result(_State::__setter(this, __r)); }
+      { _M_state()._M_set_result(_State::__setter(this, __r)); }
 
       void
       set_exception(exception_ptr __p)
-      { _M_future->_M_set_result(_State::__setter(__p, this)); }
+      { _M_state()._M_set_result(_State::__setter(__p, this)); }
 
       void
       set_value_at_thread_exit(_Res& __r)
       {
-	_M_future->_M_set_delayed_result(_State::__setter(this, __r),
+	_M_state()._M_set_delayed_result(_State::__setter(this, __r),
 					 _M_future);
       }
 
       void
       set_exception_at_thread_exit(exception_ptr __p)
       {
-	_M_future->_M_set_delayed_result(_State::__setter(__p, this),
+	_M_state()._M_set_delayed_result(_State::__setter(__p, this),
 					 _M_future);
       }
+
+    private:
+      _State&
+      _M_state()
+      {
+	__future_base::_State_base::_S_check(_M_future);
+	return *_M_future;
+      }
     };
 
   /// Explicit specialization for promise<void>
@@ -1333,22 +1349,30 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       // Setting the result
       void
       set_value()
-      { _M_future->_M_set_result(_State::__setter(this)); }
+      { _M_state()._M_set_result(_State::__setter(this)); }
 
       void
       set_exception(exception_ptr __p)
-      { _M_future->_M_set_result(_State::__setter(__p, this)); }
+      { _M_state()._M_set_result(_State::__setter(__p, this)); }
 
       void
       set_value_at_thread_exit()
-      { _M_future->_M_set_delayed_result(_State::__setter(this), _M_future); }
+      { _M_state()._M_set_delayed_result(_State::__setter(this), _M_future); }
 
       void
       set_exception_at_thread_exit(exception_ptr __p)
       {
-	_M_future->_M_set_delayed_result(_State::__setter(__p, this),
+	_M_state()._M_set_delayed_result(_State::__setter(__p, this),
 					 _M_future);
       }
+
+    private:
+      _State&
+      _M_state()
+      {
+	__future_base::_State_base::_S_check(_M_future);
+	return *_M_future;
+      }
     };
 
   template<typename _Ptr_type, typename _Fn, typename _Res>
diff --git a/libstdc++-v3/testsuite/30_threads/promise/members/at_thread_exit2.cc b/libstdc++-v3/testsuite/30_threads/promise/members/at_thread_exit2.cc
index 9f824efde52..679d580cee3 100644
--- a/libstdc++-v3/testsuite/30_threads/promise/members/at_thread_exit2.cc
+++ b/libstdc++-v3/testsuite/30_threads/promise/members/at_thread_exit2.cc
@@ -118,7 +118,6 @@ void test02()
 void test03()
 {
   std::promise<void> p1;
-  int i = 0;
   p1.set_value();
   try {
     p1.set_value_at_thread_exit();

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

end of thread, other threads:[~2021-05-04 22:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-04 22:17 [committed 1/4] libstdc++ Fix undefined behaviour in testsuite Jonathan Wakely
2021-05-04 22:18 ` [committed 2/4] libstdc++: Fix null dereference in pb_ds containers Jonathan Wakely
2021-05-04 22:19 ` [committed 3/4] libstdc++: Fix undefined behaviour in std::string Jonathan Wakely
2021-05-04 22:20 ` [committed 4/4] libstdc++: Fix null dereferences in std::promise Jonathan Wakely

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