Skip to content

Commit

Permalink
Reapply: [llvm-size] Tidy up error messages (PR42970)
Browse files Browse the repository at this point in the history 
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

llvm-svn: 374793
  • Loading branch information
@rupprecht
rupprecht committed Oct 14, 2019
1 parent eef612b commit 1d32d8b
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 32 deletions.
4 changes: 2 additions & 2 deletions llvm/test/Object/macho-invalid.test
View file
Expand Up @@ -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)
Expand Down
10 changes: 3 additions & 7 deletions llvm/test/tools/llvm-size/invalid-input.test
View file
@@ -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:
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/tools/llvm-size/no-input.test
View file
@@ -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.

Expand Down
44 changes: 22 additions & 22 deletions llvm/tools/llvm-size/llvm-size.cpp
View file
Expand Up @@ -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 <algorithm>
#include <string>
Expand Down Expand Up @@ -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:
Expand All @@ -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<StringRef> NameOrErr = C.getName();
// TODO: if we have a error getting the name then it would be nice to print
Expand All @@ -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
Expand All @@ -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 << ") ";
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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<ObjectFile>(&Bin)) {
Expand All @@ -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)
Expand Down

0 comments on commit 1d32d8b

Please sign in to comment.