Skip to content

[ObjCopy][DX] Support for -dump-section flag #159999

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

Merged
merged 5 commits into from
Oct 14, 2025
Merged

[ObjCopy][DX] Support for -dump-section flag #159999

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2f5835f
[ObjCopy][DX] Support for -dump-section flag
llvm-beanz Sep 21, 2025
826d77c
Respond to review feedback.
llvm-beanz Sep 25, 2025
8acfeef
Fixing up tests based on feedback.
llvm-beanz Oct 6, 2025
cbe01cb
Update llvm/lib/ObjCopy/DXContainer/DXContainerObjcopy.cpp
llvm-beanz Oct 10, 2025
8cc7959
Updates based on review feedback
llvm-beanz Oct 13, 2025
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
16 changes: 8 additions & 8 deletions llvm/lib/ObjCopy/ConfigManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,14 @@ ConfigManager::getDXContainerConfig() const {
if (!Common.AddGnuDebugLink.empty() || !Common.SplitDWO.empty() ||
!Common.AllocSectionsPrefix.empty() ||
Common.DiscardMode != DiscardType::None || !Common.AddSection.empty() ||
!Common.DumpSection.empty() || !Common.KeepSection.empty() ||
!Common.SectionsToRename.empty() || !Common.SetSectionAlignment.empty() ||
!Common.SetSectionFlags.empty() || !Common.SetSectionType.empty() ||
Common.ExtractDWO || Common.OnlyKeepDebug || Common.StripAllGNU ||
Common.StripDWO || Common.StripDebug || Common.StripNonAlloc ||
Common.StripSections || Common.StripUnneeded ||
Common.DecompressDebugSections || Common.GapFill != 0 ||
Common.PadTo != 0 || Common.ChangeSectionLMAValAll != 0 ||
!Common.KeepSection.empty() || !Common.SectionsToRename.empty() ||
!Common.SetSectionAlignment.empty() || !Common.SetSectionFlags.empty() ||
!Common.SetSectionType.empty() || Common.ExtractDWO ||
Common.OnlyKeepDebug || Common.StripAllGNU || Common.StripDWO ||
Common.StripDebug || Common.StripNonAlloc || Common.StripSections ||
Common.StripUnneeded || Common.DecompressDebugSections ||
Common.GapFill != 0 || Common.PadTo != 0 ||
Common.ChangeSectionLMAValAll != 0 ||
!Common.ChangeSectionAddress.empty()) {
return createStringError(llvm::errc::invalid_argument,
"option is not supported for DXContainer");
Expand Down
35 changes: 35 additions & 0 deletions llvm/lib/ObjCopy/DXContainer/DXContainerObjcopy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
#include "llvm/ObjCopy/DXContainer/DXContainerObjcopy.h"
#include "DXContainerReader.h"
#include "DXContainerWriter.h"
#include "llvm/BinaryFormat/DXContainer.h"
#include "llvm/ObjCopy/CommonConfig.h"
#include "llvm/ObjCopy/DXContainer/DXContainerConfig.h"
#include "llvm/Support/FileOutputBuffer.h"
#include "llvm/Support/raw_ostream.h"

namespace llvm {
Expand Down Expand Up @@ -42,7 +44,40 @@ static Error extractPartAsObject(StringRef PartName, StringRef OutFilename,
"part '%s' not found", PartName.str().c_str());
}

static Error dumpPartToFile(StringRef PartName, StringRef Filename,
StringRef InputFilename, Object &Obj) {
for (const Part &P : Obj.Parts) {
if (P.Name == PartName) {
Copy link
Contributor

@bogner bogner Oct 8, 2025

Choose a reason for hiding this comment

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

This seems like a weird pattern for this. Why not use find_if and return early if it isn't found instead?

  auto PartIter = llvm::find_if(
      Obj.Parts, [&PartName](const Part &P) { return P.Name == PartName; });
  if (PartIter == Obj.Parts.end())
    return createFileError(...);

ArrayRef<uint8_t> Contents = P.Data;
// For parts containing llvm bitcode, don't dump the program headers so
// that we get a valid .bc file.
if (PartName == "DXIL" || PartName == "STAT")
Contents = Contents.drop_front(sizeof(llvm::dxbc::ProgramHeader));
Expected<std::unique_ptr<FileOutputBuffer>> BufferOrErr =
FileOutputBuffer::create(Filename, Contents.size());
if (!BufferOrErr)
return createFileError(Filename, BufferOrErr.takeError());
Copy link
Collaborator

@jh7370 jh7370 Sep 23, 2025

Choose a reason for hiding this comment

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

Test cases for this and other error paths are required.

Copy link
Collaborator

@jh7370 jh7370 Sep 30, 2025

Choose a reason for hiding this comment

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

I assume it's not practical to test both the create and commit error paths? (I see one is tested, but not the other).

std::unique_ptr<FileOutputBuffer> Buf = std::move(*BufferOrErr);
llvm::copy(Contents, Buf->getBufferStart());
if (Error E = Buf->commit())
return createFileError(Filename, std::move(E));
Copy link
Contributor

@inbelic inbelic Sep 22, 2025

Choose a reason for hiding this comment

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

ELF behaviour is to create an error if the contents are empty. I am not sure if that is generated here? Can we add a test-case to demonstrate that error is generated

return Error::success();
}
}
return createFileError(Filename,
std::make_error_code(std::errc::invalid_argument),
"part '%s' not found", PartName.str().c_str());
}

static Error handleArgs(const CommonConfig &Config, Object &Obj) {
for (StringRef Flag : Config.DumpSection) {
StringRef SecName;
StringRef FileName;
std::tie(SecName, FileName) = Flag.split("=");
if (Error E = dumpPartToFile(SecName, FileName, Config.InputFilename, Obj))
return E;
}

// Extract all sections before any modifications.
for (StringRef Flag : Config.ExtractSection) {
StringRef SectionName;
Expand Down
Loading
Loading