From d80b0ee6f8529d98a28664288c959e192a5686ce Mon Sep 17 00:00:00 2001 From: AmrDeveloper Date: Sat, 1 Feb 2025 10:33:33 +0100 Subject: [PATCH 1/8] [llvm-objcopy] Fix prints wrong path when section output path doesn't exist --- llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp | 66 +++++++++++------- llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp | 28 ++++---- llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp | 16 +++-- .../dump-section-directory-not-exists.test | 36 ++++++++++ .../dump-section-directory-not-exists.test | 67 +++++++++++++++++++ .../dump-section-directory-not-exists.test | 24 +++++++ 6 files changed, 191 insertions(+), 46 deletions(-) create mode 100644 llvm/test/tools/llvm-objcopy/ELF/dump-section-directory-not-exists.test create mode 100644 llvm/test/tools/llvm-objcopy/MachO/dump-section-directory-not-exists.test create mode 100644 llvm/test/tools/llvm-objcopy/wasm/dump-section-directory-not-exists.test diff --git a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp index 5aa0079f3fbc7..9d979a7c094d4 100644 --- a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp +++ b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp @@ -186,27 +186,32 @@ 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()); + if (Sec.Type == SHT_NOBITS) { + Error E = + createStringError(object_error::parse_failed, + "cannot dump section '%s': it has no contents", + SecName.str().c_str()); + return createFileError(InputFilename, std::move(E)); + } 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()); + Error E = createStringError(object_error::parse_failed, + "section '%s' not found", SecName.str().c_str()); + + return createFileError(InputFilename, std::move(E)); } Error Object::compressOrDecompressSections(const CommonConfig &Config) { @@ -798,7 +803,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 +813,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,21 +832,24 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, if (Config.ChangeSectionLMAValAll > 0 && Seg.PAddr > std::numeric_limits::max() - Config.ChangeSectionLMAValAll) { - return createStringError( + Error E = createStringError( errc::invalid_argument, "address 0x" + Twine::utohexstr(Seg.PAddr) + " cannot be increased by 0x" + Twine::utohexstr(Config.ChangeSectionLMAValAll) + ". The result would overflow"); + return createFileError(Config.InputFilename, std::move(E)); } else if (Config.ChangeSectionLMAValAll < 0 && Seg.PAddr < std::numeric_limits::min() - Config.ChangeSectionLMAValAll) { - return createStringError( + Error E = createStringError( errc::invalid_argument, "address 0x" + Twine::utohexstr(Seg.PAddr) + " cannot be decreased by 0x" + Twine::utohexstr(std::abs(Config.ChangeSectionLMAValAll)) + ". The result would underflow"); + + return createFileError(Config.InputFilename, std::move(E)); } Seg.PAddr += Config.ChangeSectionLMAValAll; } @@ -848,11 +857,12 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, } if (!Config.ChangeSectionAddress.empty()) { - if (Obj.Type != ELF::ET_REL) - return createStringError( + if (Obj.Type != ELF::ET_REL) { + Error E = createStringError( object_error::invalid_file_type, "cannot change section address in a non-relocatable file"); - + return createFileError(Config.InputFilename, std::move(E)); + } StringMap SectionsToUpdateAddress; for (const SectionPatternAddressUpdate &PatternUpdate : make_range(Config.ChangeSectionAddress.rbegin(), @@ -863,22 +873,26 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, .second) { if (PatternUpdate.Update.Kind == AdjustKind::Subtract && Sec.Addr < PatternUpdate.Update.Value) { - return createStringError( + Error E = createStringError( errc::invalid_argument, "address 0x" + Twine::utohexstr(Sec.Addr) + " cannot be decreased by 0x" + Twine::utohexstr(PatternUpdate.Update.Value) + ". The result would underflow"); + + return createFileError(Config.InputFilename, std::move(E)); } if (PatternUpdate.Update.Kind == AdjustKind::Add && Sec.Addr > std::numeric_limits::max() - PatternUpdate.Update.Value) { - return createStringError( + Error E = createStringError( errc::invalid_argument, "address 0x" + Twine::utohexstr(Sec.Addr) + " cannot be increased by 0x" + Twine::utohexstr(PatternUpdate.Update.Value) + ". The result would overflow"); + + return createFileError(Config.InputFilename, std::move(E)); } switch (PatternUpdate.Update.Kind) { @@ -909,7 +923,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 +938,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 +946,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 +957,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 +969,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 +988,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 +1105,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..49ac903e99573 100644 --- a/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp +++ b/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp @@ -306,25 +306,26 @@ 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()); + Error E = createStringError(object_error::parse_failed, + "section '%s' not found", SecName.str().c_str()); + return createFileError(InputFilename, std::move(E)); } static Error addSection(const NewSectionInfo &NewSection, Object &Obj) { @@ -426,12 +427,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 +448,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 +481,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..3ace39ac558f5 100644 --- a/llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp +++ b/llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp @@ -38,23 +38,24 @@ 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()); + Error E = createStringError(errc::invalid_argument, "section '%s' not found", + SecName.str().c_str()); + return createFileError(Filename, std::move(E)); } static void removeSections(const CommonConfig &Config, Object &Obj) { @@ -115,8 +116,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-directory-not-exists.test b/llvm/test/tools/llvm-objcopy/ELF/dump-section-directory-not-exists.test new file mode 100644 index 0000000000000..462ebd691ae49 --- /dev/null +++ b/llvm/test/tools/llvm-objcopy/ELF/dump-section-directory-not-exists.test @@ -0,0 +1,36 @@ +## Show that llvm-objcopy report that section file path is not exists. + +# RUN: yaml2obj %s -o %t + +# RUN: not llvm-objcopy --dump-section .text=not_exists/text-section %t 2>&1 \ +# RUN: | FileCheck %s -DINPUT=%t --check-prefix=NO-SUCH-PATH +# NO-SUCH-PATH: error: 'not_exists/text-section': No such file or directory + +!ELF +FileHeader: + Class: ELFCLASS64 + Data: ELFDATA2LSB + Type: ET_EXEC + Machine: EM_X86_64 +Sections: + - Name: .text + Type: SHT_PROGBITS + Flags: [ SHF_ALLOC, SHF_EXECINSTR ] + AddressAlign: 0x0000000000001000 + Content: "DEADBEEF" + - Name: .foo + Type: SHT_PROGBITS + Flags: [ SHF_WRITE ] + Content: "CAFE" + - Name: .empty + Type: SHT_PROGBITS + Flags: [ SHF_ALLOC ] + - Name: .bar + Type: SHT_NOBITS + Flags: [ SHF_WRITE ] +ProgramHeaders: + - Type: PT_LOAD + Flags: [ PF_X, PF_R ] + FirstSec: .text + LastSec: .text + diff --git a/llvm/test/tools/llvm-objcopy/MachO/dump-section-directory-not-exists.test b/llvm/test/tools/llvm-objcopy/MachO/dump-section-directory-not-exists.test new file mode 100644 index 0000000000000..ac597b362b4c7 --- /dev/null +++ b/llvm/test/tools/llvm-objcopy/MachO/dump-section-directory-not-exists.test @@ -0,0 +1,67 @@ +## Show that llvm-objcopy report that section file path is not exists. + +# RUN: yaml2obj %s -o %t + +# RUN: not llvm-objcopy --dump-section __TEXT,__text=not_exists/text-section %t 2>&1 \ +# RUN: | FileCheck %s -DINPUT=%t --check-prefix=NO-SUCH-PATH +# NO-SUCH-PATH: error: 'not_exists/text-section': No such file or directory + +--- !mach-o +FileHeader: + magic: 0xFEEDFACF + cputype: 0x01000007 + cpusubtype: 0x00000003 + filetype: 0x00000001 + ncmds: 1 + sizeofcmds: 312 + flags: 0x00002000 + reserved: 0x00000000 +LoadCommands: + - cmd: LC_SEGMENT_64 + cmdsize: 312 + segname: '' + vmaddr: 0 + vmsize: 12 + fileoff: 344 + filesize: 12 + maxprot: 7 + initprot: 7 + nsects: 3 + flags: 0 + Sections: + - sectname: __text + segname: __TEXT + addr: 0x0000000000000000 + content: 'AABBCCDD' + size: 4 + offset: 344 + align: 0 + reloff: 0x00000000 + nreloc: 0 + flags: 0x80000400 + reserved1: 0x00000000 + reserved2: 0x00000000 + - sectname: __data + segname: __DATA + addr: 0x0000000000000004 + content: 'EEFFEEFF' + size: 4 + offset: 348 + align: 0 + reloff: 0x00000000 + nreloc: 0 + flags: 0x00000000 + reserved1: 0x00000000 + reserved2: 0x00000000 + - sectname: __const + segname: __TEXT + addr: 0x0000000000000008 + content: 'EEFFEEFF' + size: 4 + offset: 352 + align: 0 + reloff: 0x00000000 + nreloc: 0 + flags: 0x00000000 + reserved1: 0x00000000 + reserved2: 0x00000000 diff --git a/llvm/test/tools/llvm-objcopy/wasm/dump-section-directory-not-exists.test b/llvm/test/tools/llvm-objcopy/wasm/dump-section-directory-not-exists.test new file mode 100644 index 0000000000000..f0ac8e980cf5d --- /dev/null +++ b/llvm/test/tools/llvm-objcopy/wasm/dump-section-directory-not-exists.test @@ -0,0 +1,24 @@ +## Show that llvm-objcopy report that section file path is not exists. + +# RUN: yaml2obj %s -o %t + +# RUN: not llvm-objcopy --dump-section producers=not_exists/text-section %t 2>&1 \ +# RUN: | FileCheck %s -DINPUT=%t --check-prefix=NO-SUCH-PATH +# NO-SUCH-PATH: error: 'not_exists/text-section': No such file or directory + +--- !WASM +FileHeader: + Version: 0x00000001 +Sections: + - Type: TYPE + Signatures: + - Index: 0 + ParamTypes: + - I32 + ReturnTypes: + - F32 + - Type: CUSTOM + Name: producers + Tools: + - Name: clang + Version: 9.0.0 From 02e66cb9b8540b7c58fa8ad1f5536d4b79590447 Mon Sep 17 00:00:00 2001 From: AmrDeveloper Date: Sat, 1 Feb 2025 17:59:00 +0100 Subject: [PATCH 2/8] Don't check the OS-dependent message --- .../llvm-objcopy/ELF/dump-section-directory-not-exists.test | 3 ++- .../llvm-objcopy/MachO/dump-section-directory-not-exists.test | 3 ++- .../llvm-objcopy/wasm/dump-section-directory-not-exists.test | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/llvm/test/tools/llvm-objcopy/ELF/dump-section-directory-not-exists.test b/llvm/test/tools/llvm-objcopy/ELF/dump-section-directory-not-exists.test index 462ebd691ae49..c11064c5f8c9b 100644 --- a/llvm/test/tools/llvm-objcopy/ELF/dump-section-directory-not-exists.test +++ b/llvm/test/tools/llvm-objcopy/ELF/dump-section-directory-not-exists.test @@ -4,7 +4,8 @@ # RUN: not llvm-objcopy --dump-section .text=not_exists/text-section %t 2>&1 \ # RUN: | FileCheck %s -DINPUT=%t --check-prefix=NO-SUCH-PATH -# NO-SUCH-PATH: error: 'not_exists/text-section': No such file or directory +# Don't check the OS-dependent message "No such file or directory". +# NO-SUCH-PATH: error: 'not_exists/text-section': !ELF FileHeader: diff --git a/llvm/test/tools/llvm-objcopy/MachO/dump-section-directory-not-exists.test b/llvm/test/tools/llvm-objcopy/MachO/dump-section-directory-not-exists.test index ac597b362b4c7..c4f218df4afc6 100644 --- a/llvm/test/tools/llvm-objcopy/MachO/dump-section-directory-not-exists.test +++ b/llvm/test/tools/llvm-objcopy/MachO/dump-section-directory-not-exists.test @@ -4,7 +4,8 @@ # RUN: not llvm-objcopy --dump-section __TEXT,__text=not_exists/text-section %t 2>&1 \ # RUN: | FileCheck %s -DINPUT=%t --check-prefix=NO-SUCH-PATH -# NO-SUCH-PATH: error: 'not_exists/text-section': No such file or directory +# Don't check the OS-dependent message "No such file or directory". +# NO-SUCH-PATH: error: 'not_exists/text-section': --- !mach-o FileHeader: diff --git a/llvm/test/tools/llvm-objcopy/wasm/dump-section-directory-not-exists.test b/llvm/test/tools/llvm-objcopy/wasm/dump-section-directory-not-exists.test index f0ac8e980cf5d..6fbb9043b6648 100644 --- a/llvm/test/tools/llvm-objcopy/wasm/dump-section-directory-not-exists.test +++ b/llvm/test/tools/llvm-objcopy/wasm/dump-section-directory-not-exists.test @@ -4,7 +4,8 @@ # RUN: not llvm-objcopy --dump-section producers=not_exists/text-section %t 2>&1 \ # RUN: | FileCheck %s -DINPUT=%t --check-prefix=NO-SUCH-PATH -# NO-SUCH-PATH: error: 'not_exists/text-section': No such file or directory +# Don't check the OS-dependent message "No such file or directory". +# NO-SUCH-PATH: error: 'not_exists/text-section': --- !WASM FileHeader: From 9a8f4c6060154bf491c1b6fe7c6340ecbf692591 Mon Sep 17 00:00:00 2001 From: AmrDeveloper Date: Sat, 1 Feb 2025 23:45:53 +0100 Subject: [PATCH 3/8] Address code review comments --- .../dump-section-directory-not-exists.test | 37 ---------- .../tools/llvm-objcopy/ELF/dump-section.test | 6 ++ .../dump-section-directory-not-exists.test | 68 ------------------- .../llvm-objcopy/MachO/dump-section.test | 4 ++ .../dump-section-directory-not-exists.test | 25 ------- .../tools/llvm-objcopy/wasm/dump-section.test | 4 ++ 6 files changed, 14 insertions(+), 130 deletions(-) delete mode 100644 llvm/test/tools/llvm-objcopy/ELF/dump-section-directory-not-exists.test delete mode 100644 llvm/test/tools/llvm-objcopy/MachO/dump-section-directory-not-exists.test delete mode 100644 llvm/test/tools/llvm-objcopy/wasm/dump-section-directory-not-exists.test diff --git a/llvm/test/tools/llvm-objcopy/ELF/dump-section-directory-not-exists.test b/llvm/test/tools/llvm-objcopy/ELF/dump-section-directory-not-exists.test deleted file mode 100644 index c11064c5f8c9b..0000000000000 --- a/llvm/test/tools/llvm-objcopy/ELF/dump-section-directory-not-exists.test +++ /dev/null @@ -1,37 +0,0 @@ -## Show that llvm-objcopy report that section file path is not exists. - -# RUN: yaml2obj %s -o %t - -# RUN: not llvm-objcopy --dump-section .text=not_exists/text-section %t 2>&1 \ -# RUN: | FileCheck %s -DINPUT=%t --check-prefix=NO-SUCH-PATH -# Don't check the OS-dependent message "No such file or directory". -# NO-SUCH-PATH: error: 'not_exists/text-section': - -!ELF -FileHeader: - Class: ELFCLASS64 - Data: ELFDATA2LSB - Type: ET_EXEC - Machine: EM_X86_64 -Sections: - - Name: .text - Type: SHT_PROGBITS - Flags: [ SHF_ALLOC, SHF_EXECINSTR ] - AddressAlign: 0x0000000000001000 - Content: "DEADBEEF" - - Name: .foo - Type: SHT_PROGBITS - Flags: [ SHF_WRITE ] - Content: "CAFE" - - Name: .empty - Type: SHT_PROGBITS - Flags: [ SHF_ALLOC ] - - Name: .bar - Type: SHT_NOBITS - Flags: [ SHF_WRITE ] -ProgramHeaders: - - Type: PT_LOAD - Flags: [ PF_X, PF_R ] - FirstSec: .text - LastSec: .text - diff --git a/llvm/test/tools/llvm-objcopy/ELF/dump-section.test b/llvm/test/tools/llvm-objcopy/ELF/dump-section.test index 037ec86090e55..9e0f5f879d300 100644 --- a/llvm/test/tools/llvm-objcopy/ELF/dump-section.test +++ b/llvm/test/tools/llvm-objcopy/ELF/dump-section.test @@ -64,3 +64,9 @@ 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-directory-not-exists.test b/llvm/test/tools/llvm-objcopy/MachO/dump-section-directory-not-exists.test deleted file mode 100644 index c4f218df4afc6..0000000000000 --- a/llvm/test/tools/llvm-objcopy/MachO/dump-section-directory-not-exists.test +++ /dev/null @@ -1,68 +0,0 @@ -## Show that llvm-objcopy report that section file path is not exists. - -# RUN: yaml2obj %s -o %t - -# RUN: not llvm-objcopy --dump-section __TEXT,__text=not_exists/text-section %t 2>&1 \ -# RUN: | FileCheck %s -DINPUT=%t --check-prefix=NO-SUCH-PATH -# Don't check the OS-dependent message "No such file or directory". -# NO-SUCH-PATH: error: 'not_exists/text-section': - ---- !mach-o -FileHeader: - magic: 0xFEEDFACF - cputype: 0x01000007 - cpusubtype: 0x00000003 - filetype: 0x00000001 - ncmds: 1 - sizeofcmds: 312 - flags: 0x00002000 - reserved: 0x00000000 -LoadCommands: - - cmd: LC_SEGMENT_64 - cmdsize: 312 - segname: '' - vmaddr: 0 - vmsize: 12 - fileoff: 344 - filesize: 12 - maxprot: 7 - initprot: 7 - nsects: 3 - flags: 0 - Sections: - - sectname: __text - segname: __TEXT - addr: 0x0000000000000000 - content: 'AABBCCDD' - size: 4 - offset: 344 - align: 0 - reloff: 0x00000000 - nreloc: 0 - flags: 0x80000400 - reserved1: 0x00000000 - reserved2: 0x00000000 - - sectname: __data - segname: __DATA - addr: 0x0000000000000004 - content: 'EEFFEEFF' - size: 4 - offset: 348 - align: 0 - reloff: 0x00000000 - nreloc: 0 - flags: 0x00000000 - reserved1: 0x00000000 - reserved2: 0x00000000 - - sectname: __const - segname: __TEXT - addr: 0x0000000000000008 - content: 'EEFFEEFF' - size: 4 - offset: 352 - align: 0 - reloff: 0x00000000 - nreloc: 0 - flags: 0x00000000 - reserved1: 0x00000000 - reserved2: 0x00000000 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-directory-not-exists.test b/llvm/test/tools/llvm-objcopy/wasm/dump-section-directory-not-exists.test deleted file mode 100644 index 6fbb9043b6648..0000000000000 --- a/llvm/test/tools/llvm-objcopy/wasm/dump-section-directory-not-exists.test +++ /dev/null @@ -1,25 +0,0 @@ -## Show that llvm-objcopy report that section file path is not exists. - -# RUN: yaml2obj %s -o %t - -# RUN: not llvm-objcopy --dump-section producers=not_exists/text-section %t 2>&1 \ -# RUN: | FileCheck %s -DINPUT=%t --check-prefix=NO-SUCH-PATH -# Don't check the OS-dependent message "No such file or directory". -# NO-SUCH-PATH: error: 'not_exists/text-section': - ---- !WASM -FileHeader: - Version: 0x00000001 -Sections: - - Type: TYPE - Signatures: - - Index: 0 - ParamTypes: - - I32 - ReturnTypes: - - F32 - - Type: CUSTOM - Name: producers - Tools: - - Name: clang - Version: 9.0.0 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 From 830a9739f016d35763d443be69e044d4399f5e1b Mon Sep 17 00:00:00 2001 From: AmrDeveloper Date: Mon, 3 Feb 2025 17:56:21 +0100 Subject: [PATCH 4/8] Remove extra new lines --- llvm/test/tools/llvm-objcopy/ELF/dump-section.test | 2 -- 1 file changed, 2 deletions(-) diff --git a/llvm/test/tools/llvm-objcopy/ELF/dump-section.test b/llvm/test/tools/llvm-objcopy/ELF/dump-section.test index 9e0f5f879d300..2dbbcc0ca568e 100644 --- a/llvm/test/tools/llvm-objcopy/ELF/dump-section.test +++ b/llvm/test/tools/llvm-objcopy/ELF/dump-section.test @@ -68,5 +68,3 @@ ProgramHeaders: # 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]] - - From df9101e8cc3ad7fadb2ac113f363c9ef4b1d4238 Mon Sep 17 00:00:00 2001 From: AmrDeveloper Date: Mon, 3 Feb 2025 18:51:10 +0100 Subject: [PATCH 5/8] Address code reviews --- llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp | 47 +++++++++---------------- llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp | 5 ++- llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp | 5 ++- 3 files changed, 21 insertions(+), 36 deletions(-) diff --git a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp index 9d979a7c094d4..9c78f7433ad33 100644 --- a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp +++ b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp @@ -189,13 +189,10 @@ static Error dumpSectionToFile(StringRef SecName, StringRef Filename, StringRef InputFilename, Object &Obj) { for (auto &Sec : Obj.sections()) { if (Sec.Name == SecName) { - if (Sec.Type == SHT_NOBITS) { - Error E = - createStringError(object_error::parse_failed, - "cannot dump section '%s': it has no contents", - SecName.str().c_str()); - return createFileError(InputFilename, std::move(E)); - } + if (Sec.Type == SHT_NOBITS) + 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) @@ -208,10 +205,9 @@ static Error dumpSectionToFile(StringRef SecName, StringRef Filename, return Error::success(); } } - Error E = createStringError(object_error::parse_failed, - "section '%s' not found", SecName.str().c_str()); - return createFileError(InputFilename, std::move(E)); + return createFileError(InputFilename, object_error::parse_failed, + "section '%s' not found", SecName.str().c_str()); } Error Object::compressOrDecompressSections(const CommonConfig &Config) { @@ -832,24 +828,21 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, if (Config.ChangeSectionLMAValAll > 0 && Seg.PAddr > std::numeric_limits::max() - Config.ChangeSectionLMAValAll) { - Error E = 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) + ". The result would overflow"); - return createFileError(Config.InputFilename, std::move(E)); } else if (Config.ChangeSectionLMAValAll < 0 && Seg.PAddr < std::numeric_limits::min() - Config.ChangeSectionLMAValAll) { - Error E = 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)) + ". The result would underflow"); - - return createFileError(Config.InputFilename, std::move(E)); } Seg.PAddr += Config.ChangeSectionLMAValAll; } @@ -857,12 +850,10 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, } if (!Config.ChangeSectionAddress.empty()) { - if (Obj.Type != ELF::ET_REL) { - Error E = createStringError( - object_error::invalid_file_type, + if (Obj.Type != ELF::ET_REL) + return createFileError( + Config.InputFilename, object_error::invalid_file_type, "cannot change section address in a non-relocatable file"); - return createFileError(Config.InputFilename, std::move(E)); - } StringMap SectionsToUpdateAddress; for (const SectionPatternAddressUpdate &PatternUpdate : make_range(Config.ChangeSectionAddress.rbegin(), @@ -873,26 +864,22 @@ static Error handleArgs(const CommonConfig &Config, const ELFConfig &ELFConfig, .second) { if (PatternUpdate.Update.Kind == AdjustKind::Subtract && Sec.Addr < PatternUpdate.Update.Value) { - Error E = 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) + ". The result would underflow"); - - return createFileError(Config.InputFilename, std::move(E)); } if (PatternUpdate.Update.Kind == AdjustKind::Add && Sec.Addr > std::numeric_limits::max() - PatternUpdate.Update.Value) { - Error E = 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) + ". The result would overflow"); - - return createFileError(Config.InputFilename, std::move(E)); } switch (PatternUpdate.Update.Kind) { diff --git a/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp b/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp index 49ac903e99573..682edffc84f34 100644 --- a/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp +++ b/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp @@ -323,9 +323,8 @@ static Error dumpSectionToFile(StringRef SecName, StringRef Filename, } } - Error E = createStringError(object_error::parse_failed, - "section '%s' not found", SecName.str().c_str()); - return createFileError(InputFilename, std::move(E)); + return createFileError(InputFilename, object_error::parse_failed, + "section '%s' not found", SecName.str().c_str()); } static Error addSection(const NewSectionInfo &NewSection, Object &Obj) { diff --git a/llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp b/llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp index 3ace39ac558f5..57fd0f5ad233c 100644 --- a/llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp +++ b/llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp @@ -53,9 +53,8 @@ static Error dumpSectionToFile(StringRef SecName, StringRef Filename, return Error::success(); } } - Error E = createStringError(errc::invalid_argument, "section '%s' not found", - SecName.str().c_str()); - return createFileError(Filename, std::move(E)); + return createFileError(Filename, errc::invalid_argument, + "section '%s' not found", SecName.str().c_str()); } static void removeSections(const CommonConfig &Config, Object &Obj) { From a6be4bd80523188cd2ce8b6205a60cc916c5cff9 Mon Sep 17 00:00:00 2001 From: AmrDeveloper Date: Thu, 6 Feb 2025 18:34:16 +0100 Subject: [PATCH 6/8] Add releasenote --- llvm/docs/ReleaseNotes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/llvm/docs/ReleaseNotes.md b/llvm/docs/ReleaseNotes.md index dc8439b288957..0493d2b32a37c 100644 --- a/llvm/docs/ReleaseNotes.md +++ b/llvm/docs/ReleaseNotes.md @@ -130,6 +130,8 @@ Changes to the Debug Info Changes to the LLVM tools --------------------------------- +- llvm-objcopy now correct path in error message when section output path doesn't exist. + Changes to LLDB --------------------------------- From 37289e68ade00b7631181936e41ccbb04dd272be Mon Sep 17 00:00:00 2001 From: AmrDeveloper Date: Fri, 7 Feb 2025 19:12:57 +0100 Subject: [PATCH 7/8] Update releasenote --- llvm/docs/ReleaseNotes.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/llvm/docs/ReleaseNotes.md b/llvm/docs/ReleaseNotes.md index 0493d2b32a37c..a5bac4d80b91a 100644 --- a/llvm/docs/ReleaseNotes.md +++ b/llvm/docs/ReleaseNotes.md @@ -130,7 +130,8 @@ Changes to the Debug Info Changes to the LLVM tools --------------------------------- -- llvm-objcopy now correct path in error message when section output path doesn't exist. +- llvm-objcopy now prints the correct file path in the error message when +the output file specified by --dump-section cannot be opened. Changes to LLDB --------------------------------- From 0c3c39e3c5c44c28067cd4bd388d72ab3b6abbe0 Mon Sep 17 00:00:00 2001 From: AmrDeveloper Date: Sat, 8 Feb 2025 13:05:44 +0100 Subject: [PATCH 8/8] Remove release note from this PR --- llvm/docs/ReleaseNotes.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/llvm/docs/ReleaseNotes.md b/llvm/docs/ReleaseNotes.md index a5bac4d80b91a..dc8439b288957 100644 --- a/llvm/docs/ReleaseNotes.md +++ b/llvm/docs/ReleaseNotes.md @@ -130,9 +130,6 @@ Changes to the Debug Info Changes to the LLVM tools --------------------------------- -- llvm-objcopy now prints the correct file path in the error message when -the output file specified by --dump-section cannot be opened. - Changes to LLDB ---------------------------------