[PATCH] D30886: [ELF] Pad x86 executable sections with 0xcc int3 instructions

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: ELF/OutputSections.cpp:239
 
+uint32_t OutputSection::getFill() {
+  // Determine what to fill gaps between InputSections with, as specified by the
----------------
ruiu wrote:
> jhenderson wrote:
> > ruiu wrote:
> > > This function doesn't use any member variable of OutputSection, so it should be a non-member function.
> > It does use two member variables - Name, and Flags, though they could relatively easily be passed in, if you prefer not to add the member-function.
> > 
> > By the way, is there a preferred policy on explicitly qualifying member variables with "this->"?
> Sometimes you need to explicitly write `this->` for a template (http://stackoverflow.com/questions/4643074/why-do-i-have-to-access-template-base-class-members-through-the-this-pointer). We write `this->` if that's the case.
Ah, I think I know why I was confused now - some places that were formally templated are no longer, so presumably they had this-> before, but no longer needed it.


================
Comment at: ELF/OutputSections.cpp:257
+  // Write leading padding.
+  size_t FillSize = Sections.empty() ? Size : Sections.front()->OutSecOff;
+  fill(Buf, FillSize, FillValue);
----------------
ruiu wrote:
> Can `Sections` be empty?
Yes, if a linker script assigns to "." or uses a BYTE()-style command within an output section, but no inputs have sections that go in that output section. I'd write a test, but PR32537 makes it quite hard to do so.


https://reviews.llvm.org/D30886