[PATCH] D67325: [ELF] Map the ELF header at imageBase

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.



>> One downside of this approach, albeit not a regression from the old behaviour, is that there is always an RO program segment. An alternative fix would be to detect that a program segment only contained the ELF Header and Program Header and move them to the first program segment that contained a SHF_ALLOC section. This would simplify the output when there is no .rodata  to something like (AArch64 values simulated by producing a file with just rodata):
>> 
>>   PHDR           0x000040 0x0000000000200040 0x0000000000200040 0x0000e0 0x0000e0 R   0x8
>>   LOAD           0x000000 0x0000000000200000 0x0000000000200000 0x000124 0x000124 RE 0x10000
>>    
>> 
>> Any thoughts?
> 
> The RO PT_LOAD is allocated w/o or w/ this change. We can apply the optimization, with the change to createPhdrs:
> 
>   --- a/ELF/Writer.cpp
>   +++ b/ELF/Writer.cpp
>   @@ -2116,3 +2116,6 @@ std::vector<PhdrEntry *> Writer<ELFT>::createPhdrs(Partition &part) {
>            sec == relroEnd) {
>   -      load = addHdr(PT_LOAD, newFlags);
>   +      if (load && load->lastSec == Out::programHeaders && (newFlags & PF_R))
>   +        load->p_flags = newFlags;
>   +      else
>   +        load = addHdr(PT_LOAD, newFlags);
>          flags = newFlags;
> 
> 
> It will affect 125 tests, though, so I'm not sure whether we want to apply this optimization.
>  A user can use --no-rosegment to ensure the RO PT_LOAD does not exist.

I think the number of people that have no rodata at all in a executable or shared object with allocated headers will be sufficiently small that it isn't worth updating 125 tests for. I'm happy enough with the change now that I understand it better. I think it will be worth waiting a few days to see if there are any more opinions, and if there aren't any go ahead with this next week.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D67325