[clang] [clang-format] Fix crash involving array designators (PR #77045)

via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 9 13:01:46 PST 2024


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.com at gmail.com>
Date: Fri, 5 Jan 2024 01:23:16 -0300
Subject: [PATCH 1/7] [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.com at gmail.com>
Date: Fri, 5 Jan 2024 11:32:37 -0300
Subject: [PATCH 2/7] 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.com at gmail.com>
Date: Fri, 5 Jan 2024 16:21:33 -0300
Subject: [PATCH 3/7] 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.com at gmail.com>
Date: Sat, 6 Jan 2024 19:30:53 -0300
Subject: [PATCH 4/7] 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.com at gmail.com>
Date: Tue, 9 Jan 2024 01:33:24 -0300
Subject: [PATCH 5/7] 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.com at gmail.com>
Date: Tue, 9 Jan 2024 12:45:42 -0300
Subject: [PATCH 6/7] 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.com at gmail.com>
Date: Tue, 9 Jan 2024 18:01:31 -0300
Subject: [PATCH 7/7] 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;
       }



More information about the cfe-commits mailing list