This is an archive of the discontinued LLVM Phabricator instance.

[yaml2obj] - Don't crash on invalid document.
AbandonedPublic

Authored by grimar on Sep 19 2017, 6:56 AM.

Details

Summary

Previously jaml2obj would segfault on empty document.
(without yaml description).
Patch fixes the issue.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar created this revision.Sep 19 2017, 6:56 AM
davide accepted this revision.Sep 19 2017, 9:01 AM
davide added a subscriber: davide.

LGTM.

This revision is now accepted and ready to land.Sep 19 2017, 9:01 AM
This revision was automatically updated to reflect the committed changes.
grimar reopened this revision.EditedSep 20 2017, 5:39 AM

It was reverted because broke BB:
http://lab.llvm.org:8011/builders/llvm-hexagon-elf/builds/9781
Reopening.

This revision is now accepted and ready to land.Sep 20 2017, 5:39 AM
grimar planned changes to this revision.Sep 20 2017, 5:39 AM
grimar updated this revision to Diff 115997.Sep 20 2017, 7:33 AM
  • Changed approach to stop breaking tests from Support/YAMLIOTest.cpp.
This revision is now accepted and ready to land.Sep 20 2017, 7:33 AM
grimar requested review of this revision.Sep 20 2017, 7:33 AM
grimar edited edge metadata.
ruiu added inline comments.Sep 21 2017, 1:39 PM
lib/ObjectYAML/ObjectYAML.cpp
38 ↗(On Diff #115997)

I think this is not correct. You need to set an error because this is an error condition.

grimar updated this revision to Diff 116329.Sep 22 2017, 4:26 AM
  • Addressed review comment.
lib/ObjectYAML/ObjectYAML.cpp
38 ↗(On Diff #115997)

I was not sure that empty document is an error. But ok, fixed.
In that case "Unknown document type!" message in following method looks excessive:
https://github.com/llvm-mirror/llvm/blob/master/tools/yaml2obj/yaml2obj.cpp#L48

grimar abandoned this revision.Dec 1 2017, 4:15 AM

Not sure it was the best approach possible. Abandoning.