[llvm-objdump] Delete argv[0] from error messages

[MaskRay]

Summary

Currently we report

${argv[0]}: error: ...

GNU objdump does not print error:

${argv[0]}: ...

error: is useful, but repeating argv[0] does not provide lots of value. So switch to:

error: ...

This is also consistent with llvm-objcopy and llvm-readobj.

[grimar]

I think it is fine. Lets see what others think.

[jhenderson]

I don't feel strongly about this, but it would be a departure from what GNU tools and clang and LLD, and even MSVC do, so we probably shouldn't do this. The exact behaviour appears to be somewhat inconsistent, but in most cases, GNU tools do:

<argv[0]>: <error message>

Clang and LLD do:

<tool name>: error: <error message>

I'd therefore suggest that perhaps the correct thing to do is change llvm-objcopy and llvm-readobj.

I guess one advantage with the tool name being printed is that it gives a bit more context in a big long build log, if there are multiple problems, it's possible to tell which error/warning comes from which tool, especially as buffering and multi-threaded builds might result in the error message being rather far from related output.

MSVC appears to do:

<tool name> : <error/warning type> <error code> : <error message>

[MaskRay]

In D66414#1635152, @jhenderson wrote:

I don't feel strongly about this, but it would be a departure from what GNU tools and clang and LLD, and even MSVC do, so we probably shouldn't do this. The exact behaviour appears to be somewhat inconsistent, but in most cases, GNU tools do:

<argv[0]>: <error message>

Clang and LLD do:

<tool name>: error: <error message>

I'd therefore suggest that perhaps the correct thing to do is change llvm-objcopy and llvm-readobj.

I guess one advantage with the tool name being printed is that it gives a bit more context in a big long build log, if there are multiple problems, it's possible to tell which error/warning comes from which tool, especially as buffering and multi-threaded builds might result in the error message being rather far from related output.

MSVC appears to do:

<tool name> : <error/warning type> <error code> : <error message>

Created D66425 to prepend argv[0] for llvm-readobj/llvm-readelf. llvm-objcopy already includes argv[0] in its error messages.

[rupprecht]

I feel a little more strongly that we need to have the toolname printed as part of the error message.

[MaskRay]

We've decided to go with D66425.

tools/llvm-objdump/llvm-objdump.cpp

Wide("wide", cl::desc("Ignored for compatibility with GNU objdump"),
          cl::cat(ObjdumpCat));
 static cl::alias WideShort("w", cl::Grouping, cl::aliasopt(Wide));
 
 static cl::extrahelp
     HelpResponse("\nPass @FILE as argument to read options from FILE.\n");
 
 static StringSet<> DisasmFuncsSet;
 static StringSet<> FoundSectionSet;
-static StringRef ToolName;
 
 typedef std::vector<std::tuple<uint64_t, StringRef, uint8_t>> SectionSymbolsTy;
 
 static bool shouldKeep(object::SectionRef S) {
   if (FilterSections.empty())
     return true;
 
   Expected<StringRef> SecNameOrErr = S.getName();

 
 SectionFilter ToolSectionFilter(object::ObjectFile const &O) {
   return SectionFilter([](object::SectionRef S) { return shouldKeep(S); }, O);
 }
 
 void error(std::error_code EC) {
   if (!EC)
     return;
-  WithColor::error(errs(), ToolName)
-      << "reading file: " << EC.message() << ".\n";
+  WithColor::error(errs()) << "reading file: " << EC.message() << ".\n";
   errs().flush();
   exit(1);
 }
 
 void error(Error E) {
   if (!E)
     return;
-  WithColor::error(errs(), ToolName) << toString(std::move(E));
+  WithColor::error(errs()) << toString(std::move(E));
   exit(1);
 }
 
 LLVM_ATTRIBUTE_NORETURN void error(Twine Message) {
-  WithColor::error(errs(), ToolName) << Message << ".\n";
+  WithColor::error(errs()) << Message << ".\n";
   errs().flush();
   exit(1);
 }
 
 void warn(Twine Message) {
   // Output order between errs() and outs() matters especially for archive
   // files where the output is per member object.
   outs().flush();
-  WithColor::warning(errs(), ToolName) << Message << "\n";
+  WithColor::warning(errs()) << Message << "\n";
   errs().flush();
 }
 
 LLVM_ATTRIBUTE_NORETURN void report_error(StringRef File, Twine Message) {
-  WithColor::error(errs(), ToolName)
-      << "'" << File << "': " << Message << ".\n";
+  WithColor::error(errs()) << "'" << File << "': " << Message << ".\n";
   exit(1);
 }
 
 LLVM_ATTRIBUTE_NORETURN void report_error(Error E, StringRef File) {
   assert(E);
   std::string Buf;
   raw_string_ostream OS(Buf);
   logAllUnhandledErrors(std::move(E), OS);
   OS.flush();
-  WithColor::error(errs(), ToolName) << "'" << File << "': " << Buf;
+  WithColor::error(errs()) << "'" << File << "': " << Buf;
   exit(1);
 }
 
 LLVM_ATTRIBUTE_NORETURN void report_error(Error E, StringRef ArchiveName,
                                           StringRef FileName,
                                           StringRef ArchitectureName) {
   assert(E);
-  WithColor::error(errs(), ToolName);
+  WithColor::error(errs());
   if (ArchiveName != "")
     errs() << ArchiveName << "(" << FileName << ")";
   else
     errs() << "'" << FileName << "'";
   if (!ArchitectureName.empty())
     errs() << " (for architecture " << ArchitectureName << ")";
   std::string Buf;
   raw_string_ostream OS(Buf);
static void printUnwindInfo(const ObjectFile *O) {
   outs() << "Unwind info:\n\n";
 
   if (const COFFObjectFile *Coff = dyn_cast<COFFObjectFile>(O))
     printCOFFUnwindInfo(Coff);
   else if (const MachOObjectFile *MachO = dyn_cast<MachOObjectFile>(O))
     printMachOUnwindInfo(MachO);
   else
     // TODO: Extract DWARF dump tool to objdump.
-    WithColor::error(errs(), ToolName)
-        << "This operation is only currently supported "
+    WithColor::error(errs()) << "This operation is only currently supported "
                                 "for COFF and MachO object files.\n";
 }
 
 /// Dump the raw contents of the __clangast section so the output can be piped
 /// into llvm-bcanalyzer.
 void printRawClangAST(const ObjectFile *Obj) {
   if (outs().is_displayed()) {
-    WithColor::error(errs(), ToolName)
+    WithColor::error(errs())
         << "The -raw-clang-ast option will dump the raw binary contents of "
            "the clang ast section.\n"
            "Please redirect the output to a file or another program such as "
            "llvm-bcanalyzer.\n";
     return;
   }
 
   StringRef ClangASTSectionName("__clangast");

 static void printFaultMaps(const ObjectFile *Obj) {
   StringRef FaultMapSectionName;
 
   if (isa<ELFObjectFileBase>(Obj)) {
     FaultMapSectionName = ".llvm_faultmaps";
   } else if (isa<MachOObjectFile>(Obj)) {
     FaultMapSectionName = "__llvm_faultmaps";
   } else {
-    WithColor::error(errs(), ToolName)
-        << "This operation is only currently supported "
+    WithColor::error(errs()) << "This operation is only currently supported "
                                 "for ELF and Mach-O executable files.\n";
     return;
   }
 
   Optional<object::SectionRef> FaultMapSection;
 
   for (auto Sec : ToolSectionFilter(*Obj)) {
     StringRef Name;
     if (Expected<StringRef> NameOrErr = Sec.getName())
static void printFileHeaders(const ObjectFile *O) {
   StringRef Fmt = O->getBytesInAddress() > 4 ? "%016" PRIx64 : "%08" PRIx64;
   outs() << "start address: "
          << "0x" << format(Fmt.data(), Address) << "\n\n";
 }
 
 static void printArchiveChild(StringRef Filename, const Archive::Child &C) {
   Expected<sys::fs::perms> ModeOrErr = C.getAccessMode();
   if (!ModeOrErr) {
-    WithColor::error(errs(), ToolName) << "ill-formed archive entry.\n";
+    WithColor::error(errs()) << "ill-formed archive entry.\n";
     consumeError(ModeOrErr.takeError());
     return;
   }
   sys::fs::perms Mode = ModeOrErr.get();
   outs() << ((Mode & sys::fs::owner_read) ? "r" : "-");
   outs() << ((Mode & sys::fs::owner_write) ? "w" : "-");
   outs() << ((Mode & sys::fs::owner_exe) ? "x" : "-");
   outs() << ((Mode & sys::fs::group_read) ? "r" : "-");
int main(int argc, char **argv) {
   // Register the target printer for --version.
   cl::AddExtraVersionPrinter(TargetRegistry::printRegisteredTargetsForVersion);
 
   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.
   if (InputFilenames.empty())
     InputFilenames.push_back("a.out");
 
   if (AllHeaders)
     ArchiveHeaders = FileHeaders = PrivateHeaders = Relocations =
         SectionHeaders = SymbolTable = true;