From 67d83413d07af4cebbf6e91b011c5f15d8fae7ba Mon Sep 17 00:00:00 2001 From: Jordan Rupprecht Date: Mon, 14 Oct 2019 17:29:15 +0000 Subject: [PATCH] Reapply: [llvm-size] Tidy up error messages (PR42970) Clean up some formatting inconsistencies in the error messages and correctly exit with non-zero in all error cases. Originally submitted as r374771 and then reverted as r374780, this patch fixes the libObject test case in Object/macho-invalid.test. Patch by Alex Cameron Differential Revision: https://reviews.llvm.org/D68906 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@374793 91177308-0d34-0410-b5e6-96231b3b80d8 --- test/Object/macho-invalid.test | 4 +-- test/tools/llvm-size/invalid-input.test | 10 ++---- test/tools/llvm-size/no-input.test | 2 +- tools/llvm-size/llvm-size.cpp | 44 ++++++++++++------------- 4 files changed, 28 insertions(+), 32 deletions(-) diff --git a/test/Object/macho-invalid.test b/test/Object/macho-invalid.test index da71ced2cb9..1cbd7469d73 100644 --- a/test/Object/macho-invalid.test +++ b/test/Object/macho-invalid.test @@ -110,8 +110,8 @@ INVALID-SEGMENT-FILEOFF: macho-invalid-segment-fileoff': truncated or malformed RUN: not llvm-nm %p/Inputs/macho-invalid-segment-fileoff 2>&1 | FileCheck -check-prefix INVALID-SEGMENT-FILEOFF-NM %s INVALID-SEGMENT-FILEOFF-NM: macho-invalid-segment-fileoff truncated or malformed object (load command 0 fileoff field in LC_SEGMENT extends past the end of the file) -RUN: not llvm-size %p/Inputs/macho-invalid-segment-fileoff 2>&1 | FileCheck -check-prefix INVALID-SEGMENT-FILEOFF-SIZE %s -INVALID-SEGMENT-FILEOFF-SIZE: macho-invalid-segment-fileoff truncated or malformed object (load command 0 fileoff field in LC_SEGMENT extends past the end of the file) +RUN: not llvm-size %p/Inputs/macho-invalid-segment-fileoff 2>&1 | FileCheck -DFILE=%p/Inputs/macho-invalid-segment-fileoff -check-prefix INVALID-SEGMENT-FILEOFF-SIZE %s +INVALID-SEGMENT-FILEOFF-SIZE: error: '[[FILE]]': truncated or malformed object (load command 0 fileoff field in LC_SEGMENT extends past the end of the file) RUN: not llvm-objdump --macho --private-headers %p/Inputs/macho-invalid-segment-filesize 2>&1 | FileCheck -check-prefix INVALID-SEGMENT-FILESIZE %s INVALID-SEGMENT-FILESIZE: macho-invalid-segment-filesize': truncated or malformed object (load command 0 fileoff field plus filesize field in LC_SEGMENT extends past the end of the file) diff --git a/test/tools/llvm-size/invalid-input.test b/test/tools/llvm-size/invalid-input.test index a27c7806ab0..568bd8201c2 100644 --- a/test/tools/llvm-size/invalid-input.test +++ b/test/tools/llvm-size/invalid-input.test @@ -1,19 +1,15 @@ ## Show that llvm-size reports an error when passed an input file in an ## unknown format. -## FIXME: The error messages tested here are not consistently formatted, and the -## second one doesn't even return with a non-zero exit code. -## See https://bugs.llvm.org/show_bug.cgi?id=42970. - # RUN: not llvm-size %s 2>&1 | FileCheck %s -DFILE=%s --check-prefix=UNRECOGNIZED -# UNRECOGNIZED: {{.*}}llvm-size{{(.*)}}: [[FILE]] The file was not recognized as a valid object file +# UNRECOGNIZED: {{.*}}llvm-size{{(.*)}}: error: '[[FILE]]': The file was not recognized as a valid object file ## Show that llvm-size reports an error when passed an input file in an ## unsupported format. # RUN: yaml2obj %s -o %t -# RUN: llvm-size %t 2>&1 | FileCheck %s -DFILE=%t --check-prefix=NOTSUPPORTED -# NOTSUPPORTED: {{.*}}llvm-size{{(.*)}}: [[FILE]]: Unrecognized file type. +# RUN: not llvm-size %t 2>&1 | FileCheck %s -DFILE=%t --check-prefix=NOTSUPPORTED +# NOTSUPPORTED: {{.*}}llvm-size{{(.*)}}: error: '[[FILE]]': unsupported file type --- !minidump Streams: diff --git a/test/tools/llvm-size/no-input.test b/test/tools/llvm-size/no-input.test index d4a145ff270..37e5745bf17 100644 --- a/test/tools/llvm-size/no-input.test +++ b/test/tools/llvm-size/no-input.test @@ -1,7 +1,7 @@ ## Show that llvm-size emits an error if passed in a non-existent file. # RUN: not llvm-size %t.blah 2>&1 | FileCheck %s -DFILE=%t.blah --check-prefix=ENOENT -# ENOENT: {{.*}}llvm-size{{.*}}: [[FILE]] {{[Nn]}}o such file or directory +# ENOENT: {{.*}}llvm-size{{.*}}: error: '[[FILE]]': {{[Nn]}}o such file or directory ## Show that llvm-size reads a.out if not passed any file. diff --git a/tools/llvm-size/llvm-size.cpp b/tools/llvm-size/llvm-size.cpp index da56199fe3c..7c63bc291f1 100644 --- a/tools/llvm-size/llvm-size.cpp +++ b/tools/llvm-size/llvm-size.cpp @@ -24,6 +24,7 @@ #include "llvm/Support/Format.h" #include "llvm/Support/InitLLVM.h" #include "llvm/Support/MemoryBuffer.h" +#include "llvm/Support/WithColor.h" #include "llvm/Support/raw_ostream.h" #include #include @@ -106,11 +107,9 @@ static bool HadError = false; static std::string ToolName; -static bool error(Twine Message) { +static void error(const Twine &Message, StringRef File) { HadError = true; - errs() << ToolName << ": " << Message << ".\n"; - errs().flush(); - return true; + WithColor::error(errs(), ToolName) << "'" << File << "': " << Message << "\n"; } // This version of error() prints the archive name and member name, for example: @@ -119,7 +118,7 @@ static bool error(Twine Message) { static void error(llvm::Error E, StringRef FileName, const Archive::Child &C, StringRef ArchitectureName = StringRef()) { HadError = true; - errs() << ToolName << ": " << FileName; + WithColor::error(errs(), ToolName) << "'" << FileName << "'"; Expected NameOrErr = C.getName(); // TODO: if we have a error getting the name then it would be nice to print @@ -138,7 +137,7 @@ static void error(llvm::Error E, StringRef FileName, const Archive::Child &C, raw_string_ostream OS(Buf); logAllUnhandledErrors(std::move(E), OS); OS.flush(); - errs() << " " << Buf << "\n"; + errs() << ": " << Buf << "\n"; } // This version of error() prints the file name and which architecture slice it // is from, for example: "foo.o (for architecture i386)" after the ToolName @@ -147,7 +146,7 @@ static void error(llvm::Error E, StringRef FileName, const Archive::Child &C, static void error(llvm::Error E, StringRef FileName, StringRef ArchitectureName = StringRef()) { HadError = true; - errs() << ToolName << ": " << FileName; + WithColor::error(errs(), ToolName) << "'" << FileName << "'"; if (!ArchitectureName.empty()) errs() << " (for architecture " << ArchitectureName << ") "; @@ -156,7 +155,7 @@ static void error(llvm::Error E, StringRef FileName, raw_string_ostream OS(Buf); logAllUnhandledErrors(std::move(E), OS); OS.flush(); - errs() << " " << Buf << "\n"; + errs() << ": " << Buf << "\n"; } /// Get the length of the string that represents @p num in Radix including the @@ -529,7 +528,7 @@ static bool checkMachOAndArchFlags(ObjectFile *O, StringRef Filename) { if (none_of(ArchFlags, [&](const std::string &Name) { return Name == T.getArchName(); })) { - error(Filename + ": No architecture specified"); + error("no architecture specified", Filename); return false; } return true; @@ -658,15 +657,15 @@ static void printFileSectionSizes(StringRef file) { error(std::move(Err), UA->getFileName()); } else { consumeError(AOrErr.takeError()); - error("Mach-O universal file: " + file + " for architecture " + - StringRef(I->getArchFlagName()) + - " is not a Mach-O file or an archive file"); + error("mach-o universal file for architecture " + + StringRef(I->getArchFlagName()) + + " is not a mach-o file or an archive file", + file); } } } if (!ArchFound) { - errs() << ToolName << ": file: " << file - << " does not contain architecture" << ArchFlags[i] << ".\n"; + error("file does not contain architecture " + ArchFlags[i], file); return; } } @@ -740,9 +739,10 @@ static void printFileSectionSizes(StringRef file) { error(std::move(Err), UA->getFileName()); } else { consumeError(AOrErr.takeError()); - error("Mach-O universal file: " + file + " for architecture " + - StringRef(I->getArchFlagName()) + - " is not a Mach-O file or an archive file"); + error("mach-o universal file for architecture " + + StringRef(I->getArchFlagName()) + + " is not a mach-o file or an archive file", + file); } return; } @@ -816,9 +816,10 @@ static void printFileSectionSizes(StringRef file) { error(std::move(Err), UA->getFileName()); } else { consumeError(AOrErr.takeError()); - error("Mach-O universal file: " + file + " for architecture " + - StringRef(I->getArchFlagName()) + - " is not a Mach-O file or an archive file"); + error("mach-o universal file for architecture " + + StringRef(I->getArchFlagName()) + + " is not a mach-o file or an archive file", + file); } } } else if (ObjectFile *o = dyn_cast(&Bin)) { @@ -836,8 +837,7 @@ static void printFileSectionSizes(StringRef file) { outs() << "\n"; } } else { - errs() << ToolName << ": " << file << ": " - << "Unrecognized file type.\n"; + error("unsupported file type", file); } // System V adds an extra newline at the end of each file. if (OutputFormat == sysv) -- 2.40.0