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

[George Rimar via llvm-commits]

>> +## Check that user can assing addressed for sections using -T option.
>> +# RUN: ld.lld -Ttext 0x0 -Tdata 0x2000 -Tbss 0x4000 %t -o %t1
>> +# RUN: llvm-objdump -section-headers %t1 | FileCheck %s --check-prefix=USERADDR
>> +# USERADDR:      Sections:
>> +# USERADDR-NEXT: Idx Name          Size      Address          Type
>> +# USERADDR-NEXT:   0               00000000 0000000000000000
>> +# USERADDR-NEXT:   1 .text         00000001 0000000000000000 TEXT DATA
>> +# USERADDR-NEXT:   2 .rodata       00000008 0000000000000001 DATA
>
>The address of .rodata is not set, should it be here instead of after .bss?

That way assumes we put all listed -T* sections first right ?

There are two ways to do this patch I think.
1) First is what this patch do, starts PT_LOADs from sections specified by -T*.
That keeps the reasonable amount of PT_LOAD, but leaves a chance to sections addresses
intersections. We probably can just error out in that case.

2) Second is to put -T* sections first. Probably more segments will be emited then.

Actually I would try to see how my approach works, it is simple but can work fine i think.

> +# USERADDR-NEXT:   3 .data         00000008 0000000000002000 DATA
> +# USERADDR-NEXT:   4 .aw           00000008 0000000000002008 DATA
> +# USERADDR-NEXT:   5 .bss          00000008 0000000000004000 BSS
>> +template <class ELFT> static uint64_t getVABase() {
>> +  if (Config->SectionStartMap.empty())
>> +    return Config->ImageBase;
>> +
>> +  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>();
>> +  if (VA > HeadersSize)
>> +    return VA - HeadersSize;
>> +  return VA;
>> +}
>
>You should be able to do this computation once and store it in
>Config->ImageBase, no?

Sure I thought about that. I was not sure it is correct as ImageBase is an cmd line option alias,
and this value is a some calculation basing on -T*. Though if you think it is fine, I am ok either.

>>    // If two sections share the same PT_LOAD the file offset is calculated
>>    // using this formula: Off2 = Off1 + (VA2 - VA1).
>> -  return First->Offset + Sec->Addr - First->Addr;
>> +  // 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);
>
>This seems wrong. If both sections are in the same PT_LOAD, the
>difference from address and offsets are set in stone.

That is not what bfd do. Problem is that First here is a elf header. Imagine we have
-Ttext=0x0.

And Sec is .text then.
That way First->Offset == 0x0, First->Addr == 0x0 and Sec->Addr == 0x0.

bfd just ignores file offsets aligning and I supposed we can do the same since that is feature
for loaders and they work with bfd. 
Gold instead sets .text VA to 0x80 or something (headers size) and keeps the aligning rules.

Which behavior we want to follow ?

Cheers,
Rafael