[PATCH] D44943: Fill the last page of each executable section with 0xcc or equivalent.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/ELF/Writer.cpp:2209
+      continue;
+    if ((Sec->Offset + Sec->Size) % Target->PageSize == 0)
+      continue;
----------------
grimar wrote:
> So, should this check be removed for simplicity of code?
> (basing on comments, it makes no sense to have it, right?)
> 
> Then, btw, code could be:
> 
> ```
>   for (OutputSection *Sec : OutputSections)
>     if (Sec->PtLoad && Sec->PtLoad == PT_LOAD && (Sec->PtLoad->p_flags & PF_X))
>       fillTrap(Buf + alignDown(Sec->Offset + Sec->Size, Target->PageSize));
> ```
No it can't. If a section ends exactly at a page boundary, no padding is needed.


================
Comment at: lld/ELF/Writer.cpp:2211
+      continue;
+    fillTrap(Buf + alignDown(Sec->Offset + Sec->Size, Target->PageSize));
+  }
----------------
espindola wrote:
> ruiu wrote:
> > espindola wrote:
> > > Iterating over OutputSections seems like a good idea, but couldn't this fill just the area between two sections?
> > We can. The reason why I'm doing this is because this way, trap instructions are always 4 byte aligned even if the last segment is not 4 byte aligned. But I can fix it some way.
> Interesting. We have the same latent bug in OutputSection::writeTo. I guess we have to add nop for ppc (where not all bytes are the same) to test a fix.
> 
I thought about how to fill only gaps between sections, but there seems no easy way of doing it. It's doable , but it's perhaps not worth writing more code for it. So I think I'd prefer the current code.


https://reviews.llvm.org/D44943