[PATCH] D19272: [ELF] - linkerscript AT keyword (in output section description) implemented.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/LinkerScript.cpp:334
@@ -333,1 +333,3 @@
 
+template <class ELFT> bool LinkerScript<ELFT>::hasLma(StringRef Name) {
+  for (const std::unique_ptr<BaseCommand> &Base : Opt.Commands)
----------------
ruiu wrote:
> Small but noticeable duplicate code. Please define `hasLma` using `getLma`.
> 
>   return getLma(Name) != -1;
> 
Not sure I could do that. getLma had VA argument to use as a Dot and returned
the result of expression. It would be not correct to compare expression result with -1,
also it would introduce excessive computation of expression (first for hasLma() and then for getLma()).

I reimplemented this.

================
Comment at: ELF/Writer.cpp:993
@@ -989,3 +992,3 @@
     uintX_t NewFlags = toPhdrFlags(Sec->getFlags());
-    if (Flags != NewFlags) {
+    if (Script<ELFT>::X->hasLma(Sec->getName()) || Flags != NewFlags) {
       Load = AddHdr(PT_LOAD, NewFlags);
----------------
ruiu wrote:
> I don't think this will create a valid PHDR if there's a section with an AT command followed by another section without AT.
> 
>   .foo 0x1000 : AT(0) { ... }
>   .bar 0x2000 { ... }
> 
> For example, if .bar's flags are the same as .foo's, then this code would put both .foo and .bar into the same segment.
It is expected behavior. AT starts new segment but other sections can be put inside it as well. 
It just like ADDR() for LMA.

Using next script ld will create 2 loads:

```
SECTIONS { 
  . = 0x1000; 
  .aaa : AT(0x2000) 
  { 
   *(.aaa) 
  } 
  .bbb : 
  { 
   *(.bbb) 
  } 
  .ccc : AT(0x3000) 
  { 
   *(.ccc) 
  } 
  .text : { *(.text*) }
}

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  LOAD           0x0000000000000000 0x0000000000000000 0x0000000000001000
                 0x0000000000001010 0x0000000000001010  R      200000
  LOAD           0x0000000000001010 0x0000000000001010 0x0000000000003000
                 0x0000000000000009 0x0000000000000009  R E    200000

 Section to Segment mapping:
  Segment Sections...
   00     .aaa .bbb 
   01     .ccc .text 
```


================
Comment at: ELF/Writer.cpp:1158
@@ +1157,3 @@
+
+    // Now set the segment's physical address.
+    if (H.p_type == PT_LOAD && First &&
----------------
Done.


https://reviews.llvm.org/D19272