This is an archive of the discontinued LLVM Phabricator instance.

[llvm-readobj] - Simplify the code that creates dumpers. NFCI.
ClosedPublic

Authored by grimar on Aug 27 2020, 8:30 AM.

Details

Summary

We have a few helper functions like the following:

std::error_code create*Dumper(...)

In fact we do not need or want to use std::error_code and the code
can be simpler if we just return std::unique_ptr<ObjDumper>.

This patch does this change and refines the signature of createDumper
as well.

Diff Detail

Event Timeline

grimar created this revision.Aug 27 2020, 8:30 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 27 2020, 8:30 AM
grimar requested review of this revision.Aug 27 2020, 8:30 AM
MaskRay added a comment.EditedAug 27 2020, 12:24 PM

The observation of this patch is that createDumper does not fail.

The construction of an ObjectFile just inspects the leading magic bytes (isELF isMachO ...). createDumper dispatches on magic bytes to different binary formats. Indeed currently the various create*Dumper do not fail, but is it possible that we will enforce stricter verification in the future? If so, the new interface with the unique_ptr return type does not seem very suitable.


The dumpObject interface change is fine: using a reference to emphasize that the pointer argument cannot be null.

grimar added a comment.EditedAug 27 2020, 2:13 PM

The observation of this patch is that createDumper does not fail.

The construction of an ObjectFile just inspects the leading magic bytes (isELF isMachO ...). createDumper dispatches on magic bytes to different binary formats. Indeed currently the various create*Dumper do not fail, but is it possible that we will enforce stricter verification in the future?

I think it should never fail. The idea of llvm-readobj/elf is all about dumping of all kind of objects, e.g. they must be able to dump broken objects.
I believe we always want to be able at least to create dumpers.

If one day situation changes, we will be able to change the API, but currently it is redundant to return an error.

MaskRay accepted this revision.Aug 27 2020, 2:18 PM

Because currently we call many print* after the dumper is created. The many print* may fail due to the same reason. If we one day we want to change, we can call a verification method before print*.

This revision is now accepted and ready to land.Aug 27 2020, 2:18 PM