[clang-tools-extra] r316659 - [clangd] Report an error on findDefinitions/signatureHelp on an unopened file instead of crashing.
Benjamin Kramer via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 26 05:28:13 PDT 2017
Author: d0k
Date: Thu Oct 26 05:28:13 2017
New Revision: 316659
URL: http://llvm.org/viewvc/llvm-project?rev=316659&view=rev
Log:
[clangd] Report an error on findDefinitions/signatureHelp on an unopened file instead of crashing.
Found by clangd-fuzzer.
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/test/clangd/definitions.test
clang-tools-extra/trunk/test/clangd/signature-help.test
clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=316659&r1=316658&r2=316659&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Thu Oct 26 05:28:13 2017
@@ -160,24 +160,24 @@ void ClangdLSPServer::onCompletion(Ctx C
void ClangdLSPServer::onSignatureHelp(Ctx C,
TextDocumentPositionParams &Params) {
- C.reply(SignatureHelp::unparse(
- Server
- .signatureHelp(
- Params.textDocument.uri.file,
- Position{Params.position.line, Params.position.character})
- .Value));
+ auto SignatureHelp = Server.signatureHelp(
+ Params.textDocument.uri.file,
+ Position{Params.position.line, Params.position.character});
+ if (!SignatureHelp)
+ return C.replyError(-32602, llvm::toString(SignatureHelp.takeError()));
+ C.reply(SignatureHelp::unparse(SignatureHelp->Value));
}
void ClangdLSPServer::onGoToDefinition(Ctx C,
TextDocumentPositionParams &Params) {
- auto Items = Server
- .findDefinitions(Params.textDocument.uri.file,
- Position{Params.position.line,
- Params.position.character})
- .Value;
+ auto Items = Server.findDefinitions(
+ Params.textDocument.uri.file,
+ Position{Params.position.line, Params.position.character});
+ if (!Items)
+ return C.replyError(-32602, llvm::toString(Items.takeError()));
std::string Locations;
- for (const auto &Item : Items) {
+ for (const auto &Item : Items->Value) {
Locations += Location::unparse(Item);
Locations += ",";
}
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=316659&r1=316658&r2=316659&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Thu Oct 26 05:28:13 2017
@@ -13,6 +13,7 @@
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Tooling/CompilationDatabase.h"
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/Errc.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/raw_ostream.h"
@@ -266,15 +267,17 @@ void ClangdServer::codeComplete(
WorkScheduler.addToFront(std::move(Task), std::move(Callback));
}
-Tagged<SignatureHelp>
+llvm::Expected<Tagged<SignatureHelp>>
ClangdServer::signatureHelp(PathRef File, Position Pos,
llvm::Optional<StringRef> OverridenContents,
IntrusiveRefCntPtr<vfs::FileSystem> *UsedFS) {
std::string DraftStorage;
if (!OverridenContents) {
auto FileContents = DraftMgr.getDraft(File);
- assert(FileContents.Draft &&
- "signatureHelp is called for non-added document");
+ if (!FileContents.Draft)
+ return llvm::make_error<llvm::StringError>(
+ "signatureHelp is called for non-added document",
+ llvm::errc::invalid_argument);
DraftStorage = std::move(*FileContents.Draft);
OverridenContents = DraftStorage;
@@ -285,7 +288,10 @@ ClangdServer::signatureHelp(PathRef File
*UsedFS = TaggedFS.Value;
std::shared_ptr<CppFile> Resources = Units.getFile(File);
- assert(Resources && "Calling signatureHelp on non-added file");
+ if (!Resources)
+ return llvm::make_error<llvm::StringError>(
+ "signatureHelp is called for non-added document",
+ llvm::errc::invalid_argument);
auto Preamble = Resources->getPossiblyStalePreamble();
auto Result = clangd::signatureHelp(File, Resources->getCompileCommand(),
@@ -347,16 +353,15 @@ std::string ClangdServer::dumpAST(PathRe
return Result;
}
-Tagged<std::vector<Location>> ClangdServer::findDefinitions(PathRef File,
- Position Pos) {
- auto FileContents = DraftMgr.getDraft(File);
- assert(FileContents.Draft &&
- "findDefinitions is called for non-added document");
-
+llvm::Expected<Tagged<std::vector<Location>>>
+ClangdServer::findDefinitions(PathRef File, Position Pos) {
auto TaggedFS = FSProvider.getTaggedFileSystem(File);
std::shared_ptr<CppFile> Resources = Units.getFile(File);
- assert(Resources && "Calling findDefinitions on non-added file");
+ if (!Resources)
+ return llvm::make_error<llvm::StringError>(
+ "findDefinitions called on non-added file",
+ llvm::errc::invalid_argument);
std::vector<Location> Result;
Resources->getAST().get()->runUnderLock([Pos, &Result, this](ParsedAST *AST) {
Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=316659&r1=316658&r2=316659&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Thu Oct 26 05:28:13 2017
@@ -271,13 +271,14 @@ public:
/// will be used. If \p UsedFS is non-null, it will be overwritten by
/// vfs::FileSystem used for signature help. This method should only be called
/// for currently tracked files.
- Tagged<SignatureHelp>
+ llvm::Expected<Tagged<SignatureHelp>>
signatureHelp(PathRef File, Position Pos,
llvm::Optional<StringRef> OverridenContents = llvm::None,
IntrusiveRefCntPtr<vfs::FileSystem> *UsedFS = nullptr);
/// Get definition of symbol at a specified \p Line and \p Column in \p File.
- Tagged<std::vector<Location>> findDefinitions(PathRef File, Position Pos);
+ llvm::Expected<Tagged<std::vector<Location>>> findDefinitions(PathRef File,
+ Position Pos);
/// Helper function that returns a path to the corresponding source file when
/// given a header file and vice versa.
Modified: clang-tools-extra/trunk/test/clangd/definitions.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/definitions.test?rev=316659&r1=316658&r2=316659&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/definitions.test (original)
+++ clang-tools-extra/trunk/test/clangd/definitions.test Thu Oct 26 05:28:13 2017
@@ -163,10 +163,15 @@ Content-Length: 148
# Go to macro, being undefined
# CHECK: {"jsonrpc":"2.0","id":1,"result":[{"uri": "file:///main.cpp", "range": {"start": {"line": 2, "character": 8}, "end": {"line": 2, "character": 13}}}]}
-Content-Length: 44
+Content-Length: 156
-{"jsonrpc":"2.0","id":3,"method":"shutdown"}
-# CHECK: {"jsonrpc":"2.0","id":3,"result":null}
+{"jsonrpc":"2.0","id":2,"method":"textDocument/definition","params":{"textDocument":{"uri":"file:///doesnotexist.cpp"},"position":{"line":4,"character":7}}}
+# CHECK: {"jsonrpc":"2.0","id":2,"error":{"code":-32602,"message":"findDefinitions called on non-added file"}}
+
+Content-Length: 48
+
+{"jsonrpc":"2.0","id":10000,"method":"shutdown"}
+# CHECK: {"jsonrpc":"2.0","id":10000,"result":null}
Content-Length: 33
{"jsonrpc":"2.0":"method":"exit"}
Modified: clang-tools-extra/trunk/test/clangd/signature-help.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/signature-help.test?rev=316659&r1=316658&r2=316659&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/signature-help.test (original)
+++ clang-tools-extra/trunk/test/clangd/signature-help.test Thu Oct 26 05:28:13 2017
@@ -36,6 +36,11 @@ Content-Length: 151
# CHECK-DAG: {"label":"bar(float x = 0, int y = 42) -> void","parameters":[{"label":"float x = 0"},{"label":"int y = 42"}]}
# CHECK: ]}
+Content-Length: 159
+
+{"jsonrpc":"2.0","id":3,"method":"textDocument/signatureHelp","params":{"textDocument":{"uri":"file:///doesnotexist.cpp"},"position":{"line":8,"character":9}}}
+# CHECK: {"jsonrpc":"2.0","id":3,"error":{"code":-32602,"message":"signatureHelp is called for non-added document"}}
+
# Shutdown.
Content-Length: 49
Modified: clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp?rev=316659&r1=316658&r2=316659&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/ClangdTests.cpp Thu Oct 26 05:28:13 2017
@@ -1056,7 +1056,7 @@ int d;
AddDocument(FileIndex);
Position Pos{LineDist(RandGen), ColumnDist(RandGen)};
- Server.findDefinitions(FilePaths[FileIndex], Pos);
+ ASSERT_TRUE(!!Server.findDefinitions(FilePaths[FileIndex], Pos));
};
std::vector<std::function<void()>> AsyncRequests = {
More information about the cfe-commits
mailing list