[llvm-objcopy] Allow empty sections in --dump-section

[rupprecht]

Summary

When running llvm-objcopy --dump-section=.foo=file, and .foo is an empty section, we currently either:

• Crash (in debug mode) for MachO/wasm
• Abort with a "empty section" type of error message for ELF

This patch changes the behavior to allow dumping an empty section to a file. The file created will be empty.

For ELF:

• If the type is SHT_NOBITS, then it really doesn't make sense to dump that kind of section, so continue to error. To be fully compatible with GNU objcopy (which also prints the message), we should not abort, but just print a warning and continue the rest of llvm-objcopy.
• If the type is something else (e.g. SHT_PROGBITS), but happens to have an empty size, then allow creating an empty file. This is incompatible with GNU objcopy (which prints a warning and doesn't create a file), but should be more useful; it means users can call llvm-objcopy without checking if the section is empty first.

This only adds test cases for ELF. For wasm, a binary was manually tested. For MachO, yaml2obj crashes when trying to create an empty section, so we'd need to fix that first.

Fixes llvm.org/PR45159

[MaskRay]

GNU objcopy emits can't dump section - it has no contents (bfd_nonfatal_message) for SHT_NOBITS, and can't dump section - it is empty (bfd_nonfatal_message) for a sh_size=0 section.

Some use cases are messages prefixed with warning: or error:, they are apparently intended as warnings/errors. Some are not => some may be conceptually considered as warnings, but some may just be informative.

I agree that disallowing a sh_size=0 section does not is not necessary.

llvm/test/tools/llvm-objcopy/ELF/dump-section-empty.test

1

I think placing the test with ELF/dump-section.test is also fine.

You can add an empty section to the existing YAML in ELF/dump-section.test.

14

Decrease indentation. Machine: is the widest key. The values should be aligned so that Machine: EM_X86_64 contains exactly one space.

[MaskRay]

CC @dschuff who added --dump-section support for wasm in D70970. I hope Derek can give advice how to craft an empty section test in test/tools/llvm-objcopy/wasm/dump-section.test

[jhenderson]

Ideally, we'd fix yaml2obj for Mach-O first, before checking this in without a test.

llvm/test/tools/llvm-objcopy/ELF/dump-section-empty.test

22

To emphasise the point, this should probably have a non-zero size.

llvm/test/tools/llvm-objcopy/ELF/dump-section.test

6

I'm not sure I agree with removing this test from here. A nobits section is not dumpable because it is nobits, not because it is empty.

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

291

Is this really a FIXME? It seems like it should be an error here, because llvm-objcopy cannot do something you've explicitly asked it to do.

I don't know what GNU objcopy does, but I think this is an okay place to diverge.

llvm/test/tools/llvm-objcopy/ELF/dump-section-empty.test

+## Test that --dump-section works on a section with no contents.
+# RUN: yaml2obj %s -o %t.o
+
+# RUN: llvm-objcopy --dump-section .empty=%t-empty.o %t.o %t2.o
+# RUN: wc -c %t-empty.o | FileCheck %s --check-prefix=EMPTY
+
+## FIXME: this should be a warning, not an error. (Keep the text matching the
+## same, but it should pass without 'not').
+# RUN: not llvm-objcopy --dump-section .nobits=%t-nobits.o %t.o %t2.o 2>&1 \
+# RUN:   | FileCheck %s --check-prefix=NOBITS -DINPUT=%t.o
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .empty
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_WRITE ]
+  - Name:            .nobits
+    Type:            SHT_NOBITS
+    Flags:           [ SHF_WRITE ]
+
+# EMPTY: 0
+# NOBITS: error: '[[INPUT]]': cannot dump section '.nobits': it has no contents

llvm/test/tools/llvm-objcopy/ELF/dump-section.test

 # RUN: yaml2obj %s -o %t
 # RUN: llvm-objcopy -O binary -j .text %t %t2
 # RUN: llvm-objcopy -O binary --only-section .text %t %t3
 # RUN: llvm-objcopy --dump-section .text=%t4 %t %t5
 # RUN: llvm-objcopy --dump-section .foo=%t6 %t %t7
-# RUN: not llvm-objcopy --dump-section .bar=%t8 %t %t9 2>&1 | FileCheck %s --check-prefix=NOBITS -DINPUT=%t
 # RUN: od -t x1 %t2 | FileCheck %s --ignore-case
 # RUN: od -t x1 %t6 | FileCheck %s --ignore-case --check-prefix=NON-ALLOC
 # RUN: wc -c %t2 | FileCheck %s --check-prefix=SIZE
 # RUN: diff %t2 %t3
 # RUN: diff %t4 %t3
 
 !ELF
 FileHeader:
   Class:           ELFCLASS64
   Data:            ELFDATA2LSB
   Type:            ET_EXEC
   Machine:         EM_X86_64
 Sections:
   - Name:            .text
     Type:            SHT_PROGBITS
     Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
     AddressAlign:    0x0000000000001000
     Content:         "DEADBEEF"
   - Name:            .foo
     Type:            SHT_PROGBITS
     Flags:           [ SHF_WRITE ]
     Content:         "CAFE"
-  - Name:            .bar
-    Type:            SHT_NOBITS
-    Flags:           [ SHF_WRITE ]
 ProgramHeaders:
 - Type: PT_LOAD
   Flags: [ PF_X, PF_R ]
   Sections:
     - Section: .text
 
 #CHECK: 0000000 de ad be ef
 
 #NON-ALLOC: 0000000 ca fe
 
 #SIZE: 4
-
-#NOBITS: error: '[[INPUT]]': cannot dump section '.bar': it has no contents

llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp

if (Error E = Writer->finalize())
     return E;
   return Writer->write();
 }
 
 static Error dumpSectionToFile(StringRef SecName, StringRef Filename,
                                Object &Obj) {
   for (auto &Sec : Obj.sections()) {
     if (Sec.Name == SecName) {
-      if (Sec.OriginalData.empty())
+      // FIXME: this should be a warning, not an error.
+      if (Sec.Type == SHT_NOBITS)
         return createStringError(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();
-      std::unique_ptr<FileOutputBuffer> Buf = std::move(*BufferOrErr);
-      std::copy(Sec.OriginalData.begin(), Sec.OriginalData.end(),
-                Buf->getBufferStart());
-      if (Error E = Buf->commit())
+      FileBuffer Buf = FileBuffer(Filename);
+      if (Error E = Buf.allocate(Sec.OriginalData.size()))
         return E;
-      return Error::success();
+      if (!Sec.OriginalData.empty())
+        llvm::copy(Sec.OriginalData, Buf.getBufferStart());
+      return Buf.commit();
     }
   }
   return createStringError(object_error::parse_failed, "section '%s' not found",
                            SecName.str().c_str());
 }
 
 static bool isCompressable(const SectionBase &Sec) {
   return !(Sec.Flags & ELF::SHF_COMPRESSED) &&

llvm/tools/llvm-objcopy/ELF/Object.h

 #include "CopyConfig.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/MC/StringTableBuilder.h"
 #include "llvm/Object/ELFObjectFile.h"
 #include "llvm/Support/Errc.h"
-#include "llvm/Support/FileOutputBuffer.h"
 #include <cstddef>
 #include <cstdint>
 #include <functional>
 #include <memory>
 #include <set>
 #include <vector>
 
 namespace llvm {

llvm/tools/llvm-objcopy/ELF/Object.cpp

 #include "llvm/ADT/Twine.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/BinaryFormat/ELF.h"
 #include "llvm/MC/MCTargetOptions.h"
 #include "llvm/Object/ELFObjectFile.h"
 #include "llvm/Support/Compression.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/ErrorHandling.h"
-#include "llvm/Support/FileOutputBuffer.h"
 #include "llvm/Support/Path.h"
 #include <algorithm>
 #include <cstddef>
 #include <cstdint>
 #include <iterator>
 #include <unordered_set>
 #include <utility>
 #include <vector>

llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp

static LoadCommand buildRPathLoadCommand(StringRef Path) {
   return LC;
 }
 
 static Error dumpSectionToFile(StringRef SecName, StringRef Filename,
                                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();
-        std::unique_ptr<FileOutputBuffer> Buf = std::move(*BufferOrErr);
-        llvm::copy(Sec->Content, Buf->getBufferStart());
-
-        if (Error E = Buf->commit())
+        FileBuffer Buf = FileBuffer(Filename);
+        if (Error E = Buf.allocate(Sec->Content.size()))
           return E;
-        return Error::success();
+        if (!Sec->Content.empty())
+          llvm::copy(Sec->Content, Buf.getBufferStart());
+        return Buf.commit();
       }
     }
 
   return createStringError(object_error::parse_failed, "section '%s' not found",
                            SecName.str().c_str());
 }
 
 static Error addSection(StringRef SecName, StringRef Filename, Object &Obj) {

llvm/tools/llvm-objcopy/wasm/WasmObjcopy.cpp

 
 using namespace object;
 
 static Error dumpSectionToFile(StringRef SecName, StringRef Filename,
                                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();
-      std::unique_ptr<FileOutputBuffer> Buf = std::move(*BufferOrErr);
-      std::copy(Contents.begin(), Contents.end(), Buf->getBufferStart());
-      if (Error E = Buf->commit())
+      FileBuffer Buf = FileBuffer(Filename);
+      if (Error E = Buf.allocate(Contents.size()))
         return E;
-      return Error::success();
+      if (!Contents.empty())
+        llvm::copy(Contents, Buf.getBufferStart());
+      return Buf.commit();
     }
   }
   return createStringError(errc::invalid_argument, "section '%s' not found",
                            SecName.str().c_str());
 }
 static Error handleArgs(const CopyConfig &Config, Object &Obj) {
   // Only support AddSection, DumpSection, RemoveSection for now.
   for (StringRef Flag : Config.DumpSection) {