https://github.com/and2049 created https://github.com/llvm/llvm-project/pull/221501
Depends on #221352. The first commit is an unchanged, rebased copy of that PR; only the second commit is new here. Ready for review, but please do not merge until #221352 has landed and this branch has been rebased to remove the prerequisite commit. Rewinding discarded macro arguments processes their preprocessor directives twice, duplicating lines and causing crashes. Restore the preprocessor bookkeeping on rewind, but retain the first set of directive lines and discard duplicates during replay. Exclude expansion EOF tokens from directive ranges so directives before empty expansions are still formatted. Fixes #131157 Assisted-by: Claude Code >From 14f33fdf81817d19314ac7ecc2e7535ae3e73ccc Mon Sep 17 00:00:00 2001 From: andre sun <[email protected]> Date: Fri, 4 Sep 2026 16:20:53 -0400 Subject: [PATCH 1/2] [clang-format] Fix crash on adjacent configured macro calls parseMacroCall() used nextToken() to consume the end of a configured macro call, which also processed the following token. For adjacent macro calls, this could expand the second call first and leave the macro reconstruction state out of order, causing a crash when the second expansion starts with a comment. Consume the last token of the call without processing the following token, preserving source-order expansion. As a result, a comment or preprocessor directive following the call is no longer part of the reconstructed macro call, which changes an existing expectation in FormatTestMacroExpansion. Previously, a trailing comment after a call could also swallow the rest of the statement, turning `ID(1) // c\n + 2;` into `ID(1) // c + 2;`. Fixes #190721 Assisted-by: DeepSeek Flash v4 --- clang/lib/Format/UnwrappedLineParser.cpp | 16 +++++++-- .../Format/FormatTestMacroExpansion.cpp | 34 +++++++++++++++++-- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp index f2ea03b86aae4..24ba062e8c1ac 100644 --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -5199,9 +5199,19 @@ std::optional<llvm::SmallVector<llvm::SmallVector<FormatToken *, 8>, 1>> UnwrappedLineParser::parseMacroCall() { std::optional<llvm::SmallVector<llvm::SmallVector<FormatToken *, 8>, 1>> Args; assert(Line->Tokens.empty()); - nextToken(); - if (FormatTok->isNot(tok::l_paren)) + // Not nextToken(), which would already expand a directly following macro + // call before the expansion of this one is inserted. + auto ConsumeLastTokenOfCall = [this] { + flushComments(isOnNewLine(*FormatTok)); + pushToken(FormatTok); + FormatTok = Tokens->getNextToken(); + }; + if (Tokens->peekNextToken(/*SkipComment=*/true)->isNot(tok::l_paren)) { + ConsumeLastTokenOfCall(); return Args; + } + nextToken(); + assert(FormatTok->is(tok::l_paren)); unsigned Position = Tokens->getPosition(); FormatToken *Tok = FormatTok; nextToken(); @@ -5223,7 +5233,7 @@ UnwrappedLineParser::parseMacroCall() { } Args->push_back({}); pushTokens(std::next(ArgStart), Line->Tokens.end(), Args->back()); - nextToken(); + ConsumeLastTokenOfCall(); return Args; } case tok::comma: { diff --git a/clang/unittests/Format/FormatTestMacroExpansion.cpp b/clang/unittests/Format/FormatTestMacroExpansion.cpp index 8ff3282d31aee..26d7aa05e1561 100644 --- a/clang/unittests/Format/FormatTestMacroExpansion.cpp +++ b/clang/unittests/Format/FormatTestMacroExpansion.cpp @@ -77,8 +77,7 @@ int f; ID( namespace foo { int a; - } -) // namespace k + }) // namespace k )", format(R"( int a; @@ -315,6 +314,37 @@ TEST_F(FormatTestMacroExpansion, ObjectLikeMacroCalledWithArgsDoesNotHang) { verifyNoCrash("CASE(1, \"1\");", Style); } +TEST_F(FormatTestMacroExpansion, ExpandsAdjacentMacroCallsInOrder) { + FormatStyle Style = getLLVMStyle(); + Style.Macros.push_back("ID(x)=x"); + + verifyFormat("ID(a;)\n" + "ID(\n" + " // c\n" + " b;)", + Style); +} + +TEST_F(FormatTestMacroExpansion, TokensAfterMacroCallAreNotPartOfCall) { + FormatStyle Style = getLLVMStyle(); + Style.Macros.push_back("ID(x)=x"); + + verifyFormat("ID(a;) // c\n" + "ID(b;)", + "ID(\n" + " a;) // c\n" + "ID(b;)", + Style); + verifyFormat("int x = ID(1) // c\n" + " + 2;", + Style); + verifyFormat("ID(a;)\n" + "#if X\n" + "int b;\n" + "#endif", + Style); +} + } // namespace } // namespace test } // namespace format >From 68dd9983a7e58c983c7e8f6f1e88f92bf6d4a539 Mon Sep 17 00:00:00 2001 From: andre sun <[email protected]> Date: Sat, 5 Sep 2026 16:48:11 -0400 Subject: [PATCH 2/2] [clang-format] Fix crashes on directives in discarded macro call args Rewinding discarded macro arguments processes their preprocessor directives twice, duplicating lines and causing crashes. Restore the preprocessor bookkeeping on rewind, but retain the first set of directive lines and discard duplicates during replay. Exclude expansion EOF tokens from directive ranges so directives before empty expansions are still formatted. Fixes #131157 Assisted-by: Claude Code --- clang/lib/Format/AffectedRangeManager.cpp | 3 +- clang/lib/Format/UnwrappedLineParser.cpp | 54 +++++++++- clang/lib/Format/UnwrappedLineParser.h | 26 ++++- .../Format/FormatTestMacroExpansion.cpp | 98 +++++++++++++++++++ 4 files changed, 174 insertions(+), 7 deletions(-) diff --git a/clang/lib/Format/AffectedRangeManager.cpp b/clang/lib/Format/AffectedRangeManager.cpp index 67108f3540191..7e9acc9fbbb04 100644 --- a/clang/lib/Format/AffectedRangeManager.cpp +++ b/clang/lib/Format/AffectedRangeManager.cpp @@ -35,7 +35,8 @@ bool AffectedRangeManager::computeAffectedLines( if (Line->InPPDirective) { FormatToken *Last = Line->Last; const auto *PPEnd = I + 1; - while (PPEnd != E && !(*PPEnd)->First->HasUnescapedNewline) { + while (PPEnd != E && !(*PPEnd)->First->HasUnescapedNewline && + (*PPEnd)->First->isNot(tok::eof)) { Last = (*PPEnd)->Last; ++PPEnd; } diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp index 24ba062e8c1ac..d726624ba7e33 100644 --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -95,13 +95,17 @@ std::ostream &operator<<(std::ostream &Stream, const UnwrappedLine &Line) { class ScopedLineState { public: + // With \c DiscardLines, the lines added while in scope are discarded. ScopedLineState(UnwrappedLineParser &Parser, - bool SwitchToPreprocessorLines = false) - : Parser(Parser), OriginalLines(Parser.CurrentLines) { + bool SwitchToPreprocessorLines = false, + bool DiscardLines = false) + : Parser(Parser), OriginalLines(Parser.CurrentLines), + DiscardLines(DiscardLines) { if (SwitchToPreprocessorLines) Parser.CurrentLines = &Parser.PreprocessorDirectives; else if (!Parser.Line->Tokens.empty()) Parser.CurrentLines = &Parser.Line->Tokens.back().Children; + OriginalNumLines = Parser.CurrentLines->size(); PreBlockLine = std::move(Parser.Line); Parser.Line = std::make_unique<UnwrappedLine>(); Parser.Line->Level = PreBlockLine->Level; @@ -115,6 +119,8 @@ class ScopedLineState { if (!Parser.Line->Tokens.empty()) Parser.addUnwrappedLine(); assert(Parser.Line->Tokens.empty()); + if (DiscardLines) + Parser.CurrentLines->truncate(OriginalNumLines); Parser.Line = std::move(PreBlockLine); if (Parser.CurrentLines == &Parser.PreprocessorDirectives) Parser.AtEndOfPPLine = true; @@ -126,6 +132,8 @@ class ScopedLineState { std::unique_ptr<UnwrappedLine> PreBlockLine; SmallVectorImpl<UnwrappedLine> *OriginalLines; + size_t OriginalNumLines; + bool DiscardLines; }; class CompoundStatementIndenter { @@ -170,6 +178,7 @@ void UnwrappedLineParser::reset() { PPBranchLevel = -1; IncludeGuard = getIncludeGuardState(Style.IndentPPDirectives); IncludeGuardToken = nullptr; + ParsedPPDirectives.clear(); Line.reset(new UnwrappedLine); CommentsBeforeNextToken.clear(); FormatTok = nullptr; @@ -1101,6 +1110,28 @@ void UnwrappedLineParser::conditionalCompilationEnd() { PPStack.pop_back(); } +UnwrappedLineParser::PPState UnwrappedLineParser::savePPState() const { + return {PPStack, + PPBranchLevel, + PPLevelBranchIndex, + PPLevelBranchCount, + PPChainBranchIndex, + IncludeGuard, + IncludeGuardToken, + AtEndOfPPLine}; +} + +void UnwrappedLineParser::restorePPState(const PPState &State) { + PPStack = State.PPStack; + PPBranchLevel = State.PPBranchLevel; + PPLevelBranchIndex = State.PPLevelBranchIndex; + PPLevelBranchCount = State.PPLevelBranchCount; + PPChainBranchIndex = State.PPChainBranchIndex; + IncludeGuard = State.IncludeGuard; + IncludeGuardToken = State.IncludeGuardToken; + AtEndOfPPLine = State.AtEndOfPPLine; +} + void UnwrappedLineParser::parsePPIf(bool IfDef) { bool IfNDef = FormatTok->is(tok::pp_ifndef); nextToken(); @@ -5049,10 +5080,15 @@ void UnwrappedLineParser::readToken(int LevelDifference) { } distributeComments(Comments, FormatTok); Comments.clear(); + // If the directive was parsed before the token stream was rewound (see + // parseMacroCall()), its lines were kept. Parse it again only for its + // effect on the preprocessor bookkeeping and discard the new lines. + const bool ParsedBefore = !ParsedPPDirectives.insert(FormatTok).second; // If there is an unfinished unwrapped line, we flush the preprocessor // directives only after that unwrapped line was finished later. bool SwitchToPreprocessorLines = !Line->Tokens.empty(); - ScopedLineState BlockState(*this, SwitchToPreprocessorLines); + ScopedLineState BlockState(*this, SwitchToPreprocessorLines, + /*DiscardLines=*/ParsedBefore); assert((LevelDifference >= 0 || static_cast<unsigned>(-LevelDifference) <= Line->Level) && "LevelDifference makes Line->Level negative"); @@ -5094,6 +5130,11 @@ void UnwrappedLineParser::readToken(int LevelDifference) { !Line->InPPDirective) { FormatToken *ID = FormatTok; unsigned Position = Tokens->getPosition(); + // Parsing the arguments of the call may parse preprocessor directives, + // which are parsed again if the token stream is rewound because the + // arguments are discarded. The preprocessor bookkeeping is restored + // whenever that happens. + const auto SavedPPState = savePPState(); // To correctly parse the code, we need to replace the tokens of the macro // call with its expansion. @@ -5102,7 +5143,7 @@ void UnwrappedLineParser::readToken(int LevelDifference) { bool OldInExpansion = InExpansion; InExpansion = true; // We parse the macro call into a new line. - auto Args = parseMacroCall(); + auto Args = parseMacroCall(SavedPPState); InExpansion = OldInExpansion; assert(Line->Tokens.front().Tok == ID); // And remember the unexpanded macro call tokens. @@ -5137,6 +5178,7 @@ void UnwrappedLineParser::readToken(int LevelDifference) { Tokens->setPosition(Position); // Not nextToken(), which would push the stale FormatTok onto the line. FormatTok = Tokens->getNextToken(); + restorePPState(SavedPPState); assert(!Args && Macros.objectLike(ID->TokenText)); } if ((!Args && Macros.objectLike(ID->TokenText)) || @@ -5167,6 +5209,7 @@ void UnwrappedLineParser::readToken(int LevelDifference) { }); Tokens->setPosition(Position); FormatTok = ID; + restorePPState(SavedPPState); } } @@ -5196,7 +5239,7 @@ void pushTokens(Iterator Begin, Iterator End, } // namespace std::optional<llvm::SmallVector<llvm::SmallVector<FormatToken *, 8>, 1>> -UnwrappedLineParser::parseMacroCall() { +UnwrappedLineParser::parseMacroCall(const PPState &SavedPPState) { std::optional<llvm::SmallVector<llvm::SmallVector<FormatToken *, 8>, 1>> Args; assert(Line->Tokens.empty()); // Not nextToken(), which would already expand a directly following macro @@ -5255,6 +5298,7 @@ UnwrappedLineParser::parseMacroCall() { Line->Tokens.resize(1); Tokens->setPosition(Position); FormatTok = Tok; + restorePPState(SavedPPState); return {}; } diff --git a/clang/lib/Format/UnwrappedLineParser.h b/clang/lib/Format/UnwrappedLineParser.h index 8fa4e9f7540d5..dcdaaf2ab2f3b 100644 --- a/clang/lib/Format/UnwrappedLineParser.h +++ b/clang/lib/Format/UnwrappedLineParser.h @@ -16,6 +16,7 @@ #define LLVM_CLANG_LIB_FORMAT_UNWRAPPEDLINEPARSER_H #include "Macros.h" +#include "llvm/ADT/SmallPtrSet.h" #include <stack> namespace clang { @@ -213,8 +214,11 @@ class UnwrappedLineParser { void parseVerilogExtern(); // Skip things that can precede the keywords like module. void skipVerilogQualifiers(); + struct PPState; + PPState savePPState() const; + void restorePPState(const PPState &State); std::optional<llvm::SmallVector<llvm::SmallVector<FormatToken *, 8>, 1>> - parseMacroCall(); + parseMacroCall(const PPState &SavedPPState); // Used by addUnwrappedLine to denote whether to keep or remove a level // when resetting the line state. @@ -416,6 +420,26 @@ class UnwrappedLineParser { // IncludeGuardState == IG_IfNdefed. FormatToken *IncludeGuardToken; + // The preprocessor bookkeeping that is rolled back when the token stream is + // rewound over preprocessor directives, which happens if the speculatively + // parsed arguments of a macro call are discarded. See readToken(). + struct PPState { + SmallVector<PPBranch, 16> PPStack; + int PPBranchLevel; + SmallVector<int, 8> PPLevelBranchIndex; + SmallVector<int, 8> PPLevelBranchCount; + std::stack<int> PPChainBranchIndex; + IncludeGuardState IncludeGuard; + FormatToken *IncludeGuardToken; + bool AtEndOfPPLine; + }; + + // The hash tokens of the parsed preprocessor directives. The lines of a + // directive are kept when the token stream is rewound over it, so it is then + // parsed again only for its effect on the preprocessor bookkeeping. See + // readToken(). + llvm::SmallPtrSet<const FormatToken *, 16> ParsedPPDirectives; + // Contains the first start column where the source begins. This is zero for // normal source code and may be nonzero when formatting a code fragment that // does not start at the beginning of the file. diff --git a/clang/unittests/Format/FormatTestMacroExpansion.cpp b/clang/unittests/Format/FormatTestMacroExpansion.cpp index 26d7aa05e1561..3100c22559c53 100644 --- a/clang/unittests/Format/FormatTestMacroExpansion.cpp +++ b/clang/unittests/Format/FormatTestMacroExpansion.cpp @@ -299,6 +299,104 @@ TEST_F(FormatTestMacroExpansion, IndentChildrenWithinMacroCall) { Style); } +TEST_F(FormatTestMacroExpansion, PPDirectiveInDiscardedMacroArgs) { + FormatStyle Style = getLLVMStyle(); + Style.Macros.push_back("A=a"); + Style.Macros.push_back("ID(x)=x"); + Style.Macros.push_back("PAIR(x, y)=x y"); + Style.Macros.push_back("STMT=f();"); + Style.Macros.push_back("EMPTY="); + + verifyIncompleteFormat("A(\n" + "#endif", + Style); + verifyIncompleteFormat("ID(\n" + "#endif", + Style); + verifyIncompleteFormat("ID(\n" + "#define X 1", + Style); + verifyFormat("A(\n" + "#if X\n" + " b;\n" + "#endif\n" + ")", + Style); + verifyFormat("ID(a,\n" + "#if X\n" + " b\n" + "#endif\n" + ");", + Style); + verifyFormat("PAIR(\n" + "#define X ,\n" + " a)", + Style); + verifyFormat("ID(\n" + "#if 0\n" + ",\n" + "#endif\n" + " if (a) {\n" + " f();\n" + " })", + Style); + verifyFormat("STMT\n" + "#define F(x) g(x)\n" + "b;", + "STMT\n" + "#define F(x) g( x )\n" + "b;", + Style); + verifyFormat("EMPTY(\n" + "#define F(x) g(x)\n" + "1)", + "EMPTY(\n" + "#define F(x) g(x)\n" + "1)", + Style); + EXPECT_EQ("A(\n" + "ID(\n" + "#define F(x) g(x)\n" + "))", + format("A(ID(\n" + "#define F(x) g(x)\n" + "))", + Style, SC_ExpectIncomplete)); + + Style.IndentPPDirectives = FormatStyle::PPDIS_BeforeHash; + verifyFormat("#if OUTER\n" + "EMPTY(\n" + " #define X 1\n" + ")\n" + "#endif", + Style); + verifyFormat("void f() {\n" + " if (x) {\n" + " ID(a,\n" + "#if Y\n" + " #define Z 1\n" + "#endif\n" + " );\n" + " }\n" + "}", + Style); +} + +TEST_F(FormatTestMacroExpansion, PPDirectiveBeforeEmptyExpansion) { + FormatStyle Style = getLLVMStyle(); + Style.Macros.push_back("ID(x)=x"); + verifyFormat("#define X 1\n" + "ID()", + "#define X 1\n" + "ID()", + Style); + verifyFormat("#define X 1\n" + "ID()", + "#define X 1\n" + "ID()", + Style, {tooling::Range(0, 13)}); // line 1 +} + TEST_F(FormatTestMacroExpansion, ObjectLikeMacroCalledWithArgsDoesNotHang) { FormatStyle Style = getLLVMStyle(); Style.Macros.push_back("CASE=case"); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
