[PATCH] D44682: [ELF] Fix X86 & X86_64 PLT retpoline padding

[Andrew Ng via Phabricator via llvm-commits]

andrewng added a comment.

In https://reviews.llvm.org/D44682#1043455, @ruiu wrote:

> Instead of doing this, we should fill executable segments with trap instructions even if linker script is in use, shouldn't we?


The current behaviour is only to fill the last page of executable segments with trap instructions. So even in the non linker script case, there still could be zero rather than trap instruction padding in the PLT. I don't know how big an issue this really is, but it feels better to be padding with trap instructions.

In https://reviews.llvm.org/D44682#1043962, @espindola wrote:

> In https://reviews.llvm.org/D44682#1043455, @ruiu wrote:
>
> > Instead of doing this, we should fill executable segments with trap instructions even if linker script is in use, shouldn't we?
>
>
> The issue with linker scripts is that they force SingleRoRx. This causes text and data to be mixed.
>
> These tests should probably use --no-rosegment instead of linkerscripts.


I don't think that this issue is directly related to this behaviour of linker scripts.



================
Comment at: ELF/Arch/X86.cpp:184
     };
+    assert(sizeof(V) == PltHeaderSize);
     memcpy(Buf, V, sizeof(V));
----------------
ruiu wrote:
> I don't like to sprinkle too many asserts in code.
Would you like me to remove all the asserts or just this one?


https://reviews.llvm.org/D44682