[PATCH] D118692: [llvm-objcopy][COFF] Fix section name encoding

Nicolas Miller via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 1 03:55:55 PST 2022


npmiller created this revision.
Herald added a reviewer: alexander-shaposhnikov.
Herald added a reviewer: rupprecht.
Herald added a reviewer: jhenderson.
Herald added a subscriber: abrachet.
npmiller requested review of this revision.
Herald added subscribers: llvm-commits, MaskRay.
Herald added a project: LLVM.

This patch is bringing the section name encoding of `llvm-objcopy` to the same level as what's done in `llvm/lib/MC/WinCOFFObjectWriter.cpp`, the base64 encoding code is taken from there.

The section name encoding for `llvm-objcopy` had two main issues, the first is that the size used for the `snprintf` in the original code is incorrect because `snprintf` adds a null byte, so this code was only able to encode offsets of 6 digits, `/`, `\0` and 6 digits of the offset, rather than the 7 digits it should support.

And the second part is that it didn't support the base64 encoding for offsets larger than 7 digits.

This issue specifically showed up when using the `clang-offload-bundler` with a binary containing a lot of symbols/sections, since it uses `llvm-objcopy` to add the sections containing the offload code.

This patch may need some modifications and/or follow up.

It would be good to de-duplicate some of this code, at least the base64 encoding code, there's a base64 encoding function in support however it encodes bytes, rather than re-interpret a number into base64, but I'm not sure where it would go best.

And I'm also a little unsure on how to approach testing for this, `yaml2obj`, however it is also missing base64 encoding which should definitely be added, but even then the yaml file ends up very large to be able to test all the different encodings. The MC counterpart of this uses `llvm-mc` and assembler macros to generate the very large string tables, would it be okay to do the same in `llvm-objcopy` testing?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118692

Files:
  llvm/tools/llvm-objcopy/COFF/Writer.cpp
  llvm/tools/llvm-objcopy/COFF/Writer.h

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D118692.404064.patch
Type: text/x-patch
Size: 3994 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220201/495d9edd/attachment.bin>


More information about the llvm-commits mailing list