https://gcc.gnu.org/g:beb0ffd36eedf0542d7f408e87efee4bee3150f8
commit r15-9709-gbeb0ffd36eedf0542d7f408e87efee4bee3150f8
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Wed Apr 30 17:31:01 2025 +0100
libstdc++: Fix dangling pointer in fs::path::operator+=(*this) [PR120029]
When concatenating a path we reallocate the left operand's storage to
make room for the new components being added. When the two operands are
the same object, or the right operand is one of the components of the
left operand, the reallocation invalidates the pointers that refer
into the right operand's storage.
The solution in this commit is to detect these aliasing cases and just
do the concatenation in terms of the contained string, as that code
already handles the case where the string aliases the path. The standard
specifies the concatenation in terms of the native() string, so all this
change does is disable the optimized implementation of concatenation for
path objects which attempts to avoid re-parsing the path from the
concatenated string.
The potential loss of performance for this case isn't likely to be an
issue, because concatenating a path with itself (or one of its existing
components) probably isn't a common use case.
The Filesystem TS implementation doesn't have the optimized form of
concatenation and always does it in terms of the native string and
reparsing the whole thing, so doesn't have this bug. A test is added to
confirm that anyway (that test has some slightly different results due
to different behaviour for trailing slashes and implicit "." filenames
in the TS spec).
libstdc++-v3/ChangeLog:
PR libstdc++/120029
* src/c++17/fs_path.cc (path::operator+=(const path&)): Handle
parameters that alias the path or one of its components.
* testsuite/27_io/filesystem/path/concat/120029.cc: New test.
* testsuite/experimental/filesystem/path/concat/120029.cc: New
test.
(cherry picked from commit a067cbcdcc5f599a2b7d607e89674533d23c652d)
Diff:
---
libstdc++-v3/src/c++17/fs_path.cc | 10 +++
.../27_io/filesystem/path/concat/120029.cc | 72 +++++++++++++++++++++
.../experimental/filesystem/path/concat/120029.cc | 74 ++++++++++++++++++++++
3 files changed, 156 insertions(+)
diff --git a/libstdc++-v3/src/c++17/fs_path.cc
b/libstdc++-v3/src/c++17/fs_path.cc
index 6582f10209a0..215afa08ad25 100644
--- a/libstdc++-v3/src/c++17/fs_path.cc
+++ b/libstdc++-v3/src/c++17/fs_path.cc
@@ -880,6 +880,16 @@ path::operator+=(const path& p)
return *this;
}
+ // Handle p += p which would otherwise access dangling pointers after
+ // reallocating _M_cmpts and _M_pathname.
+ if (&p == this) [[unlikely]]
+ return *this += p.native();
+ // Handle p += *i where i is in [p.begin(),p.end()), for the same reason.
+ if (_M_type() == _Type::_Multi && p._M_type() != _Type::_Multi)
+ for (const path& cmpt : *this)
+ if (&cmpt == &p) [[unlikely]]
+ return *this += p.native();
+
#if _GLIBCXX_FILESYSTEM_IS_WINDOWS
if (_M_type() == _Type::_Root_name
|| (_M_type() == _Type::_Filename && _M_pathname.size() == 1))
diff --git a/libstdc++-v3/testsuite/27_io/filesystem/path/concat/120029.cc
b/libstdc++-v3/testsuite/27_io/filesystem/path/concat/120029.cc
new file mode 100644
index 000000000000..5153d594b50f
--- /dev/null
+++ b/libstdc++-v3/testsuite/27_io/filesystem/path/concat/120029.cc
@@ -0,0 +1,72 @@
+// { dg-do run { target c++17 } }
+
+// Bug libstdc++/120029
+// Dangling iterator usage in path::operator+=(const path& p) when this == p
+
+#include <filesystem>
+#include <testsuite_hooks.h>
+
+namespace fs = std::filesystem;
+
+void
+test_root_dir()
+{
+ fs::path p = "/";
+ p += p;
+ p += p;
+ VERIFY( p == "////" );
+ p += p.filename();
+ VERIFY( p == "////" );
+ p += *std::prev(p.end());
+ VERIFY( p == "////" );
+}
+
+void
+test_root_name()
+{
+ fs::path p = "C:/";
+ p += p;
+ p += p;
+ VERIFY( p == "C:/C:/C:/C:/" );
+ p += p.filename();
+ VERIFY( p == "C:/C:/C:/C:/" );
+ p += *std::prev(p.end());
+ VERIFY( p == "C:/C:/C:/C:/" );
+}
+
+void
+test_filename()
+{
+ fs::path p = "file";
+ p += p;
+ p += p;
+ VERIFY( p == "filefilefilefile" );
+ p += p.filename();
+ VERIFY( p == "filefilefilefilefilefilefilefile" );
+ p += *std::prev(p.end());
+ VERIFY( p ==
"filefilefilefilefilefilefilefilefilefilefilefilefilefilefilefile" );
+}
+
+void
+test_multi()
+{
+ fs::path p = "/home/username/Documents/mu";
+ p += p;
+ p += p;
+ VERIFY( p ==
"/home/username/Documents/mu/home/username/Documents/mu/home/username/Documents/mu/home/username/Documents/mu"
);
+ p += p.filename();
+ VERIFY( p ==
"/home/username/Documents/mu/home/username/Documents/mu/home/username/Documents/mu/home/username/Documents/mumu"
);
+ p += *std::prev(p.end());
+ VERIFY( p ==
"/home/username/Documents/mu/home/username/Documents/mu/home/username/Documents/mu/home/username/Documents/mumumumu"
);
+ auto n = std::distance(p.begin(), p.end());
+ for (int i = 0; i < n; ++i)
+ p += *std::next(p.begin(), i);
+}
+
+int main()
+{
+ test_root_dir();
+ test_root_name();
+ test_filename();
+ test_multi();
+}
diff --git
a/libstdc++-v3/testsuite/experimental/filesystem/path/concat/120029.cc
b/libstdc++-v3/testsuite/experimental/filesystem/path/concat/120029.cc
new file mode 100644
index 000000000000..209d96806c6a
--- /dev/null
+++ b/libstdc++-v3/testsuite/experimental/filesystem/path/concat/120029.cc
@@ -0,0 +1,74 @@
+// { dg-options "-DUSE_FILESYSTEM_TS -lstdc++fs" }
+// { dg-do run { target c++11 } }
+// { dg-require-filesystem-ts "" }
+
+// Bug libstdc++/120029
+// Dangling iterator usage in path::operator+=(const path& p) when this == p
+
+#include <experimental/filesystem>
+#include <testsuite_hooks.h>
+
+namespace fs = std::experimental::filesystem;
+
+void
+test_root_dir()
+{
+ fs::path p = "/";
+ p += p;
+ p += p;
+ VERIFY( p == "////" );
+ p += p.filename();
+ VERIFY( p == "////////" );
+ p += *std::prev(p.end());
+ VERIFY( p == "////////////////" );
+}
+
+void
+test_root_name()
+{
+ fs::path p = "C:/";
+ p += p;
+ p += p;
+ VERIFY( p == "C:/C:/C:/C:/" );
+ p += p.filename(); // For Filesystem TS the filename is "."
+ VERIFY( p == "C:/C:/C:/C:/." );
+ p += *std::prev(p.end());
+ VERIFY( p == "C:/C:/C:/C:/.." );
+}
+
+void
+test_filename()
+{
+ fs::path p = "file";
+ p += p;
+ p += p;
+ VERIFY( p == "filefilefilefile" );
+ p += p.filename();
+ VERIFY( p == "filefilefilefilefilefilefilefile" );
+ p += *std::prev(p.end());
+ VERIFY( p ==
"filefilefilefilefilefilefilefilefilefilefilefilefilefilefilefile" );
+}
+
+void
+test_multi()
+{
+ fs::path p = "/home/username/Documents/mu";
+ p += p;
+ p += p;
+ VERIFY( p ==
"/home/username/Documents/mu/home/username/Documents/mu/home/username/Documents/mu/home/username/Documents/mu"
);
+ p += p.filename();
+ VERIFY( p ==
"/home/username/Documents/mu/home/username/Documents/mu/home/username/Documents/mu/home/username/Documents/mumu"
);
+ p += *std::prev(p.end());
+ VERIFY( p ==
"/home/username/Documents/mu/home/username/Documents/mu/home/username/Documents/mu/home/username/Documents/mumumumu"
);
+ auto n = std::distance(p.begin(), p.end());
+ for (int i = 0; i < n; ++i)
+ p += *std::next(p.begin(), i);
+}
+
+int main()
+{
+ test_root_dir();
+ test_root_name();
+ test_filename();
+ test_multi();
+}
