From 774770093703bf9fb8e32a9fb056d0ec0b925701 Mon Sep 17 00:00:00 2001 From: Alex Brachet Date: Tue, 18 Jun 2019 00:39:10 +0000 Subject: [PATCH] [llvm-strip] Error when using stdin twice Summary: Implements bug [[ https://bugs.llvm.org/show_bug.cgi?id=42204 | 42204 ]]. llvm-strip now warns when the same input file is used more than once, and errors when stdin is used more than once. Reviewers: jhenderson, rupprecht, espindola, alexshap Reviewed By: jhenderson, rupprecht Subscribers: emaste, arichardson, jakehehrlich, MaskRay, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D63122 llvm-svn: 363638 --- .../llvm-objcopy/ELF/same-file-strip.test | 26 +++++++++++++++++++ llvm/tools/llvm-objcopy/CopyConfig.cpp | 16 +++++++++++- llvm/tools/llvm-objcopy/CopyConfig.h | 7 +++-- llvm/tools/llvm-objcopy/llvm-objcopy.cpp | 8 +++++- 4 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 llvm/test/tools/llvm-objcopy/ELF/same-file-strip.test diff --git a/llvm/test/tools/llvm-objcopy/ELF/same-file-strip.test b/llvm/test/tools/llvm-objcopy/ELF/same-file-strip.test new file mode 100644 index 0000000000000..304ecaf0e1fad --- /dev/null +++ b/llvm/test/tools/llvm-objcopy/ELF/same-file-strip.test @@ -0,0 +1,26 @@ +## Test llvm-strip using the same input file more than once. +## When using stdin ('-') more than once llvm-strip should give an error +## while a file more than once should be simply a warning. + +# RUN: yaml2obj %s -o %t + +# RUN: not llvm-strip - - < %t 2>&1 | FileCheck -check-prefix=ERR %s +# RUN: not llvm-strip - %t - < %t 2>&1 | FileCheck -check-prefix=ERR %s + +# ERR: error: cannot specify '-' as an input file more than once + +# RUN: llvm-strip %t %t 2>&1 | FileCheck -check-prefix=WARN %s -DFILE=%t +# RUN: llvm-strip %t %t %t 2>&1 | FileCheck -check-prefix=WARN %s -DFILE=%t + +# WARN: warning: '[[FILE]]' was already specified + +--- !ELF +FileHeader: + Class: ELFCLASS64 + Data: ELFDATA2LSB + Type: ET_REL + Machine: EM_X86_64 +Sections: + - Name: .alloc + Type: SHT_PROGBITS + Flags: [ SHF_ALLOC ] diff --git a/llvm/tools/llvm-objcopy/CopyConfig.cpp b/llvm/tools/llvm-objcopy/CopyConfig.cpp index 577e9644dc929..a654d8713aa4d 100644 --- a/llvm/tools/llvm-objcopy/CopyConfig.cpp +++ b/llvm/tools/llvm-objcopy/CopyConfig.cpp @@ -11,6 +11,7 @@ #include "llvm/ADT/Optional.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" +#include "llvm/ADT/StringSet.h" #include "llvm/Option/Arg.h" #include "llvm/Option/ArgList.h" #include "llvm/Support/CommandLine.h" @@ -722,7 +723,9 @@ Expected parseObjcopyOptions(ArrayRef ArgsArr) { // ParseStripOptions returns the config and sets the input arguments. If a // help flag is set then ParseStripOptions will print the help messege and // exit. -Expected parseStripOptions(ArrayRef ArgsArr) { +Expected +parseStripOptions(ArrayRef ArgsArr, + std::function ErrorCallback) { StripOptTable T; unsigned MissingArgumentIndex, MissingArgumentCount; llvm::opt::InputArgList InputArgs = @@ -809,7 +812,18 @@ Expected parseStripOptions(ArrayRef ArgsArr) { InputArgs.getLastArgValue(STRIP_output, Positional[0]); DC.CopyConfigs.push_back(std::move(Config)); } else { + StringMap InputFiles; for (StringRef Filename : Positional) { + if (InputFiles[Filename]++ == 1) { + if (Filename == "-") + return createStringError( + errc::invalid_argument, + "cannot specify '-' as an input file more than once"); + if (Error E = ErrorCallback(createStringError( + errc::invalid_argument, "'%s' was already specified", + Filename.str().c_str()))) + return std::move(E); + } Config.InputFilename = Filename; Config.OutputFilename = Filename; DC.CopyConfigs.push_back(Config); diff --git a/llvm/tools/llvm-objcopy/CopyConfig.h b/llvm/tools/llvm-objcopy/CopyConfig.h index 06b3efddb5adf..9ae4270f4fe56 100644 --- a/llvm/tools/llvm-objcopy/CopyConfig.h +++ b/llvm/tools/llvm-objcopy/CopyConfig.h @@ -188,8 +188,11 @@ Expected parseObjcopyOptions(ArrayRef ArgsArr); // ParseStripOptions returns the config and sets the input arguments. If a // help flag is set then ParseStripOptions will print the help messege and -// exit. -Expected parseStripOptions(ArrayRef ArgsArr); +// exit. ErrorCallback is used to handle recoverable errors. An Error returned +// by the callback aborts the parsing and is then returned by this function. +Expected +parseStripOptions(ArrayRef ArgsArr, + std::function ErrorCallback); } // namespace objcopy } // namespace llvm diff --git a/llvm/tools/llvm-objcopy/llvm-objcopy.cpp b/llvm/tools/llvm-objcopy/llvm-objcopy.cpp index 416b295ab3ed5..2ab77ea5c8684 100644 --- a/llvm/tools/llvm-objcopy/llvm-objcopy.cpp +++ b/llvm/tools/llvm-objcopy/llvm-objcopy.cpp @@ -82,6 +82,12 @@ LLVM_ATTRIBUTE_NORETURN void reportError(StringRef File, Error E) { exit(1); } +ErrorSuccess reportWarning(Error E) { + assert(E); + WithColor::warning(errs(), ToolName) << toString(std::move(E)); + return Error::success(); +} + } // end namespace objcopy } // end namespace llvm @@ -263,7 +269,7 @@ int main(int argc, char **argv) { ToolName = argv[0]; bool IsStrip = sys::path::stem(ToolName).contains("strip"); Expected DriverConfig = - IsStrip ? parseStripOptions(makeArrayRef(argv + 1, argc)) + IsStrip ? parseStripOptions(makeArrayRef(argv + 1, argc), reportWarning) : parseObjcopyOptions(makeArrayRef(argv + 1, argc)); if (!DriverConfig) { logAllUnhandledErrors(DriverConfig.takeError(),