[PATCH] D131675: [clang] SIGSEGV fix at clang::ASTContext::getRawCommentForDeclNoCacheImpl

[Ivan Murashko via Phabricator via cfe-commits]

ivanmurashko created this revision.
ivanmurashko added reviewers: sammccall, aaron.ballman.
Herald added subscribers: usaxena95, kadircet.
Herald added a project: All.
ivanmurashko requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added a project: clang.

The `File` might point to an invalid `FileID` at the case of broken AST. That 
leads to clang/clangd crash while processing comments. Relevant part of the 
crash is below

   #4 0x00007f1d7fbf95bc std::_Rb_tree<unsigned int, std::pair<unsigned int 
const, clang::RawComment*>, std::_Select1st<std::pair<unsigned int const, 
clang::RawComment*>>, std::less<unsigned int>, 
std::allocator<std::pair<unsigned int const
  , clang::RawComment*>>>::_M_lower_bound(std::_Rb_tree_node<std::pair<unsigned 
int const, clang::RawComment*>> const*, std::_Rb_tree_node_base const*, 
unsigned int const&) const /usr/include/c++/8/bits/stl_tree.h:1911:2
   #5 0x00007f1d7fbf95bc std::_Rb_tree<unsigned int, std::pair<unsigned int 
const, clang::RawComment*>, std::_Select1st<std::pair<unsigned int const, 
clang::RawComment*>>, std::less<unsigned int>, 
std::allocator<std::pair<unsigned int const
  , clang::RawComment*>>>::lower_bound(unsigned int const&) const 
/usr/include/c++/8/bits/stl_tree.h:1214:56
   #6 0x00007f1d7fbf95bc std::map<unsigned int, clang::RawComment*, 
std::less<unsigned int>, std::allocator<std::pair<unsigned int const, 
clang::RawComment*>>>::lower_bound(unsigned int const&) const 
/usr/include/c++/8/bits/stl_map.h:1264:3
  6
   #7 0x00007f1d7fbf95bc 
clang::ASTContext::getRawCommentForDeclNoCacheImpl(clang::Decl const*, 
clang::SourceLocation, std::map<unsigned int, clang::RawComment*, 
std::less<unsigned int>, std::allocator<std::pair<unsigned int const, clang::R
  awComment*>>> const&) const 
/home/ivanmurashko/local/llvm-project/clang/lib/AST/ASTContext.cpp:226:57

The corresponding lit test that reproduces that crash was also added


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131675

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/AST/ast-crash-doc.cpp


Index: clang/test/AST/ast-crash-doc.cpp
===================================================================
--- /dev/null
+++ clang/test/AST/ast-crash-doc.cpp
@@ -0,0 +1,30 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -emit-module -x c++ -fmodules -I %t/Inputs -fmodule-name=aa 
%t/Inputs/module.modulemap -o %t/aa.pcm
+// RUN: rm %t/Inputs/b.h
+// RUN: not %clang_cc1 -x c++ -Wdocumentation -ast-dump-all -fmodules -I 
%t/Inputs -fmodule-file=%t/aa.pcm %t/test.cpp | FileCheck %s
+
+//--- Inputs/module.modulemap
+module aa {
+    header "a.h"
+    header "b.h"
+}
+
+//--- Inputs/a.h
+// empty file
+
+//--- Inputs/b.h
+/// test foo @return
+int foo();
+
+
+//--- test.cpp
+#include "a.h"
+
+/// test comment at the primary file
+
+int a = foo();
+
+
+// CHECK: TranslationUnitDecl
Index: clang/lib/AST/ASTContext.cpp
===================================================================
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -298,6 +298,9 @@
     return nullptr;
 
   const FileID File = SourceMgr.getDecomposedLoc(DeclLoc).first;
+  if (!File.isValid()) {
+    return nullptr;
+  }
   const auto CommentsInThisFile = Comments.getCommentsInFile(File);
   if (!CommentsInThisFile || CommentsInThisFile->empty())
     return nullptr;



Index: clang/test/AST/ast-crash-doc.cpp
===================================================================
--- /dev/null
+++ clang/test/AST/ast-crash-doc.cpp
@@ -0,0 +1,30 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -emit-module -x c++ -fmodules -I %t/Inputs -fmodule-name=aa %t/Inputs/module.modulemap -o %t/aa.pcm
+// RUN: rm %t/Inputs/b.h
+// RUN: not %clang_cc1 -x c++ -Wdocumentation -ast-dump-all -fmodules -I %t/Inputs -fmodule-file=%t/aa.pcm %t/test.cpp | FileCheck %s
+
+//--- Inputs/module.modulemap
+module aa {
+    header "a.h"
+    header "b.h"
+}
+
+//--- Inputs/a.h
+// empty file
+
+//--- Inputs/b.h
+/// test foo @return
+int foo();
+
+
+//--- test.cpp
+#include "a.h"
+
+/// test comment at the primary file
+
+int a = foo();
+
+
+// CHECK: TranslationUnitDecl
Index: clang/lib/AST/ASTContext.cpp
===================================================================
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -298,6 +298,9 @@
     return nullptr;
 
   const FileID File = SourceMgr.getDecomposedLoc(DeclLoc).first;
+  if (!File.isValid()) {
+    return nullptr;
+  }
   const auto CommentsInThisFile = Comments.getCommentsInFile(File);
   if (!CommentsInThisFile || CommentsInThisFile->empty())
     return nullptr;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits