[PATCH] D18499: [ELF] - Implemented prototype of location counter support.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/LinkerScript.cpp:113
@@ -75,1 +112,3 @@
 
+uint64_t LinkerScript::getSectionVA(StringRef Name, uint64_t VA) {
+  auto It = SecLoc.find(Name);
----------------
ruiu wrote:
> grimar wrote:
> > ruiu wrote:
> > > So, it looks like we need to call this function with section names in the same order as the section names appear in the linker script. Is this correct? If so, is that a good API?
> > > 
> > > It feel to me that this patch doesn't capture a correct abstraction of the writer and the linker script. Both LinkerScript and Writer have only partial knowledge of the section layout, and the interaction between them is not well defined. I'd like to see a different approach.
> > > 
> > > So, if a linker script has a SECTIONS command, the entire file layout is determined by the linker script, right? Can you pass the list of sections or the symbol table to LinkerScript and let it layout all the things?
> > >> So, it looks like we need to call this function with section names in the same order as the section names appear in the linker script. Is this correct?
> > 
> > No, that not correct.
> > 
> > We are calling this when already have a list of Output sections and just need to assign addresses:
> > ```
> > for (OutputSectionBase<ELFT> *Sec : OutputSections) {
> > ...
> >     VA = Script->getSectionVA(Sec->getName(), VA);
> > 
> > ```
> > 
> > It does not matter what is the order of sections in script at that time, because we already have a list of outputsections and just iterating over it.
> > Location counter can not be moved backward, so it can be moved only forward.
> > We push the VA to script because it uses is to calculate "." variable. It returns back updated location.
> So you cannot call this function with a random order, right?
> 
> Let's say you have a linker script with this command.
> 
>   SECTIONS {
>     .data : { *(.data) }
>     .text : { *(.text) }
>   }
> 
> And if you call getSectionVA(".text", VA) and then getSectionVA(".data", VA), they wouldn't return correct values, no?
That is very wierd usecase of this patch. Why would you want to call it in that order ? How can this happen now ? It's just not what this patch intended to handle.

So your idea to have something like:
```
Script->getSectionVA(Sec->getName());
```
(without current VA, which we know only during actual assigninAddress() now). 
Which will always return the VA no matter from where you are calling it. So we will not need assignAddressScript() at all then, almost, as we will already have all sections layouted.



http://reviews.llvm.org/D18499