[clang] 4357571 - [Clang] Fix crash caused by line splicing in doc comment

[Corentin Jabot via cfe-commits]

Author: Corentin Jabot
Date: 2023-04-12T10:07:51+02:00
New Revision: 43575719d0c6d8cf5afedf39f6d89f69231aedc4

URL: https://github.com/llvm/llvm-project/commit/43575719d0c6d8cf5afedf39f6d89f69231aedc4
DIFF: https://github.com/llvm/llvm-project/commit/43575719d0c6d8cf5afedf39f6d89f69231aedc4.diff

LOG: [Clang] Fix crash caused by line splicing in doc comment

Because the comment parser does not support slices,
we emit a warning for comments that do contain
a splice within their delimiter, and do not add them as
documentation comment.

Fixes #62054

Reviewed By: shafik, aaron.ballman

Differential Revision: https://reviews.llvm.org/D148029

Added: 
    

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/include/clang/AST/RawCommentList.h
    clang/include/clang/Basic/DiagnosticSemaKinds.td
    clang/lib/Sema/Sema.cpp
    clang/test/Lexer/comment-escape.c

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7dff43ba30930..dd66c71122999 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -290,6 +290,8 @@ Bug Fixes in This Version
   (`#61142 <https://github.com/llvm/llvm-project/issues/61142>`_)
 - Clang now better diagnose placeholder types constrained with a concept that is
   not a type concept.
+- Fix crash when a doc comment contains a line splicing.
+  (`#62054 <https://github.com/llvm/llvm-project/issues/62054>`_)
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

diff  --git a/clang/include/clang/AST/RawCommentList.h b/clang/include/clang/AST/RawCommentList.h
index 1bb8d7ce40a90..a293e219e2502 100644
--- a/clang/include/clang/AST/RawCommentList.h
+++ b/clang/include/clang/AST/RawCommentList.h
@@ -115,6 +115,17 @@ class RawComment {
     return extractBriefText(Context);
   }
 
+  bool hasUnsupportedSplice(const SourceManager &SourceMgr) const {
+    if (!isInvalid())
+      return false;
+    StringRef Text = getRawText(SourceMgr);
+    if (Text.size() < 6 || Text[0] != '/')
+      return false;
+    if (Text[1] == '*')
+      return Text[Text.size() - 1] != '/' || Text[Text.size() - 2] != '*';
+    return Text[1] != '/';
+  }
+
   /// Returns sanitized comment text, suitable for presentation in editor UIs.
   /// E.g. will transform:
   ///     // This is a long multiline comment.

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 379554baaaa1f..cd5930d385e69 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11377,6 +11377,8 @@ def err_coro_invalid_addr_of_label : Error<
 let CategoryName = "Documentation Issue" in {
 def warn_not_a_doxygen_trailing_member_comment : Warning<
   "not a Doxygen trailing comment">, InGroup<Documentation>, DefaultIgnore;
+def warn_splice_in_doxygen_comment : Warning<
+  "line splicing in Doxygen comments are not supported">, InGroup<Documentation>, DefaultIgnore;
 } // end of documentation issue category
 
 let CategoryName = "Nullability Issue" in {

diff  --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 89ac016f60e97..e1b309bd01938 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -2390,7 +2390,7 @@ void Sema::ActOnComment(SourceRange Comment) {
       SourceMgr.isInSystemHeader(Comment.getBegin()))
     return;
   RawComment RC(SourceMgr, Comment, LangOpts.CommentOpts, false);
-  if (RC.isAlmostTrailingComment()) {
+  if (RC.isAlmostTrailingComment() || RC.hasUnsupportedSplice(SourceMgr)) {
     SourceRange MagicMarkerRange(Comment.getBegin(),
                                  Comment.getBegin().getLocWithOffset(3));
     StringRef MagicMarkerText;
@@ -2401,6 +2401,11 @@ void Sema::ActOnComment(SourceRange Comment) {
     case RawComment::RCK_OrdinaryC:
       MagicMarkerText = "/**<";
       break;
+    case RawComment::RCK_Invalid:
+      // FIXME: are there other scenarios that could produce an invalid
+      // raw comment here?
+      Diag(Comment.getBegin(), diag::warn_splice_in_doxygen_comment);
+      return;
     default:
       llvm_unreachable("if this is an almost Doxygen comment, "
                        "it should be ordinary");

diff  --git a/clang/test/Lexer/comment-escape.c b/clang/test/Lexer/comment-escape.c
index 191e65441dd47..e9851caf2ce21 100644
--- a/clang/test/Lexer/comment-escape.c
+++ b/clang/test/Lexer/comment-escape.c
@@ -1,6 +1,38 @@
-// RUN: %clang -fsyntax-only %s 
+// RUN: %clang -fsyntax-only -Wdocumentation %s
 // rdar://6757323
 // foo \
 
 #define blork 32
 
+// GH62054
+
+/**<*\
+/
+//expected-warning at -2 {{escaped newline between}} \
+//expected-warning at -2 {{line splicing in Doxygen comments are not supported}}
+
+/**<*\	
+/
+//expected-warning at -2 {{escaped newline between}} \
+//expected-warning at -2 {{backslash and newline separated by space}} \
+//expected-warning at -2 {{line splicing in Doxygen comments are not supported}}
+
+
+/*<*\
+/
+//expected-warning at -2 {{escaped newline between}}  \
+//expected-warning at -2 {{line splicing in Doxygen comments are not supported}}
+
+/*<*\	
+/
+//expected-warning at -2 {{escaped newline between}} \
+//expected-warning at -2 {{backslash and newline separated by space}} \
+//expected-warning at -2 {{line splicing in Doxygen comments are not supported}}
+
+/\
+*<**/
+//expected-warning at -2 {{line splicing in Doxygen comments are not supported}}
+
+/\
+/<*
+//expected-warning at -2 {{line splicing in Doxygen comments are not supported}}