echo/CMake
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

cm/filesystem: Fix crash with pre-C++11 std::string GNU ABI in C++17

Browse Source 
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
This commit is contained in:
Brad King 2022-10-20 17:59:03 -04:00
parent 96172ba2d1
commit ee9805ccd1
3 changed files with 18 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
Source/Checks/cm_cxx_features.cmake
View File

@ -80,9 +80,7 @@ if(CMake_HAVE_CXX_MAKE_UNIQUE)
set(CMake_HAVE_CXX_UNIQUE_PTR 1)
endif()
cm_check_cxx_feature(unique_ptr)
if (NOT CMAKE_CXX_STANDARD LESS "17"
AND NOT MSYS # FIXME: RunCMake.cmake_path cases crash with MSYS std::filesystem
)
if (NOT CMAKE_CXX_STANDARD LESS "17")
if (NOT CMAKE_CROSSCOMPILING OR CMAKE_CROSSCOMPILING_EMULATOR)
cm_check_cxx_feature(filesystem TRY_RUN)
else()

11
Source/Checks/cm_cxx_filesystem.cxx
View File

@ -23,5 +23,16 @@ int main()
}
#endif
// If std::string is copy-on-write, the std::filesystem::path
// implementation may accidentally trigger a reallocation and compute
// an offset between two allocations, leading to undefined behavior.
#if defined(__GLIBCXX__) && \
(!defined(_GLIBCXX_USE_CXX11_ABI) || !_GLIBCXX_USE_CXX11_ABI)
std::string p5s1 = "/path";
std::string p5s2 = std::move(p5s1);
std::filesystem::path p5 = std::string(p5s2);
p5.remove_filename();
#endif
return 0;
}

16
Utilities/std/cm/filesystem
View File

@ -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] != '.') {