Take SHF_ARM_PURECODE into consideration when setting the program header flags

[• espindola]

Summary

Hi Peter,

Is this how SHF_ARM_PURECODE is supposed to work?

[ruiu]

ELF/OutputSections.cpp

49–50

Does this mean PURECODE is not readable but executable? (Just wondering)

[peter.smith]

Thanks for doing this. It is close, but I think that it will clear the PF_R flag too aggressively at the moment. In the comments I've put a link to a very short white paper and a link to the entry in Arm ELF which unfortunately uses an old name for the flag SHF_ARM_NOREAD, first introduced by Arm's proprietary compiler but later changed to SHF_ARM_PURECODE when implemented in gcc.

ELF/OutputSections.cpp

47

I think that this will clear PF_R if at least one InputSection within the OutputSection has SHF_ARM_PURECODE. The intention is to only clear PF_R if all the InputSections within the OutputSection have SHF_ARM_PURECODE. In other words the combination of flags (SHF_ALLOC, SHF_EXECINSTR, SHF_ARM_PURECODE), and (SHF_ALLOC, SHF_EXECINSTR) is (SHF_ALLOC, SHF_EXECINSTR). It is up to the user to write a linker script that collects together all their SHF_ARM_PURECODE in the same OutputSection. You might be able to alter addSection to guarantee that if the OutputSection has the flag all the input sections do.

It is unfortunate that the version of ELF for the Arm Architecture on the Arm Website still uses SHF_ARM_NOREAD (later changed to SHF_ARM_PURECODE). I've quoted from 4.3.3 http://infocenter.arm.com/help/topic/com.arm.doc.ihi0044f/IHI0044F_aaelf.pdf

4.3.3 Section Attribute Flags
...

Name	Value	Comment
SHF_ARM_NOREAD	0x20000000	The content of this section should not be read by program executor

If all the sections contained by a segment have the SHF_ARM_NOREAD section attribute set, the PF_R attribute
should be unset in the program header for the segment.

49–50

Yes. The original name for the concept was execute only. It is intended for the Microcontroller market for device manufacturers, primarily as a security feature that they could deploy with just a memory controller that could distinguish between execute and read.

https://community.arm.com/cfs-file/__key/telligent-evolution-components-attachments/01-1989-00-00-00-00-70-86/Whitepaper-_2D00_-Separating-instructions-and-data-with-PureCode.pdf

test/ELF/arm-execute-only.s

19

Will be good to have a tests for.

1 OutputSection with 1 InputSection with SHF_ARM_PURECODE (no PF_R)
1 OutputSection with > 1 InputSection all with SHF_ARM_PURECODE (no PF_R)
1 OutputSection with > 1 InputSection with some SHF_ARM_PURECODE and some not. (PF_R set)

The concept itself doesn't make a lot of sense outside of Microcontrollers, for example I don't know whether a DSO containing SHF_ARM_PURECODE could respect the flag, unless it were like RELRO, although for the purposes of testing the flag it probably doesn't matter that much.

[• espindola]

Correctly update SHF_ARM_PURECODE on the output sections.
Update testcase.

[peter.smith]

Thanks very much. Looks good to me.

[• espindola]

r326207

ELF/OutputSections.cpp

 OutputSection *Out::ElfHeader;
 OutputSection *Out::ProgramHeaders;
 OutputSection *Out::PreinitArray;
 OutputSection *Out::InitArray;
 OutputSection *Out::FiniArray;
 
 std::vector<OutputSection *> elf::OutputSections;
 
 uint32_t OutputSection::getPhdrFlags() const {
-  uint32_t Ret = PF_R;
+  uint32_t Ret = 0;
+  if (Config->EMachine != EM_ARM || !(Flags & SHF_ARM_PURECODE))
+    Ret |= PF_R;
   if (Flags & SHF_WRITE)
     Ret |= PF_W;
   if (Flags & SHF_EXECINSTR)
     Ret |= PF_X;
   return Ret;
 }
 
 template <class ELFT>
static bool canMergeToProgbits(unsigned Type) {
   return Type == SHT_NOBITS || Type == SHT_PROGBITS || Type == SHT_INIT_ARRAY ||
          Type == SHT_PREINIT_ARRAY || Type == SHT_FINI_ARRAY ||
          Type == SHT_NOTE;
 }
 
 void OutputSection::addSection(InputSection *IS) {
   if (!Live) {
     // If IS is the first section to be added to this section,
-    // initialize Type and Entsize from IS.
+    // initialize Type, Entsize and flags from IS.
     Live = true;
     Type = IS->Type;
     Entsize = IS->Entsize;
+    Flags = IS->Flags;
   } else {
     // Otherwise, check if new type or flags are compatible with existing ones.
     if ((Flags & (SHF_ALLOC | SHF_TLS)) != (IS->Flags & (SHF_ALLOC | SHF_TLS)))
       error("incompatible section flags for " + Name + "\n>>> " + toString(IS) +
             ": 0x" + utohexstr(IS->Flags) + "\n>>> output section " + Name +
             ": 0x" + utohexstr(Flags));
 
     if (Type != IS->Type) {
       if (!canMergeToProgbits(Type) || !canMergeToProgbits(IS->Type))
         error("section type mismatch for " + IS->Name + "\n>>> " +
               toString(IS) + ": " +
               getELFSectionTypeName(Config->EMachine, IS->Type) +
               "\n>>> output section " + Name + ": " +
               getELFSectionTypeName(Config->EMachine, Type));
       Type = SHT_PROGBITS;
     }
   }
 
   IS->Parent = this;
-  Flags |= IS->Flags;
+  uint64_t AndMask = Config->EMachine == EM_ARM ? SHF_ARM_PURECODE : 0;
+  uint64_t OrMask = ~AndMask;
+  uint64_t AndFlags = (Flags & IS->Flags) & AndMask;
+  uint64_t OrFlags = (Flags | IS->Flags) & OrMask;
+  Flags = AndFlags | OrFlags;
+
   Alignment = std::max(Alignment, IS->Alignment);
   IS->OutSecOff = Size++;
 
   // If this section contains a table of fixed-size entries, sh_entsize
   // holds the element size. If it contains elements of different size we
   // set sh_entsize to 0.
   if (Entsize != IS->Entsize)
     Entsize = 0;

test/ELF/arm-execute-only.s

+// RUN: llvm-mc -filetype=obj -triple=armv7-pc-linux %s -o %t.o
+// RUN: ld.lld %t.o -o %t.so -shared
+// RUN: llvm-readelf -l %t.so | FileCheck %s
+
+// RUN: ld.lld %t.o %t.o -o %t.so -shared
+// RUN: llvm-readelf -l %t.so | FileCheck %s
+
+// RUN: echo ".section .foo,\"ax\"; \
+// RUN:       bx lr" > %t.s
+// RUN: llvm-mc -filetype=obj -triple=armv7-pc-linux %t.s -o %t2.o
+// RUN: ld.lld %t.o %t2.o -o %t.so -shared
+// RUN: llvm-readelf -l %t.so | FileCheck --check-prefix=DIFF %s
+
+// CHECK-NOT:  LOAD
+// CHECK:      LOAD           0x000000 0x00000000 0x00000000 0x00169  0x00169  R   0x1000
+// CHECK:      LOAD           0x001000 0x00001000 0x00001000 0x{{.*}} 0x{{.*}} R E 0x1000
+// CHECK:      LOAD           0x002000 0x00002000 0x00002000 0x{{.*}} 0x{{.*}}   E 0x1000
+// CHECK:      LOAD           0x003000 0x00003000 0x00003000 0x00038  0x00038  RW  0x1000
+// CHECK-NOT:  LOAD
+
+// CHECK: 01     .dynsym .gnu.hash .hash .dynstr
+// CHECK: 02     .text
+// CHECK: 03     .foo
+// CHECK: 04     .dynamic
+
+// DIFF-NOT:  LOAD
+// DIFF:      LOAD           0x000000 0x00000000 0x00000000 0x00149 0x00149 R   0x1000
+// DIFF:      LOAD           0x001000 0x00001000 0x00001000 0x0000c 0x0000c R E 0x1000
+// DIFF:      LOAD           0x002000 0x00002000 0x00002000 0x00038 0x00038 RW  0x1000
+// DIFF-NOT:  LOAD
+
+// DIFF: 01     .dynsym .gnu.hash .hash .dynstr
+// DIFF: 02     .text .foo
+// DIFF: 03     .dynamic
+
+        bx lr
+        .section .foo,"axy"
+        bx lr