[llvm-readobj] Prepend argv[0] to error messages

[MaskRay]

Summary

Currently, we report:

error: ...

Prepend argv[0] (tool name):

llvm-readobj: error: ...

This is consistent with most GNU binutils/clang/lld. It gives a bit more
context in a long build log.

[grimar]

test/tools/llvm-readobj/error-format.test

8

It fails under windows for me. Because of .exe suffix:

D:\Work2\llvm\llvm\test\tools\llvm-readobj\error-format.test:6:12: error: READELF: expected string not found in input
# READELF: llvm-readelf: error: '{{.*}}':
           ^
<stdin>:2:1: note: scanning from here
d:\work2\llvm\llvm_build\debug\bin\llvm-readelf.exe: error: 'D:\Work2\llvm\llvm\
test\tools\llvm-readobj/non-existent': no such file or directory
^

[grimar]

test/tools/llvm-readobj/error-format.test

8

The following helps:

# READELF: llvm-readelf{{(.exe)?}}: error: '{{.*}}':
# READOBJ: llvm-readobj{{(.exe)?}}: error: '{{.*}}':

(no more other readelf tests are failing for me, btw).

I see that out other tests sometimes using something like: llvm-ar{{(.exe|.EXE)?}},
but I am not sure why handling the uppercase .EXE is useful/important.

[MaskRay]

Use llvm-readelf{{(\.exe)?}}: error: '{{.*}}': for Windows. Thanks grimar for pointing that out.

The concrete error message is OS specific "No such file or ..." so we can't test it.

[MaskRay]

test/tools/llvm-readobj/error-format.test

8

Thanks for testing! I was indeed thinking what Windows would do...

So the error message is ineed OS dependent. On Linux:

% errno 2
ENOENT 2 No such file or directory

On Windows it is not capitalized: no such file or directory

I think .EXE is probably 8.3 filename remnant from DOS or Windows 95... I think it is not useful unless the basename also follows 8.3, i.e. LLVM-R~1.EXE LLVM-R~2.EXE

[grimar]

LGTM, I can confirm it works now. A minor suggestion is inlined.

test/tools/llvm-readobj/error-format.test

8

Some of the our tests use: {{[N|n]}}o such file or directory.

Perhaps I would do that:

# READELF: llvm-readelf{{(\.exe)?}}: error: '{{.*}}': {{[N|n]}}o such file or directory
# READOBJ: llvm-readobj{{(\.exe)?}}: error: '{{.*}}': {{[N|n]}}o such file or directory

[MaskRay]

Add {{[N|n]}}o such file or directory

[jhenderson]

test/tools/llvm-readobj/error-format.test

2

How about a warning case too?

[MaskRay]

Add a warning test

[jhenderson]

test/tools/llvm-readobj/error-format.test

7–8

Have you considered folding these two together using FileCheck's -D option?

# RUN: ... | FileCheck --check-prefix=ERR %s -DTOOL=readelf
# RUN: ... | FileCheck --check-prefix=ERR %s -DTOOL=readobj

# ERR: llvm-[[TOOL]]{{(\.exe)?}}: ...

Same goes for the warning case.

10–13

I think we should probably just use yaml2obj here instead of relying on pre-built binaries. I don't feel strongly about it though.

[MaskRay]

Use -DTOOL=

[MaskRay]

test/tools/llvm-readobj/error-format.test

10–13

This is the simplest way to get a warning. I don't want to be distracted by the lengthy YAML...

[jhenderson]

include/llvm/BinaryFormat/ELF.h

1422

Unrelated?

[grimar]

test/tools/llvm-readobj/error-format.test

10–13

If we do not want to convert the precomipled binaries in test cases to YAML generally, then I'll stop my attemps to improve that. Generally I supposed that having precompiled binaries is too bad to have.

[MaskRay]

Use yaml2obj

[grimar]

test/tools/llvm-readobj/error-format.test

20

Thanks :)

[jhenderson]

LGTM.

[thakis]

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

708 ↗

(On Diff #216113)

lld and clang only print the basename, see D49189 D49189 for lld for example (clang has similar code).

[thakis]

Sorry the 2nd link should've been D51133.

include/llvm/BinaryFormat/ELF.h

enum : unsigned {
   NT_FILE = 0x46494c45,
   NT_PRXFPREG = 0x46e62b7f,
   NT_SIGINFO = 0x53494749,
 };
 
 // LLVM-specific notes.
 enum {
   NT_LLVM_HWASAN_GLOBALS = 3,
+  NT_LLVM_WATERMARK = 4,
 };
 
 // GNU note types
 enum {
   NT_GNU_ABI_TAG = 1,
   NT_GNU_HWCAP = 2,
   NT_GNU_BUILD_ID = 3,
   NT_GNU_GOLD_VERSION = 4,

test/tools/llvm-readobj/error-format.test

+## This test shows that we include the tool name in error/warning messages.
+
+# RUN: not llvm-readelf %S/non-existent 2>&1 | FileCheck --check-prefix=ERR %s -DTOOL=readelf
+# RUN: not llvm-readobj %S/non-existent 2>&1 | FileCheck --check-prefix=ERR %s -DTOOL=readobj
+
+# ERR: llvm-[[TOOL]]{{(\.exe)?}}: error: '{{.*}}': {{[Nn]}}o such file or directory
+
+# RUN: llvm-readelf -x 10 %S/Inputs/trivial.obj.elf-x86-64 2>&1 | \
+# RUN:   FileCheck --check-prefix=WARN %s -DTOOL=readelf
+# RUN: llvm-readobj -x 10 %S/Inputs/trivial.obj.elf-x86-64 2>&1 | \
+# RUN:   FileCheck --check-prefix=WARN %s -DTOOL=readobj
+
+# WARN: llvm-[[TOOL]]{{(\.exe)?}}: warning: '{{.*}}': could not find section 10

tools/llvm-readobj/llvm-readobj.cpp

cl::opt<OutputStyleTy>
              cl::values(clEnumVal(LLVM, "LLVM default style"),
                         clEnumVal(GNU, "GNU readelf style")),
              cl::init(LLVM));
 
   cl::extrahelp
       HelpResponse("\nPass @FILE as argument to read options from FILE.\n");
 } // namespace opts
 
+static StringRef ToolName;
+
 namespace llvm {
 
 LLVM_ATTRIBUTE_NORETURN static void error(Twine Msg) {
   // Flush the standard output to print the error at a
   // proper place.
   fouts().flush();
   errs() << "\n";
-  WithColor::error(errs()) << Msg << "\n";
+  WithColor::error(errs(), ToolName) << Msg << "\n";
   exit(1);
 }
 
 LLVM_ATTRIBUTE_NORETURN void reportError(Error Err, StringRef Input) {
   assert(Err);
   if (Input == "-")
     Input = "<stdin>";
   handleAllErrors(createFileError(Input, std::move(Err)),
                   [&](const ErrorInfoBase &EI) { error(EI.message()); });
   llvm_unreachable("error() call should never return");
 }
 
 void reportWarning(Error Err, StringRef Input) {
   assert(Err);
   if (Input == "-")
     Input = "<stdin>";
 
   // Flush the standard output to print the warning at a
   // proper place.
   fouts().flush();
-  handleAllErrors(createFileError(Input, std::move(Err)),
-                  [&](const ErrorInfoBase &EI) {
-                    errs() << "\n";
-                    WithColor::warning(errs()) << EI.message() << "\n";
-                  });
+  handleAllErrors(
+      createFileError(Input, std::move(Err)), [&](const ErrorInfoBase &EI) {
+        WithColor::warning(errs(), ToolName) << EI.message() << "\n";
 }
 
 LLVM_ATTRIBUTE_NORETURN void reportError(std::error_code EC, StringRef Input) {
   assert(EC != readobj_error::success);
   reportError(errorCodeToError(EC), Input);
 }
 
 } // namespace llvm
for (auto &OptEntry : cl::getRegisteredOptions()) {
     cl::Option *Option = OptEntry.getValue();
     if (ArgName.size() == 1)
       apply(Option, cl::Grouping);
   }
 }
 
 int main(int argc, const char *argv[]) {
   InitLLVM X(argc, argv);
+  ToolName = argv[0];
 
   // Register the target printer for --version.
   cl::AddExtraVersionPrinter(TargetRegistry::printRegisteredTargetsForVersion);
 
   if (sys::path::stem(argv[0]).contains("readelf")) {
     opts::Output = opts::GNU;
     registerReadelfAliases();
   } else {