[PATCH] D17817: [ELF] do not allow .bss to occupy the file space when producing relocatable output.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/Writer.cpp:1325
@@ -1324,1 +1324,3 @@
+      FileOff = alignTo(FileOff, Sec->getAlign());
     Sec->setFileOffset(FileOff);
+    if (Sec->getType() != SHT_NOBITS)
----------------
grimar wrote:
> grimar wrote:
> > We just do the same in assignAddresses():
> > 
> > 
> > ```
> >     if (Sec->getType() != SHT_NOBITS)
> >       FileOff = alignTo(FileOff, Align);
> >     Sec->setFileOffset(FileOff);
> >     if (Sec->getType() != SHT_NOBITS)
> >       FileOff += Sec->getSize();
> > ```
> And yes, I think we need to set the offset.
> Below is relocatable output from gold for the testcase.
> .bss has the Offset set.
> I am just not sure why we might not to do that ?
> 
> Section Headers:
>   [Nr] Name              Type             Address           Offset
>        Size              EntSize          Flags  Link  Info  Align
>   [ 0]                   NULL             0000000000000000  00000000
>        0000000000000000  0000000000000000           0     0     0
>   [ 1] .text             PROGBITS         0000000000000000  00000040
>        0000000000000001  0000000000000000  AX       0     0     4
>   [ 2] .bss              NOBITS           0000000000000000  00000041
>        0000000000500000  0000000000000000  WA       0     0     1
>   [ 3] .symtab           SYMTAB           0000000000000000  00000048
>        0000000000000030  0000000000000018           4     1     8
> 
That gold sets the file offset does not mean that we have to do the same. I believe that behavior is not implemented to gold intentionally, but just that it happened to be that behavior. For us,

  for (OutputSectionBase<ELFT> *Sec : OutputSections) {
    if (Sec->getType() == SHT_NOBITS)
      continue;
    FileOff = alignTo(FileOff, Sec->getAlign());
    Sec->setFileOffset(FileOff);
    FileOff += Sec->getSize();
  }

is more straightforward, no?


http://reviews.llvm.org/D17817