[PATCH] D74755: [llvm-objcopy] Attribute an empty section to a segment ending at its address

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D74755#1928548 <https://reviews.llvm.org/D74755#1928548>, @jhenderson wrote:

> > In the case of nested segments, e.g. PT_LOAD⊇PT_GNU_RELRO⊇PT_TLS. We want ParentSegment to refer
> >  to PT_LOAD so that layoutSectionsForOnlyKeepDebug() will advance sh_offset to make sh_offset≡sh_addr
> >  (mod p_align), this is naturally satisfied because PT_LOAD comes before PT_TLS (GNU ld/gold/lld).
> > 
> > (If PT_LOAD comes after PT_TLS, the rule will break, but this is really impractical.)
>
> I still don't understand what is meant by "really impractical". It is not hard to write a linker script where this is the case and produce runnable output on my Linux VM, and whilst I wouldn't generally encourage people to write linker scripts, there are plenty of people out there who do for various perfectly valid reasons.


"really impratical" because - as my previous comment said, I tried to break the code but I can't. If you have such a use case, let's consider it.

By `(If PT_LOAD comes after PT_TLS, the rule will break, but this is really impractical.)`, it was my hypothesis. I can't find a use case to prove it. If PT_TLS comes before PT_LOAD, I won't be surprised if some other binary manipulation tools will break.

> I've been thinking about alternatives that would a) get rid of the size 1, and b) solve the problem, without changing the existing behaviour in bad ways for non-empty segments/sections.
> 
> The problem as I understand it is that an empty section is not allocated to an empty segment at the same location. How about we just change the `sectionWithinSegment` check for empty sections to simply say something like:
> 
>   if (Section.Size == 0 && Segment.FileSize == 0)
>     return Segment.OriginalOffset == Section.OriginalOffset;
>   // Possibly add equivalent case for SHT_NOBITS using addresses.
> 
> 
> This ensures empty sections are considered to be within empty segments if they share the same offset (I think this makes a reasonable amount of sense, although I could imagine some weird issues if you end up with multiple empty sections and segments at the same location with different alignments). We might want similar logic added to `segmentOverlapsSegment`, although I'm not sure.

As I mentioned, I am happy that we achieve @kongyi's goal by deleting code. One fallacy of GNU is that they add a bunch of rules which cannot easily be simplified now. When a concrete use case surfaces, I am happy to adapt. We may find a better way to do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74755