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

[Rafael Avila de Espindola via llvm-commits]

I have applied the patch and spent some time comparing what bfd and
this patch do. With that, I have some extra comments.

Rafael Avila de Espindola <rafael.espindola at gmail.com> writes:

>> +  // 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;
>
> Not sure I understand the comment. Maybe just say that some programs
> using -TText expect to also see the ro section in the selected address?

This looks to be independent from the rest of the patch and should be in
a followup if needed.

>
>>  // Parses -image-base option.
>>  static uint64_t getImageBase(opt::InputArgList &Args) {
>> +  // When -T<section> option is specified, lowest VA value
>> +  // becomes the image base address.
>> +  if (!Config->SectionStartMap.empty()) {
>> +    uint64_t VA = (uint64_t)-1;
>> +    for (auto I = Config->SectionStartMap.begin();
>> +         I != Config->SectionStartMap.end(); ++I) {
>> +      if (VA > I->second)
>> +        VA = I->second;
>> +    }
>> +    return VA;
>> +  }
>
> If that is higher than the default base we probably want to use the
> default, no?

The situation is actually quite a bit more complicated. If we want to
fully support putting a section at a given address, what we have to do
is

* Find the minimum address requested for any section.
* See if the headers would fit in that, if so, subtract the header
  size. If not, remember that we are not allocating the headers.
* If the computed value is less than the image base, adjust the image
  base.

A particular annoyance is that mips writes the image base to the dynamic
table, so we have to this before finalize().

The attached patch is a work in progress. It doesn't cover the mips case
and doesn't make sure different sections with the address set are
ordered correctly.

This is becoming a bit too much of a cargo cult I think. We are
flipping bits to look more like bfd until something works.

Ed, before we continue down this path I want to understand exactly what
the boot loader build is trying to do. I will write down my guesses
based on how I expect a boot loader to work, let me know if that is not
the case:

* The binary being produced is not ELF, it is just a blob.
* The blob will be loaded at address 0 by the bios, or the previous
  stage.
* The code is not position independent, so addresses do have to start at 0.

If the above is really true, then I think the correct thing to do is
only support setting section addresses if the output is binary, not
elf. This avoids all the above problems:

* There are to PT_LOADs to worry about.
* getHeaderSize returns 0, so it always "fits".

Simon, what about mips-got-page.s? I assume you used --section-start
just for convenience in the test, correct? Could you change it to avoid
it? A linker script should be fine.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: t.diff
Type: text/x-patch
Size: 6266 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161212/759ccd05/attachment.bin>
-------------- next part --------------

Cheers,
Rafeal