[PATCH] D112925: [ELF] Place an orphan section to the same memory region as its anchor section

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

I had a play around with ld.bfd and found it really hard to get anything like what I'd expect to work. I had the best luck when using ! due to the loose way the sections match and no direct assignment to memory regions.

Could you try something like this:

  .section .text.01, "ax", %progbits
   nop
  
   .section .rodata.01, "a", %progbits
   .word 10
  
   .section .init_array,"aw", at init_array
   .zero 0x10

With

  MEMORY
  {
    TEXT (RX) : ORIGIN = 0x8000, LENGTH = 0x1000
    DATA (W!X) : ORIGIN = 0x9000, LENGTH = 0x1000
  }
  
  SECTIONS {
    .text : { *(.text.01) }
  }

In this case I get the map file:

  Memory Configuration
  
  Name             Origin             Length             Attributes
  TEXT             0x0000000000008000 0x0000000000001000 xr
  DATA             0x0000000000009000 0x0000000000001000 w !x
  *default*        0x0000000000000000 0xffffffffffffffff
  
  Linker script and memory map
  
  LOAD ld.o
  
  .text           0x0000000000008000        0x4
   *(.text.01)
   .text.01       0x0000000000008000        0x4 ld.o
   .text          0x0000000000008004        0x0 ld.o
  OUTPUT(a.out elf64-littleaarch64)
  LOAD linker stubs
  
  .rodata.01      0x0000000000008004        0x4
   .rodata.01     0x0000000000008004        0x4 ld.o
  
  .init_array     0x0000000000009000       0x10
   .init_array    0x0000000000009000       0x10 ld.o

With .init_array correctly in the `W!X` region.

I think that this test case on LLD with the proposed change would put .init_array in TEXT as it would follow .rodata.01 which would be assigned to TEXT? I've not had a chance to build LLD with the patch to check. If I'm right it may be worth checking that the previous region has compatible attributes.



================
Comment at: lld/ELF/LinkerScript.cpp:903
 
+  // An allocatable orphan section should continue the previous memory region.
+  if (sec->sectionIndex == UINT32_MAX && sec->flags & SHF_ALLOC && prev)
----------------
ikudrin wrote:
> peter.smith wrote:
> > My first reaction is that I'd expect this to be below the // See if a region can be found by matching section flags.
> > 
> > Looking at LD https://sourceware.org/binutils/docs/ld/MEMORY.html 
> > ```
> > The attr string is an optional list of attributes that specify whether to use a particular memory region for an input section which is not explicitly mapped in the linker script. As described in SECTIONS, if you do not specify an output section for some input section, the linker will create an output section with the same name as the input section. If you define region attributes, the linker will use them to select the memory region for the output section that it creates.
> > ```
> > That implies that if there are attributes/flags for the memory region then we should try these first to make sure that we respect RO, RW etc. If the MEMORY region doesn't have flags then we could fall back on prev in that case. Apologies if I've missed something obvious.
> It looks like their implementation does not fully follow that rule. If the script in the test is changed like this:
> ```
> MEMORY
> {
>   TEXT (rwx) : ORIGIN = 0x8000, LENGTH = 0x1000
>   DATA (rwx) : ORIGIN = 0x9000, LENGTH = 0x1000
> }
> ...
> ```
> GNU ld still puts `.init_array` into `DATA`, not `TEXT`.
I think there is a chance that if the prev has incompatible flags with sec we could assign sec incorrectly. I think that in the majority, perhaps all real programs we'll be saved by the sorting order so this won't happen. I think that there is a possible corner case (I've put the test in the main comment). If I'm right it would be useful to test that the sec->flags are compatible with prev.  


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

https://reviews.llvm.org/D112925