[PATCH] D103815: [LLD][ELF] Fix PT_LOAD program header creation for NO_LOAD sections

Mon Jun 14 03:36:39 PDT 2021

kschwarz added inline comments.

================
Comment at: lld/test/ELF/linkerscript/noload.s:13
 # CHECK:      Type Offset   VirtAddr           PhysAddr
+# CHECK-NEXT: LOAD 0x001000 0x0000000000000000 0x0000000000000000
 # CHECK-NEXT: LOAD 0x001000 0x0000000000020000 0x0000000000020000
----------------
MaskRay wrote:
> kschwarz wrote:
> > GNU ld creates the following program headers:
> > 
> > ```
> >   LOAD           0x000000 0x0000000000000000 0x0000000000000000 0x000000 0x001000 RW  0x1000
> >   LOAD           0x000000 0x0000000000010000 0x0000000000010000 0x000000 0x001001 RW  0x1000
> >   LOAD           0x001000 0x0000000000020000 0x0000000000020000 0x000001 0x000001 R E 0x1000
> > ```
> > 
> > Should we create a new header whenever a section has an `addrExpr` set, similar to our condition for lmaExpr?
> > 
> > GNU ld will only create a new header if the current section is placed at a discontiguous address.
> Try merging PT_LOAD with same flags and contiguous address ranges.
> 
> Doing this rigidly can make the code more complex. In such cases we may trade the merged behavior for simpler implementation.
I had a look when/where the merging of PT_LOAD segments with contiguous address ranges should occur.
If no `addrExpr`s are used, the VMAs will usually be contiguous and we can keep the current behavior, i.e. check whether the `memregion`s are identical.

However if `addrExpr`s are used, we cannot know whether two output sections will have contiguous address ranges until after the final layout is fixed. In that case, we would need a final "merge pass", maybe at the same time we remove empty PT_LOAD segments.

If you agree, I'd like to keep these changes separate and first commit this change (creating PT_LOAD headers for NOLOAD sections).

Creating a new PT_LOAD header for every section that uses an `addrExpr` breaks quite some tests in the test suite, so that definitely needs a closer look.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103815