[PATCH] D38361: [ELF] Stop setting output section size early

[Rafael Avila de Espindola via llvm-commits]

Peter Smith <peter.smith at linaro.org> writes:

>> I don't like the idea of adding a member for the section position. At no
>> point we need the position and the offset simultaneously and having both
>> variables means they can get inconsistent.
>>
>> One way to look at the current issue is that the early estimate of the
>> offset is correct in the common case, which makes it really easy to use
>> it incorrectly before the final value is computed.
>>
>> If we make the value almost always wrong, that problem goes away. What
>> do you think of the attached patch? We could rename OutSecOff to
>> OutSecOffsetOrIndex, but that doesn't seem worth it.
>>
>>
>
> A previous linker that I worked on tracked the state of some
> properties of the link such as "All Sections Assigned An Offset", "All
> Sections Given Final Address", functions would check properties that
> they required as pre-conditions and would update the properties if
> they invalidated them. I'm hesitant to propose that as a solution as
> it requires quite a bit of discipline to check and update all the
> state or introducing a lot of wrapper functions. Perhaps more
> pre-condition comments using keywords might help.

I think the previous lld version also had a notion of a pass manager. It
does seem a bit much for what a linker has to do.

>> +  // Calculate the section offsets and size pre-compression. Doing this now
>> +  // instead of delaying maybeCompress until after a run of assignAddresses
>> +  // means that we don't support linker scripts adding data to compressed debug
>> +  // sections. That is probably a reasonable restriction.
>> +  Size = 0;
>> +  for (BaseCommand *Cmd : SectionCommands)
>> +    if (auto *ISD = dyn_cast<InputSectionDescription>(Cmd))
>> +      for (InputSection *IS : ISD->Sections) {
>> +        IS->OutSecOff = alignTo(Size, IS->Alignment);
>> +        Size = IS->OutSecOff + IS->getSize();
>> +      }
>> +
>
> I think that is a reasonable feature trade off.

OK. James, if you agree feel free to commit as it is basically your
patch without the extra variables.

> I'm wondering if it is
> feasible to check and disable compression if a non input section
> description command is detected in the debug section being compressed?

It should be possible to at least produce an error message.

I am not too sure why the compression is done as early as it is.

Cheers,
Rafael