[PATCH] D34345: [LLD][ELF] Reset any accumulated state before calculating addresses

[Peter Smith via llvm-commits]

I had a quick go this afternoon and it raised some interesting
questions with the CurOutSec field and the MemoryRegion. I'll keep
going and will post something for review tomorrow, I'd appreciate any
thoughts you might have, particularly with CurOutSec?

The CurOutSec is used within getSymbolValue(Loc, Name). Unfortunately
it isn't possible to just add AddressState as a parameter as this
function is stored in ScriptParser::readAssignment() when CurOutSec
isn't known; to be executed later when the context of CurOutSec is
known. I need to find a way to get the current CurOutSec from
AddressState to the executing getSymbolValue(). The simplest thing I
can think of is to leave CurOutSec outside AddressState and have the
destructor for AddressState set CurOutSec to nullptr.

Of lesser significance, the MemoryRegion is used after
AddressAssignment although the MR.Offset field isn't, it is just used
to trigger an error if the memory region overflows. My plan is to move
the Offset field out of MemoryRegion and into AddressState.

Thanks

Peter

On 4 July 2017 at 17:42, Rafael Avila de Espindola
<rafael.espindola at gmail.com> wrote:
> Peter Smith via Phabricator <reviews at reviews.llvm.org> writes:
>
>> +// Reset the members associated with address assigment to their initial values
>> +// this permits addressAssignment to be run again.
>> +void LinkerScript::resetAddressState() {
>> +  LMAOffset = 0;
>> +  CurOutSec = nullptr;
>> +  CurMemRegion = nullptr;
>> +  ThreadBssOffset = 0;
>> +  for (auto &MRI : Opt.MemoryRegions) {
>> +    MemoryRegion &MR = MRI.second;
>> +    MR.Offset = MR.Origin;
>> +  }
>> +}
>
> These are also not used after address assignment, right?
>
> I am afraid we might miss which variables have to be here. How about
> extracting those variables from LinkerScrip.h to a AddressState
> class. That way assignAddresses would look like
>
> void LinkerScript::assignAddresses(std::vector<PhdrEntry> &Phdrs) {
>   AddressState State; ...  }
>
> And we know to only put stuff in LinkerScript if it has to survive after
> assignAddresses.
>
> Cheers,
> Rafael