[PATCH] D49456: [AArch64] Support execute-only LOAD segments.

Wed Jul 18 14:35:04 PDT 2018

ivanlozano added a comment.

In https://reviews.llvm.org/D49456#1166818, @ruiu wrote:

> Is this for compatibility with an existing option, or is this a new option? If this is new, I think `-aarch64` prefix might be undesirable -- even though "execute only" pages are currently supported by AArch64 among major ISAs, I can imagine that other ISA could support it. So a more platform-neutral naming might be better.

Yea this is for a new option; I'll rename it when I update the patch, thanks!

In https://reviews.llvm.org/D49456#1166366, @peter.smith wrote:

> I've got a couple of comments surrounding what happens in the linker script case. As it stands the change above will do the right thing with the default section order as all the executable sections will be in the same OutputSection and the ro-data will be separate. A hand written linker script could easily be written that could mix ro-data and code and wouldn't be caught by the  Config->SingleRoRx check. What I suggest is to either not support the option when a linker script is used, or add a check in OutputSection::addSection() that can detect if ro-data and code is mixed in the same OutputSection. Other than that the implementation looks good to me.

My first thought was to tackle this via OutputSection::addSection(), however on closer inspection SingleRoRx is always enabled in the ELF/ScriptParser.cpp function ScriptParser::readSections when reading a linker script that includes sections. Given that, it may be your first suggestion (disabling execute-only when using a linker script with sections) is the appropriate solution. (In practice this is already happening because computeFlags returns early on SingleRoRx, but it'll be good to explicitly set the execute-only flag to false in ScriptParser::readSections). I'll give this some more thought, but this might be the approach that I take.

Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D49456