[PATCH] D31888: [LLD][ELF] Always use Script::assignAddresses()

[Mikulin, Dmitry via llvm-commits]

Thanks for looking into it.

> On Apr 28, 2017, at 6:30 AM, Peter Smith via Phabricator <reviews at reviews.llvm.org> wrote:
> 
> peter.smith added a comment.
> 
> I've been having a bit of trouble trying to reproduce the problem from the source code with enough confidence to know it is the same problem you are seeing. I've definitely found one other problem which I have a fix for but I don't think it is the same thing.
> 
> - The first thing I did was to fix the problem you mention and pass AllocateHeader to FabricateDefaultCommands(). I don't see any test failures but that doesn't mean there isn't a problem.
> - From what I can see of AllocateHeader = allocateHeaders(Phdrs, OutputSections, Min) is that it returns false only when there are no PT_LOAD program headers or if HeaderSize > Min.
> - Min is set to either -1 (Unsigned so Wrap to max) or the minimum VA of the sections in the SectionStartMap. These are AFAIK only set with -Ttext=addr or --section-start .text=addr
>  - Min is set regardless of whether the section is SHF_ALLOC or not
> - If I'm correct, I assume that your target is using some kind of SectionStartMap, or you have some downstream modification that I don't know about.
>  - Can you let me know how you achieve AllocateHeader=false?

It bails out of allocateHeader() returning false right after ELF header and program header addresses are set. 

> - If AllocateHeader=false then the first call to allocateHeaders terminates before do alignDown(Min - HeaderSize)

The first time it’s called from fixHeaders() and that’s when AllocateHeader is set. 
The problem is with the second call: Min is set to MinVA and alignDown() brings it below MinVA.
We force some sections to be non-allocatable and they precede allocatable ones in the file. I’m still trying to understand the logic, but intuitively it feels like header addresses should be set once after all sections are laid out.

It’s hard to come up with a test case as what I see depends on a bunch of proprietary code.
I’ll spend some more time on it next week.

> - There is a problem in Script::fabricateDefaultCommands() in that it uses the first section in the SectionStartMap to find the minimum address instead of iterating through it and find the lowest so we can get the wrong value for lowest and get an error message from setDot(). This is trivial to fix by looping through and finding the minimum.
>  - As you don't mention an error I'm guessing that this isn't the problem that you are seeing.
> - The second call to allocateHeaders() in assignAddresses() sets the Min from the lowest SHF_ALLOC section address.
>  - As non SHF_ALLOC sections are ignored we can enter assignAddresses() with a different value of Min from the first call as I can do --section-start non_alloc_section=0x0 which isn't ignored for the first call, but is for the second. This means that the second call to assignAddresses() can in theory get to alignDown(Min - HeaderSize)
>  - As the SHF_ALLOC sections are given addresses from the SectionStartMap they should get the same address from the script so if HeaderSize < Min in the first call it should also be the case in the second.
> 
> Does this match anything that you are seeing? I think that there are some potential holes here, but I'm not confident that they are actually causing your problem. Would you be able to send some more information, and send a reproducer?
> 
> I'll pick this back up on Tuesday.
> 
> 
> Repository:
>  rL LLVM
> 
> https://reviews.llvm.org/D31888
> 
> 
>