[PATCH] D48792: [ARM] Set execute-only flags in .text.

[Ivan Lozano via Phabricator via llvm-commits]

ivanlozano added a comment.

In https://reviews.llvm.org/D48792#1217179, @peter.smith wrote:

> I've added a couple of comments. One other thing to think about is the -S output from the compiler. If I compile a trivial test case with -mexecute-only I get the output:
>
>   	.text
>   	.syntax unified
>   	.eabi_attribute	67, "2.09"	@ Tag_conformance
>   ...
>   	.section	.text,"axy",%progbits,unique,0
>   	.globl	func                    @ -- Begin function func
>   ...
>
>
> I'm not sure how easy it will be to change .text to .section .text, "axy", %progbits. In theory if this is assembled with MC and this patch it should come out with execute only (may be worth a test case). Re-assembling with GNU as will likely create a .text section without SHF_ARM_PURECODE but I don't think that this isn't likely enough in practice to worry too much about.


I tested this with llc and Clang, the output section in both cases has the flag set correctly:

  .section        .text,"axy",%progbits,unique,0

I'll go ahead and add a test for this.



================
Comment at: lib/MC/ELFObjectWriter.cpp:1121
+        !TextSection->hasData()) {
+      if (OWriter.TargetObjectWriter->getEMachine() == ELF::EM_ARM) {
+        TextSection->setFlags(TextSection->getFlags() | ELF::SHF_ARM_PURECODE);
----------------
peter.smith wrote:
> If the TextSection is completely empty it should have 0 fragments going into MCAssembler::layout(). The layout function then creates a single empty MCDataFragment. I'm thinking it might be more precise and reliable to set a flag like HasNoUserFragments in layout(). That flag can be tested for here.
> 
This seems reasonable, I'll put this together and see that it works as expected.


================
Comment at: lib/MC/MCObjectStreamer.cpp:499
   DF->getContents().append(Data.begin(), Data.end());
+  MCSection *Sec = getCurrentSectionOnly();
+  Sec->setHasData(true);
----------------
peter.smith wrote:
> I think that this would catch the majority of cases where data is written but could we be sure it will catch all of them. For example it is possible to write something like:
> 
> ```
> .text
> .balign 32
> ```
> That will create an MCAlignFragment that doesn't go via emitBytes(). Although one would expect that the alignment padding here wouldn't be read I guess we couldn't be 100% sure.
> 
Yea I'm also not sure if there isn't somewhere else this needs to be set. However at least in the .balign case it is emitting the appropriate section flags when I test it. The resulting .text in the object file is still size 0 and marked execute-only.

I think .balign only pads if the size isn't already a multiple of N, and .align and .p2align function similarly.


https://reviews.llvm.org/D48792