Skip to content

[Clang] Fix an out of bound access in -verify comment parsing #141940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 29, 2025

Conversation

cor3ntin
Copy link
Contributor

When the comment ends with a splice at EOF.

Fixes #141221

When the comment ends with a splice at EOF.

Fixes llvm#141221
@cor3ntin cor3ntin marked this pull request as ready for review May 29, 2025 13:18
@llvmbot llvmbot added the clang Clang issues not falling into any other category label May 29, 2025
@llvmbot
Copy link
Member

llvmbot commented May 29, 2025

@llvm/pr-subscribers-clang

Author: cor3ntin (cor3ntin)

Changes

When the comment ends with a splice at EOF.

Fixes #141221


Full diff: https://github.com/llvm/llvm-project/pull/141940.diff

2 Files Affected:

  • (modified) clang/lib/Frontend/VerifyDiagnosticConsumer.cpp (+1-1)
  • (added) clang/test/Frontend/verify-gh141221.c (+6)
diff --git a/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp b/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
index 89fda3e839cb9..eb241f34d095e 100644
--- a/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
+++ b/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
@@ -812,7 +812,7 @@ bool VerifyDiagnosticConsumer::HandleComment(Preprocessor &PP,
     C2 += C.substr(last, loc-last);
     last = loc + 1;
 
-    if (C[last] == '\n' || C[last] == '\r') {
+    if (last < C.size() && (C[last] == '\n' || C[last] == '\r')) {
       ++last;
 
       // Escape \r\n  or \n\r, but not \n\n.
diff --git a/clang/test/Frontend/verify-gh141221.c b/clang/test/Frontend/verify-gh141221.c
new file mode 100644
index 0000000000000..eb0be46a3da8b
--- /dev/null
+++ b/clang/test/Frontend/verify-gh141221.c
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -verify %s
+
+// Check that we don't crash if the file ends in a splice
+// This file should *NOT* end with a new line
+a;
+// expected-error@-1 {{}} \
\ No newline at end of file

@llvmbot llvmbot added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label May 29, 2025
@cor3ntin cor3ntin force-pushed the corentin/gh141221 branch from 10f354d to ca2ea73 Compare May 29, 2025 16:40
@cor3ntin cor3ntin merged commit 3315b84 into llvm:main May 29, 2025
21 checks passed
svkeerthy pushed a commit that referenced this pull request May 29, 2025
google-yfyang pushed a commit to google-yfyang/llvm-project that referenced this pull request May 29, 2025
Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you not do a release note b/c this was introduced w/ a recent change?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

out of bound access on a -verify test line ending with a splice before EOF
4 participants