From 0e73a637c0bf671d950c435dc7afab3c7be26de6 Mon Sep 17 00:00:00 2001 From: Amr Hesham Date: Sat, 8 Feb 2025 14:14:16 +0100 Subject: [PATCH] [llvm-objcopy] Fix prints wrong path when dump-section output path doesn't exist (#125345) Fix printing the correct file path in the error message when the output file specified by `--dump-section` cannot be opened Fixes: #125113 on ELF, MachO, Wasm --- llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp | 59 ++++++++++--------- llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp | 27 +++++---- llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp | 15 ++--- .../tools/llvm-objcopy/ELF/dump-section.test | 4 ++ .../llvm-objcopy/MachO/dump-section.test | 4 ++ .../tools/llvm-objcopy/wasm/dump-section.test | 4 ++ 6 files changed, 64 insertions(+), 49 deletions(-) diff --git a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp index 5aa0079f3fbc7..9c78f7433ad33 100644 --- a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp +++ b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp @@ -186,27 +186,28 @@ static std::unique_ptr createWriter(const CommonConfig &Config, } static Error dumpSectionToFile(StringRef SecName, StringRef Filename, - Object &Obj) { + StringRef InputFilename, Object &Obj) { for (auto &Sec : Obj.sections()) { if (Sec.Name == SecName) { if (Sec.Type == SHT_NOBITS) - return createStringError(object_error::parse_failed, - "cannot dump section '%s': it has no contents", - SecName.str().c_str()); + return createFileError(InputFilename, object_error::parse_failed, + "cannot dump section '%s': it has no contents", + SecName.str().c_str()); Expected> BufferOrErr = FileOutputBuffer::create(Filename, Sec.OriginalData.size()); if (!BufferOrErr) - return BufferOrErr.takeError(); + return createFileError(Filename, BufferOrErr.takeError()); std::unique_ptr Buf = std::move(*BufferOrErr); std::copy(Sec.OriginalData.begin(), Sec.OriginalData.end(), Buf->getBufferStart()); if (Error E = Buf->commit()) - return E; + return createFileError(Filename, std::move(E)); return Error::success(); } } - return createStringError(object_error::parse_failed, "section '%s' not found", - SecName.str().c_str()); + + return createFileError(InputFilename, object_error::parse_failed, + "section '%s' not found", SecName.str().c_str()); } Error Object::compressOrDecompressSections(const CommonConfig &Config) { @@ -798,7 +799,8 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, StringRef SectionName; StringRef FileName; std::tie(SectionName, FileName) = Flag.split('='); - if (Error E = dumpSectionToFile(SectionName, FileName, Obj)) + if (Error E = + dumpSectionToFile(SectionName, FileName, Config.InputFilename, Obj)) return E; } @@ -807,10 +809,10 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, // us to avoid reporting the inappropriate errors about removing symbols // named in relocations. if (Error E = replaceAndRemoveSections(Config, ELFConfig, Obj)) - return E; + return createFileError(Config.InputFilename, std::move(E)); if (Error E = updateAndRemoveSymbols(Config, ELFConfig, Obj)) - return E; + return createFileError(Config.InputFilename, std::move(E)); if (!Config.SetSectionAlignment.empty()) { for (SectionBase &Sec : Obj.sections()) { @@ -826,8 +828,8 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, if (Config.ChangeSectionLMAValAll > 0 && Seg.PAddr > std::numeric_limits::max() - Config.ChangeSectionLMAValAll) { - return createStringError( - errc::invalid_argument, + return createFileError( + Config.InputFilename, errc::invalid_argument, "address 0x" + Twine::utohexstr(Seg.PAddr) + " cannot be increased by 0x" + Twine::utohexstr(Config.ChangeSectionLMAValAll) + @@ -835,8 +837,8 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, } else if (Config.ChangeSectionLMAValAll < 0 && Seg.PAddr < std::numeric_limits::min() - Config.ChangeSectionLMAValAll) { - return createStringError( - errc::invalid_argument, + return createFileError( + Config.InputFilename, errc::invalid_argument, "address 0x" + Twine::utohexstr(Seg.PAddr) + " cannot be decreased by 0x" + Twine::utohexstr(std::abs(Config.ChangeSectionLMAValAll)) + @@ -849,10 +851,9 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, if (!Config.ChangeSectionAddress.empty()) { if (Obj.Type != ELF::ET_REL) - return createStringError( - object_error::invalid_file_type, + return createFileError( + Config.InputFilename, object_error::invalid_file_type, "cannot change section address in a non-relocatable file"); - StringMap SectionsToUpdateAddress; for (const SectionPatternAddressUpdate &PatternUpdate : make_range(Config.ChangeSectionAddress.rbegin(), @@ -863,8 +864,8 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, .second) { if (PatternUpdate.Update.Kind == AdjustKind::Subtract && Sec.Addr < PatternUpdate.Update.Value) { - return createStringError( - errc::invalid_argument, + return createFileError( + Config.InputFilename, errc::invalid_argument, "address 0x" + Twine::utohexstr(Sec.Addr) + " cannot be decreased by 0x" + Twine::utohexstr(PatternUpdate.Update.Value) + @@ -873,8 +874,8 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, if (PatternUpdate.Update.Kind == AdjustKind::Add && Sec.Addr > std::numeric_limits::max() - PatternUpdate.Update.Value) { - return createStringError( - errc::invalid_argument, + return createFileError( + Config.InputFilename, errc::invalid_argument, "address 0x" + Twine::utohexstr(Sec.Addr) + " cannot be increased by 0x" + Twine::utohexstr(PatternUpdate.Update.Value) + @@ -909,7 +910,7 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, if (!ELFConfig.NotesToRemove.empty()) { if (Error Err = removeNotes(Obj, E, ELFConfig.NotesToRemove, Config.ErrorCallback)) - return Err; + return createFileError(Config.InputFilename, std::move(Err)); } for (const NewSectionInfo &AddedSection : Config.AddSection) { @@ -924,7 +925,7 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, return Error::success(); }; if (Error E = handleUserSection(AddedSection, AddSection)) - return E; + return createFileError(Config.InputFilename, std::move(E)); } for (const NewSectionInfo &NewSection : Config.UpdateSection) { @@ -932,7 +933,7 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, return Obj.updateSection(Name, Data); }; if (Error E = handleUserSection(NewSection, UpdateSection)) - return E; + return createFileError(Config.InputFilename, std::move(E)); } if (!Config.AddGnuDebugLink.empty()) @@ -943,7 +944,7 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, // before adding new symbols. if (!Obj.SymbolTable && !Config.SymbolsToAdd.empty()) if (Error E = Obj.addNewSymbolTable()) - return E; + return createFileError(Config.InputFilename, std::move(E)); for (const NewSymbolInfo &SI : Config.SymbolsToAdd) addSymbol(Obj, SI, ELFConfig.NewSymbolVisibility); @@ -955,7 +956,7 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, if (Iter != Config.SetSectionFlags.end()) { const SectionFlagsUpdate &SFU = Iter->second; if (Error E = setSectionFlagsAndType(Sec, SFU.NewFlags, Obj.Machine)) - return E; + return createFileError(Config.InputFilename, std::move(E)); } auto It2 = Config.SetSectionType.find(Sec.Name); if (It2 != Config.SetSectionType.end()) @@ -974,7 +975,7 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, Sec.Name = std::string(SR.NewName); if (SR.NewFlags) { if (Error E = setSectionFlagsAndType(Sec, *SR.NewFlags, Obj.Machine)) - return E; + return createFileError(Config.InputFilename, std::move(E)); } RenamedSections.insert(&Sec); } else if (RelocSec && !(Sec.Flags & SHF_ALLOC)) @@ -1091,7 +1092,7 @@ Error objcopy::elf::executeObjcopyOnBinary(const CommonConfig &Config, : getOutputElfType(In); if (Error E = handleArgs(Config, ELFConfig, OutputElfType, **Obj)) - return createFileError(Config.InputFilename, std::move(E)); + return E; if (Error E = writeOutput(Config, **Obj, Out, OutputElfType)) return createFileError(Config.InputFilename, std::move(E)); diff --git a/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp b/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp index a188425b283fa..682edffc84f34 100644 --- a/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp +++ b/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp @@ -306,25 +306,25 @@ static Error processLoadCommands(const MachOConfig &MachOConfig, Object &Obj) { } static Error dumpSectionToFile(StringRef SecName, StringRef Filename, - Object &Obj) { + StringRef InputFilename, Object &Obj) { for (LoadCommand &LC : Obj.LoadCommands) for (const std::unique_ptr
&Sec : LC.Sections) { if (Sec->CanonicalName == SecName) { Expected> BufferOrErr = FileOutputBuffer::create(Filename, Sec->Content.size()); if (!BufferOrErr) - return BufferOrErr.takeError(); + return createFileError(Filename, BufferOrErr.takeError()); std::unique_ptr Buf = std::move(*BufferOrErr); llvm::copy(Sec->Content, Buf->getBufferStart()); if (Error E = Buf->commit()) - return E; + return createFileError(Filename, std::move(E)); return Error::success(); } } - return createStringError(object_error::parse_failed, "section '%s' not found", - SecName.str().c_str()); + return createFileError(InputFilename, object_error::parse_failed, + "section '%s' not found", SecName.str().c_str()); } static Error addSection(const NewSectionInfo &NewSection, Object &Obj) { @@ -426,12 +426,13 @@ static Error handleArgs(const CommonConfig &Config, StringRef SectionName; StringRef FileName; std::tie(SectionName, FileName) = Flag.split('='); - if (Error E = dumpSectionToFile(SectionName, FileName, Obj)) + if (Error E = + dumpSectionToFile(SectionName, FileName, Config.InputFilename, Obj)) return E; } if (Error E = removeSections(Config, Obj)) - return E; + return createFileError(Config.InputFilename, std::move(E)); // Mark symbols to determine which symbols are still needed. if (Config.StripAll) @@ -446,20 +447,20 @@ static Error handleArgs(const CommonConfig &Config, for (const NewSectionInfo &NewSection : Config.AddSection) { if (Error E = isValidMachOCannonicalName(NewSection.SectionName)) - return E; + return createFileError(Config.InputFilename, std::move(E)); if (Error E = addSection(NewSection, Obj)) - return E; + return createFileError(Config.InputFilename, std::move(E)); } for (const NewSectionInfo &NewSection : Config.UpdateSection) { if (Error E = isValidMachOCannonicalName(NewSection.SectionName)) - return E; + return createFileError(Config.InputFilename, std::move(E)); if (Error E = updateSection(NewSection, Obj)) - return E; + return createFileError(Config.InputFilename, std::move(E)); } if (Error E = processLoadCommands(MachOConfig, Obj)) - return E; + return createFileError(Config.InputFilename, std::move(E)); return Error::success(); } @@ -479,7 +480,7 @@ Error objcopy::macho::executeObjcopyOnBinary(const CommonConfig &Config, Config.InputFilename.str().c_str()); if (Error E = handleArgs(Config, MachOConfig, **O)) - return createFileError(Config.InputFilename, std::move(E)); + return E; // Page size used for alignment of segment sizes in Mach-O executables and // dynamic libraries. diff --git a/llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp b/llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp index cf3d884bee3bd..57fd0f5ad233c 100644 --- a/llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp +++ b/llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp @@ -38,23 +38,23 @@ static bool isCommentSection(const Section &Sec) { } static Error dumpSectionToFile(StringRef SecName, StringRef Filename, - Object &Obj) { + StringRef InputFilename, Object &Obj) { for (const Section &Sec : Obj.Sections) { if (Sec.Name == SecName) { ArrayRef Contents = Sec.Contents; Expected> BufferOrErr = FileOutputBuffer::create(Filename, Contents.size()); if (!BufferOrErr) - return BufferOrErr.takeError(); + return createFileError(Filename, BufferOrErr.takeError()); std::unique_ptr Buf = std::move(*BufferOrErr); std::copy(Contents.begin(), Contents.end(), Buf->getBufferStart()); if (Error E = Buf->commit()) - return E; + return createFileError(Filename, std::move(E)); return Error::success(); } } - return createStringError(errc::invalid_argument, "section '%s' not found", - SecName.str().c_str()); + return createFileError(Filename, errc::invalid_argument, + "section '%s' not found", SecName.str().c_str()); } static void removeSections(const CommonConfig &Config, Object &Obj) { @@ -115,8 +115,9 @@ static Error handleArgs(const CommonConfig &Config, Object &Obj) { StringRef SecName; StringRef FileName; std::tie(SecName, FileName) = Flag.split("="); - if (Error E = dumpSectionToFile(SecName, FileName, Obj)) - return createFileError(FileName, std::move(E)); + if (Error E = + dumpSectionToFile(SecName, FileName, Config.InputFilename, Obj)) + return E; } removeSections(Config, Obj); diff --git a/llvm/test/tools/llvm-objcopy/ELF/dump-section.test b/llvm/test/tools/llvm-objcopy/ELF/dump-section.test index 037ec86090e55..2dbbcc0ca568e 100644 --- a/llvm/test/tools/llvm-objcopy/ELF/dump-section.test +++ b/llvm/test/tools/llvm-objcopy/ELF/dump-section.test @@ -64,3 +64,7 @@ ProgramHeaders: # RUN: not llvm-objcopy --dump-section .text= %t /dev/null 2>&1 | FileCheck %s --check-prefix=ERR2 # ERR2: error: bad format for --dump-section, expected section=file + +# RUN: not llvm-objcopy --dump-section .text=not_exists/text-section %t 2>&1 \ +# RUN: | FileCheck -DMSG=%errc_ENOENT %s -DINPUT=%t --check-prefix=NO-SUCH-PATH +# NO-SUCH-PATH: error: 'not_exists/text-section': [[MSG]] diff --git a/llvm/test/tools/llvm-objcopy/MachO/dump-section.test b/llvm/test/tools/llvm-objcopy/MachO/dump-section.test index 9a1227cdbbda1..d54a50b557bb7 100644 --- a/llvm/test/tools/llvm-objcopy/MachO/dump-section.test +++ b/llvm/test/tools/llvm-objcopy/MachO/dump-section.test @@ -21,6 +21,10 @@ # RUN: | FileCheck %s -DINPUT=%t --check-prefix=NO-SUCH-SECTION # NO-SUCH-SECTION: error: '[[INPUT]]': section '__TEXT,__foo' not found +# RUN: not llvm-objcopy --dump-section __TEXT,__text=not_exists/text-section %t 2>&1 \ +# RUN: | FileCheck -DMSG=%errc_ENOENT %s -DINPUT=%t --check-prefix=NO-SUCH-PATH +# NO-SUCH-PATH: error: 'not_exists/text-section': [[MSG]] + --- !mach-o FileHeader: magic: 0xFEEDFACF diff --git a/llvm/test/tools/llvm-objcopy/wasm/dump-section.test b/llvm/test/tools/llvm-objcopy/wasm/dump-section.test index 983a581e03fe2..2d1533f06df10 100644 --- a/llvm/test/tools/llvm-objcopy/wasm/dump-section.test +++ b/llvm/test/tools/llvm-objcopy/wasm/dump-section.test @@ -28,6 +28,10 @@ # REMOVED-NOT: producers +# RUN: not llvm-objcopy --dump-section producers=not_exists/text-section %t 2>&1 \ +# RUN: | FileCheck -DMSG=%errc_ENOENT %s -DINPUT=%t --check-prefix=NO-SUCH-PATH +# NO-SUCH-PATH: error: 'not_exists/text-section': [[MSG]] + --- !WASM FileHeader: Version: 0x00000001