[tools] [llvm-objdump] Default to reading from stdin not a.out

[abrachet]

Summary

This moves away from defaulting to a.out and uses stdin only if stdin has a file redirected to it. This has been discussed on the llvm-dev mailing list here.

[MaskRay]

llvm/test/tools/llvm-objdump/stdin.test

9

You can writes this as:

not llvm-objdump -h < /dev/null 2>&1 | FileCheck %s

to test that llvm-objdump doesn't read from a.out

[compnerd]

This breaks compatibility with objdump. Please ensure that the original behavior is preserved when invoked as objdump rather than llvm-objdump.

[abrachet]

This breaks compatibility with objdump. Please ensure that the original behavior is preserved when invoked as objdump rather than llvm-objdump.

This was discussed on the mailing list, it was decided that defaulting to a.out was not something that was worth maintaining compatibility with GNU and favored standardizing the llvm tools, ie reading from stdin on default.

FWIW, I favored defaulting to a.out when stdin is a tty and stdin when a file has been redirected in.

[jhenderson]

In D64706#1584887, @compnerd wrote:

This breaks compatibility with objdump. Please ensure that the original behavior is preserved when invoked as objdump rather than llvm-objdump.

Hi @compnerd,

Do you have a concrete use-case for reading from a.out? The mailing list discussion was of the opinion that this was not an area we need GNU compatibility with. However, we could probably be persuaded otherwise with appropriate evidence.

[compnerd]

objcopy is often used in autotools to run tests of what the compiler is doing. I'm afraid that changing that behaviour will break the autoconf tests which rely on the behaviour (a.out is what the compiler generates by default, and that is why the tool defaults to that filename). So, if you have something like the following:

cc test.c
objcopy --extract-dwo

to test whether you can do split-debug with dwarf packages, it will break.

[abrachet]

We could do:

if (InputFilenames.empty())
  InputFilenames.push_back(StandardInIsUserInput() ? "a.out" : "-");

This behavior would be very confusing though. Not to mention wouldn't even work always (doesn't for lit which seemingly redirects something to stdin).

FWIW, @MaskRay pointed out that addr2line defaults to a.out but llvm-addr2line doesn't. Since that thread llvm-nm also defaults to reading from standard input while nm uses a.out, see rL365889. I have no preference I don't mind going back on this or rL365889.

[jhenderson]

In D64706#1585753, @compnerd wrote:

objcopy is often used in autotools to run tests of what the compiler is doing. I'm afraid that changing that behaviour will break the autoconf tests which rely on the behaviour (a.out is what the compiler generates by default, and that is why the tool defaults to that filename). So, if you have something like the following:

cc test.c
objcopy --extract-dwo

to test whether you can do split-debug with dwarf packages, it will break.

It's worth noting that this is not about llvm-objcopy. It's about llvm-objdump. I don't believe our llvm-objcopy has the behaviour you've described implemented anyway. That being said, I think it's a valid point and you should bring it up on the thread on the mailing list to discuss it, so that this discussion gets more visibility: https://lists.llvm.org/pipermail/llvm-dev/2019-July/133642.html

[compnerd]

Thats what I get for not having coffee before responding. objdump is used quite a bit in tests as well (objdump -t | grep ..., objdump -h | grep ..., etc).

llvm/docs/CommandGuide/llvm-objdump.rst

 llvm-objdump - LLVM's object file dumper
 ========================================
 
 .. program:: llvm-objdump
 
 SYNOPSIS
 --------
 
 :program:`llvm-objdump` [*commands*] [*options*] [*filenames...*]
 
 DESCRIPTION
 -----------
 The :program:`llvm-objdump` utility prints the contents of object files and
-final linked images named on the command line. If no file name is specified,
-:program:`llvm-objdump` will attempt to read from *a.out*. If *-* is used as a
-file name, :program:`llvm-objdump` will process a file on its standard input
-stream.
+final linked images named on the command line. If no filename is specified,
+or *-* is used as a filename, :program:`llvm-objdump` will read a file from
+its standard input stream.
 
 COMMANDS
 --------
 At least one of the following commands are required, and some commands can be
 combined with other commands:
 
 .. option:: -a, --archive-headers
 

llvm/test/tools/llvm-objdump/stdin.test

+## Test llvm-objdump when using stdin both explicitly (using '-' as a filename)
+## and implicitly (not specifying any filename).
+
+# RUN: yaml2obj %s > a.out
+
+## This will fail because llvm-lit redirects to stdin by default, it will be
+## read but is not a valid object file, test that it fails because of this
+## and does not use a.out as the default file.
+# RUN: not llvm-objdump -h 2>&1 | FileCheck %s
+# CHECK-NOT: error: 'a.out': No such file or directory
+
+## This is a hack to get the base output to use the filename '<stdin>' which
+## is used as the filename in the llvm-objdumps output when reading a file
+## from standard input.
+# RUN: cp a.out \<stdin\>
+
+## Pass an explicit filename to produce a baseline output. llvm-objdump should
+## have the same behavior when opening a file itself and when reading that
+## file from its standard input stream.
+# RUN: llvm-objdump -h \<stdin\> > %t.base 2> %t.err
+# RUN: FileCheck %s --input-file=%t.err --allow-empty --implicit-check-not={{.}}
+
+# RUN: llvm-objdump -h - < a.out > %t.explicit 2> %t.err
+# RUN: FileCheck %s --input-file=%t.err --allow-empty --implicit-check-not={{.}}
+# UN: cmp %t.base %t.explicit
+
+# RUN: llvm-objdump -h < a.out > %t.implicit 2> %t.err
+# RUN: FileCheck %s --input-file=%t.err --allow-empty --implicit-check-not={{.}}
+# RUN: cmp %t.base %t.implicit
+
+# RUN: rm -f a.out \<stdin\>
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name:         .text
+    Type:         SHT_PROGBITS
+    Flags:        [SHF_ALLOC, SHF_EXECINSTR]
+    Address:      0x400
+    AddressAlign: 0x10
+    Content:      'c3'

llvm/tools/llvm-objdump/llvm-objdump.cpp

 #include "llvm/Support/Debug.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/GraphWriter.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cctype>
else
                    A->getFileName());
   }
   if (Err)
     report_error(std::move(Err), A->getFileName());
 }
 
 /// Open file and figure out how to dump it.
 static void dumpInput(StringRef file) {
+  if (file == "-" && sys::Process::StandardInIsUserInput()) {
+    WithColor::warning(errs(), ToolName) << "can't read from terminal\n";
+    cl::PrintHelpMessage();
+    exit(1);
+  }
+
   // If we are using the Mach-O specific object file parser, then let it parse
   // the file and process the command line options.  So the -arch flags can
   // be used to select specific slices, etc.
   if (MachOOpt) {
     parseInputMachO(file);
     return;
   }
 
int main(int argc, char **argv) {
 
   cl::ParseCommandLineOptions(argc, argv, "llvm object file dumper\n");
 
   if (StartAddress >= StopAddress)
     error("start address should be less than stop address");
 
   ToolName = argv[0];
 
-  // Defaults to a.out if no filenames specified.
+  // Defaults to stdin if no filenames are specified.
   if (InputFilenames.empty())
-    InputFilenames.push_back("a.out");
+    InputFilenames.push_back("-");
 
   if (AllHeaders)
     ArchiveHeaders = FileHeaders = PrivateHeaders = Relocations =
         SectionHeaders = SymbolTable = true;
 
   if (DisassembleAll || PrintSource || PrintLines ||
       (!DisassembleFunctions.empty()))
     Disassemble = true;