[PATCH] D95199: [ELF] Write output sections in PT_LOAD segment order

Peter Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Feb 7 04:18:54 PST 2021


peter.smith added a comment.

Thanks for the updates. My day job has been very busy of late so I've not got a lot of spare time to look. As it stands I'm still a bit nervous about the test changes, it is also worth mentioing that I'd want at least one of grimar and MaskRay to agree on any changes in this area as they were both initially hesitant.

I did have a thought on what might make this (and your other patch) easier to upstream. Instead of changing the code directly keep the existing code below:

  // Layout SHF_ALLOC sections before non-SHF_ALLOC sections. A non-SHF_ALLOC
  // will not occupy file offsets contained by a PT_LOAD.

Given that the problem with ordering only exists with PHDR commands, then we can do something like (pseudo code):

  if linker script has program headers {
      reorder Output Sections covered by PT_LOAD
  }
  ...
  // Layout SHF_ALLOC sections before non-SHF_ALLOC sections. A non-SHF_ALLOC
  // will not occupy file offsets contained by a PT_LOAD.

While it may be a bit more code, it is isolated to when PT_LOAD is used, which should limit the impact on the existing tests to the tests using PHDR. >From reading the partition documentation (see below) this feature is incompatible with linker scripts and PHDR anyway. This should make it easier to review, although I can't speak for MaskRay and grimar here.

For the partitions bit:
Sadly the LLD tests don't always capture all of the properties that need to hold. I've had a scan through the documentation and some of the reviews. The code does mention partiion end markers which tools like objdump use to extract partitions. I

References for the partition 
Initial RFC https://lists.llvm.org/pipermail/llvm-dev/2019-February/130583.html
LLD reviews: D60242 <https://reviews.llvm.org/D60242> D60353 <https://reviews.llvm.org/D60353> D62350 <https://reviews.llvm.org/D62350> D62349 <https://reviews.llvm.org/D62349>

The author of partitions is pcc , he may be able to comment on the changes, although is also frequently busy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95199



More information about the llvm-commits mailing list