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

[James Henderson via Phabricator via llvm-commits]

jhenderson marked 3 inline comments as done.
jhenderson added inline comments.


================
Comment at: ELF/LinkerScript.cpp:943
         return Cmd->Filler;
-  return 0;
+  return Optional<uint32_t>();
 }
----------------
ruiu wrote:
> Return `None`.
Thanks, I didn't know about that.


================
Comment at: ELF/OutputSections.cpp:239
 
+uint32_t OutputSection::getFill() {
+  // Determine what to fill gaps between InputSections with, as specified by the
----------------
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->"?


================
Comment at: ELF/OutputSections.cpp:257-264
+  if (!Sections.empty()) {
+    // Write any leading padding, in case the first input section does not
+    // appear at offset zero.
+    fill(Buf, Sections.front()->OutSecOff, FillValue);
+  } else {
+    // If the section has no input sections, fill it completely with the fill.
+    fill(Buf, this->Size, FillValue);
----------------
ruiu wrote:
> Not sure if we need to care about these edge cases. I wouldn't probably do anything for them to keep code simple.
How strong are you objections? The behaviour matches ld.bfd, gold, and what lld did before. In the example use case of using trap instructions to catch misjumps, we would want the entire executable section to be padded, not just the bits between InputSections.

That said, I can make a simpler change to get the same behaviour.


https://reviews.llvm.org/D30886