Skip to content

Commit a2f7352

Browse files
committed
[clangd] Dont run raw-lexer for OOB source locations
We can get stale source locations from preamble, make sure we don't access those locations without checking first. Fixes clangd/clangd#1636. Differential Revision: https://reviews.llvm.org/D151321
1 parent d5c56c5 commit a2f7352

File tree

2 files changed

+44
-4
lines changed

2 files changed

+44
-4
lines changed

clang-tools-extra/clangd/Diagnostics.cpp

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,8 @@ bool locationInRange(SourceLocation L, CharSourceRange R,
9696

9797
// Clang diags have a location (shown as ^) and 0 or more ranges (~~~~).
9898
// LSP needs a single range.
99-
Range diagnosticRange(const clang::Diagnostic &D, const LangOptions &L) {
99+
std::optional<Range> diagnosticRange(const clang::Diagnostic &D,
100+
const LangOptions &L) {
100101
auto &M = D.getSourceManager();
101102
auto PatchedRange = [&M](CharSourceRange &R) {
102103
R.setBegin(translatePreamblePatchLocation(R.getBegin(), M));
@@ -115,13 +116,18 @@ Range diagnosticRange(const clang::Diagnostic &D, const LangOptions &L) {
115116
if (locationInRange(Loc, R, M))
116117
return halfOpenToRange(M, PatchedRange(R));
117118
}
119+
// Source locations from stale preambles might become OOB.
120+
// FIXME: These diagnostics might point to wrong locations even when they're
121+
// not OOB.
122+
auto [FID, Offset] = M.getDecomposedLoc(Loc);
123+
if (Offset > M.getBufferData(FID).size())
124+
return std::nullopt;
118125
// If the token at the location is not a comment, we use the token.
119126
// If we can't get the token at the location, fall back to using the location
120127
auto R = CharSourceRange::getCharRange(Loc);
121128
Token Tok;
122-
if (!Lexer::getRawToken(Loc, Tok, M, L, true) && Tok.isNot(tok::comment)) {
129+
if (!Lexer::getRawToken(Loc, Tok, M, L, true) && Tok.isNot(tok::comment))
123130
R = CharSourceRange::getTokenRange(Tok.getLocation(), Tok.getEndLoc());
124-
}
125131
return halfOpenToRange(M, PatchedRange(R));
126132
}
127133

@@ -697,7 +703,10 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
697703
SourceLocation PatchLoc =
698704
translatePreamblePatchLocation(Info.getLocation(), SM);
699705
D.InsideMainFile = isInsideMainFile(PatchLoc, SM);
700-
D.Range = diagnosticRange(Info, *LangOpts);
706+
if (auto DRange = diagnosticRange(Info, *LangOpts))
707+
D.Range = *DRange;
708+
else
709+
D.Severity = DiagnosticsEngine::Ignored;
701710
auto FID = SM.getFileID(Info.getLocation());
702711
if (const auto FE = SM.getFileEntryRefForID(FID)) {
703712
D.File = FE->getName().str();

clang-tools-extra/clangd/unittests/PreambleTests.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -829,6 +829,37 @@ x>)");
829829
auto AST = createPatchedAST(Code.code(), NewCode.code());
830830
EXPECT_THAT(*AST->getDiagnostics(), IsEmpty());
831831
}
832+
{
833+
Annotations Code(R"(
834+
#ifndef FOO
835+
#define FOO
836+
void foo();
837+
#endif)");
838+
// This code will emit a diagnostic for unterminated #ifndef (as stale
839+
// preamble has the conditional but main file doesn't terminate it).
840+
// We shouldn't emit any diagnotiscs (and shouldn't crash).
841+
Annotations NewCode("");
842+
auto AST = createPatchedAST(Code.code(), NewCode.code());
843+
EXPECT_THAT(*AST->getDiagnostics(), IsEmpty());
844+
}
845+
{
846+
Annotations Code(R"(
847+
#ifndef FOO
848+
#define FOO
849+
void foo();
850+
#endif)");
851+
// This code will emit a diagnostic for unterminated #ifndef (as stale
852+
// preamble has the conditional but main file doesn't terminate it).
853+
// We shouldn't emit any diagnotiscs (and shouldn't crash).
854+
// FIXME: Patch/ignore diagnostics in such cases.
855+
Annotations NewCode(R"(
856+
i[[nt]] xyz;
857+
)");
858+
auto AST = createPatchedAST(Code.code(), NewCode.code());
859+
EXPECT_THAT(
860+
*AST->getDiagnostics(),
861+
ElementsAre(Diag(NewCode.range(), "pp_unterminated_conditional")));
862+
}
832863
}
833864

834865
MATCHER_P2(Mark, Range, Text, "") {

0 commit comments

Comments
 (0)