[committed] libstdc++: Fix dangling string_view in filesystem::path [PR102592]

[committed] libstdc++: Fix dangling string_view in filesystem::path [PR102592]

[Jonathan Wakely]

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

When creating a path from a pair of non-contiguous iterators we pass the
iterators to _S_convert(Iter, Iter). That function passes the iterators
to __string_from_range to get a contiguous sequence of characters, and
then calls _S_convert(const C*, const C*) to perform the encoding
conversions. If the value type, C, is char8_t, then no conversion is
needed and the _S_convert<char8_t>(const char8_t*, const char8_t*)
specialization casts the pointer to const char* and returns a
std::string_view that refs to the char8_t sequence. However, that
sequence is owned by the std::u8string rvalue returned by
__string_from_range, which goes out of scope when _S_convert(Iter, Iter)
returns. That means the std::string_view is dangling and we get
undefined behaviour when parsing it as a path.

The same problem does not exist for the path members taking a "Source"
argument, because those functions all convert a non-contiguous range
into a basic_string<C> immediately, using __effective_range(__source).
That means that the rvalue string returned by that function is still in
scope for the full expression, so the string_view does not dangle.

The solution for the buggy functions is to do the same thing, and call
__string_from_range immediately, so that the returned rvalue is still in
scope for the lifetime of the string_view returned by _S_convert. To
avoid reintroducing the same problem, remove the _S_convert(Iter, Iter)
overload that calls __string_from_range and returns a dangling view.

libstdc++-v3/ChangeLog:

	PR libstdc++/102592
	* include/bits/fs_path.h (path::path(Iter, Iter, format))
	(path::append(Iter, Iter), path::concat(Iter, Iter)): Call
	__string_from_range directly, instead of two-argument overload
	of _S_convert.
	(path::_S_convert(Iter, Iter)): Remove.
	* testsuite/27_io/filesystem/path/construct/102592.C: New test.

Tested powerpc64le-linux. Committed to trunk.


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

commit 85b24e32dc27ec2e70b853713e0713cbc1ff08c3
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Oct 13 17:02:59 2021

    libstdc++: Fix dangling string_view in filesystem::path [PR102592]
    
    When creating a path from a pair of non-contiguous iterators we pass the
    iterators to _S_convert(Iter, Iter). That function passes the iterators
    to __string_from_range to get a contiguous sequence of characters, and
    then calls _S_convert(const C*, const C*) to perform the encoding
    conversions. If the value type, C, is char8_t, then no conversion is
    needed and the _S_convert<char8_t>(const char8_t*, const char8_t*)
    specialization casts the pointer to const char* and returns a
    std::string_view that refs to the char8_t sequence. However, that
    sequence is owned by the std::u8string rvalue returned by
    __string_from_range, which goes out of scope when _S_convert(Iter, Iter)
    returns. That means the std::string_view is dangling and we get
    undefined behaviour when parsing it as a path.
    
    The same problem does not exist for the path members taking a "Source"
    argument, because those functions all convert a non-contiguous range
    into a basic_string<C> immediately, using __effective_range(__source).
    That means that the rvalue string returned by that function is still in
    scope for the full expression, so the string_view does not dangle.
    
    The solution for the buggy functions is to do the same thing, and call
    __string_from_range immediately, so that the returned rvalue is still in
    scope for the lifetime of the string_view returned by _S_convert. To
    avoid reintroducing the same problem, remove the _S_convert(Iter, Iter)
    overload that calls __string_from_range and returns a dangling view.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/102592
            * include/bits/fs_path.h (path::path(Iter, Iter, format))
            (path::append(Iter, Iter), path::concat(Iter, Iter)): Call
            __string_from_range directly, instead of two-argument overload
            of _S_convert.
            (path::_S_convert(Iter, Iter)): Remove.
            * testsuite/27_io/filesystem/path/construct/102592.C: New test.

diff --git a/libstdc++-v3/include/bits/fs_path.h b/libstdc++-v3/include/bits/fs_path.h
index 1918c243d74..7ead8ac299c 100644
--- a/libstdc++-v3/include/bits/fs_path.h
+++ b/libstdc++-v3/include/bits/fs_path.h
@@ -292,7 +292,7 @@ namespace __detail
     template<typename _InputIterator,
 	     typename _Require = __detail::_Path2<_InputIterator>>
       path(_InputIterator __first, _InputIterator __last, format = auto_format)
-      : _M_pathname(_S_convert(__first, __last))
+      : _M_pathname(_S_convert(__detail::__string_from_range(__first, __last)))
       { _M_split_cmpts(); }
 
     template<typename _Source,
@@ -358,7 +358,7 @@ namespace __detail
       __detail::_Path2<_InputIterator>&
       append(_InputIterator __first, _InputIterator __last)
       {
-	_M_append(_S_convert(__first, __last));
+	_M_append(_S_convert(__detail::__string_from_range(__first, __last)));
 	return *this;
       }
 
@@ -390,7 +390,7 @@ namespace __detail
       __detail::_Path2<_InputIterator>&
       concat(_InputIterator __first, _InputIterator __last)
       {
-	_M_concat(_S_convert(__first, __last));
+	_M_concat(_S_convert(__detail::__string_from_range(__first, __last)));
 	return *this;
       }
 
@@ -602,11 +602,6 @@ namespace __detail
       static auto
       _S_convert(const _EcharT* __first, const _EcharT* __last);
 
-    template<typename _Iter>
-      static auto
-      _S_convert(_Iter __first, _Iter __last)
-      { return _S_convert(__detail::__string_from_range(__first, __last)); }
-
     static string_type
     _S_convert_loc(const char* __first, const char* __last,
 		   const std::locale& __loc);
diff --git a/libstdc++-v3/testsuite/27_io/filesystem/path/construct/102592.C b/libstdc++-v3/testsuite/27_io/filesystem/path/construct/102592.C
new file mode 100644
index 00000000000..3bbd07e2494
--- /dev/null
+++ b/libstdc++-v3/testsuite/27_io/filesystem/path/construct/102592.C
@@ -0,0 +1,28 @@
+// { dg-options "-fchar8_t" }
+// { dg-do run { target c++17 } }
+
+#include <filesystem>
+#include <testsuite_hooks.h>
+#include <testsuite_iterators.h>
+
+using __gnu_test::input_iterator_wrapper;
+using __gnu_test::input_container;
+
+void test01()
+{
+  const char8_t src[] = u8"/long/path/to/a/file/to/avoid/small/string";
+  input_container<const char8_t> c1(src);     // includes null terminator
+  std::filesystem::path p1(c1.begin());       // read up to null terminator
+  VERIFY( p1.u8string() == src );
+
+  std::u8string_view sv = src;
+  input_container<const char8_t> c2(sv.data(), sv.data() + sv.size());
+  std::filesystem::path p2(c2.begin(), c2.end()); // PR libstdc++/102592
+  VERIFY( p2.u8string() == src );
+  VERIFY( p1 == p2 );
+}
+
+int main()
+{
+  test01();
+}