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

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

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.



================
Comment at: lib/MC/ELFObjectWriter.cpp:1121
+        !TextSection->hasData()) {
+      if (OWriter.TargetObjectWriter->getEMachine() == ELF::EM_ARM) {
+        TextSection->setFlags(TextSection->getFlags() | ELF::SHF_ARM_PURECODE);
----------------
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.



================
Comment at: lib/MC/MCObjectStreamer.cpp:499
   DF->getContents().append(Data.begin(), Data.end());
+  MCSection *Sec = getCurrentSectionOnly();
+  Sec->setHasData(true);
----------------
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.



https://reviews.llvm.org/D48792