Skip to content

Conversation

llvm-beanz
Copy link
Collaborator

This adds support for the -dump-section=

= flag for the DXContainer file format. This flag dumps the contents of a named section to the specified file.

This flag is particularly handy for ripping DXIL bitcode out of the object files so that we can use LLVM tools to inspect and operate on the bitcode. To facilitate that workflow this flag also strips the program headers from parts containing DXIL so that the resulting file is a valid bitcode file.

This adds support for the -dump-section=<section>=<file> flag for the
DXContainer file format. This flag dumps the contents of a named section
to the specified file.

This flag is particularly handy for ripping DXIL bitcode out of the
object files so that we can use LLVM tools to inspect and operate on
the bitcode. To facilitate that workflow this flag also strips the
program headers from parts containing DXIL so that the resulting file is
a valid bitcode file.
../llvm/lib/ObjCopy/DXContainer/DXContainerObjcopy.cpp
../llvm/test/tools/llvm-objcopy/DXContainer/dump-section.yaml
@llvmbot
Copy link
Member

llvmbot commented Sep 21, 2025

@llvm/pr-subscribers-llvm-binary-utilities

@llvm/pr-subscribers-backend-directx

Author: Chris B (llvm-beanz)

Changes

This adds support for the -dump-section=<section>=<file> flag for the DXContainer file format. This flag dumps the contents of a named section to the specified file.

This flag is particularly handy for ripping DXIL bitcode out of the object files so that we can use LLVM tools to inspect and operate on the bitcode. To facilitate that workflow this flag also strips the program headers from parts containing DXIL so that the resulting file is a valid bitcode file.


Patch is 21.45 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/159999.diff

3 Files Affected:

  • (modified) llvm/lib/ObjCopy/ConfigManager.cpp (+8-8)
  • (modified) llvm/lib/ObjCopy/DXContainer/DXContainerObjcopy.cpp (+35)
  • (added) llvm/test/tools/llvm-objcopy/DXContainer/dump-section.yaml (+283)
diff --git a/llvm/lib/ObjCopy/ConfigManager.cpp b/llvm/lib/ObjCopy/ConfigManager.cpp
index eef8a2190c4d2..6b7b4f116f583 100644
--- a/llvm/lib/ObjCopy/ConfigManager.cpp
+++ b/llvm/lib/ObjCopy/ConfigManager.cpp
@@ -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");
diff --git a/llvm/lib/ObjCopy/DXContainer/DXContainerObjcopy.cpp b/llvm/lib/ObjCopy/DXContainer/DXContainerObjcopy.cpp
index d7f3c0d1f7b36..e41f3fe6bedb2 100644
--- a/llvm/lib/ObjCopy/DXContainer/DXContainerObjcopy.cpp
+++ b/llvm/lib/ObjCopy/DXContainer/DXContainerObjcopy.cpp
@@ -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 {
@@ -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) {
+      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());
+      std::unique_ptr<FileOutputBuffer> Buf = std::move(*BufferOrErr);
+      llvm::copy(Contents, Buf->getBufferStart());
+      if (Error E = Buf->commit())
+        return createFileError(Filename, std::move(E));
+      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;
diff --git a/llvm/test/tools/llvm-objcopy/DXContainer/dump-section.yaml b/llvm/test/tools/llvm-objcopy/DXContainer/dump-section.yaml
new file mode 100644
index 0000000000000..faaf59b61638d
--- /dev/null
+++ b/llvm/test/tools/llvm-objcopy/DXContainer/dump-section.yaml
@@ -0,0 +1,283 @@
+# RUN: yaml2obj %s -o %t.dxbc
+# RUN: llvm-objcopy --dump-section=DXIL=%t.bc %t.dxbc
+# RUN: llvm-dis %t.bc -o - | FileCheck %s
+# RUN: wc -c %t.bc | FileCheck %s --check-prefix=DXIL-SIZE
+
+# Verify that when dumping the DXIL part we get a valid bitcode file.
+# CHECK: define void @main()
+# Verify the size of the bitcode data.
+# DXIL-SIZE: 1708
+
+# Dump the PSV0 part and verify its size.
+# RUN: llvm-objcopy --dump-section=PSV0=%t.psv0 %t.dxbc
+# RUN: wc -c %t.psv0 | FileCheck %s --check-prefix=PSV0-SIZE
+# PSV0-SIZE: 76
+--- !dxcontainer
+Header:
+  Hash:            [ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
+                     0x0, 0x0, 0x0, 0x0, 0x0, 0x0 ]
+  Version:
+    Major:           1
+    Minor:           0
+  FileSize:        1956
+  PartCount:       6
+  PartOffsets:     [ 56, 1796, 1812, 1840, 1856, 1872 ]
+Parts:
+  - Name:            DXIL
+    Size:            1732
+    Program:
+      MajorVersion:    6
+      MinorVersion:    0
+      ShaderKind:      5
+      Size:            433
+      DXILMajorVersion: 1
+      DXILMinorVersion: 0
+      DXILSize:        1708
+      DXIL:            [ 0x42, 0x43, 0xC0, 0xDE, 0x21, 0xC, 0x0, 0x0, 0xA8,
+                         0x1, 0x0, 0x0, 0xB, 0x82, 0x20, 0x0, 0x2, 0x0,
+                         0x0, 0x0, 0x13, 0x0, 0x0, 0x0, 0x7, 0x81, 0x23,
+                         0x91, 0x41, 0xC8, 0x4, 0x49, 0x6, 0x10, 0x32,
+                         0x39, 0x92, 0x1, 0x84, 0xC, 0x25, 0x5, 0x8, 0x19,
+                         0x1E, 0x4, 0x8B, 0x62, 0x80, 0x10, 0x45, 0x2,
+                         0x42, 0x92, 0xB, 0x42, 0x84, 0x10, 0x32, 0x14,
+                         0x38, 0x8, 0x18, 0x4B, 0xA, 0x32, 0x42, 0x88,
+                         0x48, 0x90, 0x14, 0x20, 0x43, 0x46, 0x88, 0xA5,
+                         0x0, 0x19, 0x32, 0x42, 0xE4, 0x48, 0xE, 0x90,
+                         0x11, 0x22, 0xC4, 0x50, 0x41, 0x51, 0x81, 0x8C,
+                         0xE1, 0x83, 0xE5, 0x8A, 0x4, 0x21, 0x46, 0x6,
+                         0x51, 0x18, 0x0, 0x0, 0x4, 0x0, 0x0, 0x0, 0x1B,
+                         0x90, 0xE0, 0xFF, 0xFF, 0xFF, 0xFF, 0x7, 0xC0,
+                         0x1, 0x24, 0x80, 0x2, 0x0, 0x0, 0x0, 0x49, 0x18,
+                         0x0, 0x0, 0x1, 0x0, 0x0, 0x0, 0x13, 0x82, 0x0,
+                         0x0, 0x89, 0x20, 0x0, 0x0, 0x11, 0x0, 0x0, 0x0,
+                         0x32, 0x22, 0x8, 0x9, 0x20, 0x64, 0x85, 0x4, 0x13,
+                         0x22, 0xA4, 0x84, 0x4, 0x13, 0x22, 0xE3, 0x84,
+                         0xA1, 0x90, 0x14, 0x12, 0x4C, 0x88, 0x8C, 0xB,
+                         0x84, 0x84, 0x4C, 0x10, 0x20, 0x73, 0x4, 0x8,
+                         0xC1, 0x65, 0xC3, 0x85, 0x2C, 0xE8, 0x3, 0x40,
+                         0x14, 0x91, 0x4E, 0xD1, 0x4A, 0x48, 0x44, 0x54,
+                         0x11, 0xC3, 0x9, 0x30, 0xC4, 0x18, 0x1, 0x30,
+                         0x2, 0x50, 0x82, 0x21, 0x1A, 0x8, 0x98, 0x23,
+                         0x0, 0x3, 0x0, 0x13, 0x14, 0x72, 0xC0, 0x87, 0x74,
+                         0x60, 0x87, 0x36, 0x68, 0x87, 0x79, 0x68, 0x3,
+                         0x72, 0xC0, 0x87, 0xD, 0xAE, 0x50, 0xE, 0x6D,
+                         0xD0, 0xE, 0x7A, 0x50, 0xE, 0x6D, 0x0, 0xF, 0x7A,
+                         0x30, 0x7, 0x72, 0xA0, 0x7, 0x73, 0x20, 0x7, 0x6D,
+                         0x90, 0xE, 0x71, 0xA0, 0x7, 0x73, 0x20, 0x7, 0x6D,
+                         0x90, 0xE, 0x78, 0xA0, 0x7, 0x78, 0xD0, 0x6, 0xE9,
+                         0x10, 0x7, 0x76, 0xA0, 0x7, 0x71, 0x60, 0x7, 0x6D,
+                         0x90, 0xE, 0x73, 0x20, 0x7, 0x7A, 0x30, 0x7, 0x72,
+                         0xD0, 0x6, 0xE9, 0x60, 0x7, 0x74, 0xA0, 0x7, 0x76,
+                         0x40, 0x7, 0x6D, 0x60, 0xE, 0x71, 0x60, 0x7, 0x7A,
+                         0x10, 0x7, 0x76, 0xD0, 0x6, 0xE6, 0x30, 0x7, 0x72,
+                         0xA0, 0x7, 0x73, 0x20, 0x7, 0x6D, 0x60, 0xE, 0x76,
+                         0x40, 0x7, 0x7A, 0x60, 0x7, 0x74, 0xD0, 0x6, 0xEE,
+                         0x80, 0x7, 0x7A, 0x10, 0x7, 0x76, 0xA0, 0x7, 0x73,
+                         0x20, 0x7, 0x7A, 0x60, 0x7, 0x74, 0x30, 0xE4,
+                         0x21, 0x0, 0x0, 0x8, 0x0, 0x0, 0x0, 0x2, 0x0,
+                         0x0, 0x0, 0x20, 0xB, 0x4, 0x7, 0x0, 0x0, 0x0,
+                         0x32, 0x1E, 0x98, 0xC, 0x19, 0x11, 0x4C, 0x90,
+                         0x8C, 0x9, 0x26, 0x47, 0xC6, 0x4, 0x43, 0xBA,
+                         0x12, 0x28, 0x88, 0x62, 0x18, 0x1, 0x28, 0x84,
+                         0x22, 0x0, 0x0, 0x0, 0x79, 0x18, 0x0, 0x0, 0xE5,
+                         0x0, 0x0, 0x0, 0x33, 0x8, 0x80, 0x1C, 0xC4, 0xE1,
+                         0x1C, 0x66, 0x14, 0x1, 0x3D, 0x88, 0x43, 0x38,
+                         0x84, 0xC3, 0x8C, 0x42, 0x80, 0x7, 0x79, 0x78,
+                         0x7, 0x73, 0x98, 0x71, 0xC, 0xE6, 0x0, 0xF, 0xED,
+                         0x10, 0xE, 0xF4, 0x80, 0xE, 0x33, 0xC, 0x42, 0x1E,
+                         0xC2, 0xC1, 0x1D, 0xCE, 0xA1, 0x1C, 0x66, 0x30,
+                         0x5, 0x3D, 0x88, 0x43, 0x38, 0x84, 0x83, 0x1B,
+                         0xCC, 0x3, 0x3D, 0xC8, 0x43, 0x3D, 0x8C, 0x3,
+                         0x3D, 0xCC, 0x78, 0x8C, 0x74, 0x70, 0x7, 0x7B,
+                         0x8, 0x7, 0x79, 0x48, 0x87, 0x70, 0x70, 0x7, 0x7A,
+                         0x70, 0x3, 0x76, 0x78, 0x87, 0x70, 0x20, 0x87,
+                         0x19, 0xCC, 0x11, 0xE, 0xEC, 0x90, 0xE, 0xE1,
+                         0x30, 0xF, 0x6E, 0x30, 0xF, 0xE3, 0xF0, 0xE, 0xF0,
+                         0x50, 0xE, 0x33, 0x10, 0xC4, 0x1D, 0xDE, 0x21,
+                         0x1C, 0xD8, 0x21, 0x1D, 0xC2, 0x61, 0x1E, 0x66,
+                         0x30, 0x89, 0x3B, 0xBC, 0x83, 0x3B, 0xD0, 0x43,
+                         0x39, 0xB4, 0x3, 0x3C, 0xBC, 0x83, 0x3C, 0x84,
+                         0x3, 0x3B, 0xCC, 0xF0, 0x14, 0x76, 0x60, 0x7,
+                         0x7B, 0x68, 0x7, 0x37, 0x68, 0x87, 0x72, 0x68,
+                         0x7, 0x37, 0x80, 0x87, 0x70, 0x90, 0x87, 0x70,
+                         0x60, 0x7, 0x76, 0x28, 0x7, 0x76, 0xF8, 0x5, 0x76,
+                         0x78, 0x87, 0x77, 0x80, 0x87, 0x5F, 0x8, 0x87,
+                         0x71, 0x18, 0x87, 0x72, 0x98, 0x87, 0x79, 0x98,
+                         0x81, 0x2C, 0xEE, 0xF0, 0xE, 0xEE, 0xE0, 0xE,
+                         0xF5, 0xC0, 0xE, 0xEC, 0x30, 0x3, 0x62, 0xC8,
+                         0xA1, 0x1C, 0xE4, 0xA1, 0x1C, 0xCC, 0xA1, 0x1C,
+                         0xE4, 0xA1, 0x1C, 0xDC, 0x61, 0x1C, 0xCA, 0x21,
+                         0x1C, 0xC4, 0x81, 0x1D, 0xCA, 0x61, 0x6, 0xD6,
+                         0x90, 0x43, 0x39, 0xC8, 0x43, 0x39, 0x98, 0x43,
+                         0x39, 0xC8, 0x43, 0x39, 0xB8, 0xC3, 0x38, 0x94,
+                         0x43, 0x38, 0x88, 0x3, 0x3B, 0x94, 0xC3, 0x2F,
+                         0xBC, 0x83, 0x3C, 0xFC, 0x82, 0x3B, 0xD4, 0x3,
+                         0x3B, 0xB0, 0xC3, 0xC, 0xC7, 0x69, 0x87, 0x70,
+                         0x58, 0x87, 0x72, 0x70, 0x83, 0x74, 0x68, 0x7,
+                         0x78, 0x60, 0x87, 0x74, 0x18, 0x87, 0x74, 0xA0,
+                         0x87, 0x19, 0xCE, 0x53, 0xF, 0xEE, 0x0, 0xF, 0xF2,
+                         0x50, 0xE, 0xE4, 0x90, 0xE, 0xE3, 0x40, 0xF, 0xE1,
+                         0x20, 0xE, 0xEC, 0x50, 0xE, 0x33, 0x20, 0x28,
+                         0x1D, 0xDC, 0xC1, 0x1E, 0xC2, 0x41, 0x1E, 0xD2,
+                         0x21, 0x1C, 0xDC, 0x81, 0x1E, 0xDC, 0xE0, 0x1C,
+                         0xE4, 0xE1, 0x1D, 0xEA, 0x1, 0x1E, 0x66, 0x18,
+                         0x51, 0x38, 0xB0, 0x43, 0x3A, 0x9C, 0x83, 0x3B,
+                         0xCC, 0x50, 0x24, 0x76, 0x60, 0x7, 0x7B, 0x68,
+                         0x7, 0x37, 0x60, 0x87, 0x77, 0x78, 0x7, 0x78,
+                         0x98, 0x51, 0x4C, 0xF4, 0x90, 0xF, 0xF0, 0x50,
+                         0xE, 0x33, 0x1E, 0x6A, 0x1E, 0xCA, 0x61, 0x1C,
+                         0xE8, 0x21, 0x1D, 0xDE, 0xC1, 0x1D, 0x7E, 0x1,
+                         0x1E, 0xE4, 0xA1, 0x1C, 0xCC, 0x21, 0x1D, 0xF0,
+                         0x61, 0x6, 0x54, 0x85, 0x83, 0x38, 0xCC, 0xC3,
+                         0x3B, 0xB0, 0x43, 0x3D, 0xD0, 0x43, 0x39, 0xFC,
+                         0xC2, 0x3C, 0xE4, 0x43, 0x3B, 0x88, 0xC3, 0x3B,
+                         0xB0, 0xC3, 0x8C, 0xC5, 0xA, 0x87, 0x79, 0x98,
+                         0x87, 0x77, 0x18, 0x87, 0x74, 0x8, 0x7, 0x7A,
+                         0x28, 0x7, 0x72, 0x98, 0x81, 0x5C, 0xE3, 0x10,
+                         0xE, 0xEC, 0xC0, 0xE, 0xE5, 0x50, 0xE, 0xF3, 0x30,
+                         0x23, 0xC1, 0xD2, 0x41, 0x1E, 0xE4, 0xE1, 0x17,
+                         0xD8, 0xE1, 0x1D, 0xDE, 0x1, 0x1E, 0x66, 0x48,
+                         0x19, 0x3B, 0xB0, 0x83, 0x3D, 0xB4, 0x83, 0x1B,
+                         0x84, 0xC3, 0x38, 0x8C, 0x43, 0x39, 0xCC, 0xC3,
+                         0x3C, 0xB8, 0xC1, 0x39, 0xC8, 0xC3, 0x3B, 0xD4,
+                         0x3, 0x3C, 0xCC, 0x48, 0xB4, 0x71, 0x8, 0x7, 0x76,
+                         0x60, 0x7, 0x71, 0x8, 0x87, 0x71, 0x58, 0x87,
+                         0x19, 0xDB, 0xC6, 0xE, 0xEC, 0x60, 0xF, 0xED,
+                         0xE0, 0x6, 0xF0, 0x20, 0xF, 0xE5, 0x30, 0xF, 0xE5,
+                         0x20, 0xF, 0xF6, 0x50, 0xE, 0x6E, 0x10, 0xE, 0xE3,
+                         0x30, 0xE, 0xE5, 0x30, 0xF, 0xF3, 0xE0, 0x6, 0xE9,
+                         0xE0, 0xE, 0xE4, 0x50, 0xE, 0xF8, 0x30, 0x23,
+                         0xE2, 0xEC, 0x61, 0x1C, 0xC2, 0x81, 0x1D, 0xD8,
+                         0xE1, 0x17, 0xEC, 0x21, 0x1D, 0xE6, 0x21, 0x1D,
+                         0xC4, 0x21, 0x1D, 0xD8, 0x21, 0x1D, 0xE8, 0x21,
+                         0x1F, 0x66, 0x20, 0x9D, 0x3B, 0xBC, 0x43, 0x3D,
+                         0xB8, 0x3, 0x39, 0x94, 0x83, 0x39, 0xCC, 0x58,
+                         0xBC, 0x70, 0x70, 0x7, 0x77, 0x78, 0x7, 0x7A,
+                         0x8, 0x7, 0x7A, 0x48, 0x87, 0x77, 0x70, 0x87,
+                         0x19, 0xCB, 0xE7, 0xE, 0xEF, 0x30, 0xF, 0xE1,
+                         0xE0, 0xE, 0xE9, 0x40, 0xF, 0xE9, 0xA0, 0xF, 0xE5,
+                         0x30, 0xC3, 0x1, 0x3, 0x73, 0xA8, 0x7, 0x77, 0x18,
+                         0x87, 0x5F, 0x98, 0x87, 0x70, 0x70, 0x87, 0x74,
+                         0xA0, 0x87, 0x74, 0xD0, 0x87, 0x72, 0x98, 0x81,
+                         0x84, 0x41, 0x39, 0xE0, 0xC3, 0x38, 0xB0, 0x43,
+                         0x3D, 0x90, 0x43, 0x39, 0xCC, 0x40, 0xC4, 0xA0,
+                         0x1D, 0xCA, 0xA1, 0x1D, 0xE0, 0x41, 0x1E, 0xDE,
+                         0xC1, 0x1C, 0x66, 0x24, 0x63, 0x30, 0xE, 0xE1,
+                         0xC0, 0xE, 0xEC, 0x30, 0xF, 0xE9, 0x40, 0xF, 0xE5,
+                         0x30, 0x43, 0x21, 0x83, 0x75, 0x18, 0x7, 0x73,
+                         0x48, 0x87, 0x5F, 0xA0, 0x87, 0x7C, 0x80, 0x87,
+                         0x72, 0x98, 0xB1, 0x94, 0x1, 0x3C, 0x8C, 0xC3,
+                         0x3C, 0x94, 0xC3, 0x38, 0xD0, 0x43, 0x3A, 0xBC,
+                         0x83, 0x3B, 0xCC, 0xC3, 0x8C, 0xC5, 0xC, 0x48,
+                         0x21, 0x15, 0x42, 0x61, 0x1E, 0xE6, 0x21, 0x1D,
+                         0xCE, 0xC1, 0x1D, 0x52, 0x81, 0x14, 0x66, 0x4C,
+                         0x67, 0x30, 0xE, 0xEF, 0x20, 0xF, 0xEF, 0xE0,
+                         0x6, 0xEF, 0x50, 0xF, 0xF4, 0x30, 0xF, 0xE9, 0x40,
+                         0xE, 0xE5, 0xE0, 0x6, 0xE6, 0x20, 0xF, 0xE1, 0xD0,
+                         0xE, 0xE5, 0x30, 0xA3, 0x40, 0x83, 0x76, 0x68,
+                         0x7, 0x79, 0x8, 0x87, 0x19, 0x52, 0x1A, 0xB8,
+                         0xC3, 0x3B, 0x84, 0x3, 0x3B, 0xA4, 0x43, 0x38,
+                         0xCC, 0x83, 0x1B, 0x84, 0x3, 0x39, 0x90, 0x83,
+                         0x3C, 0xCC, 0x3, 0x3C, 0x84, 0xC3, 0x38, 0x94,
+                         0xC3, 0xC, 0x46, 0xD, 0xC6, 0x21, 0x1C, 0xD8,
+                         0x81, 0x1D, 0xCA, 0xA1, 0x1C, 0x7E, 0x81, 0x1E,
+                         0xF2, 0x1, 0x1E, 0xCA, 0x61, 0xC6, 0xB1, 0x6,
+                         0xEE, 0xF0, 0xE, 0xE6, 0x20, 0xF, 0xE5, 0x50,
+                         0xE, 0x33, 0x1C, 0x36, 0x20, 0x7, 0x7C, 0x70,
+                         0x3, 0x77, 0x78, 0x7, 0x77, 0xA8, 0x7, 0x77, 0x48,
+                         0x7, 0x73, 0x78, 0x7, 0x79, 0x68, 0x87, 0x19,
+                         0x55, 0x1B, 0x90, 0x3, 0x3E, 0xB8, 0xC1, 0x38,
+                         0xBC, 0x83, 0x3B, 0xD0, 0x83, 0x3C, 0xBC, 0x3,
+                         0x3B, 0x98, 0x3, 0x3B, 0xBC, 0xC3, 0x3D, 0xB8,
+                         0x1, 0x3A, 0xA4, 0x83, 0x3B, 0xD0, 0xC3, 0x3C,
+                         0xCC, 0x58, 0xDC, 0x80, 0x1C, 0xF0, 0xC1, 0xD,
+                         0xE0, 0x41, 0x1E, 0xCA, 0x61, 0x1C, 0xD2, 0x61,
+                         0x1E, 0xCA, 0x1, 0x0, 0x79, 0x28, 0x0, 0x0, 0x52,
+                         0x0, 0x0, 0x0, 0xC2, 0x3C, 0x90, 0x40, 0x86, 0x10,
+                         0x19, 0x32, 0xE2, 0x64, 0x90, 0x40, 0x46, 0x2,
+                         0x19, 0x23, 0x23, 0x46, 0x2, 0x13, 0x24, 0xC6,
+                         0x0, 0x13, 0x74, 0xCE, 0x61, 0x8C, 0x2D, 0xCC,
+                         0xED, 0xC, 0xC4, 0xAE, 0x4C, 0x6E, 0x2E, 0xED,
+                         0xCD, 0xD, 0x44, 0x46, 0xC6, 0x5, 0xC6, 0x5, 0xE6,
+                         0x2C, 0x8D, 0xE, 0x4, 0xE5, 0x2C, 0x8D, 0xE, 0xE8,
+                         0x2C, 0x8D, 0xE, 0xAD, 0x4E, 0xCC, 0x65, 0xEC,
+                         0xAD, 0x4D, 0x87, 0x8D, 0xCD, 0xAE, 0xED, 0x85,
+                         0x8D, 0xCD, 0xAE, 0xAD, 0x5, 0x4E, 0xEE, 0x4D,
+                         0xAD, 0x6C, 0x8C, 0xCE, 0xE5, 0x2C, 0x8D, 0xE,
+                         0xA4, 0xEC, 0xC6, 0x86, 0xA6, 0x2C, 0x26, 0x7,
+                         0xA6, 0xAC, 0xC, 0x26, 0x26, 0xE7, 0x46, 0x6C,
+                         0x2C, 0xA6, 0xC, 0x66, 0xA6, 0x6C, 0xC6, 0x4C,
+                         0x6, 0x86, 0x6C, 0x4C, 0x6, 0x46, 0xCC, 0x66,
+                         0x6C, 0x2C, 0xC, 0x27, 0x46, 0x6C, 0x86, 0x6C,
+                         0x2C, 0xE5, 0x8, 0x63, 0x73, 0x87, 0x68, 0xB,
+                         0x4B, 0x73, 0x3B, 0xCA, 0xDD, 0x18, 0x5A, 0x98,
+                         0xDC, 0xD7, 0x5C, 0x9A, 0x5E, 0xD9, 0x69, 0xCC,
+                         0xE4, 0xC2, 0xDA, 0xCA, 0x5A, 0xE0, 0xDE, 0xD2,
+                         0xDC, 0xE8, 0xCA, 0xE4, 0x86, 0x20, 0x1C, 0xC1,
+                         0x10, 0x84, 0x43, 0x18, 0x82, 0x70, 0xC, 0x43,
+                         0x10, 0xE, 0x62, 0x8, 0x42, 0x1, 0xC, 0x41, 0x38,
+                         0x8A, 0x21, 0x8, 0x87, 0x31, 0x6, 0xC1, 0x38,
+                         0xC6, 0x10, 0x4, 0x63, 0x18, 0x4, 0x24, 0x19,
+                         0x83, 0x60, 0x24, 0x63, 0x18, 0xC, 0xC3, 0x18,
+                         0x83, 0xB0, 0x44, 0x63, 0x28, 0x94, 0x1, 0x0,
+                         0xA4, 0x31, 0xC, 0x6, 0xB1, 0x8C, 0x61, 0x60,
+                         0xA, 0xC6, 0x24, 0x64, 0x78, 0x2E, 0x76, 0x61,
+                         0x6C, 0x76, 0x65, 0x72, 0x43, 0x9, 0x18, 0xA3,
+                         0xB0, 0xB1, 0xD9, 0xB5, 0xB9, 0xA4, 0x91, 0x95,
+                         0xB9, 0xD1, 0xD, 0x25, 0x68, 0x8C, 0x43, 0x86,
+                         0xE7, 0x32, 0x87, 0x16, 0x46, 0x56, 0x26, 0xD7,
+                         0xF4, 0x46, 0x56, 0xC6, 0x36, 0x94, 0xC0, 0x31,
+                         0xA, 0x19, 0x9E, 0x8B, 0x5D, 0x99, 0xDC, 0x5C,
+                         0xDA, 0x9B, 0xDB, 0x50, 0x82, 0xC7, 0x38, 0x64,
+                         0x78, 0x2E, 0x65, 0x6E, 0x74, 0x72, 0x79, 0x50,
+                         0x6F, 0x69, 0x6E, 0x74, 0x73, 0x43, 0x9, 0x24,
+                         0x13, 0xB1, 0xB1, 0xD9, 0xB5, 0xB9, 0xB4, 0xBD,
+                         0x91, 0xD5, 0xB1, 0x95, 0xB9, 0x98, 0xB1, 0x85,
+                         0x9D, 0xCD, 0xD, 0x45, 0x98, 0x28, 0x0, 0x71,
+                         0x20, 0x0, 0x0, 0...
[truncated]

Copy link
Contributor

@inbelic inbelic left a comment

Choose a reason for hiding this comment

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

Largely LGTM, I will note the pr description seems incomplete/mis-typed

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: file naming convention seems to be .test

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm okay with switching to .yaml for DXContainer tests, if you prefer (as long as they contain YAML, obviosuly). At the time llvm-objcopy was being brought up,".yaml" wasn't recognised as a test file extension, hence the generic ".test" was used.

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm okay with switching to .yaml for DXContainer tests, if you prefer (as long as they contain YAML, obviosuly). At the time llvm-objcopy was being brought up,".yaml" wasn't recognised as a test file extension, hence the generic ".test" was used.

Expected<std::unique_ptr<FileOutputBuffer>> BufferOrErr =
FileOutputBuffer::create(Filename, Contents.size());
if (!BufferOrErr)
return createFileError(Filename, 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.

Test cases for this and other error paths are required.

Copy link
Collaborator

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).

../llvm/lib/ObjCopy/DXContainer/DXContainerObjcopy.cpp
../llvm/test/tools/llvm-objcopy/DXContainer/dump-section-errors.yaml
../llvm/test/tools/llvm-objcopy/DXContainer/dump-section.yaml
# RUN: not llvm-objcopy --dump-section=FKE3=%t.fek1 %t.dxbc 2>&1 | FileCheck %s --check-prefix=CHECK-MISSING
# RUN: not llvm-objcopy --dump-section=FKE2=%t/does_not_exist/.fek2 %t.dxbc 2>&1 | FileCheck %s --check-prefix=CHECK-BAD-PATH

# CHECK-EMPTY: error: {{.*}}: part 'FKE0' is empty
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's explicitly match the file name (use -DFILE=... in the FileCheck command and [[FILE]] here, or equivalent): we've had problems in the past with --dump-section for other formats resulting in the wrong file name being used in error messages.


# CHECK-EMPTY: error: {{.*}}: part 'FKE0' is empty
# CHECK-MISSING: error: {{.*}}: part 'FKE3' not found
# CHECK-BAD-PATH: error: {{.*}}: No such file or directory
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should use the lit substitution for the corresponding error message type, since the message is platform-specific (the test currently fails on Windows). I believe the substitution you'll need is %errc_ENOENT (which you should combine with FileCheck's -D option, similar to above).

Expected<std::unique_ptr<FileOutputBuffer>> BufferOrErr =
FileOutputBuffer::create(Filename, Contents.size());
if (!BufferOrErr)
return createFileError(Filename, 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.

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

# RUN: llvm-objcopy --dump-section=PSV0=%t.psv0 %t.dxbc
# RUN: wc -c %t.psv0 | FileCheck %s --check-prefix=PSV0-SIZE
# PSV0-SIZE: 76
--- !dxcontainer
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: I suggest adding a blank line between the last check and the YAML, to help clarify the YAML isn't tied to that specific case.

IncludesSource: false
Digest: [ 0x2, 0xB5, 0x24, 0x1, 0x50, 0x48, 0xC3, 0x12,
0x39, 0x28, 0xAE, 0x46, 0x68, 0x14, 0x84, 0xDA ]
- Name: ISG1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: is it possible to drop some of these parts that aren't needed for the test? It'll help future readers see what's important for the test case. (If it's not possible due to the way the data is read, that's fine of course).

## Dump the PSV0 part and verify its size.
# RUN: llvm-objcopy --dump-section=PSV0=%t.psv0 %t.dxbc
# RUN: wc -c %t.psv0 | FileCheck %s --check-prefix=PSV0-SIZE
# PSV0-SIZE: 76
Copy link
Collaborator

Choose a reason for hiding this comment

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

If there's a way we can at least partially verify the contents are correct and not just the size, that would be great. I'm slightly worried that this could miss a case where we're writing garbage/all zeros to the file rather than the real data we want.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is binary data that's a bit awkward to match against. I think the best way to do this would be to also implement -add-section so that we could extract the data and then insert it into a new file and run llvm-objdump or obj2yaml to verify.

Do you have an alternate suggestion, or would you be okay with me filing an issue to track adding -add-section in a separate PR and fixing up the testing?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Adding --add-section support seems like a reasonable approach to me. As an interim, you could use something like od just to check the first few bytes or something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants