From: Seiya Nuta Date: Fri, 5 Jul 2019 05:28:38 +0000 (+0000) Subject: [llvm-objcopy][NFC] Refactor output target parsing v2 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1025e4c956bee6549cb86459bba4195ee87d198b;p=llvm [llvm-objcopy][NFC] Refactor output target parsing v2 Summary: Use an enum instead of string to hold the output file format in Config.InputFormat and Config.OutputFormat. It's essential to support other output file formats other than ELF. This patch originally has been submitted as D63239. However, there was an use-of-uninitialized-value bug and reverted in r364379 (git commit 4ee933c). This patch includes the fix for the bug by setting Config.InputFormat/Config.OutputFormat in parseStripOptions. Reviewers: espindola, alexshap, rupprecht, jhenderson Reviewed By: jhenderson Subscribers: emaste, arichardson, jakehehrlich, MaskRay, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D64170 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@365173 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/tools/llvm-objcopy/CopyConfig.cpp b/tools/llvm-objcopy/CopyConfig.cpp index a654d8713aa..8d6431b3044 100644 --- a/tools/llvm-objcopy/CopyConfig.cpp +++ b/tools/llvm-objcopy/CopyConfig.cpp @@ -277,8 +277,13 @@ static Expected getMachineInfo(StringRef Arch) { return Iter->getValue(); } +struct TargetInfo { + FileFormat Format; + MachineInfo Machine; +}; + // FIXME: consolidate with the bfd parsing used by lld. -static const StringMap OutputFormatMap{ +static const StringMap TargetMap{ // Name, {EMachine, 64bit, LittleEndian} // x86 {"elf32-i386", {ELF::EM_386, false, true}}, @@ -312,18 +317,28 @@ static const StringMap OutputFormatMap{ {"elf32-sparcel", {ELF::EM_SPARC, false, true}}, }; -static Expected getOutputFormatMachineInfo(StringRef Format) { - StringRef OriginalFormat = Format; - bool IsFreeBSD = Format.consume_back("-freebsd"); - auto Iter = OutputFormatMap.find(Format); - if (Iter == std::end(OutputFormatMap)) +static Expected +getOutputTargetInfoByTargetName(StringRef TargetName) { + StringRef OriginalTargetName = TargetName; + bool IsFreeBSD = TargetName.consume_back("-freebsd"); + auto Iter = TargetMap.find(TargetName); + if (Iter == std::end(TargetMap)) return createStringError(errc::invalid_argument, "invalid output format: '%s'", - OriginalFormat.str().c_str()); + OriginalTargetName.str().c_str()); MachineInfo MI = Iter->getValue(); if (IsFreeBSD) MI.OSABI = ELF::ELFOSABI_FREEBSD; - return {MI}; + + FileFormat Format; + if (TargetName.startswith("elf")) + Format = FileFormat::ELF; + else + // This should never happen because `TargetName` is valid (it certainly + // exists in the TargetMap). + llvm_unreachable("unknown target prefix"); + + return {TargetInfo{Format, MI}}; } static Error addSymbolsFromFile(std::vector &Symbols, @@ -445,14 +460,23 @@ Expected parseObjcopyOptions(ArrayRef ArgsArr) { "--target cannot be used with --input-target or --output-target"); bool UseRegex = InputArgs.hasArg(OBJCOPY_regex); + StringRef InputFormat, OutputFormat; if (InputArgs.hasArg(OBJCOPY_target)) { - Config.InputFormat = InputArgs.getLastArgValue(OBJCOPY_target); - Config.OutputFormat = InputArgs.getLastArgValue(OBJCOPY_target); + InputFormat = InputArgs.getLastArgValue(OBJCOPY_target); + OutputFormat = InputArgs.getLastArgValue(OBJCOPY_target); } else { - Config.InputFormat = InputArgs.getLastArgValue(OBJCOPY_input_target); - Config.OutputFormat = InputArgs.getLastArgValue(OBJCOPY_output_target); + InputFormat = InputArgs.getLastArgValue(OBJCOPY_input_target); + OutputFormat = InputArgs.getLastArgValue(OBJCOPY_output_target); } - if (Config.InputFormat == "binary") { + + // FIXME: Currently, we ignore the target for non-binary/ihex formats + // explicitly specified by -I option (e.g. -Ielf32-x86-64) and guess the + // format by llvm::object::createBinary regardless of the option value. + Config.InputFormat = StringSwitch(InputFormat) + .Case("binary", FileFormat::Binary) + .Case("ihex", FileFormat::IHex) + .Default(FileFormat::Unspecified); + if (Config.InputFormat == FileFormat::Binary) { auto BinaryArch = InputArgs.getLastArgValue(OBJCOPY_binary_architecture); if (BinaryArch.empty()) return createStringError( @@ -463,12 +487,17 @@ Expected parseObjcopyOptions(ArrayRef ArgsArr) { return MI.takeError(); Config.BinaryArch = *MI; } - if (!Config.OutputFormat.empty() && Config.OutputFormat != "binary" && - Config.OutputFormat != "ihex") { - Expected MI = getOutputFormatMachineInfo(Config.OutputFormat); - if (!MI) - return MI.takeError(); - Config.OutputArch = *MI; + + Config.OutputFormat = StringSwitch(OutputFormat) + .Case("binary", FileFormat::Binary) + .Case("ihex", FileFormat::IHex) + .Default(FileFormat::Unspecified); + if (Config.OutputFormat == FileFormat::Unspecified && !OutputFormat.empty()) { + Expected Target = getOutputTargetInfoByTargetName(OutputFormat); + if (!Target) + return Target.takeError(); + Config.OutputFormat = Target->Format; + Config.OutputArch = Target->Machine; } if (auto Arg = InputArgs.getLastArg(OBJCOPY_compress_debug_sections, @@ -804,6 +833,8 @@ parseStripOptions(ArrayRef ArgsArr, STRIP_disable_deterministic_archives, /*default=*/true); Config.PreserveDates = InputArgs.hasArg(STRIP_preserve_dates); + Config.InputFormat = FileFormat::Unspecified; + Config.OutputFormat = FileFormat::Unspecified; DriverConfig DC; if (Positional.size() == 1) { diff --git a/tools/llvm-objcopy/CopyConfig.h b/tools/llvm-objcopy/CopyConfig.h index 9ae4270f4fe..aff3631a487 100644 --- a/tools/llvm-objcopy/CopyConfig.h +++ b/tools/llvm-objcopy/CopyConfig.h @@ -26,6 +26,13 @@ namespace llvm { namespace objcopy { +enum class FileFormat { + Unspecified, + ELF, + Binary, + IHex, +}; + // This type keeps track of the machine info for various architectures. This // lets us map architecture names to ELF types and the e_machine value of the // ELF file. @@ -104,9 +111,9 @@ struct NewSymbolInfo { struct CopyConfig { // Main input/output options StringRef InputFilename; - StringRef InputFormat; + FileFormat InputFormat; StringRef OutputFilename; - StringRef OutputFormat; + FileFormat OutputFormat; // Only applicable for --input-format=binary MachineInfo BinaryArch; diff --git a/tools/llvm-objcopy/ELF/ELFObjcopy.cpp b/tools/llvm-objcopy/ELF/ELFObjcopy.cpp index b6af729afe8..b366c6e5598 100644 --- a/tools/llvm-objcopy/ELF/ELFObjcopy.cpp +++ b/tools/llvm-objcopy/ELF/ELFObjcopy.cpp @@ -154,12 +154,14 @@ static std::unique_ptr createELFWriter(const CopyConfig &Config, static std::unique_ptr createWriter(const CopyConfig &Config, Object &Obj, Buffer &Buf, ElfType OutputElfType) { - using Functor = std::function()>; - return StringSwitch(Config.OutputFormat) - .Case("binary", [&] { return llvm::make_unique(Obj, Buf); }) - .Case("ihex", [&] { return llvm::make_unique(Obj, Buf); }) - .Default( - [&] { return createELFWriter(Config, Obj, Buf, OutputElfType); })(); + switch (Config.OutputFormat) { + case FileFormat::Binary: + return llvm::make_unique(Obj, Buf); + case FileFormat::IHex: + return llvm::make_unique(Obj, Buf); + default: + return createELFWriter(Config, Obj, Buf, OutputElfType); + } } template diff --git a/tools/llvm-objcopy/llvm-objcopy.cpp b/tools/llvm-objcopy/llvm-objcopy.cpp index d04b1cf462f..2b0fef117f8 100644 --- a/tools/llvm-objcopy/llvm-objcopy.cpp +++ b/tools/llvm-objcopy/llvm-objcopy.cpp @@ -140,11 +140,18 @@ static Error executeObjcopyOnIHex(const CopyConfig &Config, MemoryBuffer &In, /// of the output specified by the command line options. static Error executeObjcopyOnRawBinary(const CopyConfig &Config, MemoryBuffer &In, Buffer &Out) { - // TODO: llvm-objcopy should parse CopyConfig.OutputFormat to recognize - // formats other than ELF / "binary" and invoke - // elf::executeObjcopyOnRawBinary, macho::executeObjcopyOnRawBinary or - // coff::executeObjcopyOnRawBinary accordingly. - return elf::executeObjcopyOnRawBinary(Config, In, Out); + switch (Config.OutputFormat) { + case FileFormat::ELF: + // FIXME: Currently, we call elf::executeObjcopyOnRawBinary even if the + // output format is binary/ihex or it's not given. This behavior differs from + // GNU objcopy. See https://bugs.llvm.org/show_bug.cgi?id=42171 for details. + case FileFormat::Binary: + case FileFormat::IHex: + case FileFormat::Unspecified: + return elf::executeObjcopyOnRawBinary(Config, In, Out); + } + + llvm_unreachable("unsupported output format"); } /// The function executeObjcopyOnBinary does the dispatch based on the format @@ -238,10 +245,17 @@ static Error executeObjcopy(const CopyConfig &Config) { } typedef Error (*ProcessRawFn)(const CopyConfig &, MemoryBuffer &, Buffer &); - auto ProcessRaw = StringSwitch(Config.InputFormat) - .Case("binary", executeObjcopyOnRawBinary) - .Case("ihex", executeObjcopyOnIHex) - .Default(nullptr); + ProcessRawFn ProcessRaw; + switch (Config.InputFormat) { + case FileFormat::Binary: + ProcessRaw = executeObjcopyOnRawBinary; + break; + case FileFormat::IHex: + ProcessRaw = executeObjcopyOnIHex; + break; + default: + ProcessRaw = nullptr; + } if (ProcessRaw) { auto BufOrErr = MemoryBuffer::getFileOrSTDIN(Config.InputFilename);