From 4ee933c76bf2a81e7d843c878ac80e8d67eaa535 Mon Sep 17 00:00:00 2001 From: Rumeet Dhindsa Date: Wed, 26 Jun 2019 03:00:57 +0000 Subject: Revert [llvm-objcopy][NFC] Refactor output target parsing This reverts r364254 (git commit 545f001d1b9a7b58a68d75e70bfc36c841de8999) This change causes some llvm-obcopy tests to fail with valgrind. Following is the output for basic-keep.test Command Output (stderr): -- ==107406== Conditional jump or move depends on uninitialised value(s) ==107406== at 0x1A30DD: executeObjcopy(llvm::objcopy::CopyConfig const&) (llvm-objcopy.cpp:235) ==107406== by 0x1A3935: main (llvm-objcopy.cpp:294) llvm-svn: 364379 --- llvm/tools/llvm-objcopy/CopyConfig.cpp | 67 +++++++++--------------------- llvm/tools/llvm-objcopy/CopyConfig.h | 11 +---- llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp | 14 +++---- llvm/tools/llvm-objcopy/llvm-objcopy.cpp | 30 ++++--------- 4 files changed, 36 insertions(+), 86 deletions(-) (limited to 'llvm/tools') diff --git a/llvm/tools/llvm-objcopy/CopyConfig.cpp b/llvm/tools/llvm-objcopy/CopyConfig.cpp index a00962287277..a654d8713aa4 100644 --- a/llvm/tools/llvm-objcopy/CopyConfig.cpp +++ b/llvm/tools/llvm-objcopy/CopyConfig.cpp @@ -277,13 +277,8 @@ 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 TargetMap{ +static const StringMap OutputFormatMap{ // Name, {EMachine, 64bit, LittleEndian} // x86 {"elf32-i386", {ELF::EM_386, false, true}}, @@ -317,28 +312,18 @@ static const StringMap TargetMap{ {"elf32-sparcel", {ELF::EM_SPARC, false, true}}, }; -static Expected -getOutputTargetInfoByTargetName(StringRef TargetName) { - StringRef OriginalTargetName = TargetName; - bool IsFreeBSD = TargetName.consume_back("-freebsd"); - auto Iter = TargetMap.find(TargetName); - if (Iter == std::end(TargetMap)) +static Expected getOutputFormatMachineInfo(StringRef Format) { + StringRef OriginalFormat = Format; + bool IsFreeBSD = Format.consume_back("-freebsd"); + auto Iter = OutputFormatMap.find(Format); + if (Iter == std::end(OutputFormatMap)) return createStringError(errc::invalid_argument, "invalid output format: '%s'", - OriginalTargetName.str().c_str()); + OriginalFormat.str().c_str()); MachineInfo MI = Iter->getValue(); if (IsFreeBSD) MI.OSABI = ELF::ELFOSABI_FREEBSD; - - 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}}; + return {MI}; } static Error addSymbolsFromFile(std::vector &Symbols, @@ -460,23 +445,14 @@ 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)) { - InputFormat = InputArgs.getLastArgValue(OBJCOPY_target); - OutputFormat = InputArgs.getLastArgValue(OBJCOPY_target); + Config.InputFormat = InputArgs.getLastArgValue(OBJCOPY_target); + Config.OutputFormat = InputArgs.getLastArgValue(OBJCOPY_target); } else { - InputFormat = InputArgs.getLastArgValue(OBJCOPY_input_target); - OutputFormat = InputArgs.getLastArgValue(OBJCOPY_output_target); + Config.InputFormat = InputArgs.getLastArgValue(OBJCOPY_input_target); + Config.OutputFormat = InputArgs.getLastArgValue(OBJCOPY_output_target); } - - // 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) { + if (Config.InputFormat == "binary") { auto BinaryArch = InputArgs.getLastArgValue(OBJCOPY_binary_architecture); if (BinaryArch.empty()) return createStringError( @@ -487,17 +463,12 @@ Expected parseObjcopyOptions(ArrayRef ArgsArr) { return MI.takeError(); Config.BinaryArch = *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 (!Config.OutputFormat.empty() && Config.OutputFormat != "binary" && + Config.OutputFormat != "ihex") { + Expected MI = getOutputFormatMachineInfo(Config.OutputFormat); + if (!MI) + return MI.takeError(); + Config.OutputArch = *MI; } if (auto Arg = InputArgs.getLastArg(OBJCOPY_compress_debug_sections, diff --git a/llvm/tools/llvm-objcopy/CopyConfig.h b/llvm/tools/llvm-objcopy/CopyConfig.h index aff3631a487c..9ae4270f4fe5 100644 --- a/llvm/tools/llvm-objcopy/CopyConfig.h +++ b/llvm/tools/llvm-objcopy/CopyConfig.h @@ -26,13 +26,6 @@ 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. @@ -111,9 +104,9 @@ struct NewSymbolInfo { struct CopyConfig { // Main input/output options StringRef InputFilename; - FileFormat InputFormat; + StringRef InputFormat; StringRef OutputFilename; - FileFormat OutputFormat; + StringRef OutputFormat; // Only applicable for --input-format=binary MachineInfo BinaryArch; diff --git a/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp b/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp index b366c6e55987..b6af729afe81 100644 --- a/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp +++ b/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp @@ -154,14 +154,12 @@ static std::unique_ptr createELFWriter(const CopyConfig &Config, static std::unique_ptr createWriter(const CopyConfig &Config, Object &Obj, Buffer &Buf, ElfType 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); - } + 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); })(); } template diff --git a/llvm/tools/llvm-objcopy/llvm-objcopy.cpp b/llvm/tools/llvm-objcopy/llvm-objcopy.cpp index 44e0220a7d37..2ab77ea5c868 100644 --- a/llvm/tools/llvm-objcopy/llvm-objcopy.cpp +++ b/llvm/tools/llvm-objcopy/llvm-objcopy.cpp @@ -140,16 +140,11 @@ 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) { - 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); - } + // 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); } /// The function executeObjcopyOnBinary does the dispatch based on the format @@ -229,17 +224,10 @@ static Error executeObjcopy(const CopyConfig &Config) { return createFileError(Config.InputFilename, EC); typedef Error (*ProcessRawFn)(const CopyConfig &, MemoryBuffer &, Buffer &); - ProcessRawFn ProcessRaw; - switch (Config.InputFormat) { - case FileFormat::Binary: - ProcessRaw = executeObjcopyOnRawBinary; - break; - case FileFormat::IHex: - ProcessRaw = executeObjcopyOnIHex; - break; - default: - ProcessRaw = nullptr; - } + auto ProcessRaw = StringSwitch(Config.InputFormat) + .Case("binary", executeObjcopyOnRawBinary) + .Case("ihex", executeObjcopyOnIHex) + .Default(nullptr); if (ProcessRaw) { auto BufOrErr = MemoryBuffer::getFileOrSTDIN(Config.InputFilename); -- cgit v1.2.3-65-gdbad