[clang] [clang-format] Fix crash involving array designators and dangling comma (PR #77045)
via cfe-commits
cfe-commits at lists.llvm.org
Sat Jan 6 15:41:53 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/5] [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/5] 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/5] 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/5] 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 29286ff7ad6c90532266805d0cd739f351769cd5 Mon Sep 17 00:00:00 2001
From: XDeme <66138117+XDeme at users.noreply.github.com>
Date: Sat, 6 Jan 2024 20:41:45 -0300
Subject: [PATCH 5/5] Apply suggestions from code review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Co-authored-by: Björn Schäpers <github at hazardy.de>
---
clang/lib/Format/WhitespaceManager.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp
index 713a613f812e22..7e83838c7b994e 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -1446,7 +1446,7 @@ WhitespaceManager::CellDescriptions WhitespaceManager::getCells(unsigned Start,
Cells.back().EndIndex = i;
if (const auto *Next = C.Tok->getNextNonComment();
- Next && Next->isNot(tok::r_brace)) { // dangling comma
+ Next && Next->isNot(tok::r_brace)) { // Dangling comma.
++Cell;
}
}
@@ -1459,7 +1459,7 @@ WhitespaceManager::CellDescriptions WhitespaceManager::getCells(unsigned Start,
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,
+ // When this happens we make the cells non rectangular,
// avoiding an access out of bound later on.
MatchingParen->MatchingParen->Previous->isNot(tok::equal)
? Cell + 1
More information about the cfe-commits
mailing list