[PATCH] D27200: [ELF] - Do not create 4gb output when -obinary -Ttext and -omagic used together.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/Writer.cpp:1130
+  // have file or program headers.
+  Phdr *Load = Config->OFormatBinary ? nullptr : AddHdr(PT_LOAD, Flags);
+  if (!ScriptConfig->HasSections && !Config->OFormatBinary) {
----------------
emaste wrote:
> ruiu wrote:
> > emaste wrote:
> > > Ah. Why do we add Load for the `!ScriptConfig->HasSections` case?
> > > 
> > > For me at least this seems to make it more clear:
> > > ```
> > > Phdr *Load = nullptr;
> > > if (!ScriptConfig->HasSections && !Config->OFormatBinary) {
> > >     Load = AddHdr(PT_LOAD, Flags);
> > >     Load->add(Out<ELFT>::ElfHeader);
> > >     Load->add(Out<ELFT>::ProgramHeaders);
> > > }
> > > ```
> > > 
> > Agreed. But furthermore, can we avoid calling createPhdrs() at all if OFormatBinary is true? If it's true, all data created in this function will be just ignored later.
> That would be even more clear :)
Well, if we will avoid calling createPhdrs() that means that we will be not able to pagealign .data/.text. 
Because page aligning as perfomed basing on loads logic (what is intentional):
```
  for (const Phdr &P : Phdrs)
    if (P.H.p_type == PT_LOAD)
      P.First->PageAlign = true;
```
As a result our output will be different from ld. 

Though looking on PR31196, it creates single load. Also I suppose binary output is used mostly with linkerscripts
to control internal layout and much probably do not need auto pagealign we provide for regular link. 
So far I suppose that should be probably OK.

But that also affects on file offsets calculation. Which is currently performed as:
```
  OutputSectionBase<ELFT> *First = Sec->FirstInPtLoad;
  // If two sections share the same PT_LOAD the file offset is calculated using
  // this formula: Off2 = Off1 + (VA2 - VA1).
  if (Sec == First)
    return alignTo(Off, Target->MaxPageSize, Sec->Addr);
  return First->Offset + Sec->Addr - First->Addr;
```
(Introduced in D25014).

where FirstInPtLoad  set from:

```
template <class ELFT> void PhdrEntry<ELFT>::add(OutputSectionBase *Sec) {
..
  if (H.p_type == PT_LOAD)
    Sec->FirstInPtLoad = First;
}
```
And I think we want to keep the effect from this logic.

So I guess technically we can stop calling createPhdrs(), but that involves much more changes than this patch do, do we want it ?



https://reviews.llvm.org/D27200