[PATCH] D50133: [LLD] Do not overwrite LMAOffset of PT_LOAD header
Konstantin Schwarz via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 1 03:38:38 PDT 2018
kschwarz added inline comments.
================
Comment at: ELF/LinkerScript.cpp:780
+ if ((Sec == L->FirstSec) ||
+ ((L->FirstSec == Out::ElfHeader) && (L->LastSec == Sec)))
+ L->LMAOffset = Ctx->LMAOffset;
----------------
grimar wrote:
> I think the comment above should be updated to reflect this change.
>
> But also: your test pass if I reduce this to
>
> ```
> if (PhdrEntry *L = Ctx->OutSec->PtLoad)
> if (Sec == L->FirstSec)
> L->LMAOffset = Ctx->LMAOffset;
> ```
>
> Can you add a test/update it accordingly to check the rest?
> (Alternatively, we can go with the reduced code in this patch I think, and leave the rest to follow up patch).
>
> Also, I think that `(L->FirstSec == Out::ElfHeader)` condition is not correct. Because
> `FirstSec` can be not the `ElfHeader`, but `ProgramHeaders`:
> https://github.com/llvm-mirror/lld/blob/master/ELF/LinkerScript.cpp#L1073
>
>
The following tests rely on the additional check and fail if it is removed:
lld :: ELF/linkerscript/at-addr.s
lld :: ELF/linkerscript/at.s
lld :: ELF/linkerscript/loadaddr.s
lld :: ELF/linkerscript/merge-header-load.s
lld :: ELF/linkerscript/overlay.test
lld :: ELF/linkerscript/segment-headers.s
lld :: ELF/note-loadaddr.s
The ELF header section is added to the first PT_LOAD header unconditionally here: https://github.com/llvm-mirror/lld/blob/master/ELF/Writer.cpp#L1807
I think we need to check for both ElfHeader and ProgramHeaders and will try to come up with an additional test case for that.
Repository:
rLLD LLVM Linker
https://reviews.llvm.org/D50133
More information about the llvm-commits
mailing list