[PATCH] D123448: [readobj] Improve the messages output by unsupported options when using --elf-output-style=GNU

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Apr 10 22:36:22 PDT 2022


jhenderson added a reviewer: MaskRay.
jhenderson added a comment.
Herald added a subscriber: StephenFan.

Hi @kazuminn,

When preparing your diff for upload to phabricator, please make sure to include full diff context, something like `-U999999` in the command that generates the diff would do well for that. This allows us to search around the code a bit more.

I'm happy to review this, but for future reference, you should look in the Github history of the file for recent committers and those who reviewed their work to find good candidates for review.

Thank you for taking a look at this, but I'm not sure you've got the right idea with this piece of work though. For ease of reference, here is the github issues content from the link you posted:

> A number of options supported by --elf-output-style=LLVM are not implemented for GNU output style, a message is output in these cases which is not too useful for the user. For example use of --elf-linker-options prints:
>
> "printELFLinkerOptions not implemented!"
>
> This is the name of the function. A description of the command line option or missing functionality would be clearer.
>
> Other examples are seen in printCGProfile() and printBBAddrMaps().

To explain further what we would like to see: at the moment, if you specify a command-line like `llvm-readobj --elf-output-style=GNU some-file.o --elf-linker-options`, you get an output as described in the quote above. To improve this, all you need to do really is replace the output with something more descriptive of the user interface, for example "--elf-linker-options not supported for GNU output style". You should then do the same for other options that are similar - look through the ElfDumper.cpp code for such examples, which should be fairly easy to spot. https://github.com/llvm/llvm-project/blob/626039cdcc16b429c4403d36fad13fba2a6c14e9/llvm/tools/llvm-readobj/ELFDumper.cpp#L5762 is the location for the --elf-linker-options option discussed directly in the issue.

Rather than printing directly to the standard output stream, it might make more sense to change these messages to warnings. Look for instances of `reportUniqueWarning` for how to use it.

Finally, you should then add a test file under llvm/test/tools/llvm-readobj/ELF which shows that the unimplemented messages are printed as expected. There are plenty of good examples of testing warning messages in the other test files in that directory for you to use as a basis. Feel free to ask for help if you get stuck though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123448/new/

https://reviews.llvm.org/D123448



More information about the llvm-commits mailing list