Fix bug 20125 - clang-format segfaults on bad config

[lifted]

Summary

http://llvm.org/bugs/show_bug.cgi?id=20125

The problem was in unchecked dyn_cast inside of Input::createHNodes.

E.g. the following YAML code

{ColumnLimit: 80; UseTab: Never}

is parsed as

{ {'ColumnLimit': '80; UseTab'} : 'Never' }

But the parser expected that mapping key is always a scalar. Unchecked cast of MappingNode to ScalarNode caused segfault later.

The cast is now checked, the example below produces the following error:

YAML:1:2: error: Map key must be a scalar
{ColumnLimit: 80; UseTab: Never}
 ^

[hiraditya]

It'll be great if you could add a test case.

[lifted]

Added a unit test. The unit test crashes without a corresponding fix.

[• rafael]

lgtm

[lifted]

Could you please commit the patch?

[• rafael]

215205

lib/Support/YAMLTraits.cpp

for (Node &SN : *SQ) {
       if (EC)
         break;
       SQHNode->Entries.push_back(Entry);
     }
     return SQHNode;
   } else if (MappingNode *Map = dyn_cast<MappingNode>(N)) {
     MapHNode *mapHNode = new MapHNode(N);
     for (KeyValueNode &KVN : *Map) {
-      ScalarNode *KeyScalar = dyn_cast<ScalarNode>(KVN.getKey());
+      Node *KeyNode = KVN.getKey();
+      ScalarNode *KeyScalar = dyn_cast<ScalarNode>(KeyNode);
+      if (!KeyScalar) {
+        setError(KeyNode, "Map key must be a scalar");
+        break;
+      }
       StringStorage.clear();
       StringRef KeyStr = KeyScalar->getValue(StringStorage);
       if (!StringStorage.empty()) {
         // Copy string to permanent storage
         unsigned Len = StringStorage.size();
         char *Buf = StringAllocator.Allocate<char>(Len);
         memcpy(Buf, &StringStorage[0], Len);
         KeyStr = StringRef(Buf, Len);

unittests/Support/YAMLIOTest.cpp

FooBar doc;
     yin >> doc;
 
     EXPECT_FALSE(yin.error());
     EXPECT_EQ(doc.foo, 3);
     EXPECT_EQ(doc.bar, 5);
   }
 }
 
+TEST(YAMLIO, TestMalformedMapRead) {
+  FooBar doc;
+  Input yin("{foo: 3; bar: 5}", nullptr, suppressErrorMessages);
+  yin >> doc;
+  EXPECT_TRUE(!!yin.error());
+}
+
 //
 // Test the reading of a yaml sequence of mappings
 //
 TEST(YAMLIO, TestSequenceMapRead) {
   FooBarSequence seq;
   Input yin("---\n - foo:  3\n   bar:  5\n - foo:  7\n   bar:  9\n...\n");
   yin >> seq;