[libc++] Fix `std::out_of_range` thrown from `basic_stringbuf::str() &&`

[pfusik]

Fixes a bug reported by @leonardchan at https://reviews.llvm.org/D155276
In the bug scenario, __view.data() == nullptr. The offset is validated by the string constructor/assign even if count == 0.

[Mordante]

I just fixed the CI can you rebase this patch on main?

[Mordante]

Thanks for the quick fix! LGTM modulo a comment.

After you land the patch, can you put it up for backporting to LLVM-17?
(The instructions are here https://www.llvm.org/docs/GitHub.html#backporting-fixes-to-the-release-branches)

libcxx/include/sstream

405

Depending on the order of evaluation __view.data() - __str_.data() may be used after __str_ has been moved. Lets determine the size before assign. This is pre-existing.

Whether this is save depends on the order in which the function arguments are evaluated. Clang and GCC use a different order.

[pfusik]

libcxx/include/sstream

405

I was also having this concern for a moment. :) But std::move is just a cast that does not modify its argument. The actual move is inside assign.

[pfusik]

Rebased.

[Mordante]

libcxx/include/sstream

405

Good point, you're right. It only does a cast and nothing else.

[philnik]

libcxx/include/sstream

405

While that is true, I think moving the calculation out of the function call doesn't hurt. This would be a real bug if assign took the string by value, which makes this really subtile and definitely worthy of a comment if we were to keep it this way.

[pfusik]

Refactor a function argument to a variable (NFC).

[pfusik]

Rebased.

[pfusik]

In D156783#4550729, @Mordante wrote:

After you land the patch, can you put it up for backporting to LLVM-17?
(The instructions are here https://www.llvm.org/docs/GitHub.html#backporting-fixes-to-the-release-branches)

https://github.com/llvm/llvm-project/issues/64346

libcxx/include/sstream

 #endif
 
 #if _LIBCPP_STD_VER <= 17 || defined(_LIBCPP_BUILDING_LIBRARY)
     string_type str() const;
 #else
     _LIBCPP_HIDE_FROM_ABI_SSTREAM string_type str() const & { return str(__str_.get_allocator()); }
 
     _LIBCPP_HIDE_FROM_ABI_SSTREAM string_type str() && {
+        string_type __result;
         const basic_string_view<_CharT, _Traits> __view = view();
-        string_type __result(std::move(__str_), __view.data() - __str_.data(), __view.size());
+        if (!__view.empty()) {
+            auto __pos = __view.data() - __str_.data();
+            __result.assign(std::move(__str_), __pos, __view.size());
+        }
         __str_.clear();
         __init_buf_ptrs();
         return __result;
     }
 #endif // _LIBCPP_STD_VER <= 17 || defined(_LIBCPP_BUILDING_LIBRARY)
 
 #if _LIBCPP_STD_VER >= 20
     template <class _SAlloc>

libcxx/test/std/input.output/string.streams/istringstream/istringstream.members/str.move.pass.cpp

 template <class CharT>
 static void test() {
   {
     std::basic_istringstream<CharT> ss(STR("testing"));
     std::basic_string<CharT> s = std::move(ss).str();
     assert(s == STR("testing"));
     assert(ss.view().empty());
   }
+  {
+    std::basic_istringstream<CharT> ss;
+    std::basic_string<CharT> s = std::move(ss).str();
+    assert(s.empty());
+    assert(ss.view().empty());
+  }
 }
 
 int main(int, char**) {
   test<char>();
 #ifndef TEST_HAS_NO_WIDE_CHARACTERS
   test<wchar_t>();
 #endif
   return 0;
 }

libcxx/test/std/input.output/string.streams/ostringstream/ostringstream.members/str.move.pass.cpp

 template <class CharT>
 static void test() {
   {
     std::basic_ostringstream<CharT> ss(STR("testing"));
     std::basic_string<CharT> s = std::move(ss).str();
     assert(s == STR("testing"));
     assert(ss.view().empty());
   }
+  {
+    std::basic_ostringstream<CharT> ss;
+    std::basic_string<CharT> s = std::move(ss).str();
+    assert(s.empty());
+    assert(ss.view().empty());
+  }
 }
 
 int main(int, char**) {
   test<char>();
 #ifndef TEST_HAS_NO_WIDE_CHARACTERS
   test<wchar_t>();
 #endif
   return 0;
 }

libcxx/test/std/input.output/string.streams/stringbuf/stringbuf.members/str.move.pass.cpp

 template <class CharT>
 static void test() {
   {
     std::basic_stringbuf<CharT> buf(STR("testing"));
     std::basic_string<CharT> s = std::move(buf).str();
     assert(s == STR("testing"));
     assert(buf.view().empty());
   }
+  {
+    std::basic_stringbuf<CharT> buf;
+    std::basic_string<CharT> s = std::move(buf).str();
+    assert(s.empty());
+    assert(buf.view().empty());
+  }
 }
 
 int main(int, char**) {
   test<char>();
 #ifndef TEST_HAS_NO_WIDE_CHARACTERS
   test<wchar_t>();
 #endif
   return 0;
 }

libcxx/test/std/input.output/string.streams/stringstream/stringstream.members/str.move.pass.cpp

 template <class CharT>
 static void test() {
   {
     std::basic_stringstream<CharT> ss(STR("testing"));
     std::basic_string<CharT> s = std::move(ss).str();
     assert(s == STR("testing"));
     assert(ss.view().empty());
   }
+  {
+    std::basic_stringstream<CharT> ss;
+    std::basic_string<CharT> s = std::move(ss).str();
+    assert(s.empty());
+    assert(ss.view().empty());
+  }
 }
 
 int main(int, char**) {
   test<char>();
 #ifndef TEST_HAS_NO_WIDE_CHARACTERS
   test<wchar_t>();
 #endif
   return 0;
 }