Page MenuHomePhabricator

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

Authored by rupprecht on Mar 10 2020, 10:53 AM.

Details

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

Diff Detail

Event Timeline

rupprecht created this revision.Mar 10 2020, 10:53 AM

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.

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

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.