[PATCH] D17269: [ELF] - Implemented linkerscript sections padding.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/InputSection.cpp:250-261
@@ +249,14 @@
+
+  // If there is some padding behind and value for padding is
+  // not null, this one fills the space repeating 4 bytes of
+  // padding value.
+  if (PadBehind && OutSec->PaddingValue) {
+    llvm::support::ubig32_t Bytes(OutSec->PaddingValue);
+    int I = OutSecOff - PadBehind;
+    while (PadBehind) {
+      Buf[OutSecOff - PadBehind] =
+          ((uint8_t *)&Bytes)[(OutSecOff - PadBehind - I) % 4];
+      --PadBehind;
+    }
+  }
+
----------------
ruiu wrote:
> This is not a right place to add this code.  You want to add code to OutputSection<ELFT>::writeTo instead. I think the following code should suffice as an initial implementation.
> 
>   +static void fill(uint8_t *Buf, size_t Size, ArrayRef<uint8_t> A) {
>   +  size_t I = 0;
>   +  for (; I < Size; I += A.size())
>   +    memcpy(Buf + I, A.data(), A.size());
>   +  memcpy(Buf + I, A.data(), Size - I);
>   +}
>   +
>    template <class ELFT> void OutputSection<ELFT>::writeTo(uint8_t *Buf) {
>   +  ArrayRef<uint8_t> Filler = Script->getFiller(Name);
>   +  if (!Filler.empty())
>   +    fill(Buf, getSize(), Filler);
>      for (InputSection<ELFT> *C : Sections)
>        C->writeTo(Buf);
>    }
Your code mixes the filling byters order it seems. I mean if we have two sections, 1 byte each with alignment 4, then if we have =0x11223344 filler:

My code will produce: [SEC1] 0x11 0x22 0x33 [SEC2].
Your one will have [SEC1] 0x22 0x33 0x44 [SEC2].

I am not sure how much that is important as I saw fillers are used for NOPs which always were 0x90909090. If we are assuming that that is not important at all, then instead of fill() call we can use 

```
memset(Buf, Filler[0], getSize());
```

But if we want to support the order (and use all bytes), then I will adopt the algorithm. 
Anyways I dont think we should use the fill() as it is now since it is somewhere in the middle.
What do you think ?


http://reviews.llvm.org/D17269