Skip to content

Commit

Permalink
[llvm-objcopy] Fix prints wrong path when dump-section output path do…
Browse files Browse the repository at this point in the history 
…esn'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
  • Loading branch information
@AmrDeveloper
AmrDeveloper authored Feb 8, 2025
1 parent ee80664 commit 66bea0d
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 49 deletions.
59 changes: 30 additions & 29 deletions llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}

Expand All @@ -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()) {
Expand All @@ -826,17 +828,17 @@ 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) +
". The result would overflow");
} 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)) +
Expand All @@ -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(),
Expand All @@ -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) +
Expand All @@ -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) +
Expand Down Expand Up @@ -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) {
Expand All @@ -924,15 +925,15 @@ 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) {
auto UpdateSection = [&](StringRef Name, ArrayRef<uint8_t> Data) {
return Obj.updateSection(Name, Data);
};
if (Error E = handleUserSection(NewSection, UpdateSection))
return E;
return createFileError(Config.InputFilename, std::move(E));
}

if (!Config.AddGnuDebugLink.empty())
Expand All @@ -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);
Expand All @@ -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())
Expand All @@ -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))
Expand Down Expand Up @@ -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));
Expand Down
27 changes: 14 additions & 13 deletions llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand All @@ -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();
}
Expand All @@ -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.
Expand Down
15 changes: 8 additions & 7 deletions llvm/lib/ObjCopy/wasm/WasmObjcopy.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions llvm/test/tools/llvm-objcopy/ELF/dump-section.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]]
4 changes: 4 additions & 0 deletions llvm/test/tools/llvm-objcopy/MachO/dump-section.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions llvm/test/tools/llvm-objcopy/wasm/dump-section.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 66bea0d

Please sign in to comment.