[PATCH] D107653: [llvm-objcopy][ELF] Avoid reordering section headers

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin updated this revision to Diff 365363.
ikudrin marked an inline comment as done.
ikudrin edited the summary of this revision.
ikudrin added a comment.

- Added a comment that 'dummy' comes before '.text' by intention.

In D107653#2936087 <https://reviews.llvm.org/D107653#2936087>, @MaskRay wrote:

> D62620 <https://reviews.llvm.org/D62620> was related to section groups: "The section header table entry for a group section must appear in the section header table before the entries for any of the sections that are members of the group."
> (ld.lld has dropped the limitation)

Right, D62620 <https://reviews.llvm.org/D62620> enforces `SHT_GROUP` sections to be moved at the start of the table. That was required because the tool sorted section headers and, without the fix, could place such sections in the table after members of their groups. This patch drops sorting, so there is no need to do anything special for group sections.

>> That can corrupt section references in the dynamic symbol table because it is a loadable section and as such is not updated by the tool. Even though the section references are not required for loading the binary correctly, they are still handy for a user who analyzes the file.
>
> All dynamic loaders I know only use 3 categories:
>
> - SHN_UNDEF
> - SHN_ABS
> - other values
>
> There are no behavior differences if an index of 1 is replaced with 2, but I agree that preserving the section index and making the output as close to the input as possible will be nice.

My point exactly. Skewed section references can bewilder a person, but not a dynamic loader.

> The approach looks good at a glance, but I'll need to look more closely tomorrow.

Thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107653/new/

https://reviews.llvm.org/D107653

Files:
  llvm/test/tools/llvm-objcopy/ELF/drawf-fission.test
  llvm/test/tools/llvm-objcopy/ELF/dwarf-fission.test
  llvm/test/tools/llvm-objcopy/ELF/dynsym-valid-refs.test
  llvm/test/tools/llvm-objcopy/ELF/group-reorder.test
  llvm/test/tools/llvm-objcopy/ELF/ihex-reader.test
  llvm/test/tools/llvm-objcopy/ELF/layout-sections-by-original-offsets.test
  llvm/test/tools/llvm-objcopy/ELF/shared-strtab-shstrtab.s
  llvm/test/tools/llvm-objcopy/ELF/strip-dwo-groups.test
  llvm/test/tools/llvm-objcopy/ELF/strip-dwo-inplace.test
  llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
  llvm/tools/llvm-objcopy/ELF/Object.cpp
  llvm/tools/llvm-objcopy/ELF/Object.h

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D107653.365363.patch
Type: text/x-patch
Size: 29026 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210810/f7926793/attachment.bin>