<p dir="ltr">Sorry for breaking stuff, will fix in about 3 hrs</p>
<div class="gmail_quote">On Nov 18, 2013 4:55 PM, "Alexander Kornienko" <<a href="mailto:alexfh@google.com">alexfh@google.com</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: alexfh<br>
Date: Mon Nov 18 09:50:04 2013<br>
New Revision: 195016<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=195016&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=195016&view=rev</a><br>
Log:<br>
Recover gracefully when deserializing invalid YAML input.<br>
Fixes <a href="http://llvm.org/PR16221" target="_blank">http://llvm.org/PR16221</a>, <a href="http://llvm.org/PR15927" target="_blank">http://llvm.org/PR15927</a><br>
Phabricator: <a href="http://llvm-reviews.chandlerc.com/D1236" target="_blank">http://llvm-reviews.chandlerc.com/D1236</a><br>
<br>
Patch by Andrew Tulloch!<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/Support/YAMLTraits.h<br>
llvm/trunk/lib/Support/YAMLTraits.cpp<br>
llvm/trunk/unittests/Support/YAMLIOTest.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/Support/YAMLTraits.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/YAMLTraits.h?rev=195016&r1=195015&r2=195016&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/YAMLTraits.h?rev=195016&r1=195015&r2=195016&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/Support/YAMLTraits.h (original)<br>
+++ llvm/trunk/include/llvm/Support/YAMLTraits.h Mon Nov 18 09:50:04 2013<br>
@@ -685,16 +685,18 @@ private:<br>
///<br>
class Input : public IO {<br>
public:<br>
- // Construct a yaml Input object from a StringRef and optional user-data.<br>
- Input(StringRef InputContent, void *Ctxt=NULL);<br>
+ // Construct a yaml Input object from a StringRef and optional<br>
+ // user-data. The DiagHandler can be specified to provide<br>
+ // alternative error reporting.<br>
+ Input(StringRef InputContent,<br>
+ void *Ctxt = NULL,<br>
+ SourceMgr::DiagHandlerTy DiagHandler = NULL,<br>
+ void *DiagHandlerCtxt = NULL);<br>
~Input();<br>
-<br>
+<br>
// Check if there was an syntax or semantic error during parsing.<br>
llvm::error_code error();<br>
<br>
- // To set alternate error reporting.<br>
- void setDiagHandler(llvm::SourceMgr::DiagHandlerTy Handler, void *Ctxt = 0);<br>
-<br>
static bool classof(const IO *io) { return !io->outputting(); }<br>
<br>
private:<br>
@@ -968,8 +970,8 @@ template <typename T><br>
inline<br>
typename llvm::enable_if_c<has_SequenceTraits<T>::value,Input &>::type<br>
operator>>(Input &yin, T &docSeq) {<br>
- yin.setCurrentDocument();<br>
- yamlize(yin, docSeq, true);<br>
+ if (yin.setCurrentDocument())<br>
+ yamlize(yin, docSeq, true);<br>
return yin;<br>
}<br>
<br>
<br>
Modified: llvm/trunk/lib/Support/YAMLTraits.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/YAMLTraits.cpp?rev=195016&r1=195015&r2=195016&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/YAMLTraits.cpp?rev=195016&r1=195015&r2=195016&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Support/YAMLTraits.cpp (original)<br>
+++ llvm/trunk/lib/Support/YAMLTraits.cpp Mon Nov 18 09:50:04 2013<br>
@@ -41,10 +41,15 @@ void IO::setContext(void *Context) {<br>
// Input<br>
//===----------------------------------------------------------------------===//<br>
<br>
-Input::Input(StringRef InputContent, void *Ctxt)<br>
+Input::Input(StringRef InputContent,<br>
+ void *Ctxt,<br>
+ SourceMgr::DiagHandlerTy DiagHandler,<br>
+ void *DiagHandlerCtxt)<br>
: IO(Ctxt),<br>
Strm(new Stream(InputContent, SrcMgr)),<br>
CurrentNode(NULL) {<br>
+ if (DiagHandler)<br>
+ SrcMgr.setDiagHandler(DiagHandler, DiagHandlerCtxt);<br>
DocIterator = Strm->begin();<br>
}<br>
<br>
@@ -55,10 +60,6 @@ error_code Input::error() {<br>
return EC;<br>
}<br>
<br>
-void Input::setDiagHandler(SourceMgr::DiagHandlerTy Handler, void *Ctxt) {<br>
- SrcMgr.setDiagHandler(Handler, Ctxt);<br>
-}<br>
-<br>
bool Input::outputting() const {<br>
return false;<br>
}<br>
@@ -66,6 +67,12 @@ bool Input::outputting() const {<br>
bool Input::setCurrentDocument() {<br>
if (DocIterator != Strm->end()) {<br>
Node *N = DocIterator->getRoot();<br>
+ if (!N) {<br>
+ assert(Strm->failed() && "Root is NULL iff parsing failed");<br>
+ EC = make_error_code(errc::invalid_argument);<br>
+ return false;<br>
+ }<br>
+<br>
if (isa<NullNode>(N)) {<br>
// Empty files are allowed and ignored<br>
++DocIterator;<br>
@@ -95,7 +102,8 @@ bool Input::mapTag(StringRef Tag, bool D<br>
void Input::beginMapping() {<br>
if (EC)<br>
return;<br>
- MapHNode *MN = dyn_cast<MapHNode>(CurrentNode);<br>
+ // CurrentNode can be null if the document is empty.<br>
+ MapHNode *MN = dyn_cast_or_null<MapHNode>(CurrentNode);<br>
if (MN) {<br>
MN->ValidKeys.clear();<br>
}<br>
@@ -106,6 +114,15 @@ bool Input::preflightKey(const char *Key<br>
UseDefault = false;<br>
if (EC)<br>
return false;<br>
+<br>
+ // CurrentNode is null for empty documents, which is an error in case required<br>
+ // nodes are present.<br>
+ if (!CurrentNode) {<br>
+ if (Required)<br>
+ EC = make_error_code(errc::invalid_argument);<br>
+ return false;<br>
+ }<br>
+<br>
MapHNode *MN = dyn_cast<MapHNode>(CurrentNode);<br>
if (!MN) {<br>
setError(CurrentNode, "not a mapping");<br>
@@ -132,7 +149,8 @@ void Input::postflightKey(void *saveInfo<br>
void Input::endMapping() {<br>
if (EC)<br>
return;<br>
- MapHNode *MN = dyn_cast<MapHNode>(CurrentNode);<br>
+ // CurrentNode can be null if the document is empty.<br>
+ MapHNode *MN = dyn_cast_or_null<MapHNode>(CurrentNode);<br>
if (!MN)<br>
return;<br>
for (MapHNode::NameToNode::iterator i = MN->Mapping.begin(),<br>
@@ -273,6 +291,7 @@ void Input::scalarString(StringRef &S) {<br>
}<br>
<br>
void Input::setError(HNode *hnode, const Twine &message) {<br>
+ assert(hnode && "HNode must not be NULL");<br>
this->setError(hnode->_node, message);<br>
}<br>
<br>
<br>
Modified: llvm/trunk/unittests/Support/YAMLIOTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/YAMLIOTest.cpp?rev=195016&r1=195015&r2=195016&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/YAMLIOTest.cpp?rev=195016&r1=195015&r2=195016&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/Support/YAMLIOTest.cpp (original)<br>
+++ llvm/trunk/unittests/Support/YAMLIOTest.cpp Mon Nov 18 09:50:04 2013<br>
@@ -58,14 +58,24 @@ namespace yaml {<br>
//<br>
TEST(YAMLIO, TestMapRead) {<br>
FooBar doc;<br>
- Input yin("---\nfoo: 3\nbar: 5\n...\n");<br>
- yin >> doc;<br>
+ {<br>
+ Input yin("---\nfoo: 3\nbar: 5\n...\n");<br>
+ yin >> doc;<br>
<br>
- EXPECT_FALSE(yin.error());<br>
- EXPECT_EQ(doc.foo, 3);<br>
- EXPECT_EQ(doc.bar,5);<br>
-}<br>
+ EXPECT_FALSE(yin.error());<br>
+ EXPECT_EQ(doc.foo, 3);<br>
+ EXPECT_EQ(doc.bar, 5);<br>
+ }<br>
<br>
+ {<br>
+ Input yin("{foo: 3, bar: 5}");<br>
+ yin >> doc;<br>
+<br>
+ EXPECT_FALSE(yin.error());<br>
+ EXPECT_EQ(doc.foo, 3);<br>
+ EXPECT_EQ(doc.bar, 5);<br>
+ }<br>
+}<br>
<br>
//<br>
// Test the reading of a yaml sequence of mappings<br>
@@ -1164,8 +1174,9 @@ TEST(YAMLIO, TestColorsReadError) {<br>
"c1: blue\n"<br>
"c2: purple\n"<br>
"c3: green\n"<br>
- "...\n");<br>
- yin.setDiagHandler(suppressErrorMessages);<br>
+ "...\n",<br>
+ /*Ctxt=*/NULL,<br>
+ suppressErrorMessages);<br>
yin >> map;<br>
EXPECT_TRUE(yin.error());<br>
}<br>
@@ -1180,8 +1191,9 @@ TEST(YAMLIO, TestFlagsReadError) {<br>
"f1: [ big ]\n"<br>
"f2: [ round, hollow ]\n"<br>
"f3: []\n"<br>
- "...\n");<br>
- yin.setDiagHandler(suppressErrorMessages);<br>
+ "...\n",<br>
+ /*Ctxt=*/NULL,<br>
+ suppressErrorMessages);<br>
yin >> map;<br>
<br>
EXPECT_TRUE(yin.error());<br>
@@ -1198,8 +1210,9 @@ TEST(YAMLIO, TestReadBuiltInTypesUint8Er<br>
"- 255\n"<br>
"- 0\n"<br>
"- 257\n"<br>
- "...\n");<br>
- yin.setDiagHandler(suppressErrorMessages);<br>
+ "...\n",<br>
+ /*Ctxt=*/NULL,<br>
+ suppressErrorMessages);<br>
yin >> seq;<br>
<br>
EXPECT_TRUE(yin.error());<br>
@@ -1216,8 +1229,9 @@ TEST(YAMLIO, TestReadBuiltInTypesUint16E<br>
"- 65535\n"<br>
"- 0\n"<br>
"- 66000\n"<br>
- "...\n");<br>
- yin.setDiagHandler(suppressErrorMessages);<br>
+ "...\n",<br>
+ /*Ctxt=*/NULL,<br>
+ suppressErrorMessages);<br>
yin >> seq;<br>
<br>
EXPECT_TRUE(yin.error());<br>
@@ -1234,8 +1248,9 @@ TEST(YAMLIO, TestReadBuiltInTypesUint32E<br>
"- 4000000000\n"<br>
"- 0\n"<br>
"- 5000000000\n"<br>
- "...\n");<br>
- yin.setDiagHandler(suppressErrorMessages);<br>
+ "...\n",<br>
+ /*Ctxt=*/NULL,<br>
+ suppressErrorMessages);<br>
yin >> seq;<br>
<br>
EXPECT_TRUE(yin.error());<br>
@@ -1252,8 +1267,9 @@ TEST(YAMLIO, TestReadBuiltInTypesUint64E<br>
"- 18446744073709551615\n"<br>
"- 0\n"<br>
"- 19446744073709551615\n"<br>
- "...\n");<br>
- yin.setDiagHandler(suppressErrorMessages);<br>
+ "...\n",<br>
+ /*Ctxt=*/NULL,<br>
+ suppressErrorMessages);<br>
yin >> seq;<br>
<br>
EXPECT_TRUE(yin.error());<br>
@@ -1271,8 +1287,9 @@ TEST(YAMLIO, TestReadBuiltInTypesint8Ove<br>
"- 0\n"<br>
"- 127\n"<br>
"- 128\n"<br>
- "...\n");<br>
- yin.setDiagHandler(suppressErrorMessages);<br>
+ "...\n",<br>
+ /*Ctxt=*/NULL,<br>
+ suppressErrorMessages);<br>
yin >> seq;<br>
<br>
EXPECT_TRUE(yin.error());<br>
@@ -1288,8 +1305,9 @@ TEST(YAMLIO, TestReadBuiltInTypesint8Und<br>
"- 0\n"<br>
"- 127\n"<br>
"- -129\n"<br>
- "...\n");<br>
- yin.setDiagHandler(suppressErrorMessages);<br>
+ "...\n",<br>
+ /*Ctxt=*/NULL,<br>
+ suppressErrorMessages);<br>
yin >> seq;<br>
<br>
EXPECT_TRUE(yin.error());<br>
@@ -1307,8 +1325,9 @@ TEST(YAMLIO, TestReadBuiltInTypesint16Un<br>
"- 0\n"<br>
"- -32768\n"<br>
"- -32769\n"<br>
- "...\n");<br>
- yin.setDiagHandler(suppressErrorMessages);<br>
+ "...\n",<br>
+ /*Ctxt=*/NULL,<br>
+ suppressErrorMessages);<br>
yin >> seq;<br>
<br>
EXPECT_TRUE(yin.error());<br>
@@ -1325,8 +1344,9 @@ TEST(YAMLIO, TestReadBuiltInTypesint16Ov<br>
"- 0\n"<br>
"- -32768\n"<br>
"- 32768\n"<br>
- "...\n");<br>
- yin.setDiagHandler(suppressErrorMessages);<br>
+ "...\n",<br>
+ /*Ctxt=*/NULL,<br>
+ suppressErrorMessages);<br>
yin >> seq;<br>
<br>
EXPECT_TRUE(yin.error());<br>
@@ -1344,8 +1364,9 @@ TEST(YAMLIO, TestReadBuiltInTypesint32Un<br>
"- 0\n"<br>
"- -2147483648\n"<br>
"- -2147483649\n"<br>
- "...\n");<br>
- yin.setDiagHandler(suppressErrorMessages);<br>
+ "...\n",<br>
+ /*Ctxt=*/NULL,<br>
+ suppressErrorMessages);<br>
yin >> seq;<br>
<br>
EXPECT_TRUE(yin.error());<br>
@@ -1361,8 +1382,9 @@ TEST(YAMLIO, TestReadBuiltInTypesint32Ov<br>
"- 0\n"<br>
"- -2147483648\n"<br>
"- 2147483649\n"<br>
- "...\n");<br>
- yin.setDiagHandler(suppressErrorMessages);<br>
+ "...\n",<br>
+ /*Ctxt=*/NULL,<br>
+ suppressErrorMessages);<br>
yin >> seq;<br>
<br>
EXPECT_TRUE(yin.error());<br>
@@ -1380,8 +1402,9 @@ TEST(YAMLIO, TestReadBuiltInTypesint64Un<br>
"- 0\n"<br>
"- 9223372036854775807\n"<br>
"- -9223372036854775809\n"<br>
- "...\n");<br>
- yin.setDiagHandler(suppressErrorMessages);<br>
+ "...\n",<br>
+ /*Ctxt=*/NULL,<br>
+ suppressErrorMessages);<br>
yin >> seq;<br>
<br>
EXPECT_TRUE(yin.error());<br>
@@ -1397,8 +1420,9 @@ TEST(YAMLIO, TestReadBuiltInTypesint64Ov<br>
"- 0\n"<br>
"- 9223372036854775807\n"<br>
"- 9223372036854775809\n"<br>
- "...\n");<br>
- yin.setDiagHandler(suppressErrorMessages);<br>
+ "...\n",<br>
+ /*Ctxt=*/NULL,<br>
+ suppressErrorMessages);<br>
yin >> seq;<br>
<br>
EXPECT_TRUE(yin.error());<br>
@@ -1415,8 +1439,9 @@ TEST(YAMLIO, TestReadBuiltInTypesFloatEr<br>
"- 1000.1\n"<br>
"- -123.456\n"<br>
"- 1.2.3\n"<br>
- "...\n");<br>
- yin.setDiagHandler(suppressErrorMessages);<br>
+ "...\n",<br>
+ /*Ctxt=*/NULL,<br>
+ suppressErrorMessages);<br>
yin >> seq;<br>
<br>
EXPECT_TRUE(yin.error());<br>
@@ -1433,8 +1458,9 @@ TEST(YAMLIO, TestReadBuiltInTypesDoubleE<br>
"- 1000.1\n"<br>
"- -123.456\n"<br>
"- 1.2.3\n"<br>
- "...\n");<br>
- yin.setDiagHandler(suppressErrorMessages);<br>
+ "...\n",<br>
+ /*Ctxt=*/NULL,<br>
+ suppressErrorMessages);<br>
yin >> seq;<br>
<br>
EXPECT_TRUE(yin.error());<br>
@@ -1450,8 +1476,9 @@ TEST(YAMLIO, TestReadBuiltInTypesHex8Err<br>
"- 0x12\n"<br>
"- 0xFE\n"<br>
"- 0x123\n"<br>
- "...\n");<br>
- yin.setDiagHandler(suppressErrorMessages);<br>
+ "...\n",<br>
+ /*Ctxt=*/NULL,<br>
+ suppressErrorMessages);<br>
yin >> seq;<br>
<br>
EXPECT_TRUE(yin.error());<br>
@@ -1468,8 +1495,9 @@ TEST(YAMLIO, TestReadBuiltInTypesHex16Er<br>
"- 0x0012\n"<br>
"- 0xFEFF\n"<br>
"- 0x12345\n"<br>
- "...\n");<br>
- yin.setDiagHandler(suppressErrorMessages);<br>
+ "...\n",<br>
+ /*Ctxt=*/NULL,<br>
+ suppressErrorMessages);<br>
yin >> seq;<br>
<br>
EXPECT_TRUE(yin.error());<br>
@@ -1485,8 +1513,9 @@ TEST(YAMLIO, TestReadBuiltInTypesHex32Er<br>
"- 0x0012\n"<br>
"- 0xFEFF0000\n"<br>
"- 0x1234556789\n"<br>
- "...\n");<br>
- yin.setDiagHandler(suppressErrorMessages);<br>
+ "...\n",<br>
+ /*Ctxt=*/NULL,<br>
+ suppressErrorMessages);<br>
yin >> seq;<br>
<br>
EXPECT_TRUE(yin.error());<br>
@@ -1502,13 +1531,31 @@ TEST(YAMLIO, TestReadBuiltInTypesHex64Er<br>
"- 0x0012\n"<br>
"- 0xFFEEDDCCBBAA9988\n"<br>
"- 0x12345567890ABCDEF0\n"<br>
- "...\n");<br>
- yin.setDiagHandler(suppressErrorMessages);<br>
+ "...\n",<br>
+ /*Ctxt=*/NULL,<br>
+ suppressErrorMessages);<br>
yin >> seq;<br>
<br>
EXPECT_TRUE(yin.error());<br>
}<br>
<br>
+TEST(YAMLIO, TestMalformedMapFailsGracefully) {<br>
+ FooBar doc;<br>
+ {<br>
+ // We pass the suppressErrorMessages handler to handle the error<br>
+ // message generated in the constructor of Input.<br>
+ Input yin("{foo:3, bar: 5}", /*Ctxt=*/NULL, suppressErrorMessages);<br>
+ yin >> doc;<br>
+ EXPECT_TRUE(yin.error());<br>
+ }<br>
+<br>
+ {<br>
+ Input yin("---\nfoo:3\nbar: 5\n...\n", /*Ctxt=*/NULL, suppressErrorMessages);<br>
+ yin >> doc;<br>
+ EXPECT_TRUE(yin.error());<br>
+ }<br>
+}<br>
+<br>
struct OptionalTest {<br>
std::vector<int> Numbers;<br>
};<br>
@@ -1572,3 +1619,26 @@ TEST(YAMLIO, SequenceElideTest) {<br>
<br>
EXPECT_TRUE(Seq2.Tests[3].Numbers.empty());<br>
}<br>
+<br>
+TEST(YAMLIO, TestEmptyStringFailsForMapWithRequiredFields) {<br>
+ FooBar doc;<br>
+ Input yin("");<br>
+ yin >> doc;<br>
+ EXPECT_TRUE(yin.error());<br>
+}<br>
+<br>
+TEST(YAMLIO, TestEmptyStringSucceedsForMapWithOptionalFields) {<br>
+ OptionalTest doc;<br>
+ Input yin("");<br>
+ yin >> doc;<br>
+ EXPECT_FALSE(yin.error());<br>
+}<br>
+<br>
+TEST(YAMLIO, TestEmptyStringSucceedsForSequence) {<br>
+ std::vector<uint8_t> seq;<br>
+ Input yin("", /*Ctxt=*/NULL, suppressErrorMessages);<br>
+ yin >> seq;<br>
+<br>
+ EXPECT_FALSE(yin.error());<br>
+ EXPECT_TRUE(seq.empty());<br>
+}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>