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

[Rafael Avila de Espindola via llvm-commits]

How about having a CurAddresState pointer in the LinkerScript class?

That way the function would look like

void LinkerScript::assignAddresses(std::vector<PhdrEntry> &Phdrs) {
  auto State = make_unique<AddressState>;
  CurAddressState = State.get();
}

And valgrind/asan would trivially get an invalid use.

Cheers,
Rafael

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

> 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