diff options
| author | Brad King <brad.king@kitware.com> | 2022-10-20 17:59:03 -0400 |
|---|---|---|
| committer | Brad King <brad.king@kitware.com> | 2022-10-20 18:31:50 -0400 |
| commit | ee9805ccd17670e945e41e62f160bfdd9ad8a386 (patch) | |
| tree | 402a9d03107dee1098eb96d58357bbaeff63aec9 /Utilities | |
| parent | 96172ba2d18b98b2a5b651dee5326d7298c5891c (diff) | |
| download | cmake-ee9805ccd17670e945e41e62f160bfdd9ad8a386.tar.gz | |
cm/filesystem: Fix crash with pre-C++11 std::string GNU ABI in C++17
The `remove_filename` and `replace_extension` methods compute an offset
between the whole path in a `std::string` and a part of a path in a
`std::string_view`. This is done by subtracting their `.data()`
pointers. However, C++17 adds a non-const `.data()` through which
modification of the string is allowed. This means the copy-on-write
implementation used by the pre-C++11 std::string GNU ABI must reallocate
if the string has been copied. Our subtraction then computes an offset
between two different allocations, which is undefined behavior.
The workaround in commit b3ca4f9ad1 (cm/filesystem: Work around crash
when compiled for CYGWIN/MSYS runtime, 2021-04-22, v3.21.0-rc1~271^2~2)
avoided the problem by calling the non-const `.data()` to reallocate
before constructing the `string_view`. Instead, explicitly call the
const `.data()` method on the string, which does not reallocate.
Fixes: #22090, #23328
Diffstat (limited to 'Utilities')
| -rw-r--r-- | Utilities/std/cm/filesystem | 16 |
1 files changed, 6 insertions, 10 deletions
diff --git a/Utilities/std/cm/filesystem b/Utilities/std/cm/filesystem index ce52fbf3b3..b1cb3668f4 100644 --- a/Utilities/std/cm/filesystem +++ b/Utilities/std/cm/filesystem @@ -809,13 +809,11 @@ public: path& remove_filename() { -# if defined(__CYGWIN__) - // FIXME: Avoid crash due to CYGWIN/MSYS bug(?). See CMake Issue 22090. - static_cast<void>(this->path_.data()); -# endif auto fname = this->get_filename(); if (!fname.empty()) { - this->path_.erase(fname.data() - this->path_.data()); + this->path_.erase(fname.data() - + // Avoid C++17 non-const .data() that may reallocate. + static_cast<path_type const&>(this->path_).data()); } return *this; } @@ -829,13 +827,11 @@ public: path& replace_extension(const path& replacement = path()) { -# if defined(__CYGWIN__) - // FIXME: Avoid crash due to CYGWIN/MSYS bug(?). See CMake Issue 22090. - static_cast<void>(this->path_.data()); -# endif auto ext = this->get_filename_fragment(filename_fragment::extension); if (!ext.empty()) { - this->path_.erase(ext.data() - this->path_.data()); + this->path_.erase(ext.data() - + // Avoid C++17 non-const .data() that may reallocate. + static_cast<path_type const&>(this->path_).data()); } if (!replacement.path_.empty()) { if (replacement.path_[0] != '.') { |