https://github.com/XDeme updated https://github.com/llvm/llvm-project/pull/77045
>From d9cbbe48b96d27bff3fc926b60d039ed05f00489 Mon Sep 17 00:00:00 2001 From: XDeme <fernando.tagawa.gamail....@gmail.com> Date: Fri, 5 Jan 2024 01:23:16 -0300 Subject: [PATCH 1/8] [clang-format] Fix crash involving array designators and dangling comma Fixes llvm/llvm-project#76716 Added a check to prevent null deferencing --- clang/lib/Format/WhitespaceManager.cpp | 3 ++- clang/unittests/Format/FormatTest.cpp | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index 3bc6915b8df0a7..95693f4588c631 100644 --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/clang/lib/Format/WhitespaceManager.cpp @@ -1444,7 +1444,8 @@ WhitespaceManager::CellDescriptions WhitespaceManager::getCells(unsigned Start, } else if (C.Tok->is(tok::comma)) { if (!Cells.empty()) Cells.back().EndIndex = i; - if (C.Tok->getNextNonComment()->isNot(tok::r_brace)) // dangling comma + const FormatToken *Next = C.Tok->getNextNonComment(); + if (Next && Next->isNot(tok::r_brace)) // dangling comma ++Cell; } } else if (Depth == 1) { diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 881993ede17c3d..c9f91953c13f52 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -21084,6 +21084,12 @@ TEST_F(FormatTest, CatchAlignArrayOfStructuresLeftAlignment) { "};", Style); + verifyNoCrash("Foo f[] = {\n" + " [0] = { 1, },\n" + " [1] { 1, },\n" + "};", + Style); + verifyFormat("return GradForUnaryCwise(g, {\n" " {{\"sign\"}, \"Sign\", {\"x\", " "\"dy\"} },\n" >From 78e006de352cb801f6c07ac7f6bd0ab881560406 Mon Sep 17 00:00:00 2001 From: XDeme <fernando.tagawa.gamail....@gmail.com> Date: Fri, 5 Jan 2024 11:32:37 -0300 Subject: [PATCH 2/8] Addresses comments --- clang/lib/Format/WhitespaceManager.cpp | 6 ++++-- clang/unittests/Format/FormatTest.cpp | 6 ++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index 95693f4588c631..aeac0ba0c6d03c 100644 --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/clang/lib/Format/WhitespaceManager.cpp @@ -1444,9 +1444,11 @@ WhitespaceManager::CellDescriptions WhitespaceManager::getCells(unsigned Start, } else if (C.Tok->is(tok::comma)) { if (!Cells.empty()) Cells.back().EndIndex = i; - const FormatToken *Next = C.Tok->getNextNonComment(); - if (Next && Next->isNot(tok::r_brace)) // dangling comma + + if (const auto *Next = C.Tok->getNextNonComment(); + Next && Next->isNot(tok::r_brace)) { // dangling comma ++Cell; + } } } else if (Depth == 1) { if (C.Tok == MatchingParen) { diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index c9f91953c13f52..d6f561e822d08a 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -20842,6 +20842,12 @@ TEST_F(FormatTest, CatchAlignArrayOfStructuresRightAlignment) { "};", Style); + verifyNoCrash("Foo f[] = {\n" + " [0] = { 1, },\n" + " [1] { 1, },\n" + "};", + Style); + verifyFormat("return GradForUnaryCwise(g, {\n" " {{\"sign\"}, \"Sign\", " " {\"x\", \"dy\"}},\n" >From 3f82d6664de1ea06589393ff27aced4e4177a72c Mon Sep 17 00:00:00 2001 From: XDeme <fernando.tagawa.gamail....@gmail.com> Date: Fri, 5 Jan 2024 16:21:33 -0300 Subject: [PATCH 3/8] Fix real problem --- clang/lib/Format/WhitespaceManager.cpp | 7 +++++-- clang/unittests/Format/FormatTest.cpp | 10 ++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index aeac0ba0c6d03c..e91ec6afec74af 100644 --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/clang/lib/Format/WhitespaceManager.cpp @@ -1455,8 +1455,11 @@ WhitespaceManager::CellDescriptions WhitespaceManager::getCells(unsigned Start, if (!Cells.empty()) Cells.back().EndIndex = i; Cells.push_back(CellDescription{i, ++Cell, i + 1, false, nullptr}); - CellCounts.push_back(C.Tok->Previous->isNot(tok::comma) ? Cell + 1 - : Cell); + CellCounts.push_back( + C.Tok->Previous->isNot(tok::comma) && + !MatchingParen->MatchingParen->Previous->is(tok::equal) + ? Cell + 1 + : Cell); // Go to the next non-comment and ensure there is a break in front const auto *NextNonComment = C.Tok->getNextNonComment(); while (NextNonComment->is(tok::comma)) diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index d6f561e822d08a..660f1a07c96707 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -20847,6 +20847,11 @@ TEST_F(FormatTest, CatchAlignArrayOfStructuresRightAlignment) { " [1] { 1, },\n" "};", Style); + verifyNoCrash("Foo foo[] = {\n" + " [0] = {1, 1},\n" + " [1] { 1, 1, },\n" + " [2] { 1, 1, },\n" + "};"); verifyFormat("return GradForUnaryCwise(g, {\n" " {{\"sign\"}, \"Sign\", " @@ -21095,6 +21100,11 @@ TEST_F(FormatTest, CatchAlignArrayOfStructuresLeftAlignment) { " [1] { 1, },\n" "};", Style); + verifyNoCrash("Foo foo[] = {\n" + " [0] = {1, 1},\n" + " [1] { 1, 1, },\n" + " [2] { 1, 1, },\n" + "};"); verifyFormat("return GradForUnaryCwise(g, {\n" " {{\"sign\"}, \"Sign\", {\"x\", " >From 59a6ea0ec1df05448077acf94890987ccbddb170 Mon Sep 17 00:00:00 2001 From: XDeme <fernando.tagawa.gamail....@gmail.com> Date: Sat, 6 Jan 2024 19:30:53 -0300 Subject: [PATCH 4/8] comment --- clang/lib/Format/WhitespaceManager.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index e91ec6afec74af..713a613f812e22 100644 --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/clang/lib/Format/WhitespaceManager.cpp @@ -1457,7 +1457,11 @@ WhitespaceManager::CellDescriptions WhitespaceManager::getCells(unsigned Start, Cells.push_back(CellDescription{i, ++Cell, i + 1, false, nullptr}); CellCounts.push_back( C.Tok->Previous->isNot(tok::comma) && - !MatchingParen->MatchingParen->Previous->is(tok::equal) + // When dealing with C array designators. There is a + // possibility of some nested array not having an `=`. + // When this happens we make the cells non retangular, + // avoiding an access out of bound later on. + MatchingParen->MatchingParen->Previous->isNot(tok::equal) ? Cell + 1 : Cell); // Go to the next non-comment and ensure there is a break in front >From d40e09fefcaff9ec4f7750d93f2f994b89782632 Mon Sep 17 00:00:00 2001 From: XDeme <fernando.tagawa.gamail....@gmail.com> Date: Tue, 9 Jan 2024 01:33:24 -0300 Subject: [PATCH 5/8] Fix root cause --- clang/lib/Format/UnwrappedLineParser.cpp | 4 ++++ clang/lib/Format/WhitespaceManager.cpp | 11 ++--------- clang/unittests/Format/TokenAnnotatorTest.cpp | 5 +++++ 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp index 684609747a5513..96f9ebe61a6652 100644 --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -2315,6 +2315,10 @@ bool UnwrappedLineParser::tryToParseLambdaIntroducer() { if (Next->is(tok::greater)) return false; } + if (const auto Kind = FormatTok->Tok.getKind(); + tok::isLiteral(Kind) && !tok::isStringLiteral(Kind)) { + return false; + } parseSquare(/*LambdaIntroducer=*/true); return true; } diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index 713a613f812e22..aeac0ba0c6d03c 100644 --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/clang/lib/Format/WhitespaceManager.cpp @@ -1455,15 +1455,8 @@ WhitespaceManager::CellDescriptions WhitespaceManager::getCells(unsigned Start, if (!Cells.empty()) Cells.back().EndIndex = i; Cells.push_back(CellDescription{i, ++Cell, i + 1, false, nullptr}); - CellCounts.push_back( - C.Tok->Previous->isNot(tok::comma) && - // When dealing with C array designators. There is a - // possibility of some nested array not having an `=`. - // When this happens we make the cells non retangular, - // avoiding an access out of bound later on. - MatchingParen->MatchingParen->Previous->isNot(tok::equal) - ? Cell + 1 - : Cell); + CellCounts.push_back(C.Tok->Previous->isNot(tok::comma) ? Cell + 1 + : Cell); // Go to the next non-comment and ensure there is a break in front const auto *NextNonComment = C.Tok->getNextNonComment(); while (NextNonComment->is(tok::comma)) diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp index 2cafc0438ffb46..ea295ef91bfe81 100644 --- a/clang/unittests/Format/TokenAnnotatorTest.cpp +++ b/clang/unittests/Format/TokenAnnotatorTest.cpp @@ -2324,6 +2324,11 @@ TEST_F(TokenAnnotatorTest, UnderstandDesignatedInitializers) { EXPECT_BRACE_KIND(Tokens[1], BK_BracedInit); EXPECT_TOKEN(Tokens[6], tok::period, TT_DesignatedInitializerPeriod); EXPECT_TOKEN(Tokens[13], tok::period, TT_DesignatedInitializerPeriod); + + Tokens = annotate("Foo foo[] = {[0]{}};"); + ASSERT_EQ(Tokens.size(), 14u) << Tokens; + EXPECT_TOKEN(Tokens[6], tok::l_square, TT_DesignatedInitializerLSquare); + EXPECT_BRACE_KIND(Tokens[9], BK_BracedInit); } TEST_F(TokenAnnotatorTest, UnderstandsJavaScript) { >From 6a2cc277ac77b3b3b84afd33262c7d5cda3182fd Mon Sep 17 00:00:00 2001 From: XDeme <fernando.tagawa.gamail....@gmail.com> Date: Tue, 9 Jan 2024 12:45:42 -0300 Subject: [PATCH 6/8] Cleanup --- clang/lib/Format/WhitespaceManager.cpp | 4 +--- clang/unittests/Format/FormatTest.cpp | 10 ---------- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index aeac0ba0c6d03c..2e3203c6482b23 100644 --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/clang/lib/Format/WhitespaceManager.cpp @@ -1445,10 +1445,8 @@ WhitespaceManager::CellDescriptions WhitespaceManager::getCells(unsigned Start, if (!Cells.empty()) Cells.back().EndIndex = i; - if (const auto *Next = C.Tok->getNextNonComment(); - Next && Next->isNot(tok::r_brace)) { // dangling comma + if (C.Tok->getNextNonComment()->isNot(tok::r_brace)) // dangling comma ++Cell; - } } } else if (Depth == 1) { if (C.Tok == MatchingParen) { diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 660f1a07c96707..72b2fc930d2d6f 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -20842,11 +20842,6 @@ TEST_F(FormatTest, CatchAlignArrayOfStructuresRightAlignment) { "};", Style); - verifyNoCrash("Foo f[] = {\n" - " [0] = { 1, },\n" - " [1] { 1, },\n" - "};", - Style); verifyNoCrash("Foo foo[] = {\n" " [0] = {1, 1},\n" " [1] { 1, 1, },\n" @@ -21095,11 +21090,6 @@ TEST_F(FormatTest, CatchAlignArrayOfStructuresLeftAlignment) { "};", Style); - verifyNoCrash("Foo f[] = {\n" - " [0] = { 1, },\n" - " [1] { 1, },\n" - "};", - Style); verifyNoCrash("Foo foo[] = {\n" " [0] = {1, 1},\n" " [1] { 1, 1, },\n" >From e4cc273384a869de7a8efd41ba5e8ccb2510abda Mon Sep 17 00:00:00 2001 From: XDeme <fernando.tagawa.gamail....@gmail.com> Date: Tue, 9 Jan 2024 18:01:31 -0300 Subject: [PATCH 7/8] Addresses comments --- clang/lib/Format/UnwrappedLineParser.cpp | 4 +--- clang/lib/Format/WhitespaceManager.cpp | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp index 96f9ebe61a6652..a80f8b5bb6fc5b 100644 --- a/clang/lib/Format/UnwrappedLineParser.cpp +++ b/clang/lib/Format/UnwrappedLineParser.cpp @@ -2315,10 +2315,8 @@ bool UnwrappedLineParser::tryToParseLambdaIntroducer() { if (Next->is(tok::greater)) return false; } - if (const auto Kind = FormatTok->Tok.getKind(); - tok::isLiteral(Kind) && !tok::isStringLiteral(Kind)) { + if (tok::isLiteral(FormatTok->Tok.getKind())) return false; - } parseSquare(/*LambdaIntroducer=*/true); return true; } diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index 2e3203c6482b23..3bc6915b8df0a7 100644 --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/clang/lib/Format/WhitespaceManager.cpp @@ -1444,7 +1444,6 @@ WhitespaceManager::CellDescriptions WhitespaceManager::getCells(unsigned Start, } else if (C.Tok->is(tok::comma)) { if (!Cells.empty()) Cells.back().EndIndex = i; - if (C.Tok->getNextNonComment()->isNot(tok::r_brace)) // dangling comma ++Cell; } >From 5b0a704a79a79d01f47ebfa93ab79971fc96fa93 Mon Sep 17 00:00:00 2001 From: XDeme <fernando.tagawa.gamail....@gmail.com> Date: Wed, 10 Jan 2024 01:07:36 -0300 Subject: [PATCH 8/8] Addresses comments --- clang/lib/Format/WhitespaceManager.h | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/lib/Format/WhitespaceManager.h b/clang/lib/Format/WhitespaceManager.h index 24fe492dcb0269..dc6f60e5deeedf 100644 --- a/clang/lib/Format/WhitespaceManager.h +++ b/clang/lib/Format/WhitespaceManager.h @@ -282,6 +282,7 @@ class WhitespaceManager { for (auto PrevIter = Start; PrevIter != End; ++PrevIter) { // If we broke the line the initial spaces are already // accounted for. + assert(PrevIter->Index < Changes.size()); if (Changes[PrevIter->Index].NewlinesBefore > 0) NetWidth = 0; NetWidth += _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits