This is an archive of the discontinued LLVM Phabricator instance.

[obj2yaml] Support ELF input format in the obj2yaml tool
ClosedPublic

Authored by atanasyan on May 6 2014, 11:04 PM.

Details

Reviewers
silvas
Bigcheese
Summary

This patch implements support of ELF input format in the obj2yaml tool.

Please note that now the ELF header e_flags field in the MIPS related test cases handled incorrectly. The obj2yaml prints too many flags. I will fix that in the next patches.

Diff Detail

Event Timeline

atanasyan updated this revision to Diff 9148.May 6 2014, 11:04 PM
atanasyan retitled this revision from to [obj2yaml] Support ELF input format in the obj2yaml tool.
atanasyan updated this object.
atanasyan edited the test plan for this revision. (Show Details)
atanasyan added reviewers: silvas, Bigcheese.
atanasyan added a subscriber: Unknown Object (MLST).
Bigcheese accepted this revision.May 7 2014, 3:53 PM
Bigcheese edited edge metadata.

With that change this looks good to me.

test/Object/obj2yaml.test
203

Content needs to be quoted. 0 is the octal prefix in YAML prior to 1.2.

This revision is now accepted and ready to land.May 7 2014, 3:53 PM
atanasyan added inline comments.May 8 2014, 4:27 AM
test/Object/obj2yaml.test
203

Now the llvm::yaml::Output class prints quotes for strings which are valid numbers. The string 0000023C... is not a valid octal number. Do you suggest to implement a general solution and always print quotes for all string which starts from a digit even if the rest of the string is not a valid number? This greatly simplify the isNumber function from the YAMLTraits.h, but I am still note sure that we need so general modification.

silvas added inline comments.May 12 2014, 11:00 AM
test/Object/obj2yaml.test
203

I wouldn't be opposed to being more conservative when quoting in general. I think that the extra quotes would probably be a net win for the human reader, which has a much harder time knowing if it is an octal number.

That can be a separate patch though. The current patch LGTM.

silvas accepted this revision.May 12 2014, 11:01 AM
silvas edited edge metadata.

(actually marking the revision as "accept").

In general I do not have any objections on Michael's suggestion. Just do not want to mix all these changes in the single patch.

atanasyan closed this revision.May 13 2014, 10:16 PM

Thanks for review.

Closed by commit rL208752.