Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clangd] Fix crash with null check for Token at Loc #94528

Merged
merged 6 commits into from
Jun 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clang-tools-extra/clangd/FindSymbols.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ class DocumentOutline {
if (!MacroName.isValid() || !MacroName.isFileID())
continue;
// All conditions satisfied, add the macro.
if (auto *Tok = AST.getTokens().spelledTokenAt(MacroName))
if (auto *Tok = AST.getTokens().spelledTokenContaining(MacroName))
CurParent = &CurParent->inMacro(
*Tok, SM, AST.getTokens().expansionStartingAt(Tok));
}
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/clangd/IncludeCleaner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ collectMacroReferences(ParsedAST &AST) {
for (const auto &[_, Refs] : AST.getMacros().MacroRefs) {
for (const auto &Ref : Refs) {
auto Loc = SM.getComposedLoc(SM.getMainFileID(), Ref.StartOffset);
const auto *Tok = AST.getTokens().spelledTokenAt(Loc);
const auto *Tok = AST.getTokens().spelledTokenContaining(Loc);
if (!Tok)
continue;
auto Macro = locateMacroAt(*Tok, PP);
Expand Down
11 changes: 5 additions & 6 deletions clang-tools-extra/clangd/SemanticHighlighting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -447,11 +447,10 @@ class HighlightingsBuilder {
if (!RLoc.isValid())
return;

const auto *RTok = TB.spelledTokenAt(RLoc);
// Handle `>>`. RLoc is always pointing at the right location, just change
// the end to be offset by 1.
// We'll either point at the beginning of `>>`, hence get a proper spelled
// or point in the middle of `>>` hence get no spelled tok.
const auto *RTok = TB.spelledTokenContaining(RLoc);
usx95 marked this conversation as resolved.
Show resolved Hide resolved
// Handle `>>`. RLoc is either part of `>>` or a spelled token on its own
// `>`. If it's the former, slice to have length of 1, if latter use the
// token as-is.
if (!RTok || RTok->kind() == tok::greatergreater) {
Position Begin = sourceLocToPosition(SourceMgr, RLoc);
Position End = sourceLocToPosition(SourceMgr, RLoc.getLocWithOffset(1));
Expand Down Expand Up @@ -577,7 +576,7 @@ class HighlightingsBuilder {
return std::nullopt;
// We might have offsets in the main file that don't correspond to any
// spelled tokens.
const auto *Tok = TB.spelledTokenAt(Loc);
const auto *Tok = TB.spelledTokenContaining(Loc);
if (!Tok)
return std::nullopt;
return halfOpenToRange(SourceMgr,
Expand Down
9 changes: 5 additions & 4 deletions clang-tools-extra/clangd/XRefs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,7 @@ std::vector<DocumentLink> getDocumentLinks(ParsedAST &AST) {
if (Inc.Resolved.empty())
continue;
auto HashLoc = SM.getComposedLoc(SM.getMainFileID(), Inc.HashOffset);
const auto *HashTok = AST.getTokens().spelledTokenAt(HashLoc);
const auto *HashTok = AST.getTokens().spelledTokenContaining(HashLoc);
assert(HashTok && "got inclusion at wrong offset");
const auto *IncludeTok = std::next(HashTok);
const auto *FileTok = std::next(IncludeTok);
Expand Down Expand Up @@ -938,7 +938,7 @@ class ReferenceFinder : public index::IndexDataConsumer {
CollectorOpts.CollectMainFileSymbols = true;
for (SourceLocation L : Locs) {
L = SM.getFileLoc(L);
if (const auto *Tok = TB.spelledTokenAt(L))
if (const auto *Tok = TB.spelledTokenContaining(L))
References.push_back(
{*Tok, Roles,
SymbolCollector::getRefContainer(ASTNode.Parent, CollectorOpts)});
Expand Down Expand Up @@ -1216,7 +1216,7 @@ DocumentHighlight toHighlight(const ReferenceFinder::Reference &Ref,
std::optional<DocumentHighlight> toHighlight(SourceLocation Loc,
const syntax::TokenBuffer &TB) {
Loc = TB.sourceManager().getFileLoc(Loc);
if (const auto *Tok = TB.spelledTokenAt(Loc)) {
if (const auto *Tok = TB.spelledTokenContaining(Loc)) {
DocumentHighlight Result;
Result.range = halfOpenToRange(
TB.sourceManager(),
Expand Down Expand Up @@ -1353,7 +1353,8 @@ maybeFindIncludeReferences(ParsedAST &AST, Position Pos,
Loc = SM.getIncludeLoc(SM.getFileID(Loc));

ReferencesResult::Reference Result;
const auto *Token = AST.getTokens().spelledTokenAt(Loc);
const auto *Token = AST.getTokens().spelledTokenContaining(Loc);
assert(Token && "references expected token here");
Result.Loc.range = Range{sourceLocToPosition(SM, Token->location()),
sourceLocToPosition(SM, Token->endLocation())};
Result.Loc.uri = URIMainFile;
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/clangd/refactor/Rename.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -748,7 +748,7 @@ std::vector<SymbolRange> collectRenameIdentifierRanges(
clangd::Range tokenRangeForLoc(ParsedAST &AST, SourceLocation TokLoc,
const SourceManager &SM,
const LangOptions &LangOpts) {
const auto *Token = AST.getTokens().spelledTokenAt(TokLoc);
const auto *Token = AST.getTokens().spelledTokenContaining(TokLoc);
assert(Token && "rename expects spelled tokens");
clangd::Range Result;
Result.start = sourceLocToPosition(SM, Token->location());
Expand Down
11 changes: 6 additions & 5 deletions clang-tools-extra/clangd/unittests/PreambleTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ TEST(PreamblePatchTest, LocateMacroAtWorks) {
ASSERT_TRUE(AST);

const auto &SM = AST->getSourceManager();
auto *MacroTok = AST->getTokens().spelledTokenAt(
auto *MacroTok = AST->getTokens().spelledTokenContaining(
SM.getComposedLoc(SM.getMainFileID(), Modified.point("use")));
ASSERT_TRUE(MacroTok);

Expand All @@ -441,7 +441,7 @@ TEST(PreamblePatchTest, LocateMacroAtDeletion) {
ASSERT_TRUE(AST);

const auto &SM = AST->getSourceManager();
auto *MacroTok = AST->getTokens().spelledTokenAt(
auto *MacroTok = AST->getTokens().spelledTokenContaining(
SM.getComposedLoc(SM.getMainFileID(), Modified.point()));
ASSERT_TRUE(MacroTok);

Expand Down Expand Up @@ -512,9 +512,10 @@ TEST(PreamblePatchTest, RefsToMacros) {
ExpectedLocations.push_back(referenceRangeIs(R));

for (const auto &P : Modified.points()) {
auto *MacroTok = AST->getTokens().spelledTokenAt(SM.getComposedLoc(
SM.getMainFileID(),
llvm::cantFail(positionToOffset(Modified.code(), P))));
auto *MacroTok =
AST->getTokens().spelledTokenContaining(SM.getComposedLoc(
SM.getMainFileID(),
llvm::cantFail(positionToOffset(Modified.code(), P))));
ASSERT_TRUE(MacroTok);
EXPECT_THAT(findReferences(*AST, P, 0).References,
testing::ElementsAreArray(ExpectedLocations));
Expand Down
16 changes: 15 additions & 1 deletion clang-tools-extra/clangd/unittests/XRefsTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2173,6 +2173,11 @@ TEST(FindReferences, WithinAST) {
using $def[[MyTypeD^ef]] = int;
enum MyEnum : $(MyEnum)[[MyTy^peDef]] { };
)cpp",
// UDL
R"cpp(
bool $decl[[operator]]"" _u^dl(unsigned long long value);
bool x = $(x)[[1_udl]];
)cpp",
};
for (const char *Test : Tests)
checkFindRefs(Test);
Expand Down Expand Up @@ -2358,7 +2363,13 @@ TEST(FindReferences, UsedSymbolsFromInclude) {

R"cpp([[#in^clude <vector>]]
std::[[vector]]<int> vec;
)cpp"};
)cpp",

R"cpp(
[[#include ^"udl_header.h"]]
auto x = [[1_b]];
)cpp",
};
for (const char *Test : Tests) {
Annotations T(Test);
auto TU = TestTU::withCode(T.code());
Expand All @@ -2375,6 +2386,9 @@ TEST(FindReferences, UsedSymbolsFromInclude) {
class vector{};
}
)cpp");
TU.AdditionalFiles["udl_header.h"] = guard(R"cpp(
bool operator"" _b(unsigned long long value);
)cpp");
TU.ExtraArgs.push_back("-isystem" + testPath("system"));

auto AST = TU.build();
Expand Down
4 changes: 2 additions & 2 deletions clang/include/clang/Tooling/Syntax/Tokens.h
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,9 @@ class TokenBuffer {
/// "DECL", "(", "a", ")", ";"}
llvm::ArrayRef<syntax::Token> spelledTokens(FileID FID) const;

/// Returns the spelled Token starting at Loc, if there are no such tokens
/// Returns the spelled Token containing the Loc, if there are no such tokens
/// returns nullptr.
const syntax::Token *spelledTokenAt(SourceLocation Loc) const;
const syntax::Token *spelledTokenContaining(SourceLocation Loc) const;

/// Get all tokens that expand a macro in \p FID. For the following input
/// #define FOO B
Expand Down
7 changes: 4 additions & 3 deletions clang/lib/Tooling/Syntax/Tokens.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -383,12 +383,13 @@ llvm::ArrayRef<syntax::Token> TokenBuffer::spelledTokens(FileID FID) const {
return It->second.SpelledTokens;
}

const syntax::Token *TokenBuffer::spelledTokenAt(SourceLocation Loc) const {
const syntax::Token *
TokenBuffer::spelledTokenContaining(SourceLocation Loc) const {
assert(Loc.isFileID());
const auto *Tok = llvm::partition_point(
spelledTokens(SourceMgr->getFileID(Loc)),
[&](const syntax::Token &Tok) { return Tok.location() < Loc; });
if (!Tok || Tok->location() != Loc)
[&](const syntax::Token &Tok) { return Tok.endLocation() <= Loc; });
if (!Tok || Loc < Tok->location())
return nullptr;
return Tok;
}
Expand Down
17 changes: 15 additions & 2 deletions clang/unittests/Tooling/Syntax/TokensTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -374,11 +374,24 @@ TEST_F(TokenCollectorTest, Locations) {

auto StartLoc = SourceMgr->getLocForStartOfFile(SourceMgr->getMainFileID());
for (auto &R : Code.ranges()) {
EXPECT_THAT(Buffer.spelledTokenAt(StartLoc.getLocWithOffset(R.Begin)),
Pointee(RangeIs(R)));
EXPECT_THAT(
Buffer.spelledTokenContaining(StartLoc.getLocWithOffset(R.Begin)),
Pointee(RangeIs(R)));
}
}

TEST_F(TokenCollectorTest, LocationInMiddleOfSpelledToken) {
llvm::Annotations Code(R"cpp(
int foo = [[baa^aar]];
)cpp");
recordTokens(Code.code());
// Check spelled tokens.
auto StartLoc = SourceMgr->getLocForStartOfFile(SourceMgr->getMainFileID());
EXPECT_THAT(
Buffer.spelledTokenContaining(StartLoc.getLocWithOffset(Code.point())),
Pointee(RangeIs(Code.range())));
}

TEST_F(TokenCollectorTest, MacroDirectives) {
// Macro directives are not stored anywhere at the moment.
std::string Code = R"cpp(
Expand Down
Loading