Page MenuHomePhabricator

[llvm-readelf/obj] - Stop calling `reportError` in `printArchSpecificInfo()`.
ClosedPublic

Authored by grimar on Nov 25 2020, 4:19 AM.

Details

Summary

This is related to MIPS. Currently we might report an error and exit,
though there is no problem to report a warning and try to continue dumping
an object. The code uses MipsGOTParser<ELFT> Parser, which is isolated
in this method.

Diff Detail

Event Timeline

grimar created this revision.Nov 25 2020, 4:19 AM
grimar requested review of this revision.Nov 25 2020, 4:19 AM
This revision is now accepted and ready to land.Nov 25 2020, 7:39 AM
jhenderson added inline comments.Nov 26 2020, 12:25 AM
llvm/test/tools/llvm-readobj/ELF/mips-got.test
374

Is there output between the end of the NO-OUTPUT block and ERR2 (also ERR3, ERR4 etc)? The ERR1 case uses -NEXT here.

377–378

Entirely optional, but if you were to replace the second and third line prefixes of this and each subsequent test case with NO-OUTPUT, and then put it at the end of the file, you wouldn't need to repeat the -EMPTY, -NOT lines for each test case. If you did that, I might reorder some things, roughly like this:

# RUN: <existing ERR1 run line(s)>

# RUN: <existing ERR2 run line(s)>

...

# NO-OUTPUT:      LoadName: <Not found>
# NO-OUTPUT-NEXT: There is no .MIPS.abiflags section in the file.
# NO-OUTPUT-NEXT: There is no .MIPS.options section in the file.
# NO-OUTPUT-NEXT: There is no .reginfo section in the file.

# ERR1-NEXT: warning: '[[FILE]]': cannot find PLTGOT dynamic tag
# ERR2: warning: '[[FILE]]': cannot find MIPS_LOCAL_GOTNO dynamic tag
...

# NO-OUTPUT-EMPTY:
# NO-OUTPUT-NOT: {{.}}

Same comment goes for the other test file.

llvm/test/tools/llvm-readobj/ELF/mips-plt.test
97

As above, can this and below use -NEXT?

grimar updated this revision to Diff 307793.Nov 26 2020, 1:57 AM
grimar marked 3 inline comments as done.
  • Addressed review comments.
llvm/test/tools/llvm-readobj/ELF/mips-got.test
374

Used -NEXT.

377–378

Done. I thought about something like this, but without grouping "ERR" check lines together (i.e. only supposed that we
can move the last 2 "NO-OUTPUT-*" lines later. Was not sure about this solution.

I think that your version where all check lines are grouped together in one place looks good.

llvm/test/tools/llvm-readobj/ELF/mips-plt.test
97

Done.