Skip to content

Commit 84bf5e3

Browse files
committed
Fix various problems found by fuzzing.
1. IndexTokenSource::getNextToken cannot return nullptr; some code was still written assuming it can; make getNextToken more resilient against incorrect input and fix its call-sites. 2. Change various asserts that can happen due to user provided input to conditionals in the code.
1 parent a82942d commit 84bf5e3

File tree

4 files changed

+28
-20
lines changed

4 files changed

+28
-20
lines changed

clang/lib/Format/ContinuationIndenter.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1986,7 +1986,9 @@ ContinuationIndenter::createBreakableToken(const FormatToken &Current,
19861986
Current.Previous->isNot(TT_ImplicitStringLiteral))) {
19871987
if (!Style.ReflowComments ||
19881988
CommentPragmasRegex.match(Current.TokenText.substr(2)) ||
1989-
switchesFormatting(Current))
1989+
switchesFormatting(Current) ||
1990+
!(Current.TokenText.startswith("//") ||
1991+
Current.TokenText.startswith("#")))
19901992
return nullptr;
19911993
return std::make_unique<BreakableLineCommentSection>(
19921994
Current, StartColumn, /*InPPDirective=*/false, Encoding, Style);
@@ -2195,11 +2197,10 @@ ContinuationIndenter::breakProtrudingToken(const FormatToken &Current,
21952197
// When breaking before a tab character, it may be moved by a few columns,
21962198
// but will still be expanded to the next tab stop, so we don't save any
21972199
// columns.
2198-
if (NewRemainingTokenColumns == RemainingTokenColumns) {
2200+
if (NewRemainingTokenColumns >= RemainingTokenColumns) {
21992201
// FIXME: Do we need to adjust the penalty?
22002202
break;
22012203
}
2202-
assert(NewRemainingTokenColumns < RemainingTokenColumns);
22032204

22042205
LLVM_DEBUG(llvm::dbgs() << " Breaking at: " << TailOffset + Split.first
22052206
<< ", " << Split.second << "\n");

clang/lib/Format/TokenAnnotator.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -946,11 +946,15 @@ class AnnotatingParser {
946946
!Line.First->isOneOf(tok::kw_enum, tok::kw_case,
947947
tok::kw_default)) {
948948
FormatToken *Prev = Tok->getPreviousNonComment();
949+
if (!Prev)
950+
break;
949951
if (Prev->isOneOf(tok::r_paren, tok::kw_noexcept))
950952
Tok->setType(TT_CtorInitializerColon);
951953
else if (Prev->is(tok::kw_try)) {
952954
// Member initializer list within function try block.
953955
FormatToken *PrevPrev = Prev->getPreviousNonComment();
956+
if (!PrevPrev)
957+
break;
954958
if (PrevPrev && PrevPrev->isOneOf(tok::r_paren, tok::kw_noexcept))
955959
Tok->setType(TT_CtorInitializerColon);
956960
} else
@@ -1578,6 +1582,8 @@ class AnnotatingParser {
15781582
if (TemplateCloser->is(tok::l_paren)) {
15791583
// No Matching Paren yet so skip to matching paren
15801584
TemplateCloser = untilMatchingParen(TemplateCloser);
1585+
if (!TemplateCloser)
1586+
break;
15811587
}
15821588
if (TemplateCloser->is(tok::less))
15831589
NestingLevel++;
@@ -2639,8 +2645,8 @@ void TokenAnnotator::calculateFormattingInformation(AnnotatedLine &Line) {
26392645
if (Current->Role)
26402646
Current->Role->precomputeFormattingInfos(Current);
26412647
if (Current->MatchingParen &&
2642-
Current->MatchingParen->opensBlockOrBlockTypeList(Style)) {
2643-
assert(IndentLevel > 0);
2648+
Current->MatchingParen->opensBlockOrBlockTypeList(Style) &&
2649+
IndentLevel > 0) {
26442650
--IndentLevel;
26452651
}
26462652
Current->IndentLevel = IndentLevel;

clang/lib/Format/UnwrappedLineParser.cpp

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,8 @@ class IndexedTokenSource : public FormatTokenSource {
199199
: Tokens(Tokens), Position(-1) {}
200200

201201
FormatToken *getNextToken() override {
202+
if (Position >= 0 && Tokens[Position]->is(tok::eof))
203+
return Tokens[Position];
202204
++Position;
203205
return Tokens[Position];
204206
}
@@ -399,7 +401,7 @@ void UnwrappedLineParser::parseLevel(bool HasOpeningBrace) {
399401
FormatToken *Next;
400402
do {
401403
Next = Tokens->getNextToken();
402-
} while (Next && Next->is(tok::comment));
404+
} while (Next->is(tok::comment));
403405
FormatTok = Tokens->setPosition(StoredPosition);
404406
if (Next && Next->isNot(tok::colon)) {
405407
// default not followed by ':' is not a case label; treat it like
@@ -1097,7 +1099,6 @@ void UnwrappedLineParser::readTokenWithJavaScriptASI() {
10971099
}
10981100

10991101
void UnwrappedLineParser::parseStructuralElement(bool IsTopLevel) {
1100-
assert(!FormatTok->is(tok::l_brace));
11011102
if (Style.Language == FormatStyle::LK_TableGen &&
11021103
FormatTok->is(tok::pp_include)) {
11031104
nextToken();
@@ -1488,7 +1489,7 @@ void UnwrappedLineParser::parseStructuralElement(bool IsTopLevel) {
14881489
unsigned StoredPosition = Tokens->getPosition();
14891490
FormatToken *Next = Tokens->getNextToken();
14901491
FormatTok = Tokens->setPosition(StoredPosition);
1491-
if (Next && !mustBeJSIdent(Keywords, Next)) {
1492+
if (!mustBeJSIdent(Keywords, Next)) {
14921493
nextToken();
14931494
break;
14941495
}
@@ -2653,23 +2654,25 @@ bool UnwrappedLineParser::tryToParseSimpleAttribute() {
26532654
ScopedTokenPosition AutoPosition(Tokens);
26542655
FormatToken *Tok = Tokens->getNextToken();
26552656
// We already read the first [ check for the second.
2656-
if (Tok && !Tok->is(tok::l_square)) {
2657+
if (!Tok->is(tok::l_square)) {
26572658
return false;
26582659
}
26592660
// Double check that the attribute is just something
26602661
// fairly simple.
2661-
while (Tok) {
2662+
while (Tok->isNot(tok::eof)) {
26622663
if (Tok->is(tok::r_square)) {
26632664
break;
26642665
}
26652666
Tok = Tokens->getNextToken();
26662667
}
2668+
if (Tok->is(tok::eof))
2669+
return false;
26672670
Tok = Tokens->getNextToken();
2668-
if (Tok && !Tok->is(tok::r_square)) {
2671+
if (!Tok->is(tok::r_square)) {
26692672
return false;
26702673
}
26712674
Tok = Tokens->getNextToken();
2672-
if (Tok && Tok->is(tok::semi)) {
2675+
if (Tok->is(tok::semi)) {
26732676
return false;
26742677
}
26752678
return true;
@@ -2682,7 +2685,7 @@ void UnwrappedLineParser::parseJavaEnumBody() {
26822685
unsigned StoredPosition = Tokens->getPosition();
26832686
bool IsSimple = true;
26842687
FormatToken *Tok = Tokens->getNextToken();
2685-
while (Tok) {
2688+
while (!Tok->is(tok::eof)) {
26862689
if (Tok->is(tok::r_brace))
26872690
break;
26882691
if (Tok->isOneOf(tok::l_brace, tok::semi)) {

clang/lib/Format/WhitespaceManager.cpp

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -372,8 +372,6 @@ AlignTokenSequence(const FormatStyle &Style, unsigned Start, unsigned End,
372372
if (ContinuedStringLiteral)
373373
Changes[i].Spaces += Shift;
374374

375-
assert(Shift >= 0);
376-
377375
Changes[i].StartOfTokenColumn += Shift;
378376
if (i + 1 != Changes.size())
379377
Changes[i + 1].PreviousEndOfTokenColumn += Shift;
@@ -915,7 +913,7 @@ void WhitespaceManager::alignTrailingComments(unsigned Start, unsigned End,
915913
Changes[i].StartOfBlockComment->StartOfTokenColumn -
916914
Changes[i].StartOfTokenColumn;
917915
}
918-
assert(Shift >= 0);
916+
if (Shift < 0) continue;
919917
Changes[i].Spaces += Shift;
920918
if (i + 1 != Changes.size())
921919
Changes[i + 1].PreviousEndOfTokenColumn += Shift;
@@ -1270,10 +1268,10 @@ WhitespaceManager::linkCells(CellDescriptions &&CellDesc) {
12701268
void WhitespaceManager::generateChanges() {
12711269
for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
12721270
const Change &C = Changes[i];
1273-
if (i > 0) {
1274-
assert(Changes[i - 1].OriginalWhitespaceRange.getBegin() !=
1275-
C.OriginalWhitespaceRange.getBegin() &&
1276-
"Generating two replacements for the same location");
1271+
if (i > 0 && Changes[i - 1].OriginalWhitespaceRange.getBegin() ==
1272+
C.OriginalWhitespaceRange.getBegin()) {
1273+
// Do not generate two replacements for the same location.
1274+
continue;
12771275
}
12781276
if (C.CreateReplacement) {
12791277
std::string ReplacementText = C.PreviousLinePostfix;

0 commit comments

Comments
 (0)