Skip to content

Commit f042a64

Browse files
zmodemzmodem
committed
Merging r366455 and r366559:
------------------------------------------------------------------------ r366455 | kadircet | 2019-07-18 18:13:23 +0200 (Thu, 18 Jul 2019) | 9 lines [clangd] Get rid of dots and dotsdots within GlobalCompilationDatabase Reviewers: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D64860 ------------------------------------------------------------------------ ------------------------------------------------------------------------ r366559 | kadircet | 2019-07-19 12:18:52 +0200 (Fri, 19 Jul 2019) | 19 lines Revert "Revert r366458, r366467 and r366468" This reverts commit 9c37710. [clangd][BackgroundIndexLoader] Directly store DependentTU while loading shard Summary: We were deferring the population of DependentTU field in LoadedShard until BackgroundIndexLoader was consumed. This actually triggers a use after free since the shards FileToTU was pointing at could've been moved while consuming the Loader. Reviewers: sammccall Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D64980 ------------------------------------------------------------------------ llvm-svn: 366718
1 parent f3456bb commit f042a64

19 files changed

+421
-239
lines changed

‎clang-tools-extra/clangd/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ add_clang_library(clangDaemon
7373
XRefs.cpp
7474

7575
index/Background.cpp
76+
index/BackgroundIndexLoader.cpp
7677
index/BackgroundIndexStorage.cpp
7778
index/BackgroundQueue.cpp
7879
index/BackgroundRebuild.cpp

‎clang-tools-extra/clangd/ClangdServer.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,8 @@ ClangdServer::ClangdServer(const GlobalCompilationDatabase &CDB,
127127
if (Opts.BackgroundIndex) {
128128
BackgroundIdx = llvm::make_unique<BackgroundIndex>(
129129
Context::current().clone(), FSProvider, CDB,
130-
BackgroundIndexStorage::createDiskBackedStorageFactory());
130+
BackgroundIndexStorage::createDiskBackedStorageFactory(
131+
[&CDB](llvm::StringRef File) { return CDB.getProjectInfo(File); }));
131132
AddIndex(BackgroundIdx.get());
132133
}
133134
if (DynamicIdx)

‎clang-tools-extra/clangd/FS.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,5 +111,11 @@ PreambleFileStatusCache::getConsumingFS(
111111
return llvm::IntrusiveRefCntPtr<CacheVFS>(new CacheVFS(std::move(FS), *this));
112112
}
113113

114+
Path removeDots(PathRef File) {
115+
llvm::SmallString<128> CanonPath(File);
116+
llvm::sys::path::remove_dots(CanonPath, /*remove_dot_dot=*/true);
117+
return CanonPath.str().str();
118+
}
119+
114120
} // namespace clangd
115121
} // namespace clang

‎clang-tools-extra/clangd/FS.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_FS_H
1010
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_FS_H
1111

12+
#include "Path.h"
1213
#include "clang/Basic/LLVM.h"
1314
#include "llvm/ADT/Optional.h"
1415
#include "llvm/Support/VirtualFileSystem.h"
@@ -65,6 +66,13 @@ class PreambleFileStatusCache {
6566
llvm::StringMap<llvm::vfs::Status> StatCache;
6667
};
6768

69+
/// Returns a version of \p File that doesn't contain dots and dot dots.
70+
/// e.g /a/b/../c -> /a/c
71+
/// /a/b/./c -> /a/b/c
72+
/// FIXME: We should avoid encountering such paths in clangd internals by
73+
/// filtering everything we get over LSP, CDB, etc.
74+
Path removeDots(PathRef File);
75+
6876
} // namespace clangd
6977
} // namespace clang
7078

‎clang-tools-extra/clangd/GlobalCompilationDatabase.cpp

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "GlobalCompilationDatabase.h"
10+
#include "FS.h"
1011
#include "Logger.h"
1112
#include "Path.h"
1213
#include "clang/Frontend/CompilerInvocation.h"
@@ -15,6 +16,7 @@
1516
#include "llvm/ADT/None.h"
1617
#include "llvm/ADT/Optional.h"
1718
#include "llvm/ADT/STLExtras.h"
19+
#include "llvm/ADT/SmallString.h"
1820
#include "llvm/Support/FileSystem.h"
1921
#include "llvm/Support/Path.h"
2022
#include <string>
@@ -147,12 +149,15 @@ DirectoryBasedGlobalCompilationDatabase::lookupCDB(
147149
getCDBInDirLocked(*CompileCommandsDir);
148150
Result.PI.SourceRoot = *CompileCommandsDir;
149151
} else {
150-
actOnAllParentDirectories(
151-
Request.FileName, [this, &SentBroadcast, &Result](PathRef Path) {
152-
std::tie(Result.CDB, SentBroadcast) = getCDBInDirLocked(Path);
153-
Result.PI.SourceRoot = Path;
154-
return Result.CDB != nullptr;
155-
});
152+
// Traverse the canonical version to prevent false positives. i.e.:
153+
// src/build/../a.cc can detect a CDB in /src/build if not canonicalized.
154+
actOnAllParentDirectories(removeDots(Request.FileName),
155+
[this, &SentBroadcast, &Result](PathRef Path) {
156+
std::tie(Result.CDB, SentBroadcast) =
157+
getCDBInDirLocked(Path);
158+
Result.PI.SourceRoot = Path;
159+
return Result.CDB != nullptr;
160+
});
156161
}
157162

158163
if (!Result.CDB)
@@ -209,7 +214,8 @@ void DirectoryBasedGlobalCompilationDatabase::broadcastCDB(
209214
actOnAllParentDirectories(File, [&](PathRef Path) {
210215
if (DirectoryHasCDB.lookup(Path)) {
211216
if (Path == Result.PI.SourceRoot)
212-
GovernedFiles.push_back(File);
217+
// Make sure listeners always get a canonical path for the file.
218+
GovernedFiles.push_back(removeDots(File));
213219
// Stop as soon as we hit a CDB.
214220
return true;
215221
}
@@ -248,7 +254,7 @@ OverlayCDB::getCompileCommand(PathRef File) const {
248254
llvm::Optional<tooling::CompileCommand> Cmd;
249255
{
250256
std::lock_guard<std::mutex> Lock(Mutex);
251-
auto It = Commands.find(File);
257+
auto It = Commands.find(removeDots(File));
252258
if (It != Commands.end())
253259
Cmd = It->second;
254260
}
@@ -272,20 +278,24 @@ tooling::CompileCommand OverlayCDB::getFallbackCommand(PathRef File) const {
272278

273279
void OverlayCDB::setCompileCommand(
274280
PathRef File, llvm::Optional<tooling::CompileCommand> Cmd) {
281+
// We store a canonical version internally to prevent mismatches between set
282+
// and get compile commands. Also it assures clients listening to broadcasts
283+
// doesn't receive different names for the same file.
284+
std::string CanonPath = removeDots(File);
275285
{
276286
std::unique_lock<std::mutex> Lock(Mutex);
277287
if (Cmd)
278-
Commands[File] = std::move(*Cmd);
288+
Commands[CanonPath] = std::move(*Cmd);
279289
else
280-
Commands.erase(File);
290+
Commands.erase(CanonPath);
281291
}
282-
OnCommandChanged.broadcast({File});
292+
OnCommandChanged.broadcast({CanonPath});
283293
}
284294

285295
llvm::Optional<ProjectInfo> OverlayCDB::getProjectInfo(PathRef File) const {
286296
{
287297
std::lock_guard<std::mutex> Lock(Mutex);
288-
auto It = Commands.find(File);
298+
auto It = Commands.find(removeDots(File));
289299
if (It != Commands.end())
290300
return ProjectInfo{};
291301
}

0 commit comments

Comments
 (0)