[llvm] [llvm-objcopy] Fix prints wrong path when section output path doesn't exist (PR #125345)
via llvm-commits
llvm-commits at lists.llvm.org
Sat Feb 1 03:49:16 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-binary-utilities
Author: Amr Hesham (AmrDeveloper)
<details>
<summary>Changes</summary>
Change the way of creating new file errors to be aware of which file it should report to
Fixes: #<!-- -->125113 on ELF, MachO, Wasm
---
Full diff: https://github.com/llvm/llvm-project/pull/125345.diff
6 Files Affected:
- (modified) llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp (+40-26)
- (modified) llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp (+15-13)
- (modified) llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp (+9-7)
- (added) llvm/test/tools/llvm-objcopy/ELF/dump-section-directory-not-exists.test (+36)
- (added) llvm/test/tools/llvm-objcopy/MachO/dump-section-directory-not-exists.test (+67)
- (added) llvm/test/tools/llvm-objcopy/wasm/dump-section-directory-not-exists.test (+24)
``````````diff
diff --git a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
index 5aa0079f3fbc7a7..9d979a7c094d415 100644
--- a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
+++ b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
@@ -186,27 +186,32 @@ static std::unique_ptr<Writer> 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<std::unique_ptr<FileOutputBuffer>> BufferOrErr =
FileOutputBuffer::create(Filename, Sec.OriginalData.size());
if (!BufferOrErr)
- return BufferOrErr.takeError();
+ return createFileError(Filename, BufferOrErr.takeError());
std::unique_ptr<FileOutputBuffer> 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<uint64_t>::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<uint64_t>::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<AddressUpdate> 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<uint64_t>::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 a188425b283fa63..49ac903e9957326 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<Section> &Sec : LC.Sections) {
if (Sec->CanonicalName == SecName) {
Expected<std::unique_ptr<FileOutputBuffer>> BufferOrErr =
FileOutputBuffer::create(Filename, Sec->Content.size());
if (!BufferOrErr)
- return BufferOrErr.takeError();
+ return createFileError(Filename, BufferOrErr.takeError());
std::unique_ptr<FileOutputBuffer> 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 cf3d884bee3bd8e..3ace39ac558f5b4 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<uint8_t> Contents = Sec.Contents;
Expected<std::unique_ptr<FileOutputBuffer>> BufferOrErr =
FileOutputBuffer::create(Filename, Contents.size());
if (!BufferOrErr)
- return BufferOrErr.takeError();
+ return createFileError(Filename, BufferOrErr.takeError());
std::unique_ptr<FileOutputBuffer> 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 000000000000000..462ebd691ae49ef
--- /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 000000000000000..ac597b362b4c7b1
--- /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 000000000000000..f0ac8e980cf5dcf
--- /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
``````````
</details>
https://github.com/llvm/llvm-project/pull/125345
More information about the llvm-commits
mailing list