Skip to content

Add --dump-offload-bundle option to llvm-objcopy #143347

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add --dump-offload-bundle option to llvm-objcopy #143347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions llvm/docs/CommandGuide/llvm-objcopy.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ multiple file formats.
For MachO objects, ``<section>`` must be formatted as
``<segment name>,<section name>``.

.. option:: --dump-offload-bundle=<URI>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: options should be in alphabetical order.


Dump the HIP Offload Bundle entry specified by the URI syntax given, into a
code object file.


.. option:: --enable-deterministic-archives, -D

Enable deterministic mode when copying archives, i.e. use 0 for archive member
Expand Down
2 changes: 2 additions & 0 deletions llvm/include/llvm/ObjCopy/CommonConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,8 @@ struct CommonConfig {
bool StripUnneeded = false;
bool Weaken = false;
bool DecompressDebugSections = false;
bool DumpOffloadBundle = false;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've only implemented this for ELF, but you've put this in CommonConfig, not ELFConfig. Why?

bool NeedPositional = true;

DebugCompressionType CompressionType = DebugCompressionType::None;

Expand Down
2 changes: 1 addition & 1 deletion llvm/include/llvm/Object/OffloadBundle.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ struct OffloadBundleURI {
OffsetStr.getAsInteger(10, O);
Str = Str.drop_front(OffsetStr.size());

if (Str.consume_front("&size="))
if (!Str.consume_front("&size="))
return createStringError(object_error::parse_failed,
"Reading 'size' in URI");

Expand Down
21 changes: 21 additions & 0 deletions llvm/lib/ObjCopy/ELF/ELFObjcopy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,27 @@ static Error dumpSectionToFile(StringRef SecName, StringRef Filename,
"section '%s' not found", SecName.str().c_str());
}

static Error dumpRawDataURIToFile(StringRef Filename, int64_t Offset,
int64_t Size, ObjectFile &Obj) {
SmallString<2048> NameBuf;
raw_svector_ostream OutputFileName(NameBuf);
OutputFileName << Obj.getFileName().str() << "-offset" << Offset << "-size"
<< Size << ".co";

Expected<std::unique_ptr<FileOutputBuffer>> BufferOrErr =
FileOutputBuffer::create(OutputFileName.str(), Size);

if (!BufferOrErr)
return BufferOrErr.takeError();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add tests that show the error handling here and elsewhere in this change is correct.


MemoryBufferRef Input = Obj.getMemoryBufferRef();
std::unique_ptr<FileOutputBuffer> Buf = std::move(*BufferOrErr);
std::copy(Input.getBufferStart(), Input.getBufferStart() + Size,
Buf->getBufferStart());
Comment on lines +228 to +229
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, you can use llvm::copy to avoid the iterators.


return Buf->commit();
}

Error Object::compressOrDecompressSections(const CommonConfig &Config) {
// Build a list of sections we are going to replace.
// We can't call `addSection` while iterating over sections,
Expand Down
60 changes: 60 additions & 0 deletions llvm/test/tools/llvm-objcopy/ELF/dump-offload-bundle.test
View file
Open in desktop

Large diffs are not rendered by default.

132 changes: 79 additions & 53 deletions llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
#include "llvm/ObjCopy/ConfigManager.h"
#include "llvm/ObjCopy/MachO/MachOConfig.h"
#include "llvm/Object/Binary.h"
#include "llvm/Object/OffloadBinary.h"
#include "llvm/Object/OffloadBundle.h"
#include "llvm/Option/Arg.h"
#include "llvm/Option/ArgList.h"
#include "llvm/Support/CRC.h"
Expand Down Expand Up @@ -284,6 +286,11 @@ static Expected<uint8_t> parseVisibilityType(StringRef VisType) {
return type;
}

static void llvm::objcopy::parseDumpOffloadBundle(StringRef URI) {
if (Error Err = object::extractOffloadBundleByURI(URI))
outs() << "Failed to extract from URI.";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

outs() or err()?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error messages should start with lowercase and not end in a period https://llvm.org/docs/CodingStandards.html#error-and-warning-messages

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouldn't be either outs() or errs(): the error should be reported using llvm-objcopy's existing error reporting mechanism, by passing an Error up the stack.

}

namespace {
struct TargetInfo {
FileFormat Format;
Expand Down Expand Up @@ -727,34 +734,45 @@ objcopy::parseObjcopyOptions(ArrayRef<const char *> ArgsArr,

SmallVector<const char *, 2> Positional;

ConfigManager ConfigMgr;
CommonConfig &Config = ConfigMgr.Common;
COFFConfig &COFFConfig = ConfigMgr.COFF;
ELFConfig &ELFConfig = ConfigMgr.ELF;
MachOConfig &MachOConfig = ConfigMgr.MachO;

if (InputArgs.hasArg(OBJCOPY_dump_offload_bundle))
Config.NeedPositional = false;

for (auto *Arg : InputArgs.filtered(OBJCOPY_UNKNOWN))
return createStringError(errc::invalid_argument, "unknown argument '%s'",
Arg->getAsString(InputArgs).c_str());

for (auto *Arg : InputArgs.filtered(OBJCOPY_INPUT))
Positional.push_back(Arg->getValue());

if (Positional.empty())
if (Positional.empty() && Config.NeedPositional)
return createStringError(errc::invalid_argument, "no input file specified");

if (Positional.size() > 2)
if (Positional.size() > 2 && Config.NeedPositional)
return createStringError(errc::invalid_argument,
"too many positional arguments");

ConfigManager ConfigMgr;
CommonConfig &Config = ConfigMgr.Common;
COFFConfig &COFFConfig = ConfigMgr.COFF;
ELFConfig &ELFConfig = ConfigMgr.ELF;
MachOConfig &MachOConfig = ConfigMgr.MachO;
Config.InputFilename = Positional[0];
Config.OutputFilename = Positional[Positional.size() == 1 ? 0 : 1];
if (InputArgs.hasArg(OBJCOPY_target) &&
(InputArgs.hasArg(OBJCOPY_input_target) ||
InputArgs.hasArg(OBJCOPY_output_target)))
return createStringError(
errc::invalid_argument,
"--target cannot be used with --input-target or --output-target");
if (Arg *A = InputArgs.getLastArg(OBJCOPY_dump_offload_bundle)) {
for (StringRef URIStr : llvm::split(A->getValue(), ",")) {
llvm::objcopy::parseDumpOffloadBundle(URIStr);
}
Comment on lines +761 to +763
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (StringRef URIStr : llvm::split(A->getValue(), ",")) {
llvm::objcopy::parseDumpOffloadBundle(URIStr);
}
for (StringRef URIStr : llvm::split(A->getValue(), ","))
llvm::objcopy::parseDumpOffloadBundle(URIStr);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

}

if (Config.NeedPositional) {
Config.InputFilename = Positional[0];
Config.OutputFilename = Positional[Positional.size() == 1 ? 0 : 1];
if (InputArgs.hasArg(OBJCOPY_target) &&
(InputArgs.hasArg(OBJCOPY_input_target) ||
InputArgs.hasArg(OBJCOPY_output_target)))
return createStringError(
errc::invalid_argument,
"--target cannot be used with --input-target or --output-target");
}
if (InputArgs.hasArg(OBJCOPY_regex) && InputArgs.hasArg(OBJCOPY_wildcard))
return createStringError(errc::invalid_argument,
"--regex and --wildcard are incompatible");
Expand Down Expand Up @@ -1417,25 +1435,26 @@ objcopy::parseInstallNameToolOptions(ArrayRef<const char *> ArgsArr) {
Arg->getAsString(InputArgs).c_str());
for (auto *Arg : InputArgs.filtered(INSTALL_NAME_TOOL_INPUT))
Positional.push_back(Arg->getValue());
if (Positional.empty())
if (Positional.empty() && Config.NeedPositional)
return createStringError(errc::invalid_argument, "no input file specified");
if (Positional.size() > 1)
if (Positional.size() > 1 && Config.NeedPositional)
return createStringError(
errc::invalid_argument,
"llvm-install-name-tool expects a single input file");
Config.InputFilename = Positional[0];
Config.OutputFilename = Positional[0];

Expected<OwningBinary<Binary>> BinaryOrErr =
createBinary(Config.InputFilename);
if (!BinaryOrErr)
return createFileError(Config.InputFilename, BinaryOrErr.takeError());
auto *Binary = (*BinaryOrErr).getBinary();
if (!Binary->isMachO() && !Binary->isMachOUniversalBinary())
return createStringError(errc::invalid_argument,
"input file: %s is not a Mach-O file",
Config.InputFilename.str().c_str());

if (Config.NeedPositional) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code refers to llvm-install-name-tool. The new option shouldn't impact it in any way.

Config.InputFilename = Positional[0];
Config.OutputFilename = Positional[0];

Expected<OwningBinary<Binary>> BinaryOrErr =
createBinary(Config.InputFilename);
if (!BinaryOrErr)
return createFileError(Config.InputFilename, BinaryOrErr.takeError());
auto *Binary = (*BinaryOrErr).getBinary();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
auto *Binary = (*BinaryOrErr).getBinary();
auto &Binary = *BinaryOrErr->getBinary();

Copy link
Contributor Author

@david-salinas david-salinas Jun 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is actually existing code that I'm just indenting because I had to add "if (Config.NeedsPositional)". I need to do this because llvm-objcopy is set up to always expect a source file argument unless the "NeedsPositional" config setting is negative/false. And with the new --dump-offload-bundle option I'm adding, it takes a URI argument which contains the source file argument. So if the --dump-offload-bundle option is specified I set "NeedsPositional" to false.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the above comment: this code isn't used by llvm-objcopy itself.

if (!Binary->isMachO() && !Binary->isMachOUniversalBinary())
return createStringError(errc::invalid_argument,
"input file: %s is not a Mach-O file",
Config.InputFilename.str().c_str());
}
DC.CopyConfigs.push_back(std::move(ConfigMgr));
return std::move(DC);
}
Expand Down Expand Up @@ -1474,13 +1493,16 @@ objcopy::parseBitcodeStripOptions(ArrayRef<const char *> ArgsArr,
Arg->getAsString(InputArgs).c_str());

SmallVector<StringRef, 2> Positional;
for (auto *Arg : InputArgs.filtered(BITCODE_STRIP_INPUT))
Positional.push_back(Arg->getValue());
if (Positional.size() > 1)
return createStringError(errc::invalid_argument,
"llvm-bitcode-strip expects a single input file");
assert(!Positional.empty());
Config.InputFilename = Positional[0];
if (Config.NeedPositional) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, this code isn't used by llvm-objcopy. It's to do with llvm-bitcode-strip.

for (auto *Arg : InputArgs.filtered(BITCODE_STRIP_INPUT))
Positional.push_back(Arg->getValue());
if (Positional.size() > 1)
return createStringError(
errc::invalid_argument,
"llvm-bitcode-strip expects a single input file");
assert(!Positional.empty());
Config.InputFilename = Positional[0];
}

if (!InputArgs.hasArg(BITCODE_STRIP_output)) {
return createStringError(errc::invalid_argument,
Expand Down Expand Up @@ -1542,27 +1564,31 @@ objcopy::parseStripOptions(ArrayRef<const char *> RawArgsArr,
exit(0);
}

SmallVector<StringRef, 2> Positional;
for (auto *Arg : InputArgs.filtered(STRIP_UNKNOWN))
return createStringError(errc::invalid_argument, "unknown argument '%s'",
Arg->getAsString(InputArgs).c_str());
for (auto *Arg : InputArgs.filtered(STRIP_INPUT))
Positional.push_back(Arg->getValue());
std::copy(DashDash, RawArgsArr.end(), std::back_inserter(Positional));

if (Positional.empty())
return createStringError(errc::invalid_argument, "no input file specified");

if (Positional.size() > 1 && InputArgs.hasArg(STRIP_output))
return createStringError(
errc::invalid_argument,
"multiple input files cannot be used in combination with -o");

ConfigManager ConfigMgr;
CommonConfig &Config = ConfigMgr.Common;
ELFConfig &ELFConfig = ConfigMgr.ELF;
MachOConfig &MachOConfig = ConfigMgr.MachO;

SmallVector<StringRef, 2> Positional;
if (Config.NeedPositional) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, not llvm-objcopy. This is for llvm-strip.

for (auto *Arg : InputArgs.filtered(STRIP_UNKNOWN))
return createStringError(errc::invalid_argument, "unknown argument '%s'",
Arg->getAsString(InputArgs).c_str());
for (auto *Arg : InputArgs.filtered(STRIP_INPUT))
Positional.push_back(Arg->getValue());
std::copy(DashDash, RawArgsArr.end(), std::back_inserter(Positional));

if (Positional.empty())
return createStringError(errc::invalid_argument,
"no input file specified");

if (Positional.size() > 1 && InputArgs.hasArg(STRIP_output)) {
return createStringError(
errc::invalid_argument,
"multiple input files cannot be used in combination with -o");
}
}

if (InputArgs.hasArg(STRIP_regex) && InputArgs.hasArg(STRIP_wildcard))
return createStringError(errc::invalid_argument,
"--regex and --wildcard are incompatible");
Expand Down
5 changes: 5 additions & 0 deletions llvm/tools/llvm-objcopy/ObjcopyOptions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,11 @@ parseBitcodeStripOptions(ArrayRef<const char *> ArgsArr,
Expected<DriverConfig>
parseStripOptions(ArrayRef<const char *> ArgsArr,
llvm::function_ref<Error(Error)> ErrorCallback);

// parseDumpURI reads a URI as a string, and extracts the raw memory into a
// code object file named from the URI string given
Comment on lines +55 to +56
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Comment uses wrong function name
  • Missing full stop
  • I think correct grammar would be no comma before "and".

static void parseDumpOffloadBundle(StringRef URI);

} // namespace objcopy
} // namespace llvm

Expand Down
3 changes: 3 additions & 0 deletions llvm/tools/llvm-objcopy/ObjcopyOpts.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,9 @@ defm dump_section
: Eq<"dump-section",
"Dump contents of section named <section> into file <file>">,
MetaVarName<"section=file">;

defm dump_offload_bundle : Eq<"dump-offload-bundle", "Dump the contents specified by URI">;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: alphabetical order.


defm prefix_symbols
: Eq<"prefix-symbols", "Add <prefix> to the start of every symbol name">,
MetaVarName<"prefix">;
Expand Down
Loading
Loading