[committed] libstdc++: Handle type-changing path concatenations (PR 94063)

Message ID 20200309232733.GA3335225@redhat.com
State New
Headers show
Series
  • [committed] libstdc++: Handle type-changing path concatenations (PR 94063)
Related show

Commit Message

Jonathan Wakely March 9, 2020, 11:27 p.m.
The filesystem::path::operator+= and filesystem::path::concat functions
operate directly on the native format of the path and so can cause a
path to mutate to a completely different type.

For Windows combining a filename "x" with a filename ":" produces a
root-name "x:". Similarly, a Cygwin root-directory "/" combined with a
root-directory and filename "/x" produces a root-name "//x".

Before this patch the implemenation didn't support those kind of
mutations, assuming that concatenating two filenames would always
produce a filename and concatenating with a root-dir would still have a
root-dir.

This patch fixes it simply by checking for the problem cases and
creating a new path by re-parsing the result of the string
concatenation. This is slightly suboptimal because the argument has
already been parsed if it's a path, but more importantly it doesn't
reuse any excess capacity that the path object being modified might
already have allocated. That can be fixed later though.

	PR libstdc++/94063
	* src/c++17/fs_path.cc (path::operator+=(const path&)): Add kluge to
	handle concatenations that change the type of the first component.
	(path::operator+=(basic_string_view<value_type>)): Likewise.
	* testsuite/27_io/filesystem/path/concat/94063.cc: New test.

Tested x86_64-pc-linux-gnu, x86_64-w64-mingw32 and x86_64-pc-cygwin.

Committed to master.

I'm very grateful to MetaNova on #cygwin for giving me access to a
machine for Cygwin testing.
commit ea182fe63634bb5b7913b3f1b6846e1900c5e0c4
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Mar 9 23:22:57 2020 +0000

    libstdc++: Handle type-changing path concatenations (PR 94063)
    
    The filesystem::path::operator+= and filesystem::path::concat functions
    operate directly on the native format of the path and so can cause a
    path to mutate to a completely different type.
    
    For Windows combining a filename "x" with a filename ":" produces a
    root-name "x:". Similarly, a Cygwin root-directory "/" combined with a
    root-directory and filename "/x" produces a root-name "//x".
    
    Before this patch the implemenation didn't support those kind of
    mutations, assuming that concatenating two filenames would always
    produce a filename and concatenating with a root-dir would still have a
    root-dir.
    
    This patch fixes it simply by checking for the problem cases and
    creating a new path by re-parsing the result of the string
    concatenation. This is slightly suboptimal because the argument has
    already been parsed if it's a path, but more importantly it doesn't
    reuse any excess capacity that the path object being modified might
    already have allocated. That can be fixed later though.
    
            PR libstdc++/94063
            * src/c++17/fs_path.cc (path::operator+=(const path&)): Add kluge to
            handle concatenations that change the type of the first component.
            (path::operator+=(basic_string_view<value_type>)): Likewise.
            * testsuite/27_io/filesystem/path/concat/94063.cc: New test.

Patch

diff --git a/libstdc++-v3/src/c++17/fs_path.cc b/libstdc++-v3/src/c++17/fs_path.cc
index 73071d8d052..5ff17741f81 100644
--- a/libstdc++-v3/src/c++17/fs_path.cc
+++ b/libstdc++-v3/src/c++17/fs_path.cc
@@ -852,6 +852,26 @@  path::operator+=(const path& p)
       return *this;
     }
 
+#if _GLIBCXX_FILESYSTEM_IS_WINDOWS
+  if (_M_type() == _Type::_Root_name
+      || (_M_type() == _Type::_Filename && _M_pathname.size() == 1))
+    {
+      // Handle path("C") += path(":") and path("C:") += path("/x")
+      // FIXME: do this more efficiently
+      *this = path(_M_pathname + p._M_pathname);
+      return *this;
+    }
+#endif
+#if SLASHSLASH_IS_ROOTNAME
+  if (_M_type() == _Type::_Root_dir)
+    {
+      // Handle path("/") += path("/x") and path("//") += path("x")
+      // FIXME: do this more efficiently
+      *this = path(_M_pathname + p._M_pathname);
+      return *this;
+    }
+#endif
+
   const auto orig_pathlen = _M_pathname.length();
   const auto orig_type = _M_type();
   const auto orig_size = _M_cmpts.size();
@@ -1038,6 +1058,26 @@  path::_M_concat(basic_string_view<value_type> s)
       return;
     }
 
+#if _GLIBCXX_FILESYSTEM_IS_WINDOWS
+  if (_M_type() == _Type::_Root_name
+      || (_M_type() == _Type::_Filename && _M_pathname.size() == 1))
+    {
+      // Handle path("C") += ":" and path("C:") += "/x"
+      // FIXME: do this more efficiently
+      *this = path(_M_pathname + string_type(s));
+      return;
+    }
+#endif
+#if SLASHSLASH_IS_ROOTNAME
+  if (_M_type() == _Type::_Root_dir)
+    {
+      // Handle path("/") += "/x" and path("//") += "x"
+      // FIXME: do this more efficiently
+      *this = path(_M_pathname + string_type(s));
+      return;
+    }
+#endif
+
   const auto orig_pathlen = _M_pathname.length();
   const auto orig_type = _M_type();
   const auto orig_size = _M_cmpts.size();
diff --git a/libstdc++-v3/testsuite/27_io/filesystem/path/concat/94063.cc b/libstdc++-v3/testsuite/27_io/filesystem/path/concat/94063.cc
new file mode 100644
index 00000000000..9f4c9c0aa08
--- /dev/null
+++ b/libstdc++-v3/testsuite/27_io/filesystem/path/concat/94063.cc
@@ -0,0 +1,111 @@ 
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++17" }
+// { dg-do run { target { *-*-*mingw* || *-*-cygwin } } }
+// { dg-require-effective-target c++17 }
+
+#include <filesystem>
+#include <testsuite_hooks.h>
+
+void
+test01()
+{
+  using std::filesystem::path;
+  path p;
+
+  // PR libstdc++/94063
+#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
+  p = L"C";
+  p += path(L":");
+  VERIFY( p.has_root_name() );
+  VERIFY( p.root_name() == p );
+  p += path(L"\\");
+  VERIFY( p.has_root_name() );
+  VERIFY( p.has_root_directory() );
+  VERIFY( p.root_name() == L"C:" );
+  VERIFY( p.root_directory() == L"\\" );
+
+  p = L"C";
+  p += L':';
+  VERIFY( p.has_root_name() );
+  VERIFY( p.root_name() == p );
+  p += L'\\';
+  VERIFY( p.has_root_name() );
+  VERIFY( p.has_root_directory() );
+  VERIFY( p.root_name() == L"C:" );
+  VERIFY( p.root_directory() == L"\\" );
+
+  p = L"C:";
+  p += path(L"/foo");
+  VERIFY( p.has_root_name() );
+  VERIFY( p.has_root_directory() );
+  VERIFY( p.root_name() == L"C:" );
+  VERIFY( p.root_directory() == L"/" );
+  VERIFY( p.filename() == L"foo" );
+
+  p = L"C:";
+  p += L"/foo";
+  VERIFY( p.has_root_name() );
+  VERIFY( p.has_root_directory() );
+  VERIFY( p.root_name() == L"C:" );
+  VERIFY( p.root_directory() == L"/" );
+  VERIFY( p.filename() == L"foo" );
+
+  p = L"C";
+  p += path(L":/foo");
+  VERIFY( p.has_root_name() );
+  VERIFY( p.has_root_directory() );
+  VERIFY( p.root_name() == L"C:" );
+  VERIFY( p.root_directory() == L"/" );
+  VERIFY( p.filename() == L"foo" );
+
+  p = L"C";
+  p += L":/foo";
+  VERIFY( p.has_root_name() );
+  VERIFY( p.has_root_directory() );
+  VERIFY( p.root_name() == L"C:" );
+  VERIFY( p.root_directory() == L"/" );
+  VERIFY( p.filename() == L"foo" );
+#elif defined __CYGWIN__
+  p = "/";
+  p += path("/x");
+  VERIFY( p.has_root_name() );
+  VERIFY( p.root_name() == p );
+
+  p = "/";
+  p += "/x";
+  VERIFY( p.has_root_name() );
+  VERIFY( p.root_name() == p );
+
+  p = "/";
+  p += path("/");
+  VERIFY( !p.has_root_name() );
+  VERIFY( p.has_root_directory() );
+
+  p = "/";
+  p += "/";
+  VERIFY( !p.has_root_name() );
+  VERIFY( p.has_root_directory() );
+#endif
+}
+
+int
+main()
+{
+  test01();
+}