Page MenuHomePhabricator

[llvm-readobj] - Remove deprecated unwrapOrError(Expected<T> EO).

Authored by grimar on Aug 8 2019, 6:07 AM.



This patch changes the code to use a modern unwrapOrError(StringRef Input, Expected<T> EO)
version that contains the input source name and removes the depricated version.

Diff Detail


Event Timeline

grimar created this revision.Aug 8 2019, 6:07 AM
jhenderson added inline comments.Aug 8 2019, 7:05 AM
229 ↗(On Diff #214129)

Aside: it's rather weird that invalid-e_shnum is being used to test an invalid e_phentsize...!

247 ↗(On Diff #214129)

I think you can use -D here, right, like in %t10 case above and others?

6 ↗(On Diff #214129)

Nit: use single-dash for -D.

74 ↗(On Diff #214129)


MaskRay added inline comments.Aug 8 2019, 7:21 PM
429 ↗(On Diff #214129)

Why can't this check use [[FILE]]?

And a typo in the title: depricated -> deprecated

grimar updated this revision to Diff 214336.Aug 9 2019, 1:58 AM
grimar marked 8 inline comments as done.
grimar retitled this revision from [llvm-readobj] - Remove depricated unwrapOrError(Expected<T> EO). to [llvm-readobj] - Remove deprecated unwrapOrError(Expected<T> EO)..
  • Addressed review comments.
229 ↗(On Diff #214129)

Initially this file was introduced to report "e_shnum should be zero if a file has no section header table" (D25090)
Then rL285738 changed it to "Invalid data was encountered while parsing the file." and
after that rL287081 changed it to "invalid e_phentsize".

This is a result of error catching code change multiplied by effect from having a black box precompiled binary committed.
(It was a binary produced using fuzzing as far I remember, so it could be broken in a many places)
We should just get rid of them all.

247 ↗(On Diff #214129)

Yes, done.

429 ↗(On Diff #214129)

It can. I did not like how -DFILE=%p/Inputs/corrupt-invalid-phentsize.elf.x86-64 looks like (too long),
but after another look, I am OK with it.

jhenderson accepted this revision.Aug 9 2019, 3:23 AM
jhenderson added inline comments.
229 ↗(On Diff #214336)

Same as @MaskRay's comment - can this use -DFILE here too?

237 ↗(On Diff #214336)


This revision is now accepted and ready to land.Aug 9 2019, 3:23 AM
MaskRay accepted this revision.Aug 9 2019, 3:39 AM
This revision was automatically updated to reflect the committed changes.
grimar marked 2 inline comments as done.
Herald added a project: Restricted Project. · View Herald TranscriptAug 9 2019, 3:53 AM