https://github.com/sstwcw created
https://github.com/llvm/llvm-project/pull/147156
In the debug build, this kind of input caused the assertion in the function
`countLeadingWhitespace` to fail. The release build without assertions
outputted `?` `?` `/` separated by spaces.
```C
#define A ??/
int i;
```
This is because the code in `countLeadingWhitespace` assumed that the
underlying lexer recognized the entire `??/` sequence as a single token. In
fact, the lexer recognized it as 3 separate tokens. The flag to make the lexer
recognize trigraphs was never enabled.
This patch enables the flag in the underlying lexer. This way, the program now
either turns the trigraph into a single `\` or removes it altogether if the
line is short enough. There are operators like the `??=` in C#. So the flag
is not enabled for all input languages. Instead the check for the token size is
moved from the assert line into the if line.
The problem was introduced by my own patch 370bee480139 from about 3 years ago.
I added code to count the number of characters in the escape sequence probably
just because the block of code used to have a comment saying someone should add
the feature. Maybe I forgot to enable assertions when I ran the code. I found
the problem because reviewing pull request 145243 made me look at the code
again.
>From 2d9e11f625747544bb0984478c9bdd8cad7aef6a Mon Sep 17 00:00:00 2001
From: sstwcw <su3e8a96kzl...@posteo.net>
Date: Sat, 5 Jul 2025 16:10:17 +0000
Subject: [PATCH] [clang-format] Stop crashing when the input contains `??/\n`
In the debug build, this kind of input caused the assertion in the
function `countLeadingWhitespace` to fail. The release build without
assertions outputted `?` `?` `/` separated by spaces.
```C
#define A ??/
int i;
```
This is because the code in `countLeadingWhitespace` assumed that the
underlying lexer recognized the entire `??/` sequence as a single token.
In fact, the lexer recognized it as 3 separate tokens. The flag to make
the lexer recognize trigraphs was never enabled.
This patch enables the flag in the underlying lexer. This way, the
program now either turns the trigraph into a single `\` or removes it
altogether if the line is short enough. There are operators like the
`??=` in C#. So the flag is not enabled for all input languages.
Instead the check for the token size is moved from the assert line into
the if line.
The problem was introduced by my own patch 370bee480139 from about 3
years ago. I added code to count the number of characters in the escape
sequence probably just because the block of code used to have a comment
saying someone should add the feature. Maybe I forgot to enable
assertions when I ran the code. I found the problem because reviewing
pull request 145243 made me look at the code again.
---
clang/lib/Format/Format.cpp | 1 +
clang/lib/Format/FormatTokenLexer.cpp | 12 +++++++-----
clang/unittests/Format/FormatTest.cpp | 25 +++++++++++++++++++++++++
3 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index bdaf264e9adce..d7e4d6bb63aeb 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -4052,6 +4052,7 @@ LangOptions getFormattingLangOpts(const FormatStyle
&Style) {
// the sequence "<::" will be unconditionally treated as "[:".
// Cf. Lexer::LexTokenInternal.
LangOpts.Digraphs = SinceCpp11;
+ LangOpts.Trigraphs = Style.isCpp();
LangOpts.LineComment = 1;
LangOpts.Bool = 1;
diff --git a/clang/lib/Format/FormatTokenLexer.cpp
b/clang/lib/Format/FormatTokenLexer.cpp
index 06f68ec8b0fc1..f9442265a8e59 100644
--- a/clang/lib/Format/FormatTokenLexer.cpp
+++ b/clang/lib/Format/FormatTokenLexer.cpp
@@ -1220,12 +1220,11 @@ static size_t countLeadingWhitespace(StringRef Text) {
break;
// Splice found, consume it.
Cur = Lookahead + 1;
- } else if (Cur[0] == '?' && Cur[1] == '?' && Cur[2] == '/' &&
- (Cur[3] == '\n' || Cur[3] == '\r')) {
+ } else if (End - Cur >= 4u && Cur[0] == '?' && Cur[1] == '?' &&
+ Cur[2] == '/' && (Cur[3] == '\n' || Cur[3] == '\r')) {
// Newlines can also be escaped by a '?' '?' '/' trigraph. By the way,
the
// characters are quoted individually in this comment because if we write
// them together some compilers warn that we have a trigraph in the code.
- assert(End - Cur >= 4);
Cur += 4;
} else {
break;
@@ -1300,8 +1299,11 @@ FormatToken *FormatTokenLexer::getNextToken() {
case '\\':
case '?':
case '/':
- // The text was entirely whitespace when this loop was entered. Thus
- // this has to be an escape sequence.
+ // The code preceding the loop and in the countLeadingWhitespace
+ // function guarantees that Text is entirely whitespace, not including
+ // comments but including escaped newlines which may be escaped with a
+ // trigraph. So if 1 of these characters show up, then it has to be in
+ // an escape sequence.
assert(Text.substr(i, 4) == "\?\?/\r" ||
Text.substr(i, 4) == "\?\?/\n" ||
(i >= 1 && (Text.substr(i - 1, 4) == "\?\?/\r" ||
diff --git a/clang/unittests/Format/FormatTest.cpp
b/clang/unittests/Format/FormatTest.cpp
index 944e7c3fb152a..40b85f0dad355 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -6681,6 +6681,31 @@ TEST_F(FormatTest, EscapedNewlines) {
" int x(int a);",
AlignLeft);
+ // Escaped with a trigraph.
+ verifyFormat("#define A \\\n"
+ " int i; \\\n"
+ " int j;",
+ "#define A \?\?/\n"
+ "int i;\?\?/\n"
+ " int j;",
+ Narrow);
+ verifyFormat("#define A \\\r\n"
+ " int i; \\\r\n"
+ " int j;",
+ "#define A \?\?/\r\n"
+ "int i;\?\?/\r\n"
+ " int j;",
+ Narrow);
+ verifyFormat("#define A int i;", "#define A \?\?/\n"
+ "int i;");
+ verifyFormat("#define A int i;", "#define A \?\?/\r\n"
+ "int i;");
+ // In a language that does not support the trigraph, the program should not
+ // crash.
+ verifyNoCrash("#define A \?\?/\n"
+ "int i;",
+ getGoogleStyle(FormatStyle::LK_CSharp));
+
// CRLF line endings
verifyFormat("#define A \\\r\n int i; \\\r\n int j;",
"#define A \\\r\nint i;\\\r\n int j;", Narrow);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
