Skip to content

Commit 626445b

Browse files
benlangmuircachemeifyoucan
authored andcommitted
[cas] Fix PathStorage::Path dangling pointer
The default copy/move for PathStorage leaves Path pointing to the previous storage, which could be a temporary. We could implement the copy/move operations, but they would need to copy the path storage small vector in general, which is slower than constructing it directly. rdar://160154209 (cherry picked from commit e4d082f)
1 parent de2f602 commit 626445b

File tree

2 files changed

+18
-2
lines changed

2 files changed

+18
-2
lines changed

llvm/include/llvm/CAS/FileSystemCache.h

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,14 @@ struct PathStorage {
3535
PathStorage() {}
3636
PathStorage(const Twine &InputPath,
3737
sys::path::Style Style = sys::path::Style::native) {
38+
assign(InputPath, Style);
39+
}
40+
PathStorage(StringRef InputPath,
41+
sys::path::Style Style = sys::path::Style::native) {
42+
assign(InputPath, Style);
43+
}
44+
void assign(const Twine &InputPath,
45+
sys::path::Style Style = sys::path::Style::native) {
3846
if (is_style_windows(Style)) {
3947
// Canonicalize to backslahes
4048
InputPath.toVector(Storage);
@@ -45,7 +53,7 @@ struct PathStorage {
4553
Path = Storage.str();
4654
}
4755
}
48-
PathStorage(StringRef InputPath,
56+
void assign(StringRef InputPath,
4957
sys::path::Style Style = sys::path::Style::native) {
5058
if (is_style_windows(Style)) {
5159
// Canonicalize to backslahes
@@ -56,6 +64,14 @@ struct PathStorage {
5664
Path = InputPath;
5765
}
5866
}
67+
68+
// Note: the default implementation of these would leave Path pointing to the
69+
// wrong storage. We could implement them, but they are also slower than
70+
// calling assign since they would copy the storage again.
71+
PathStorage(const PathStorage &) = delete;
72+
PathStorage(PathStorage &&) = delete;
73+
PathStorage &operator=(PathStorage &&) = delete;
74+
PathStorage &operator=(const PathStorage &) = delete;
5975
};
6076

6177
/// Caching for lazily discovering a CAS-based filesystem.

llvm/lib/CAS/CachingOnDiskFileSystem.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -915,7 +915,7 @@ void CachingOnDiskFileSystemImpl::TreeBuilder::pushEntry(
915915
}
916916

917917
Error CachingOnDiskFileSystemImpl::TreeBuilder::push(const Twine &Path) {
918-
Storage = PathStorage(Path);
918+
Storage.assign(Path);
919919
StringRef PathRef = Storage.Path;
920920

921921
// Look for Path without following symlinks. Failure here indicates that Path

0 commit comments

Comments
 (0)