[PATCH] D56442: [clangd] Fix a crash when reading an empty index file.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric.

Unfortunately, yaml::Input::setCurrentDocument() and 
yaml::Input::nextDocument() are
internal APIs, the way we use them may cause a nullptr accessing when
processing an empty YAML file.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D56442

Files:
  clangd/index/YAMLSerialization.cpp
  unittests/clangd/SerializationTests.cpp


Index: unittests/clangd/SerializationTests.cpp
===================================================================
--- unittests/clangd/SerializationTests.cpp
+++ unittests/clangd/SerializationTests.cpp
@@ -91,6 +91,10 @@
   return (arg.IncludeHeader == IncludeHeader) && (arg.References == 
References);
 }
 
+TEST(SerializationTest, NoCrashOnEmptyYAML) {
+  EXPECT_TRUE(bool(readIndexFile("")));
+}
+
 TEST(SerializationTest, YAMLConversions) {
   auto In = readIndexFile(YAML);
   EXPECT_TRUE(bool(In)) << In.takeError();
Index: clangd/index/YAMLSerialization.cpp
===================================================================
--- clangd/index/YAMLSerialization.cpp
+++ clangd/index/YAMLSerialization.cpp
@@ -314,17 +314,20 @@
       Arena; // store the underlying data of Position::FileURI.
   llvm::UniqueStringSaver Strings(Arena);
   llvm::yaml::Input Yin(Data, &Strings);
-  do {
+  while (Yin.setCurrentDocument()) {
+    llvm::yaml::EmptyContext Ctx;
     VariantEntry Variant;
-    Yin >> Variant;
+    yamlize(Yin, Variant, true, Ctx);
     if (Yin.error())
       return llvm::errorCodeToError(Yin.error());
+
     if (Variant.Symbol)
       Symbols.insert(*Variant.Symbol);
     if (Variant.Refs)
       for (const auto &Ref : Variant.Refs->second)
         Refs.insert(Variant.Refs->first, Ref);
-  } while (Yin.nextDocument());
+    Yin.nextDocument();
+  }
 
   IndexFileIn Result;
   Result.Symbols.emplace(std::move(Symbols).build());



Index: unittests/clangd/SerializationTests.cpp
===================================================================
--- unittests/clangd/SerializationTests.cpp
+++ unittests/clangd/SerializationTests.cpp
@@ -91,6 +91,10 @@
   return (arg.IncludeHeader == IncludeHeader) && (arg.References == References);
 }
 
+TEST(SerializationTest, NoCrashOnEmptyYAML) {
+  EXPECT_TRUE(bool(readIndexFile("")));
+}
+
 TEST(SerializationTest, YAMLConversions) {
   auto In = readIndexFile(YAML);
   EXPECT_TRUE(bool(In)) << In.takeError();
Index: clangd/index/YAMLSerialization.cpp
===================================================================
--- clangd/index/YAMLSerialization.cpp
+++ clangd/index/YAMLSerialization.cpp
@@ -314,17 +314,20 @@
       Arena; // store the underlying data of Position::FileURI.
   llvm::UniqueStringSaver Strings(Arena);
   llvm::yaml::Input Yin(Data, &Strings);
-  do {
+  while (Yin.setCurrentDocument()) {
+    llvm::yaml::EmptyContext Ctx;
     VariantEntry Variant;
-    Yin >> Variant;
+    yamlize(Yin, Variant, true, Ctx);
     if (Yin.error())
       return llvm::errorCodeToError(Yin.error());
+
     if (Variant.Symbol)
       Symbols.insert(*Variant.Symbol);
     if (Variant.Refs)
       for (const auto &Ref : Variant.Refs->second)
         Refs.insert(Variant.Refs->first, Ref);
-  } while (Yin.nextDocument());
+    Yin.nextDocument();
+  }
 
   IndexFileIn Result;
   Result.Symbols.emplace(std::move(Symbols).build());

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