[llvm-readobj] Print error when executed with no input files

[Elvina]

Summary

This patch changes llvm-readelf (and llvm-readobj for consistency) behavior. This means to print the error when executed with no input files. Reading from stdin can be achieved via a '-' for the input object.
Fixes https://bugs.llvm.org/show_bug.cgi?id=46400

Yours,
Elvina
Software Engineer
Advanced Software Technology Lab, Huawei

[MaskRay]

LGTM, but I'd prefer splitting the large amount of - appending to a separate patch and commit it beforehand.

[jyknight]

FWIW, it rather bugs me when tools print their entire (long) help upon command-line syntax error. Simply printing an error is generally better. (E.g. "error: no input files specified.")

[sbc100]

In D83704#2148210, @jyknight wrote:

FWIW, it rather bugs me when tools print their entire (long) help upon command-line syntax error. Simply printing an error is generally better. (E.g. "error: no input files specified.")

+1 to not printing full help unless it is explicitly asked for.

[jhenderson]

Thanks for looking at this. Is there a specific test-case for llvm-readobj's behaviour with no input file? You should add one if not.

GNU readelf prints the following warning in this case readelf: Warning: Nothing to do. and returns 1. I'm happy for us to deviate from GNU in this case slightly, perhaps as suggested above with the simple error message, but we definitely should return non-zero.

[Elvina]

Thanks to everyone for the review! I changed llvm-readobj's behavior as suggested, so now an error message is displayed if no input files are provided and also made a separate patch for all updated tests https://reviews.llvm.org/D83912

[jhenderson]

LGTM, once a couple of nits are fixed.

By the way, you can show that two commits have dependencies on each other by using the "Edit Related Revisions" Phabricator UI option, or by putting "Depends on DXXXXXX" in the description of the patch.

llvm/test/tools/llvm-readobj/basic.test

7

Nit: missing full stop.

llvm/tools/llvm-readobj/llvm-readobj.cpp

695

Use lower case for the first letter of error and warning messages (see https://llvm.org/docs/CodingStandards.html#error-and-warning-messages).

[jhenderson]

Oh, this probably warrants a release note adding too. Can you do that @Elvina?

[Elvina]

@jhenderson Sure, done!

[Elvina]

@jhenderson Could you please commit on my behalf? I don't have commit rights.

[jhenderson]

In D83704#2155933, @Elvina wrote:

@jhenderson Could you please commit on my behalf? I don't have commit rights.

Can do. What do you want your git commit author name and email address to be? (Feel free to directly email me if you don't want it posted on Phabricator directly).

[jhenderson]

By the way, if you address a comment by a reviewer, you can check the "Done" box on the inline comment before uploading your diff, and it will be marked as done. It helps highlight when a comment has been addressed/not addressed.

llvm/docs/ReleaseNotes.rst

126–128

I would rephrase this slightly as follows:

"llvm-readobj and llvm-readelf behavior has changed to report an error when executed with no input files instead of reading an input from stdin. Reading from stdin can still be achieved by specifying `-` as an input file.

[Elvina]

Updated Release Notes

[Elvina]

In D83704#2157586, @jhenderson wrote:

In D83704#2155933, @Elvina wrote:

@jhenderson Could you please commit on my behalf? I don't have commit rights.

Can do. What do you want your git commit author name and email address to be? (Feel free to directly email me if you don't want it posted on Phabricator directly).

Thanks a lot! I sent you an email.

llvm/docs/CommandGuide/llvm-readelf.rst

 llvm-readelf - GNU-style LLVM Object Reader
 ===========================================
 
 .. program:: llvm-readelf
 
 SYNOPSIS
 --------
 
 :program:`llvm-readelf` [*options*] [*input...*]
 
 DESCRIPTION
 -----------
 
 The :program:`llvm-readelf` tool displays low-level format-specific information
 about one or more object files.
 
-If ``input`` is "``-``" or omitted, :program:`llvm-readelf` reads from standard
+If ``input`` is "``-``", :program:`llvm-readelf` reads from standard
 input. Otherwise, it will read from the specified ``filenames``.
 
 OPTIONS
 -------
 
 .. option:: --all
 
  Equivalent to specifying all the main display options.

llvm/docs/CommandGuide/llvm-readobj.rst

 llvm-readobj - LLVM Object Reader
 =================================
 
 .. program:: llvm-readobj
 
 SYNOPSIS
 --------
 
 :program:`llvm-readobj` [*options*] [*input...*]
 
 DESCRIPTION
 -----------
 
 The :program:`llvm-readobj` tool displays low-level format-specific information
 about one or more object files.
 
-If ``input`` is "``-``" or omitted, :program:`llvm-readobj` reads from standard
+If ``input`` is "``-``", :program:`llvm-readobj` reads from standard
 input. Otherwise, it will read from the specified ``filenames``.
 
 DIFFERENCES TO LLVM-READELF
 ---------------------------
 
 :program:`llvm-readelf` is an alias for the :manpage:`llvm-readobj` tool with a
 slightly different command-line interface and output that is GNU compatible.
 Following is a list of differences between :program:`llvm-readelf` and

llvm/docs/ReleaseNotes.rst

 Changes to the Debug Info
 ---------------------------------
 
 During this release ...
 
 Changes to the LLVM tools
 ---------------------------------
 
-During this release ...
+* llvm-readobj and llvm-readelf behavior has changed to report an error when
+  executed with no input files instead of reading an input from stdin.
+  Reading from stdin can still be achieved by specifying `-` as an input file.
 
 Changes to LLDB
 ---------------------------------
 
 External Open Source Projects Using LLVM 12
 ===========================================
 
 * A project...

llvm/test/tools/llvm-readobj/basic.test

 ## Test case where input file does not exit.
 RUN: not llvm-readobj %t.blah 2>&1 | FileCheck --check-prefix=ENOENT -DTOOL=readobj %s
 RUN: not llvm-readelf %t.blah 2>&1 | FileCheck --check-prefix=ENOENT -DTOOL=readelf %s
 
 ENOENT: llvm-[[TOOL]]{{(\.exe)?}}: error: '{{.*}}.blah': {{[Nn]}}o such file or directory
 
+# Test case with no input file.
+RUN: not llvm-readobj 2>&1 | FileCheck %s --check-prefix=NO-FILE
+RUN: not llvm-readelf 2>&1  | FileCheck %s --check-prefix=NO-FILE
+NO-FILE: error: no input files specified
+
 # Test case where input file is too small to be a recognised object file.
 RUN: touch %t.empty
 RUN: not llvm-readobj %t.empty 2>&1 | FileCheck --check-prefix=EMPTY %s
 EMPTY: '{{.*}}.empty': The file was not recognized as a valid object file
 
 # Test that unrecognised files in archives are ignored.
 RUN: rm -f %t.a
 RUN: llvm-ar rc %t.a %t.empty

llvm/tools/llvm-readobj/llvm-readobj.cpp

if (sys::path::stem(argv[0]).contains("readelf")) {
     opts::Output = opts::GNU;
     registerReadelfAliases();
   } else {
     registerReadobjAliases();
   }
 
   cl::ParseCommandLineOptions(argc, argv, "LLVM Object Reader\n");
 
+  // Default to print error if no filename is specified.
+  if (opts::InputFilenames.empty()) {
+    error("no input files specified");
+  }
+
   if (opts::All) {
     opts::FileHeaders = true;
     opts::ProgramHeaders = true;
     opts::SectionHeaders = true;
     opts::Symbols = true;
     opts::Relocations = true;
     opts::DynamicTable = true;
     opts::Notes = true;
     opts::VersionInfo = true;
     opts::UnwindInfo = true;
     opts::SectionGroups = true;
     opts::HashHistogram = true;
     if (opts::Output == opts::LLVM) {
       opts::Addrsig = true;
       opts::PrintStackSizes = true;
     }
   }
 
   if (opts::Headers) {
     opts::FileHeaders = true;
     opts::ProgramHeaders = true;
     opts::SectionHeaders = true;
   }
 
-  // Default to stdin if no filename is specified.
-  if (opts::InputFilenames.empty())
-    opts::InputFilenames.push_back("-");
-
   ScopedPrinter Writer(fouts());
   for (const std::string &I : opts::InputFilenames)
     dumpInput(I, Writer);
 
   if (opts::CodeViewMergedTypes) {
     if (opts::CodeViewEnableGHash)
       dumpCodeViewMergedTypes(Writer, CVTypes.GlobalIDTable.records(),
                               CVTypes.GlobalTypeTable.records());
     else
       dumpCodeViewMergedTypes(Writer, CVTypes.IDTable.records(),
                               CVTypes.TypeTable.records());
   }
 
   return 0;
 }