hokein created this revision.
hokein added a reviewer: kadircet.
Herald added subscribers: arphaman, jkorous, MaskRay, ioeric, ilya-biryukov.
Fix a use-after-free bug. The inner loop may add element to `Dependencies`
vector, which may cause memory reallocation of the vector.
Also fix an assertion, we have to take errors when llvm::Expected holds an
error.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D56655
Files:
clangd/index/Background.cpp
Index: clangd/index/Background.cpp
===================================================================
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -484,7 +484,7 @@
// Goes over each dependency.
for (size_t CurrentDependency = 0; CurrentDependency < Dependencies.size();
CurrentDependency++) {
- llvm::StringRef CurDependencyPath = Dependencies[CurrentDependency].Path;
+ std::string CurDependencyPath = Dependencies[CurrentDependency].Path;
// If we have already seen this shard before(either loaded or failed) don't
// re-try again. Since the information in the shard won't change from one
TU
// to another.
@@ -504,11 +504,18 @@
// These are the edges in the include graph for current dependency.
for (const auto &I : *Shard->Sources) {
auto U = URI::parse(I.getKey());
- if (!U)
+ if (!U) {
+ elog("Failed to parse URI {0}: {1}", I.getKey(), U.takeError());
continue;
+ }
+
auto AbsolutePath = URI::resolve(*U, CurDependencyPath);
- if (!AbsolutePath)
+ if (!AbsolutePath) {
+ elog("Failed to resolve URI {0}: {1}", I.getKey(),
+ AbsolutePath.takeError());
continue;
+ }
+
// Add file as dependency if haven't seen before.
if (InQueue.try_emplace(*AbsolutePath).second)
Dependencies.emplace_back(*AbsolutePath, true);
@@ -527,6 +534,7 @@
// Check if the source needs re-indexing.
// Get the digest, skip it if file doesn't exist.
auto Buf = FS->getBufferForFile(CurDependencyPath);
+
if (!Buf) {
elog("Couldn't get buffer for file: {0}: {1}", CurDependencyPath,
Buf.getError().message());
Index: clangd/index/Background.cpp
===================================================================
--- clangd/index/Background.cpp
+++ clangd/index/Background.cpp
@@ -484,7 +484,7 @@
// Goes over each dependency.
for (size_t CurrentDependency = 0; CurrentDependency < Dependencies.size();
CurrentDependency++) {
- llvm::StringRef CurDependencyPath = Dependencies[CurrentDependency].Path;
+ std::string CurDependencyPath = Dependencies[CurrentDependency].Path;
// If we have already seen this shard before(either loaded or failed) don't
// re-try again. Since the information in the shard won't change from one TU
// to another.
@@ -504,11 +504,18 @@
// These are the edges in the include graph for current dependency.
for (const auto &I : *Shard->Sources) {
auto U = URI::parse(I.getKey());
- if (!U)
+ if (!U) {
+ elog("Failed to parse URI {0}: {1}", I.getKey(), U.takeError());
continue;
+ }
+
auto AbsolutePath = URI::resolve(*U, CurDependencyPath);
- if (!AbsolutePath)
+ if (!AbsolutePath) {
+ elog("Failed to resolve URI {0}: {1}", I.getKey(),
+ AbsolutePath.takeError());
continue;
+ }
+
// Add file as dependency if haven't seen before.
if (InQueue.try_emplace(*AbsolutePath).second)
Dependencies.emplace_back(*AbsolutePath, true);
@@ -527,6 +534,7 @@
// Check if the source needs re-indexing.
// Get the digest, skip it if file doesn't exist.
auto Buf = FS->getBufferForFile(CurDependencyPath);
+
if (!Buf) {
elog("Couldn't get buffer for file: {0}: {1}", CurDependencyPath,
Buf.getError().message());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
