[PATCH] libstdc++: Fix constexpr basic_string union member [PR113294]

[PATCH] libstdc++: Fix constexpr basic_string union member [PR113294]

[Paul Keir]

A call to `basic_string::clear()` in the std::string move assignment operator leads to a constexpr error from an access of inactive union member `_M_local_buf` in the added test (`test_move()`). Changing `__str._M_local_buf` to `__str._M_use_local_data()` in `operator=(basic_string&& __str)` fixes this.

Running `make check RUNTESTFLAGS="conformance.exp=21_strings/basic_string/*"` on x86_64-pc-linux-gnu gave unchanged summary results (# of expected passes 570 # of expected failures 4 # of unsupported tests 4).

I don't have write access.

Signed-off-by: Paul Keir <paul.keir@uws.ac.uk>

diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
index 43efc99bea9..8a695a494ef 100644
--- a/libstdc++-v3/include/bits/basic_string.h
+++ b/libstdc++-v3/include/bits/basic_string.h
@@ -909,7 +909,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
                __str._M_capacity(__capacity);
              }
            else
-             __str._M_data(__str._M_local_buf);
+             __str._M_data(__str._M_use_local_data());
          }
        else // Need to do a deep copy
          assign(__str);
diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/constexpr.cc b/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/constexpr.cc
index 0e28a6d4487..ad7548f3236 100644
--- a/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/constexpr.cc
+++ b/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/constexpr.cc
@@ -50,3 +50,16 @@ test_erasure()
 }

 static_assert( test_erasure() );
+
+constexpr bool
+test_move()
+{
+  std::string s1;
+  std::string s2 = "1234567890123456"; // 16 chars: more than _S_local_capacity
+  s1 = std::move(s2);
+  VERIFY( s1 == "1234567890123456" );
+
+  return true;
+}
+
+static_assert( test_move() );
--


Please consider the environment and think before you print.

The University of the West of Scotland is a registered Scottish charity. Charity number SC002520.

This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender.

Please note that any views or opinions presented in this email are solely those of the author and do not necessarily represent those of the University of the West of Scotland.

As a public body, the University of the West of Scotland may be required to make available emails as well as other written forms of information as a result of a request made under the Freedom of Information (Scotland) Act 2002.

Re: [PATCH] libstdc++: Fix constexpr basic_string union member [PR113294]

[Patrick Palka]

On Mon, 12 Feb 2024, Paul Keir wrote:

> A call to `basic_string::clear()` in the std::string move assignment operator leads to a constexpr error from an access of inactive union member `_M_local_buf` in the added test (`test_move()`). Changing `__str._M_local_buf` to `__str._M_use_local_data()` in `operator=(basic_string&& __str)` fixes this.
> 
> Running `make check RUNTESTFLAGS="conformance.exp=21_strings/basic_string/*"` on x86_64-pc-linux-gnu gave unchanged summary results (# of expected passes 570 # of expected failures 4 # of unsupported tests 4).
> 
> I don't have write access.
> 
> Signed-off-by: Paul Keir <paul.keir@uws.ac.uk>

LGTM!  A ChangeLog entry is missing from the commit message,
so here's one that we can use for this commit:

	PR libstdc++/113294

libstdc++-v3/ChangeLog:

	* include/bits/basic_string.h (basic_string::operator=): Use
	_M_use_local_data() instead of _M_local_buf on the moved-from
	string.
	* testsuite/21_strings/basic_string/modifiers/constexpr.cc
	(test_move): New test.

> 
> diff --git a/libstdc++-v3/include/bits/basic_string.h b/libstdc++-v3/include/bits/basic_string.h
> index 43efc99bea9..8a695a494ef 100644
> --- a/libstdc++-v3/include/bits/basic_string.h
> +++ b/libstdc++-v3/include/bits/basic_string.h
> @@ -909,7 +909,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
>                 __str._M_capacity(__capacity);
>               }
>             else
> -             __str._M_data(__str._M_local_buf);
> +             __str._M_data(__str._M_use_local_data());
>           }
>         else // Need to do a deep copy
>           assign(__str);
> diff --git a/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/constexpr.cc b/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/constexpr.cc
> index 0e28a6d4487..ad7548f3236 100644
> --- a/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/constexpr.cc
> +++ b/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/constexpr.cc
> @@ -50,3 +50,16 @@ test_erasure()
>  }
> 
>  static_assert( test_erasure() );
> +
> +constexpr bool
> +test_move()
> +{
> +  std::string s1;
> +  std::string s2 = "1234567890123456"; // 16 chars: more than _S_local_capacity
> +  s1 = std::move(s2);
> +  VERIFY( s1 == "1234567890123456" );
> +
> +  return true;
> +}
> +
> +static_assert( test_move() );
> --
> 
> 
> Please consider the environment and think before you print.
> 
> The University of the West of Scotland is a registered Scottish charity. Charity number SC002520.
> 
> This e-mail and any attachment is for authorised use by the intended recipient(s) only. It may contain proprietary material, confidential information and/or be subject to legal privilege. It should not be copied, disclosed to, retained or used by, any other party. If you are not an intended recipient then please promptly delete this e-mail and any attachment and all copies and inform the sender.
> 
> Please note that any views or opinions presented in this email are solely those of the author and do not necessarily represent those of the University of the West of Scotland.
> 
> As a public body, the University of the West of Scotland may be required to make available emails as well as other written forms of information as a result of a request made under the Freedom of Information (Scotland) Act 2002.
> 
> 

Re: [PATCH] libstdc++: Fix constexpr basic_string union member [PR113294]

[Jonathan Wakely]

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

On Mon, 12 Feb 2024 at 22:17, Patrick Palka <ppalka@redhat.com> wrote:

> On Mon, 12 Feb 2024, Paul Keir wrote:
>
> > A call to `basic_string::clear()` in the std::string move assignment
> operator leads to a constexpr error from an access of inactive union member
> `_M_local_buf` in the added test (`test_move()`). Changing
> `__str._M_local_buf` to `__str._M_use_local_data()` in
> `operator=(basic_string&& __str)` fixes this.
> >
> > Running `make check
> RUNTESTFLAGS="conformance.exp=21_strings/basic_string/*"` on
> x86_64-pc-linux-gnu gave unchanged summary results (# of expected passes
> 570 # of expected failures 4 # of unsupported tests 4).
> >
> > I don't have write access.
> >
> > Signed-off-by: Paul Keir <paul.keir@uws.ac.uk>
>
> LGTM!  A ChangeLog entry is missing from the commit message,
> so here's one that we can use for this commit:
>
>         PR libstdc++/113294
>
> libstdc++-v3/ChangeLog:
>
>         * include/bits/basic_string.h (basic_string::operator=): Use
>         _M_use_local_data() instead of _M_local_buf on the moved-from
>         string.
>         * testsuite/21_strings/basic_string/modifiers/constexpr.cc
>         (test_move): New test.
>

The patch is OK for trunk and gcc-13 with that commit msg. Patrick, could
you push it for Paul?

Thank you both.



> >
> > diff --git a/libstdc++-v3/include/bits/basic_string.h
> b/libstdc++-v3/include/bits/basic_string.h
> > index 43efc99bea9..8a695a494ef 100644
> > --- a/libstdc++-v3/include/bits/basic_string.h
> > +++ b/libstdc++-v3/include/bits/basic_string.h
> > @@ -909,7 +909,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
> >                 __str._M_capacity(__capacity);
> >               }
> >             else
> > -             __str._M_data(__str._M_local_buf);
> > +             __str._M_data(__str._M_use_local_data());
> >           }
> >         else // Need to do a deep copy
> >           assign(__str);
> > diff --git
> a/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/constexpr.cc
> b/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/constexpr.cc
> > index 0e28a6d4487..ad7548f3236 100644
> > ---
> a/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/constexpr.cc
> > +++
> b/libstdc++-v3/testsuite/21_strings/basic_string/modifiers/constexpr.cc
> > @@ -50,3 +50,16 @@ test_erasure()
> >  }
> >
> >  static_assert( test_erasure() );
> > +
> > +constexpr bool
> > +test_move()
> > +{
> > +  std::string s1;
> > +  std::string s2 = "1234567890123456"; // 16 chars: more than
> _S_local_capacity
> > +  s1 = std::move(s2);
> > +  VERIFY( s1 == "1234567890123456" );
> > +
> > +  return true;
> > +}
> > +
> > +static_assert( test_move() );
> > --
> >
> >
> > Please consider the environment and think before you print.
> >
> > The University of the West of Scotland is a registered Scottish charity.
> Charity number SC002520.
> >
> > This e-mail and any attachment is for authorised use by the intended
> recipient(s) only. It may contain proprietary material, confidential
> information and/or be subject to legal privilege. It should not be copied,
> disclosed to, retained or used by, any other party. If you are not an
> intended recipient then please promptly delete this e-mail and any
> attachment and all copies and inform the sender.
> >
> > Please note that any views or opinions presented in this email are
> solely those of the author and do not necessarily represent those of the
> University of the West of Scotland.
> >
> > As a public body, the University of the West of Scotland may be required
> to make available emails as well as other written forms of information as a
> result of a request made under the Freedom of Information (Scotland) Act
> 2002.
> >
> >
>
>