[PATCH] D27613: [ELF] - Change how -Ttext/-Tdata/-Tbss works.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/Driver.cpp:584-588
+  // When user gives -Ttext/-Tdata/-Tbss GNU linkers either set addresses
+  // of corresponding sections or addressed for PT_LOADs. That means we want
+  // to have "text segment" to work with. It includes RO sections as well.
+  if (!Config->SectionStartMap.empty())
+    Config->SingleRoRx = true;
----------------
ruiu wrote:
> I do not get the meaning of this code. Can you give me an example?
Sure, simplified from testcases, imagine we have next code:

```
.text
.globl _start
_start:
 nop

.section .rodata,"a"
 .quad 0
```

We have next loads then: R (headers, .rodata), RX(.text), RW(.data).
In that case if user gives -Ttext=0x0 we want to set .text VA to that value.
gnu linkers does not apply -rosegment by default how we do.
We want to do that It think because otherwise there is no easy way to make .text to have -Ttext VA.

After applying my lines we have: RX (headers, .rodata, .text), RO(.data). So We can place .text
to be first section and assign VA starting from it.


================
Comment at: ELF/Writer.cpp:1299-1308
+  uint64_t VA = (uint64_t)-1;
+  for (auto I = Config->SectionStartMap.begin();
+       I != Config->SectionStartMap.end(); ++I) {
+    if (VA > I->second)
+      VA = I->second;
+  }
+  uint64_t HeadersSize = getHeaderSize<ELFT>();
----------------
ruiu wrote:
> What does this code do? Needs explanation.
Moved to getImageBase(), added comment.


================
Comment at: ELF/Writer.cpp:1369-1372
+  // We take max value here for case when user set VA of section using -T
+  // below address of headers. We do not align offsets then, that is
+  // consistent with bfd behavior.
+  return std::max<uintX_t>(Off, First->Offset + Sec->Addr - First->Addr);
----------------
ruiu wrote:
> This seems too tricky. Is there any better way?
Correctness of that place is under discussions in llvm-mails at this moment, I leaved it as is for now.
If we deside to leave this logic, I can suggest just to move it upper as a "better way":

```
if (!Config->SectionStartMap.empty())
  return Off;
```

That is a bit simpler though does not align file offset for data segment. I do not know if that is important.
bfd ignores aligning for text segment, I guess we can do the same for .data as well then.

Another way can probably be:

```
if (First->Addr > Sec->Addr)
  return Off;
return First->Offset + Sec->Addr - First->Addr;
```

That should keep consistency with bfd here.


https://reviews.llvm.org/D27613