release/20.x: [llvm-objcopy] Fix prints wrong path when dump-section output path doesn't exist (#125345) #126367
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@AmrDeveloper What do you think about merging this PR to the release branch? |
|
@llvm/pr-subscribers-llvm-support @llvm/pr-subscribers-llvm-binary-utilities Author: None (llvmbot) ChangesBackport 66bea0d Requested by: @AmrDeveloper Full diff: https://github.com/llvm/llvm-project/pull/126367.diff 6 Files Affected:
diff --git a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
index 5aa0079f3fbc7a7..9c78f7433ad3390 100644
--- a/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
+++ b/llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
@@ -186,27 +186,28 @@ 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());
+ return createFileError(InputFilename, object_error::parse_failed,
+ "cannot dump section '%s': it has no contents",
+ SecName.str().c_str());
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());
+
+ 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<uint64_t>::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<uint64_t>::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<AddressUpdate> 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<uint64_t>::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 a188425b283fa63..682edffc84f3463 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<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());
+ 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 cf3d884bee3bd8e..57fd0f5ad233ccc 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<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());
+ 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 037ec86090e556c..2dbbcc0ca568eec 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 9a1227cdbbda166..d54a50b557bb705 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 983a581e03fe28d..2d1533f06df1015 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
|
jh7370
approved these changes
Feb 10, 2025
Not sure if I can push release note on this PR only, but I will create a follow-up PR to add release note on release branch @jh7370 |
AmrDeveloper
approved these changes
Feb 10, 2025
…esn't exist (llvm#125345) Fix printing the correct file path in the error message when the output file specified by `--dump-section` cannot be opened Fixes: llvm#125113 on ELF, MachO, Wasm (cherry picked from commit 66bea0d)
Add new CreateFileError functions to create a StringError with the specified error code and prepend the file path to it Needed for: llvm#125345 (cherry picked from commit 2464f4b)
|
@AmrDeveloper (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport 66bea0d
Requested by: @AmrDeveloper