[PATCH] D94919: [clangd] Fix a crash when indexing invalid ObjC method declaration

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz updated this revision to Diff 318529.
adamcz marked an inline comment as done.
adamcz added a comment.

addressed review comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94919/new/

https://reviews.llvm.org/D94919

Files:
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
  clang/lib/Sema/SemaCodeComplete.cpp


Index: clang/lib/Sema/SemaCodeComplete.cpp
===================================================================
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -3506,9 +3506,11 @@
         Result.AddTypedTextChunk("");
     }
     unsigned Idx = 0;
+    // The extra Idx < Sel.getNumArgs() check is needed due to legacy C-style
+    // method parameters.
     for (ObjCMethodDecl::param_const_iterator P = Method->param_begin(),
                                               PEnd = Method->param_end();
-         P != PEnd; (void)++P, ++Idx) {
+         P != PEnd && Idx < Sel.getNumArgs(); (void)++P, ++Idx) {
       if (Idx > 0) {
         std::string Keyword;
         if (Idx > StartParameter)
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1801,6 +1801,20 @@
   EXPECT_THAT(TU.headerSymbols(), Not(Contains(QName("X"))));
 }
 
+TEST_F(SymbolCollectorTest, NoCrashOnObjCMethodCStyleParam) {
+  auto TU = TestTU::withCode(R"objc(
+    @interface Foo
+    - (void)fun:(bool)foo, bool bar;
+    @end
+  )objc");
+  TU.ExtraArgs.push_back("-xobjective-c++");
+
+  TU.build();
+  // We mostly care about not crashing.
+  EXPECT_THAT(TU.headerSymbols(),
+              UnorderedElementsAre(QName("Foo"), QName("Foo::fun:")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang



Index: clang/lib/Sema/SemaCodeComplete.cpp
===================================================================
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -3506,9 +3506,11 @@
         Result.AddTypedTextChunk("");
     }
     unsigned Idx = 0;
+    // The extra Idx < Sel.getNumArgs() check is needed due to legacy C-style
+    // method parameters.
     for (ObjCMethodDecl::param_const_iterator P = Method->param_begin(),
                                               PEnd = Method->param_end();
-         P != PEnd; (void)++P, ++Idx) {
+         P != PEnd && Idx < Sel.getNumArgs(); (void)++P, ++Idx) {
       if (Idx > 0) {
         std::string Keyword;
         if (Idx > StartParameter)
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1801,6 +1801,20 @@
   EXPECT_THAT(TU.headerSymbols(), Not(Contains(QName("X"))));
 }
 
+TEST_F(SymbolCollectorTest, NoCrashOnObjCMethodCStyleParam) {
+  auto TU = TestTU::withCode(R"objc(
+    @interface Foo
+    - (void)fun:(bool)foo, bool bar;
+    @end
+  )objc");
+  TU.ExtraArgs.push_back("-xobjective-c++");
+
+  TU.build();
+  // We mostly care about not crashing.
+  EXPECT_THAT(TU.headerSymbols(),
+              UnorderedElementsAre(QName("Foo"), QName("Foo::fun:")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang

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